From cc60e46dd8178f8ad1629552c58b3eecc8f0f0a9 Mon Sep 17 00:00:00 2001 From: Sny Date: Thu, 5 Aug 2021 10:36:57 +0530 Subject: [PATCH 1/3] OpenConceptLab/ocl_issues#878 | Locale type standardization --- core/collections/models.py | 6 +-- core/concepts/constants.py | 4 +- core/concepts/custom_validators.py | 13 ++++--- .../migrations/0019_auto_20210805_0351.py | 39 +++++++++++++++++++ core/concepts/models.py | 28 ++++++++++--- core/concepts/tests/tests.py | 22 ++++++++++- core/integration_tests/tests_concepts.py | 2 +- 7 files changed, 93 insertions(+), 21 deletions(-) create mode 100644 core/concepts/migrations/0019_auto_20210805_0351.py diff --git a/core/collections/models.py b/core/collections/models.py index dd23f8a08..34ab4d932 100644 --- a/core/collections/models.py +++ b/core/collections/models.py @@ -17,7 +17,7 @@ ) from core.common.models import ConceptContainerModel from core.common.utils import is_valid_uri, drop_version -from core.concepts.constants import LOCALES_FULLY_SPECIFIED +from core.concepts.constants import FULLY_SPECIFIED from core.concepts.models import Concept from core.concepts.views import ConceptListView from core.mappings.models import Mapping @@ -139,7 +139,7 @@ def validate(self, reference): concept = reference.concepts[0] self.check_concept_uniqueness_in_collection_and_locale_by_name_attribute( - concept, attribute='type__in', value=LOCALES_FULLY_SPECIFIED, + concept, attribute='type', value=FULLY_SPECIFIED, error_message=CONCEPT_FULLY_SPECIFIED_NAME_UNIQUE_PER_COLLECTION_AND_LOCALE ) self.check_concept_uniqueness_in_collection_and_locale_by_name_attribute( @@ -240,7 +240,7 @@ def add_references_in_bulk(self, expressions, user=None): # pylint: disable=too for concept in ref.concepts: try: self.check_concept_uniqueness_in_collection_and_locale_by_name_attribute( - concept, attribute='type__in', value=LOCALES_FULLY_SPECIFIED, + concept, attribute='type', value=FULLY_SPECIFIED, error_message=CONCEPT_FULLY_SPECIFIED_NAME_UNIQUE_PER_COLLECTION_AND_LOCALE ) self.check_concept_uniqueness_in_collection_and_locale_by_name_attribute( diff --git a/core/concepts/constants.py b/core/concepts/constants.py index cdaac094d..6c9b2650b 100644 --- a/core/concepts/constants.py +++ b/core/concepts/constants.py @@ -4,9 +4,7 @@ CONCEPT_TYPE = 'Concept' SHORT = "SHORT" FULLY_SPECIFIED = "FULLY_SPECIFIED" -LOCALES_FULLY_SPECIFIED = (FULLY_SPECIFIED, "Fully Specified") -LOCALES_SHORT = (SHORT, "Short") -LOCALES_SEARCH_INDEX_TERM = (INDEX_TERM, "Index Term") +DEFINITION = 'Definition' OPENMRS_ONE_FULLY_SPECIFIED_NAME_PER_LOCALE = 'A concept may not have more than one fully specified name in any locale' OPENMRS_NO_MORE_THAN_ONE_SHORT_NAME_PER_LOCALE = 'A concept cannot have more than one short name in a locale' diff --git a/core/concepts/custom_validators.py b/core/concepts/custom_validators.py index 571cefee1..61acd3780 100644 --- a/core/concepts/custom_validators.py +++ b/core/concepts/custom_validators.py @@ -5,12 +5,12 @@ from core.concepts.constants import ( OPENMRS_MUST_HAVE_EXACTLY_ONE_PREFERRED_NAME, OPENMRS_AT_LEAST_ONE_FULLY_SPECIFIED_NAME, OPENMRS_PREFERRED_NAME_UNIQUE_PER_SOURCE_LOCALE, - OPENMRS_FULLY_SPECIFIED_NAME_UNIQUE_PER_SOURCE_LOCALE, LOCALES_SHORT, OPENMRS_SHORT_NAME_CANNOT_BE_PREFERRED, + OPENMRS_FULLY_SPECIFIED_NAME_UNIQUE_PER_SOURCE_LOCALE, OPENMRS_SHORT_NAME_CANNOT_BE_PREFERRED, OPENMRS_NAMES_EXCEPT_SHORT_MUST_BE_UNIQUE, OPENMRS_ONE_FULLY_SPECIFIED_NAME_PER_LOCALE, OPENMRS_NO_MORE_THAN_ONE_SHORT_NAME_PER_LOCALE, OPENMRS_CONCEPT_CLASS, OPENMRS_DATATYPE, OPENMRS_NAME_TYPE, OPENMRS_DESCRIPTION_TYPE, OPENMRS_NAME_LOCALE, OPENMRS_DESCRIPTION_LOCALE, - LOCALES_SEARCH_INDEX_TERM, INDEX_TERM, FULLY_SPECIFIED, SHORT) + INDEX_TERM, FULLY_SPECIFIED, SHORT) from core.concepts.validators import BaseConceptValidator, message_with_name_details @@ -89,8 +89,9 @@ def no_other_record_has_same_name(self, name, versioned_object_id): return not self.repo.concepts_set.exclude( versioned_object_id=versioned_object_id - ).exclude(names__type__in=(*LOCALES_SHORT, *LOCALES_SEARCH_INDEX_TERM)).filter( - is_active=True, retired=False, is_latest_version=True, names__locale=name.locale, names__name=name.name + ).filter( + names__type=FULLY_SPECIFIED, names__locale=name.locale, names__name=name.name, + is_active=True, retired=False, is_latest_version=True, ).exists() @staticmethod @@ -102,8 +103,8 @@ def short_name_cannot_be_marked_as_locale_preferred(concept): if short_preferred_names_in_concept: raise ValidationError({ - 'names': [message_with_name_details(OPENMRS_SHORT_NAME_CANNOT_BE_PREFERRED, - short_preferred_names_in_concept[0])] + 'names': [message_with_name_details( + OPENMRS_SHORT_NAME_CANNOT_BE_PREFERRED, short_preferred_names_in_concept[0])] }) @staticmethod diff --git a/core/concepts/migrations/0019_auto_20210805_0351.py b/core/concepts/migrations/0019_auto_20210805_0351.py new file mode 100644 index 000000000..7f9ed9fa2 --- /dev/null +++ b/core/concepts/migrations/0019_auto_20210805_0351.py @@ -0,0 +1,39 @@ +# Generated by Django 3.1.9 on 2021-08-05 03:51 + +from django.db import migrations + + +def standardize_locale_type(apps, _): + LocalizeText = apps.get_model('concepts', 'LocalizedText') + + def batch_update(values_from, value_to): + batch_size = 1000 + queryset = LocalizeText.objects.filter(type__in=values_from) + total = queryset.count() + for start in range(0, total, batch_size): + end = min(start + batch_size, total) + locales = LocalizeText.objects.filter( + id__in=queryset.order_by('id')[start:end].only('id').values_list('id', flat=True)) + locales.update(type=value_to) + + batch_update( + values_from=['Fully Specified', 'Fully_Specified', 'fully_specified', + 'fully specified', 'full-specified', 'FULLY-SPECIFIED'], + value_to='FULLY_SPECIFIED' + ) + batch_update( + values_from=['index term', 'index_term', 'index-term', 'Index Term', 'Index_Term', 'Index-Term'], + value_to='INDEX_TERM' + ) + batch_update(values_from=['short', 'Short'], value_to='SHORT') + + +class Migration(migrations.Migration): + + dependencies = [ + ('concepts', '0018_auto_20210804_0957'), + ] + + operations = [ + migrations.RunPython(standardize_locale_type) + ] diff --git a/core/concepts/models.py b/core/concepts/models.py index b57d80190..101b18df8 100644 --- a/core/concepts/models.py +++ b/core/concepts/models.py @@ -12,9 +12,10 @@ process_hierarchy_for_new_parent_concept_version from core.common.utils import parse_updated_since_param, generate_temp_version, drop_version, \ encode_string, decode_string -from core.concepts.constants import CONCEPT_TYPE, LOCALES_FULLY_SPECIFIED, LOCALES_SHORT, LOCALES_SEARCH_INDEX_TERM, \ - CONCEPT_WAS_RETIRED, CONCEPT_IS_ALREADY_RETIRED, CONCEPT_IS_ALREADY_NOT_RETIRED, CONCEPT_WAS_UNRETIRED, \ - PERSIST_CLONE_ERROR, PERSIST_CLONE_SPECIFY_USER_ERROR, ALREADY_EXISTS, CONCEPT_REGEX +from core.concepts.constants import CONCEPT_TYPE, CONCEPT_WAS_RETIRED, CONCEPT_IS_ALREADY_RETIRED, \ + CONCEPT_IS_ALREADY_NOT_RETIRED, CONCEPT_WAS_UNRETIRED, \ + PERSIST_CLONE_ERROR, PERSIST_CLONE_SPECIFY_USER_ERROR, ALREADY_EXISTS, CONCEPT_REGEX, FULLY_SPECIFIED, SHORT, \ + INDEX_TERM, DEFINITION from core.concepts.mixins import ConceptValidationMixin @@ -40,6 +41,7 @@ class Meta: created_at = models.DateTimeField(auto_now_add=True) def save(self, force_insert=False, force_update=False, using=None, update_fields=None): + self.format_type() if not self.internal_reference_id and self.id: self.internal_reference_id = str(self.id) super().save(force_insert, force_update, using, update_fields) @@ -59,6 +61,20 @@ def clone(self): locale_preferred=self.locale_preferred ) + def clean(self): + self.format_type() + super().clean() + + def format_type(self): + if self.type and self.type not in ['None', DEFINITION]: + self.type = self.get_formatted_type(self.type) + + @staticmethod + def get_formatted_type(locale_type): + if not locale_type: + return locale_type + return locale_type.upper().replace(' ', '_') + @classmethod def build(cls, params, used_as='name'): instance = None @@ -105,15 +121,15 @@ def build_locales(cls, locale_params, used_as='name'): @property def is_fully_specified(self): - return self.type in LOCALES_FULLY_SPECIFIED + return self.type == FULLY_SPECIFIED @property def is_short(self): - return self.type in LOCALES_SHORT + return self.type == SHORT @property def is_search_index_term(self): - return self.type in LOCALES_SEARCH_INDEX_TERM + return self.type == INDEX_TERM class HierarchicalConcepts(models.Model): diff --git a/core/concepts/tests/tests.py b/core/concepts/tests/tests.py index 66191a1f8..64721ec55 100644 --- a/core/concepts/tests/tests.py +++ b/core/concepts/tests/tests.py @@ -9,8 +9,9 @@ OPENMRS_PREFERRED_NAME_UNIQUE_PER_SOURCE_LOCALE, OPENMRS_SHORT_NAME_CANNOT_BE_PREFERRED, SHORT, INDEX_TERM, OPENMRS_NAMES_EXCEPT_SHORT_MUST_BE_UNIQUE, OPENMRS_ONE_FULLY_SPECIFIED_NAME_PER_LOCALE, OPENMRS_NO_MORE_THAN_ONE_SHORT_NAME_PER_LOCALE, CONCEPT_IS_ALREADY_RETIRED, CONCEPT_IS_ALREADY_NOT_RETIRED, - OPENMRS_CONCEPT_CLASS, OPENMRS_DATATYPE, OPENMRS_DESCRIPTION_TYPE, OPENMRS_NAME_LOCALE, OPENMRS_DESCRIPTION_LOCALE) -from core.concepts.models import Concept + OPENMRS_CONCEPT_CLASS, OPENMRS_DATATYPE, OPENMRS_DESCRIPTION_TYPE, OPENMRS_NAME_LOCALE, OPENMRS_DESCRIPTION_LOCALE, + FULLY_SPECIFIED) +from core.concepts.models import Concept, LocalizedText from core.concepts.tests.factories import LocalizedTextFactory, ConceptFactory from core.concepts.validators import ValidatorSpecifier from core.mappings.tests.factories import MappingFactory @@ -28,6 +29,23 @@ def test_clone(self): self.assertNotEqual(saved_locale.id, cloned_locale.id) self.assertIsNone(cloned_locale.internal_reference_id) + def test_formatted_type(self): + for locale_type in [ + 'fully specified', 'Fully Specified', 'fully_specified', 'Fully_Specified', 'FULLY_SPECIFIED']: + locale = LocalizedText(name='foo', locale='bar', type=locale_type) + locale.full_clean() + self.assertEqual(locale.type, FULLY_SPECIFIED) + + for locale_type in ['short', 'SHORT']: + locale = LocalizedText(name='foo', locale='bar', type=locale_type) + locale.full_clean() + self.assertEqual(locale.type, SHORT) + + for locale_type in ['index term', 'index_term', 'Index Term', 'Index_Term', 'INDEX_TERM']: + locale = LocalizedText(name='foo', locale='bar', type=locale_type) + locale.full_clean() + self.assertEqual(locale.type, INDEX_TERM) + class ConceptTest(OCLTestCase): def test_is_versioned(self): diff --git a/core/integration_tests/tests_concepts.py b/core/integration_tests/tests_concepts.py index effdcac80..454ec1de1 100644 --- a/core/integration_tests/tests_concepts.py +++ b/core/integration_tests/tests_concepts.py @@ -617,7 +617,7 @@ def test_names_post_201(self): "locale": 'en', "locale_preferred": False, "name": 'foo', - "name_type": "Fully Specified" + "name_type": "FULLY_SPECIFIED" } ) self.assertEqual(concept.names.count(), 2) From 5116a9e4bf0c1c27b2c37d7b9bc8aed476868d9f Mon Sep 17 00:00:00 2001 From: Sny Date: Thu, 5 Aug 2021 11:24:07 +0530 Subject: [PATCH 2/3] OpenConceptLab/ocl_issues#878 | name locale type standardization | backfill migration --- core/concepts/models.py | 6 +++--- core/concepts/serializers.py | 16 ++++++++-------- core/concepts/tests/tests.py | 18 +++++++++--------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/core/concepts/models.py b/core/concepts/models.py index 101b18df8..a9a20924b 100644 --- a/core/concepts/models.py +++ b/core/concepts/models.py @@ -92,9 +92,9 @@ def build_name(cls, params): if not name_type or name_type == 'ConceptName': name_type = _type - return cls( - **{**params, 'type': name_type} - ) + instance = cls(**{**params, 'type': name_type}) + instance.format_type() + return instance @classmethod def build_description(cls, params): diff --git a/core/concepts/serializers.py b/core/concepts/serializers.py index 01a3a3939..d1c0e9518 100644 --- a/core/concepts/serializers.py +++ b/core/concepts/serializers.py @@ -61,14 +61,14 @@ class Meta: @staticmethod def create(attrs, instance=None): # pylint: disable=arguments-differ - concept_desc = instance if instance else LocalizedText() - concept_desc.name = attrs.get('name', concept_desc.name) - concept_desc.locale = attrs.get('locale', concept_desc.locale) - concept_desc.locale_preferred = attrs.get('locale_preferred', concept_desc.locale_preferred) - concept_desc.type = attrs.get('type', concept_desc.type) - concept_desc.external_id = attrs.get('external_id', concept_desc.external_id) - concept_desc.save() - return concept_desc + locale = instance if instance else LocalizedText() + locale.name = attrs.get('name', locale.name) + locale.locale = attrs.get('locale', locale.locale) + locale.locale_preferred = attrs.get('locale_preferred', locale.locale_preferred) + locale.type = attrs.get('type', locale.type) + locale.external_id = attrs.get('external_id', locale.external_id) + locale.save() + return locale class ConceptNameSerializer(ConceptLabelSerializer): diff --git a/core/concepts/tests/tests.py b/core/concepts/tests/tests.py index 64721ec55..3deba9ecd 100644 --- a/core/concepts/tests/tests.py +++ b/core/concepts/tests/tests.py @@ -68,7 +68,7 @@ def test_display_name(self): parent=source, names=[ LocalizedTextFactory(locale_preferred=True, locale='en', name='MALARIA SMEAR, QUALITATIVE'), - LocalizedTextFactory(type='SHORT', locale_preferred=False, locale='en', name='malaria sm, qual'), + LocalizedTextFactory(type=SHORT, locale_preferred=False, locale='en', name='malaria sm, qual'), LocalizedTextFactory(locale_preferred=False, locale='en', name='Jungle fever smear'), LocalizedTextFactory(locale_preferred=True, locale='fr', name='FROTTIS POUR DÉTECTER PALUDISME'), LocalizedTextFactory(locale_preferred=False, locale='ht', name='tès MALARYA , kalitatif'), @@ -1078,8 +1078,8 @@ def test_at_least_one_fully_specified_name_per_concept_negative(self): dict( mnemonic='concept', version=HEAD, name='concept', parent=source, concept_class='Diagnosis', datatype='None', names=[ - LocalizedTextFactory.build(name='Fully Specified Name 1', locale='tr', type='Short'), - LocalizedTextFactory.build(name='Fully Specified Name 2', locale='en', type='Short') + LocalizedTextFactory.build(name='Fully Specified Name 1', locale='tr', type=SHORT), + LocalizedTextFactory.build(name='Fully Specified Name 2', locale='en', type=SHORT) ] ) ) @@ -1096,7 +1096,7 @@ def test_duplicate_preferred_name_per_source_should_fail(self): concept_class='Diagnosis', datatype='None', names=[ LocalizedTextFactory.build( name='Concept Non Unique Preferred Name', locale='en', - locale_preferred=True, type='Fully Specified' + locale_preferred=True, type=FULLY_SPECIFIED ), ] ) @@ -1109,7 +1109,7 @@ def test_duplicate_preferred_name_per_source_should_fail(self): name='Concept Non Unique Preferred Name', locale='en', locale_preferred=True, type='None' ), LocalizedTextFactory.build( - name='any name', locale='en', locale_preferred=False, type='Fully Specified' + name='any name', locale='en', locale_preferred=False, type=FULLY_SPECIFIED ), ] ) @@ -1154,7 +1154,7 @@ def test_a_preferred_name_can_not_be_a_short_name(self): dict( mnemonic='concept', version=HEAD, name='concept', parent=source, concept_class='Diagnosis', datatype='None', names=[ - LocalizedTextFactory.build(name="ShortName", locale_preferred=True, type="Short", locale='fr'), + LocalizedTextFactory.build(name="ShortName", locale_preferred=True, type=SHORT, locale='fr'), LocalizedTextFactory.build(name='Fully Specified Name'), ] ) @@ -1239,8 +1239,8 @@ def test_no_more_than_one_short_name_per_locale(self): dict( mnemonic='concept', version=HEAD, name='concept', parent=source, concept_class='Diagnosis', datatype='None', names=[ - LocalizedTextFactory.build(name="fully specified name1", locale='en', type='Short'), - LocalizedTextFactory.build(name='fully specified name2', locale='en', type='Short'), + LocalizedTextFactory.build(name="fully specified name1", locale='en', type=SHORT), + LocalizedTextFactory.build(name='fully specified name2', locale='en', type=SHORT), LocalizedTextFactory.build(name='fully specified name3', locale='fr'), ] ) @@ -1259,7 +1259,7 @@ def test_locale_preferred_name_uniqueness_doesnt_apply_to_shorts(self): mnemonic='concept', version=HEAD, name='concept', parent=source, concept_class='Diagnosis', datatype='None', names=[ LocalizedTextFactory.build(name="mg", locale='en', locale_preferred=True), - LocalizedTextFactory.build(name='mg', locale='en', type='Short'), + LocalizedTextFactory.build(name='mg', locale='en', type=SHORT), ] ) ) From 7c4dc0abfe0a486fb1e4fbfac08fa84d0efaed54 Mon Sep 17 00:00:00 2001 From: Sny Date: Tue, 10 Aug 2021 18:48:51 +0530 Subject: [PATCH 3/3] OpenConceptLab/ocl_issues#878 | updated migration to run SQL directly --- .../migrations/0019_auto_20210805_0351.py | 31 +++---------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/core/concepts/migrations/0019_auto_20210805_0351.py b/core/concepts/migrations/0019_auto_20210805_0351.py index 7f9ed9fa2..c16c03907 100644 --- a/core/concepts/migrations/0019_auto_20210805_0351.py +++ b/core/concepts/migrations/0019_auto_20210805_0351.py @@ -3,31 +3,6 @@ from django.db import migrations -def standardize_locale_type(apps, _): - LocalizeText = apps.get_model('concepts', 'LocalizedText') - - def batch_update(values_from, value_to): - batch_size = 1000 - queryset = LocalizeText.objects.filter(type__in=values_from) - total = queryset.count() - for start in range(0, total, batch_size): - end = min(start + batch_size, total) - locales = LocalizeText.objects.filter( - id__in=queryset.order_by('id')[start:end].only('id').values_list('id', flat=True)) - locales.update(type=value_to) - - batch_update( - values_from=['Fully Specified', 'Fully_Specified', 'fully_specified', - 'fully specified', 'full-specified', 'FULLY-SPECIFIED'], - value_to='FULLY_SPECIFIED' - ) - batch_update( - values_from=['index term', 'index_term', 'index-term', 'Index Term', 'Index_Term', 'Index-Term'], - value_to='INDEX_TERM' - ) - batch_update(values_from=['short', 'Short'], value_to='SHORT') - - class Migration(migrations.Migration): dependencies = [ @@ -35,5 +10,9 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RunPython(standardize_locale_type) + migrations.RunSQL( + sql=r"UPDATE localized_texts SET type='FULLY_SPECIFIED' WHERE type IN " + r"('Fully Specified','Fully_Specified','fully_specified'," + r"'fully specified','full-specified','FULLY-SPECIFIED');", + ), ]