Show Cinder Policy for each Review Action Reason and vice versa in ad…

…min (#21919)

Also make cinder policy required only if review action reason is active.

src/olympia/abuse/admin.py

@@ -2,6 +2,8 @@
 from django.core.paginator import Paginator
 from django.db.models import Count
 from django.template import loader
+from django.urls import reverse
+from django.utils.html import format_html, format_html_join
 
 from olympia.addons.models import Addon, AddonApprovalsCounter
 from olympia.amo.admin import AMOModelAdmin, DateRangeFilter, FakeChoicesMixin
@@ -365,6 +367,7 @@ class CinderPolicyAdmin(AMOModelAdmin):
         'uuid',
         'parent',
         'name',
+        'linked_review_reasons',
         'text',
     )
     ordering = ('parent__name', 'name')
@@ -380,6 +383,24 @@ def has_change_permission(self, request, obj=None):
     def has_delete_permission(self, request, obj=None):
         return False
 
+    @admin.display(ordering='reviewactionreason__name')
+    def linked_review_reasons(self, obj):
+        review_reasons = [
+            (
+                reverse('admin:reviewers_reviewactionreason_change', args=(reason.pk,)),
+                reason,
+            )
+            for reason in obj.reviewactionreason_set.all()
+        ]
+
+        return format_html(
+            '<ul>{}</ul>',
+            format_html_join('\n', '<li><a href="{}">{}</a></li>', review_reasons),
+        )
+
+    def get_queryset(self, request):
+        return super().get_queryset(request).prefetch_related('reviewactionreason_set')
+
 
 admin.site.register(AbuseReport, AbuseReportAdmin)
 admin.site.register(CinderPolicy, CinderPolicyAdmin)

src/olympia/abuse/tests/test_admin.py

@@ -1,3 +1,4 @@
+import uuid
 from datetime import date, datetime
 from urllib.parse import parse_qsl, urlparse
 
@@ -7,7 +8,7 @@
 from pyquery import PyQuery as pq
 
 from olympia import amo
-from olympia.abuse.models import AbuseReport
+from olympia.abuse.models import AbuseReport, CinderPolicy
 from olympia.addons.models import AddonApprovalsCounter
 from olympia.amo.tests import (
     TestCase,
@@ -18,7 +19,7 @@
     user_factory,
 )
 from olympia.ratings.models import Rating
-from olympia.reviewers.models import AutoApprovalSummary
+from olympia.reviewers.models import AutoApprovalSummary, ReviewActionReason
 from olympia.versions.models import VersionPreview
 
 
@@ -573,3 +574,75 @@ def test_detail_rating_report(self):
         assert not doc('.addon-info-and-previews')
         assert not doc('.field-addon_name')
         assert self.report_rating.rating.body in doc('.field-rating').text()
+
+
+class TestCinderPolicyAdmin(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = user_factory(email='someone@mozilla.com')
+        grant_permission(cls.user, '*:*', 'Admins')
+
+    def setUp(self):
+        self.client.force_login(self.user)
+        self.list_url = reverse('admin:abuse_cinderpolicy_changelist')
+
+    def test_list_no_permission(self):
+        user = user_factory(email='nobody@mozilla.com')
+        self.client.force_login(user)
+        response = self.client.get(self.list_url)
+        assert response.status_code == 403
+
+    def test_list(self):
+        foo = CinderPolicy.objects.create(name='Foo')
+        CinderPolicy.objects.create(name='Bar', parent=foo, uuid=uuid.uuid4())
+        zab = CinderPolicy.objects.create(name='Zab', parent=foo, uuid=uuid.uuid4())
+        lorem = CinderPolicy.objects.create(name='Lorem', uuid=uuid.uuid4())
+        CinderPolicy.objects.create(name='Ipsum', uuid=uuid.uuid4())
+        ReviewActionReason.objects.create(name='Attached to Zab', cinder_policy=zab)
+        ReviewActionReason.objects.create(name='Attached to Lorem', cinder_policy=lorem)
+        ReviewActionReason.objects.create(
+            name='Also attached to Lorem', cinder_policy=lorem
+        )
+
+        with self.assertNumQueries(7):
+            # - 2 savepoints (tests)
+            # - 2 current user & groups
+            # - 1 count cinder policies
+            # - 1 cinder policies
+            # - 1 review action reasons
+            response = self.client.get(self.list_url)
+        assert response.status_code == 200
+        doc = pq(response.content)
+        assert len(doc('#result_list tbody tr')) == CinderPolicy.objects.count()
+        assert doc('#result_list td.field-name').text() == 'Foo Ipsum Lorem Bar Zab'
+        assert (
+            doc('#result_list td.field-linked_review_reasons')[2].text_content()
+            == 'Also attached to Lorem\nAttached to Lorem'
+        )
+
+    def test_list_order_by_reviewreason(self):
+        foo = CinderPolicy.objects.create(name='Foo')
+        CinderPolicy.objects.create(name='Bar', parent=foo, uuid=uuid.uuid4())
+        zab = CinderPolicy.objects.create(name='Zab', parent=foo, uuid=uuid.uuid4())
+        lorem = CinderPolicy.objects.create(name='Lorem', uuid=uuid.uuid4())
+        CinderPolicy.objects.create(name='Ipsum', uuid=uuid.uuid4())
+        ReviewActionReason.objects.create(name='Attached to Zab', cinder_policy=zab)
+        ReviewActionReason.objects.create(name='Attached to Lorem', cinder_policy=lorem)
+
+        with self.assertNumQueries(7):
+            # - 2 savepoints (tests)
+            # - 2 current user & groups
+            # - 1 count cinder policies
+            # - 1 cinder policies
+            # - 1 review action reasons
+            # Linked reason is the 4th field, so we have to pass o=4 parameter
+            # to order on it.
+            response = self.client.get(self.list_url, {'o': '4'})
+        assert response.status_code == 200
+        doc = pq(response.content)
+        assert len(doc('#result_list tbody tr')) == CinderPolicy.objects.count()
+        assert doc('#result_list td.field-name').text() == 'Foo Ipsum Bar Lorem Zab'
+        assert (
+            doc('#result_list td.field-linked_review_reasons')[4].text_content()
+            == 'Attached to Zab'
+        )

src/olympia/reviewers/admin.py

@@ -1,12 +1,31 @@
+from django import forms
 from django.contrib import admin
 
 from olympia.amo.admin import AMOModelAdmin
+from olympia.zadmin.admin import related_single_content_link
 
 from .models import NeedsHumanReview, ReviewActionReason, UsageTier
 
 
+class ReviewActionReasonAdminForm(forms.ModelForm):
+    def clean(self):
+        is_active = self.cleaned_data.get('is_active', False)
+        if is_active and not self.cleaned_data.get('cinder_policy'):
+            msg = forms.ValidationError(
+                self.fields['cinder_policy'].error_messages['required']
+            )
+            self.add_error('cinder_policy', msg)
+        return self.cleaned_data
+
+
 class ReviewActionReasonAdmin(AMOModelAdmin):
-    list_display = ('name', 'addon_type', 'is_active')
+    form = ReviewActionReasonAdminForm
+    list_display = (
+        'name',
+        'linked_cinder_policy',
+        'addon_type',
+        'is_active',
+    )
     list_filter = (
         'addon_type',
         'is_active',
@@ -21,6 +40,10 @@ class ReviewActionReasonAdmin(AMOModelAdmin):
     )
     raw_id_fields = ('cinder_policy',)
     view_on_site = False
+    list_select_related = ('cinder_policy', 'cinder_policy__parent')
+
+    def linked_cinder_policy(self, obj):
+        return related_single_content_link(obj, 'cinder_policy')
 
 
 admin.site.register(ReviewActionReason, ReviewActionReasonAdmin)

src/olympia/reviewers/migrations/0035_alter_reviewactionreason_cinder_policy.py

@@ -0,0 +1,24 @@
+# Generated by Django 4.2.9 on 2024-02-23 11:48
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("abuse", "0025_cinderjob_resolvable_in_reviewer_tools_and_more"),
+        ("reviewers", "0034_create-specific-cinder-switch"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="reviewactionreason",
+            name="cinder_policy",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                to="abuse.cinderpolicy",
+            ),
+        ),
+    ]

src/olympia/reviewers/models.py

@@ -697,7 +697,7 @@ class ReviewActionReason(ModelBase):
         default=amo.ADDON_ANY,
     )
     cinder_policy = models.ForeignKey(
-        CinderPolicy, null=True, on_delete=models.SET_NULL
+        CinderPolicy, blank=True, null=True, on_delete=models.SET_NULL
     )
 
     def labelled_name(self):

src/olympia/reviewers/tests/test_admin.py

@@ -1,3 +1,5 @@
+import uuid
+
 from django.conf import settings
 from django.contrib import admin
 from django.contrib.messages.storage import default_storage as default_messages_storage
@@ -7,9 +9,16 @@
 from pyquery import PyQuery as pq
 
 from olympia import core
-from olympia.amo.tests import TestCase, addon_factory, user_factory, version_factory
+from olympia.abuse.models import CinderPolicy
+from olympia.amo.tests import (
+    TestCase,
+    addon_factory,
+    grant_permission,
+    user_factory,
+    version_factory,
+)
 from olympia.reviewers.admin import NeedsHumanReviewAdmin
-from olympia.reviewers.models import NeedsHumanReview
+from olympia.reviewers.models import NeedsHumanReview, ReviewActionReason
 
 
 class TestNeedsHumanReviewAdmin(TestCase):
@@ -148,3 +157,50 @@ def test_activate_selected_action(self):
         v2.reload()
         assert nhr2.is_active
         assert v2.due_date
+
+
+class TestReviewActionReasonAdmin(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = user_factory(email='someone@mozilla.com')
+        grant_permission(cls.user, '*:*', 'Admins')
+
+    def setUp(self):
+        self.client.force_login(self.user)
+        self.list_url = reverse('admin:reviewers_reviewactionreason_changelist')
+
+    def test_list_no_permission(self):
+        user = user_factory(email='nobody@mozilla.com')
+        self.client.force_login(user)
+        response = self.client.get(self.list_url)
+        assert response.status_code == 403
+
+    def test_list(self):
+        foo = CinderPolicy.objects.create(name='Foo')
+        CinderPolicy.objects.create(name='Bar', parent=foo, uuid=uuid.uuid4())
+        zab = CinderPolicy.objects.create(name='Zab', parent=foo, uuid=uuid.uuid4())
+        lorem = CinderPolicy.objects.create(name='Lorem', uuid=uuid.uuid4())
+        CinderPolicy.objects.create(name='Ipsum', uuid=uuid.uuid4())
+        ReviewActionReason.objects.create(name='Attached to Zab', cinder_policy=zab)
+        ReviewActionReason.objects.create(name='Attached to Lorem', cinder_policy=lorem)
+        ReviewActionReason.objects.create(
+            name='Also attached to Lorem', cinder_policy=lorem
+        )
+
+        with self.assertNumQueries(6):
+            # - 2 savepoints (tests)
+            # - 2 current user & groups
+            # - 1 count review action reasons
+            # - 1 review action reasons (+ cinder policies and parents in one query)
+            response = self.client.get(self.list_url)
+        assert response.status_code == 200
+        doc = pq(response.content)
+        assert len(doc('#result_list tbody tr')) == ReviewActionReason.objects.count()
+        assert (
+            doc('#result_list th.field-name').text()
+            == 'Also attached to Lorem Attached to Lorem Attached to Zab'
+        )
+        assert (
+            doc('#result_list td.field-linked_cinder_policy')[2].text_content()
+            == 'Foo, specifically Zab'
+        )