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-481: Clean up parameter passing in Person Service class search method #441

Merged
merged 8 commits into from May 12, 2023

Conversation

mherman22
Copy link
Contributor

@mherman22 mherman22 commented Nov 4, 2022

Description of what I changed

  • Added PersonSearchParams class that extends the BaseResourceSearchParams.
  • Refactor all provider classes (r3 and r4).
  • Ensure all tests are passing

Issue I worked on

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

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 Nov 4, 2022

Codecov Report

Base: 79.58% // Head: 79.58% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (172a986) compared to base (aa2a070).
Patch coverage: 87.10% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #441      +/-   ##
============================================
- Coverage     79.58%   79.58%   -0.01%     
- Complexity     2539     2548       +9     
============================================
  Files           233      234       +1     
  Lines          6901     6918      +17     
  Branches        834      834              
============================================
+ Hits           5492     5505      +13     
- Misses          896      899       +3     
- Partials        513      514       +1     
Impacted Files Coverage Δ
...ule/fhir2/api/search/param/PersonSearchParams.java 85.71% <85.71%> (ø)
...s/module/fhir2/api/impl/FhirPersonServiceImpl.java 37.50% <100.00%> (-36.18%) ⬇️
...fhir2/providers/r3/PersonFhirResourceProvider.java 89.47% <100.00%> (ø)
...fhir2/providers/r4/PersonFhirResourceProvider.java 88.24% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

Comment on lines 66 to 69
.addParameter(FhirConstants.CITY_SEARCH_HANDLER, FhirConstants.CITY_PROPERTY, getCity())
.addParameter(FhirConstants.STATE_SEARCH_HANDLER, FhirConstants.STATE_PROPERTY, getState())
.addParameter(FhirConstants.POSTALCODE_SEARCH_HANDLER, FhirConstants.POSTAL_CODE_PROPERTY, getPostalCode())
.addParameter(FhirConstants.COUNTRY_SEARCH_HANDLER, FhirConstants.COUNTRY_PROPERTY, getCountry());
Copy link
Member

Choose a reason for hiding this comment

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

These should all be using the ADDRESS_SEARCH_HANDLER, shouldn't they?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, will update that

@ibacher ibacher merged commit 8373a4b into openmrs:master May 12, 2023
@mherman22 mherman22 deleted the FM2-481-personSearchParams branch May 13, 2023 18:11
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