Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
39 changes: 39 additions & 0 deletions core/concepts/migrations/0019_auto_20210805_0351.py
Original file line number Diff line number Diff line change
@@ -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))
Copy link
Copy Markdown
Contributor

@rkorytkowski rkorytkowski Aug 10, 2021

Choose a reason for hiding this comment

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

I feel this will not perform well... start:end is a slow one for millions of records to update as it uses OFFSET. Also if you use 'values_list' you don't need 'only'. We should be able to change it to an update without a subquery like:
update localized_texts set type='FULLY_SPECIFIED' where type in ('Fullly Specified', 'Fully_Specified', ...)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ya, I can change that to run direct SQL:
update localized_texts set type='FULLY_SPECIFIED' where type in ('Fullly Specified', 'Fully_Specified', ...)

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)
]
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