diff --git a/requirements_dev.txt b/requirements_dev.txt
index e589676c..47bb70ca 100644
--- a/requirements_dev.txt
+++ b/requirements_dev.txt
@@ -6,6 +6,7 @@ django-debug-toolbar==4.2.0
factory-boy==3.2.1
Faker==13.15.1
filelock==3.7.1
+freezegun==1.5.1
identify==2.5.2
mdgen==0.1.10
nodeenv==1.7.0
diff --git a/reviewers/migrations/0012_approvalqueue.py b/reviewers/migrations/0012_approvalqueue.py
new file mode 100644
index 00000000..bacf49ca
--- /dev/null
+++ b/reviewers/migrations/0012_approvalqueue.py
@@ -0,0 +1,52 @@
+# Generated by Django 4.2.11 on 2024-08-23 09:42
+
+from django.db import migrations, models
+from django.db.models import Count
+import django.db.models.deletion
+
+
+def populate_approval_queue(apps, schema_editor):
+ ApprovalActivity = apps.get_model('reviewers', 'ApprovalActivity')
+ ApprovalQueue = apps.get_model('reviewers', 'ApprovalQueue')
+ processed_extensions = set()
+ STATUS_CHANGE_TYPES = {"AWR": 0x02, "AWC": 0x01, "APR": 0x00}
+ activity_counts = {}
+ for row in ApprovalActivity.objects.all().values('extension__pk').annotate(count=Count('id')).order_by():
+ activity_counts[row['extension__pk']] = row['count']
+ for activity in ApprovalActivity.objects.order_by('-id').all():
+ extension = activity.extension
+ if extension in processed_extensions:
+ continue
+ if activity.type in STATUS_CHANGE_TYPES:
+ activity_count = activity_counts[extension.pk]
+ timestamp = int(activity.date_created.timestamp())
+ sortkey = (STATUS_CHANGE_TYPES[activity.type] << 32) | timestamp
+ ApprovalQueue.objects.create(
+ activity_count=activity_count,
+ extension=extension,
+ latest_activity=activity,
+ sortkey=sortkey,
+ )
+ processed_extensions.add(extension)
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('extensions', '0042_platform_name'),
+ ('reviewers', '0011_alter_approvalactivity_user'),
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name='ApprovalQueue',
+ fields=[
+ ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+ ('activity_count', models.PositiveIntegerField()),
+ ('sortkey', models.BigIntegerField()),
+ ('extension', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='extensions.extension')),
+ ('latest_activity', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='reviewers.approvalactivity')),
+ ],
+ ),
+ migrations.RunPython(populate_approval_queue, migrations.RunPython.noop),
+ ]
diff --git a/reviewers/models.py b/reviewers/models.py
index b4f3dd7b..29c008e5 100644
--- a/reviewers/models.py
+++ b/reviewers/models.py
@@ -28,6 +28,8 @@ class CannedResponse(CreatedModifiedMixin, models.Model):
class ApprovalActivity(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
+ STATUS_CHANGE_TYPES = {"AWR": 0x02, "AWC": 0x01, "APR": 0x00}
+
class ActivityType(models.TextChoices):
COMMENT = "COM", _("Comment")
APPROVED = "APR", _("Approved")
@@ -53,3 +55,37 @@ class ApprovalActivity(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
def __str__(self):
return f"{self.extension.name}: {self.get_type_display()}"
+
+ @property
+ def queue_sortkey(self):
+ """Sorting by moderation status and latest status change timestamp.
+
+ The queue is ordered by status: first "awaiting review', then "awaiting changes", then
+ "approved".
+ Within each group items with most recent status change are sorted to the top.
+
+ Integer timestamp representation takes 4 bytes, the resulting bigint is composed of
+ 0x000000SSTTTTTTTT, where SS byte represents the status change type, and TT bytes represent
+ timestamp bytes.
+ """
+ timestamp = int(self.date_created.timestamp())
+ return (self.STATUS_CHANGE_TYPES[self.type] << 32) | timestamp
+
+
+class ApprovalQueue(models.Model):
+ activity_count = models.PositiveIntegerField()
+ extension = models.OneToOneField(
+ 'extensions.Extension',
+ on_delete=models.CASCADE,
+ )
+ latest_activity = models.ForeignKey(
+ ApprovalActivity,
+ # we don't delete activity yet, if we start this needs to be updated via a pre_delete signal
+ on_delete=models.PROTECT,
+ )
+ sortkey = models.BigIntegerField()
+
+ class Meta:
+ indexes = [
+ models.Index(fields=['sortkey']),
+ ]
diff --git a/reviewers/signals.py b/reviewers/signals.py
index 2311e524..9ed8ec91 100644
--- a/reviewers/signals.py
+++ b/reviewers/signals.py
@@ -4,7 +4,7 @@ from django.db.models.signals import post_save, pre_delete
from django.dispatch import receiver
from constants.activity import Flag, Verb
-from reviewers.models import ApprovalActivity
+from reviewers.models import ApprovalActivity, ApprovalQueue
@receiver(post_save, sender=ApprovalActivity)
@@ -40,6 +40,35 @@ def _create_action_from_review_and_follow(
)
+@receiver(post_save, sender=ApprovalActivity)
+def _update_approval_queue(
+ sender: object,
+ instance: ApprovalActivity,
+ created: bool,
+ raw: bool,
+ **kwargs: object,
+) -> None:
+ if raw:
+ return
+ if not created:
+ return
+
+ activity_count = ApprovalActivity.objects.filter(extension=instance.extension).count()
+ if instance.type in ApprovalActivity.STATUS_CHANGE_TYPES:
+ ApprovalQueue.objects.update_or_create(
+ extension=instance.extension,
+ defaults={
+ 'activity_count': activity_count,
+ 'latest_activity': instance,
+ 'sortkey': instance.queue_sortkey,
+ },
+ )
+ else:
+ if item := ApprovalQueue.objects.filter(extension=instance.extension).first():
+ item.activity_count = activity_count
+ item.save(update_fields={'activity_count'})
+
+
@receiver(pre_delete, sender=ApprovalActivity)
def _log_deletion(sender: object, instance: ApprovalActivity, **kwargs: object) -> None:
instance.record_deletion()
diff --git a/reviewers/templates/reviewers/components/review_list_item.html b/reviewers/templates/reviewers/components/review_list_item.html
index 5b30c9df..0a2998dd 100644
--- a/reviewers/templates/reviewers/components/review_list_item.html
+++ b/reviewers/templates/reviewers/components/review_list_item.html
@@ -22,20 +22,20 @@
| {{ extension.date_created|naturaltime_compact }} |
-
- {{ stats.last_activity.date_created|naturaltime_compact }}
+
+ {{ item.latest_activity.date_created|naturaltime_compact }}
{% include "files/components/scan_details_flag.html" with suspicious_files=extension.suspicious_files %}
|
- {{ stats.count }}
+ {{ item.activity_count }}
|
- {% with last_type=stats.last_type_display|default:"Awaiting Review" %}
- {% include "common/components/status.html" with label=last_type slug=last_type|slugify object=extension classes="d-block" icon=True %}
+ {% with latest_type=item.latest_activity.get_type_display %}
+ {% include "common/components/status.html" with label=latest_type slug=latest_type|slugify object=extension classes="d-block" icon=True %}
{% endwith %}
|
diff --git a/reviewers/templates/reviewers/extensions_review_list.html b/reviewers/templates/reviewers/extensions_review_list.html
index 13b6a66f..6dd75034 100644
--- a/reviewers/templates/reviewers/extensions_review_list.html
+++ b/reviewers/templates/reviewers/extensions_review_list.html
@@ -36,9 +36,9 @@
- {% for stats in object_list %}
- {% with extension=stats.extension %}
- {% include 'reviewers/components/review_list_item.html' with extension=extension stats=stats %}
+ {% for item in object_list %}
+ {% with extension=item.extension %}
+ {% include 'reviewers/components/review_list_item.html' with extension=extension item=item %}
{% endwith %}
{% endfor %}
diff --git a/reviewers/tests/test_views.py b/reviewers/tests/test_views.py
index d2028a1d..fa5f4a70 100644
--- a/reviewers/tests/test_views.py
+++ b/reviewers/tests/test_views.py
@@ -1,7 +1,11 @@
+from datetime import datetime, timedelta, timezone
+
from django.test import TestCase
from django.shortcuts import reverse
+from freezegun import freeze_time
from common.tests.factories.extensions import create_version
+from common.tests.factories.users import UserFactory
from files.models import File
from reviewers.models import ApprovalActivity
@@ -13,7 +17,7 @@ class CommentsViewTest(TestCase):
version = create_version(status=File.STATUSES.AWAITING_REVIEW)
self.default_version = version
ApprovalActivity(
- type=ApprovalActivity.ActivityType.COMMENT,
+ type=ApprovalActivity.ActivityType.AWAITING_REVIEW,
user=version.files.first().user,
extension=version.extension,
message='test comment',
@@ -40,3 +44,97 @@ class CommentsViewTest(TestCase):
# Display only previews and version that are 'pending review'
# Approval requires manager role
+
+
+class QueueOrderTest(TestCase):
+ fixtures = ['licenses']
+
+ def test_order(self):
+ extensions = []
+ user = UserFactory()
+ for i in range(5):
+ version = create_version(status=File.STATUSES.AWAITING_REVIEW)
+ extension = version.extension
+ extensions.append(extension)
+
+ # to control the distance between timestamps
+ starting_datetime = datetime(2020, 12, 31, 23, 2, 3, tzinfo=timezone.utc)
+ delta_s = 0
+ with freeze_time(starting_datetime + timedelta(seconds=delta_s)):
+ ApprovalActivity(
+ type=ApprovalActivity.ActivityType.AWAITING_REVIEW,
+ user=user,
+ extension=extensions[0],
+ message='test comment',
+ ).save()
+
+ delta_s += 1
+ with freeze_time(starting_datetime + timedelta(seconds=delta_s)):
+ ApprovalActivity(
+ type=ApprovalActivity.ActivityType.AWAITING_CHANGES,
+ user=user,
+ extension=extensions[1],
+ message='test comment',
+ ).save()
+
+ delta_s += 1
+ with freeze_time(starting_datetime + timedelta(seconds=delta_s)):
+ ApprovalActivity(
+ type=ApprovalActivity.ActivityType.AWAITING_CHANGES,
+ user=user,
+ extension=extensions[2],
+ message='test comment',
+ ).save()
+
+ apr = None
+ delta_s += 1
+ with freeze_time(starting_datetime + timedelta(seconds=delta_s)):
+ apr = ApprovalActivity(
+ date_created=starting_datetime + timedelta(seconds=delta_s),
+ type=ApprovalActivity.ActivityType.APPROVED,
+ user=user,
+ extension=extensions[3],
+ message='test comment',
+ )
+ apr.save()
+ delta_s += 1
+ with freeze_time(starting_datetime + timedelta(seconds=delta_s)):
+ ApprovalActivity(
+ date_created=starting_datetime + timedelta(seconds=delta_s),
+ type=ApprovalActivity.ActivityType.COMMENT,
+ user=user,
+ extension=extensions[3],
+ message='test comment',
+ ).save()
+
+ delta_s += 1
+ with freeze_time(starting_datetime + timedelta(seconds=delta_s)):
+ ApprovalActivity(
+ date_created=starting_datetime + timedelta(seconds=delta_s),
+ type=ApprovalActivity.ActivityType.COMMENT,
+ user=user,
+ extension=extensions[4],
+ message='test comment',
+ ).save()
+
+ response = self.client.get(reverse('reviewers:approval-queue'))
+ queue = response.context['object_list']
+
+ # extensions[4] doesn't have meaningful activity
+ self.assertEqual(len(queue), 4)
+
+ # ordered by category
+ self.assertEqual(queue[0].extension, extensions[0])
+ # and within category by recent timestamp
+ self.assertEqual(queue[1].extension, extensions[2])
+ self.assertEqual(queue[2].extension, extensions[1])
+ self.assertEqual(queue[3].extension, extensions[3])
+
+ # counts are correct
+ self.assertEqual(queue[0].activity_count, 1)
+ self.assertEqual(queue[1].activity_count, 1)
+ self.assertEqual(queue[2].activity_count, 1)
+ self.assertEqual(queue[3].activity_count, 2)
+
+ # latest_activity doesn't account for non-meaningful activity
+ self.assertEqual(queue[3].latest_activity, apr)
diff --git a/reviewers/views.py b/reviewers/views.py
index a119e363..b11a79a8 100644
--- a/reviewers/views.py
+++ b/reviewers/views.py
@@ -1,4 +1,3 @@
-from collections import defaultdict
import logging
from actstream.actions import follow, is_following, unfollow
@@ -11,62 +10,30 @@ import django.forms
from constants.activity import Flag
from extensions.models import Extension
from reviewers.forms import CommentForm
-from reviewers.models import ApprovalActivity
+from reviewers.models import ApprovalActivity, ApprovalQueue
log = logging.getLogger(__name__)
-# the ordering of this list determines the order of rows in approval queue listing
-STATUS_CHANGE_TYPES = [
- ApprovalActivity.ActivityType.AWAITING_REVIEW,
- ApprovalActivity.ActivityType.AWAITING_CHANGES,
- ApprovalActivity.ActivityType.APPROVED,
-]
-
class ApprovalQueueView(ListView):
model = Extension
paginate_by = 50
def get_queryset(self):
- qs = ApprovalActivity.objects.prefetch_related(
- 'extension',
- 'extension__authors',
- 'extension__icon',
- 'extension__versions',
- 'extension__versions__files',
- 'extension__versions__files__validation',
- ).order_by('-id')
- by_extension = {}
- by_date_created = []
- 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 by_date_created also ordered by item.date_created
- stats = {
- 'count': 0,
- 'extension': extension,
- 'last_activity': None,
- 'last_type_display': None,
- 'type': None,
- }
- by_extension[extension] = stats
- by_date_created.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()
- stats['type'] = item.type
- groupped_by_type = defaultdict(list)
- for stats in by_date_created:
- type = stats['type'] or ApprovalActivity.ActivityType.AWAITING_REVIEW
- groupped_by_type[type].append(stats)
- result = []
- for type in STATUS_CHANGE_TYPES:
- result.extend(groupped_by_type[type])
- return result
+ return (
+ ApprovalQueue.objects.select_related(
+ 'extension',
+ 'latest_activity',
+ )
+ .prefetch_related(
+ 'extension__authors',
+ 'extension__icon',
+ 'extension__versions',
+ 'extension__versions__files',
+ 'extension__versions__files__validation',
+ )
+ .order_by('-sortkey')
+ )
template_name = 'reviewers/extensions_review_list.html'
@@ -102,7 +69,7 @@ class ExtensionsApprovalDetailView(DetailView):
activity.user_is_maintainer = True
ctx['review_activity'] = review_activity
- ctx['status_change_types'] = STATUS_CHANGE_TYPES
+ ctx['status_change_types'] = ApprovalActivity.STATUS_CHANGE_TYPES
if self.request.user.is_authenticated:
initial = {}