Adds Lab results (tests/samples/AST) tab to the cases view#13987
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a "Laboratory results" tab to the Case view. Three new fields ( ChangesCase Laboratory Results Tab
Sequence Diagram(s)sequenceDiagram
participant User
participant CaseLabResultsView
participant CaseController
participant CaseFacadeEjb
participant SampleFacade
participant PathogenTestFacade
User->>CaseLabResultsView: open Lab Results tab
CaseLabResultsView->>CaseController: getLabResultsEditComponent(caseUuid)
CaseController->>CaseFacadeEjb: getCaseDataByUuid(caseUuid)
CaseFacadeEjb-->>CaseController: CaseDataDto
CaseController-->>CaseLabResultsView: CommitDiscardWrapperComponent<CaseLabResultsForm>
CaseLabResultsView->>CaseLabResultsView: initView / refreshTables
CaseLabResultsView->>SampleFacade: getIndexList(caseCriteria)
SampleFacade-->>CaseLabResultsView: List<SampleIndexDto>
CaseLabResultsView->>PathogenTestFacade: getAllByCaseUuid(caseUuid)
PathogenTestFacade-->>CaseLabResultsView: List<PathogenTestDto>
CaseLabResultsView->>CaseLabResultsView: buildTestsGrid / buildSamplesGrid
CaseLabResultsView->>CaseLabResultsView: flattenDrugSusceptibility → List<AstRow>
CaseLabResultsView->>CaseLabResultsView: buildAstGrid (conditional)
User->>CaseLabResultsView: commit header form
CaseLabResultsView->>CaseFacadeEjb: getCaseDataByUuid (re-fetch)
CaseLabResultsView->>CaseFacadeEjb: saveCase(dto with dateOther/dateOtherDetails/externalComments)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.43.0)sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseFacadeEjbTest.javaThanks 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/resources/sql/sormas_schema.sql`:
- Around line 16298-16307: The migration metadata contains inconsistent issue
reference numbers that do not match the PR objective. Update the issue reference
number from `#13948` to `#13955` in two locations: the migration comment line
starting with "-- 2026-06-08 Laboratory results tab: date other + external
comments on case" and the INSERT INTO schema_version statement with
version_number 638. This ensures the release traceability accurately reflects
the correct issue identifier throughout the migration.
In
`@sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseFacadeEjbTest.java`:
- Around line 3458-3466: The assertEquals assertion for dateOther in the
round-trip test is brittle because exact Date equality does not account for
database/JPA precision loss. Replace the exact equality check for the dateOther
field (at the assertEquals call comparing dateOther with
reloaded.getDateOther()) with a day-granularity comparison. Either truncate both
dates to remove time components before assertion, or use a date-granularity
assertion helper method that compares only the day portion of the dates.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseLabResultsView.java`:
- Around line 169-189: The Tests grid and AST grid in CaseLabResultsView are
missing the mandatory Comments column that is required according to the tab
specification. Add a grid.addColumn() call for the Comments field to both
affected grids at the locations mentioned (the current diff showing lines
169-189 for the Tests grid, and the sibling location at lines 250-257 for the
AST grid). Use the same pattern as the other columns being added to the grid,
retrieving the comments property from PathogenTestDto and setting an appropriate
caption using I18nProperties, so both grids match the Samples grid's
completeness.
- Around line 192-196: The grid item click listeners throughout the
CaseLabResultsView file currently require double-click to open detail views, but
the requirement specifies that selecting a row should open the corresponding
detail view (single-click behavior). Remove the isDoubleClick() condition from
the if statement in all affected grid.addItemClickListener calls so that a
single row selection triggers the edit action. This change needs to be applied
at all locations mentioned: the pathogen test controller edit call at lines
192-196, and the corresponding listener implementations for other grid types at
lines 229-233, 245-260, and 359-375. Ensure each grid's item click listener
directly calls its respective controller's edit method (such as
ControllerProvider.getPathogenTestController().edit,
ControllerProvider.getTestResultController().edit, or equivalent) without
requiring a double-click condition.
- Around line 279-306: Empty rows are being added to the results when all the
displayed data fields are null. After reading all the properties with
readProperty calls (method, mic, zoneDiameter, clinical, and surveillance), add
a check before the rows.add() call to verify that at least one of these values
is not null. If all of them are null, skip adding that row by using continue to
move to the next iteration of the loop.
- Around line 128-133: The testCountBySampleUuid calculation in the stream chain
is only counting tests from displayTests, which excludes AST results that are
displayed in a separate table below. To fix the underreporting of performed
tests, combine the test counts from both the regular tests (displayTests) and
the AST tests when calculating the grouped count by sample UUID. Identify the
collection that holds the AST tests (likely named similarly to displayTests but
for AST data) and include those tests in the groupBy operation to ensure the
performed-test count reflects all tests visible in the UI.
🪄 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: a1a8e814-57e4-48bb-9b11-085bcc55e6ab
📒 Files selected for processing (13)
sormas-api/src/main/java/de/symeda/sormas/api/caze/CaseDataDto.javasormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.javasormas-api/src/main/java/de/symeda/sormas/api/i18n/Strings.javasormas-api/src/main/resources/captions.propertiessormas-api/src/main/resources/strings.propertiessormas-backend/src/main/java/de/symeda/sormas/backend/caze/Case.javasormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseFacadeEjbTest.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/AbstractCaseView.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseLabResultsForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseLabResultsView.java
Fixes #13955
Summary by CodeRabbit