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

WIP Fixes #6800,#6799 fhir/std api sort/pagination #6801

Conversation

adunsulag
Copy link
Sponsor Member

Fixes #6800
Fixes #6799

Add common _sort parameter that can be used in the _rest_routes

Add date,_offset,_limit query parameters to the /api/patient route

Added helper method QueryUtils::escapeLimit for escaping limit parameters in SQL queries.

Changed standard api to return the links parameters if there are paginated responses for the search (someone searched with the _limit/_offset parameters).

Added to FhirPatientService the _lastUpdated search parameter that can be used.

Changed FHIR api to support in the base service the ability to do a searchForOpenEMRRecordsWithConfig method that will allow more customized configuration options in the future in addition to the basic search parameters. It currently supports pagination and sort order. Right now only the patient service supports the options.

I modified the ResourceServiceSearchTrait to be able to handle the _sort,_limit,_count parameters and put them all in a _config property that is handled in the FhirServiceBase class to generate the search config and pass it down to services that implement the searchForOpenEMRRecordsWithConfig method.

This is a work in progress commit as the FHIR links in the bundles still need to be populated. Also need to do more extensive testing with the FHIR api on the sort order and _offset/_limit parameters.

Not sure I like seperating out the clause builder for the where clause and having a separate functionality for the orderby / pagination pieces. It might make sense to have it all in one central function governed by the search parameters. Not sure if I like the centralization, or if it violates the Separation of Concerns principle. Still thinking it through but this is an initial stab at things. I really wish we had done doctrine back when we started all of this as it feels very much like we're getting further and further into reproducing all of the goodies of the doctrine ORM. That'd be a major overhaul that would require a good bit of funding unfortunately.

Add common _sort parameter that can be used in the _rest_routes

Add date,_offset,_limit query parameters to the /api/patient route

Added helper method QueryUtils::escapeLimit for escaping limit
parameters in SQL queries.

Changed standard api to return the links parameters if there are
paginated responses for the search (someone searched with the
_limit/_offset parameters).

Added to FhirPatientService the _lastUpdated search parameter that can
be used.

Changed FHIR api to support in the base service the ability to do a
searchForOpenEMRRecordsWithConfig method that will allow more customized
configuration options in the future in addition to the basic search
parameters.  It currently supports pagination and sort order.  Right now
only the patient service supports the options.

I modified the ResourceServiceSearchTrait to be able to handle the
_sort,_limit,_count parameters and put them all in a _config property
that is handled in the FhirServiceBase class to generate the search
config and pass it down to services that implement the
searchForOpenEMRRecordsWithConfig method.

This is a work in progress commit as the FHIR links in the bundles still
need to be populated.  Also need to do more extensive testing with the
FHIR api on the sort order and _offset/_limit parameters.

Not sure I like seperating out the clause builder for the where clause
and having a separate functionality for the orderby / pagination pieces.
It might make sense to have it all in one central function governed by
the search parameters.  Not sure if I like the centralization, or if it
violates the Separation of Concerns principle.  Still thinking it
through but this is an initial stab at things.  I really wish we had
done doctrine back when we started all of this as it feels very much
like we're getting further and further into reproducing all of the
goodies of the doctrine ORM.  That'd be a major overhaul that would
require a good bit of funding unfortunately.
@adunsulag
Copy link
Sponsor Member Author

@sjpadgett and @bradymiller I'd be interested in hearing your thoughts on how FHIR wants us to return a totalCount when we do pagination. When I do that we end up doing 3 queries on every patient search instead of our previous 2 queries per search. One to grab the count, one to grab the unique uuids, and one to grab the individual patients.

Is it worth complying with that part of the spec? In FHIR its a 'should' but we may want to skip it. For example anywhere in the codebase we do a getOne() operation on retrieving the patient we end up doing 3 queries. Not sure sure I like it, but with a hot db cache it may not be that big of a deal.

@adunsulag
Copy link
Sponsor Member Author

Hah, well I ran the API tests but apparently skipping the full test suite is a bad idea lol.

@bradymiller
Copy link
Sponsor Member

2DV-2(1)

@adunsulag adunsulag marked this pull request as ready for review August 31, 2023 17:14
@adunsulag adunsulag merged commit 6c97865 into openemr:master Sep 2, 2023
23 checks passed
@adunsulag adunsulag deleted the feat-openemr-fix-6799-6800-api-sort-last-updated branch April 5, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Patient api endpoints support _lastUpdated search parameter feat: API support for _sort parameter
2 participants