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

Concepts domain should support other mapping types #193

Closed brandones closed 2 years ago

brandones commented 2 years ago

Right now, the concepts domain only supports SAME-AS mappings. It should support others, such as NARROWER-THAN.

I propose that the headers should be changed to Mappings|SAME-AS, Mappings|NARROWER-THAN, etc. Same as mappings would be interpreted as Mappings|SAME-AS for backwards compatibility.

mogoodrich commented 2 years ago

+1 thanks @brandones

mseaton commented 2 years ago

Instead of cluttering up the headers, which could make for some very noisy and hard to read csvs, what about keeping the existing concept csv mapping headers to assume SAME-AS, and recommend use for things we consider codes and consistent across all concepts (like the PIH and CIEL mappings). Then, we'd have a new domain - conceptreferenceterm (or similar), which would have a CSV with columns:

concept | source | term | type

This would allow multiple csvs (one per source) or a single CSV that contains multiple source/mapping pairs.

@mks-d / @rbuisson thoughts?

mks-d commented 2 years ago

@mseaton very much like with concept names I believe the two options can certainly coexist. Especially if implementers see value in @brandones's suggestion. Your proposal is definitely better for being more complete at properly addressing the details of those mapping types in the data model.

A bit of a non-answer I know, which means that we're supportive of both options and leave it up to you to implement first (and perhaps only) the one that you prefer.

mseaton commented 2 years ago

That makes sense to me. I'll create a new ticket around the conceptreferenceterm domain as needed. Leaving this ticket as enhancing the concept domain, I'd like to modify the existing proposal on this ticket to support the following as supported header formats:

MAPPING-TYPE

MAPPING-TYPE|SOURCE"

MAPPING-TYPE|SOURCE|UNIQUE-KEY

@brandones / @mogoodrich happy to discuss and brainstorm these thoughts further.

mseaton commented 2 years ago

I have a PR open and linked above. @brandones would you be able to build and test this out with the work you are currently doing, and @ibacher or @mks-d would one of you be able to review?

I implemented things as I laid out above, except I ended up keeping Brandon's original suggestion of having a "Mapping" prefix on each column. This seemed a bit safer.