Restrict hard deletion of versioned concepts (solves https://github.com/OpenConceptLab/ocl_issues/issues/2570)

core/common/mixins.py

@@ -3,6 +3,7 @@
 from urllib import parse
 from urllib.parse import urlencode
 
+from celery.states import SUCCESS
 from django.core.cache import cache
 from django.core.exceptions import ValidationError
 from django.core.paginator import Paginator
@@ -634,6 +635,32 @@ def latest_source_version(self):
     def _cached_latest_source_version(self):
         return self.parent.get_latest_released_version()
 
+    def belongs_to_non_head_source_version(self):
+        versioned_object_id = self.versioned_object_id or self.id
+        version_ids = self.__class__.objects.filter(
+            parent_id=self.parent_id,
+            versioned_object_id=versioned_object_id,
+        ).values('id')
+        return self.sources.through.objects.filter(
+            **{f'{self._meta.model_name}_id__in': version_ids}
+        ).exclude(source__version=HEAD).exists()
+
+    def has_pending_source_version_seed(self):
+        """Return whether an unfinished source snapshot may include this resource."""
+        versioned_object_id = self.versioned_object_id or self.id
+        created_at = self.__class__.objects.filter(id=versioned_object_id).values_list(
+            'created_at', flat=True
+        ).first()
+        if not created_at:
+            return False
+
+        source_versions = self.parent.versions.exclude(version=HEAD).filter(created_at__gte=created_at)
+        for source_version in source_versions:
+            seed_task = source_version.get_seed_new_version_task()
+            if seed_task and seed_task.state != SUCCESS:
+                return True
+        return False
+
     @staticmethod
     def is_strictly_equal(instance1, instance2):
         return instance1.get_checksums() == instance2.get_checksums()

core/common/models.py

@@ -775,6 +775,7 @@ def persist_new(cls, obj, created_by, **kwargs):
 
     @classmethod
     def persist_new_version(cls, obj, user=None, **kwargs):
+        """Persist a repository version and schedule its children snapshot."""
         errors = {}
 
         obj.is_active = True
@@ -787,24 +788,39 @@ def persist_new_version(cls, obj, user=None, **kwargs):
         if not head:
             errors[repo_resource_name.lower()] = 'Version Head not found.'
             return errors
-        obj.update_version_data(head)
-        obj.save(**kwargs)
-
-        if obj.id:
-            obj.sibling_versions.update(is_latest_version=False)
 
         is_test_mode = get(settings, 'TEST_MODE', False)
-        if is_test_mode or sync:
-            seed_children_to_new_version(obj.resource_type.lower(), obj.id, not is_test_mode, sync)
-        else:
-            from core.tasks.models import Task
-            task = Task.new(queue='default', user=user, name=seed_children_to_new_version.__name__)
-            seed_children_to_new_version.apply_async(
-                (obj.resource_type.lower(), obj.id, True, sync),
-                task_id=task.id,
-                queue='default',
-                persist_args=True
-            )
+        with transaction.atomic():
+            # Serialize version creation with destructive child operations on the HEAD repository.
+            head = cls.objects.select_for_update().get(id=head.id)
+            obj.update_version_data(head)
+            obj.save(**kwargs)
+
+            if obj.id:
+                obj.sibling_versions.update(is_latest_version=False)
+
+            task_args = (obj.resource_type.lower(), obj.id, not is_test_mode, sync)
+            if is_test_mode or sync:
+                seed_children_to_new_version(*task_args)
+            else:
+                from core.tasks.models import Task
+                task = Task.new(
+                    queue='default',
+                    user=user,
+                    name=seed_children_to_new_version.__name__,
+                    args=task_args,
+                )
+
+                def enqueue_seed_task():
+                    """Queue snapshot seeding only after its repository version is committed."""
+                    seed_children_to_new_version.apply_async(
+                        task_args,
+                        task_id=task.id,
+                        queue='default',
+                        persist_args=True
+                    )
+
+                transaction.on_commit(enqueue_seed_task)
 
         return errors
 

core/concepts/constants.py

@@ -38,6 +38,9 @@
 CONCEPT_WAS_UNRETIRED = 'Concept was un-retired'
 CONCEPT_IS_ALREADY_RETIRED = 'Concept is already retired'
 CONCEPT_IS_ALREADY_NOT_RETIRED = 'Concept is already not retired'
+CONCEPT_HARD_DELETE_REQUIRES_HEAD_ONLY = (
+    'Concept cannot be hard deleted because it belongs to a source version.'
+)
 ALREADY_EXISTS = "Concept ID must be unique within a source."
 PERSIST_CLONE_SPECIFY_USER_ERROR = "Must specify which user is attempting to create a new version."
 PERSIST_CLONE_ERROR = 'An error occurred while saving new version.'

core/concepts/permissions.py

@@ -1,6 +1,6 @@
 from rest_framework.permissions import BasePermission, IsAuthenticatedOrReadOnly
 
-from core.common.permissions import CanViewConceptDictionary, CanEditConceptDictionary
+from core.common.permissions import CanViewConceptDictionary, CanEditConceptDictionary, HasPrivateAccess
 
 
 class CanAccessParentDictionary(BasePermission):
@@ -27,5 +27,10 @@ class CanEditParentDictionary(IsAuthenticatedOrReadOnly, CanAccessParentDictiona
     parent_permission_class = CanEditConceptDictionary
 
 
+class CanAdministerParentDictionary(IsAuthenticatedOrReadOnly, CanAccessParentDictionary, HasPrivateAccess):
+    """Repo admins: platform staff or the dictionary's owner/org-members (its authors/editors)."""
+    parent_permission_class = HasPrivateAccess
+
+
 class CanViewParentDictionaryAsGuest(CanAccessParentDictionary, CanViewConceptDictionary):
     parent_permission_class = CanViewConceptDictionary

core/concepts/views.py

@@ -41,10 +41,13 @@
                                drop_version, get_falsy_values)
 from core.common.views import SourceChildCommonBaseView, SourceChildExtrasView, \
     SourceChildExtraRetrieveUpdateDestroyView, BaseAPIView
-from core.concepts.constants import PARENT_VERSION_NOT_LATEST_CANNOT_UPDATE_CONCEPT
+from core.concepts.constants import (
+    CONCEPT_HARD_DELETE_REQUIRES_HEAD_ONLY,
+    PARENT_VERSION_NOT_LATEST_CANNOT_UPDATE_CONCEPT,
+)
 from core.concepts.documents import ConceptDocument
 from core.concepts.models import Concept, ConceptName
-from core.concepts.permissions import CanViewParentDictionary, CanEditParentDictionary
+from core.concepts.permissions import CanViewParentDictionary, CanEditParentDictionary, CanAdministerParentDictionary
 from core.concepts.search import ConceptFacetedSearch, ConceptFuzzySearch
 from core.concepts.serializers import (
     ConceptDetailSerializer, ConceptListSerializer, ConceptDescriptionSerializer, ConceptNameSerializer,
@@ -344,8 +347,14 @@ def get_permissions(self):
         if self.request.method in ['GET']:
             return [CanViewParentDictionary(), ]
 
-        if self.request.method == 'DELETE' and self.is_hard_delete_requested():
-            return [IsAdminUser(), ]
+        # async/db hard deletes are power operations reserved for repo admins (the dictionary's
+        # owner/org-members) and platform staff. A regular HEAD-only hard delete stays open to any
+        # repo editor. The HEAD-only safety check (409 for non-staff) is enforced in the handlers.
+        if (
+                self.request.method == 'DELETE' and self.is_hard_delete_requested() and
+                (self.is_async_requested() or self.is_db_delete_requested())
+        ):
+            return [CanAdministerParentDictionary(), ]
 
         return [CanEditParentDictionary(), ]
 
@@ -383,30 +392,79 @@ def update(self, request, *args, **kwargs):
     def is_db_delete_requested(self):
         return self.request.query_params.get('db', None) in TRUTHY
 
+    def _db_hard_delete(self):
+        parent_filters = Concept.get_parent_and_owner_filters_from_kwargs(self.kwargs)
+        concepts = Concept.objects.filter(mnemonic=self.kwargs['concept'], **parent_filters)
+        concept = concepts.filter(id=F('versioned_object_id')).first()
+        if not concept:
+            raise Http404()
+        self.check_object_permissions(self.request, concept)
+
+        # The raw delete skips the cascade path, so re-apply the HEAD-only guard non-staff would
+        # otherwise hit in `_hard_delete`, keeping snapshotted concepts protected from `db=true`.
+        if not IsAdminUser().has_permission(self.request, self) and (
+                concept.belongs_to_non_head_source_version() or
+                concept.has_pending_source_version_seed()
+        ):
+            return Response(
+                {'detail': CONCEPT_HARD_DELETE_REQUIRES_HEAD_ONLY},
+                status=status.HTTP_409_CONFLICT,
+            )
+
+        parent = concept.parent
+        result = concepts.delete()
+        parent.update_concepts_count()
+        return Response(result, status=status.HTTP_204_NO_CONTENT)
+
+    def _hard_delete(self, request, concept):
+        parent = concept.parent
+        with transaction.atomic():
+            # Source version creation locks the same HEAD row before registering its seed task.
+            parent.__class__.objects.select_for_update().get(id=concept.parent_id)
+            locked_concepts = list(Concept.objects.select_for_update().filter(
+                parent_id=concept.parent_id,
+                versioned_object_id=concept.versioned_object_id,
+            ))
+            versioned_concept = next(
+                (candidate for candidate in locked_concepts if candidate.id == concept.id),
+                None,
+            )
+            if not versioned_concept:
+                raise Http404()
+
+            is_admin = IsAdminUser().has_permission(request, self)
+            if not is_admin and (
+                    versioned_concept.belongs_to_non_head_source_version() or
+                    versioned_concept.has_pending_source_version_seed()
+            ):
+                return Response(
+                    {'detail': CONCEPT_HARD_DELETE_REQUIRES_HEAD_ONLY},
+                    status=status.HTTP_409_CONFLICT,
+                )
+
+            if self.is_async_requested():
+                task = Task.new(queue='default', user=request.user, name=delete_concept.__name__)
+                delete_concept.apply_async((concept.id,), queue=task.queue, task_id=task.id)
+                return Response(status=status.HTTP_204_NO_CONTENT)
+
+            # Versions reference the versioned concept with on_delete=CASCADE.
+            # Deleting the root removes every HEAD-only version and its related rows.
+            versioned_concept.delete()
+        parent.update_concepts_count()
+        return Response(status=status.HTTP_204_NO_CONTENT)
+
     def destroy(self, request, *args, **kwargs):
         if self.is_container_version_specified():
             return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED)
         is_hard_delete_requested = self.is_hard_delete_requested()
         if self.is_db_delete_requested() and is_hard_delete_requested:
-            parent_filters = Concept.get_parent_and_owner_filters_from_kwargs(self.kwargs)
-            concepts = Concept.objects.filter(mnemonic=self.kwargs['concept'], **parent_filters)
-            concept = concepts.filter(id=F('versioned_object_id')).first()
-            parent = concept.parent
-            result = concepts.delete()
-            parent.update_concepts_count()
-            return Response(result, status=status.HTTP_204_NO_CONTENT)
+            return self._db_hard_delete()
 
         concept = self.get_object()
         parent = concept.parent
 
         if is_hard_delete_requested:
-            if self.is_async_requested():
-                task = Task.new(queue='default', user=request.user, name=delete_concept.__name__)
-                delete_concept.apply_async((concept.id,), queue=task.queue, task_id=task.id)
-                return Response(status=status.HTTP_204_NO_CONTENT)
-            concept.delete()
-            parent.update_concepts_count()
-            return Response(status=status.HTTP_204_NO_CONTENT)
+            return self._hard_delete(request, concept)
 
         comment = request.data.get('update_comment', None) or request.data.get('comment', None)
         reason = request.data.get('retire_reason', None)
@@ -722,7 +780,10 @@ def update(self, request, **_):  # pylint: disable=arguments-differ
                         new_version, subject_label_attr, [*[locale.clone() for locale in locales.all()], saved_instance]
                     )
                     new_version.comment = f'Updated {saved_instance.name} in {self.parent_list_attribute}.'
-                    errors = new_version.save_as_new_version(request.user)
+                    # (Un)retiring a locale doesn't affect the standard checksum, so skip the
+                    # duplicate-version guard for it; content edits still change the checksum.
+                    errors = new_version.save_as_new_version(
+                        request.user, skip_duplicate_version_check='retired' in request.data)
                     if errors:
                         raise ValidationError(errors)
             except ValidationError as e:
@@ -751,7 +812,9 @@ def delete(self, request, *args, **kwargs):
             labels.append(retired_locale)
             setattr(new_version, subject_label_attr, labels)
             new_version.comment = f'Retired {instance.name} in {self.parent_list_attribute}.'
-            errors = new_version.save_as_new_version(request.user)
+            # Retiring a locale is an intentional change, but the standard checksum ignores the
+            # locale `retired` flag, so skip the duplicate-version guard that would block it.
+            errors = new_version.save_as_new_version(request.user, skip_duplicate_version_check=True)
             if errors:
                 return Response(errors, status=status.HTTP_400_BAD_REQUEST)
         return Response(status=status.HTTP_204_NO_CONTENT)