Add support for include to delete requests #4811
Conversation
LTA-Thinking
added
Enhancement
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -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
...rosoft.Health.Fhir.Shared.Core/Features/Resources/Delete/ConditionalDeleteResourceHandler.cs
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Delete/DeletionService.cs
Show resolved
Hide resolved
| @@ -183,6 +195,12 @@ public async Task<long> DeleteMultipleAsync(ConditionalDeleteResourceRequest req | |||
| onlyIds: true, | |||
| logger: _logger); | |||
| } | |||
|
|
|||
| if (CheckFhirContextIssues()) | |||
There was a problem hiding this comment.
NIT - could this function name be closer to what the function does?? - like CheckFhirContextTooManyIncludes. Here it seems like any context issue means tooManyIncludeResults
There was a problem hiding this comment.
Fair, I had originally intended for the function to look for any context issues but the only one that is relevant right now is the too many include results warning.
| { | ||
| CheckBulkDeleteEnabled(); | ||
|
|
||
| var resourceTypes = new Dictionary<string, long>() |
There was a problem hiding this comment.
Would it make sense to have multiple links / resource types in this test? Could make the test more robust.
There was a problem hiding this comment.
Do you mean like having another include statement and a resource for it to find?
There was a problem hiding this comment.
I was thinking (resource w/ reference) -> (two different resources).
Example is Encounter.participant[x].actor or Encounter.appointment.
If this doesn't add value - can be skipped - but my though was > 1 resource would be a more full test.
...rosoft.Health.Fhir.Shared.Core/Features/Resources/Delete/ConditionalDeleteResourceHandler.cs
Show resolved
Hide resolved
Description
Adds support for
_includeand_revincludeto 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