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
5 changed files with 80 additions and 43 deletions

View File

@ -6,27 +6,25 @@
{{ extension.name }}
</a>
</td>
<td>
{% if extension.authors.count %}
{% include "extensions/components/authors.html" %}
{% endif %}
</td>
<td>{% include "extensions/components/authors.html" %}</td>
<td title="{{ extension.date_created }}">{{ extension.date_created|naturaltime_compact }}</td>
<td class="d-flex">
<a href="{{ extension.get_review_url }}#activity">
<span>{{ extension.review_activity.all|length }}</span>
<span>{{ stats.count }}</span>
</a>
{% if extension.review_activity.all %}
<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 href="{{ extension.get_review_url }}#activity-{{ stats.last_activity.id }}" class="ml-3">
<span>{{ stats.last_activity.date_created|naturaltime_compact }}</span>
</a>
{% endif %}
{% include "files/components/scan_details_flag.html" with suspicious_files=extension.suspicious_files %}
</td>
<td>
<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>
</td>
</tr>

View File

@ -101,8 +101,7 @@
{% for activity in extension.review_activity.all %}
<li id="activity-{{ activity.id }}">
{# All activities except comments. #}
{% if activity.type != 'COM' %}
{% if activity.type in status_change_types %}
<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>

View File

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

View File

@ -3,13 +3,21 @@ from django.shortcuts import reverse
from common.tests.factories.extensions import create_version
from files.models import File
from reviewers.models import ApprovalActivity
class CommentsViewTest(TestCase):
fixtures = ['licenses']
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
def test_list_visibility(self):

View File

@ -13,15 +13,51 @@ from reviewers.models import ApprovalActivity
log = logging.getLogger(__name__)
STATUS_CHANGE_TYPES = [
ApprovalActivity.ActivityType.APPROVED,
ApprovalActivity.ActivityType.AWAITING_CHANGES,
ApprovalActivity.ActivityType.AWAITING_REVIEW,
]
class ApprovalQueueView(ListView):
model = Extension
paginate_by = 100
def get_queryset(self):
return Extension.objects.exclude(status=Extension.STATUSES.APPROVED).order_by(
'-date_created'
qs = (
ApprovalActivity.objects.prefetch_related(
Review

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.
Review

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'
@ -36,32 +72,30 @@ class ExtensionsApprovalDetailView(DetailView):
ctx['pending_previews'] = self.object.preview_set.exclude(
file__status=File.STATUSES.APPROVED
)
ctx['status_change_types'] = STATUS_CHANGE_TYPES
if self.request.user.is_authenticated:
form = ctx['comment_form'] = CommentForm()
# Remove 'Approved' status from dropdown it not moderator
filtered_activity_types = ApprovalActivity.ActivityType.choices
# anyone can comment
filtered_activity_types = {ApprovalActivity.ActivityType.COMMENT}
user = self.request.user
if not (user.is_moderator or user.is_superuser):
filtered_activity_types = [
t
for t in ApprovalActivity.ActivityType.choices
if t[0]
not in [
if self.object.has_maintainer(user):
filtered_activity_types.add(ApprovalActivity.ActivityType.AWAITING_REVIEW)
if user.is_moderator or user.is_superuser:
filtered_activity_types.update(
[
ApprovalActivity.ActivityType.APPROVED,
ApprovalActivity.ActivityType.AWAITING_CHANGES,
]
]
if not self.object.has_maintainer(user):
# Other accounts can only comment
filtered_activity_types = [
t
for t in ApprovalActivity.ActivityType.choices
if t[0] == ApprovalActivity.ActivityType.COMMENT
]
form.fields['type'].choices = filtered_activity_types
form.fields['type'].widget.choices = filtered_activity_types
if len(filtered_activity_types) == 1:
)
choices = list(
filter(
lambda c: c[0] in filtered_activity_types, ApprovalActivity.ActivityType.choices
)
)
form.fields['type'].choices = choices
form.fields['type'].widget.choices = choices
if len(choices) == 1:
form.fields['type'].widget = django.forms.HiddenInput()
return ctx