diff --git a/core/common/db_functions.py b/core/common/db_functions.py new file mode 100644 index 000000000..bc6c54966 --- /dev/null +++ b/core/common/db_functions.py @@ -0,0 +1,7 @@ +from django.db.models import Func, TextField + + +class ImmutableUnaccent(Func): # pylint: disable=abstract-method + # Immutable wrapper over unaccent (created in common/0004) so it can be used in index expressions. + function = 'public.immutable_unaccent' + output_field = TextField() diff --git a/core/common/migrations/0004_create_ext_unaccent.py b/core/common/migrations/0004_create_ext_unaccent.py new file mode 100644 index 000000000..79c783f58 --- /dev/null +++ b/core/common/migrations/0004_create_ext_unaccent.py @@ -0,0 +1,23 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('common', '0003_create_ext_btree_gin'), + ] + + operations = [ + migrations.RunSQL( + 'CREATE EXTENSION IF NOT EXISTS unaccent SCHEMA public;', + reverse_sql=migrations.RunSQL.noop, + ), + migrations.RunSQL( + """ + CREATE OR REPLACE FUNCTION public.immutable_unaccent(text) RETURNS text + AS $$ SELECT public.unaccent('public.unaccent'::regdictionary, $1) $$ + LANGUAGE sql IMMUTABLE PARALLEL SAFE STRICT; + """, + reverse_sql='DROP FUNCTION IF EXISTS public.immutable_unaccent(text);', + ), + ] diff --git a/core/concepts/custom_validators.py b/core/concepts/custom_validators.py index e60608b9b..1ca93f9eb 100644 --- a/core/concepts/custom_validators.py +++ b/core/concepts/custom_validators.py @@ -4,6 +4,7 @@ from pydash import get from core.common.constants import LOOKUP_CONCEPT_CLASSES +from core.common.db_functions import ImmutableUnaccent from core.common.utils import clean_term from core.concepts.constants import ( OPENMRS_MUST_HAVE_EXACTLY_ONE_PREFERRED_NAME, @@ -94,38 +95,45 @@ def attribute_should_be_unique_for_source_and_locale(self, concept, attribute, e names = [name for name in concept.saved_unsaved_names if getattr(name, attribute)] for name in names: - if self.no_other_record_has_same_name(name, versioned_object_id, filters): + conflicting_concept_id = self.get_conflicting_concept_id(name, versioned_object_id, filters) + if conflicting_concept_id is None: continue - raise ValidationError({'names': [message_with_name_details(error_message, name)]}) + message = message_with_name_details(error_message, name) + raise ValidationError( + {'names': [f"{message}, conflicts with {self.repo.mnemonic}:{conflicting_concept_id}"]}) - def no_other_record_has_same_name(self, name, versioned_object_id, filters=None): + def get_conflicting_concept_id(self, name, versioned_object_id, filters=None): if not self.repo: - return True + return None if not filters: filters = {} + # Case and accent insensitive match; MD5 lookup uses concept_nam_md5_unacc_loc_idx. + normalized_name = Upper(ImmutableUnaccent(Value(name.name))) # Query the localized text row directly so all name constraints apply to the same related record. - return not ConceptName.objects.exclude( + conflicting_concept_ids = ConceptName.objects.exclude( concept__versioned_object_id=versioned_object_id ).exclude( type__in=(*LOCALES_SHORT, *LOCALES_SEARCH_INDEX_TERM, '', None) ).exclude( type__isnull=True ).alias( - name_upper_md5=MD5(Upper('name')) + normalized_name=Upper(ImmutableUnaccent('name')), + normalized_name_md5=MD5(Upper(ImmutableUnaccent('name'))) ).filter( concept__parent=self.repo, concept__is_active=True, concept__retired=False, concept__is_latest_version=True, locale=name.locale, - name__iexact=name.name, - name_upper_md5=MD5(Upper(Value(name.name))), + normalized_name=normalized_name, + normalized_name_md5=MD5(normalized_name), retired=False, **filters - ).exists() + ).values_list('concept__mnemonic', flat=True)[:1] + return next(iter(conflicting_concept_ids), None) @staticmethod def short_name_cannot_be_marked_as_locale_preferred(concept): diff --git a/core/concepts/migrations/0088_conceptname_md5_upper_unaccent_name_locale.py b/core/concepts/migrations/0088_conceptname_md5_upper_unaccent_name_locale.py new file mode 100644 index 000000000..b512b2fd5 --- /dev/null +++ b/core/concepts/migrations/0088_conceptname_md5_upper_unaccent_name_locale.py @@ -0,0 +1,29 @@ +from django.contrib.postgres.operations import AddIndexConcurrently, RemoveIndexConcurrently +from django.db import migrations, models +from django.db.models.functions import MD5, Upper + +import core.common.db_functions + + +class Migration(migrations.Migration): + atomic = False + + dependencies = [ + ('common', '0004_create_ext_unaccent'), + ('concepts', '0087_conceptname_md5_upper_name_locale'), + ] + + operations = [ + AddIndexConcurrently( + model_name='conceptname', + index=models.Index( + MD5(Upper(core.common.db_functions.ImmutableUnaccent('name'))), 'locale', + name='concept_nam_md5_unacc_loc_idx', + condition=models.Q(retired=False), + ), + ), + RemoveIndexConcurrently( + model_name='conceptname', + name='concept_nam_md5_upper_loc_idx', + ), + ] diff --git a/core/concepts/migrations/0089_merge_20261005_0816.py b/core/concepts/migrations/0089_merge_20261005_0816.py new file mode 100644 index 000000000..e0413e4d9 --- /dev/null +++ b/core/concepts/migrations/0089_merge_20261005_0816.py @@ -0,0 +1,14 @@ +# Generated by Django 5.2.17 on 2026-10-05 08:16 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('concepts', '0088_conceptname_md5_upper_unaccent_name_locale'), + ('concepts', '0088_retire_public_edit_access'), + ] + + operations = [ + ] diff --git a/core/concepts/models.py b/core/concepts/models.py index 25dac908a..58da2309f 100644 --- a/core/concepts/models.py +++ b/core/concepts/models.py @@ -9,6 +9,7 @@ from core.common.checksums import ChecksumModel from core.common.constants import ISO_639_1, LATEST, HEAD, ALL +from core.common.db_functions import ImmutableUnaccent from core.common.mixins import SourceChildMixin from core.common.models import VersionedModel, ConceptContainerModel from core.common.tasks import process_hierarchy_for_new_concept, process_hierarchy_for_concept_version, \ @@ -168,8 +169,8 @@ class Meta: condition=Q(locale_preferred=True, retired=False) ), models.Index( - MD5(Upper('name')), 'locale', - name='concept_nam_md5_upper_loc_idx', + MD5(Upper(ImmutableUnaccent('name'))), 'locale', + name='concept_nam_md5_unacc_loc_idx', condition=Q(retired=False) ), ] diff --git a/core/concepts/tests/tests.py b/core/concepts/tests/tests.py index 39fdd3ef6..1679d9468 100644 --- a/core/concepts/tests/tests.py +++ b/core/concepts/tests/tests.py @@ -3094,7 +3094,8 @@ def test_concepts_should_have_unique_fully_specified_name_per_locale(self): concept2.errors, { 'names': [OPENMRS_FULLY_SPECIFIED_NAME_UNIQUE_PER_SOURCE_LOCALE + - ': FullySpecifiedName1 (locale: en, preferred: no)'] + ': FullySpecifiedName1 (locale: en, preferred: no)' + + f', conflicts with {source.mnemonic}:c1'] } ) @@ -3184,7 +3185,88 @@ def test_duplicate_fully_specified_name_per_source_should_fail_case_insensitivel concept2.errors, { 'names': [OPENMRS_FULLY_SPECIFIED_NAME_UNIQUE_PER_SOURCE_LOCALE + - ': cerebral malaria (locale: en, preferred: yes)'] + ': cerebral malaria (locale: en, preferred: yes)' + + f', conflicts with {source.mnemonic}:cerebral-malaria-existing'] + } + ) + + def test_duplicate_fully_specified_name_per_source_should_fail_accent_insensitively(self): + source = OrganizationSourceFactory(custom_validation_schema=OPENMRS_VALIDATION_SCHEMA, version=HEAD) + + def build_concept(mnemonic, name, locale='en'): + return Concept.persist_new( + { + 'mnemonic': mnemonic, + 'version': HEAD, + 'parent': source, + 'concept_class': 'Diagnosis', + 'datatype': 'None', + 'names': [ + ConceptNameFactory.build( + name=name, locale=locale, locale_preferred=True, type='Fully Specified' + ), + ] + } + ) + + concept1 = build_concept('cafe-existing', 'Café') + self.assertEqual(concept1.errors, {}) + + for mnemonic, name in [('cafe-lower', 'cafe'), ('cafe-upper', 'CAFE'), ('cafe-upper-accent', 'CAFÉ')]: + concept = build_concept(mnemonic, name) + self.assertEqual( + concept.errors, + { + 'names': [OPENMRS_FULLY_SPECIFIED_NAME_UNIQUE_PER_SOURCE_LOCALE + + f': {name} (locale: en, preferred: yes), conflicts with {source.mnemonic}:cafe-existing'] + } + ) + self.assertIsNone(concept.id) + + concept_fr = build_concept('cafe-fr', 'cafe', 'fr') + self.assertEqual(concept_fr.errors, {}) + self.assertIsNotNone(concept_fr.id) + + def test_duplicate_preferred_name_per_source_should_fail_accent_insensitively(self): + source = OrganizationSourceFactory(custom_validation_schema=OPENMRS_VALIDATION_SCHEMA, version=HEAD) + concept1 = Concept.persist_new( + { + 'mnemonic': 'concept1', + 'version': HEAD, + 'parent': source, + 'concept_class': 'Diagnosis', + 'datatype': 'None', + 'names': [ + ConceptNameFactory.build( + name='Déjà vu', locale='en', locale_preferred=True, type='Fully Specified' + ), + ] + } + ) + concept2 = Concept.persist_new( + { + 'mnemonic': 'concept2', + 'version': HEAD, + 'parent': source, + 'concept_class': 'Diagnosis', + 'datatype': 'None', + 'names': [ + ConceptNameFactory.build( + name='deja vu', locale='en', locale_preferred=True, type='None' + ), + ConceptNameFactory.build( + name='any name', locale='en', locale_preferred=False, type='Fully Specified' + ), + ] + } + ) + + self.assertEqual(concept1.errors, {}) + self.assertEqual( + concept2.errors, + { + 'names': [OPENMRS_PREFERRED_NAME_UNIQUE_PER_SOURCE_LOCALE + + f': deja vu (locale: en, preferred: yes), conflicts with {source.mnemonic}:concept1'] } ) @@ -3249,7 +3331,8 @@ def test_duplicate_preferred_name_per_source_should_fail(self): concept2.errors, { 'names': [OPENMRS_PREFERRED_NAME_UNIQUE_PER_SOURCE_LOCALE + - ': Concept Non Unique Preferred Name (locale: en, preferred: yes)'] + ': Concept Non Unique Preferred Name (locale: en, preferred: yes)' + + f', conflicts with {source.mnemonic}:concept1'] } )