FM2-347: Add support for _has for ServiceRequest and Observation

[md-at-slashwhy]

Description of what I changed

This PR adds the reverse chaining operator (_has) for TestOrders given the restrictions laid out in the ticket.

Issue I worked on

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

Checklist: I completed these to help reviewers :)

• My IDE is configured to follow the code style of this project.

• I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)

• I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.

• All new and existing tests passed.

• My pull request is based on the latest changes of the master branch.

[mherman22]

it's better to use this format we have used for other search params. i.e https://github.com/openmrs/openmrs-module-fhir2/blob/master/api/src/main/java/org/openmrs/module/fhir2/api/search/param/RelatedPersonSearchParams.java

We use this class here to contain shared search (and search result) parameters that apply to all resources.

[md-at-slashwhy]

Thank you for the hint. I tried to make my changes analogous to another has PR and didn't notice that a base class has been introduced in the meantime.
I updated the SearchParams and updated the setters and getters for the old uuid property to use the id property of the base class.

[mherman22]

i would add this explanation in the method javadoc

[md-at-slashwhy]

I was unable to determine why these properties couldn't be represented, but added the information to the javadoc.

[codecov]

Codecov Report

Attention: 39 lines in your changes are missing coverage. Please review.

Comparison is base (6b00ddb) 77.79% compared to head (bc81e6b) 77.78%.
Report is 6 commits behind head on master.

❗ Current head bc81e6b differs from pull request most recent head 9e793c0. Consider uploading reports for the commit 9e793c0 to get more accurate results

Files	Patch %	Lines
.../fhir2/api/dao/impl/FhirServiceRequestDaoImpl.java	75.78%	24 Missing and 7 partials ⚠️
...2/api/search/param/ServiceRequestSearchParams.java	70.83%	6 Missing and 1 partial ⚠️
...2/providers/r3/MedicationFhirResourceProvider.java	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #529      +/-   ##
============================================
- Coverage     77.79%   77.78%   -0.01%     
- Complexity     2679     2722      +43     
============================================
  Files           239      240       +1     
  Lines          7430     7607     +177     
  Branches        897      906       +9     
============================================
+ Hits           5780     5917     +137     
- Misses         1114     1145      +31     
- Partials        536      545       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[md-at-slashwhy]

I added additional tests to check failure cases. Would you kindly run the workflow again?

[itstanany]

Thanks for this pull request! It addresses a valuable functionality gap in the OpenMRS FHIR2 module by implementing the _has parameter for reverse chaining related to ServiceRequest and Observation resources.

The description clearly outlines the problem and the proposed solution using the HAPI HasParam class. The inclusion of unit tests demonstrates a commitment to code quality and ensures the functionality works as expected.

Here are some additional points to consider:

Logging: While the core functionality seems sound, consider if there's a benefit to adding logging statements for _has parameter usage. These logs could help with debugging and monitoring potential issues related to this specific search parameter.

Documentation: Double-check if the pull request includes comprehensive documentation for the implemented _has functionality. This documentation should be clear for developers who might use this functionality in the future. It could include code comments or an update to the FHIR2 module documentation.

Future Considerations: The pull request currently focuses on _has for ServiceRequest and Observation. Could there be potential future enhancements to support _has for other resource types within the OpenMRS FHIR2 module? Mentioning this as a future consideration could spark further discussion.

Overall, this pull request effectively addresses the identified issue and represents a positive contribution to the OpenMRS FHIR2 module. The inclusion of unit tests and clear descriptions strengthen the quality of this contribution.

[md-at-slashwhy]

Thank you for the feedback Tanany, glad to hear this feature is valued and that the effort towards quality is noticeable!
I tried adding warning level logs to each known edge and failure case that might produce unexpected results. Adding trace logging for building the query seemed a little too much, as the resulting query can be logged already.
Regarding documentation it's hard for me to judge what might or might not be useful documentation. I tried adding documentation to each method that does not adhere to running standards in the project (as I understood them). But I'd be glad to add more if you see anything that is unclear or might confuse or mislead somebody. Feel free to add comments and I'll try my best to clarify (and refactor) the code to be more understandable.

Regarding future considerations: As this is my first PR in the OpenMRS project we decided to cut the scope of the PR to have an achievable goal. In addition to extending the _has Parameter to more Searches, the current implementation of the _has Parameter does not allow for all the options described in the FHIR spec. For example, the implementation of a _has...:not was something that was explored early during this Tickets development but then removed from the scope. I totally agree that this functionality could be expended much further!

[md-at-slashwhy]

Development on the feature (#530) this PR is blocked by seems to have stalled and with no clear information on when this PR might be able to move forward, I can no longer dedicate time to maintaining this PR.
Feel free to fork the feature and continue development once #530 is done.