Filter cases and the sample dashboard by pathogen test result, serogr… - #13990
Merged
Merged
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughAdds pathogen test result, serogroup, and disease variant as filterable criteria in the case directory and sample dashboard. API criteria classes gain new fields and accessors, backend services extend JPA queries with EXISTS subqueries, Vaadin UI controls wire new filters, and integration tests verify filtering semantics. Additionally refactors sample generation to maintain disease consistency and improves pathogen test list rendering with conditional metadata icons. ChangesLab Filter Criteria: Test Result, Serogroup, Disease Variant
Sample Generation Consistency and Test Display Improvements
Sequence Diagram(s)sequenceDiagram
participant User
participant CaseFilterForm
participant CaseFacade
participant CaseService
rect rgba(173, 216, 230, 0.5)
Note over User, CaseFilterForm: Case Directory Filtering
User->>CaseFilterForm: selects pathogenTestResult or enters serogroup
CaseFilterForm->>CaseFacade: getCases(CaseCriteria)
CaseFacade->>CaseService: createCriteriaFilter(caseCriteria)
CaseService->>CaseService: EXISTS subquery on Sample.PATHOGEN_TEST_RESULT
CaseService->>CaseService: EXISTS subquery on PathogenTest.SEROTYPE_TEXT (unaccented ILIKE)
CaseService-->>CaseFacade: filtered cases
CaseFacade-->>CaseFilterForm: matching cases displayed
end
rect rgba(144, 238, 144, 0.5)
Note over User, CaseFilterForm: Sample Dashboard Filtering
User->>CaseFilterForm: selects disease, then pathogenTestResult / serogroup / diseaseVariant
CaseFilterForm->>CaseFilterForm: disease change repopulates variant options
CaseFilterForm->>CaseFilterForm: environment-only material disables lab filters
CaseFilterForm->>CaseFacade: getSampleCountsByResultType(SampleDashboardCriteria)
CaseFacade->>CaseService: createSampleFilter with combined predicates
CaseService->>CaseService: equality on PATHOGEN_TEST_RESULT + EXISTS subqueries on SEROTYPE_TEXT and diseaseVariant
CaseService-->>CaseFacade: filtered counts
CaseFacade-->>CaseFilterForm: dashboard results updated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardFilterLayout.java (1)
115-133:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep disease-variant filter disabled in environment-only mode.
Line 132 always repopulates the disease-variant combo, which can re-enable it after
setLabFiltersEnabled(false)has disabled lab filters for environment-only sample material. That breaks the intended state gating.Suggested fix
diseaseFilter.addValueChangeListener(e -> { Object filterValue = diseaseFilter.getValue(); Disease selectedDisease = null; if (filterValue instanceof Disease) { selectedDisease = (Disease) filterValue; dashboardDataProvider.setDisease(selectedDisease); dashboardDataProvider.setWithNoDisease(null); } else if (filterValue == SampleDashboardCustomDiseaseFilter.NO_DISEASE) { dashboardDataProvider.setDisease(null); dashboardDataProvider.setWithNoDisease(true); } else if (filterValue == null) { dashboardDataProvider.setDisease(null); dashboardDataProvider.setWithNoDisease(null); } else { throw new RuntimeException("Disease filter [" + filterValue + "] not handled!"); } - // The disease variant is only meaningful for a selected disease; repopulate (and clear) it. - repopulateDiseaseVariantFilter(selectedDisease); + boolean environmentOnly = + dashboardDataProvider.getSampleMaterial() == null + && dashboardDataProvider.getEnvironmentSampleMaterial() != null; + if (environmentOnly) { + setLabFiltersEnabled(false); + } else { + // The disease variant is only meaningful for a selected disease; repopulate (and clear) it. + repopulateDiseaseVariantFilter(selectedDisease); + } });🤖 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-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardFilterLayout.java` around lines 115 - 133, The call to repopulateDiseaseVariantFilter(selectedDisease) at the end of the diseaseFilter value change listener unconditionally repopulates the disease-variant combo box, which can re-enable it after setLabFiltersEnabled(false) has disabled lab filters for environment-only sample material. Modify the diseaseFilter value change listener to make the repopulateDiseaseVariantFilter call conditional by checking whether lab filters are currently enabled before invoking it, ensuring the disease-variant filter remains disabled when operating in environment-only mode.
🧹 Nitpick comments (2)
sormas-backend/src/test/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardFacadeEjbTest.java (1)
391-434: ⚡ Quick winAlso validate lab filters through
getTestResultCountsByResultType.This method only verifies
getSampleCountsByResultType, but this PR also changes lab-filter behavior ingetTestResultCountsByResultType. Add one assertion path there to lock in both contracts.🤖 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/backend/dashboard/sample/SampleDashboardFacadeEjbTest.java` around lines 391 - 434, The test currently only validates the getSampleCountsByResultType method but does not verify the getTestResultCountsByResultType method, which is also modified by this PR's lab-filter behavior changes. Add assertion statements that call getSampleDashboardFacade().getTestResultCountsByResultType() with the same SampleDashboardCriteria filters (pathogenTestResult, serogroup, and diseaseVariant) to ensure the lab-filter behavior is consistent and correctly implemented in both methods.sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseFacadeEjbTest.java (1)
3472-3566: ⚡ Quick winAdd a deletion-state assertion for the new lab filters.
This test covers matching semantics well, but it doesn’t verify the new “exclude logically deleted sample/pathogen test” behavior. Adding one deleted-row assertion here would prevent regressions in the new EXISTS predicates.
Suggested addition
+ // Deleted sample/pathogen-test rows must not match filters. + SampleDto deletedSample = creator.createSample( + caseA.toReference(), + surveillanceSupervisor.toReference(), + rdcf.facility, + s -> s.setPathogenTestResult(PathogenTestResultType.POSITIVE)); + PathogenTestDto deletedTest = creator.createPathogenTest(deletedSample.toReference(), surveillanceSupervisor.toReference(), t -> { + t.setTestedDisease(Disease.EVD); + t.setLab(rdcf.facility); + t.setSerotypeText("Deleted Serogroup"); + }); + getPathogenTestFacade().delete(deletedTest.getUuid(), new DeletionDetails(DeletionReason.OTHER_REASON, "test")); + getSampleFacade().delete(deletedSample.getUuid(), new DeletionDetails(DeletionReason.OTHER_REASON, "test")); + + assertEquals( + 0, + getCaseFacade() + .getIndexList(new CaseCriteria().pathogenTestResult(PathogenTestResultType.POSITIVE).serogroup("deleted"), 0, 100, null) + .size());🤖 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/backend/caze/CaseFacadeEjbTest.java` around lines 3472 - 3566, The test method testFilterCasesByTestResultAndSerogroup verifies the matching semantics of the test result and serogroup filters but does not validate that logically deleted samples and pathogen tests are properly excluded from the filter results. Add a test case (similar to caseA, caseB, and caseC) that creates a case with a sample or pathogen test that is marked as deleted, then apply the pathogenTestResult and serogroup filters using getCaseFacade().getIndexList() with the new CaseCriteria, and assert that the deleted row is not included in the results to prevent regressions in the new EXISTS predicates.
🤖 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.
Outside diff comments:
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardFilterLayout.java`:
- Around line 115-133: The call to
repopulateDiseaseVariantFilter(selectedDisease) at the end of the diseaseFilter
value change listener unconditionally repopulates the disease-variant combo box,
which can re-enable it after setLabFiltersEnabled(false) has disabled lab
filters for environment-only sample material. Modify the diseaseFilter value
change listener to make the repopulateDiseaseVariantFilter call conditional by
checking whether lab filters are currently enabled before invoking it, ensuring
the disease-variant filter remains disabled when operating in environment-only
mode.
---
Nitpick comments:
In
`@sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseFacadeEjbTest.java`:
- Around line 3472-3566: The test method testFilterCasesByTestResultAndSerogroup
verifies the matching semantics of the test result and serogroup filters but
does not validate that logically deleted samples and pathogen tests are properly
excluded from the filter results. Add a test case (similar to caseA, caseB, and
caseC) that creates a case with a sample or pathogen test that is marked as
deleted, then apply the pathogenTestResult and serogroup filters using
getCaseFacade().getIndexList() with the new CaseCriteria, and assert that the
deleted row is not included in the results to prevent regressions in the new
EXISTS predicates.
In
`@sormas-backend/src/test/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardFacadeEjbTest.java`:
- Around line 391-434: The test currently only validates the
getSampleCountsByResultType method but does not verify the
getTestResultCountsByResultType method, which is also modified by this PR's
lab-filter behavior changes. Add assertion statements that call
getSampleDashboardFacade().getTestResultCountsByResultType() with the same
SampleDashboardCriteria filters (pathogenTestResult, serogroup, and
diseaseVariant) to ensure the lab-filter behavior is consistent and correctly
implemented in both methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d22c166d-9e95-48ea-99d9-df11e4d998f9
📒 Files selected for processing (12)
sormas-api/src/main/java/de/symeda/sormas/api/caze/CaseCriteria.javasormas-api/src/main/java/de/symeda/sormas/api/dashboard/SampleDashboardCriteria.javasormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.javasormas-api/src/main/resources/captions.propertiessormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseService.javasormas-backend/src/main/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardService.javasormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTest.javasormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseFacadeEjbTest.javasormas-backend/src/test/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardFacadeEjbTest.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseFilterForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardDataProvider.javasormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardFilterLayout.java
KarnaiahPesula
approved these changes
Jun 18, 2026
This was referenced Jun 29, 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.
…oup and variant
Fixes #13957
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests