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.
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
TRUNK-4954 For an obs, obs and its encounter must point to same person #3125
Conversation
@Am-Coder can you include the ticket id in the commit message as recommended at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips |
I amended the commit message but its not showing here. Should I create a new PR? |
@Am-Coder You can just change the title of the PR, if that's what you're looking for. |
@Am-Coder is this a draft because you are still doing more work on it? |
Now that the issue needs to be resolved via a validation check its supposed to be a draft PR. But mistakenly I changed it to ready for review and this process cannot be reverted back. So should I add DRAFT in the title of this PR or create a new PR. |
<obs obs_id="8" person_id="8" concept_id="1" status="FINAL" obs_datetime="2006-02-16 00:00:00.0" location_id="1" value_numeric="8.0" creator="1" date_created="2006-02-17 15:57:35.0" voided="false" uuid="0ee1248e-08aa-4a2c-9f38-fb3875f605e0"/> | ||
<obs obs_id="9" person_id="9" concept_id="1" status="FINAL" obs_datetime="2004-01-01 00:00:00.0" location_id="1" value_numeric="9.0" obs_group_id="2" creator="1" date_created="2006-02-17 15:57:35.0" voided="false" uuid="0ee1248e-08aa-4a2c-9f38-fb3875f605e1"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason behind changing the person_id from 9 to 7?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because in one of the failing tests with obs_id=9 had encounter with person_id=7
@@ -23,15 +23,16 @@ | |||
<obs obs_id="3" person_id="6" concept_id="1" status="FINAL" obs_datetime="2006-02-11 00:00:00.0" location_id="1" value_numeric="3.0" creator="1" date_created="2006-02-12 15:57:35.0" voided="false" uuid="efbba621-51ca-428b-8671-4f4eede10b4b" accession_number="AN1"/> | |||
<obs obs_id="4" person_id="6" concept_id="1" status="FINAL" order_id="42" obs_datetime="2006-02-12 00:00:00.0" location_id="1" value_numeric="4.0" creator="1" date_created="2006-02-13 15:57:35.0" voided="false" uuid="144cb55d-69ab-4009-8256-f4750e8770ee" accession_number="AN2"/> | |||
<obs obs_id="5" person_id="7" concept_id="1" status="FINAL" obs_datetime="2006-02-13 01:00:00.0" location_id="1" value_numeric="5.0" creator="1" date_created="2006-02-14 15:57:35.0" voided="false" uuid="6e76c1f1-6886-49bb-9dc0-f3b304936100"/> | |||
<obs obs_id="6" person_id="7" concept_id="1" status="FINAL" obs_datetime="2006-02-14 00:00:00.0" location_id="1" value_numeric="6.0" creator="1" date_created="2006-02-15 15:57:35.0" voided="false" uuid="14d3b79b-1f8f-4bb3-93c2-3eac710dc053"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason behind changing these lines?
@@ -13,7 +13,7 @@ | |||
<dataset> | |||
<encounter encounter_id="2" encounter_type="2" patient_id="7" location_id="1" form_id="1" encounter_datetime="2008-08-01 00:00:00.0" creator="1" date_created="2008-08-18 14:09:05.0" voided="false" uuid="8382158d-401e-4190-a75d-347b5f04a893"/> | |||
<encounter_provider encounter_provider_id="1" encounter_id="2" provider_id="1" encounter_role_id="1" creator="1" date_created="2006-03-11 15:57:35.0" voided="false" uuid="363fe09b-c1e8-4bdf-aa7d-dd212632f295"/> | |||
<obs obs_id="13" person_id="6" status="FINAL" concept_id="5089" encounter_id="2" obs_datetime="2008-07-01 00:00:00.0" location_id="1" value_numeric="50.0" comments="" creator="1" date_created="2008-08-18 14:09:35.0" voided="false" uuid="4db1b07d-2fc5-46ad-8aa1-ac8357f88457"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we changing the person id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, the reason is Person_id mismatch with its encounter for obs.
@@ -1863,7 +1864,7 @@ public void saveObs_shouldNotVoidAnObsWithNoChanges() { | |||
@Test | |||
public void saveObs_shouldCopyTheFormNamespaceAndPathFieldInEditedObs() { | |||
executeDataSet(INITIAL_OBS_XML); | |||
Obs obs = Context.getObsService().getObs(7); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we change the obs id from 7 to 6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the dataset the namespace parameter has been shifted from from obs_id=7 to obs_is=6. So that test still accesses the obs with formnamespace parameter, I did this.
@@ -1554,8 +1554,8 @@ public void saveObs_shouldCascadeUpdateToNewChildObsGroups() { | |||
Obs origParentObs = obsService.getObs(2); | |||
Set<Obs> originalMembers = new HashSet<>(origParentObs.getGroupMembers(true)); | |||
assertEquals(3, originalMembers.size()); | |||
assertTrue(originalMembers.contains(obsService.getObs(9))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we change the order of these lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am really sorry for this. I did this by mistake. Forgot to change it back.
@@ -1062,7 +1062,7 @@ public void getObservationCount_shouldReturnCountOfObsWhosePersonIsAPatientOnly( | |||
Integer count = obsService.getObservationCount(null, null, null, null, | |||
Collections.singletonList(PERSON_TYPE.PATIENT), null, null, null, null, false, null); | |||
|
|||
Assert.assertEquals(15, count.intValue()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why increase the count to 16?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On making the changes the number of persons that are patients increased to 16.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This increased after making person_id=7 from person_id=9 in obs_id=9. This is in accordance with standardTestDataset
@Am-Coder are you still working on this? |
Yes I am working on it |
e8f7850
to
1877e06
Compare
tl;dr our action detected no activity on this PR and will close it in 30 days if the stale label is not removed. OpenMRS welcomes your contribution! It means a lot to us that you want to contribute to equity in healthcare! This PR has not seen any activity in the last 5 months. That is why we wanted to check whether you are still working on it or need assistance from our side. If you do not have time to continue the work or have moved on you don’t need to do anything. We will automatically close the PR in 30 days. We hope to see you back soon :) |
tl;dr closing this PR since it has not seen any activity in the last 6 months. OpenMRS welcomes your contribution! It means a lot to us that you want to contribute to equity in healthcare! We certainly do not want to discourage you from contributing. We do need to be honest in that OpenMRS has limited resources for reviewing PRs. Your PR has not seen any activity in the last 6 months. This is why we have decided to close this PR for now. This allows us OpenMRS reviewers to focus our limited time to review all other PRs in a timely and professional manner. Please feel free to reassign yourself to the issue you worked on in our JIRA when you have time to focus on it. After that reopen a new PR and we will be glad to work with you to get your contribution merged. Thank you very much for your help and understanding :) |
Description of what I changed
Whenever an obs is saved the person in its encounter will be made same as person in obs. The test given in issue description now passes and I have added it also.
Issue I worked on
see https://issues.openmrs.org/browse/TRUNK-4954
Checklist: I completed these to help reviewers :)
My pull request only contains ONE single commit
(the number above, next to the 'Commits' tab is 1).
No? -> read here on how to squash multiple commits into one
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 andadded 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