From 3959146242ae383b226439fd6e6afd9a69f0cb30 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 8 Oct 2026 12:22:48 +0530 Subject: [PATCH] OpenConceptLab/ocl_online#271 | fixing clone _index --- core/common/mixins.py | 7 ++++--- core/concepts/models.py | 2 +- core/concepts/tests/tests.py | 35 ++++++++++++++++++++++++++++++++++ core/importers/models.py | 8 +++++--- core/mappings/models.py | 2 +- core/mappings/tests/tests.py | 37 ++++++++++++++++++++++++++++++++++++ 6 files changed, 83 insertions(+), 8 deletions(-) diff --git a/core/common/mixins.py b/core/common/mixins.py index 3feb5291..fe577192 100644 --- a/core/common/mixins.py +++ b/core/common/mixins.py @@ -787,11 +787,11 @@ def parent_url_kwargs(self): kwargs['version'] = None return kwargs - def retire(self, user, comment=None, reason=None): + def retire(self, user, comment=None, reason=None, index=True): if self.versioned_object.retired: return {'__all__': self.ALREADY_RETIRED} - return self.__update_retire(True, user, comment or self.WAS_RETIRED, reason) + return self.__update_retire(True, user, comment or self.WAS_RETIRED, reason, index) def unretire(self, user, comment=None): if not self.versioned_object.retired: @@ -799,9 +799,10 @@ def unretire(self, user, comment=None): return self.__update_retire(False, user, comment or self.WAS_UNRETIRED) - def __update_retire(self, retired, user, comment, reason=None): + def __update_retire(self, retired, user, comment, reason=None, index=True): # pylint: disable=too-many-arguments latest_version = self.get_latest_version() or self.get_last_version() new_version = latest_version.clone() + new_version._index = index # pylint: disable=protected-access new_version.retired = retired new_version.comment = comment if reason is not None: diff --git a/core/concepts/models.py b/core/concepts/models.py index 25dac908..3f3e69f4 100644 --- a/core/concepts/models.py +++ b/core/concepts/models.py @@ -559,7 +559,6 @@ def clone(self): is_latest_version=self.is_latest_version, parent_id=self.parent_id, versioned_object_id=self.versioned_object_id, - _index=self._index ) concept_version.cloned_names = self.clone_name_locales() concept_version.cloned_descriptions = self.clone_description_locales() @@ -582,6 +581,7 @@ def version_for_concept(cls, concept, version_label, parent_version=None): @classmethod def create_initial_version(cls, concept, **kwargs): initial_version = cls.version_for_concept(concept, generate_temp_version()) + initial_version._index = concept._index # pylint: disable=protected-access initial_version.comment = concept.comment initial_version.save(**kwargs) initial_version.version = initial_version.id diff --git a/core/concepts/tests/tests.py b/core/concepts/tests/tests.py index 39fdd3ef..dcf98676 100644 --- a/core/concepts/tests/tests.py +++ b/core/concepts/tests/tests.py @@ -1532,6 +1532,41 @@ def test_unretire(self): {'__all__': CONCEPT_IS_ALREADY_NOT_RETIRED} ) + @patch.object(Concept, 'index', autospec=True) + def test_edit_and_retire_index_versions_cloned_from_deferred_rows( # pylint: disable=protected-access + self, index_mock): + 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)] + }) + rows = Concept.objects.filter(versioned_object_id=concept.id) + + for action in ('edit', 'retire'): + rows.update(_index=False) # as bulk imports leave them + concept.refresh_from_db() + prev_latest = concept.get_latest_version() + index_mock.reset_mock() + + with self.captureOnCommitCallbacks(execute=True): + if action == 'edit': + new_version = concept.clone() + new_version.datatype = 'foobar' + self.assertEqual(new_version.save_as_new_version(concept.created_by), {}) + else: + self.assertEqual(concept.retire(concept.created_by), {}) + + prev_latest.refresh_from_db() + latest = concept.get_latest_version() + self.assertNotEqual(latest.id, prev_latest.id) + self.assertTrue(latest._index) + self.assertTrue(prev_latest._index) + self.assertFalse(prev_latest.is_latest_version) + self.assertEqual( + {call.args[0].id for call in index_mock.call_args_list}, {latest.id, prev_latest.id}) + + self.assertTrue(concept.get_latest_version().retired) + def test_concept_access_changes_with_source(self): source = OrganizationSourceFactory(version=HEAD, public_access=ACCESS_TYPE_NONE) concept = ConceptFactory(parent=source, public_access=ACCESS_TYPE_NONE) diff --git a/core/importers/models.py b/core/importers/models.py index fe8888c4..c06b2af2 100644 --- a/core/importers/models.py +++ b/core/importers/models.py @@ -195,12 +195,14 @@ def process(self): def retire_resource(self, resource): """ Retires a concept or mapping and keeps the version the retire saved as the instance, marked with the one it - superseded, as process() keeps what it saves. The retire clones the latest version, so both are left deferred - (_index=False) when the latest version was. + superseded, as process() keeps what it saves. Both are left deferred (_index=False) when the latest version + was. """ prev_latest = resource.get_latest_version() resource.retire( - self.user, self.data.get('update_comment') or self.data.get('comment'), self.data.get('retire_reason')) + self.user, self.data.get('update_comment') or self.data.get('comment'), self.data.get('retire_reason'), + index=get(prev_latest, '_index', True) + ) self.instance = resource.get_latest_version() if self.instance: self.instance.prev_latest_version_id = get(prev_latest, 'id') diff --git a/core/mappings/models.py b/core/mappings/models.py index 28a77046..90d57654 100644 --- a/core/mappings/models.py +++ b/core/mappings/models.py @@ -286,7 +286,6 @@ def clone(self, user=None, from_concept=None, to_concept=None): from_source_id=self.from_source_id, from_source_url=self.from_source_url, from_source_version=self.from_source_version, - _index=self._index, sort_weight=self.sort_weight ) if to_concept: @@ -308,6 +307,7 @@ def build(cls, map_type, from_concept, to_concept, **kwargs): @classmethod def create_initial_version(cls, mapping, **kwargs): initial_version = mapping.clone() + initial_version._index = mapping._index # pylint: disable=protected-access initial_version.created_by = initial_version.updated_by = mapping.created_by initial_version.comment = mapping.comment initial_version.save(**kwargs) diff --git a/core/mappings/tests/tests.py b/core/mappings/tests/tests.py index b65f2810..8a36b39d 100644 --- a/core/mappings/tests/tests.py +++ b/core/mappings/tests/tests.py @@ -1,3 +1,4 @@ +from unittest.mock import patch from uuid import UUID import factory @@ -321,6 +322,42 @@ def test_save_as_new_version(self): persisted_mapping.version_url, persisted_mapping.uri ) + @patch.object(Mapping, 'index_from_concept') + @patch.object(Mapping, 'index', autospec=True) + def test_edit_and_retire_index_versions_cloned_from_deferred_rows( # pylint: disable=protected-access + self, index_mock, _): + source = OrganizationSourceFactory(version=HEAD) + mapping = Mapping.persist_new({ + **factory.build(dict, FACTORY_CLASS=MappingFactory), 'from_concept': ConceptFactory(parent=source), + 'to_concept': ConceptFactory(parent=source), 'parent_id': source.id + }, source.created_by) + rows = Mapping.objects.filter(versioned_object_id=mapping.id) + + for action in ('edit', 'retire'): + rows.update(_index=False) # as bulk imports leave them + mapping.refresh_from_db() + prev_latest = mapping.get_latest_version() + index_mock.reset_mock() + + with self.captureOnCommitCallbacks(execute=True): + if action == 'edit': + new_version = mapping.clone(mapping.created_by) + new_version.extras = {'foo': 'bar'} + self.assertEqual(new_version.save_as_new_version(mapping.created_by), {}) + else: + self.assertEqual(mapping.retire(mapping.created_by), {}) + + prev_latest.refresh_from_db() + latest = mapping.get_latest_version() + self.assertNotEqual(latest.id, prev_latest.id) + self.assertTrue(latest._index) + self.assertTrue(prev_latest._index) + self.assertFalse(prev_latest.is_latest_version) + self.assertEqual( + {call.args[0].id for call in index_mock.call_args_list}, {latest.id, prev_latest.id}) + + self.assertTrue(mapping.get_latest_version().retired) + def test_get_serializer_class(self): self.assertEqual(Mapping.get_serializer_class(), MappingListSerializer) self.assertEqual(Mapping.get_serializer_class(version=True), MappingVersionListSerializer)