ApprovalQueue: use a materialized table #240

Merged
Oleg-Komarov merged 4 commits from approval-queue-fix into main 2024-08-23 15:11:46 +02:00
7 changed files with 129 additions and 59 deletions
Showing only changes of commit daad143fbe - Show all commits

View File

@ -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),
]

View File

@ -28,6 +28,8 @@ class CannedResponse(CreatedModifiedMixin, models.Model):
class ApprovalActivity(CreatedModifiedMixin, RecordDeletionMixin, models.Model): class ApprovalActivity(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
STATUS_CHANGE_TYPES = {"AWR": 0x02, "AWC": 0x01, "APR": 0x00}
class ActivityType(models.TextChoices): class ActivityType(models.TextChoices):
COMMENT = "COM", _("Comment") COMMENT = "COM", _("Comment")
APPROVED = "APR", _("Approved") APPROVED = "APR", _("Approved")
@ -53,3 +55,27 @@ class ApprovalActivity(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
def __str__(self): def __str__(self):
return f"{self.extension.name}: {self.get_type_display()}" return f"{self.extension.name}: {self.get_type_display()}"
@property
def queue_sortkey(self):
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']),
]

View File

@ -4,7 +4,7 @@ from django.db.models.signals import post_save, pre_delete
from django.dispatch import receiver from django.dispatch import receiver
from constants.activity import Flag, Verb from constants.activity import Flag, Verb
from reviewers.models import ApprovalActivity from reviewers.models import ApprovalActivity, ApprovalQueue
@receiver(post_save, sender=ApprovalActivity) @receiver(post_save, sender=ApprovalActivity)
@ -40,6 +40,31 @@ 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
if instance.type not in ApprovalActivity.STATUS_CHANGE_TYPES:
return
activity_count = ApprovalActivity.objects.filter(extension=instance.extension).count()
ApprovalQueue.objects.update_or_create(
extension=instance.extension,
defaults={
'activity_count': activity_count,
'latest_activity': instance,
'sortkey': instance.queue_sortkey,
},
)
@receiver(pre_delete, sender=ApprovalActivity) @receiver(pre_delete, sender=ApprovalActivity)
def _log_deletion(sender: object, instance: ApprovalActivity, **kwargs: object) -> None: def _log_deletion(sender: object, instance: ApprovalActivity, **kwargs: object) -> None:
instance.record_deletion() instance.record_deletion()

View File

@ -22,20 +22,20 @@
</td> </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="ext-review-list-activity" colspan="2"> <td class="ext-review-list-activity" colspan="2">
<a href="{{ extension.get_review_url }}#activity-{{ stats.last_activity.id }}"> <a href="{{ extension.get_review_url }}#activity-{{ item.latest_activity.id }}">
<span>{{ stats.last_activity.date_created|naturaltime_compact }}</span> <span>{{ item.latest_activity.date_created|naturaltime_compact }}</span>
</a> </a>
{% 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 class="ext-review-list-activity ps-0 text-center"> <td class="ext-review-list-activity ps-0 text-center">
<a href="{{ extension.get_review_url }}#activity"> <a href="{{ extension.get_review_url }}#activity">
<span class="badge">{{ stats.count }}</span> <span class="badge">{{ item.activity_count }}</span>
</a> </a>
</td> </td>
<td class="ext-review-list-status"> <td class="ext-review-list-status">
<a href="{{ extension.get_review_url }}" class="text-decoration-none"> <a href="{{ extension.get_review_url }}" class="text-decoration-none">
{% with last_type=stats.last_type_display|default:"Awaiting Review" %} {% with latest_type=item.latest_activity.get_type_display %}
{% include "common/components/status.html" with label=last_type slug=last_type|slugify object=extension classes="d-block" icon=True %} {% include "common/components/status.html" with label=latest_type slug=latest_type|slugify object=extension classes="d-block" icon=True %}
{% endwith %} {% endwith %}
</a> </a>
</td> </td>

View File

@ -36,9 +36,9 @@
</tr> </tr>
</thead> </thead>
<tbody> <tbody>
{% for stats in object_list %} {% for item in object_list %}
{% with extension=stats.extension %} {% with extension=item.extension %}
{% include 'reviewers/components/review_list_item.html' with extension=extension stats=stats %} {% include 'reviewers/components/review_list_item.html' with extension=extension item=item %}
{% endwith %} {% endwith %}
{% endfor %} {% endfor %}
</tbody> </tbody>

View File

@ -13,7 +13,7 @@ class CommentsViewTest(TestCase):
version = create_version(status=File.STATUSES.AWAITING_REVIEW) version = create_version(status=File.STATUSES.AWAITING_REVIEW)
self.default_version = version self.default_version = version
ApprovalActivity( ApprovalActivity(
type=ApprovalActivity.ActivityType.COMMENT, type=ApprovalActivity.ActivityType.AWAITING_REVIEW,
user=version.files.first().user, user=version.files.first().user,
extension=version.extension, extension=version.extension,
message='test comment', message='test comment',

View File

@ -1,4 +1,3 @@
from collections import defaultdict
import logging import logging
from actstream.actions import follow, is_following, unfollow from actstream.actions import follow, is_following, unfollow
@ -11,62 +10,30 @@ import django.forms
from constants.activity import Flag from constants.activity import Flag
from extensions.models import Extension from extensions.models import Extension
from reviewers.forms import CommentForm from reviewers.forms import CommentForm
from reviewers.models import ApprovalActivity from reviewers.models import ApprovalActivity, ApprovalQueue
log = logging.getLogger(__name__) 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): class ApprovalQueueView(ListView):
model = Extension model = Extension
paginate_by = 50 paginate_by = 50
def get_queryset(self): def get_queryset(self):
qs = ApprovalActivity.objects.prefetch_related( return (
ApprovalQueue.objects.select_related(
'extension', 'extension',
'latest_activity',
)
.prefetch_related(
'extension__authors', 'extension__authors',
'extension__icon', 'extension__icon',
'extension__versions', 'extension__versions',
'extension__versions__files', 'extension__versions__files',
'extension__versions__files__validation', 'extension__versions__files__validation',
).order_by('-id') )
by_extension = {} .order_by('-sortkey')
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
template_name = 'reviewers/extensions_review_list.html' template_name = 'reviewers/extensions_review_list.html'
@ -102,7 +69,7 @@ class ExtensionsApprovalDetailView(DetailView):
activity.user_is_maintainer = True activity.user_is_maintainer = True
ctx['review_activity'] = review_activity 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: if self.request.user.is_authenticated:
initial = {} initial = {}