prime-reportstream icon indicating copy to clipboard operation
prime-reportstream copied to clipboard

Update FHIR Resource Deletion Strategy

Open david-navapbc opened this issue 1 year ago • 4 comments

User Story

As a receiver of reports from ReportStream, I want the received items to be "properly" cleaned up when resources are removed, so that my system can safely process the items and confusion can be limited.

Description/Use Case

Presently, when a resource is deleted there is complicated and poorly tested logic that runs to "clean up" the bundle. It has been decided this functionality should be:

  1. Cleaned up
  2. Tested

The resource deletion strategy should be updated to adhere to the following requirements:

  • Resources that are selected for deletion are deleted. This applies to:
    • resources selected to be deleted by FHIR transforms (see #15325)
    • Any Observation resource that fails a conditionFilter/mappedConditionFilter
  • Any references to deleted resources are removed (ex: If DiagnosticReport has a reference to a deleted observation, remove that REFERENCE)
  • "Orphaned" resources (i.e. resources that are no longer referenced by anything) are NOT removed (Ex: If a DiagnosticReport resource no longer references any observations because they were all deleted, DO NOT remove the DiagnosticReport resource)
  • Test suite created to validate the above

Risks/Impacts/Considerations

Dev Notes

Acceptance Criteria

  • [ ] Resource deletion requirements described above are implemented
  • [ ] Resource deletion requirements described above are tested
  • [ ] Existing special handling of DiagnosticReport deletion is removed with the understanding if this needs to happen, it will be done so as part of a FHIR->FHIR enrichment

david-navapbc avatar May 30 '24 22:05 david-navapbc

@GilmoreA6 SME guidance required for the requirements listed above.

arnejduranovic avatar Jul 29 '24 15:07 arnejduranovic

Hey team! Please add your planning poker estimate with Zenhub @brick-green @david-navapbc @jack-h-wang @jalbinson @JFisk42 @mkalish @thetaurean

arnejduranovic avatar Jul 29 '24 16:07 arnejduranovic

Please add your planning poker estimate with Zenhub @adegolier

arnejduranovic avatar Jul 29 '24 16:07 arnejduranovic

Discussion with James (SME):

  • Base logic should not delete orphaned DiagnosticReports
  • If a message is part of an ELR topic, we should delete orphaned DiagnosticReports
    • This can be done via FHIR->FHIR transforms in the FHIRTransform pipeline step

Example how engagement would configure this:

- receiver
- name : ELR_EXAMPLE
- Topic: FULL_ELR
- FHIRTransforms: [ELR_EXAMPLE_REMOVE_OBSERVATIONS.yml, GENERIC_REMOVE_ORPHAN_DIAGNOSTIC_REPORTS.yml]

arnejduranovic avatar Jul 29 '24 20:07 arnejduranovic

Spent time today learning about mappings and how messages are transformed. Specifically FHIR->FHIR. I still need to trace through the data flows and testing rig to better understand how things work presently.

david-navapbc avatar Aug 28 '24 02:08 david-navapbc

talked with @arnejduranovic and @JFisk42 today to get some clarity on the flow involving FhirTransformSchemaElementAction.DELETE

refactored FHIRReceiverFilter.evaluateObservationConditionFilters() to reduce redundant code and improve algorithmic efficiency. all existing tests are still passing.

going to look more closely at the FHIRBundleHelpers methods to ensure they are doing things in a grokkable and efficient manner and shore up testing.

david-navapbc avatar Sep 06 '24 03:09 david-navapbc

PR up. As per scope of ticket I left most of the things alone. I did validate the flow has a robust set of tests and I did refactor the method in FhirReceiverFilter that coordinates filtering/removal of resources. I also sanity checked the utility methods in FHIRBundleHelpers to ensure they were doing it right.

david-navapbc avatar Sep 08 '24 20:09 david-navapbc

we're going a different direction with this but we wanted to save the work I did - which is here in this draft pr

https://github.com/CDCgov/prime-reportstream/pull/15814

david-navapbc avatar Sep 10 '24 16:09 david-navapbc

Some more context: We are currently working on the design for ReceiverFilterFunction which will remove the need for conditionFilter and mappedConditionFilter and the deletion of resources (and the associated cleanup) is currently being planned to happen completely in FHIR->FHIR transforms, thus making this ticket redundant in the long term. The design is not set in stone and may change, but that is where we are at now.

arnejduranovic avatar Sep 12 '24 14:09 arnejduranovic