Address issue #184, observations not being updated when changed #185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
dustinburson
commented
Apr 15, 2022
dustinburson
commented
- Address issue Observations not being updated when new data arrives #184, observations not being updated when changed.
- Defect was due to using two different references to the same object for the change comparison.
- Updated CodeValueFhirTemplateProcessor.MergeObservation to perform a DeepCopy of the passed in Observation parameter. The copy is what is returned now by the function to ensure modifications are made to a new instance.
- Moved the logic to set the status of the Observation from the MergeObservation function to the R4FhirImportService.SaveObservationAsync logic. Without the move, the first time a created observation is "updated" will always result in the FHIR resource being saved even if nothing else changed because we were always setting the status to amended.
- Updated unit tests to account for new logic.
- Added extension methods for some common scenarios
- Rename FhirClientValidator to FhirClientValidatorExtensions to match other extension class names
- Rename ServiceCollectionExtrensions to FhirClientExtensions to match purpose.
- Merge content of HttpClientBuilderRegistraionExtensions into FhirClientExtensions.
- Remove registering ITelemetryLogger singleton with FhirClient. ITelemetryLogger is resolved from the service provider now. This was preventing local debugging with the Azure Function.
- Update AddAuthenticationHandler to use the registered singletons for the auth service instead of creating new instances.
- Defect was due to using two different references to the same object for the change comparison. - Updated CodeValueFhirTemplateProcessor.MergeObservation to perform a DeepCopy of the passed in Observation parameter. The copy is what is returned now by the function to ensure modifications are made to a new instance. - Moved the logic to set the status of the Observation from the MergeObservation function to the R4FhirImportService.SaveObservationAsync logic. Without the move, the first time a created observation is "updated" will always result in the FHIR resource being saved even if nothing else changed because we were always setting the status to amended. - Updated unit tests to account for new logic. - Added extension methods for some common scenarios * Rename FhirClientValidator to FhirClientValidatorExtensions to match other extension class names * Rename ServiceCollectionExtrensions to FhirClientExtensions to match purpose. * Merge content of HttpClientBuilderRegistraionExtensions into FhirClientExtensions. * Remove registering ITelemetryLogger singleton with FhirClient. ITelemetryLogger is resolved from the service provider now. This was preventing local debugging with the Azure Function. * Update AddAuthenticationHandler to use the registered singletons for the auth service instead of creating new instances.
rogordon01
approved these changes
Apr 15, 2022
| @@ -113,8 +113,8 @@ public virtual async Task<string> SaveObservationAsync(ILookupTemplate<IFhirTemp | |||
| // Merge the new data with the existing Observation. | |||
| var mergedObservation = MergeObservation(config, existingObservation, observationGroup); | |||
|
|
|||
| // Check to see if there are any changes after merging. | |||
| if (mergedObservation.IsExactly(existingObservation)) | |||
| // Check to see if there are any changes after merging and update the Status to ammended if changed. | |||
There was a problem hiding this comment.
Thanks, fixed in latest version.
| /// <typeparam name="TResource">Type of resource to return.</typeparam> | ||
| /// <param name="resource">Resource to copy.</param> | ||
| /// <returns>New resource object with the contents of the original.</returns> | ||
| public static TResource FullCopy<TResource>(this TResource resource) |
There was a problem hiding this comment.
I only see this referenced in unit tests. Is that intended?
There was a problem hiding this comment.
Covered in other comment but yes for now it is due to project compatibility issues.
| @@ -128,42 +128,42 @@ protected override Observation MergeObservationImpl(CodeValueFhirTemplate templa | |||
| EnsureArg.IsNotNull(grp, nameof(grp)); | |||
| EnsureArg.IsNotNull(existingObservation, nameof(existingObservation)); | |||
|
|
|||
| existingObservation.Status = ObservationStatus.Amended; | |||
| var mergedObservation = existingObservation.DeepCopy() as Observation; | |||
There was a problem hiding this comment.
Should this be using the new extension method?:
var mergedObservation = existingObservation.FullCopy()
There was a problem hiding this comment.
Unfortunately we can't right now. This project is .NET 2.0 standard where as the Extenstions,Fhir.R4 project is .NET 6.0 and can't be downgraded due to dependencies. This project needs to stay .NET Standard till we can move our RP to .NET 6.0.
I could have introduced a completely separate project just for this method but it seemed like overkill.
|
Have you deployed and tested this change out in a personal cluster? |
…ameter. Update the FHIR Service UpdateResourceAsync to automatically set the ifMatchVersion if one isn't already specified and the resource being updated has a version set (i.e. not new).
Not yet in a personal cluster. I ran the Azure Function locally pointed to a remote FHIR Service using these instructions https://github.com/microsoft/iomt-fhir/blob/main/docs/Debugging.md#example-post-body testing various update and create scenarios. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.