infrastructure/extensions-website
4
8
Fork 10
Code Issues 20 Pull Requests 2 Projects Releases Wiki Activity

Refactor approval queue to display last meaningful status #94

Merged
Oleg-Komarov merged 13 commits from approval-queue into main 2024-04-30 10:48:38 +02:00
Conversation 1 Commits 13 Files Changed 5 +80 -43
5 changed files with 80 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
reviewers/templates/reviewers/components/review_list_item.html
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="ml-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="ml-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 suspicious_files=extension.suspicious_files %}
</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>

3
reviewers/templates/reviewers/extensions_review_detail.html
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>

10
reviewers/templates/reviewers/extensions_review_list.html
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>

10
reviewers/tests/test_views.py
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):

78
reviewers/views.py
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(
Anna Sirota commented 2024-04-29 19:24:11 +02:00
Review
Copy Link

For already existing extensions without activity records ApprovalActivity will have to be created (with the Extension.date_status_changed dates as date_created) to ensure they still show up in the approval queue.

For already existing extensions without activity records `ApprovalActivity` will have to be created (with the `Extension.date_status_changed` dates as `date_created`) to ensure they still show up in the approval queue.
Oleg-Komarov commented 2024-04-30 10:48:12 +02:00
Review
Copy Link

running in shell

from extensions.models import Extension
from reviewers.models import ApprovalActivity

to_create = []
for ext in Extension.objects.filter(status=2).all():
    if ext.review_activity.count() == 0:
        to_create.append(ApprovalActivity(
            type=ApprovalActivity.ActivityType.AWAITING_REVIEW,
            extension=ext,
            user=ext.authors.all()[0],
            date_created=ext.date_status_changed,
            message='Extension is ready for initial review',
        ))

ApprovalActivity.objects.bulk_create(to_create)

for at in to_create:
    at.date_created = at.extension.date_status_changed

ApprovalActivity.objects.bulk_update(to_create, ['date_created'])
running in shell ``` from extensions.models import Extension from reviewers.models import ApprovalActivity to_create = [] for ext in Extension.objects.filter(status=2).all(): if ext.review_activity.count() == 0: to_create.append(ApprovalActivity( type=ApprovalActivity.ActivityType.AWAITING_REVIEW, extension=ext, user=ext.authors.all()[0], date_created=ext.date_status_changed, message='Extension is ready for initial review', )) ApprovalActivity.objects.bulk_create(to_create) for at in to_create: at.date_created = at.extension.date_status_changed ApprovalActivity.objects.bulk_update(to_create, ['date_created']) ```
'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,33 +72,31 @@ 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 form.fields['type'].widget = django.forms.HiddenInput()
if len(filtered_activity_types) == 1:
form.fields['type'].widget = django.forms.HiddenInput()
return ctx return ctx