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

Support _sort with _include and _revinclude #1915

Closed
lmsurpre opened this issue Feb 5, 2021 · 1 comment
Closed

Support _sort with _include and _revinclude #1915

lmsurpre opened this issue Feb 5, 2021 · 1 comment
Assignees
Labels
P2 Priority 2 - Should Have search showcase Used to Identify End-of-Sprint Demos

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Feb 5, 2021

Is your feature request related to a problem? Please describe.
In SearchUtil.java we have the following code snippet:

if (queryParameters.containsKey(SearchConstants.INCLUDE)
                || queryParameters.containsKey(SearchConstants.REVINCLUDE)) {
            // Make sure _sort is not present with _include and/or _revinclude.
            // TODO: do we really need to forbid this?
            if (queryParameters.containsKey(SearchConstants.SORT)) {
                throw SearchExceptionUtil.buildNewInvalidSearchException(
                        "_sort search result parameter not supported with _include or _revinclude.");
            }

I think we made that simplifying assumption when this was first implemented; we weren't sure what the proper behavior would be (should the sort apply to the include resources or just the match resources?).

Now I'm pretty confident that it should only apply to the match resources and I think we should be able to support these parameters when used in combination.

Describe the solution you'd like
Apply the _sort to the search so that we get the first _count target resources that match the query. Then add in the include/revinclude resources for this page of results.

Describe alternatives you've considered
Continue to disable the use of these parameters together.

Additional context

@lmsurpre lmsurpre added the search label Feb 5, 2021
@lmsurpre lmsurpre added the P2 Priority 2 - Should Have label Mar 11, 2021
@tbieste tbieste self-assigned this Mar 25, 2021
@tbieste tbieste added this to the Sprint 2021-04 milestone Mar 26, 2021
tbieste added a commit that referenced this issue Mar 26, 2021
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 29, 2021
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 29, 2021
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 29, 2021
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 29, 2021
Issue #1915 - Support _sort with _include and _revinclude
@lmsurpre
Copy link
Member Author

I verified this via the following queries in our connectathon environment:

{{baseUrl}}/ExplanationOfBenefit?_include=ExplanationOfBenefit:*&_sort=_lastUpdated&_elements=meta
returned 4 EOB resources, properly sorted by their Resource.meta.lastUpdated values, followed by included Coverage, Organization, and Patient resources.

{{baseUrl}}/ExplanationOfBenefit?_include=ExplanationOfBenefit:*&_sort=-_lastUpdated&_elements=meta
returned 4 EOB resources, reverse sorted by their Resource.meta.lastUpdated values, followed by included Coverage, Organization, and Patient resources.

@tbieste tbieste added the showcase Used to Identify End-of-Sprint Demos label Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Priority 2 - Should Have search showcase Used to Identify End-of-Sprint Demos
Projects
None yet
Development

No branches or pull requests

2 participants