Patient Merge, encounter.getAllObs is not giving all obs.

Hey team,

We are upgrading openmrs to 2.0.x. Currently, we are using the openmrs 2.0.2. We are facing an issue while merging two patients. After merge when we call encounter.getAllObs() it doesn’t give all observations instead it is giving just top level obs for newly created obs.

Here is a test reproducing the scenario:-

    @Test
    public void shouldMoveAllObsWithSameHierarchy() throws Exception {
        executeDataSet("testDataSets/mergeDS/obsPatientMergeDS.xml");

        Patient notPreffered = patientService.getPatient(11);
        Patient preffered = patientService.getPatient(21);

        List<Encounter> prefferedEncounters = encounterService.getEncountersByPatient(preffered);
        List<Encounter> notPrefferedEncounters = encounterService.getEncountersByPatient(notPreffered);
        assertEquals(1, prefferedEncounters.size());
        assertEquals(1, notPrefferedEncounters.size());
        assertEquals(0, prefferedEncounters.get(0).getAllObs(true).size());
        assertEquals(1, notPrefferedEncounters.get(0).getObsAtTopLevel(true).size());
        assertEquals(4, notPrefferedEncounters.get(0).getAllObs(true).size());

        patientService.mergePatients(preffered, notPreffered);

        List<Encounter> mergedPrefferedEncounters = encounterService.getEncountersByPatient(preffered);
        List<Encounter> mergedNotPrefferedEncounters = encounterService.getEncountersByPatient(notPreffered);
        assertEquals(0, mergedNotPrefferedEncounters.size());
        assertEquals(2, mergedPrefferedEncounters.size());
        
        assertEquals(1,mergedPrefferedEncounters.get(0).getObsAtTopLevel(false).size());
        // Call to getAllObs also just returns only one obs. It should return 4 obs ideally.
        assertEquals(1, mergedPrefferedEncounters.get(0).getAllObs(false).size());
    }

The test is passing. If we see the last assertion I am calling getAllObs which should return 4 same as notPreffered before merge but it gives 1.

Do you get the same problem with platform 2.0.4.1?

No, We have not tried with 2.0.4.1. We have taken Bahmni 0.87 which comes with openmrs 2.0.2. Though I can upgrade the omod just to check if it is not there 2.0.4.1.

@dkayiwa, I tried upgrading to 2.0.4.1. Now the encounter.getAllObs() is returning correct number of obs. Still, I feel it’s not the expected result. I will add more details about it.

Just before merge if I check the obs in that encounter this is what I get

Where obs-111 is the root obs. obs-112 is child of obs-111 and obs-113 is child of obs-112. There is another obs obs-114 child of obs-111 but it’s voided so its not there in the result. Now ideally after merge these should be voided and there should be 3 new obs 115,116,117 with same hierarchy.

This is what I get after merge

I was expecting 112 and 113 to be voided, but they are still unvoided and referring to older parent-obs as obs-group. Obs-115 has two different children which are not coming in the result for some reason though one of them is not voided.

These are observations for encounter.getAllObs() which doesn’t include voided encounters. If I call encounter.getAllObs(true) then before merge the number of obs is 4(1 voided, 3 unvoided) and after merge it is 5(2 voided, 3 unvoided). In my opinion it should be 8(5 voided, 3 unvoided).

Do you have a failing test that runs out of the box on the core platform?

Hey, I have written the test in openmrs core, but its in my local. I can post the test code and test-datasets if you want.

Can you do a pull request for the test?

Thanks @mddubey. This definitely looks like a bug related to obs group handling in the patient merge process.

hey @dkayiwa, I have created a pull request here(https://github.com/openmrs/openmrs-core/pull/2066). I have just added the failing test. It’s the last test in PatientServiceTest. If I have missed anything let me know.

@mddubey can you also create a ticket and add this pull request link?

I have created the ticket. https://issues.openmrs.org/browse/TRUNK-5112

I have made it ready for work. @mddubey do you think you can go ahead and claim the ticket to fix it? :slight_smile:

Hey @dkayiwa, I had already looked at the code. In my local repo, will push in the same pull request. I have made the tests passing as well. The fix which I did is:-

In EncounterServiceImpl at the end of saveEncounter, we are removing the exisitng obs and adding new obs:-

for (Obs o : toRemove) {
    encounter.removeObs(o);
}
for (Obs o : toAdd) {
    encounter.addObs(o);
}

The above part just removes/adds the single root level of obs. I am not sure what is the reason actully, might be changes in cascading of groupMembers column.

What I have done is replaced this part as remove existing obs with it’s group member recursively and then add new obs with it’s group member recursively. With this way all the tests are passing, but I am not sure if there should be another way to fix it.

Do you mind claiming the ticket and follow the guidelines put here? https://wiki.openmrs.org/display/docs/Pull+Request+Tips Then we shall follow up with a review. Thanks again for taking the time to fix this. :slight_smile:

Okey, I will go through this and create a new pull request since I started working on this without having a jira card. I will close the older pull request.