diff --git a/apps/annotations/migrations/0010_graph_deleted_at_graph_deleted_by.py b/apps/annotations/migrations/0010_graph_deleted_at_graph_deleted_by.py new file mode 100644 index 0000000..909dad7 --- /dev/null +++ b/apps/annotations/migrations/0010_graph_deleted_at_graph_deleted_by.py @@ -0,0 +1,29 @@ +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("annotations", "0009_alter_graph_allograph"), + ] + + operations = [ + migrations.AddField( + model_name="graph", + name="deleted_at", + field=models.DateTimeField(blank=True, db_index=True, null=True), + ), + migrations.AddField( + model_name="graph", + name="deleted_by", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="+", + to=settings.AUTH_USER_MODEL, + ), + ), + ] diff --git a/apps/annotations/models.py b/apps/annotations/models.py index b3600a2..4178581 100644 --- a/apps/annotations/models.py +++ b/apps/annotations/models.py @@ -1,7 +1,18 @@ from django.db import models +from apps.common.models import SoftDeleteModel -class Graph(models.Model): + +class GraphQuerySet(models.QuerySet): + def live(self) -> GraphQuerySet: + """Rows not in the trash — every public/read surface must use this.""" + return self.filter(deleted_at__isnull=True) # type: ignore[no-any-return] + + def trashed(self) -> GraphQuerySet: + return self.filter(deleted_at__isnull=False) # type: ignore[no-any-return] + + +class Graph(SoftDeleteModel): class AnnotationType(models.TextChoices): IMAGE = "image", "Image" TEXT = "text", "Text" @@ -38,6 +49,8 @@ class AnnotationType(models.TextChoices): # sparkline, which silently ignores null-created rows. created = models.DateTimeField(auto_now_add=True, null=True, blank=True, db_index=True) + objects = GraphQuerySet.as_manager() + class Meta: ordering = ["id"] constraints = [ diff --git a/apps/annotations/schema.yaml b/apps/annotations/schema.yaml index 608642e..ac1b640 100644 --- a/apps/annotations/schema.yaml +++ b/apps/annotations/schema.yaml @@ -92,6 +92,41 @@ paths: summary: List graphs for management security: - api_key: [] + parameters: + - in: query + name: deleted + schema: + type: boolean + required: false + description: >- + When true, list trashed graphs (soft-deleted, newest first) instead + of live ones. Default lists live graphs only. + - in: query + name: annotation_type + schema: + type: string + enum: [image, text, editorial, unknown] + required: false + - in: query + name: deleted_by__username + schema: + type: string + required: false + description: Username of the user who trashed the row. + - in: query + name: deleted_at__gte + schema: + type: string + format: date-time + required: false + description: Only rows trashed at or after this instant (ISO 8601). + - in: query + name: deleted_at__lte + schema: + type: string + format: date-time + required: false + description: Only rows trashed at or before this instant (ISO 8601). responses: 200: description: List graphs for management. @@ -123,6 +158,72 @@ paths: $ref: '#/components/schemas/Graph' tags: - annotations-management + /api/v1/management/annotations/graphs/trash-actors/: + get: + operationId: management-graphs-trash-actors + summary: Usernames that currently have graphs in the trash + description: >- + Distinct, sorted usernames of users with at least one trashed graph — + the option list for the trash "deleted by" filter. Rows whose deleter + no longer exists are omitted. + security: + - api_key: [] + responses: + 200: + description: Usernames with at least one trashed graph. + content: + application/json: + schema: + type: array + items: + type: string + tags: + - annotations-management + /api/v1/management/annotations/graphs/{id}/restore/: + post: + operationId: management-graphs-restore + summary: Restore a trashed graph + security: + - api_key: [] + parameters: + - name: id + in: path + required: true + schema: + type: integer + responses: + 200: + description: The restored graph (deleted_at/deleted_by cleared). + content: + application/json: + schema: + $ref: '#/components/schemas/Graph' + 404: + description: No trashed graph with this id. + tags: + - annotations-management + /api/v1/management/annotations/graphs/{id}/purge/: + delete: + operationId: management-graphs-purge + summary: Permanently delete a trashed graph + security: + - api_key: [] + parameters: + - name: id + in: path + required: true + schema: + type: integer + responses: + 204: + description: >- + Trashed graph permanently deleted (cascades to its components; + for TEXT graphs the corresp reference is stripped from the + transcription). Cannot be undone. + 404: + description: No trashed graph with this id. + tags: + - annotations-management /api/v1/management/annotations/graph-components/: get: operationId: management-graph-components-list diff --git a/apps/annotations/serializers.py b/apps/annotations/serializers.py index bafae19..285af97 100644 --- a/apps/annotations/serializers.py +++ b/apps/annotations/serializers.py @@ -126,6 +126,7 @@ class GraphManagementSerializer(GraphDescriptionMixin, serializers.ModelSerializ position_details = serializers.SerializerMethodField(read_only=True) num_features = serializers.SerializerMethodField() is_described = serializers.SerializerMethodField() + deleted_by = serializers.SlugRelatedField(slug_field="username", read_only=True) class Meta: model = Graph @@ -147,7 +148,11 @@ class Meta: "graphcomponent_set", "num_features", "is_described", + "created", + "deleted_at", + "deleted_by", ] + read_only_fields = ["created", "deleted_at", "deleted_by"] def _replace_graph_components(graph: Graph, components_data: list[dict]) -> None: diff --git a/apps/annotations/tests/test_graph_trash.py b/apps/annotations/tests/test_graph_trash.py new file mode 100644 index 0000000..fec7bc1 --- /dev/null +++ b/apps/annotations/tests/test_graph_trash.py @@ -0,0 +1,318 @@ +"""Graph trash (soft delete): every delete path trashes instead of destroying, +trashed rows leak into no read surface, and restore/purge behave as documented. + +Trash is a save() — the pre_delete corresp-strip signal must NOT fire (so a +restored TEXT graph keeps its text link); purge is a real delete and must fire +it. See content_trash_feature_plan.md. +""" + +import pytest +import rest_framework + +from apps.annotations.models import Graph +from apps.annotations.tests.factories import GraphFactory +from apps.common.models import EditEvent +from apps.manuscripts.models import ImageText +from apps.manuscripts.tests.factories import ItemImageFactory +from apps.scribes.services import get_scribe_idiographs +from apps.search.documents.item_images import build_item_image_document +from apps.search.registry import get_queryset_for_index +from apps.search.types import IndexType + +VIEWER_URL = "/api/v1/annotations/graphs/" +PUBLIC_URL = "/api/v1/manuscripts/graphs/" +MANAGEMENT_URL = "/api/v1/management/annotations/graphs/" + + +@pytest.mark.django_db +def test_viewer_delete_moves_to_trash(authenticated_client): + graph = GraphFactory() + + res = authenticated_client.delete(f"{VIEWER_URL}{graph.id}/") + + assert res.status_code == rest_framework.status.HTTP_204_NO_CONTENT + graph.refresh_from_db() + assert graph.deleted_at is not None + assert graph.deleted_by is not None + # A trashed row is invisible to the viewer write surface too. + assert authenticated_client.delete(f"{VIEWER_URL}{graph.id}/").status_code == 404 + assert authenticated_client.patch(f"{VIEWER_URL}{graph.id}/", {"note": "x"}, format="json").status_code == 404 + + +@pytest.mark.django_db +def test_management_delete_moves_to_trash(management_client): + graph = GraphFactory() + + res = management_client.delete(f"{MANAGEMENT_URL}{graph.id}/") + + assert res.status_code == rest_framework.status.HTTP_204_NO_CONTENT + graph.refresh_from_db() + assert graph.deleted_at is not None + + +@pytest.mark.django_db +def test_trashed_graph_hidden_from_read_surfaces(management_client): + graph = GraphFactory() + image = graph.item_image + graph.soft_delete() + + # Public list + detail (superuser client = widest visibility). + listed = management_client.get(f"{PUBLIC_URL}?item_image={image.id}") + assert all(row["id"] != graph.id for row in listed.data) + assert management_client.get(f"{PUBLIC_URL}{graph.id}/").status_code == 404 + # W3C annotation endpoint. + assert management_client.get(f"/api/v1/annotations-w3c/graphs/{graph.id}/").status_code == 404 + # Aggregate counts on the owning image. + assert image.number_of_annotations() == 0 + + +@pytest.mark.django_db +def test_management_list_deleted_param(management_client): + live = GraphFactory() + trashed = GraphFactory(item_image=live.item_image, allograph=live.allograph, hand=live.hand) + trashed.soft_delete() + + default_rows = management_client.get(MANAGEMENT_URL).data["results"] + assert {row["id"] for row in default_rows} == {live.id} + + trash_rows = management_client.get(f"{MANAGEMENT_URL}?deleted=true").data["results"] + assert {row["id"] for row in trash_rows} == {trashed.id} + assert trash_rows[0]["deleted_at"] is not None + + +@pytest.mark.django_db +def test_trash_list_filters(management_client): + """The trash surface filters by annotation type, who trashed it, and when.""" + from datetime import timedelta + + from django.utils import timezone + + from apps.users.tests.factories import UserFactory + + alice = UserFactory(username="alice") + bob = UserFactory(username="bob") + image = ItemImageFactory() + + old = GraphFactory(item_image=image) + old.soft_delete(user=alice) + Graph.objects.filter(pk=old.pk).update(deleted_at=timezone.now() - timedelta(days=10)) + + recent = GraphFactory(item_image=image, allograph=old.allograph, hand=old.hand) + recent.soft_delete(user=bob) + + editorial = GraphFactory( + item_image=image, allograph=None, hand=None, annotation_type=Graph.AnnotationType.EDITORIAL + ) + editorial.soft_delete(user=bob) + + def ids(query: str) -> set[int]: + res = management_client.get(f"{MANAGEMENT_URL}?deleted=true&{query}") + assert res.status_code == rest_framework.status.HTTP_200_OK, res.data + return {row["id"] for row in res.data["results"]} + + assert ids("") == {old.id, recent.id, editorial.id} + assert ids("annotation_type=editorial") == {editorial.id} + assert ids("deleted_by__username=alice") == {old.id} + assert ids("deleted_by__username=bob") == {recent.id, editorial.id} + + # "Z" rather than "+00:00": a raw "+" in a query string decodes to a space, + # which the date parser rejects. This is the shape the frontend sends + # (Date.toISOString()), so the test exercises the real contract. + cutoff = (timezone.now() - timedelta(days=1)).isoformat().replace("+00:00", "Z") + assert ids(f"deleted_at__gte={cutoff}") == {recent.id, editorial.id} + assert ids(f"deleted_at__lte={cutoff}") == {old.id} + # Filters compose. + assert ids(f"deleted_at__gte={cutoff}&deleted_by__username=bob&annotation_type=editorial") == {editorial.id} + + +@pytest.mark.django_db +def test_trash_actors_lists_only_users_with_trashed_rows(management_client, authenticated_client): + from apps.users.tests.factories import UserFactory + + alice = UserFactory(username="alice") + UserFactory(username="never_deleted_anything") + image = ItemImageFactory() + + # Two rows from alice: she must appear exactly once, not twice. Meta.ordering + # would silently break the DISTINCT if the view didn't override it. + first = GraphFactory(item_image=image) + first.soft_delete(user=alice) + GraphFactory(item_image=image, allograph=first.allograph, hand=first.hand).soft_delete(user=alice) + + # A live row's owner is not a trash actor. + GraphFactory(item_image=image, allograph=first.allograph, hand=first.hand) + # Neither is a trashed row with no recorded deleter. + GraphFactory(item_image=image, allograph=first.allograph, hand=first.hand).soft_delete() + + res = management_client.get(f"{MANAGEMENT_URL}trash-actors/") + + assert res.status_code == rest_framework.status.HTTP_200_OK + assert res.data == ["alice"] + + # Superuser-only, like the rest of the management surface. + assert authenticated_client.get(f"{MANAGEMENT_URL}trash-actors/").status_code == 403 + + +@pytest.mark.django_db +def test_live_list_filters_still_work(management_client): + """Switching filterset_fields to dict form must not rename existing params.""" + image = ItemImageFactory() + live = GraphFactory(item_image=image, annotation_type=Graph.AnnotationType.IMAGE) + # GraphFactory leaves annotation_type NULL by default, so this row must not + # match an `annotation_type=image` filter. + GraphFactory(item_image=image, allograph=live.allograph, hand=live.hand) + + res = management_client.get(f"{MANAGEMENT_URL}?annotation_type=image&item_image={image.id}") + + assert res.status_code == rest_framework.status.HTTP_200_OK + assert {row["id"] for row in res.data["results"]} == {live.id} + + +@pytest.mark.django_db +def test_restore(management_client, authenticated_client): + graph = GraphFactory() + graph.soft_delete() + + # Restore is superuser-only. + assert authenticated_client.post(f"{MANAGEMENT_URL}{graph.id}/restore/").status_code == 403 + + res = management_client.post(f"{MANAGEMENT_URL}{graph.id}/restore/") + assert res.status_code == rest_framework.status.HTTP_200_OK + graph.refresh_from_db() + assert graph.deleted_at is None + assert graph.deleted_by is None + # Restoring a live row 404s (restore targets the trash only). + assert management_client.post(f"{MANAGEMENT_URL}{graph.id}/restore/").status_code == 404 + + +@pytest.mark.django_db +def test_purge(management_client, authenticated_client): + graph = GraphFactory() + graph_id = graph.id + + # Purge targets trashed rows only, and is superuser-only. + assert management_client.delete(f"{MANAGEMENT_URL}{graph_id}/purge/").status_code == 404 + graph.soft_delete() + assert authenticated_client.delete(f"{MANAGEMENT_URL}{graph_id}/purge/").status_code == 403 + + res = management_client.delete(f"{MANAGEMENT_URL}{graph_id}/purge/") + assert res.status_code == rest_framework.status.HTTP_204_NO_CONTENT + assert not Graph.objects.filter(id=graph_id).exists() + event = EditEvent.objects.filter(target_type="graph", target_id=graph_id, action=EditEvent.Action.DELETED).first() + assert event is not None + assert event.actor is not None + + +@pytest.mark.django_db +def test_non_superuser_cannot_trash_editorial(authenticated_client): + editorial = GraphFactory(allograph=None, hand=None, annotation_type=Graph.AnnotationType.EDITORIAL) + + assert authenticated_client.delete(f"{VIEWER_URL}{editorial.id}/").status_code == 404 + editorial.refresh_from_db() + assert editorial.deleted_at is None + + +@pytest.mark.django_db +def test_trash_preserves_corresp_and_purge_strips_it(management_client): + image = ItemImageFactory() + graph = Graph.objects.create( + item_image=image, + annotation={"type": "Feature", "geometry": {"type": "Polygon", "coordinates": []}}, + annotation_type="text", + ) + text = ImageText.objects.create( + item_image=image, + content=f'

Alpha

', + type=ImageText.Type.TRANSCRIPTION, + status=ImageText.Status.DRAFT, + language="la", + ) + + # Trash: a save(), so the pre_delete corresp-strip must NOT run. + res = management_client.delete(f"{MANAGEMENT_URL}{graph.id}/") + assert res.status_code == rest_framework.status.HTTP_204_NO_CONTENT + text.refresh_from_db() + assert f"gid-{graph.id}" in text.content + + # Restore: the link is intact with no replay logic. + management_client.post(f"{MANAGEMENT_URL}{graph.id}/restore/") + text.refresh_from_db() + assert f"gid-{graph.id}" in text.content + + # Purge: a real delete — the signal strips the reference. + graph.refresh_from_db() + graph.soft_delete() + res = management_client.delete(f"{MANAGEMENT_URL}{graph.id}/purge/") + assert res.status_code == rest_framework.status.HTTP_204_NO_CONTENT + text.refresh_from_db() + assert f"gid-{graph.id}" not in text.content + + +@pytest.mark.django_db +def test_unlink_region_still_hard_deletes(management_client): + """Deliberate decision: unlink-region keeps its hard delete (it strips the + ref first, so a restored region would be an unreachable orphan).""" + image = ItemImageFactory() + graph = Graph.objects.create( + item_image=image, + annotation={"type": "Feature", "geometry": {"type": "Polygon", "coordinates": []}}, + annotation_type="text", + ) + text = ImageText.objects.create( + item_image=image, + content=f'

Alpha

', + type=ImageText.Type.TRANSCRIPTION, + status=ImageText.Status.DRAFT, + language="la", + ) + + res = management_client.post( + f"/api/v1/manuscripts/management/image-texts/{text.id}/unlink-region/", + {"graph_id": graph.id}, + format="json", + ) + + assert res.status_code == 200 + assert not Graph.objects.filter(id=graph.id).exists() + + +@pytest.mark.django_db +def test_search_queryset_and_image_document_exclude_trashed(): + graph = GraphFactory() + image = graph.item_image + + assert graph.id in set(get_queryset_for_index(IndexType.GRAPHS).values_list("id", flat=True)) + assert build_item_image_document(image)["number_of_annotations"] == 1 + + graph.soft_delete() + + assert graph.id not in set(get_queryset_for_index(IndexType.GRAPHS).values_list("id", flat=True)) + assert build_item_image_document(image)["number_of_annotations"] == 0 + + +@pytest.mark.django_db +def test_scribe_idiographs_exclude_trashed(): + graph = GraphFactory() + scribe = graph.hand.scribe + + assert [a.id for a in get_scribe_idiographs(scribe)] == [graph.allograph_id] + + graph.soft_delete() + + assert get_scribe_idiographs(scribe) == [] + + +@pytest.mark.django_db +def test_components_of_trashed_graph_hidden(management_client): + from apps.annotations.tests.factories import GraphComponentFactory + + gc = GraphComponentFactory() + url = "/api/v1/management/annotations/graph-components/" + + rows = management_client.get(f"{url}?graph={gc.graph_id}").data["results"] + assert {row["id"] for row in rows} == {gc.id} + + gc.graph.soft_delete() + + rows = management_client.get(f"{url}?graph={gc.graph_id}").data["results"] + assert rows == [] diff --git a/apps/annotations/views.py b/apps/annotations/views.py index 4b19dbb..97569ba 100644 --- a/apps/annotations/views.py +++ b/apps/annotations/views.py @@ -1,11 +1,19 @@ from django.db.models import Count, QuerySet from django_filters import rest_framework as filters -from rest_framework import viewsets +from rest_framework import status, viewsets +from rest_framework.decorators import action from rest_framework.exceptions import PermissionDenied from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response from apps.annotations.models import Graph, GraphComponent -from apps.common.views import ActionSerializerMixin, AuditActorMixin, FilterablePrivilegedViewSet +from apps.common.audit import audit_actor +from apps.common.views import ( + ActionSerializerMixin, + AuditActorMixin, + FilterablePrivilegedViewSet, + TrashableViewSetMixin, +) from .serializers import ( GraphComponentManagementSerializer, @@ -32,7 +40,7 @@ class GraphViewSet(viewsets.ReadOnlyModelViewSet): filterset_fields = ["item_image", "annotation_type", "hand", "allograph"] def get_queryset(self): - queryset = super().get_queryset() + queryset = super().get_queryset().live() user = getattr(self.request, "user", None) if getattr(user, "is_authenticated", False): @@ -41,7 +49,7 @@ def get_queryset(self): return queryset.exclude(annotation_type=Graph.AnnotationType.EDITORIAL) -class GraphViewerWriteViewSet(AuditActorMixin, viewsets.ModelViewSet): +class GraphViewerWriteViewSet(TrashableViewSetMixin, AuditActorMixin, viewsets.ModelViewSet): permission_classes = [IsAuthenticated] queryset = ( Graph.objects.select_related("allograph", "hand", "item_image") @@ -56,7 +64,8 @@ class GraphViewerWriteViewSet(AuditActorMixin, viewsets.ModelViewSet): http_method_names = ["post", "patch", "delete", "head", "options"] def get_queryset(self): - queryset = super().get_queryset() + # Restore/purge live on the management API, so trashed rows are out of reach here. + queryset = super().get_queryset().live() user = getattr(self.request, "user", None) if getattr(user, "is_superuser", False): return queryset @@ -73,11 +82,12 @@ def perform_create(self, serializer): super().perform_create(serializer) -class GraphManagementViewSet(ActionSerializerMixin, FilterablePrivilegedViewSet): +class GraphManagementViewSet(TrashableViewSetMixin, ActionSerializerMixin, FilterablePrivilegedViewSet): queryset = ( # item_image__item_part is joined because the management serializer - # reads item_image.item_part.historical_item_id per row. - Graph.objects.select_related("allograph", "hand", "item_image", "item_image__item_part") + # reads item_image.item_part.historical_item_id per row; deleted_by + # because the trash list shows who trashed each row. + Graph.objects.select_related("allograph", "hand", "item_image", "item_image__item_part", "deleted_by") .prefetch_related( "positions", "graphcomponent_set__component", @@ -85,7 +95,17 @@ class GraphManagementViewSet(ActionSerializerMixin, FilterablePrivilegedViewSet) ) .annotate(num_features=Count("graphcomponent__features")) ) - filterset_fields = ["item_image", "annotation_type", "hand", "allograph"] + # Dict form so the trash can filter a `deleted_at` range. `exact` takes no + # suffix, so the existing `?annotation_type=` / `?hand=` params are unchanged. + filterset_fields = { + "item_image": ["exact"], + "annotation_type": ["exact"], + "hand": ["exact"], + "allograph": ["exact"], + # Username, matching what the serializer exposes. + "deleted_by__username": ["exact"], + "deleted_at": ["gte", "lte"], + } serializer_class = GraphManagementSerializer action_serializer_classes = { @@ -94,10 +114,55 @@ class GraphManagementViewSet(ActionSerializerMixin, FilterablePrivilegedViewSet) "partial_update": GraphWriteManagementSerializer, } + def get_queryset(self): + queryset = super().get_queryset() + if self.action in ("restore", "purge"): + # Both target the trash, so a live id 404s. + return queryset.trashed() + if self.action == "list" and self.request.query_params.get("deleted") in ("true", "1"): + return queryset.trashed().order_by("-deleted_at") + return queryset.live() + + @action(detail=False, methods=["get"], url_path="trash-actors") + def trash_actors(self, request): + """Usernames that currently have something in the trash. + + Backs the "deleted by" filter, so it never offers a value that returns + no rows. Not `get_queryset()`: its `annotate()` GROUP BY would break the + DISTINCT. The explicit `order_by` overrides `Meta.ordering`, which would + otherwise add `id` to the SELECT and make rows distinct per row. + """ + usernames = ( + Graph.objects.trashed() + .exclude(deleted_by__isnull=True) + .order_by("deleted_by__username") + .values_list("deleted_by__username", flat=True) + .distinct() + ) + return Response(list(usernames)) + + @action(detail=True, methods=["post"]) + def restore(self, request, pk=None): + graph = self.get_object() + with audit_actor(request.user): + graph.restore() + return Response(self.get_serializer(graph).data) + + @action(detail=True, methods=["delete"]) + def purge(self, request, pk=None): + """Hard-delete a trashed row. Unlike trash, this is a real delete: the + pre_delete corresp-strip and the EditEvent `deleted` signal both fire.""" + graph = self.get_object() + with audit_actor(request.user): + graph.delete() + return Response(status=status.HTTP_204_NO_CONTENT) + class GraphComponentManagementViewSet(FilterablePrivilegedViewSet): queryset: QuerySet[GraphComponent] = ( - GraphComponent.objects.select_related("component").prefetch_related("features").all() + GraphComponent.objects.select_related("component") + .prefetch_related("features") + .filter(graph__deleted_at__isnull=True) ) serializer_class = GraphComponentManagementSerializer filterset_fields = ["graph"] diff --git a/apps/annotations_w3c/views.py b/apps/annotations_w3c/views.py index 352aafb..98e70b3 100644 --- a/apps/annotations_w3c/views.py +++ b/apps/annotations_w3c/views.py @@ -43,7 +43,7 @@ def _visible_image_texts(request: Request): @permission_classes([]) def graph_annotation(request: Request, graph_id: int) -> Response: """A single image region as a W3C Web Annotation.""" - graph = get_object_or_404(Graph.objects.select_related("item_image"), pk=graph_id) + graph = get_object_or_404(Graph.objects.live().select_related("item_image"), pk=graph_id) doc = graph_to_w3c(graph, base_url=_base_url(request), image_height=_image_height(graph.item_image)) return Response(doc, content_type=_JSONLD) @@ -54,7 +54,7 @@ def image_text_page(request: Request, text_id: int) -> Response: """An ImageText's linked elements as a W3C AnnotationPage.""" image_text = get_object_or_404(_visible_image_texts(request), pk=text_id) wanted = referenced_graph_ids(image_text.content or "") - graph_lookup = {g.id: g for g in Graph.objects.filter(id__in=wanted).select_related("item_image")} + graph_lookup = {g.id: g for g in Graph.objects.live().filter(id__in=wanted).select_related("item_image")} doc = imagetext_to_w3c( image_text, graph_lookup=graph_lookup, diff --git a/apps/common/models.py b/apps/common/models.py index 0797a1e..1a0f234 100644 --- a/apps/common/models.py +++ b/apps/common/models.py @@ -1,5 +1,36 @@ from django.conf import settings from django.db import models +from django.utils import timezone + + +class SoftDeleteModel(models.Model): + """A row with `deleted_at` set is trashed, not gone. + + The default manager stays unfiltered, so every read path that must hide + trashed rows filters explicitly. A real `.delete()` still purges. + """ + + deleted_at = models.DateTimeField(null=True, blank=True, db_index=True) + deleted_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name="+", + ) + + class Meta: + abstract = True + + def soft_delete(self, user=None) -> None: + self.deleted_at = timezone.now() + self.deleted_by = user if getattr(user, "is_authenticated", False) else None + self.save(update_fields=["deleted_at", "deleted_by"]) + + def restore(self) -> None: + self.deleted_at = None + self.deleted_by = None + self.save(update_fields=["deleted_at", "deleted_by"]) class Date(models.Model): diff --git a/apps/common/views.py b/apps/common/views.py index 5f4b047..c6a4f7e 100644 --- a/apps/common/views.py +++ b/apps/common/views.py @@ -41,6 +41,18 @@ def perform_destroy(self, instance): super().perform_destroy(instance) +class TrashableViewSetMixin: + """Turn DRF's destroy into a soft delete for SoftDeleteModel rows. + + List before AuditActorMixin/ModelViewSet so this perform_destroy wins. + Being a save(), it does not fire pre_delete/post_delete — only a purge does. + """ + + def perform_destroy(self, instance): + with audit_actor(getattr(self.request, "user", None)): + instance.soft_delete(user=getattr(self.request, "user", None)) + + class BasePrivilegedViewSet(AuditActorMixin, viewsets.ModelViewSet): """All privileged ViewSets require superuser permissions.""" diff --git a/apps/iiif_presentation/views.py b/apps/iiif_presentation/views.py index 2492d1a..62227eb 100644 --- a/apps/iiif_presentation/views.py +++ b/apps/iiif_presentation/views.py @@ -78,7 +78,7 @@ def _load_item_part_iiif_data(request: Request, item_part_id: int): texts_by_image.setdefault(text.item_image_id, []).append(text) wanted |= referenced_graph_ids(text.content or "") - graph_lookup = {g.id: g for g in Graph.objects.filter(id__in=wanted).select_related("item_image")} + graph_lookup = {g.id: g for g in Graph.objects.live().filter(id__in=wanted).select_related("item_image")} return item_part, images, texts_by_image, graph_lookup diff --git a/apps/manuscripts/models.py b/apps/manuscripts/models.py index 29248e3..61003cd 100644 --- a/apps/manuscripts/models.py +++ b/apps/manuscripts/models.py @@ -216,10 +216,10 @@ class Meta: ordering = ["item_part", "locus"] def number_of_annotations(self): - return self.graphs.count() + return self.graphs.filter(deleted_at__isnull=True).count() def number_of_image_annotations(self): - return self.graphs.filter(annotation_type="image").count() + return self.graphs.filter(annotation_type="image", deleted_at__isnull=True).count() def __str__(self) -> str: return f"{self.item_part} (locus: {self.locus})" diff --git a/apps/manuscripts/tests/test_tei_links.py b/apps/manuscripts/tests/test_tei_links.py index 5eb090b..92e1751 100644 --- a/apps/manuscripts/tests/test_tei_links.py +++ b/apps/manuscripts/tests/test_tei_links.py @@ -410,9 +410,12 @@ def test_image_graph_delete_leaves_text_untouched(): @pytest.mark.django_db -def test_graph_viewer_write_delete_endpoint_strips_corresp(authenticated_client): - # The HTTP delete path (e.g. backoffice / viewer write viewset) also strips - # corresp via the signal — the dangling-corresp gap is closed server-side. +def test_graph_viewer_write_delete_trashes_and_preserves_corresp(authenticated_client): + # The viewer delete is now a soft delete (trash): the row survives with + # deleted_at set and the corresp reference is deliberately left in place, + # so a restore brings the text↔region link back with no replay logic. + # The corresp-strip signal fires only on a real delete (purge / cascade) — + # covered in apps/annotations/tests/test_graph_trash.py. image = ItemImageFactory() graph = Graph.objects.create( item_image=image, @@ -430,9 +433,10 @@ def test_graph_viewer_write_delete_endpoint_strips_corresp(authenticated_client) res = authenticated_client.delete(f"/api/v1/annotations/graphs/{graph.id}/") assert res.status_code in (200, 204) - assert not Graph.objects.filter(id=graph.id).exists() + graph.refresh_from_db() + assert graph.deleted_at is not None text.refresh_from_db() - assert f"gid-{graph.id}" not in text.content + assert f"gid-{graph.id}" in text.content @pytest.mark.django_db diff --git a/apps/manuscripts/views.py b/apps/manuscripts/views.py index 58a9c6a..9a38b30 100644 --- a/apps/manuscripts/views.py +++ b/apps/manuscripts/views.py @@ -173,10 +173,10 @@ class ImageViewSet(GenericViewSet, ListModelMixin, RetrieveModelMixin): # applied verbatim. queryset = ( ItemImage.objects.annotate( - annotation_count=Count("graphs", distinct=True), + annotation_count=Count("graphs", filter=Q(graphs__deleted_at__isnull=True), distinct=True), image_annotation_count=Count( "graphs", - filter=Q(graphs__annotation_type="image"), + filter=Q(graphs__annotation_type="image", graphs__deleted_at__isnull=True), distinct=True, ), ) @@ -252,7 +252,9 @@ def regions(self, request: Request, pk: str | None = None) -> Response: wanted = {gid for ref in refs for gid in ref.graph_ids} graphs = { g.id: g - for g in Graph.objects.filter(id__in=wanted).only("id", "annotation_type", "annotation", "item_image") + for g in Graph.objects.live() + .filter(id__in=wanted) + .only("id", "annotation_type", "annotation", "item_image") } out = [] for ref in refs: @@ -415,7 +417,9 @@ def tei(self, request: Request, pk=None) -> HttpResponse: class ItemImageManagementViewSet(FilterablePrivilegedViewSet): queryset = ( - ItemImage.objects.prefetch_related("texts").annotate(annotation_count=Count("graphs", distinct=True)).all() + ItemImage.objects.prefetch_related("texts") + .annotate(annotation_count=Count("graphs", filter=Q(graphs__deleted_at__isnull=True), distinct=True)) + .all() ) serializer_class = ItemImageManagementSerializer filterset_fields = ["item_part"] @@ -558,7 +562,11 @@ def link_region(self, request: Request, pk=None) -> Response: graph = None if graph_id is not None: - graph = Graph.objects.filter(id=graph_id, annotation_type="text", item_image_id=text.item_image_id).first() + graph = ( + Graph.objects.live() + .filter(id=graph_id, annotation_type="text", item_image_id=text.item_image_id) + .first() + ) if graph is None: return Response( {"detail": "No text region with that graph_id on this image."}, diff --git a/apps/scribes/services.py b/apps/scribes/services.py index 1617951..1652860 100644 --- a/apps/scribes/services.py +++ b/apps/scribes/services.py @@ -25,10 +25,18 @@ def get_scribe_idiographs(scribe: Scribe) -> list[Allograph]: hands = scribe.hand_set.all() for hand in hands: for graph in hand.graph_set.all(): + # Filtered in python, not via Prefetch: scribes may not import the + # annotations app (CI-enforced boundary). + if graph.deleted_at is not None: + continue allograph = graph.allograph idiographs_by_id[allograph.id] = allograph if idiographs_by_id: return sorted(idiographs_by_id.values(), key=lambda allograph: allograph.name.lower()) - return list(Allograph.objects.filter(graph__hand__scribe=scribe).distinct().select_related("character")) + return list( + Allograph.objects.filter(graph__hand__scribe=scribe, graph__deleted_at__isnull=True) + .distinct() + .select_related("character") + ) diff --git a/apps/search/documents/item_images.py b/apps/search/documents/item_images.py index e08d943..36de113 100644 --- a/apps/search/documents/item_images.py +++ b/apps/search/documents/item_images.py @@ -5,7 +5,9 @@ def build_item_image_document(obj) -> dict: """Build a search document from an ItemImage instance.""" - graphs = list(obj.graphs.all()) + # Filtered in python because the registry's prefetch_related is a plain + # string tuple (no Prefetch objects). + graphs = [g for g in obj.graphs.all() if g.deleted_at is None] components = [] features = [] component_features = [] diff --git a/apps/search/documents/texts.py b/apps/search/documents/texts.py index 43432b9..2468fbd 100644 --- a/apps/search/documents/texts.py +++ b/apps/search/documents/texts.py @@ -81,7 +81,7 @@ def _get_annotation_coordinates(annotation_id: int | None) -> str | None: if annotation_id is None: return None try: - annotation = Graph.objects.only("annotation").get(id=annotation_id).annotation + annotation = Graph.objects.live().only("annotation").get(id=annotation_id).annotation except Graph.DoesNotExist: return None if annotation is None: diff --git a/apps/search/documents/utils.py b/apps/search/documents/utils.py index 3584b60..68e827b 100644 --- a/apps/search/documents/utils.py +++ b/apps/search/documents/utils.py @@ -47,7 +47,7 @@ def annotation_coordinates_map(entries: list[dict]) -> dict[int, str]: return {} coordinates_by_id = {} - graphs = Graph.objects.filter(id__in=annotation_ids) + graphs = Graph.objects.live().filter(id__in=annotation_ids) if hasattr(graphs, "only"): graphs = graphs.only("id", "annotation") for graph in graphs: diff --git a/apps/search/quality_endpoints.py b/apps/search/quality_endpoints.py index 077e83d..4562669 100644 --- a/apps/search/quality_endpoints.py +++ b/apps/search/quality_endpoints.py @@ -56,7 +56,9 @@ def _untyped_clauses() -> dict: def _undescribed_graphs() -> dict: - qs = Graph.objects.annotate(component_count=Count("graphcomponent")).filter(component_count=0).order_by("-id") + qs = ( + Graph.objects.live().annotate(component_count=Count("graphcomponent")).filter(component_count=0).order_by("-id") + ) return { "id": "undescribed-graphs", "label": "Graphs with no components", @@ -84,8 +86,10 @@ def _orphan_text_graphs() -> dict: `data-graph-id`. If every text on its image is blank, nothing can refer to it — that's a likely orphan. """ - qs = Graph.objects.filter(annotation_type=Graph.AnnotationType.TEXT).exclude( - item_image__texts__content__regex=r".+" + qs = ( + Graph.objects.live() + .filter(annotation_type=Graph.AnnotationType.TEXT) + .exclude(item_image__texts__content__regex=r".+") ) return { "id": "orphan-text-graphs", diff --git a/apps/search/registry.py b/apps/search/registry.py index 98a06bb..eba094e 100644 --- a/apps/search/registry.py +++ b/apps/search/registry.py @@ -315,6 +315,8 @@ def url_segment(self) -> str: "scribe", "components", ], + # Also feeds the admin "in sync" expected count, so both stay symmetric. + queryset_filter={"deleted_at__isnull": True}, ), IndexType.TEXTS: IndexRegistration( index_type=IndexType.TEXTS, diff --git a/apps/search/tests/test_annotation_id_documents.py b/apps/search/tests/test_annotation_id_documents.py index 880e10f..75fddcd 100644 --- a/apps/search/tests/test_annotation_id_documents.py +++ b/apps/search/tests/test_annotation_id_documents.py @@ -70,7 +70,9 @@ def test_clause_people_place_builders_emit_annotation_id_or_null(): 77: SimpleNamespace(id=77, annotation={"type": "Feature", "geometry": {"type": "Polygon"}}), } utils_docs.Graph.objects = SimpleNamespace( - filter=lambda **kwargs: [g for gid, g in graphs_by_id.items() if gid in kwargs.get("id__in", ())] + live=lambda: SimpleNamespace( + filter=lambda **kwargs: [g for gid, g in graphs_by_id.items() if gid in kwargs.get("id__in", ())] + ) ) clause_docs = clauses_docs.build_clause_documents(obj) @@ -92,8 +94,10 @@ def test_text_builder_sets_annotation_id_when_any_dpt_annotation_exists(): ) texts_docs.Graph.objects = SimpleNamespace( - only=lambda *_: SimpleNamespace( - get=lambda **__: SimpleNamespace(annotation={"type": "Feature", "geometry": {"type": "Polygon"}}) + live=lambda: SimpleNamespace( + only=lambda *_: SimpleNamespace( + get=lambda **__: SimpleNamespace(annotation={"type": "Feature", "geometry": {"type": "Polygon"}}) + ) ) ) diff --git a/apps/search/text_monitoring_endpoints.py b/apps/search/text_monitoring_endpoints.py index 1049117..a4057ff 100644 --- a/apps/search/text_monitoring_endpoints.py +++ b/apps/search/text_monitoring_endpoints.py @@ -193,7 +193,7 @@ def _annotation_activity(days: int = 30) -> list[dict[str, Any]]: """ cutoff = timezone.now() - timedelta(days=days) - qs = Graph.objects.filter(annotation_type=Graph.AnnotationType.TEXT, created__gte=cutoff).values("created") + qs = Graph.objects.live().filter(annotation_type=Graph.AnnotationType.TEXT, created__gte=cutoff).values("created") buckets: dict[str, int] = defaultdict(int) for row in qs: buckets[row["created"].date().isoformat()] += 1 @@ -209,7 +209,7 @@ def _annotation_health() -> dict[str, Any]: total = ImageText.objects.count() with_content = ImageText.objects.exclude(content="").count() - annotations_total = Graph.objects.filter(annotation_type=Graph.AnnotationType.TEXT).count() + annotations_total = Graph.objects.live().filter(annotation_type=Graph.AnnotationType.TEXT).count() return { "image_texts_total": total, "image_texts_with_content": with_content,