optimize PromotedAddon admin list view (#15116)

src/olympia/addons/tests/test_models.py

@@ -1598,6 +1598,7 @@ class TestAddonModels(TestCase):
 
         addon.current_version.promoted_approvals.all()[0].update(
             group_id=SPOTLIGHT.id)
+        del addon.current_version.approved_for_groups
         addon.promotedaddon.update(group_id=RECOMMENDED.id)
         del addon.is_recommended
         # similarly if the current_version wasn't reviewed for recommended
@@ -1657,6 +1658,7 @@ class TestAddonModels(TestCase):
         # The latest version is approved for the same group.
         PromotedApproval.objects.create(
             version=addon.current_version, group_id=SPOTLIGHT.id)
+        del addon.current_version.approved_for_groups
         assert addon.is_promoted()
         assert addon.is_promoted(group=SPOTLIGHT)
         # not for other groups though
@@ -1674,6 +1676,7 @@ class TestAddonModels(TestCase):
 
         PromotedApproval.objects.create(
             version=addon.current_version, group_id=VERIFIED_ONE.id)
+        del addon.current_version.approved_for_groups
         assert addon.is_promoted(group=VERIFIED_ONE)
 
         # Application specific group membership should work too

src/olympia/hero/tests/test_models.py

@@ -37,6 +37,7 @@ class TestPrimaryHero(TestCase):
         PromotedApproval.objects.create(
             version=ph.promoted_addon.addon.current_version,
             group_id=RECOMMENDED.id)
+        ph.reload()
         assert ph.promoted_addon.addon.is_promoted(group=RECOMMENDED)
         ph.clean()  # it raises if there's an error
 
@@ -62,6 +63,7 @@ class TestPrimaryHero(TestCase):
         PromotedApproval.objects.create(
             version=ph.promoted_addon.addon.current_version,
             group_id=RECOMMENDED.id)
+        ph.reload()
         assert not ph.enabled
         ph.clean()  # it raises if there's an error
         ph.enabled = True
@@ -87,6 +89,7 @@ class TestPrimaryHero(TestCase):
         PromotedApproval.objects.create(
             version=hero.promoted_addon.addon.current_version,
             group_id=RECOMMENDED.id)
+        hero.reload()
         assert not hero.enabled
         assert not PrimaryHero.objects.filter(enabled=True).exists()
         # It should still validate even if there are no other enabled shelves,

src/olympia/promoted/admin.py

@@ -5,6 +5,7 @@ from django.forms.models import modelformset_factory
 from olympia.addons.models import Addon
 from olympia.discovery.admin import SlugOrPkChoiceField
 from olympia.hero.admin import PrimaryHeroInline
+from olympia.versions.models import Version
 
 from .forms import AdminBasePromotedApprovalFormSet
 from .models import PromotedAddon, PromotedApproval
@@ -67,13 +68,16 @@ class PromotedAddonAdmin(admin.ModelAdmin):
     list_filter = ('group_id', 'application_id')
     inlines = (PromotedApprovalInline, PrimaryHeroInline)
 
+    @classmethod
+    def _transformer(self, objs):
+        Version.transformer_promoted(
+            [promo.addon._current_version for promo in objs]
+        )
+
     def get_queryset(self, request):
         # Select `primaryhero`, `addon` and it's `_current_version`.
         # We are forced to use `prefetch_related` to ensure transforms
         # are being run, though, we only care about translations
-        #
-        # TODO: optimize this query to preload the PromotedApprovals fk'd to
-        # versions.
         qset = (
             self.model.objects.all()
             .select_related('primaryhero')
@@ -83,7 +87,8 @@ class PromotedAddonAdmin(admin.ModelAdmin):
                     queryset=(
                         Addon.unfiltered.all()
                         .select_related('_current_version')
-                        .only_translations()))))
+                        .only_translations())))
+            .transform(self._transformer))
         return qset
 
     def addon__name(self, obj):

src/olympia/promoted/models.py

@@ -46,8 +46,7 @@ class PromotedAddon(ModelBase):
         return bool(
             self.addon.current_version and
             self.group != NOT_PROMOTED.id and
-            self.addon.current_version.promoted_approvals.filter(
-                group_id=self.group_id).exists())
+            self.group in self.addon.current_version.approved_for_groups)
 
 
 class PromotedApproval(ModelBase):

src/olympia/promoted/tests/test_admin.py

@@ -67,10 +67,28 @@ class TestPromotedAddonAdmin(TestCase):
         self.grant_permission(user, 'Admin:Tools')
         self.grant_permission(user, 'Discovery:Edit')
         self.client.login(email=user.email)
-        response = self.client.get(self.list_url, follow=True)
+
+        with self.assertNumQueries(10):
+            # 1. select current user
+            # 2. savepoint (because we're in tests)
+            # 3. select groups
+            # 4. pagination count
+            # 5. pagination count (double…)
+            # 6. select list of promoted addons, ordered
+            # 7. prefetch add-ons
+            # 8. select translations for add-ons from 7.
+            # 9. prefetch PromotedApprovals for add-ons current_versions
+            # 10. savepoint (because we're in tests)
+            response = self.client.get(self.list_url, follow=True)
+
         assert response.status_code == 200
         assert 'FooBâr' in response.content.decode('utf-8')
 
+        # double check it scales.
+        PromotedAddon.objects.create(addon=addon_factory(name='FooBâr'))
+        with self.assertNumQueries(10):
+            self.client.get(self.list_url, follow=True)
+
     def test_can_edit_with_discovery_edit_permission(self):
         addon = addon_factory()
         item = PromotedAddon.objects.create(

src/olympia/promoted/tests/test_models.py

@@ -25,6 +25,7 @@ class TestPromotedAddon(TestCase):
         # the current version needs to be approved also
         PromotedApproval.objects.create(
             version=addon.current_version, group_id=promoted.LINE.id)
+        addon.reload()
         assert addon.promotedaddon.is_addon_currently_promoted
 
         # but not if it's for a different type of promotion

src/olympia/versions/models.py

@@ -31,7 +31,7 @@ from olympia.amo.urlresolvers import reverse
 from olympia.amo.utils import sorted_groupby, utc_millesecs_from_epoch
 from olympia.applications.models import AppVersion
 from olympia.constants.licenses import LICENSES_BY_BUILTIN
-from olympia.constants.promoted import RECOMMENDED
+from olympia.constants.promoted import PROMOTED_GROUPS_BY_ID, RECOMMENDED
 from olympia.constants.scanners import MAD
 from olympia.files import utils
 from olympia.files.models import File, cleanup_file
@@ -604,6 +604,31 @@ class Version(OnChangeMixin, ModelBase):
             for f in version.all_files:
                 f.version = version
 
+    @classmethod
+    def transformer_promoted(cls, versions):
+        """Attach the promoted approvals to the versions."""
+        if not versions:
+            return
+
+        PromotedApproval = versions[0].promoted_approvals.model
+
+        ids = set(v.id for v in versions)
+
+        approvals = list(
+            PromotedApproval.objects.filter(version_id__in=ids)
+                            .values_list('version_id', 'group_id', named=True))
+
+        promoted_dict = {
+            version_id: list(groups)
+            for version_id, groups in sorted_groupby(approvals, 'version_id')}
+        for version in versions:
+            v_id = version.id
+            groups = [
+                PROMOTED_GROUPS_BY_ID.get(pro.group_id)
+                for pro in promoted_dict.get(v_id, [])
+                if pro.group_id in PROMOTED_GROUPS_BY_ID]
+            version.approved_for_groups = groups
+
     @classmethod
     def transformer_activity(cls, versions):
         """Attach all the activity to the versions."""
@@ -748,6 +773,15 @@ class Version(OnChangeMixin, ModelBase):
             score = None
         return '{:0.0f}%'.format(score * 100) if score else 'n/a'
 
+    @cached_property
+    def approved_for_groups(self):
+        approvals = list(self.promoted_approvals.all())
+        return [
+            PROMOTED_GROUPS_BY_ID.get(pro.group_id)
+            for pro in approvals
+            if pro.group_id in PROMOTED_GROUPS_BY_ID
+        ]
+
 
 class VersionReviewerFlags(ModelBase):
     version = models.OneToOneField(

src/olympia/versions/tests/test_models.py

@@ -24,7 +24,7 @@ from olympia.amo.tests.test_models import BasePreviewMixin
 from olympia.amo.utils import utc_millesecs_from_epoch
 from olympia.applications.models import AppVersion
 from olympia.blocklist.models import Block
-from olympia.constants.promoted import NOT_PROMOTED, RECOMMENDED
+from olympia.constants.promoted import LINE, NOT_PROMOTED, RECOMMENDED
 from olympia.constants.scanners import CUSTOMS, WAT, YARA, MAD
 from olympia.files.models import File, FileUpload
 from olympia.files.tests.test_models import UploadTest
@@ -719,6 +719,46 @@ class TestVersion(TestCase):
         ScannerResult.objects.create(version=version, scanner=MAD, score=score)
         assert version.scanners_score == '15%'
 
+    def test_approved_for_groups(self):
+        version = addon_factory().current_version
+        assert version.approved_for_groups == []
+
+        # give it some promoted approvals
+        PromotedApproval.objects.create(
+            version=version, group_id=LINE.id)
+        PromotedApproval.objects.create(
+            version=version, group_id=RECOMMENDED.id)
+
+        del version.approved_for_groups
+        assert version.approved_for_groups == [LINE, RECOMMENDED]
+
+    def test_transform_promoted(self):
+        version_a = addon_factory().current_version
+        version_b = addon_factory().current_version
+        versions = (
+            Version.objects.filter(id__in=(version_a.id, version_b.id))
+                           .transform(Version.transformer_promoted))
+        list(versions)  # to evaluate the queryset
+        with self.assertNumQueries(0):
+            assert versions[0].approved_for_groups == []
+            assert versions[1].approved_for_groups == []
+
+        # give them some promoted approvals
+        PromotedApproval.objects.create(
+            version=version_a, group_id=LINE.id)
+        PromotedApproval.objects.create(
+            version=version_a, group_id=RECOMMENDED.id)
+        PromotedApproval.objects.create(
+            version=version_b, group_id=RECOMMENDED.id)
+
+        versions = (
+            Version.objects.filter(id__in=(version_a.id, version_b.id))
+                           .transform(Version.transformer_promoted))
+        list(versions)  # to evaluate the queryset
+        with self.assertNumQueries(0):
+            assert versions[1].approved_for_groups == [RECOMMENDED, LINE]
+            assert versions[0].approved_for_groups == [RECOMMENDED]
+
 
 @pytest.mark.parametrize("addon_status,file_status,is_unreviewed", [
     (amo.STATUS_NOMINATED, amo.STATUS_AWAITING_REVIEW, True),