Allowing configurable value override when equal and DataReplacementStrategy.IF_NOT_ALREADY_PRESENT#14127
Merged
Conversation
…tStrategy.IF_NOT_ALREADY_PRESENT
📝 WalkthroughWalkthroughAdds a new ChangesEqual Value Override Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant DataPatcherImpl
participant EqualValueOverrideHelper
participant SystemConfigurationValueFacade
Client->>DataPatcherImpl: patch(field, newValue)
DataPatcherImpl->>DataPatcherImpl: compare currentValue vs newTypedValue
alt values differ
DataPatcherImpl->>EqualValueOverrideHelper: allowedOverride(currentValue)
EqualValueOverrideHelper->>SystemConfigurationValueFacade: getValue(ALLOWED_EQUALITY_VALUE_OVERRIDE_KEY)
SystemConfigurationValueFacade-->>EqualValueOverrideHelper: configured allowlist
EqualValueOverrideHelper-->>DataPatcherImpl: allowed / not allowed
alt allowed
DataPatcherImpl->>DataPatcherImpl: apply newTypedValue
else not allowed
DataPatcherImpl-->>Client: DataPatchFailure(FORBIDDEN_VALUE_OVERRIDE)
end
else values equal
DataPatcherImpl->>DataPatcherImpl: skip override
end
Related issues: Suggested labels: backend, enhancement, patch Suggested reviewers: backend maintainers familiar with 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.java (1)
88-110: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInitialize the new helper in the explicit constructor.
Instances created through the public constructor leave
equalValueOverrideHelpernull, then line 311 can throw whenIF_NOT_ALREADY_PRESENTsees a differing value.Proposed fix
public DataPatcherImpl( PatchFieldHelper patchFieldHelper, ValueMapperRegistry valueMapperRegistry, FieldCustomMapperRegistry fieldCustomMapperRegistry, PatchEqualityCheckersRegistry patchEqualityCheckersRegistry, BusinessDtoFacade businessDtoFacade, FeatureConfigurationFacadeEjb.FeatureConfigurationFacadeEjbLocal featureConfigurationFacade, - ConfigFacadeEjb.ConfigFacadeEjbLocal configFacade) { + ConfigFacadeEjb.ConfigFacadeEjbLocal configFacade, + EqualValueOverrideHelper equalValueOverrideHelper) { this.patchFieldHelper = patchFieldHelper; this.valueMapperRegistry = valueMapperRegistry; this.fieldCustomMapperRegistry = fieldCustomMapperRegistry; this.patchEqualityCheckersRegistry = patchEqualityCheckersRegistry; this.businessDtoFacade = businessDtoFacade; this.featureConfigurationFacade = featureConfigurationFacade; this.configFacade = configFacade; + this.equalValueOverrideHelper = equalValueOverrideHelper; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.java` around lines 88 - 110, The public DataPatcherImpl constructor is missing initialization for equalValueOverrideHelper, so instances created there can hit a null reference when IF_NOT_ALREADY_PRESENT is processed. Update the explicit constructor to accept and assign EqualValueOverrideHelper alongside the other dependencies, and make sure the field is consistently set for both construction paths in DataPatcherImpl.
🧹 Nitpick comments (1)
sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java (1)
773-793: 📐 Maintainability & Code Quality | 🔵 TrivialMinor: exception-driven control flow to check for an existing default category.
Catching
IllegalStateExceptionto detect "category doesn't exist yet" works but is a smell ifSystemConfigurationCategoryServiceexposes (or could expose) a non-throwing lookup (e.g.,Optional-returning finder). Not blocking since this is test-only setup code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java` around lines 773 - 793, The helper setAllowedEqualityValueOverride currently uses exception-driven control flow to detect whether the default SystemConfigurationCategory exists. Update the setup to use a non-throwing lookup from SystemConfigurationCategoryService if available, or add one, and only create/persist a new category when the lookup returns empty instead of catching IllegalStateException. Keep the rest of the test setup unchanged, including the SystemConfigurationValueDto creation and save path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.java`:
- Around line 310-320: The override check in DataPatcherImpl currently validates
currentValue, but it should validate the incoming configured exception value
instead so allowlisted inputs like YesNoUnknown.UNKNOWN/NA/Unk can pass through
IF_NOT_ALREADY_PRESENT. Update the condition around
patchEqualityCheckersRegistry.areEqual and
equalValueOverrideHelper.allowedOverride to compare against the incoming
untypedTargetValue/newTypedValue as appropriate, and keep the
FORBIDDEN_VALUE_OVERRIDE failure path for non-allowlisted inputs.
---
Outside diff comments:
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.java`:
- Around line 88-110: The public DataPatcherImpl constructor is missing
initialization for equalValueOverrideHelper, so instances created there can hit
a null reference when IF_NOT_ALREADY_PRESENT is processed. Update the explicit
constructor to accept and assign EqualValueOverrideHelper alongside the other
dependencies, and make sure the field is consistently set for both construction
paths in DataPatcherImpl.
---
Nitpick comments:
In
`@sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java`:
- Around line 773-793: The helper setAllowedEqualityValueOverride currently uses
exception-driven control flow to detect whether the default
SystemConfigurationCategory exists. Update the setup to use a non-throwing
lookup from SystemConfigurationCategoryService if available, or add one, and
only create/persist a new category when the lookup returns empty instead of
catching IllegalStateException. Keep the rest of the test setup unchanged,
including the SystemConfigurationValueDto creation and save path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a99b6bc-4022-4dcd-b063-8d7c92d65fb9
📒 Files selected for processing (4)
sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.javasormas-backend/src/main/java/de/symeda/sormas/backend/patch/EqualValueOverrideHelper.javasormas-backend/src/test/java/de/symeda/sormas/backend/patch/EqualValueOverrideHelperTest.javasormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java
Pa-Touche
changed the title
obinna-h-n
approved these changes
Jul 2, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes #14123
Summary by CodeRabbit
New Features
Bug Fixes
Tests