Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion core/concepts/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,48 @@ def create_initial_version(cls, concept, **kwargs):
initial_version.save()
return initial_version

@staticmethod
def _locale_status_changed(previous_locales, new_locales, locale_payloads):
"""Detect locale status edits that the standard checksum intentionally ignores."""
previous_by_identity = {
(
locale.external_id,
locale.name,
locale.type,
locale.locale,
locale.locale_preferred,
): locale
for locale in previous_locales
}
previous_by_content = {
(locale.name, locale.type, locale.locale, locale.locale_preferred): locale
for locale in previous_locales
}
status_fields = {'retired', 'retire_reason'}
for new_locale, locale_payload in zip(new_locales, locale_payloads):
if not status_fields.intersection(locale_payload):
continue
previous_locale = previous_by_identity.get((
new_locale.external_id,
new_locale.name,
new_locale.type,
new_locale.locale,
new_locale.locale_preferred,
))
if not previous_locale:
previous_locale = previous_by_content.get((
new_locale.name,
new_locale.type,
new_locale.locale,
new_locale.locale_preferred,
))
if previous_locale and (
previous_locale.retired != new_locale.retired or
previous_locale.retire_reason != new_locale.retire_reason
):
return True
return False

@classmethod
def create_new_version_for(
cls, instance, data, user, create_parent_version=True, add_prev_version_children=True,
Expand All @@ -583,6 +625,8 @@ def create_new_version_for(
prev_latest = Concept.objects.filter(
mnemonic=instance.mnemonic, parent_id=instance.parent_id, is_latest_version=True
).first()
names_payload = data.get('names', [])
descriptions_payload = data.get('descriptions', [])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize nullable description payloads before status checks

When a concept update sends "descriptions": null (explicitly accepted by ConceptDetailSerializer.descriptions with allow_null=True), this line stores None; if no name status change short-circuits first, the new call to _locale_status_changed passes that None into zip(new_locales, locale_payloads) and raises TypeError: 'NoneType' object is not iterable. Existing build logic treats None as an empty description list, so normalize this payload with or [] before the new status-change check.

Useful? React with 👍 / 👎.

instance.id = None # Clear id so it is persisted as a new object
instance.version = data.get('version', None)
instance.concept_class = data.get('concept_class', instance.concept_class)
Expand Down Expand Up @@ -611,13 +655,21 @@ def create_new_version_for(
if not parent_concept_uris and has_parent_concept_uris_attr:
parent_concept_uris = []

has_locale_status_change = False
if prev_latest:
has_locale_status_change = cls._locale_status_changed(
prev_latest.clone_name_locales(), instance.cloned_names, names_payload
) or cls._locale_status_changed(
prev_latest.clone_description_locales(), instance.cloned_descriptions, descriptions_payload
)

errors = instance.save_as_new_version(
user=user,
create_parent_version=create_parent_version,
parent_concept_uris=parent_concept_uris,
add_prev_version_children=add_prev_version_children,
_hierarchy_processing=_hierarchy_processing,
skip_duplicate_version_check=bool(mappings_payload)
skip_duplicate_version_check=bool(mappings_payload) or has_locale_status_change
)

if errors or mappings_payload is None:
Expand Down
58 changes: 58 additions & 0 deletions core/integration_tests/tests_concepts.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,64 @@ def test_put_200(self): # pylint: disable=too-many-statements
self.assertEqual(response.data['display_name'], prev_version.display_name)
self.assertEqual(concept.datatype, "N/A")

def test_put_200_when_name_retired_changes_only(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filiperochalopes This test passes on master with ocldev==0.2.3 as well.

names = [
ConceptNameFactory.build(name='Active name', locale='es', locale_preferred=True),
ConceptNameFactory.build(name='Retirable name', locale='es', locale_preferred=False),
]
concept = ConceptFactory(
parent=self.source,
concept_class='Procedure',
datatype='Coded',
names=names,
descriptions=[
ConceptDescriptionFactory.build(
name='Concept description', locale='es', locale_preferred=True
)
]
)
concepts_url = f"/orgs/{self.organization.mnemonic}/sources/{self.source.mnemonic}/concepts/{concept.mnemonic}/"
names_payload = [
{
'external_id': name.external_id,
'name': name.name,
'locale': name.locale,
'locale_preferred': name.locale_preferred,
'name_type': name.type,
'retired': name.name == 'Retirable name',
}
for name in concept.names.order_by('id')
]

response = self.client.put(
concepts_url,
{
'datatype': concept.datatype,
'concept_class': concept.concept_class,
'extras': concept.extras,
'descriptions': [
{
'description': description.name,
'description_type': description.type,
'external_id': description.external_id,
'locale': description.locale,
'locale_preferred': description.locale_preferred,
}
for description in concept.descriptions.order_by('id')
],
'external_id': concept.external_id or '',
'id': concept.mnemonic,
'names': names_payload,
},
HTTP_AUTHORIZATION='Token ' + self.token,
format='json'
)

self.assertEqual(response.status_code, 200)
self.assertEqual(concept.versions.count(), 2)
self.assertFalse(concept.get_latest_version().active_names.get(name='Active name').retired)
self.assertTrue(concept.get_latest_version().retired_names.get(name='Retirable name').retired)

def test_put_200_with_mappings(self): # pylint: disable=too-many-statements
concept = ConceptFactory(parent=self.source, datatype="N/A")
self.assertEqual(concept.versions.count(), 1)
Expand Down