From 9cc66bca359086f26e47f208db8880e083e66f4d Mon Sep 17 00:00:00 2001 From: Jonathan Sundqvist Date: Sat, 12 Oct 2019 10:44:18 +0200 Subject: [PATCH] Reduce the amount of queries from 11000 in admin to 14 (#157) --- djangocms_moderation/admin.py | 13 +++++++- djangocms_moderation/managers.py | 57 ++++++++++++++++++++++++++++++-- djangocms_moderation/models.py | 29 ++++++---------- tests/test_managers.py | 14 ++++++++ 4 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 tests/test_managers.py diff --git a/djangocms_moderation/admin.py b/djangocms_moderation/admin.py index e4acc258..306a2ed8 100644 --- a/djangocms_moderation/admin.py +++ b/djangocms_moderation/admin.py @@ -969,6 +969,12 @@ class Media: actions = None # remove `delete_selected` for now, it will be handled later list_filter = [ModeratorFilter, "status", "date_created", ReviewerFilter] list_display_links = None + list_per_page = 100 + + def get_queryset(self, request): + qs = super().get_queryset(request) + qs = qs.prefetch_reviewers() + return qs def get_list_display(self, request): list_display = [ @@ -977,7 +983,7 @@ def get_list_display(self, request): "author", "workflow", "status", - "reviewers", + "commaseparated_reviewers", "date_created", "list_display_actions", ] @@ -986,6 +992,11 @@ def get_list_display(self, request): def job_id(self, obj): return obj.pk + def commaseparated_reviewers(self, obj): + reviewers = self.model.objects.reviewers(obj) + return ", ".join(map(get_user_model().get_full_name, reviewers)) + commaseparated_reviewers.short_description = _('reviewers') + def list_display_actions(self, obj): """Display links to state change endpoints """ diff --git a/djangocms_moderation/managers.py b/djangocms_moderation/managers.py index a8552bd4..2d8f7e6a 100644 --- a/djangocms_moderation/managers.py +++ b/djangocms_moderation/managers.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +from django.db import models from django.db.models import Manager, Q from .constants import ( @@ -8,6 +9,7 @@ ACCESS_PAGE, ACCESS_PAGE_AND_CHILDREN, ACCESS_PAGE_AND_DESCENDANTS, + COLLECTING, ) @@ -32,8 +34,57 @@ def for_page(self, page): ) query |= Q(extended_object=page) & ( - Q(grant_on=ACCESS_PAGE_AND_DESCENDANTS) - | Q(grant_on=ACCESS_PAGE_AND_CHILDREN) - | Q(grant_on=ACCESS_PAGE) + Q(grant_on=ACCESS_PAGE_AND_DESCENDANTS) | + Q(grant_on=ACCESS_PAGE_AND_CHILDREN) | + Q(grant_on=ACCESS_PAGE) ) return self.filter(query).order_by("-extended_object__node__depth").first() + + +class CollectionQuerySet(models.QuerySet): + def prefetch_reviewers(self): + """ + Prefetch all necessary relations so it's possible to get reviewers + without incurring extra queries. + """ + return self.prefetch_related( + 'moderation_requests', + 'moderation_requests__actions', + 'moderation_requests__actions__to_user', + 'workflow__steps__role__group__user_set' + ) + + +class CollectionManager(Manager): + + def get_queryset(self): + return CollectionQuerySet(self.model, using=self._db) + + def reviewers(self, collection): + """ + Returns a set of all reviewers assigned to any ModerationRequestAction + associated with this collection. If none are associated with a given + action then get the role for the step in the workflow and include all + reviewers within that list. + + Please note that if collection has not been prefetched using + `prefetch_reviewers` this has a chance of making a massive overhead. + """ + + reviewers = set() + moderation_requests = collection.moderation_requests.all() + for mr in moderation_requests: + moderation_request_actions = mr.actions.all() + reviewers_in_actions = set() + for mra in moderation_request_actions: + if mra.to_user: + reviewers_in_actions.add(mra.to_user) + reviewers.add(mra.to_user) + + if not reviewers_in_actions and collection.status != COLLECTING: + role = collection.workflow.first_step.role + users = role.get_users_queryset() + for user in users: + reviewers.add(user) + + return reviewers diff --git a/djangocms_moderation/models.py b/djangocms_moderation/models.py index 05ee7d12..2b840e33 100644 --- a/djangocms_moderation/models.py +++ b/djangocms_moderation/models.py @@ -18,6 +18,7 @@ from treebeard.mp_tree import MP_Node from .emails import notify_collection_moderators +from .managers import CollectionManager from .utils import generate_compliance_number @@ -224,6 +225,8 @@ class ModerationCollection(models.Model): date_created = models.DateTimeField(auto_now_add=True) date_modified = models.DateTimeField(auto_now=True) + objects = CollectionManager() + class Meta: verbose_name = _("collection") permissions = ( @@ -245,25 +248,13 @@ def author_name(self): @property def reviewers(self): """ - Find all the reviewers assigned to any moderationrequestaction associated with this collection. - If none are associated with a given action, - then get the role for the step in the workflow and include all reviewers within that list. + DEPRECATED - if you need to get a list of reviewers use the following instead + obj = ModerationCollection.objects.all().prefetch_reviewers().first() + reviewers = ModerationCollection.objects.reviewers(obj) + + The above method makes sure that you won't incur a query overhead """ - reviewers = set() - moderation_requests = self.moderation_requests.all() - for mr in moderation_requests: - moderation_request_actions = mr.actions.all() - reviewers_in_actions = set() - for mra in moderation_request_actions: - if mra.to_user: - reviewers_in_actions.add(mra.to_user) - reviewers.add(mra.to_user) - - if not reviewers_in_actions and self.status != constants.COLLECTING: - role = self.workflow.first_step.role - users = role.get_users_queryset() - for user in users: - reviewers.add(user) + reviewers = self.objects.reviewers(self) return ", ".join(map(get_user_model().get_full_name, reviewers)) def allow_submit_for_review(self, user): @@ -453,7 +444,7 @@ class Meta: ordering = ["id"] def __str__(self): - return "{} {}".format(self.pk, self.version.pk) + return "{} {}".format(self.pk, self.version_id) @cached_property def workflow(self): diff --git a/tests/test_managers.py b/tests/test_managers.py new file mode 100644 index 00000000..f2161983 --- /dev/null +++ b/tests/test_managers.py @@ -0,0 +1,14 @@ +from djangocms_moderation.models import ModerationCollection + +from .utils.base import BaseTestCase + + +class CollectionManangerTest(BaseTestCase): + + def test_reviewers_wont_execute_too_many_queries(self): + """This works as a stop gap that will prevent any further changes to + execute more than 9 queries for prefetching_reviweers""" + with self.assertNumQueries(9): + colls = ModerationCollection.objects.all().prefetch_reviewers() + for collection in colls: + ModerationCollection.objects.reviewers(collection)