#13883 - Fixed IGRA inputs value change listeners#13884
Open
raulbob wants to merge 3 commits intodevelopmentfrom
Open
#13883 - Fixed IGRA inputs value change listeners#13884raulbob wants to merge 3 commits intodevelopmentfrom
raulbob wants to merge 3 commits intodevelopmentfrom
Conversation
- Refactored duplicated listeners for IGRA inputs into two listeners - Fixed error caused by locale conversions with IGRA numeric inputs
📝 WalkthroughWalkthroughReplaces inline ValueChangeListener lambdas in PathogenTestForm with two new protected static inner listeners to centralize numeric-to-boolean synchronization for Tuberculosis IGRA fields, add numeric validation/conversion, derive GT10 when unset, and handle invalid input and ReadOnlyException cases. (34 words) Changes
Sequence Diagram(s)(Skipped — changes are localized UI listener refactors and do not introduce a multi-component sequential flow that meets diagram criteria.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java`:
- Around line 1067-1081: The backing GT10 fields are tri-state and null must be
preserved, so do not derive/write a boolean back when the numeric "checked"
value is null; in both branches around igraGT10Field (the primitive-branch using
Boolean.TRUE.equals(igraGT10Field.getValue()) and the collection-branch using
currentSet.contains(Boolean.TRUE)), add an early guard so you only
compute/compare/set the GT10 field when checked != null, otherwise leave
igraGT10Field untouched; ensure the collection-branch still constructs a
HashSet<Boolean> and sets it only when checked is non-null and differs from the
current state (same change needed at the other occurrence around lines
1158-1171).
- Around line 1036-1044: In PathogenTestForm, update the numeric conversion
logic that builds a converter via ConverterUtil.getConverter and calls
convertToModel so it passes the field/UI locale (e.g., use
igraInputField.getLocale() or the UI locale) instead of null, and wrap the
convertToModel call in a try/catch for ConversionException to translate
conversion failures into a validation error on igraInputField (or add the error
to igraValueProp) so exceptions do not escape the listener; apply the same
change to the identical block around lines 1128-1134.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f887afd-9bb4-402b-b27c-bf692c1c7b2f
📒 Files selected for processing (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
Outdated
Show resolved
Hide resolved
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
Show resolved
Hide resolved
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13884 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java`:
- Around line 46-52: The class uses the nested type Property.ValueChangeListener
but only imports Property, so add an explicit import for the nested interface
(import com.vaadin.v7.data.Property.ValueChangeListener;) to PathogenTestForm so
references to ValueChangeListener compile; update imports at the top of
PathogenTestForm.java next to the other com.vaadin.v7.data imports (used by
BeanFieldGroup, BeanItem, Converter, etc.).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3ef9b0d-be35-45ce-907d-b658c92e5759
📒 Files selected for processing (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
Show resolved
Hide resolved
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13884 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java (2)
1208-1210: Minor typo in comment.Line 1209: "an greater" → "and greater".
- || (!checked && igraNewValue != null && igraNewValue.floatValue() > 10) // not checked but value is filled in an greater than 10 + || (!checked && igraNewValue != null && igraNewValue.floatValue() > 10) // not checked but value is filled in and greater than 10🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java` around lines 1208 - 1210, Fix the typo in the inline comment inside PathogenTestForm near the conditional using variables checked and igraNewValue: change "an greater" to "and greater" so the comment reads "...not checked but value is filled in and greater than 10)". Ensure you update only the comment text adjacent to that if condition in the method containing this conditional.
1181-1190: Consider initializingcheckedtonullfor clarity.The initialization to
falseon line 1181 is immediately overwritten, butnullbetter represents the "unknown" semantic before the field value is examined.Suggested improvement
- Boolean checked = false; + Boolean checked = null; // value can be true or false/null(presumed false) if (!isCollection) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java` around lines 1181 - 1190, The local variable checked in PathogenTestForm (inside the block handling igraGT10Field) should be initialized to null rather than false to reflect the "unknown" semantic before reading the field; change the declaration of checked to Boolean checked = null and keep the existing branches that assign null/Boolean.TRUE/Boolean.FALSE based on igr aGT10Field.getValue() (refer to the igraGT10Field handling code and the Collection<?> set branch) so the initial value is not misleading or overwritten.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java`:
- Around line 1208-1210: Fix the typo in the inline comment inside
PathogenTestForm near the conditional using variables checked and igraNewValue:
change "an greater" to "and greater" so the comment reads "...not checked but
value is filled in and greater than 10)". Ensure you update only the comment
text adjacent to that if condition in the method containing this conditional.
- Around line 1181-1190: The local variable checked in PathogenTestForm (inside
the block handling igraGT10Field) should be initialized to null rather than
false to reflect the "unknown" semantic before reading the field; change the
declaration of checked to Boolean checked = null and keep the existing branches
that assign null/Boolean.TRUE/Boolean.FALSE based on igr aGT10Field.getValue()
(refer to the igraGT10Field handling code and the Collection<?> set branch) so
the initial value is not misleading or overwritten.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 398b353a-f44d-44af-ab9b-450032a15dbd
📒 Files selected for processing (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13884 |
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 #13883 :
Summary by CodeRabbit
Refactor
Bug Fixes
Documentation