COH-64 add FHIR API to the Cohort module#40
COH-64 add FHIR API to the Cohort module#40parthfloyd wants to merge 12 commits intoopenmrs:masterfrom
Conversation
|
@dkayiwa the build is failing on RestConstants (unless i am mistaken with any config error). Lmk if this solution makes sense: parthfloyd@692d2ea |
|
@parthfloyd did you get a chance to look at the build failure? |
ibacher
left a comment
ibacher
left a comment
There was a problem hiding this comment.
Generally, for these kinds of things, instead of adding FHIR dependencies in the API submodule, it would be cleaner to create a fhir submodule. That submodule can then be selectively loaded depending on whether the FHIR2 module is present and we avoid needing to force everyone who uses this module to use the FHIR2 module. See the patientflags module for an example of this.
| coding.setCode(cohortType.getUuid()); | ||
| } | ||
| if (StringUtils.isNotBlank(cohortType.getName())) { | ||
| coding.setDisplay(cohortType.getName()); |
There was a problem hiding this comment.
For display strings, we'd usually want to call the FhirUtils#getMetadataTranslation() class to get the display string. This way the display value is translatable.
There was a problem hiding this comment.
It does mean, however, that there is no easy way to go from the CohortType display name to the CohortType defintion.
There was a problem hiding this comment.
Thanks @ibacher for the suggestion.
since FhirUtils method expected OpenmrsMetaData child, I created a seperate class CohortFhirUtils#getDataTranslation() to translate BaseOpenmrsData. Let me know if this approach requires change.
- seperate module for fhir
- added default rest uri prefix for tests.
| <dependency> | ||
| <groupId>org.openmrs.module</groupId> | ||
| <artifactId>fhir2-omod</artifactId> | ||
| <version>2.5.0</version> |
There was a problem hiding this comment.
Which fhir2 version are you using? 2.5.0 or 2.7.0?
There was a problem hiding this comment.
You're right, should be 2.7.0 | Looks like a missed commit, pushing it
| <dependency> | ||
| <groupId>org.openmrs.module</groupId> | ||
| <artifactId>fhir2-api</artifactId> | ||
| <version>${openmrsFhir2Version}</version> |
There was a problem hiding this comment.
Of what scope is this dependency?
There was a problem hiding this comment.
I am updating with provided scope assuming if a person uses fhir apis for cohort would have fhir2 module active in the system.
| --> | ||
| <!DOCTYPE module PUBLIC "-//OpenMRS//DTD OpenMRS Config 1.0//EN" | ||
| "https://resources.openmrs.org/doctype/config-1.6.dtd"> | ||
| <module configVersion="1.6"> |
There was a problem hiding this comment.
Each module only has one config.xml—at least that actually does anything—and that's in the OMOD. This change should be applied there, but marking the FHIR2 module as an aware_of_module with a conditional_resource that loads the Jar from this project if the FHIR2 module is present.
There was a problem hiding this comment.
Thanks @ibacher I've updated accordingly.
feel free to let me know if any change is required.
Co-authored-by: Ian <52504170+ibacher@users.noreply.github.com>
3 participants
to support adding cohort using fhir api.
Integrate GroupTranslator & ResourceProvider to cohort module
Related talk: Adding Cohort member through FHIR API.
cc: @ibacher , @dkayiwa @icrc-jofrancisco