Skip to content
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

FM2-589: Add support for Json patching operations - Encounter Resource #489

Merged
merged 8 commits into from Jul 7, 2023

Conversation

mherman22
Copy link
Contributor

@mherman22 mherman22 commented Jun 11, 2023

Description of what I changed

Ensure Encounter resource(R4) supports json merge patch operations.

Issue I worked on

see https://issues.openmrs.org/browse/FM2-589

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

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Patch coverage: 50.00% and project coverage change: -0.08 ⚠️

Comparison is base (b40bb10) 77.50% compared to head (c1b51d3) 77.43%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #489      +/-   ##
============================================
- Coverage     77.50%   77.43%   -0.08%     
  Complexity     2631     2631              
============================================
  Files           243      243              
  Lines          7370     7380      +10     
  Branches        908      910       +2     
============================================
+ Hits           5712     5714       +2     
- Misses         1115     1121       +6     
- Partials        543      545       +2     
Impacted Files Coverage Δ
...r2/providers/r4/EncounterFhirResourceProvider.java 33.87% <0.00%> (-2.34%) ⬇️
.../api/translators/impl/EncounterTranslatorImpl.java 90.70% <66.67%> (-9.30%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -106,6 +114,62 @@ public void shouldReturnExistingEncounterAsJson() throws Exception {
assertThat(encounter, validResource());
}

@Test
public void shouldPatchExistingMedicationViaJsonMergePatch() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

Check the naming of functions. These should also be immediately after the update tests.

@mherman22 mherman22 requested a review from ibacher June 27, 2023 17:47
@ibacher ibacher changed the title FM2-589: Add support for Json Merge Patch operations - Encounter Resource FM2-589: Add support for Json patching operations - Encounter Resource Jun 30, 2023
@@ -49,8 +53,12 @@ public class EncounterFhirResourceProviderIntegrationTest extends BaseFhirR4Inte

private static final String MEDICATION_REQUEST_QUERY_INITIAL_DATA_XML = "org/openmrs/module/fhir2/api/dao/impl/FhirEncounterDaoImplTest_initial_data.xml"; // not loaded for all tests

private static final String JSON_PATCH_ENCOUNTER_PATH = "org/openmrs/module/fhir2/providers/Encounter_json_patch.json";
Copy link
Member

Choose a reason for hiding this comment

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

Could you rearrange these so it goes:

  1. Create
    1a. Create JSON
    1b. Create XML
  2. Patch
    2a. Patch JSON

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ibacher ibacher merged commit 68a7900 into openmrs:master Jul 7, 2023
2 of 4 checks passed
@@ -814,6 +822,62 @@ public void shouldThrow404WhenUpdatingNonExistingVisitAsJson() throws Exception
assertThat(response.getContentAsString(), notNullValue());
}

@Test
public void shouldPatchExistingMedicationUsingJsonMergePatch() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

@mherman22 I missed this when reviewing. Could you submit a new PR renaming these methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changes are part of #506

@mherman22 mherman22 deleted the FM2-589 branch July 13, 2023 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants