fix(ui): say why a blocked delete failed instead of printing the error repr

dojo/api_v2/exception_handler.py

@@ -5,7 +5,6 @@
 from django.db.utils import IntegrityError
 from django.utils.log import log_response
 from django.utils.translation import gettext as _
-from django.utils.translation import ngettext
 from rest_framework.exceptions import ParseError
 from rest_framework.exceptions import ValidationError as DRFValidationError
 from rest_framework.response import Response
@@ -22,6 +21,7 @@
 from dojo.location.api.deprecation import sunset_body
 from dojo.models import System_Settings
 from dojo.product_announcements import ErrorPageProductAnnouncement
+from dojo.utils import restricted_delete_message
 
 logger = logging.getLogger(__name__)
 
@@ -123,23 +123,10 @@ def custom_exception_handler(exc, context):
         response.status_code = HTTP_400_BAD_REQUEST
         response.data = {"message": _("JSON request content is malformed")}
     elif isinstance(exc, RestrictedError):
-        # An object cannot be deleted because it has dependent objects. str() of the error is a tuple repr that
-        # names every one of them, so the message counts them and names only their kinds (what to reassign).
+        # An object cannot be deleted because it has dependent objects; say how many and of which kinds.
         response = Response()
         response.status_code = HTTP_409_CONFLICT
-        response.data = {}
-        restricted = list(exc.restricted_objects or ())
-        if restricted:
-            message = ngettext(
-                "It cannot be deleted while %(count)s other record still refers to it.",
-                "It cannot be deleted while %(count)s other records still refer to it.",
-                len(restricted),
-            ) % {"count": len(restricted)}
-            kinds = sorted({str(obj._meta.verbose_name) for obj in restricted})
-            message += " " + _("Records that refer to it: %(kinds)s.") % {"kinds": ", ".join(kinds)}
-            response.data["message"] = message
-        else:
-            response.data["message"] = _("It cannot be deleted while other records still refer to it.")
+        response.data = {"message": restricted_delete_message(exc)}
     elif isinstance(exc, ValidationError):
         response = Response()
         response.status_code = HTTP_400_BAD_REQUEST

dojo/development_environment/ui/views.py

@@ -13,7 +13,7 @@
 from dojo.development_environment.models import Development_Environment
 from dojo.development_environment.ui.forms import Delete_Dev_EnvironmentForm, Development_EnvironmentForm
 from dojo.filters import DevelopmentEnvironmentFilter
-from dojo.utils import add_breadcrumb, get_page_items
+from dojo.utils import add_breadcrumb, get_page_items, restricted_delete_reason
 
 logger = logging.getLogger(__name__)
 
@@ -83,7 +83,7 @@ def edit_dev_env(request, deid):
             except RestrictedError as err:
                 messages.add_message(request,
                                         messages.WARNING,
-                                        f"Environment cannot be deleted: {err}",
+                                        _("Environment cannot be deleted: %(reason)s") % {"reason": restricted_delete_reason(err)},
                                         extra_tags="alert-warning")
             return HttpResponseRedirect(reverse("dev_env"))
 

dojo/product_attributes/ui/views.py

@@ -23,7 +23,7 @@
     ProductOriginForm,
     ProductPlatformForm,
 )
-from dojo.utils import add_breadcrumb, get_page_items
+from dojo.utils import add_breadcrumb, get_page_items, restricted_delete_reason
 
 logger = logging.getLogger(__name__)
 
@@ -124,7 +124,8 @@ def _edit(request, kind, pk):
                                  extra_tags="alert-success")
         except RestrictedError as err:
             messages.add_message(request, messages.WARNING,
-                                 f"{cfg['label']} cannot be deleted: {err}",
+                                 _("%(label)s cannot be deleted: %(reason)s") % {
+                                     "label": cfg["label"], "reason": restricted_delete_reason(err)},
                                  extra_tags="alert-warning")
         return HttpResponseRedirect(reverse(cfg["list_url"]))
     add_breadcrumb(title=f"Edit {cfg['label']}", top_level=False, request=request)

dojo/user/ui/views.py

@@ -54,7 +54,13 @@
     UserContactInfoForm,
 )
 from dojo.user.utils import user_may_delete_account
-from dojo.utils import add_breadcrumb, get_page_items, get_setting, get_system_setting
+from dojo.utils import (
+    add_breadcrumb,
+    get_page_items,
+    get_setting,
+    get_system_setting,
+    restricted_delete_reason,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -497,7 +503,7 @@ def delete_user(request, uid):
                     except RestrictedError as err:
                         messages.add_message(request,
                                             messages.WARNING,
-                                            _("User cannot be deleted: %(error)s") % {"error": err},
+                                            _("User cannot be deleted: %(error)s") % {"error": restricted_delete_reason(err)},
                                             extra_tags="alert-warning")
                     return HttpResponseRedirect(reverse("users"))
 

dojo/utils.py

@@ -55,6 +55,7 @@
 from django.utils.html import escape, format_html
 from django.utils.http import url_has_allowed_host_and_scheme
 from django.utils.translation import gettext as _
+from django.utils.translation import ngettext
 from kombu import Connection
 
 from dojo.celery import app
@@ -231,6 +232,45 @@ def findings_this_period(findings, period_type, stuff, o_stuff, a_stuff):
         a_stuff.append(a_counts)
 
 
+def _restricted_kinds(restricted):
+    return ", ".join(sorted({str(obj._meta.verbose_name) for obj in restricted}))
+
+
+def restricted_delete_message(err):
+    """
+    A sentence saying why a delete was blocked by dependent rows, for a person to read.
+
+    str() of a RestrictedError is a tuple repr that names every referencing row, so this counts them and names
+    only their kinds (what to reassign).
+    """
+    restricted = list(err.restricted_objects or ())
+    if not restricted:
+        return _("It cannot be deleted while other records still refer to it.")
+    message = ngettext(
+        "It cannot be deleted while %(count)s other record still refers to it.",
+        "It cannot be deleted while %(count)s other records still refer to it.",
+        len(restricted),
+    ) % {"count": len(restricted)}
+    return message + " " + _("Records that refer to it: %(kinds)s.") % {"kinds": _restricted_kinds(restricted)}
+
+
+def restricted_delete_reason(err):
+    """
+    The reason alone, for a warning that already says what cannot be deleted ("User cannot be deleted: ...").
+
+    Counts the referencing rows and names their kinds, as :func:`restricted_delete_message` does.
+    """
+    restricted = list(err.restricted_objects or ())
+    if not restricted:
+        return _("other records still refer to it.")
+    reason = ngettext(
+        "%(count)s other record still refers to it.",
+        "%(count)s other records still refer to it.",
+        len(restricted),
+    ) % {"count": len(restricted)}
+    return reason + " " + _("Records that refer to it: %(kinds)s.") % {"kinds": _restricted_kinds(restricted)}
+
+
 def add_breadcrumb(parent=None,
                    title=None,
                    *,

unittests/test_classic_delete_warning_message.py

@@ -0,0 +1,81 @@
+"""
+Regression tests: a Classic UI delete blocked by dependent rows warns with a sentence, not a tuple repr.
+
+The user, environment and product attribute delete views put ``str(err)`` of a ``RestrictedError`` in their
+warning. That is a Python tuple repr naming every referencing row, so a user referenced by many findings got a
+very long banner listing them. The warning now counts them and names their kinds, as the API's answer to a
+blocked delete does.
+"""
+
+from unittest.mock import patch
+
+from django.contrib.messages import get_messages
+from django.db.models.deletion import RestrictedError
+from django.test import override_settings
+from django.urls import reverse
+
+from dojo.models import Development_Environment, Dojo_User, Finding, Product_Platform
+from unittests.dojo_test_case import DojoTestCase, versioned_fixtures
+
+BLOCKED = "2 other records still refer to it. Records that refer to it: finding."
+
+
+def _blocked():
+    return RestrictedError(
+        "Cannot delete some instances of model 'X'",
+        {Finding(id=1, title="secret finding one"), Finding(id=2, title="secret finding two")},
+    )
+
+
+@versioned_fixtures
+@override_settings(SECURE_SSL_REDIRECT=False)
+class ClassicDeleteWarningMessageTest(DojoTestCase):
+    fixtures = ["dojo_testdata.json"]
+
+    def setUp(self):
+        self.client.force_login(Dojo_User.objects.get(username="admin"))
+
+    def _warning(self, response):
+        warnings = [str(m) for m in get_messages(response.wsgi_request) if m.level_tag == "warning"]
+        self.assertEqual(1, len(warnings), warnings)
+        self.assertNotIn("secret", warnings[0])
+        self.assertNotIn("<Finding", warnings[0])
+        return warnings[0]
+
+    def test_a_blocked_user_delete(self):
+        user = Dojo_User.objects.create(username="blocked-user-for-delete")
+        with patch.object(Dojo_User, "delete", side_effect=_blocked()):
+            response = self.client.post(reverse("delete_user", args=(user.id,)), {"id": user.id})
+        self.assertEqual(f"User cannot be deleted: {BLOCKED}", self._warning(response))
+
+    def test_a_blocked_environment_delete(self):
+        environment = Development_Environment.objects.create(name="Blocked Environment")
+        with patch.object(Development_Environment, "delete", side_effect=_blocked()):
+            response = self.client.post(
+                reverse("edit_dev_env", args=(environment.id,)), {"delete_dev_env": "1", "id": environment.id},
+            )
+        self.assertEqual(f"Environment cannot be deleted: {BLOCKED}", self._warning(response))
+
+    def test_a_blocked_product_attribute_delete(self):
+        platform = Product_Platform.objects.create(name="Blocked Platform")
+        with patch.object(Product_Platform, "delete", side_effect=_blocked()):
+            response = self.client.post(
+                reverse("edit_product_platform", args=(platform.id,)), {"delete_product_platform": "1"},
+            )
+        self.assertEqual(f"Platform cannot be deleted: {BLOCKED}", self._warning(response))
+
+    def test_one_referencing_record_reads_in_the_singular(self):
+        user = Dojo_User.objects.create(username="blocked-by-one")
+        blocked = RestrictedError("Cannot delete some instances of model 'X'", {Finding(id=1, title="secret")})
+        with patch.object(Dojo_User, "delete", side_effect=blocked):
+            response = self.client.post(reverse("delete_user", args=(user.id,)), {"id": user.id})
+        self.assertEqual(
+            "User cannot be deleted: 1 other record still refers to it. Records that refer to it: finding.",
+            self._warning(response),
+        )
+
+    def test_an_error_without_its_records_still_reads(self):
+        user = Dojo_User.objects.create(username="blocked-by-unknown")
+        with patch.object(Dojo_User, "delete", side_effect=RestrictedError("Cannot delete", set())):
+            response = self.client.post(reverse("delete_user", args=(user.id,)), {"id": user.id})
+        self.assertEqual("User cannot be deleted: other records still refer to it.", self._warning(response))