-
Notifications
You must be signed in to change notification settings - Fork 1.4k
made $hapi.fhir.undo-merge generic #7476
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
base: master
Are you sure you want to change the base?
Conversation
|
Formatting check succeeded! |
1b7c63c to
5d7dcc6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request makes the $hapi.fhir.undo-merge operation generic, enabling it to work with any FHIR resource type (not just Patient resources). The implementation maintains backward compatibility for Patient resources, which can use either patient-specific or generic parameter names.
Changes:
- Extended
$hapi.fhir.undo-mergeto support all resource types throughBaseJpaResourceProvider - Added backward compatibility logic to handle both patient-specific and generic parameter names for Patient resources
- Updated provenance lookup to try multiple parameter name sets for Patient resources
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
MergeProvenanceSvc.java |
Modified findProvenanceByTargetIdAndSourceIdentifiers to support multiple parameter name sets for backward compatibility |
AbstractMergeOperationInputParameterNames.java |
Added getParameterNamesForResourceType() to return applicable parameter names for a given resource type |
BaseJpaResourceProvider.java |
Added generic resourceUndoMerge() operation for all resource types |
IMergeOperationProviderSvc.java |
Added undoMerge() interface method |
MergeOperationTestHelper.java |
Added callUndoMergeOperation() helper method for testing |
AbstractMergeTestScenario.java |
Added undo-merge parameter building and operation calling methods; fixed DAO lookup to be generic |
PractitionerUndoMergeR4Test.java |
New test class for Practitioner undo-merge operations |
PatientUndoMergeR4Test.java |
Refactored to extend abstract base class; added cross-endpoint compatibility tests |
AbstractGenericUndoMergeR4Test.java |
New abstract base test class providing common undo-merge test methods |
ResourceUndoMergeService.java |
Updated to accept parameter names dynamically instead of hardcoding Patient names |
PatientMergeProvider.java |
Removed patient-specific undo-merge method (moved to generic provider) |
MergeOperationProviderSvc.java |
Added generic undoMerge() implementation with backward compatibility logic |
MergeOperationParametersUtil.java |
Added utility methods for extracting undo-merge parameters from Parameters resources |
JpaR4Config.java |
Updated bean configuration to wire undo-merge service into merge operation provider |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...paserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/MergeOperationParametersUtil.java
Outdated
Show resolved
Hide resolved
5d7dcc6 to
d0eaecd
Compare
| * @param theResourceLimit Resource limit for the operation | ||
| * @return UndoMergeOperationInputParameters ready for use with ResourceUndoMergeService | ||
| */ | ||
| public static UndoMergeOperationInputParameters undoInputParametersFromParameters( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All new methods below don't have unit test coverage. It would be nice to have at least partial unit test coverage for them.
| /** | ||
| * Builds undo-merge operation parameters using this scenario's configuration. | ||
| * Uses resource IDs by default for both source and target. | ||
| * | ||
| * @return the operation input parameters | ||
| */ | ||
| @Nonnull | ||
| public Parameters buildUndoMergeParameters() { | ||
| return buildUndoMergeParameters(true, true); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trivial: unused method, can be removed
Summary
This pull request extends the
$hapi.fhir.undo-mergeoperation to support any FHIR resource type, not just Patient resources. Previously, this operation was only available atPatient/$hapi.fhir.undo-merge. Now it can be invoked on any resource type via{resourceType}/$hapi.fhir.undo-merge(e.g.,Practitioner/$hapi.fhir.undo-merge,Organization/$hapi.fhir.undo-merge).Parameter Names
The generic operation uses resource-agnostic parameter names, following the same pattern as the
$hapi.fhir.mergeoperation:source-resourcesource-patienttarget-resourcetarget-patientsource-resource-identifiersource-patient-identifiertarget-resource-identifiertarget-patient-identifierExample: Generic (Practitioner)
Example: Patient (backward compatible)
Backward Compatibility
For Patient resources, both parameter name styles are supported:
source-resource,target-resource, etc.source-patient,target-patient, etc.Mixing parameter name styles will not work; use either all generic names or all patient-specific names.
Changes
$hapi.fhir.undo-mergeto support all resource types throughBaseJpaResourceProviderPatientMergeProvidertoMergeOperationProviderSvcviaIMergeOperationProviderSvc.undoMerge()MergeOperationParametersUtil.undoInputParametersFromParameters()andundoInputParametersFromOperationParameters()for parameter extractionResourceUndoMergeService.undoMerge()to accept parameter names, enabling support for both generic and patient-specific namesissue: #7477