mekomsolutions / openmrs-module-initializer

The OpenMRS Initializer module is an API-only module that processes the content of the configuration folder when it is found inside OpenMRS' application data directory.
MIT License
23 stars 79 forks source link

[#136] Concept names defined with no UUIDs to recreate "same" existing names with unmatched UUIDs. #167

Closed Ruhanga closed 2 years ago

Ruhanga commented 2 years ago

Before ConceptNames were made more configurable [before #136], existing ConceptNames were detached/removed from the main Concept in favour of new names brought in by the config irrespective of whether the string names were equal or not. This behaviour should be maintained.

Currently CSV names not configured with a uuid could match existing names with different uuids and not get their uuids updated with the default seeded uuid of their associated concepts.

This PR therefore allow for the CSV concept names, not configured with a uuid to be saved with a default seeded uuid from the associated concept. This may resulting in the voiding of existing equal names but with different uuids in favour of the CSV defined name.

Ruhanga commented 2 years ago

@mseaton, @mks-d, after a conversation with @ibacher and following the suggestion you mentioned, I've added commits resulting in existing names having different uuids from equivalent csv names being voided with a modification on the unit tests to this effect.

mks-d commented 2 years ago

My description of this change is the following: when a CSV does not set UUIDs for the concept names that it defines then it implicitly says that the UUIDs should be seeded. Therefore any existing names that match (same type, same locale, same name string) will be voided if their UUID is in contravention with the UUID specified by the CSV (that is: seeded UUID).

So yes maybe there are existing names whose UUID was once set to be something specific on purpose (and maybe other processes rely on those UUIDs to be what they are, like a sync process or otherwise) in which case the CSV redefinition of those names need to respecify those UUIDs again, otherwise they are at risk of being voided.

mseaton commented 2 years ago

@mks-d and @Ruhanga - sorry I was offline for a bit. This all sounds good. Thanks for the work on this.

icrc-fdeniger commented 2 years ago

Hi, I've just tested the last version 2.3.0-SNAPHOST, and I got a lot of error like: org.openmrs.api.DuplicateConceptNameException: 'XXXXX' is a duplicate name in locale 'en' for the same concept

Before using the last version, we had 195k concept_names with 2.5k voided concept_names After restarting OpenMRS with the last iniz version, we got 7.5k voided concept names. I'd expect to have almost 195k voided concept_name.

I tried also to calculate an uuid for a concept name by using Utils.generateUuidFromObjects and I didn't get the expected result

Ruhanga commented 2 years ago

Thanks @icrc-fdeniger for reporting this. It is possible that the new ConceptNamess are saved before the old ones are updated which may result into the error/exception you have shared. To resolve this I've pushed a PR updating existing names before new ones are created/added.