diff --git a/errata/admin.py b/errata/admin.py index d50567e..8a955e5 100644 --- a/errata/admin.py +++ b/errata/admin.py @@ -3,7 +3,12 @@ from simple_history.admin import SimpleHistoryAdmin from django.contrib import admin +from django.utils import timezone +from .mail import ( + send_erratum_classified_notification, + send_new_erratum_notification, +) from .models import ( Erratum, StagedErratum, @@ -13,6 +18,10 @@ DirtyBits, ) +# Statuses that represent a classification of a previously "reported" erratum. +# Matches the transitions handled by errata.views.reported_classify. +CLASSIFIED_STATUS_SLUGS = {"verified", "rejected", "held_for_doc_update"} + class ErratumAdmin(SimpleHistoryAdmin): search_fields = ["rfc_number", "verifier_name", "verifier_email", "submitter_email"] @@ -28,6 +37,53 @@ class ErratumAdmin(SimpleHistoryAdmin): ] list_filter = ["status", "erratum_type"] + def save_model(self, request, obj, form, change): + """Trigger the same notifications the public workflow sends. + + Editing errata through the admin bypasses the views in errata.views, + so we mirror their notification side effects here: + + * Creating an erratum in the "reported" state notifies stakeholders + the same way promoting a staged erratum does + (see staged_rpc_add_to_unverified). + * Moving a "reported" erratum to a classified state notifies the same + way reported_classify does. The acting user is recorded as the + verifier only when the form leaves the verifier fields blank, so an + admin can attribute the classification to someone else. + """ + notify_new = False + notify_classified = False + + if not change: + notify_new = obj.status_id == "reported" + else: + previous_status = ( + Erratum.objects.filter(pk=obj.pk) + .values_list("status_id", flat=True) + .first() + ) + if ( + previous_status == "reported" + and obj.status_id in CLASSIFIED_STATUS_SLUGS + ): + notify_classified = True + # Preserve verifier details entered in the admin form; only + # fall back to the acting user when they were left blank. This + # lets an admin record a classification performed by someone + # else while still defaulting to themselves in the common case. + if not obj.verifier_email: + obj.verifier_name = request.user.name + obj.verifier_email = request.user.email + if obj.verified_at is None: + obj.verified_at = timezone.now() + + super().save_model(request, obj, form, change) + + if notify_new: + send_new_erratum_notification(obj, request.user) + elif notify_classified: + send_erratum_classified_notification(obj, request.user) + admin.site.register(Erratum, ErratumAdmin) diff --git a/errata/mail.py b/errata/mail.py index 4271525..93bdb66 100644 --- a/errata/mail.py +++ b/errata/mail.py @@ -193,8 +193,10 @@ def send_erratum_classified_notification(erratum, user): ) to = [] cc = [] - verifier_email = user.email - assert verifier_email == erratum.verifier_email + # The verifier of record is whoever is recorded on the erratum, which is + # not necessarily the user triggering the notification (e.g. an admin may + # record a classification performed by someone else). + verifier_email = erratum.verifier_email if erratum.erratum_type.slug == "technical": if stream == "legacy": to.append(erratum.submitter_email) diff --git a/errata/tests.py b/errata/tests.py index c2c352b..ff0e627 100644 --- a/errata/tests.py +++ b/errata/tests.py @@ -5,6 +5,7 @@ from django.test import TestCase, override_settings from django.urls import reverse +from django.utils import timezone from errata.factories import ( ErratumFactory, @@ -1125,3 +1126,103 @@ def test_can_classify_false_for_nonexistent_erratum(self): from errata.utils import can_classify self.assertFalse(can_classify(self.rpc_user, 999999)) + + +class ErratumAdminTest(TestCase): + """Admin edits should fire the same notifications as the public workflow.""" + + def setUp(self): + from django.contrib import admin as django_admin + from django.test import RequestFactory + + from errata.admin import ErratumAdmin + + self.admin = ErratumAdmin(Erratum, django_admin.site) + self.user = RpcUserFactory() + self.request = RequestFactory().post("/admin/") + self.request.user = self.user + self.rfc = RfcMetadataFactory() + + def _build_erratum(self, status_slug): + return Erratum( + rfc_number=self.rfc.rfc_number, + rfc_metadata=self.rfc, + status=Status.objects.get(slug=status_slug), + erratum_type=ErratumType.objects.get(slug="technical"), + section="1", + orig_text="Original text", + corrected_text="Corrected text", + submitter_name="Test Submitter", + submitter_email="submitter@example.com", + submitted_at=timezone.now(), + ) + + @patch("errata.admin.send_new_erratum_notification") + def test_create_reported_erratum_sends_new_notification(self, mock_notify): + obj = self._build_erratum("reported") + self.admin.save_model(self.request, obj, form=None, change=False) + self.assertIsNotNone(obj.pk) + mock_notify.assert_called_once_with(obj, self.user) + + @patch("errata.admin.send_new_erratum_notification") + def test_create_non_reported_erratum_does_not_notify(self, mock_notify): + obj = self._build_erratum("verified") + self.admin.save_model(self.request, obj, form=None, change=False) + mock_notify.assert_not_called() + + @patch("errata.admin.send_erratum_classified_notification") + def test_classifying_reported_erratum_sends_classified_notification( + self, mock_notify + ): + erratum = ErratumFactory( + rfc_metadata=self.rfc, rfc_number=self.rfc.rfc_number + ) + erratum.status = Status.objects.get(slug="verified") + self.admin.save_model(self.request, erratum, form=None, change=True) + erratum.refresh_from_db() + self.assertEqual(erratum.status_id, "verified") + # With no verifier supplied, the acting admin is recorded as verifier. + self.assertEqual(erratum.verifier_name, self.user.name) + self.assertEqual(erratum.verifier_email, self.user.email) + self.assertIsNotNone(erratum.verified_at) + mock_notify.assert_called_once_with(erratum, self.user) + + @patch("errata.admin.send_erratum_classified_notification") + def test_classifying_preserves_admin_supplied_verifier(self, mock_notify): + erratum = ErratumFactory( + rfc_metadata=self.rfc, rfc_number=self.rfc.rfc_number + ) + erratum.status = Status.objects.get(slug="verified") + erratum.verifier_name = "Real Verifier" + erratum.verifier_email = "real.verifier@example.com" + self.admin.save_model(self.request, erratum, form=None, change=True) + erratum.refresh_from_db() + # Verifier details entered in the form are kept, not overwritten by the + # acting admin. + self.assertEqual(erratum.verifier_name, "Real Verifier") + self.assertEqual(erratum.verifier_email, "real.verifier@example.com") + mock_notify.assert_called_once_with(erratum, self.user) + + @patch("errata.admin.send_erratum_classified_notification") + @patch("errata.admin.send_new_erratum_notification") + def test_editing_reported_erratum_without_status_change_does_not_notify( + self, mock_new, mock_classified + ): + erratum = ErratumFactory( + rfc_metadata=self.rfc, rfc_number=self.rfc.rfc_number + ) + erratum.notes = "Edited via admin" + self.admin.save_model(self.request, erratum, form=None, change=True) + mock_new.assert_not_called() + mock_classified.assert_not_called() + + @patch("errata.admin.send_erratum_classified_notification") + def test_editing_already_classified_erratum_does_not_notify(self, mock_notify): + erratum = ErratumFactory( + rfc_metadata=self.rfc, + rfc_number=self.rfc.rfc_number, + status=Status.objects.get(slug="verified"), + ) + erratum.notes = "Edited again" + self.admin.save_model(self.request, erratum, form=None, change=True) + mock_notify.assert_not_called()