UI: Web Assets v2 upgrade #85

Merged
Márton Lente merged 52 commits from martonlente/extensions-website:ui/web-assets-v2-upgrade into main 2024-04-30 17:57:33 +02:00
20 changed files with 198 additions and 82 deletions
Showing only changes of commit 1e5f401b1f - Show all commits

View File

@ -10,6 +10,7 @@ class Verb:
REPORTED_RATING = 'reported rating' REPORTED_RATING = 'reported rating'
REQUESTED_CHANGES = 'requested changes' REQUESTED_CHANGES = 'requested changes'
REQUESTED_REVIEW = 'requested review' REQUESTED_REVIEW = 'requested review'
UPLOADED_NEW_VERSION = 'uploaded new version'
class Flag: class Flag:

View File

@ -353,7 +353,7 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
"""Return True if given user is listed as a maintainer.""" """Return True if given user is listed as a maintainer."""
if user is None or user.is_anonymous: if user is None or user.is_anonymous:
return False return False
return self.authors.filter(maintainer__user_id=user.pk).exists() return user in self.authors.all()
def can_rate(self, user) -> bool: def can_rate(self, user) -> bool:
"""Return True if given user can rate this extension. """Return True if given user can rate this extension.
@ -368,6 +368,10 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
).exists() ).exists()
) )
def suspicious_files(self):
versions = self.versions.all()
return [v.file for v in versions if not v.file.validation.is_ok]
@classmethod @classmethod
def get_lookup_field(cls, identifier): def get_lookup_field(cls, identifier):
lookup_field = 'pk' lookup_field = 'pk'

View File

@ -8,6 +8,7 @@ from django.db.models.signals import m2m_changed, pre_save, post_save, pre_delet
from django.dispatch import receiver from django.dispatch import receiver
from constants.activity import Flag from constants.activity import Flag
from reviewers.models import ApprovalActivity
import extensions.models import extensions.models
import files.models import files.models
@ -165,3 +166,26 @@ def _auto_approve_subsequent_uploads(
args = {'f_id': file.pk, 'pk': instance.pk, 'sender': sender, 's': file.source.name} args = {'f_id': file.pk, 'pk': instance.pk, 'sender': sender, 's': file.source.name}
logger.info('Auto-approving file pk=%(f_id)s of %(sender)s pk=%(pk)s source=%(s)s', args) logger.info('Auto-approving file pk=%(f_id)s of %(sender)s pk=%(pk)s source=%(s)s', args)
file.save(update_fields={'status', 'date_modified'}) file.save(update_fields={'status', 'date_modified'})
@receiver(post_save, sender=extensions.models.Version)
def _create_approval_activity_for_new_version_if_listed(
sender: object,
instance: extensions.models.Version,
created: bool,
raw: bool,
**kwargs: object,
):
if raw:
return
if not created:
return
extension = instance.extension
if not extension.is_listed or not instance.file:
return
ApprovalActivity(
type=ApprovalActivity.ActivityType.UPLOADED_NEW_VERSION,
user=instance.file.user,
extension=instance.extension,
message=f'uploaded new version: {instance.version}',
).save()

View File

@ -5,11 +5,7 @@
{% block page_title %}{{ extension.name }}{% endblock page_title %} {% block page_title %}{{ extension.name }}{% endblock page_title %}
{% block content %} {% block content %}
{% if extension.latest_version %} {% include "files/components/scan_details.html" with suspicious_files=extension.suspicious_files %}
{% with latest=extension.latest_version %}
{% include "files/components/scan_details.html" with file=latest.file %}
{% endwith %}
{% endif %}
{% has_maintainer extension as is_maintainer %} {% has_maintainer extension as is_maintainer %}
{% with latest=extension.latest_version %} {% with latest=extension.latest_version %}
@ -57,7 +53,7 @@
{% if extension.versions.listed|length > 1 %} {% if extension.versions.listed|length > 1 %}
<p> <p>
<a href="{{ extension.get_versions_url }}" class="d-block mt-3"> <a href="{{ extension.get_versions_url }}" class="d-block mt-3">
See all changelogs See all versions
</a> </a>
</p> </p>
{% endif %} {% endif %}

View File

@ -12,13 +12,11 @@
<h3>Tags</h3> <h3>Tags</h3>
<ul> <ul>
{% for list_tag in tags %} {% for list_tag in tags %}
{% if list_tag.versions.all|length %}
<li class="{% if tag == list_tag %}is-active{% endif %}"> <li class="{% if tag == list_tag %}is-active{% endif %}">
<a href="{% url "extensions:by-tag" tag_slug=list_tag.slug %}" title="{{ list_tag.name }}"> <a href="{% url "extensions:by-tag" tag_slug=list_tag.slug %}" title="{{ list_tag.name }}">
{{ list_tag.name }} {{ list_tag.name }}
</a> </a>
</li> </li>
{% endif %}
{% endfor %} {% endfor %}
</ul> </ul>
</div> </div>

View File

@ -10,6 +10,7 @@ from common.tests.factories.users import UserFactory
from common.tests.utils import _get_all_form_errors from common.tests.utils import _get_all_form_errors
from extensions.models import Extension, Version from extensions.models import Extension, Version
from files.models import File from files.models import File
from reviewers.models import ApprovalActivity
import utils import utils
@ -425,6 +426,14 @@ class NewVersionTest(TestCase):
f'/add-ons/{self.extension.slug}/manage/versions/new/{file.pk}/', f'/add-ons/{self.extension.slug}/manage/versions/new/{file.pk}/',
) )
self.assertEqual(self.extension.versions.count(), 1) self.assertEqual(self.extension.versions.count(), 1)
self.extension.approve()
self.assertEqual(
ApprovalActivity.objects.filter(
extension=self.extension,
type=ApprovalActivity.ActivityType.UPLOADED_NEW_VERSION,
).count(),
0,
)
# Check step 2: finalise new version and send to review # Check step 2: finalise new version and send to review
url = response['Location'] url = response['Location']
@ -444,3 +453,10 @@ class NewVersionTest(TestCase):
self.assertEqual(new_version.schema_version, '1.0.0') self.assertEqual(new_version.schema_version, '1.0.0')
self.assertEqual(new_version.release_notes, 'new version') self.assertEqual(new_version.release_notes, 'new version')
self.assertEqual(new_version.file.get_status_display(), 'Approved') self.assertEqual(new_version.file.get_status_display(), 'Approved')
self.assertEqual(
ApprovalActivity.objects.filter(
extension=self.extension,
type=ApprovalActivity.ActivityType.UPLOADED_NEW_VERSION,
).count(),
1,
)

View File

@ -4,6 +4,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin
from django.contrib.messages.views import SuccessMessageMixin from django.contrib.messages.views import SuccessMessageMixin
from django.db import transaction from django.db import transaction
from django.shortcuts import get_object_or_404, reverse from django.shortcuts import get_object_or_404, reverse
from django.utils.translation import gettext_lazy as _
from django.views.generic import DetailView, ListView from django.views.generic import DetailView, ListView
from django.views.generic.edit import CreateView, UpdateView, DeleteView, FormView from django.views.generic.edit import CreateView, UpdateView, DeleteView, FormView
@ -42,7 +43,12 @@ class ExtensionDetailView(ExtensionQuerysetMixin, DetailView):
* maintainers should be able to preview their yet unlisted add-ons; * maintainers should be able to preview their yet unlisted add-ons;
* staff should be able to preview yet unlisted add-ons; * staff should be able to preview yet unlisted add-ons;
""" """
return self.get_extension_queryset() return self.get_extension_queryset().prefetch_related(
'authors',
'versions',
'versions__file',
'versions__file__validation',
)
def get_object(self, queryset=None): def get_object(self, queryset=None):
"""Record a page view when returning the Extension object.""" """Record a page view when returning the Extension object."""
@ -353,6 +359,7 @@ class DraftExtensionView(
): ):
template_name = 'extensions/draft_finalise.html' template_name = 'extensions/draft_finalise.html'
form_class = VersionForm form_class = VersionForm
msg_awaiting_review = _('Extension is ready for initial review')
@property @property
def success_message(self) -> str: def success_message(self) -> str:
@ -418,7 +425,7 @@ class DraftExtensionView(
user=self.request.user, user=self.request.user,
extension=extension_form.instance, extension=extension_form.instance,
type=ApprovalActivity.ActivityType.AWAITING_REVIEW, type=ApprovalActivity.ActivityType.AWAITING_REVIEW,
message="initial submission", message=self.msg_awaiting_review,
).save() ).save()
return super().form_valid(form) return super().form_valid(form)
except forms.ValidationError as e: except forms.ValidationError as e:

View File

@ -66,6 +66,14 @@ class DraftMixin:
"""If the extension is incomplete, returns the FinalizeDraftView""" """If the extension is incomplete, returns the FinalizeDraftView"""
def dispatch(self, request, *args, **kwargs): def dispatch(self, request, *args, **kwargs):
if (
'slug' in kwargs
and Extension.objects.filter(
slug=kwargs['slug'], status=Extension.STATUSES.APPROVED
).first()
):
return super().dispatch(request, *args, **kwargs)
extension = ( extension = (
Extension.objects.listed_or_authored_by(user_id=self.request.user.pk) Extension.objects.listed_or_authored_by(user_id=self.request.user.pk)
.filter(status=Extension.STATUSES.INCOMPLETE) .filter(status=Extension.STATUSES.INCOMPLETE)

View File

@ -104,7 +104,15 @@ class SearchView(ListedExtensionsView):
| Q(versions__tags__name__icontains=token) | Q(versions__tags__name__icontains=token)
) )
queryset = queryset.filter(search_query).distinct() queryset = queryset.filter(search_query).distinct()
return queryset return queryset.prefetch_related(
'authors',
'preview_set',
'preview_set__file',
'ratings',
'versions',
'versions__file',
'versions__tags',
)
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs) context = super().get_context_data(**kwargs)
@ -121,9 +129,9 @@ class SearchView(ListedExtensionsView):
# Determine which tags to list depending on the context. # Determine which tags to list depending on the context.
if context.get('type'): if context.get('type'):
tag_type_id = self._get_type_id_by_slug() tag_type_id = self._get_type_id_by_slug()
context['tags'] = Tag.objects.filter(type=tag_type_id) context['tags'] = Tag.objects.filter(type=tag_type_id).exclude(versions=None)
elif context.get('tag'): elif context.get('tag'):
tag_type_id = context['tag'].type tag_type_id = context['tag'].type
context['tags'] = Tag.objects.filter(type=tag_type_id) context['tags'] = Tag.objects.filter(type=tag_type_id).exclude(versions=None)
return context return context

View File

@ -1,21 +1,19 @@
{% load common i18n %} {% load common i18n %}
{# FIXME: we might want to rephrase is_moderator in terms of Django's (group) permissions #} {# FIXME: we might want to rephrase is_moderator in terms of Django's (group) permissions #}
{% if perms.files.view_file or request.user.is_moderator %} {% if perms.files.view_file or request.user.is_moderator %}
{% with file_validation=file.validation %} {% if suspicious_files %}
{% if file_validation and not file_validation.is_ok %}
<section> <section>
<div class="card pb-3 pt-4 px-4 mb-3 ext-detail-download-danger"> <div class="card pb-3 pt-4 px-4 mb-3 ext-detail-download-danger">
<h3>&nbsp;{% trans "Suspicious upload" %}</h3> <h3>&nbsp;{% trans "Suspicious upload" %}</h3>
{% blocktrans asvar alert_text %}Scan of the {{ file }} indicates malicious content.{% endblocktrans %} {% blocktrans asvar alert_text %}Scan of the {{ suspicious_files.0 }} indicates malicious content.{% endblocktrans %}
<h4> <h4>
{{ alert_text }} {{ alert_text }}
{% if perms.files.view_file %}{# Moderators don't necessarily have access to the admin #} {% if perms.files.view_file %}{# Moderators don't necessarily have access to the admin #}
{% url 'admin:files_file_change' file.pk as admin_file_url %} {% url 'admin:files_file_change' suspicious_files.0.pk as admin_file_url %}
<a href="{{ admin_file_url }}" target="_blank">{% trans "See details" %}</a> <a href="{{ admin_file_url }}" target="_blank">{% trans "See details" %}</a>
{% endif %} {% endif %}
</h4> </h4>
</div> </div>
</section> </section>
{% endif %} {% endif %}
{% endwith %}
{% endif %} {% endif %}

View File

@ -1,10 +1,8 @@
{% load common i18n %} {% load common i18n %}
{# FIXME: we might want to rephrase is_moderator in terms of Django's (group) permissions #} {# FIXME: we might want to rephrase is_moderator in terms of Django's (group) permissions #}
{% if perms.files.view_file or request.user.is_moderator %} {% if perms.files.view_file or request.user.is_moderator %}
{% with file_validation=file.validation %} {% if suspicious_files %}
{% if file_validation and not file_validation.is_ok %} {% blocktrans asvar alert_text %}Scan of the {{ suspicious_files.0 }} indicates malicious content.{% endblocktrans %}
{% blocktrans asvar alert_text %}Scan of the {{ file }} indicates malicious content.{% endblocktrans %}
<b class="text-danger pt-2" title="{{ alert_text }}"></b> <b class="text-danger pt-2" title="{{ alert_text }}"></b>
{% endif %} {% endif %}
{% endwith %}
{% endif %} {% endif %}

View File

@ -18,6 +18,7 @@ VERB2FLAGS = {
Verb.REPORTED_RATING: [Flag.MODERATOR], Verb.REPORTED_RATING: [Flag.MODERATOR],
Verb.REQUESTED_CHANGES: [Flag.AUTHOR, Flag.REVIEWER], Verb.REQUESTED_CHANGES: [Flag.AUTHOR, Flag.REVIEWER],
Verb.REQUESTED_REVIEW: [Flag.MODERATOR, Flag.REVIEWER], Verb.REQUESTED_REVIEW: [Flag.MODERATOR, Flag.REVIEWER],
Verb.UPLOADED_NEW_VERSION: [],
} }
@ -41,7 +42,7 @@ def _create_notifications(
notifications = [] notifications = []
flags = VERB2FLAGS.get(instance.verb, None) flags = VERB2FLAGS.get(instance.verb, None)
if not flags: if flags is None:
logger.warning(f'no follower flags for verb={instance.verb}, nobody will be notified') logger.warning(f'no follower flags for verb={instance.verb}, nobody will be notified')
return return

View File

@ -0,0 +1,18 @@
# Generated by Django 4.2.11 on 2024-04-29 17:33
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('reviewers', '0008_alter_approvalactivity_message'),
]
operations = [
migrations.AlterField(
model_name='approvalactivity',
name='type',
field=models.CharField(choices=[('COM', 'Comment'), ('APR', 'Approved'), ('AWC', 'Awaiting Changes'), ('AWR', 'Awaiting Review'), ('UNV', 'Uploaded New Version')], default='COM', max_length=3),
),
]

View File

@ -80,6 +80,7 @@ class ApprovalActivity(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
APPROVED = "APR", _("Approved") APPROVED = "APR", _("Approved")
AWAITING_CHANGES = "AWC", _("Awaiting Changes") AWAITING_CHANGES = "AWC", _("Awaiting Changes")
AWAITING_REVIEW = "AWR", _("Awaiting Review") AWAITING_REVIEW = "AWR", _("Awaiting Review")
UPLOADED_NEW_VERSION = "UNV", _("Uploaded New Version")
user = models.ForeignKey(User, on_delete=models.CASCADE, blank=True, null=True) user = models.ForeignKey(User, on_delete=models.CASCADE, blank=True, null=True)
extension = models.ForeignKey( extension = models.ForeignKey(

View File

@ -30,6 +30,7 @@ def _create_action_from_review_and_follow(
ApprovalActivity.ActivityType.AWAITING_CHANGES: Verb.REQUESTED_CHANGES, ApprovalActivity.ActivityType.AWAITING_CHANGES: Verb.REQUESTED_CHANGES,
ApprovalActivity.ActivityType.AWAITING_REVIEW: Verb.REQUESTED_REVIEW, ApprovalActivity.ActivityType.AWAITING_REVIEW: Verb.REQUESTED_REVIEW,
ApprovalActivity.ActivityType.COMMENT: Verb.COMMENTED, ApprovalActivity.ActivityType.COMMENT: Verb.COMMENTED,
ApprovalActivity.ActivityType.UPLOADED_NEW_VERSION: Verb.UPLOADED_NEW_VERSION,
} }
action.send( action.send(
instance.user, instance.user,

View File

@ -6,27 +6,25 @@
{{ extension.name }} {{ extension.name }}
</a> </a>
</td> </td>
<td> <td>{% include "extensions/components/authors.html" %}</td>
{% if extension.authors.count %}
{% include "extensions/components/authors.html" %}
{% endif %}
</td>
<td title="{{ extension.date_created }}">{{ extension.date_created|naturaltime_compact }}</td> <td title="{{ extension.date_created }}">{{ extension.date_created|naturaltime_compact }}</td>
<td class="d-flex"> <td class="d-flex">
<a href="{{ extension.get_review_url }}#activity"> <a href="{{ extension.get_review_url }}#activity">
<span>{{ extension.review_activity.all|length }}</span> <span>{{ stats.count }}</span>
</a> </a>
<a href="{{ extension.get_review_url }}#activity-{{ stats.last_activity.id }}" class="ms-3">
{% if extension.review_activity.all %} <span>{{ stats.last_activity.date_created|naturaltime_compact }}</span>
<a href="{{ extension.get_review_url }}#activity-{{ extension.review_activity.all.last.id }}" class="ms-3">
<span>{{ extension.review_activity.all.last.date_created|naturaltime_compact }}</span>
</a> </a>
{% endif %} {% include "files/components/scan_details_flag.html" with suspicious_files=extension.suspicious_files %}
{% include "files/components/scan_details_flag.html" with file=extension.latest_version.file %}
</td> </td>
<td> <td>
<a href="{{ extension.get_review_url }}" class="text-decoration-none"> <a href="{{ extension.get_review_url }}" class="text-decoration-none">
{% include "common/components/status.html" with object=extension class="d-block" %} {% with last_type=stats.last_type_display|default:"Awaiting Review" %}
<div class="d-block badge badge-status-{{ last_type|slugify }}">
<i class="i-eye"></i>
<span>{{ last_type }}</span>
</div>
{% endwith %}
</a> </a>
</td> </td>
</tr> </tr>

View File

@ -101,8 +101,7 @@
{% for activity in extension.review_activity.all %} {% for activity in extension.review_activity.all %}
<li id="activity-{{ activity.id }}"> <li id="activity-{{ activity.id }}">
{# All activities except comments. #} {% if activity.type in status_change_types %}
{% if activity.type != 'COM' %}
<div class="activity-item activity-status-change activity-status-{{ activity.get_type_display|slugify }}"> <div class="activity-item activity-status-change activity-status-{{ activity.get_type_display|slugify }}">
<i class="activity-icon i-activity-{{ activity.get_type_display|slugify }}"></i> <i class="activity-icon i-activity-{{ activity.get_type_display|slugify }}"></i>

View File

@ -34,12 +34,10 @@
</tr> </tr>
</thead> </thead>
<tbody> <tbody>
{% for extension in object_list %} {% for stats in object_list %}
{% if user.is_moderator %} {% with extension=stats.extension %}
{% include 'reviewers/components/review_list_item.html' %} {% include 'reviewers/components/review_list_item.html' with extension=extension stats=stats %}
{% elif extension.status_slug == 'awaiting-review' %} {% endwith %}
{% include 'reviewers/components/review_list_item.html' %}
{% endif %}
{% endfor %} {% endfor %}
</tbody> </tbody>
</table> </table>

View File

@ -3,13 +3,21 @@ from django.shortcuts import reverse
from common.tests.factories.extensions import create_version from common.tests.factories.extensions import create_version
from files.models import File from files.models import File
from reviewers.models import ApprovalActivity
class CommentsViewTest(TestCase): class CommentsViewTest(TestCase):
fixtures = ['licenses'] fixtures = ['licenses']
def setUp(self): def setUp(self):
self.default_version = create_version(file__status=File.STATUSES.AWAITING_REVIEW) version = create_version(file__status=File.STATUSES.AWAITING_REVIEW)
self.default_version = version
ApprovalActivity(
type=ApprovalActivity.ActivityType.COMMENT,
user=version.file.user,
extension=version.extension,
message='test comment',
).save()
# List of extensions under review does not require authentication # List of extensions under review does not require authentication
def test_list_visibility(self): def test_list_visibility(self):

View File

@ -13,15 +13,51 @@ from reviewers.models import ApprovalActivity
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
STATUS_CHANGE_TYPES = [
ApprovalActivity.ActivityType.APPROVED,
ApprovalActivity.ActivityType.AWAITING_CHANGES,
ApprovalActivity.ActivityType.AWAITING_REVIEW,
]
class ApprovalQueueView(ListView): class ApprovalQueueView(ListView):
model = Extension model = Extension
paginate_by = 100 paginate_by = 100
def get_queryset(self): def get_queryset(self):
return Extension.objects.exclude(status=Extension.STATUSES.APPROVED).order_by( qs = (
'-date_created' ApprovalActivity.objects.prefetch_related(
'extension',
'extension__authors',
'extension__versions',
'extension__versions__file',
'extension__versions__file__validation',
) )
.order_by('-date_created')
.all()
)
by_extension = {}
result = []
for item in qs:
extension = item.extension
stats = by_extension.get(extension, None)
if not stats:
# this check guarantees that we add a record only once per extension,
# and iterating over qs we get result also ordered by item.date_created
stats = {
'count': 0,
'extension': extension,
'last_activity': None,
'last_type_display': None,
}
by_extension[extension] = stats
result.append(stats)
stats['count'] += 1
if not stats.get('last_activity', None):
stats['last_activity'] = item
if not stats.get('last_type_display', None) and item.type in STATUS_CHANGE_TYPES:
stats['last_type_display'] = item.get_type_display
return result
template_name = 'reviewers/extensions_review_list.html' template_name = 'reviewers/extensions_review_list.html'
@ -36,32 +72,30 @@ class ExtensionsApprovalDetailView(DetailView):
ctx['pending_previews'] = self.object.preview_set.exclude( ctx['pending_previews'] = self.object.preview_set.exclude(
file__status=File.STATUSES.APPROVED file__status=File.STATUSES.APPROVED
) )
ctx['status_change_types'] = STATUS_CHANGE_TYPES
if self.request.user.is_authenticated: if self.request.user.is_authenticated:
form = ctx['comment_form'] = CommentForm() form = ctx['comment_form'] = CommentForm()
# Remove 'Approved' status from dropdown it not moderator # anyone can comment
filtered_activity_types = ApprovalActivity.ActivityType.choices filtered_activity_types = {ApprovalActivity.ActivityType.COMMENT}
user = self.request.user user = self.request.user
if not (user.is_moderator or user.is_superuser): if self.object.has_maintainer(user):
filtered_activity_types = [ filtered_activity_types.add(ApprovalActivity.ActivityType.AWAITING_REVIEW)
t if user.is_moderator or user.is_superuser:
for t in ApprovalActivity.ActivityType.choices filtered_activity_types.update(
if t[0] [
not in [
ApprovalActivity.ActivityType.APPROVED, ApprovalActivity.ActivityType.APPROVED,
ApprovalActivity.ActivityType.AWAITING_CHANGES, ApprovalActivity.ActivityType.AWAITING_CHANGES,
] ]
] )
if not self.object.has_maintainer(user): choices = list(
# Other accounts can only comment filter(
filtered_activity_types = [ lambda c: c[0] in filtered_activity_types, ApprovalActivity.ActivityType.choices
t )
for t in ApprovalActivity.ActivityType.choices )
if t[0] == ApprovalActivity.ActivityType.COMMENT form.fields['type'].choices = choices
] form.fields['type'].widget.choices = choices
form.fields['type'].choices = filtered_activity_types if len(choices) == 1:
form.fields['type'].widget.choices = filtered_activity_types
if len(filtered_activity_types) == 1:
form.fields['type'].widget = django.forms.HiddenInput() form.fields['type'].widget = django.forms.HiddenInput()
return ctx return ctx