Protect lists against deletion if still used #10555

arches/app/models/models.py

@@ -33,6 +33,7 @@
 from django.template.loader import get_template, render_to_string
 from django.core.validators import MinValueValidator, RegexValidator
 from django.db.models import Q, Max
+from django.db.models.fields.json import KT
 from django.db.models.signals import post_delete, pre_save, post_save
 from django.dispatch import receiver
 from django.utils import translation
@@ -620,6 +621,16 @@ class Meta:
         )
 
 
+class WithControlledListManager(models.Manager):
+    def get_queryset(self):
+        return super().get_queryset().annotate(
+            controlled_list=Cast(
+                KT("config__controlledList"),
+                output_field=models.UUIDField(),
+            )
+        )
+
+
 class Node(models.Model):
     """
     Name is unique across all resources because it ties a node to values within tiles. Recommend prepending resource class to node name.
@@ -653,6 +664,11 @@ def __init__(self, *args, **kwargs):
         GraphXPublishedGraph, db_column="sourcebranchpublicationid", blank=True, null=True, on_delete=models.SET_NULL
     )
 
+    objects = models.Manager()
+    # custom manager provides indexed lookup on controlled lists, e.g.
+    # Node.with_controlled_list.filter(controlled_list=your_list_id_as_uuid)
+    with_controlled_list = WithControlledListManager()
+
     def get_child_nodes_and_edges(self):
         """
         gather up the child nodes and edges of this node
@@ -729,7 +745,7 @@ class Meta:
         ]
         indexes = [
             models.Index(
-                Cast(KT("config__controlledList"), output_field=models.UUIDField(null=True)),
+                Cast(KT("config__controlledList"), output_field=models.UUIDField()),
                 name="lists_reffed_by_node_idx",
             )
         ]

arches/app/src/components/ControlledListManager/ControlledListInventory.vue

@@ -111,14 +111,22 @@ const deleteLists = async (selectedLists: ControlledList[]) => {
                 props.setDisplayedList(null);
             }
         }
-        if (responses.some((resp) => !resp.ok)) {
-            throw new Error();
-        }
+        responses.forEach(async (response) => {
+            if (!response.ok) {
+                const body = await response.json();
+                toast.add({
+                    severity: ERROR,
+                    summary: $gettext("List deletion failed"),
+                    detail: body.message,
+                    life: 8000,
+                });
+            }
+        });
     } catch {
         toast.add({
             severity: ERROR,
-            summary: $gettext("One or more lists failed to delete."),
-            life: 3000,
+            summary: $gettext("List deletion failed"),
+            life: 5000,
         });
     }
     await fetchLists();

arches/app/views/controlled_lists.py

@@ -1,12 +1,11 @@
 from collections import defaultdict
 from datetime import datetime
+from typing import TYPE_CHECKING
 
 from django.contrib.postgres.expressions import ArraySubquery
 from django.core.exceptions import ValidationError
 from django.db import IntegrityError, transaction
-from django.db.models import Max, OuterRef, UUIDField
-from django.db.models.fields.json import KT
-from django.db.models.functions import Cast
+from django.db.models import Max, OuterRef
 from django.views.generic import View
 from django.utils.decorators import method_decorator
 from django.utils.translation import get_language, gettext as _
@@ -25,6 +24,10 @@
 from arches.app.utils.string_utils import str_to_bool
 
 
+if TYPE_CHECKING:
+    from uuid import UUID
+
+
 def serialize(obj, depth_map=None, flat=False, nodes=False):
     """
     This is a recursive function. The first caller (you) doesn't need
@@ -34,11 +37,9 @@ def serialize(obj, depth_map=None, flat=False, nodes=False):
     flat=True is just that, flat (used in reference datatype widget).
 
     nodes=True assumes a controlled list model instance has been
-    annotated with node_id and node_names for associated nodes.
-
-    The default nodes=False is mainly here to facilitate reusing
-    this as a helper method when setting up unit tests
-    (where no such annotation is done.)
+    .annotate()'d with various node-related arrays. (The default
+    nodes=False is mainly here to facilitate reusing this as a helper
+    method when setting up unit tests; the view should set nodes=True.
     """
     if depth_map is None:
         depth_map = defaultdict(int)
@@ -194,10 +195,8 @@ def get(self, request):
     @staticmethod
     def node_subquery(node_field: str = "pk"):
         return ArraySubquery(
-            Node.objects.annotate(
-                as_uuid=Cast(KT("config__controlledList"), output_field=UUIDField())
-            )
-            .filter(as_uuid=OuterRef("id"))
+            Node.with_controlled_list
+            .filter(controlled_list=OuterRef("id"))
             .order_by("pk")
             .values(node_field)
         )
@@ -274,8 +273,23 @@ def post(self, request, **kwargs):
         return JSONResponse(status=200)
 
     def delete(self, request, **kwargs):
-        list_id = kwargs.get("id")
-        objs_deleted, _ = ControlledList.objects.filter(pk=list_id).delete()
+        list_id: UUID = kwargs.get("id")
+        for node in Node.with_controlled_list.filter(controlled_list=list_id):
+            try:
+                lst = ControlledList.objects.get(id=list_id)
+            except ControlledList.DoesNotExist:
+                return JSONErrorResponse(status=404)
+            return JSONErrorResponse(
+                message=_(
+                    "{controlled_list} could not be deleted: still in use by {graph} - {node}".format(
+                        controlled_list=lst.name,
+                        graph=node.graph.name,
+                        node=node.name,
+                    )
+                ),
+                status=400,
+            )
+        objs_deleted, unused = ControlledList.objects.filter(pk=list_id).delete()
         if not objs_deleted:
             return JSONErrorResponse(status=404)
         return JSONResponse(status=204)