FM2-640: added FHIR Flag resource #80

ManojLL · 2024-07-09T18:22:06Z

Description of what I changed

added fhir resource provider, flag dao, translator and service for FHIR flag mapping

Issue I worked on

see https://openmrs.atlassian.net/browse/FM2-640

Checklist: I completed these to help reviewers :)

My IDE is configured to follow the code style of this project.

No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend
I have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)

No? -> write tests and add them to this commit git add . && git commit --amend
I ran mvn clean package right before creating this pull request and
added all formatting changes to my commit.

No? -> execute above command
All new and existing tests passed.

No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.
My pull request is based on the latest changes of the master branch.

No? Unsure? -> execute command git pull --rebase upstream master

ibacher · 2024-07-15T17:22:32Z

fhir/src/main/java/org/openmrs/patinetflags/translators/impl/PatientFlagTranslatorImpl.java

+        Narrative narrative = new Narrative();
+        narrative.setStatus(Narrative.NarrativeStatus.GENERATED);
+
+        XhtmlNode xhtmlNode = new XhtmlNode();
+        xhtmlNode.setName(patientFlag.getMessage());
+        narrative.setDiv(xhtmlNode);
+        flag.setText(narrative);


Don't do this in a translator.

@ibacher where should I add this ?

It'd be best if you could plug into the narrative generation system here, otherwise, just leave it alone for now (I don't, off-hand, remember how that system works).

ibacher · 2024-07-15T17:23:03Z

fhir/src/main/java/org/openmrs/patinetflags/translators/impl/PatientFlagTranslatorImpl.java

+        Identifier identifier = new Identifier();
+        identifier.setValue(String.valueOf(patientFlag.getPatientFlagId()));
+        flag.setIdentifier(Collections.singletonList(identifier));


identifier is always the UUID. Never use the database identifier for anything except internal operations.

ibacher · 2024-07-15T17:24:13Z

fhir/src/main/java/org/openmrs/patinetflags/translators/impl/PatientFlagTranslatorImpl.java

+            flag.addCategory(codeableConcept);
+        });
+
+        flag.setCode(priorityTranslator.toFhirResource(patientFlag.getFlag().getPriority()));


Code should be the actual message to display to the user.

ibacher · 2024-07-16T19:24:04Z

fhir/src/main/java/org/openmrs/patinetflags/FhirConstants.java

Module name has a typo here.

Also, instead of just copying this, you should be able to rely on the FhirConstants file from the FHIR2 module, i.e., the fhir submodule should have the fhir2 OMOD as part of its dependencies.

ibacher · 2024-07-16T19:29:16Z

fhir/src/main/java/org/openmrs/patinetflags/impl/FhirFlagServiceImpl.java

+    public IBundleProvider searchFlags(ReferenceAndListParam patient) {
+        SearchParameterMap theParams = new SearchParameterMap();
+        if(patient != null ){
+            theParams.addParameter("aa",patient);


The parameter should probably be FhirConstants.PATIENT_REFERENCE_SEARCH_HANDLER, which will (hopefully) make things easier.

There no issue adding the value null to the SearchParameterMap.

@ibacher I made some changes on the search operation, could you please review ?

Currently, flags can be searched by patient ID, name, and identifiers and also I am working on implementing the ability to search flags using code (flag message), date, and category.

FLAG-67: fixed flag rendering on platform lower versions FLAG-67: change code styles

ibacher

Thanks @ManojLL! Mostly looks good. A few comments though.

omod/src/main/resources/config.xml

ibacher · 2024-07-30T19:47:29Z

pom.xml

 		<module>omod</module>
+		<module>fhir</module>


OMOD is the final artifact, so it always goes last.

Suggested change

<module>omod</module>

<module>fhir</module>

<module>fhir</module>

<module>omod</module>

ibacher · 2024-07-30T19:48:09Z

pom.xml

 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
 		<uiframeworkVersion>3.11.0</uiframeworkVersion>
 		<metadatadeployVersion>1.8.1</metadatadeployVersion>
 		<metadatasharingVersion>1.2.2</metadatasharingVersion>
 		<webservices.rest-omod.version>2.40.0</webservices.rest-omod.version>
 		<legacyuiVersion>1.16.0</legacyuiVersion>
+		<fhirversion>2.2.0</fhirversion>


We should probably depend on an older version of the FHIR2 module. Maybe 1.9.0?

Also fhirVersion like the others

@ibacher if use fhir version 1.9.0 , I get this error: https://pastebin.com/kz3RRTbX. with version 2.0.0 working correctly.

Ok, fair enough.

ibacher · 2024-07-30T19:48:51Z

omod/pom.xml

+        <dependency>
+            <groupId>org.openmrs.module</groupId>
+            <artifactId>patientflags-fhir</artifactId>
+            <version>3.0.8-SNAPSHOT</version>


Suggested change

<version>3.0.8-SNAPSHOT</version>

<version>${project.parent.version}</version>

ibacher · 2024-07-30T19:48:57Z

.../src/test/resources/org/openmrs/module/patientflags/testdata/fhirPatientflagtest-dataset.xml

+	<patientflags_patient_flag patient_flag_id="1" patient_id="1" flag_id="1" message="Test" creator="1" date_created="2007-05-01 00:00:00.0" voided="false" uuid="7d89924e-e8df-4553-a956-95de80529735"/>
+	<patientflags_patient_flag patient_flag_id="2" patient_id="2" flag_id="1" message="Test" creator="1" date_created="2007-05-01 00:00:00.0" voided="true" uuid="7d89924e-e8df-4543-a956-95de80529735"/>
+
+</dataset>


Suggested change

</dataset>

</dataset>

ibacher · 2024-07-30T19:51:41Z

fhir/src/main/java/org/openmrs/module/patienttflags/translators/impl/FlagTranslatorImpl.java

+        if (system == null) {
+            coding.setDisplay(flag.getName());
+        }


Shouldn't this be localizable in some fashion?

Specifically something like FhirUtils#getMetadataTranslation(), but I'm not entirely sure how the flag name gets populated, etc.

ibacher · 2024-07-30T19:52:25Z

fhir/src/main/java/org/openmrs/module/patienttflags/providers/FlagFhirResourceProvider.java

+    public IBundleProvider searchFlags(@OptionalParam(name = Flag.SP_PATIENT, chainWhitelist = { "", Patient.SP_IDENTIFIER,
+                                                Patient.SP_GIVEN, Patient.SP_FAMILY, Patient.SP_NAME }, targetTypes = Patient.class) ReferenceAndListParam patientReference,
+                                       @OptionalParam(name = Flag.SP_RES_ID) TokenAndListParam id,
+                                       @OptionalParam(name = "code") StringAndListParam codeParam,


Code should always be TokenAndListParam

ibacher · 2024-07-30T19:52:33Z

fhir/src/main/java/org/openmrs/module/patienttflags/providers/FlagFhirResourceProvider.java

+                                                Patient.SP_GIVEN, Patient.SP_FAMILY, Patient.SP_NAME }, targetTypes = Patient.class) ReferenceAndListParam patientReference,
+                                       @OptionalParam(name = Flag.SP_RES_ID) TokenAndListParam id,
+                                       @OptionalParam(name = "code") StringAndListParam codeParam,
+                                       @OptionalParam(name = "category") StringAndListParam category,


Ditto category

Also shouldn't category at least have an SP constant?

no, we don't have constant for category and code like flag.SP_CODE and flag.SP_CATEGORY

ibacher · 2024-07-30T19:53:52Z

fhir/src/main/java/org/openmrs/module/patienttflags/impl/FhirFlagServiceImpl.java

+    public IBundleProvider searchFlags(FlagSearchParams patient) {
+        return searchQuery.getQueryResults(patient.toSearchParameterMap(), dao, translator, searchQueryInclude);
+    }
+}


Suggested change

}

}

ibacher · 2024-07-30T19:56:12Z

fhir/pom.xml

+	<properties>
+		<MODULE_ID>${project.parent.artifactId}</MODULE_ID>
+		<MODULE_NAME>${project.parent.name}</MODULE_NAME>
+		<MODULE_VERSION>${project.parent.version}</MODULE_VERSION>
+		<MODULE_PACKAGE>${project.parent.groupId}.${project.parent.artifactId}</MODULE_PACKAGE>
+	</properties>


What are these for?

this is not needed, I will remove this.

ManojLL · 2024-08-01T19:30:09Z

@ibacher I made the requested changes.

ibacher

Small things, but basically looks fine.

ibacher · 2024-08-07T15:57:07Z

fhir/src/test/java/org/openmrs/module/patientflags/impl/FhirFlagServiceImplTest.java

+            @Override
+            protected void validateObject(PatientFlag object) {
+            }


Is this override strictly necessary? We shouldn't be invoking any of the endpoints that use it (create, update, patch).

I removed this. I just added here as it is used in other services

ibacher · 2024-08-07T15:58:33Z

omod/src/main/resources/config.xml

+			<modules>
+				<module>
+					<moduleId>fhir2</moduleId>
+					<version>1.9.0</version>


I think you could use ${fhirversion} here instead and have that correctly filled in.

I changed hard coded fhir version to ${fhirversion}

ManojLL · 2024-08-07T18:21:37Z

@ibacher Do I need to update the OpenMRS FHIR Implementation Guide, or is it sufficient to update the flag module documentation to include information about the FHIR flag mapping?

ibacher · 2024-08-07T19:58:55Z

I think just documenting it here is sufficient. We need to give some thought about how we handle these things with IGs.

ManojLL added 4 commits July 9, 2024 21:14

FM2-640: added FHIR FLag resource

4ed742a

FM2-640: fixed fhir module loading issue

3e43860

FM2-640: added priority translator

4db1284

FM2-640: remove unwanted imports

abf7082

ibacher reviewed Jul 15, 2024

View reviewed changes

ManojLL added 3 commits July 15, 2024 23:36

FM2-640: added flag translator

44fc7f5

FM2-640: added flag translator

426021e

FM2-640: added flag search for patient

5ae17b0

ibacher reviewed Jul 16, 2024

View reviewed changes

ManojLL and others added 21 commits July 17, 2024 12:58

FM2-640: added FHIR2 omod dependency

b3db661

FM2-640: added search flag using ref patient

13e0f84

FM2-640: added search params for search patient flags

4042779

FM2-640: Improved the flag search by adding date and category

7dcf60d

FLAG 67: fixed flags rendering issue on patient page (openmrs#77)

87bec2d

(chore) O3-3551: Use latest Actions in GH Actions (openmrs#81)

1791da9

FLAG-67: fixed flag rendering on platform lower versions (openmrs#82)

d898a0e

FLAG-67: fixed flag rendering on platform lower versions FLAG-67: change code styles

FLAG-75: refactor getFlagsForPatient method with stream API (openmrs#65)

8bb38c0

FLAG-78: added test for priority services (openmrs#84)

676e850

FLAG 78: added unit tests for patient flag (openmrs#78)

5961c59

FM2-640: added unit tests for fhir module

83b44cb

FM2-640: added unit test for service

8596b8a

FM2-640: added flag dao test

74d14f7

FM2-640: updated pom file

2e8f8d7

Merge branch 'master' into FM2-640

7d76bfc

FM2-640: fixed the test error

fadbbd2

FM2-640: add test for FHIR flag service

003c8f4

FM2-640: added test for fhirFlagDao

3558739

FM2-640: fixed Dao test executing issue

d885314

FM2-640: fixed Dao test executing issue

1d36ae2

FM2-640: fixed Dao test executing issue

95241ea

ManojLL added 2 commits July 26, 2024 12:13

FM2-640: added FhirFlagDao tests

17e90b8

FM2-640: added unit test for search queries

acc7041

ManojLL marked this pull request as ready for review July 26, 2024 10:11

ManojLL requested a review from ibacher July 26, 2024 10:24

ibacher requested changes Jul 30, 2024

View reviewed changes

ManojLL added 2 commits August 1, 2024 23:06

FM2-640: changed fhir2 module version 2.2 to 2.0

df8718b

FM2-640: localized using FHIR FhirUtils #getMetadataTranslation()

60fe9a5

ManojLL requested a review from ibacher August 1, 2024 19:30

ibacher approved these changes Aug 7, 2024

View reviewed changes

FM2-640: Implement requested changes

0e60f5d

wikumChamith merged commit 28a792a into openmrs:master Aug 17, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FM2-640: added FHIR Flag resource #80

FM2-640: added FHIR Flag resource #80

ManojLL commented Jul 9, 2024 •

edited

Loading

ibacher Jul 15, 2024

ManojLL Jul 15, 2024

ibacher Jul 15, 2024

ibacher Jul 15, 2024

ibacher Jul 15, 2024

ibacher Jul 16, 2024

ibacher Jul 16, 2024

ibacher Jul 16, 2024

ManojLL Jul 18, 2024

ibacher left a comment

ibacher Jul 30, 2024

ibacher Jul 30, 2024

ibacher Jul 30, 2024

ManojLL Aug 1, 2024 •

edited

Loading

ibacher Aug 1, 2024

ibacher Jul 30, 2024

ibacher Jul 30, 2024

ibacher Jul 30, 2024

ibacher Aug 1, 2024

ibacher Jul 30, 2024

ibacher Jul 30, 2024

ibacher Jul 30, 2024

ManojLL Aug 1, 2024

ibacher Jul 30, 2024

ibacher Jul 30, 2024

ManojLL Aug 1, 2024

ManojLL commented Aug 1, 2024

ibacher left a comment

ibacher Aug 7, 2024

ManojLL Aug 7, 2024

ibacher Aug 7, 2024

ManojLL Aug 7, 2024

ManojLL commented Aug 7, 2024

ibacher commented Aug 7, 2024

	<version>3.0.8-SNAPSHOT</version>
	<version>${project.parent.version}</version>

FM2-640: added FHIR Flag resource #80

FM2-640: added FHIR Flag resource #80

Conversation

ManojLL commented Jul 9, 2024 • edited Loading

Description of what I changed

Issue I worked on

Checklist: I completed these to help reviewers :)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibacher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ManojLL Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ManojLL commented Aug 1, 2024

ibacher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ManojLL commented Aug 7, 2024

ibacher commented Aug 7, 2024

ManojLL commented Jul 9, 2024 •

edited

Loading

ManojLL Aug 1, 2024 •

edited

Loading