diff --git a/src/olympia/abuse/admin.py b/src/olympia/abuse/admin.py
index 22b479b4516..7df0b82ed86 100644
--- a/src/olympia/abuse/admin.py
+++ b/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(
+ '
',
+ format_html_join('\n', '{}', 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)
diff --git a/src/olympia/abuse/tests/test_admin.py b/src/olympia/abuse/tests/test_admin.py
index a8108942120..aef22ab1ad5 100644
--- a/src/olympia/abuse/tests/test_admin.py
+++ b/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'
+ )
diff --git a/src/olympia/reviewers/admin.py b/src/olympia/reviewers/admin.py
index 5055c39a70a..d539ac7c4d2 100644
--- a/src/olympia/reviewers/admin.py
+++ b/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)
diff --git a/src/olympia/reviewers/migrations/0035_alter_reviewactionreason_cinder_policy.py b/src/olympia/reviewers/migrations/0035_alter_reviewactionreason_cinder_policy.py
new file mode 100644
index 00000000000..8b68e332b53
--- /dev/null
+++ b/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",
+ ),
+ ),
+ ]
diff --git a/src/olympia/reviewers/models.py b/src/olympia/reviewers/models.py
index 773e82e5e32..600a7d46eec 100644
--- a/src/olympia/reviewers/models.py
+++ b/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):
diff --git a/src/olympia/reviewers/tests/test_admin.py b/src/olympia/reviewers/tests/test_admin.py
index ec29b615289..e6d5128300e 100644
--- a/src/olympia/reviewers/tests/test_admin.py
+++ b/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'
+ )