Skip to content

OpenConceptLab/ocl_issues#878 | Locale type standardization #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 3 additions & 3 deletions core/collections/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 1 addition & 3 deletions core/concepts/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
13 changes: 7 additions & 6 deletions core/concepts/custom_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 18 additions & 0 deletions core/concepts/migrations/0019_auto_20210805_0351.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.1.9 on 2021-08-05 03:51

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('concepts', '0018_auto_20210804_0957'),
]

operations = [
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');",
),
]
34 changes: 25 additions & 9 deletions core/concepts/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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)
Expand All @@ -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
Expand All @@ -76,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):
Expand All @@ -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):
Expand Down
16 changes: 8 additions & 8 deletions core/concepts/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
40 changes: 29 additions & 11 deletions core/concepts/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -50,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'),
Expand Down Expand Up @@ -1060,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)
]
)
)
Expand All @@ -1078,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
),
]
)
Expand All @@ -1091,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
),
]
)
Expand Down Expand Up @@ -1136,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'),
]
)
Expand Down Expand Up @@ -1221,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'),
]
)
Expand All @@ -1241,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),
]
)
)
Expand Down
2 changes: 1 addition & 1 deletion core/integration_tests/tests_concepts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down