-
Notifications
You must be signed in to change notification settings - Fork 523
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
Add support for include to delete requests #4811
base: main
Are you sure you want to change the base?
Conversation
@@ -73,14 +73,14 @@ | |||
|
|||
_contextAccessor.RequestContext = fhirRequestContext; | |||
var result = new BulkDeleteResult(); | |||
long numDeleted; | |||
IDictionary<string, long> resourcesDeleted = new Dictionary<string, long>(); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
resourcesDeleted
@@ -86,33 +92,47 @@ public async Task<DeleteResourceResponse> Handle(ConditionalDeleteResourceReques | |||
|
|||
private async Task<DeleteResourceResponse> DeleteSingleAsync(ConditionalDeleteResourceRequest request, CancellationToken cancellationToken) | |||
{ | |||
var matchedResults = await _searchService.ConditionalSearchAsync( | |||
var results = await _searchService.ConditionalSearchAsync( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we a limit on the number of included items coming back from ConditionalSearch? i.e. if we delete the primary resource and some of the included items, do some then become orphaned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The limit is still there. On line 108 there is a check to see if we got the "include results truncated" issue. If we did it returns 408 and says there are too many included results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still need to add similar behavior to bulk delete. I'm debating how best to do that so it still deletes as many results as possible. Once we have pagination for include I'll just have it page through the results.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Description
Adds support for
_include
and_revinclude
to conditional and bulk delete requests. Singular deletes still do not support extra parameters.Related issues
Addresses User Story 139073: Phase 1: Work needed for getting cascading delete to work
Testing
New E2E tests have been added
FHIR Team Checklist
Semver Change (docs)
Patch