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

Export with history and soft deletes #3519

Merged
merged 67 commits into from
Nov 10, 2023

Conversation

mikaelweave
Copy link
Contributor

@mikaelweave mikaelweave commented Sep 11, 2023

Description

  • Adds new query parameters (_includeHistory and _includeDeleted) to Export operation.
    • Note - not compatible with _typeFilter because there are no longer search parameters with history/soft deletes. Validation added to address this.
  • Updated search options to include IncludeHistory and IncludeDeleted. This is passed down to the data store during searches.
  • Cosmos search will not add filters for history or deleted if either of the corresponding parameters are present and set to true.
  • Updated E2E ExportFixture to add test data. Should improve performance of LongRunningExportTests which have been updated to use this.
    • Note: Other export E2E tests are not using this yet.
  • Updated LongRunningExportTests and added three new tests for historical/delete test cases.
  • Added logic for history/delete export for SQL.

Testing

  • Manual testing by exporting data, modifying resources, deleting resources, and testing again.
  • Added additional E2E testing for export with history/soft deletes.

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)

@mikaelweave mikaelweave added the Area-BulkExport Area related to bulk export. label Sep 11, 2023
@mikaelweave mikaelweave added the Enhancement Enhancement on existing functionality. label Sep 11, 2023
@mikaelweave mikaelweave added this to the S124 milestone Sep 13, 2023
@mikaelweave
Copy link
Contributor Author

@LTA-Thinking - ready for SQL implementation when you are ready.

I have a couple open questions for you as well:

  • For exporting of soft-deleted resources, we have a requirement to add an extension flagging it as "deleted". I put the code in ResourceToNdjsonBytesSerializer as it's only ~3 lines. Looking for feedback here - is it worth creating a new generalized service for any resource modifications that need to occur in $export?
  • Did I miss any area for testing?

@LTA-Thinking
Copy link
Collaborator

Can you also update the Operation Definition for export?

@LTA-Thinking
Copy link
Collaborator

@LTA-Thinking - ready for SQL implementation when you are ready.

I have a couple open questions for you as well:

  • For exporting of soft-deleted resources, we have a requirement to add an extension flagging it as "deleted". I put the code in ResourceToNdjsonBytesSerializer as it's only ~3 lines. Looking for feedback here - is it worth creating a new generalized service for any resource modifications that need to occur in $export?
  • Did I miss any area for testing?

I think adding it to the serializer is sufficient. I don't think a generic solution is needed right now.
The test coverage looks good, thanks for fixing up the export test files. Maybe add one to the SearchOptionsFactoryTests to cover the new query parameters?

@mikaelweave mikaelweave added 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 labels Sep 21, 2023
@mikaelweave mikaelweave modified the milestones: S125, S127 Nov 3, 2023
SergeyGaluzo
SergeyGaluzo previously approved these changes Nov 7, 2023
Copy link
Contributor

@SergeyGaluzo SergeyGaluzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@mikaelweave mikaelweave merged commit a938337 into main Nov 10, 2023
6 checks passed
@mikaelweave mikaelweave deleted the feature/export/include-history-soft-delete branch November 10, 2023 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-BulkExport Area related to bulk export. 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 Enhancement Enhancement on existing functionality. Schema Version backward compatible SQL Scripts If SQL scripts are added to the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants