Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bulk delete fixes #3689

Merged
merged 3 commits into from
Jan 26, 2024
Merged

Bulk delete fixes #3689

merged 3 commits into from
Jan 26, 2024

Conversation

LTA-Thinking
Copy link
Contributor

Description

Fixes two bugs in bulk delete:

  • Improper scoping for search service
  • Loading full resources for deletions

Related issues

Addresses [issue #].

Testing

Still need to test

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.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@LTA-Thinking LTA-Thinking added Bug Bug bug bug. 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 Jan 24, 2024
@LTA-Thinking LTA-Thinking added this to the S133 milestone Jan 24, 2024
@LTA-Thinking LTA-Thinking marked this pull request as ready for review January 26, 2024 21:28
@LTA-Thinking LTA-Thinking requested a review from a team as a code owner January 26, 2024 21:28
Copy link
Contributor

@mikaelweave mikaelweave left a comment

Choose a reason for hiding this comment

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

Approved - a couple suggestions which can come in follow up PR:

  • extend onlyIds to Cosmos backed (already implemented through query projection)
  • test to show resource is blank for SQL when onlyIds is specified for SQL

@LTA-Thinking
Copy link
Contributor Author

Approved - a couple suggestions which can come in follow up PR:

  • extend onlyIds to Cosmos backed (already implemented through query projection)
  • test to show resource is blank for SQL when onlyIds is specified

onlyIds is already supported in Cosmos, it uses the existing QueryProjection system.
Agreed that more tests would be good.

@LTA-Thinking LTA-Thinking merged commit e0d665b into main Jan 26, 2024
5 checks passed
@LTA-Thinking LTA-Thinking deleted the personal/rojo/bulk-delete-bugs branch January 26, 2024 22:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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 Bug Bug bug bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants