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

Update FHIR Resource Deletion Strategy #14568

Open
3 tasks
david-navapbc opened this issue May 30, 2024 · 9 comments
Open
3 tasks

Update FHIR Resource Deletion Strategy #14568

david-navapbc opened this issue May 30, 2024 · 9 comments
Assignees
Labels
blocked Issue State label to flag PRs and issues to show they are blocked platform Platform Team

Comments

@david-navapbc
Copy link
Collaborator

david-navapbc commented May 30, 2024

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:
  • 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
  • resources that no longer reference 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
@arnejduranovic
Copy link
Collaborator

@GilmoreA6 SME guidance required for the requirements listed above.

@arnejduranovic arnejduranovic changed the title write suite of tests that exercises orphaned resource pruning Update FHIR Resource Deletion Strategy Jul 29, 2024
@arnejduranovic
Copy link
Collaborator

@arnejduranovic
Copy link
Collaborator

Please add your planning poker estimate with Zenhub @adegolier

@Andrey-Glazkv Andrey-Glazkv removed the ready-for-refinement Ticket is a point where we can productively discuss it label Jul 29, 2024
@arnejduranovic
Copy link
Collaborator

arnejduranovic commented Jul 29, 2024

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]

@david-navapbc
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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.

@Andrey-Glazkv Andrey-Glazkv added blocked Issue State label to flag PRs and issues to show they are blocked ready-for-refinement Ticket is a point where we can productively discuss it labels Sep 10, 2024
@david-navapbc
Copy link
Collaborator Author

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

#15814

@arnejduranovic
Copy link
Collaborator

arnejduranovic commented Sep 12, 2024

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 arnejduranovic removed the ready-for-refinement Ticket is a point where we can productively discuss it label Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Issue State label to flag PRs and issues to show they are blocked platform Platform Team
Projects
Development

No branches or pull requests

4 participants