Prevent deletion of Handle instances via ORM.

`Codelist` are created with one `Handle`. They may gain more later, but
business logic in `actions.py` does not permit deletion of any of them,
and parts of the application may expect that there is always a current
`Handle` for a `Codelist`. So we ought not permit it in application code.

The only models with deletion in `codelists/actions.py` or `builder/actions.py`
are `Codelist` (when discarding the last draft version), `CodelistVersion` (when
discarding drafts), and `Search` (when deleting a search in the builder).

We don't think that we have application code that does this (see `actions.py`)
but have observered it twice in production in recent months, see parent issue
#2893. This also helps
avoid any use of the developer Django shell to delete `Handles`.

We enforce this constraint only in the Django model layer. That does not
stop `Handles` being deleted by other means such as via direct SQL, such
as via `on_delete=models.CASCADE`, see
https://docs.djangoproject.com/en/5.2/topics/db/queries/#deleting-objects

> Keep in mind that this will, whenever possible, be executed purely in SQL,
> and so the delete() methods of individual object instances will not
> necessarily be called during the process.

That includes `Handle` when deleted via `Codelist` `CASCADE` as they do not
have any dependent models, triggers etc. See
https://github.com/django/django/blob/cb1d2854ed2b13799f2b0cc6e04019df181bacd4/django/db/models/deletion.py#L501

Trying locally via deleting a codelist draft in the web UI led to:

```
[debug    ] (0.007) DELETE FROM "codelists_handle" WHERE "codelists_handle"."codelist_id" IN (9979); args=(9979,); alias=default [django.db.backends]
```

... and I can find and delete multiple codelist with multiple handles in the
Django shell, which deletes the handles via cascade:

```
>>> from django.db.models import Count
>>> five_handle=Codelist.objects.annotate(hcount=Count("handles")).filter(hcount=5)
>>> len(five_handle)
2
>>> five_handle.delete()
<SUCCESS MESSAGE>

>>> five_handle=Codelist.objects.annotate(hcount=Count("handles")).filter(hcount=5)
>>> len(five_handle)
0

```

... and I cannot delete via the handle instances or `QuerySet` methods directly in the Django shell:

```
>>> three_handle_cl=Codelist.objects.annotate(hcount=Count("handles")).filter(hcount=3).first()
>>> three_handle_cl.handles.all().delete()
codelists.models.DeleteHandleException: Bulk deletion of Handle instances via the ORM is not permitted. Attempted on QuerySet `<NoDeleteHandleQuerySet [<Handle: Handle object (2800)>, <Handle: Handle object (2811)>, <Handle: Handle object (2908)>]>`
>>> three_handle_cl.handles.all().first().delete()
codelists.models.DeleteHandleException: May not delete handles - attempted on `Handle object (2800)` for codelist `BNF codes for NSAIDs (Medication Safety Indicator GIB01)`
```

Added automated tests for the overriden methods and manager queryset. There are
already automated tests for discarding drafts etc. in
`builder/tests/test_views.py`:
- test_discard_only_draft_version
- test_discard_one_draft_version

Author: mikerkelly

codelists/models.py

@@ -4,6 +4,7 @@
 from django.core.validators import MinLengthValidator, RegexValidator
 from django.db import models
 from django.db.models import Q
+from django.db.models.query import QuerySet
 from django.urls import reverse
 from django.utils.functional import cached_property
 from taggit.managers import TaggableManager
@@ -217,6 +218,19 @@ def has_published_versions(self):
         return self.versions.filter(status="published").exists()
 
 
+class DeleteHandleException(Exception):
+    """Raised when attempting to delete Handle via the ORM."""
+
+
+class NoDeleteHandleQuerySet(QuerySet):
+    def delete(self):
+        """Prevent deletion of Handle QuerySets via ORM."""
+        raise DeleteHandleException(
+            "Bulk deletion of Handle instances via the ORM is not permitted. "
+            f"Attempted on QuerySet `{self}`"
+        )
+
+
 class Handle(models.Model):
     codelist = models.ForeignKey(
         "Codelist", on_delete=models.CASCADE, related_name="handles"
@@ -271,6 +285,27 @@ class Meta:
     def owner(self):
         return self.organisation or self.user
 
+    # Attempt to stop deletion of Handle instances via the ORM.
+    # Codelist are created with one Handle. They may gain more later, but
+    # business logic in actions.py does not permit deletion of any of them,
+    # and parts of the application may expect that there is always a current
+    # Handle for a Codelist.
+    # Note that this does not stop deletion via on_delete=models.CASCADE, see
+    # https://docs.djangoproject.com/en/5.2/topics/db/queries/#deleting-objects
+    # We enforce this constraint only in the Django model layer. That does not
+    # stop Handles being deleted by other means such as via direct SQL,  such
+    # as via a cascade.
+
+    # Prevent deletion via QuerySet.
+    objects = NoDeleteHandleQuerySet.as_manager()
+
+    # Prevent deletion via instance method.
+    def delete(self, using=None, keep_parents=False):
+        """Prevent deletion of instances via the ORM."""
+        raise DeleteHandleException(
+            f"May not delete handles - attempted on `{self}` for codelist `{self.codelist}`"
+        )
+
 
 class Status(models.TextChoices):
     DRAFT = "draft"  # the version is being edited in the builder

codelists/tests/test_api.py

@@ -3,9 +3,10 @@
 from datetime import datetime
 
 import pytest
+from django.db import connection
 
 from codelists.actions import update_codelist
-from codelists.models import Codelist
+from codelists.models import Codelist, Handle
 from mappings.dmdvmpprevmap.models import Mapping as VmpPrevMapping
 from opencodelists.tests.assertions import assert_difference, assert_no_difference
 
@@ -272,9 +273,13 @@ def test_codelists_get_all_still_works_for_a_codelist_with_a_missing_handle(
     # We found a Codelist without an associated Handle,
     # which broke this API endpoint.
 
-    # Delete the Handle for one codelist.
+    # Delete the Handle for one codelist. Via SQL as we disallow via ORM.
     codelist_to_break = Codelist.objects.get(handles__name="User Codelist From Scratch")
-    codelist_to_break.current_handle.delete()
+    table = Handle._meta.db_table
+    with connection.cursor() as cursor:
+        cursor.execute(
+            f'DELETE FROM "{table}" WHERE id = {codelist_to_break.current_handle.pk}'
+        )
 
     rsp = client.get("/api/v1/codelist/?include-users")
     assert rsp.status_code == 200

codelists/tests/test_models.py

@@ -3,7 +3,7 @@
 from django.db.utils import IntegrityError
 
 from builder import actions as builder_actions
-from codelists.models import CodelistVersion, Handle
+from codelists.models import CodelistVersion, DeleteHandleException, Handle
 
 
 def test_handle_cannot_belong_to_user_and_organisation(codelist, user, organisation):
@@ -150,6 +150,16 @@ def test_handle_name_validation_valid(user_codelist):
     assert clean_name == name
 
 
+def test_handle_delete_not_allowed_method(codelist):
+    with pytest.raises(DeleteHandleException):
+        codelist.current_handle.delete()
+
+
+def test_handle_delete_not_allowed_queryset(codelist):
+    with pytest.raises(DeleteHandleException):
+        Handle.objects.first().delete()
+
+
 def test_old_style_codes(old_style_version):
     assert old_style_version.codes == (
         "128133004",