Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix FHIR Search query with _count=0 and SqlCustomQueryTest issue #3491

Merged
merged 7 commits into from
Nov 9, 2023

Conversation

mahajan-xor
Copy link
Contributor

@mahajan-xor mahajan-xor commented Aug 24, 2023

Description

Fix FHIR Serach query to return result same as _summary=count when passing _count=0.
Previously, 400 response was there for _count=0.
Fix SqlCustomQueryTest issue.

Updated following logic to fix this issue.

  1. Adds logic to remove the _count param and replaces it with _summary param in SearchOptionsFactory .
  2. Updates GetSummaryTypeOrDefault method in HttpContextExtensions class to return summary type.

Related issues

Addresses [issue #3230 , #105699].

Testing

Adds unit test under SearchOperationsFactoryTests
Adds unit test under HttpContextExtensionsTests

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@mahajan-xor mahajan-xor added Bug Bug bug bug. Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR labels Aug 24, 2023
@mahajan-xor mahajan-xor requested a review from a team as a code owner August 24, 2023 13:21
@mahajan-xor mahajan-xor added this to the S122 milestone Aug 24, 2023
@mahajan-xor mahajan-xor changed the title Fix FHIR Serach query with _count=0 Fix FHIR Serach query with _count=0 and SqlCustomQueryTest issue Sep 15, 2023
@mahajan-xor mahajan-xor changed the title Fix FHIR Serach query with _count=0 and SqlCustomQueryTest issue Fix FHIR Search query with _count=0 and SqlCustomQueryTest issue Oct 30, 2023
@mahajan-xor mahajan-xor modified the milestones: S122, 127 Nov 2, 2023
@LTA-Thinking
Copy link
Contributor

What happens if a user specifies both _count=0 and _summary? This looks like it will take whichever is later in the query parameter array. I'm going to run a test on this.

@LTA-Thinking
Copy link
Contributor

What happens if a user specifies both _count=0 and _summary? This looks like it will take whichever is later in the query parameter array. I'm going to run a test on this.

Yeah, these queries give different results:
https://localhost:44348/Patient?_summary=text&_count=0
https://localhost:44348/Patient?_count=0&_summary=text

The first one gives the number of Patients and nothing else.
The second one gives the text summary.

The FHIR spec doesn't say what should be done when multiple summary parameters are given. I feel this should be an error.

Also, we have a bug. When returning just the count of resources we don't return a self link. But section 3.2.1.7.5 of the FHIR search spec says we should.

I know we have an existing issue about the order of search parameters affecting results. @brendankowitz thoughts?

@LTA-Thinking
Copy link
Contributor

Nevermind, I found the section in the spec. It says this is left up to implementations, but they recommend returning an error.

Note that with the exception of _include and _revinclude, search result parameters SHOULD only appear once in a search. If such a parameter appears more than once, the behavior is undefined and a server MAY treat the situation as an error

@mahajan-xor I don't think this needs to be part of this PR as it is a wider issue in our service. It is a bug though, and we should track it. I'll make an item for it.

@LTA-Thinking
Copy link
Contributor

Bug to track this: Bug 111917: Multiple of the same parameter should be an error

@mahajan-xor mahajan-xor merged commit 5b8e2e4 into main Nov 9, 2023
@mahajan-xor mahajan-xor deleted the personal/anilm/fhir-searchquery-fixes branch November 9, 2023 18:44
@brendankowitz brendankowitz added the Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs label Jan 17, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Bug Bug bug bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants