Skip to content

Adding test for Obsgroup. #565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
parthfloyd wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Adding test for Obsgroup. #565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
372d2f0
- created test to find issue for obsgroup
parthfloyd May 11, 2025
37d4684
Fix some minor issues
ibacher May 27, 2025
455546d
removed value for obs group
parthfloyd Jun 2, 2025
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
20 changes: 16 additions & 4 deletions ...rc/main/java/org/openmrs/module/fhir2/api/translators/impl/ObservationTranslatorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.openmrs.module.fhir2.api.translators.ObservationTranslator;
import org.openmrs.module.fhir2.api.translators.ObservationValueTranslator;
import org.openmrs.module.fhir2.api.translators.PatientReferenceTranslator;
import org.openmrs.module.fhir2.api.util.ObsGroupHelper;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

Expand Down Expand Up @@ -84,6 +85,9 @@ public class ObservationTranslatorImpl implements ObservationTranslator {
@Autowired
private ObservationEffectiveDatetimeTranslator datetimeTranslator;

@Autowired
private ObsGroupHelper obsGroupHelper;

@Override
public Observation toFhirResource(@Nonnull Obs observation) {
notNull(observation, "The Obs object should not be null");
Expand Down Expand Up @@ -115,8 +119,9 @@ public Observation toFhirResource(@Nonnull Obs observation) {
}
}
}

obs.setValue(observationValueTranslator.toFhirResource(observation));
if (!obs.hasHasMember()) {
obs.setValue(observationValueTranslator.toFhirResource(observation));
}

obs.addInterpretation(interpretationTranslator.toFhirResource(observation));

Expand Down Expand Up @@ -161,10 +166,17 @@ public Obs toOpenmrsType(Obs existingObs, Observation observation, Supplier<Obs>
existingObs.setConcept(conceptTranslator.toOpenmrsType(observation.getCode()));

for (Reference reference : observation.getHasMember()) {
existingObs.addGroupMember(observationReferenceTranslator.toOpenmrsType(reference));
Obs childObservation = observationReferenceTranslator.toOpenmrsType(reference);
if (childObservation.getObsGroup() == null) {
Copy link
Member

@ibacher ibacher Jun 9, 2025

Choose a reason for hiding this comment

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

Either way, if the observation exists, we need to void it and add it to the obs group. I'm not sure that we need this if... else here. I also think moving things into the ObsGroupHelper probably isn't helping....

Copy link
Member

@ibacher ibacher Jun 9, 2025

Choose a reason for hiding this comment

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

The real question, though, is what happens if childObservation is null (because it couldn't be converted?

Copy link
Member Author

@parthfloyd parthfloyd Jun 15, 2025

Choose a reason for hiding this comment

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

You're right, I mistakenly thought about creation vs retrieval but that won't be case when we're translating to OpenMRS

Copy link
Member Author

@parthfloyd parthfloyd Jun 15, 2025

Choose a reason for hiding this comment

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

This is a good question.
In that case shall we:

  1. Not create the Obsgroup (In this case user can re send the request once they've the child observation fixed/created on server).
  2. Create Obsgroup without the null childObservation (This could result in inconsistent data, think It'd make it tricky to handle as user then needs to update this obsgroup once child observation is fixed).

Let me know which option makes more sense or if we need more input, can send this to Slack or on OpenMRS Talk.

obsGroupHelper.voidAndAddToGroupNewObservation(existingObs, childObservation);
} else {
existingObs.addGroupMember(childObservation);
}
}

observationValueTranslator.toOpenmrsType(existingObs, observation.getValue());
if (observation.hasValue()) {
observationValueTranslator.toOpenmrsType(existingObs, observation.getValue());
}

if (!observation.getInterpretation().isEmpty()) {
interpretationTranslator.toOpenmrsType(existingObs, observation.getInterpretation().get(0));
Expand Down
44 changes: 44 additions & 0 deletions api/src/main/java/org/openmrs/module/fhir2/api/util/ObsGroupHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public License,
* v. 2.0. If a copy of the MPL was not distributed with this file, You can
* obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under
* the terms of the Healthcare Disclaimer located at http://openmrs.org/license.
*
* Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS
* graphic logo is a trademark of OpenMRS Inc.
*/
package org.openmrs.module.fhir2.api.util;

import lombok.Setter;
import org.openmrs.Obs;
import org.openmrs.api.ObsService;
import org.openmrs.module.fhir2.api.dao.FhirObservationDao;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

@Component
@Setter
public class ObsGroupHelper {

@Autowired
private ObsService obsService;

@Autowired
private FhirObservationDao observationDao;

public void voidAndAddToGroupNewObservation(Obs obsGroup, Obs oldObservation) {
Obs newObservation = Obs.newInstance(oldObservation);

//void oldObservation
obsService.voidObs(oldObservation, "voided while translating Observation to add to Obs Group");

//Add to group new observation
newObservation.setObsGroup(obsGroup);
obsGroup.addGroupMember(newObservation);

// @TODO: save new observation, currently this is failing
// observationDao.createOrUpdate(newObservation);
// obsService.saveObs(newObservation, "created while translating observation after adding to Obs Group");
Comment on lines +39 to +41
Copy link
Member

@ibacher ibacher Jun 9, 2025

Choose a reason for hiding this comment

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

Right, you can't save the child observation by itself. Don't do this. Save the obsGroup itself.

Copy link
Member Author

@parthfloyd parthfloyd Jun 15, 2025

Choose a reason for hiding this comment

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

Thanks Ian, however when I'm just saving the Obsgroup, it is not saving the newObervation (the new instance(with obsGroup) of the old child observation)


}
}
25 changes: 25 additions & 0 deletions ...org/openmrs/module/fhir2/providers/r4/ObservationFhirResourceProviderIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.Objects;
import java.util.stream.Collectors;

import ca.uhn.fhir.context.FhirContext;
import lombok.AccessLevel;
import lombok.Getter;
import org.hl7.fhir.r4.model.Bundle;
Expand All @@ -59,6 +60,8 @@ public class ObservationFhirResourceProviderIntegrationTest extends BaseFhirR4In

private static final String JSON_CREATE_OBS_DOCUMENT = "org/openmrs/module/fhir2/providers/ObservationWebTest_create.json";

private static final String JSON_CREATE_OBS_GROUP_DOCUMENT = "org/openmrs/module/fhir2/providers/ObservationGroupWebTest_create.json";

private static final String XML_CREATE_OBS_DOCUMENT = "org/openmrs/module/fhir2/providers/ObservationWebTest_create.xml";

private static final String OBS_JSON_UPDATE_PATH = "org/openmrs/module/fhir2/providers/ObservationWebTest_update.json";
Expand Down Expand Up @@ -274,6 +277,28 @@ public void shouldCreateNewObservationAsJson() throws Exception {

assertThat(newObservation.getId(), equalTo(observation.getId()));
}

@Test
public void shouldCreateNewObservationGroupAsJson() throws Exception {
// read JSON record
String jsonObs;
try (InputStream is = this.getClass().getClassLoader().getResourceAsStream(JSON_CREATE_OBS_GROUP_DOCUMENT)) {
Objects.requireNonNull(is);
jsonObs = inputStreamToString(is, UTF_8);
}

// create obs
MockHttpServletResponse response = post("/Observation").accept(FhirMediaTypes.JSON).jsonContent(jsonObs).go();

// verify created correctly
assertThat(response, isCreated());

String stringContent = response.getContentAsString();
Observation obs = FhirContext.forR4().newJsonParser().parseResource(Observation.class, stringContent);
MockHttpServletResponse childResponse = get("/Observation/"+obs.getHasMember().get(0).getReference().substring(12))
.accept(FhirMediaTypes.JSON).go();
assertThat(childResponse, isOk());
}

@Test
public void shouldCreateNewObservationAsXML() throws Exception {
Expand Down
6 changes: 3 additions & 3 deletions ...ces/org/openmrs/module/fhir2/api/dao/impl/FhirObservationDaoImplTest_initial_data_2.1.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<fhir_concept_source fhir_concept_source_id="3" name="LOINC" url="http://loinc.org" concept_source_id="6" creator="1" date_created="2005-01-01 00:00:00.0" retired="0" uuid="30a5aa84-2df5-46da-aed7-451bafe5593b" />
<fhir_observation_category_map observation_category_map_id="1" concept_class_id="1" observation_category="laboratory" creator="1" date_created="2005-01-01 00:00:00.0" retired="0" uuid="7117a470-0d6c-4bae-b708-c951bccf1133"/>
<fhir_observation_category_map observation_category_map_id="2" concept_class_id="8" observation_category="laboratory" creator="1" date_created="2005-01-01 00:00:00.0" retired="0" uuid="d2cbb346-9404-477f-b3bd-f099a6ce41c1"/>
<concept concept_id="5085" retired="false" datatype_id="1" class_id="1" is_set="false" creator="1" date_created="2004-08-12 00:00:00.0" version="" changed_by="1" date_changed="2005-02-25 11:43:43.0" uuid="5085AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"/>
Copy link
Member

@ibacher ibacher Jun 9, 2025

Choose a reason for hiding this comment

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

I'm also unsure why we're dropping the datatype?

<concept concept_id="5085" retired="false" class_id="1" is_set="false" creator="1" date_created="2004-08-12 00:00:00.0" version="" changed_by="1" date_changed="2005-02-25 11:43:43.0" uuid="5085AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"/>
<concept concept_id="5086" retired="false" datatype_id="1" class_id="1" is_set="false" creator="1" date_created="2004-08-12 00:00:00.0" version="" changed_by="1" date_changed="2005-02-25 11:43:43.0" uuid="5086AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"/>
<concept concept_id="5087" retired="false" datatype_id="1" class_id="1" is_set="false" creator="1" date_created="2004-08-12 00:00:00.0" version="" changed_by="1" date_changed="2005-02-25 11:43:43.0" uuid="5087AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"/>
<concept concept_id="5088" retired="false" datatype_id="1" class_id="1" is_set="false" creator="1" date_created="2004-08-12 00:00:00.0" version="" changed_by="1" date_changed="2005-02-25 11:43:43.0" uuid="5088AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"/>
Expand Down Expand Up @@ -59,12 +59,12 @@
<concept_name concept_id="5089" name="Weight" locale="en_GB" creator="1" date_created="2004-08-12 00:00:00.0" concept_name_id="1407" concept_name_type="FULLY_SPECIFIED" locale_preferred="1" voided="false" uuid="40a1cab8-b4c6-4b0b-9aa1-8770541a02c9"/>
<concept_numeric concept_id="5089" hi_normal="250.0" low_normal="0.0" units="kg" allow_decimal="false" display_precision="10" />
<concept_numeric concept_id="5090" hi_normal="272.0" low_normal="10.0" units="cm" allow_decimal="false" display_precision="10" />
<concept_numeric concept_id="5085" hi_normal="250.0" low_normal="0.0" units="kg" allow_decimal="false" display_precision="10" />
<concept_numeric concept_id="5085" hi_normal="250.0" low_normal="0.0" units="mmHg" allow_decimal="false" display_precision="10" />
Copy link
Member

@ibacher ibacher Jun 9, 2025

Choose a reason for hiding this comment

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

I don't really understand the unit change here?

Copy link
Member Author

@parthfloyd parthfloyd Jun 15, 2025

Choose a reason for hiding this comment

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

This was part of this commit (37d4684), i assume it fixed something during testing?

<obs obs_id="17" person_id="7" concept_id="5085" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="115.0" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="[NULL]" value_coded_name_id="[NULL]" value_complex="[NULL]" value_text="[NULL]" value_datetime="[NULL]" value_drug="[NULL]" uuid="b0b9c14f-2123-4c0f-9a5c-918e192629f0"/>
<obs obs_id="18" person_id="7" concept_id="5086" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="55.0" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="[NULL]" value_coded_name_id="[NULL]" value_complex="[NULL]" value_text="[NULL]" value_datetime="[NULL]" value_drug="[NULL]" uuid="f0af1014-37db-43ea-ab6d-560459aa6573"/>
<obs obs_id="19" person_id="7" concept_id="5087" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="40.0" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="[NULL]" value_coded_name_id="[NULL]" value_complex="[NULL]" value_text="[NULL]" value_datetime="[NULL]" value_drug="[NULL]" uuid="1a9b5242-0ff9-4c5c-adb3-d517632ea63e"/>
<obs obs_id="20" person_id="7" concept_id="5088" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="35.8" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="[NULL]" value_coded_name_id="[NULL]" value_complex="[NULL]" value_text="[NULL]" value_datetime="[NULL]" value_drug="[NULL]" uuid="7e77d071-1f7d-4394-a316-0a458edf28c3"/>
<obs obs_id="21" person_id="7" concept_id="5090" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="188.2" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="[NULL]" value_coded_name_id="[NULL]" value_complex="[NULL]" value_text="[NULL]" value_datetime="[NULL]" value_drug="[NULL]" uuid="30ba0383-9377-46e9-aab3-5fee12e5ed0a"/>
<obs obs_id="21" person_id="7" concept_id="5090" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="188" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="[NULL]" value_coded_name_id="[NULL]" value_complex="[NULL]" value_text="[NULL]" value_datetime="[NULL]" value_drug="[NULL]" uuid="30ba0383-9377-46e9-aab3-5fee12e5ed0a"/>
<obs obs_id="22" person_id="7" concept_id="5092" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="98.3" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="[NULL]" value_coded_name_id="[NULL]" value_complex="[NULL]" value_text="[NULL]" value_datetime="[NULL]" value_drug="[NULL]" uuid="4efa62d2-6b8b-4803-a8fa-3f32ee54db4f"/>
<obs obs_id="23" person_id="7" obs_group_id="22" concept_id="5242" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="14.0" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="[NULL]" value_coded_name_id="[NULL]" value_complex="[NULL]" value_text="[NULL]" value_datetime="[NULL]" value_drug="[NULL]" uuid="744b91f8-bdbc-4950-833b-002244e9fa2b"/>
<obs obs_id="24" person_id="7" concept_id="5242" encounter_id="3" status="FINAL" obs_datetime="2008-07-01 00:00:00.0" interpretation="NORMAL" location_id="1" value_numeric="134.0" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" value_coded="5242" value_coded_name_id="1439" value_complex="cbdft" value_text="AFH56" value_datetime="2008-08-18 14:09:35.0" value_drug="[NULL]" uuid="785li1f8-bdbc-4950-833b-002244e9fa2b"/>
Expand Down
24 changes: 24 additions & 0 deletions ...src/main/resources/org/openmrs/module/fhir2/providers/ObservationGroupWebTest_create.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"resourceType": "Observation",
"status": "final",
"code": {
"coding": [
{
"code": "5085AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
}
]
},
"subject": {
"reference": "Patient/5946f880-b197-400b-9caa-a3c661d23041"
},
"encounter": {
"reference": "Encounter/6519d653-393b-4118-9c83-a3715b82d4ac"
},
"effectiveDateTime": "2014-11-25T22:17:00+11:00",
"hasMember": [
{
"reference": "Observation/30ba0383-9377-46e9-aab3-5fee12e5ed0a",
"type": "Observation"
}
]
}
Loading