#13557 - Make "Verified by Supervisor" checkbox in the pathogen test … #13783
Conversation
…form non-mandatory
📝 WalkthroughWalkthroughThis change makes the testResultVerified ("Verified by Supervisor") field optional except when a test is marked as via LIMS. It updates DTO and entity nullability, backend validation, DB schema, UI form behavior, and adds null-safe checks in web and mobile clients. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Web / Mobile)
participant UI as Web UI Form / Mobile Activity
participant Controller as PathogenTestController
participant Backend as PathogenTestFacadeEjb
participant DB as Database
Note over User,UI: Create / Edit pathogen test
User->>UI: submit form (includes viaLims checkbox, testResultVerified may be null)
UI->>Controller: send PathogenTestDto
Controller->>Backend: create/update request
alt viaLims == true
Backend->>Backend: validate testResultVerified != null
Backend->>Controller: validation error (if null)
else viaLims == false
Backend->>Backend: skip required testResultVerified validation
end
Backend->>DB: persist PathogenTest (nullable testResultVerified column)
DB-->>Backend: persist result
Backend-->>Controller: response
Controller-->>UI: update result
UI-->>User: show success / validation errors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.javasormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTest.javasormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjb.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
💤 Files with no reviewable changes (1)
- sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: SORMAS CI
- GitHub Check: android app test (28)
- GitHub Check: android app test (26)
- GitHub Check: android app test (27)
🔇 Additional comments (4)
sormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjb.java (1)
493-499: LGTM! Conditional validation logic correctly implemented.The validation now appropriately requires
testResultVerifiedonly when the test comes via LIMS (laboratory is directly connected), matching the PR objectives. The comment clearly explains the conditional requirement, and the logic correctly handles the case where manual lab entry doesn't require supervisor verification.sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java (2)
432-432: Good practice: capturing field reference for dynamic validation.Declaring
viaLimsFieldasCheckBoxenables the conditional validation logic added later in the method, which is a clean approach to implementing the viaLims-dependent requirement.
956-963: LGTM! Conditional UI validation correctly synchronized with backend logic.The implementation correctly makes
TEST_RESULT_VERIFIEDrequired only whenviaLimsis true:
- Value change listener (lines 957-960) handles dynamic updates when the checkbox changes
- Initial state setup (lines 962-963) ensures correct requirement state on form load
- Null-safe boolean check prevents NPE
- Backend validation in
PathogenTestFacadeEjb(lines 493-499) enforces the same rulesormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTest.java (1)
369-369: Database migration script already exists for the schema change.The removal of
nullable = falsealigns with the existing database migration insormas-backend/src/main/resources/sql/sormas_schema.sql(lines 15067-15068), which properly drops the NOT NULL constraint from bothpathogentestandpathogentest_historytables.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sormas-app/app/src/main/java/de/symeda/sormas/app/pathogentest/edit/PathogenTestEditActivity.java (1)
157-160: Consider using.equals()for consistency.Line 158 uses
Boolean.TRUE ==while line 103 in the same file usesBoolean.TRUE.equals(). Both are null-safe, but using.equals()consistently is more idiomatic and improves readability.🔎 Suggested consistency improvement
if (sampleOfPathogenTestToSave != null - && Boolean.TRUE == pathogenTestToSave.getTestResultVerified() + && Boolean.TRUE.equals(pathogenTestToSave.getTestResultVerified()) && pathogenTestToSave.getTestedDisease() == associatedCase.getDisease() && pathogenTestToSave.getTestResult() != sampleOfPathogenTestToSave.getPathogenTestResult()) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
sormas-app/app/src/main/java/de/symeda/sormas/app/pathogentest/edit/PathogenTestEditActivity.javasormas-app/app/src/main/java/de/symeda/sormas/app/pathogentest/edit/PathogenTestNewActivity.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestController.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: android app test (28)
- GitHub Check: android app test (27)
- GitHub Check: android app test (26)
- GitHub Check: SORMAS CI
🔇 Additional comments (5)
sormas-app/app/src/main/java/de/symeda/sormas/app/pathogentest/edit/PathogenTestEditActivity.java (1)
102-104: LGTM! Null-safe verification check correctly implemented.The use of
Boolean.TRUE.equals(pathogenTestToSave.getTestResultVerified())correctly handles the now-optional testResultVerified field, ensuring the disease variant update dialog only triggers when the test result is explicitly verified (not null or false).sormas-app/app/src/main/java/de/symeda/sormas/app/pathogentest/edit/PathogenTestNewActivity.java (1)
158-160: LGTM! Null-safe verification check correctly implemented.The use of
Boolean.TRUE.equals(pathogenTestToSave.getTestResultVerified())properly handles the now-optional testResultVerified field, ensuring the disease variant update dialog is only triggered when verification is explicitly true.sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestController.java (3)
373-405: LGTM! Null-safe verification checks correctly implemented for case handling.All test result verification checks in
handleAssociatedCasenow useBoolean.TRUE.equals(t.getTestResultVerified()), properly handling the optional testResultVerified field. This ensures that only explicitly verified positive/negative results trigger case update workflows, disease variant updates, and case cloning dialogs.
442-471: LGTM! Null-safe verification checks correctly implemented for contact handling.All test result verification checks in
handleAssociatedContactnow useBoolean.TRUE.equals(t.getTestResultVerified()), ensuring that only explicitly verified results trigger contact-to-case conversion workflows and sample result updates.
505-535: LGTM! Null-safe verification checks correctly implemented for event participant handling.All test result verification checks in
handleAssociatedEventParticipantnow useBoolean.TRUE.equals(t.getTestResultVerified()), ensuring that only explicitly verified results trigger event participant-to-case conversion workflows and related updates.
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13783 |
1 similar comment
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13783 |
…form non-mandatory
Fixes #13557
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.