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

Move Bundle notification to Core #2478

Merged
merged 1 commit into from
Feb 18, 2022
Merged

Conversation

LTA-Thinking
Copy link
Collaborator

@LTA-Thinking LTA-Thinking commented Feb 10, 2022

Description

Hotfix to move bundle notification to core. Do not merge the PR.
With the introduction of the Resource.Bundle namespace to Core the Resource references to the string resources file had to be made more explicit.

Related issues

Addresses Bug 88575: Move Bundle Meter Notification to Core

Testing

Describe how this change was tested.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 50 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, Dependencies, Enhancement, or New-Feature
  • Tag the PR with Azure API for FHIR if this will release to the Azure API for FHIR managed service (CosmosDB or common code related to service)
  • Tag the PR with Azure Healthcare APIs if this will release to the Azure Healthcare APIs managed service (Sql server or common code related to service)
  • CI is green before merge
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@LTA-Thinking LTA-Thinking requested a review from a team as a code owner February 10, 2022 19:48
@rotodd rotodd self-requested a review February 10, 2022 19:57
{
FhirOperation = bundleType == BundleType.Batch ? AuditEventSubType.Batch : AuditEventSubType.Transaction;
FhirOperation = bundleType;
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, this logic has been moved to BundleHandler.cs since the Hl7.Fhir.Model package is version dependent.


namespace Microsoft.Health.Fhir.Api.Features.Resources.Bundle
namespace Microsoft.Health.Fhir.Core.Features.Resources.Bundle
Copy link
Contributor

Choose a reason for hiding this comment

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

To document my learnings: BundleMetricsNotification now lives in a new namespace, which requires us to explicitly indicate that resources are being referenced from Core.Resource and not Core.Features.Resource. No string resources have been moved - this PR clears up the ambiguity that comes with this namespace change.

@LTA-Thinking LTA-Thinking changed the base branch from releases/2-2-82 to main February 11, 2022 23:39
@LTA-Thinking
Copy link
Collaborator Author

LTA-Thinking commented Feb 11, 2022

Changing to merge to main as hotfix was determined to be unnecessary

@LTA-Thinking LTA-Thinking merged commit 11aa071 into main Feb 18, 2022
@LTA-Thinking LTA-Thinking deleted the hotfix/bundle-notification-move branch February 18, 2022 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants