From 6ee4e5c3e791c2f64f8d5b24f66783ce082ca960 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Tue, 26 May 2026 08:29:09 +0530 Subject: [PATCH 1/2] OpenConceptLab/ocl_issues#2519 | concept/mapping.retire_reason --- core/common/mixins.py | 20 ++- core/concept_maps/serializers.py | 2 +- .../0082_concept_retire_reason_and_more.py | 28 ++++ core/concepts/models.py | 18 ++- core/concepts/serializers.py | 9 +- core/concepts/tests/tests.py | 77 +++++++++- core/concepts/views.py | 3 +- core/importers/models.py | 17 ++- core/integration_tests/tests_concepts.py | 142 +++++++++++++++++- core/integration_tests/tests_mappings.py | 1 + .../migrations/0058_mapping_retire_reason.py | 18 +++ core/mappings/models.py | 18 ++- core/mappings/serializers.py | 2 +- core/mappings/views.py | 3 +- 14 files changed, 320 insertions(+), 38 deletions(-) create mode 100644 core/concepts/migrations/0082_concept_retire_reason_and_more.py create mode 100644 core/mappings/migrations/0058_mapping_retire_reason.py diff --git a/core/common/mixins.py b/core/common/mixins.py index 2af38335..67afe5e3 100644 --- a/core/common/mixins.py +++ b/core/common/mixins.py @@ -6,7 +6,7 @@ from django.core.cache import cache from django.core.exceptions import ValidationError from django.core.paginator import Paginator -from django.db import transaction +from django.db import transaction, models from django.db.models import Q, F, QuerySet from django.http import HttpResponseForbidden, Http404 from django.shortcuts import get_object_or_404, redirect @@ -601,6 +601,15 @@ def get_repo_events(self, private=False): class SourceChildMixin(ChecksumModel): + external_id = models.TextField(null=True, blank=True) + comment = models.TextField(null=True, blank=True) + retire_reason = models.TextField(null=True, blank=True) + versioned_object = models.ForeignKey( + 'self', related_name='versions_set', null=True, blank=True, on_delete=models.CASCADE + ) + _counted = models.BooleanField(default=True, null=True, blank=True) + _index = models.BooleanField(default=True) + class Meta: abstract = True @@ -721,23 +730,24 @@ def parent_resource(self): def parent_url(self): return get(self.parent, 'uri') - def retire(self, user, comment=None): + def retire(self, user, comment=None, reason=None): if self.versioned_object.retired: return {'__all__': self.ALREADY_RETIRED} - return self.__update_retire(True, comment or self.WAS_RETIRED, user) + return self.__update_retire(True, user, comment or self.WAS_RETIRED, reason) def unretire(self, user, comment=None): if not self.versioned_object.retired: return {'__all__': self.ALREADY_NOT_RETIRED} - return self.__update_retire(False, comment or self.WAS_UNRETIRED, user) + return self.__update_retire(False, user, comment or self.WAS_UNRETIRED) - def __update_retire(self, retired, comment, user): + def __update_retire(self, retired, user, comment, reason=None): latest_version = self.get_latest_version() or self.get_last_version() new_version = latest_version.clone() new_version.retired = retired new_version.comment = comment + new_version.retire_reason = reason if retired else None return new_version.save_as_new_version(user) @classmethod diff --git a/core/concept_maps/serializers.py b/core/concept_maps/serializers.py index d7d191b1..91ca3d7c 100644 --- a/core/concept_maps/serializers.py +++ b/core/concept_maps/serializers.py @@ -245,7 +245,7 @@ def update(self, instance, validated_data): if ConceptMapDetailSerializer.is_mapping_same(mapping, new_mapping): found = True if not found: - mapping.retire(user, 'Deleted from ConceptMap resource') + mapping.retire(user, None, 'Deleted from ConceptMap resource') source.refresh_from_db() diff --git a/core/concepts/migrations/0082_concept_retire_reason_and_more.py b/core/concepts/migrations/0082_concept_retire_reason_and_more.py new file mode 100644 index 00000000..24aff42c --- /dev/null +++ b/core/concepts/migrations/0082_concept_retire_reason_and_more.py @@ -0,0 +1,28 @@ +# Generated by Django 5.1.15 on 2026-05-26 02:43 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('concepts', '0081_remove_conceptname_preferred_locale_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='concept', + name='retire_reason', + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name='conceptdescription', + name='retire_reason', + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name='conceptname', + name='retire_reason', + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/core/concepts/models.py b/core/concepts/models.py index 36cd5235..97591bee 100644 --- a/core/concepts/models.py +++ b/core/concepts/models.py @@ -34,6 +34,7 @@ class Meta: locale_preferred = models.BooleanField(default=False) created_at = models.DateTimeField(auto_now_add=True) retired = models.BooleanField(default=False) + retire_reason = models.TextField(null=True, blank=True) SMART_CHECKSUM_KEY = None @@ -226,23 +227,16 @@ class Meta: ) ] + VersionedModel.Meta.indexes - external_id = models.TextField(null=True, blank=True) concept_class = models.TextField() datatype = models.TextField() - comment = models.TextField(null=True, blank=True) parent = models.ForeignKey('sources.Source', related_name='concepts_set', on_delete=models.CASCADE) sources = models.ManyToManyField('sources.Source', related_name='concepts') - versioned_object = models.ForeignKey( - 'self', related_name='versions_set', null=True, blank=True, on_delete=models.CASCADE - ) parent_concepts = models.ManyToManyField( 'self', through='HierarchicalConcepts', symmetrical=False, related_name='child_concepts' ) mnemonic = models.CharField( max_length=255, validators=[RegexValidator(regex=CONCEPT_REGEX)], ) - _counted = models.BooleanField(default=True, null=True, blank=True) - _index = models.BooleanField(default=True) logo_path = None name = None full_name = None @@ -531,6 +525,7 @@ def clone(self): concept_class=self.concept_class, datatype=self.datatype, retired=self.retired, + retire_reason=self.retire_reason, released=self.released, extras=self.extras or {}, parent=self.parent, @@ -584,6 +579,14 @@ def create_new_version_for( instance.extras = data.get('extras', instance.extras) instance.external_id = data.get('external_id', instance.external_id) instance.comment = data.get('update_comment') or data.get('comment') + instance.retire_reason = data.get('retire_reason', instance.retire_reason) + is_retired = data.get('retired', None) + if is_retired is True and not instance.retired: + instance.retire_comment = instance.comment + instance.comment = instance.WAS_RETIRED + elif instance.retired and is_retired is False: + instance.retire_comment = None + instance.comment = instance.comment or instance.WAS_RETIRED instance.retired = data.get('retired', instance.retired) if is_patch: prev = instance.versions.exclude(id=instance.id).filter(is_latest_version=True).first() @@ -1143,6 +1146,7 @@ def update_versioned_object(self): concept.concept_class = self.concept_class concept.datatype = self.datatype concept.retired = self.retired + concept.retire_reason = self.retire_reason concept.external_id = self.external_id or concept.external_id concept.updated_by_id = self.updated_by_id concept.save() diff --git a/core/concepts/serializers.py b/core/concepts/serializers.py index fb5df1ec..8b01990e 100644 --- a/core/concepts/serializers.py +++ b/core/concepts/serializers.py @@ -307,7 +307,7 @@ class Meta: 'owner', 'owner_type', 'owner_url', 'display_name', 'display_locale', 'version', 'update_comment', 'locale', 'version_created_by', 'version_created_on', 'mappings', 'is_latest_version', 'versions_url', 'version_url', 'extras', 'type', 'versioned_object_id', 'version_updated_on', 'version_updated_by', - 'latest_source_version', 'property' + 'latest_source_version', 'property', 'retire_reason' ) @@ -466,7 +466,7 @@ class Meta: 'owner', 'owner_type', 'owner_url', 'display_name', 'display_locale', 'names', 'descriptions', 'created_on', 'updated_on', 'versions_url', 'version', 'extras', 'parent_id', 'type', 'update_comment', 'version_url', 'updated_by', 'created_by', - 'public_can_view', 'versioned_object_id', 'latest_source_version', 'property' + 'public_can_view', 'versioned_object_id', 'latest_source_version', 'property', 'retire_reason' ) def create(self, validated_data): @@ -518,7 +518,8 @@ class Meta: 'names', 'descriptions', 'extras', 'retired', 'source', 'source_url', 'owner', 'owner_name', 'owner_url', 'version', 'created_on', 'updated_on', 'version_created_on', 'version_created_by', 'update_comment', 'is_latest_version', 'locale', 'url', 'owner_type', 'version_url', 'previous_version_url', - 'parent_concept_urls', 'child_concept_urls', 'version_updated_on', 'version_updated_by', 'checksums' + 'parent_concept_urls', 'child_concept_urls', 'version_updated_on', 'version_updated_by', 'checksums', + 'retire_reason' ) @staticmethod @@ -597,7 +598,7 @@ class Meta: 'is_latest_version', 'locale', 'url', 'owner_type', 'version_url', 'mappings', 'previous_version_url', 'parent_concepts', 'child_concepts', 'parent_concept_urls', 'child_concept_urls', 'source_versions', 'collection_versions', 'versioned_object_id', 'references', 'checksums', - 'version_updated_on', 'version_updated_by', 'latest_source_version', + 'version_updated_on', 'version_updated_by', 'latest_source_version', 'retire_reason' ) def get_references(self, obj): diff --git a/core/concepts/tests/tests.py b/core/concepts/tests/tests.py index 52d8f4d0..6529c798 100644 --- a/core/concepts/tests/tests.py +++ b/core/concepts/tests/tests.py @@ -880,6 +880,41 @@ def test_retire(self): self.assertTrue(concept.is_versioned_object) self.assertTrue(concept_v1.is_latest_version) + concept_v1.retire(concept_v1.created_by, None, 'Forceful retirement') # concept will become old/prev version + concept.refresh_from_db() + concept_v1.refresh_from_db() + + self.assertFalse(concept_v1.is_latest_version) + self.assertEqual(concept.versions.count(), 3) + self.assertTrue(concept.retired) + latest_version = concept.get_latest_version() + self.assertTrue(latest_version.retired) + self.assertEqual(latest_version.retire_reason, 'Forceful retirement') + self.assertEqual(latest_version.comment, 'Concept was retired') + + self.assertEqual( + concept.retire(concept.created_by), + {'__all__': CONCEPT_IS_ALREADY_RETIRED} + ) + + def test_retire_without_retire_reason(self): + source = OrganizationSourceFactory(version=HEAD) + concept = Concept.persist_new({ + **factory.build(dict, FACTORY_CLASS=ConceptFactory), 'mnemonic': 'c1', 'parent': source, + 'names': [ConceptNameFactory.build(locale='en', name='English', locale_preferred=True)] + }) + concept_v1 = concept.clone() + concept_v1.datatype = 'foobar' + concept_v1.save_as_new_version(concept.created_by) + concept_v1 = Concept.objects.order_by('-created_at').first() + concept.refresh_from_db() + + self.assertEqual(concept.versions.count(), 2) + self.assertFalse(concept.retired) + self.assertFalse(concept.is_latest_version) + self.assertTrue(concept.is_versioned_object) + self.assertTrue(concept_v1.is_latest_version) + concept_v1.retire(concept_v1.created_by, 'Forceful retirement') # concept will become old/prev version concept.refresh_from_db() concept_v1.refresh_from_db() @@ -889,6 +924,7 @@ def test_retire(self): self.assertTrue(concept.retired) latest_version = concept.get_latest_version() self.assertTrue(latest_version.retired) + self.assertEqual(latest_version.retire_reason, None) self.assertEqual(latest_version.comment, 'Forceful retirement') self.assertEqual( @@ -896,10 +932,10 @@ def test_retire(self): {'__all__': CONCEPT_IS_ALREADY_RETIRED} ) - def test_unretire(self): + def test_retire_with_default_comment(self): source = OrganizationSourceFactory(version=HEAD) concept = Concept.persist_new({ - **factory.build(dict, FACTORY_CLASS=ConceptFactory), 'mnemonic': 'c1', 'parent': source, 'retired': True, + **factory.build(dict, FACTORY_CLASS=ConceptFactory), 'mnemonic': 'c1', 'parent': source, 'names': [ConceptNameFactory.build(locale='en', name='English', locale_preferred=True)] }) concept_v1 = concept.clone() @@ -908,6 +944,42 @@ def test_unretire(self): concept_v1 = Concept.objects.order_by('-created_at').first() concept.refresh_from_db() + self.assertEqual(concept.versions.count(), 2) + self.assertFalse(concept.retired) + self.assertFalse(concept.is_latest_version) + self.assertTrue(concept.is_versioned_object) + self.assertTrue(concept_v1.is_latest_version) + + concept_v1.retire(concept_v1.created_by) # concept will become old/prev version + concept.refresh_from_db() + concept_v1.refresh_from_db() + + self.assertFalse(concept_v1.is_latest_version) + self.assertEqual(concept.versions.count(), 3) + self.assertTrue(concept.retired) + latest_version = concept.get_latest_version() + self.assertTrue(latest_version.retired) + self.assertEqual(latest_version.retire_reason, None) + self.assertEqual(latest_version.comment, 'Concept was retired') + + self.assertEqual( + concept.retire(concept.created_by), + {'__all__': CONCEPT_IS_ALREADY_RETIRED} + ) + + def test_unretire(self): + source = OrganizationSourceFactory(version=HEAD) + concept = Concept.persist_new({ + **factory.build(dict, FACTORY_CLASS=ConceptFactory), 'mnemonic': 'c1', 'parent': source, + 'names': [ConceptNameFactory.build(locale='en', name='English', locale_preferred=True)], + 'retire_reason': 'unwanted', 'retired': True + }) + concept_v1 = concept.clone() + concept_v1.datatype = 'foobar' + concept_v1.save_as_new_version(concept.created_by) + concept_v1 = Concept.objects.order_by('-created_at').first() + concept.refresh_from_db() + self.assertEqual(concept.versions.count(), 2) self.assertTrue(concept.retired) self.assertFalse(concept.is_latest_version) @@ -924,6 +996,7 @@ def test_unretire(self): latest_version = concept.get_latest_version() self.assertFalse(latest_version.retired) self.assertEqual(latest_version.comment, 'World needs you!') + self.assertEqual(latest_version.retire_reason, None) self.assertEqual( concept.unretire(concept.created_by), diff --git a/core/concepts/views.py b/core/concepts/views.py index b2510690..89d3c285 100644 --- a/core/concepts/views.py +++ b/core/concepts/views.py @@ -409,7 +409,8 @@ def destroy(self, request, *args, **kwargs): return Response(status=status.HTTP_204_NO_CONTENT) comment = request.data.get('update_comment', None) or request.data.get('comment', None) - errors = concept.retire(request.user, comment) + reason = request.data.get('retire_reason', None) + errors = concept.retire(request.user, comment, reason) if errors: return Response(errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/core/importers/models.py b/core/importers/models.py index 35623011..ce99e80e 100644 --- a/core/importers/models.py +++ b/core/importers/models.py @@ -409,7 +409,7 @@ class ConceptImporter(BaseResourceImporter): mandatory_fields = {"concept_class"} allowed_fields = [ "id", "external_id", "concept_class", "datatype", "names", "descriptions", "retired", "extras", - "parent_concept_urls", 'update_comment', 'comment' + "parent_concept_urls", 'update_comment', 'comment', 'retire_reason' ] @staticmethod @@ -502,7 +502,11 @@ def delete(self): try: if parent.has_edit_access(self.user): concept = self.get_queryset().first() - concept.retire(self.user) + concept.retire( + self.user, + self.data.get('update_comment') or self.data.get('comment'), + self.data.get('retire_reason') + ) return DELETED return PERMISSION_DENIED except Exception as ex: @@ -515,7 +519,8 @@ class MappingImporter(BaseResourceImporter): mandatory_fields = {"map_type", "from_concept_url"} allowed_fields = [ "id", "map_type", "from_concept_url", "to_source_url", "to_concept_url", "to_concept_code", - "to_concept_name", "extras", "external_id", "retired", 'update_comment', 'comment', 'sort_weight' + "to_concept_name", "extras", "external_id", "retired", 'update_comment', 'comment', 'sort_weight', + 'retire_reason' ] @staticmethod @@ -649,7 +654,11 @@ def delete(self): try: if parent.has_edit_access(self.user): mapping = self.get_queryset().first() - mapping.retire(self.user) + mapping.retire( + self.user, + self.data.get('update_comment') or self.data.get('comment'), + self.data.get('retire_reason') + ) return DELETED return PERMISSION_DENIED except Exception as ex: diff --git a/core/integration_tests/tests_concepts.py b/core/integration_tests/tests_concepts.py index 4f72d4be..44089f53 100644 --- a/core/integration_tests/tests_concepts.py +++ b/core/integration_tests/tests_concepts.py @@ -97,6 +97,7 @@ def test_post_201(self): 'extras', 'type', 'update_comment', + 'retire_reason', 'version_url', 'updated_by', 'created_by', @@ -207,6 +208,7 @@ def test_post_201_with_mappings(self): 'extras', 'type', 'update_comment', + 'retire_reason', 'version_url', 'updated_by', 'created_by', @@ -364,6 +366,7 @@ def test_put_200(self): # pylint: disable=too-many-statements 'extras', 'type', 'update_comment', + 'retire_reason', 'version_url', 'updated_by', 'created_by', @@ -456,6 +459,7 @@ def test_put_200(self): # pylint: disable=too-many-statements 'extras', 'type', 'update_comment', + 'retire_reason', 'version_url', 'updated_by', 'created_by', @@ -549,6 +553,7 @@ def test_put_200_with_mappings(self): # pylint: disable=too-many-statements 'extras', 'type', 'update_comment', + 'retire_reason', 'version_url', 'updated_by', 'created_by', @@ -782,6 +787,7 @@ def test_put_200_openmrs_schema(self): # pylint: disable=too-many-statements 'extras', 'type', 'update_comment', + 'retire_reason', 'version_url', 'updated_by', 'created_by', @@ -878,7 +884,55 @@ def test_delete_204(self): response = self.client.delete( concepts_url, - {'update_comment': 'Deleting it'}, + { + 'retire_reason': "Deprecated 5/1/2026, replaced by more granular MySource:A1b2C3", + 'comment': 'Retired' + }, + HTTP_AUTHORIZATION='Token ' + self.token, + format='json' + ) + + self.assertEqual(response.status_code, 204) + + concept.refresh_from_db() + + self.assertEqual(concept.versions.count(), 2) + latest = concept.versions.order_by('-created_at').first() + self.assertTrue(latest.retired) + self.assertTrue(concept.retired) + self.assertTrue(latest.retire_reason, "Deprecated 5/1/2026, replaced by more granular MySource:A1b2C3") + self.assertTrue(latest.comment, 'Retired') + + def test_delete_204_without_retire_reason(self): + names = [ConceptNameFactory.build()] + concept = ConceptFactory(parent=self.source, names=names) + concepts_url = f"/orgs/{self.organization.mnemonic}/sources/{self.source.mnemonic}/concepts/{concept.mnemonic}/" + + response = self.client.delete( + concepts_url, + {'update_comment': 'Retired'}, + HTTP_AUTHORIZATION='Token ' + self.token, + format='json' + ) + + self.assertEqual(response.status_code, 204) + + concept.refresh_from_db() + + self.assertEqual(concept.versions.count(), 2) + latest_version = concept.versions.order_by('-created_at').first() + self.assertTrue(latest_version.retired) + self.assertTrue(concept.retired) + self.assertEqual(latest_version.retire_reason, None) + self.assertEqual(latest_version.comment, 'Retired') + + def test_delete_204_without_payload(self): + names = [ConceptNameFactory.build()] + concept = ConceptFactory(parent=self.source, names=names) + concepts_url = f"/orgs/{self.organization.mnemonic}/sources/{self.source.mnemonic}/concepts/{concept.mnemonic}/" + + response = self.client.delete( + concepts_url, HTTP_AUTHORIZATION='Token ' + self.token, format='json' ) @@ -891,7 +945,87 @@ def test_delete_204(self): latest_version = concept.versions.order_by('-created_at').first() self.assertTrue(latest_version.retired) self.assertTrue(concept.retired) - self.assertTrue(latest_version.comment, 'Deleting it') + self.assertEqual(latest_version.retire_reason, None) + self.assertEqual(latest_version.comment, 'Concept was retired') + + def test_retire_update_and_unretire_preserves_version_metadata(self): # pylint: disable=too-many-statements + names = [ConceptNameFactory.build()] + concept = ConceptFactory(parent=self.source, names=names) + retire_reason = "Deprecated 5/1/2026, replaced by more granular MySource:A1b2C3" + retire_comment = 'Retired for replacement' + retired_update_comment = 'Updated while retired' + unretire_comment = 'Reactivated for reuse' + + response = self.client.delete( + concept.uri, + {'retire_reason': retire_reason, 'update_comment': retire_comment}, + HTTP_AUTHORIZATION='Token ' + self.token, + format='json' + ) + + self.assertEqual(response.status_code, 204) + + concept.refresh_from_db() + retired_version = concept.get_latest_version() + initial_version = retired_version.prev_version + + self.assertTrue(retired_version.retired) + self.assertTrue(concept.retired) + self.assertEqual(retired_version.retire_reason, retire_reason) + self.assertEqual(concept.retire_reason, retire_reason) + self.assertEqual(retired_version.comment, retire_comment) + self.assertIsNone(initial_version.retire_reason) + self.assertIsNone(initial_version.comment) + + response = self.client.patch( + concept.uri, + {'datatype': 'Text', 'update_comment': retired_update_comment}, + HTTP_AUTHORIZATION='Token ' + self.token, + format='json' + ) + + self.assertEqual(response.status_code, 200) + + concept.refresh_from_db() + retired_updated_version = concept.get_latest_version() + retired_prev_version = retired_updated_version.prev_version + + self.assertTrue(retired_updated_version.retired) + self.assertTrue(concept.retired) + self.assertEqual(retired_updated_version.retire_reason, retire_reason) + self.assertEqual(concept.retire_reason, retire_reason) + self.assertEqual(retired_updated_version.comment, retired_update_comment) + self.assertEqual(retired_prev_version.retire_reason, retire_reason) + self.assertEqual(retired_prev_version.comment, retire_comment) + self.assertIsNone(retired_prev_version.prev_version.retire_reason) + self.assertIsNone(retired_prev_version.prev_version.comment) + + response = self.client.put( + concept.uri + 'reactivate/', + {'update_comment': unretire_comment}, + HTTP_AUTHORIZATION='Token ' + self.token, + format='json' + ) + + self.assertEqual(response.status_code, 204) + + concept.refresh_from_db() + unretired_version = concept.get_latest_version() + unretired_prev_version = unretired_version.prev_version + + self.assertFalse(unretired_version.retired) + self.assertFalse(concept.retired) + self.assertIsNone(unretired_version.retire_reason) + self.assertIsNone(concept.retire_reason) + self.assertEqual(unretired_version.comment, unretire_comment) + self.assertTrue(unretired_prev_version.retired) + self.assertEqual(unretired_prev_version.retire_reason, retire_reason) + self.assertEqual(unretired_prev_version.comment, retired_update_comment) + self.assertTrue(unretired_prev_version.prev_version.retired) + self.assertEqual(unretired_prev_version.prev_version.retire_reason, retire_reason) + self.assertEqual(unretired_prev_version.prev_version.comment, retire_comment) + self.assertIsNone(unretired_prev_version.prev_version.prev_version.retire_reason) + self.assertIsNone(unretired_prev_version.prev_version.prev_version.comment) def test_db_hard_delete_204(self): names = [ConceptNameFactory.build()] @@ -1225,7 +1359,7 @@ def test_get_200_with_response_modes(self): 'owner', 'owner_type', 'owner_url', 'display_name', 'display_locale', 'version', 'update_comment', 'locale', 'version_created_by', 'version_created_on', 'is_latest_version', 'latest_source_version', 'versions_url', 'version_url', 'type', 'versioned_object_id', - 'version_updated_on', 'version_updated_by', 'checksums', 'property']) + 'version_updated_on', 'version_updated_by', 'checksums', 'property', 'retire_reason']) ) response = self.client.get( @@ -1240,7 +1374,7 @@ def test_get_200_with_response_modes(self): 'owner', 'owner_type', 'owner_url', 'display_name', 'display_locale', 'names', 'descriptions', 'created_on', 'updated_on', 'versions_url', 'version', 'extras', 'type', 'latest_source_version', 'update_comment', 'version_url', 'updated_by', 'created_by', - 'public_can_view', 'versioned_object_id', 'checksums', 'property']) + 'public_can_view', 'versioned_object_id', 'checksums', 'property', 'retire_reason']) ) response = self.client.get( diff --git a/core/integration_tests/tests_mappings.py b/core/integration_tests/tests_mappings.py index cf865062..07c9dc1d 100644 --- a/core/integration_tests/tests_mappings.py +++ b/core/integration_tests/tests_mappings.py @@ -708,6 +708,7 @@ def test_delete_204(self): latest_version = self.mapping.get_latest_version() self.assertTrue(latest_version.retired) self.assertEqual(latest_version.comment, 'Mapping was retired') + self.assertEqual(latest_version.retire_reason, None) self.mapping.refresh_from_db() self.assertTrue(self.mapping.retired) diff --git a/core/mappings/migrations/0058_mapping_retire_reason.py b/core/mappings/migrations/0058_mapping_retire_reason.py new file mode 100644 index 00000000..7722c5ab --- /dev/null +++ b/core/mappings/migrations/0058_mapping_retire_reason.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1.15 on 2026-05-26 02:43 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('mappings', '0057_mappings_sources_source_mapping_idx'), + ] + + operations = [ + migrations.AddField( + model_name='mapping', + name='retire_reason', + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/core/mappings/models.py b/core/mappings/models.py index e956e758..52c82757 100644 --- a/core/mappings/models.py +++ b/core/mappings/models.py @@ -69,11 +69,6 @@ class Meta: map_type = models.TextField() sort_weight = models.FloatField(db_index=True, null=True, blank=True) sources = models.ManyToManyField('sources.Source', related_name='mappings') - external_id = models.TextField(null=True, blank=True) - comment = models.TextField(null=True, blank=True) - versioned_object = models.ForeignKey( - 'self', related_name='versions_set', null=True, blank=True, on_delete=models.CASCADE - ) mnemonic = models.CharField( max_length=255, validators=[RegexValidator(regex=NAMESPACE_REGEX)], default=uuid.uuid4, ) @@ -100,8 +95,6 @@ class Meta: to_concept_name = models.TextField(null=True, blank=True) to_source_url = models.TextField(null=True, blank=True, db_index=True) to_source_version = models.TextField(null=True, blank=True) - _counted = models.BooleanField(default=True, null=True, blank=True) - _index = models.BooleanField(default=True) logo_path = None name = None @@ -274,6 +267,7 @@ def clone(self, user=None, from_concept=None, to_concept=None): parent_id=self.parent_id, map_type=self.map_type, retired=self.retired, + retire_reason=self.retire_reason, released=self.released, is_latest_version=self.is_latest_version, extras=self.extras, @@ -403,10 +397,18 @@ def create_new_version_for(cls, instance, data, user): instance.extras = data.get('extras', instance.extras) instance.external_id = data.get('external_id', instance.external_id) instance.comment = data.get('update_comment') or data.get('comment') - instance.retired = data.get('retired', instance.retired) instance.mnemonic = data.get('mnemonic', instance.mnemonic) instance.map_type = data.get('map_type', instance.map_type) instance.sort_weight = data.get('sort_weight', instance.sort_weight) + instance.retire_reason = data.get('retire_reason', instance.retire_reason) + is_retired = data.get('retired', None) + if is_retired is True and not instance.retired: + instance.retire_comment = instance.comment + instance.comment = instance.WAS_RETIRED + elif instance.retired and is_retired is False: + instance.retire_comment = None + instance.comment = instance.comment or instance.WAS_RETIRED + instance.retired = data.get('retired', instance.retired) return instance.save_as_new_version(user) diff --git a/core/mappings/serializers.py b/core/mappings/serializers.py index 61caa85a..e71d0847 100644 --- a/core/mappings/serializers.py +++ b/core/mappings/serializers.py @@ -130,7 +130,7 @@ class Meta: 'is_latest_version', 'update_comment', 'version_url', 'uuid', 'version_created_on', 'from_source_version', 'to_source_version', 'from_concept_name_resolved', 'to_concept_name_resolved', 'type', 'sort_weight', - 'version_updated_on', 'version_updated_by', 'latest_source_version' + 'version_updated_on', 'version_updated_by', 'latest_source_version', 'retire_reason' ) diff --git a/core/mappings/views.py b/core/mappings/views.py index bd569bbe..70b14e84 100644 --- a/core/mappings/views.py +++ b/core/mappings/views.py @@ -234,12 +234,13 @@ def destroy(self, request, *args, **kwargs): mapping = self.get_object() parent = mapping.parent comment = request.data.get('update_comment', None) or request.data.get('comment', None) + reason = request.data.get('retire_reason', None) if self.is_hard_delete_requested(): mapping.delete() parent.update_mappings_count() return Response(status=status.HTTP_204_NO_CONTENT) - errors = mapping.retire(request.user, comment) + errors = mapping.retire(request.user, comment, reason) if errors: return Response(errors, status=status.HTTP_400_BAD_REQUEST) From e341103d23d35a903e6f991ebd0eb360d1f42a03 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Tue, 26 May 2026 09:33:00 +0530 Subject: [PATCH 2/2] OpenConceptLab/ocl_issues#2519 | concept..retire_reason --- core/concepts/models.py | 1 + core/concepts/serializers.py | 9 +- core/concepts/views.py | 1 + core/integration_tests/tests_concepts.py | 168 ++++++++++++++++++++++- 4 files changed, 177 insertions(+), 2 deletions(-) diff --git a/core/concepts/models.py b/core/concepts/models.py index 97591bee..386d51f4 100644 --- a/core/concepts/models.py +++ b/core/concepts/models.py @@ -56,6 +56,7 @@ def clone(self): locale=self.locale, locale_preferred=self.locale_preferred, retired=self.retired, + retire_reason=self.retire_reason, ) @staticmethod diff --git a/core/concepts/serializers.py b/core/concepts/serializers.py index 8b01990e..013c1862 100644 --- a/core/concepts/serializers.py +++ b/core/concepts/serializers.py @@ -63,7 +63,7 @@ class ConceptLocaleSerializer(ModelSerializer): class Meta: model = ConceptName fields = ( - 'uuid', 'external_id', 'type', 'locale', 'locale_preferred', 'concept_id', 'retired' + 'uuid', 'external_id', 'type', 'locale', 'locale_preferred', 'concept_id', 'retired', 'retire_reason' ) @staticmethod @@ -80,7 +80,14 @@ def create(self, validated_data, instance=None): # pylint: disable=arguments-di locale.name = validated_data.get('name', locale.name) locale.locale = validated_data.get('locale', locale.locale) locale.locale_preferred = validated_data.get('locale_preferred', locale.locale_preferred) + + retired = validated_data.get('retired', None) + if retired is True and not locale.retired: + locale.retire_reason = validated_data.get('retire_reason', None) + elif locale.retired and retired is False: + locale.retire_reason = None locale.retired = validated_data.get('retired', locale.retired) + locale.type = self.get_locale_type(validated_data, locale) locale.external_id = validated_data.get('external_id', locale.external_id) locale.concept_id = validated_data.get('concept_id', locale.concept_id) diff --git a/core/concepts/views.py b/core/concepts/views.py index 89d3c285..914c43eb 100644 --- a/core/concepts/views.py +++ b/core/concepts/views.py @@ -746,6 +746,7 @@ def delete(self, request, *args, **kwargs): ] retired_locale = instance.clone() retired_locale.retired = True + retired_locale.retire_reason = request.data.get('retire_reason', None) labels.append(retired_locale) setattr(new_version, subject_label_attr, labels) new_version.comment = f'Retired {instance.name} in {self.parent_list_attribute}.' diff --git a/core/integration_tests/tests_concepts.py b/core/integration_tests/tests_concepts.py index 44089f53..2ea352dc 100644 --- a/core/integration_tests/tests_concepts.py +++ b/core/integration_tests/tests_concepts.py @@ -1261,6 +1261,7 @@ def test_names_get_200(self): "name": name.name, "name_type": "FULLY_SPECIFIED", "retired": False, + "retire_reason": None, } ) @@ -1291,7 +1292,8 @@ def test_names_post_201(self): "locale_preferred": False, "name": 'foo', "name_type": "Fully Specified", - "retired": False + "retired": False, + "retire_reason": None, } ) self.assertEqual(concept.names.count(), 2) @@ -2749,13 +2751,137 @@ def test_put_200_retired(self): self.assertEqual(self.concept.get_latest_version().active_names.count(), 1) self.assertEqual(self.concept.get_latest_version().active_names.first().name, 'froobar') self.assertEqual(self.concept.get_latest_version().active_names.first().retired, False) + self.assertEqual(self.concept.get_latest_version().active_names.first().retire_reason, None) self.assertEqual(self.concept.get_latest_version().retired_names.first().name, 'retraité') self.assertEqual(self.concept.get_latest_version().retired_names.first().retired, True) + self.assertEqual(self.concept.get_latest_version().retired_names.first().retire_reason, None) self.assertEqual(self.concept.get_latest_version().prev_version.active_names.count(), 2) self.assertEqual(self.concept.active_names.count(), 1) self.assertEqual(self.concept.active_names.first().name, 'froobar') self.assertEqual(self.concept.retired_names.first().name, 'retraité') + def test_put_200_retired_with_reason(self): + self.assertEqual(self.concept.versions.count(), 1) + self.assertEqual(self.concept.names.count(), 1) + self.assertEqual(self.concept.active_names.count(), 1) + + response = self.client.put( + self.url, + {'retired': True, 'retire_reason': 'Not needed!'}, + HTTP_AUTHORIZATION='Token ' + self.token, + + ) + + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'names': ['A concept must have at least one name']}) + self.assertEqual(self.concept.versions.count(), 1) + + name2 = ConceptNameFactory(locale='fr', name='retraité', concept=self.concept) + ConceptNameFactory(locale='fr', name='retraité', concept=self.concept.get_latest_version()) + self.assertEqual(self.concept.names.count(), 2) + self.assertEqual(self.concept.active_names.count(), 2) + + response = self.client.put( + self.concept.uri + f'names/{name2.id}/', + {'retired': True, 'retire_reason': 'Not needed!'}, + HTTP_AUTHORIZATION='Token ' + self.token, + + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual(self.concept.versions.count(), 2) + self.assertEqual(self.concept.get_latest_version().active_names.count(), 1) + self.assertEqual(self.concept.get_latest_version().active_names.first().name, 'froobar') + self.assertEqual(self.concept.get_latest_version().active_names.first().retired, False) + self.assertEqual(self.concept.get_latest_version().active_names.first().retire_reason, None) + self.assertEqual(self.concept.get_latest_version().retired_names.first().name, 'retraité') + self.assertEqual(self.concept.get_latest_version().retired_names.first().retired, True) + self.assertEqual(self.concept.get_latest_version().retired_names.first().retire_reason, 'Not needed!') + self.assertEqual(self.concept.get_latest_version().prev_version.active_names.count(), 2) + self.assertEqual(self.concept.active_names.count(), 1) + self.assertEqual(self.concept.active_names.first().name, 'froobar') + self.assertEqual(self.concept.retired_names.first().name, 'retraité') + + def test_put_retire_update_and_unretire_preserves_name_retire_reason(self): # pylint: disable=too-many-statements + self.assertEqual(self.concept.versions.count(), 1) + + name2 = ConceptNameFactory(locale='fr', name='retraité', concept=self.concept) + ConceptNameFactory(locale='fr', name='retraité', concept=self.concept.get_latest_version()) + retire_reason = 'Not needed!' + + response = self.client.put( + self.concept.uri + f'names/{name2.id}/', + {'retired': True, 'retire_reason': retire_reason}, + HTTP_AUTHORIZATION='Token ' + self.token, + ) + + self.assertEqual(response.status_code, 200) + + latest_version = self.concept.get_latest_version() + retired_name = latest_version.retired_names.get(locale='fr') + prev_version = latest_version.prev_version + prev_active_name = prev_version.active_names.get(locale='fr', name='retraité') + + self.assertEqual(self.concept.versions.count(), 2) + self.assertTrue(retired_name.retired) + self.assertEqual(retired_name.retire_reason, retire_reason) + self.assertTrue(self.concept.retired_names.get(locale='fr').retired) + self.assertEqual(self.concept.retired_names.get(locale='fr').retire_reason, retire_reason) + self.assertFalse(prev_active_name.retired) + self.assertIsNone(prev_active_name.retire_reason) + + response = self.client.patch( + self.concept.uri + f'names/{self.concept.retired_names.get(locale="fr").id}/', + {'name_type': 'Short'}, + HTTP_AUTHORIZATION='Token ' + self.token, + ) + + self.assertEqual(response.status_code, 200) + + latest_version = self.concept.get_latest_version() + updated_retired_name = latest_version.retired_names.get(locale='fr') + retired_prev_version = latest_version.prev_version + retired_prev_name = retired_prev_version.retired_names.get(locale='fr') + original_prev_name = retired_prev_version.prev_version.active_names.get(locale='fr', name='retraité') + + self.assertEqual(self.concept.versions.count(), 3) + self.assertTrue(updated_retired_name.retired) + self.assertEqual(updated_retired_name.retire_reason, retire_reason) + self.assertEqual(updated_retired_name.type, 'Short') + self.assertTrue(self.concept.retired_names.get(locale='fr').retired) + self.assertEqual(self.concept.retired_names.get(locale='fr').retire_reason, retire_reason) + self.assertEqual(self.concept.retired_names.get(locale='fr').type, 'Short') + self.assertTrue(retired_prev_name.retired) + self.assertEqual(retired_prev_name.retire_reason, retire_reason) + self.assertIsNone(original_prev_name.retire_reason) + self.assertFalse(original_prev_name.retired) + + response = self.client.patch( + self.concept.uri + f'names/{self.concept.retired_names.get(locale="fr").id}/', + {'retired': False}, + HTTP_AUTHORIZATION='Token ' + self.token, + ) + + self.assertEqual(response.status_code, 200) + + latest_version = self.concept.get_latest_version() + unretired_name = latest_version.active_names.get(locale='fr', name='retraité') + unretired_prev_version = latest_version.prev_version + unretired_prev_name = unretired_prev_version.retired_names.get(locale='fr') + original_prev_name = unretired_prev_version.prev_version.retired_names.get(locale='fr') + + self.assertEqual(self.concept.versions.count(), 4) + self.assertFalse(unretired_name.retired) + self.assertIsNone(unretired_name.retire_reason) + self.assertEqual(unretired_name.type, 'Short') + self.assertFalse(self.concept.active_names.get(locale='fr', name='retraité').retired) + self.assertIsNone(self.concept.active_names.get(locale='fr', name='retraité').retire_reason) + self.assertEqual(self.concept.active_names.get(locale='fr', name='retraité').type, 'Short') + self.assertTrue(unretired_prev_name.retired) + self.assertEqual(unretired_prev_name.retire_reason, retire_reason) + self.assertTrue(original_prev_name.retired) + self.assertEqual(original_prev_name.retire_reason, retire_reason) + def test_delete_204(self): self.assertEqual(self.concept.versions.count(), 1) @@ -2783,8 +2909,48 @@ def test_delete_204(self): self.assertEqual(self.concept.get_latest_version().active_names.count(), 1) self.assertEqual(self.concept.get_latest_version().active_names.first().name, 'froobar') self.assertEqual(self.concept.get_latest_version().active_names.first().retired, False) + self.assertEqual(self.concept.get_latest_version().active_names.first().retire_reason, None) + self.assertEqual(self.concept.get_latest_version().retired_names.first().name, 'retraité') + self.assertEqual(self.concept.get_latest_version().retired_names.first().retired, True) + self.assertEqual(self.concept.get_latest_version().retired_names.first().retire_reason, None) + self.assertEqual(self.concept.get_latest_version().prev_version.active_names.count(), 2) + self.assertEqual(self.concept.active_names.count(), 1) + self.assertEqual(self.concept.active_names.first().name, 'froobar') + self.assertEqual(self.concept.retired_names.first().name, 'retraité') + + def test_delete_204_with_retire_reason(self): + self.assertEqual(self.concept.versions.count(), 1) + + response = self.client.delete( + self.url, + {'retire_reason': 'Not needed!'}, + HTTP_AUTHORIZATION='Token ' + self.token, + + ) + + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'names': ['A concept must have at least one name']}) + + name2 = ConceptNameFactory(locale='fr', name='retraité', concept=self.concept) + ConceptNameFactory(locale='fr', name='retraité', concept=self.concept.get_latest_version()) + self.assertEqual(self.concept.names.count(), 2) + + response = self.client.delete( + self.concept.uri + f'names/{name2.id}/', + {'retire_reason': 'Not needed!'}, + HTTP_AUTHORIZATION='Token ' + self.token, + + ) + self.assertEqual(response.status_code, 204) + + self.assertEqual(self.concept.versions.count(), 2) + self.assertEqual(self.concept.get_latest_version().active_names.count(), 1) + self.assertEqual(self.concept.get_latest_version().active_names.first().name, 'froobar') + self.assertEqual(self.concept.get_latest_version().active_names.first().retired, False) + self.assertEqual(self.concept.get_latest_version().active_names.first().retire_reason, None) self.assertEqual(self.concept.get_latest_version().retired_names.first().name, 'retraité') self.assertEqual(self.concept.get_latest_version().retired_names.first().retired, True) + self.assertEqual(self.concept.get_latest_version().retired_names.first().retire_reason, 'Not needed!') self.assertEqual(self.concept.get_latest_version().prev_version.active_names.count(), 2) self.assertEqual(self.concept.active_names.count(), 1) self.assertEqual(self.concept.active_names.first().name, 'froobar')