feat(microbiology): add case workbench and manual AST (OGC-782) - #4134
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abf57b35f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @PostMapping("/runs") | ||
| @PreAuthorize("isAuthenticated()") | ||
| public ResponseEntity<MicroAstRunForm> startRun(@RequestBody MicroAstRunRequestForm request, |
There was a problem hiding this comment.
Require a result-entry role for AST mutations
Replace isAuthenticated() with the appropriate result-entry/supervisor role check for AST writes. As implemented, any authenticated account—including users without RESULTS privileges—can start runs, enter readings, override interpretations, and mark runs reviewed; the new frontend route is likewise configured without a role. Comparable result-entry controllers require hasRole('RESULTS'), and these clinical mutations must be protected by RBAC.
AGENTS.md reference: AGENTS.md:L473-L485
Useful? React with 👍 / 👎.
| MicroAstRun run = runDAO.get(runId).orElseThrow(() -> new IllegalArgumentException("AST run not found")); | ||
| MicroIsolate isolate = isolateDAO.get(run.getIsolateId()) | ||
| .orElseThrow(() -> new IllegalArgumentException("Isolate not found")); | ||
| requireMutableCase(isolate.getCaseId()); | ||
| run.setStatus(MicroAstRunStatus.REVIEWED.name()); |
There was a problem hiding this comment.
Reject reviewing AST runs with no readings
Validate that the run has at least one reading before changing it to REVIEWED. The React button enforces this only client-side, so an authenticated caller can POST directly to /review, review an empty run, and make MicroCaseReadinessServiceImpl report a clinically significant isolate as final-release ready without any AST result. This invariant belongs in the service before persistence.
AGENTS.md reference: AGENTS.md:L340-L349
Useful? React with 👍 / 👎.
| MicroAstRun run = runDAO.get(runId).orElseThrow(() -> new IllegalArgumentException("AST run not found")); | ||
| MicroIsolate isolate = isolateDAO.get(run.getIsolateId()) | ||
| .orElseThrow(() -> new IllegalArgumentException("Isolate not found")); | ||
| requireMutableCase(isolate.getCaseId()); |
There was a problem hiding this comment.
Prevent mutations after an AST run is reviewed
Reject recordReading and overrideReading when the parent run is already REVIEWED. Only the UI disables these actions; direct REST calls can still add a new antibiotic or alter an override after review, while readiness remains true and no second review is required, allowing unreviewed clinical changes to flow toward reporting. The run-state validation should be enforced in the service before writing.
AGENTS.md reference: AGENTS.md:L340-L349
Useful? React with 👍 / 👎.
| List<MicroAstRun> runs = astRunDAO.getByIsolateId(isolateId); | ||
| for (MicroAstRun run : runs) { | ||
| if (MicroAstRunStatus.REVIEWED.name().equals(run.getStatus())) { | ||
| return true; |
There was a problem hiding this comment.
Make an unreviewed repeat run block readiness
Do not return readiness merely because any historical run is reviewed. When an isolate has a reviewed run and a newer repeat/retest run is still IN_PROGRESS, this loop returns true for the old run, so the case is presented as final-release ready even though the current AST work has not been reviewed; readiness must account for the latest or otherwise active run.
Useful? React with 👍 / 👎.
| reading.setOverrideInterpretation(overrideInterpretation.name()); | ||
| reading.setOverrideReason(overrideReason); | ||
| MicroAstReading updated = readingDAO.update(reading); | ||
| recordActivity(isolate.getCaseId(), MicroCaseActivityType.AST_READING_OVERRIDDEN, performedBy, | ||
| "AST interpretation overridden", "{\"readingId\":\"" + readingId + "\"}"); |
There was a problem hiding this comment.
Preserve every AST override in the audit history
Persist an append-only override record, or include the prior/new interpretation and reason in an immutable audit record. A second override currently overwrites the only overrideInterpretation and overrideReason values, while the activity stores only readingId, so the first interpretation change and its reason can no longer be reconstructed for the required clinical audit trail.
AGENTS.md reference: AGENTS.md:L479-L485
Useful? React with 👍 / 👎.
| disabled={ | ||
| busy || | ||
| !!currentRun || | ||
| !selectedIsolateId || | ||
| !selectedPanelId |
There was a problem hiding this comment.
Allow a new AST run after review
Enable starting another run once the current run is no longer in progress. currentRun falls back to the last historical run whenever runs is nonempty, and !!currentRun therefore disables the start button forever after the first run is reviewed, making the specified repeat/retest workflow inaccessible from the workbench.
Useful? React with 👍 / 👎.
| private static final String DEFAULT_BREAKPOINT_AUTHORITY = "CLSI"; | ||
| private static final String DEFAULT_BREAKPOINT_VERSION = "2026"; |
There was a problem hiding this comment.
Resolve the default breakpoint standard from configuration
Remove the hard-coded CLSI/2026 fallback and resolve the deployment-configured default, or require an explicit snapshotted standard. For runs created without breakpointStandardId, installations using EUCAST, another CLSI edition, or a future yearly standard get no rule and silently classify every reading as NO_BREAKPOINT despite having active breakpoint data.
AGENTS.md reference: AGENTS.md:L262-L275
Useful? React with 👍 / 👎.
abf57b3 to
0b69229
Compare
There was a problem hiding this comment.
Pull request overview
Adds the second implementation layer for OGC-782 by introducing the microbiology case workbench UI and the manual AST workflow end-to-end (schema → services → REST → React UI → tests), building on the microbiology foundations delivered in #3789.
Changes:
- Introduce manual AST domain (run + readings), persistence (Liquibase + JPA), service logic (interpretation/override/review), and REST endpoints.
- Add case workbench UI (timeline, isolates, AST entry) plus supporting API client functions and route wiring.
- Add/extend unit, integration, ORM, controller, and Playwright tests to cover the new workflows.
Reviewed changes
Copilot reviewed 67 out of 67 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/persistence/test-persistence.xml | Registers new microbiology AST entities for test persistence. |
| src/test/java/org/openelisglobal/microbiology/service/MicroIsolateServiceTest.java | Expands isolate tests (organismId normalization, final-release guard). |
| src/test/java/org/openelisglobal/microbiology/service/MicroCaseReadinessServiceTest.java | Adds readiness tests for final-release gating based on AST review. |
| src/test/java/org/openelisglobal/microbiology/service/MicroBreakpointServiceTest.java | Adds test for listing active breakpoint standards. |
| src/test/java/org/openelisglobal/microbiology/service/MicroAstServiceTest.java | Adds unit tests for AST run creation and interpretation behavior. |
| src/test/java/org/openelisglobal/microbiology/service/MicroAstInterpretationServiceTest.java | Adds interpretation + override validation tests. |
| src/test/java/org/openelisglobal/microbiology/MicrobiologyOrmValidationTest.java | Extends ORM validation coverage to AST entities. |
| src/test/java/org/openelisglobal/microbiology/MicrobiologyArchitectureTest.java | Extends architecture test to cover all new microbiology controllers. |
| src/test/java/org/openelisglobal/microbiology/MicroAstIntegrationTest.java | Adds integration tests covering AST run/read/override/review flows. |
| src/test/java/org/openelisglobal/microbiology/controller/MicroCaseRestControllerTest.java | Expands controller tests for new activity/order/isolate endpoints + actor behavior. |
| src/test/java/org/openelisglobal/microbiology/controller/MicroCaseLookupRestControllerTest.java | Updates constructor wiring for controller tests. |
| src/test/java/org/openelisglobal/microbiology/controller/MicrobiologyRestExceptionHandlerTest.java | Adds tests for locked-case REST error mapping. |
| src/test/java/org/openelisglobal/AppTestConfig.java | Updates component scan formatting/structure for tests. |
| src/main/resources/persistence/persistence.xml | Registers new AST entities in main persistence unit. |
| src/main/resources/liquibase/3.5.x.x/base.xml | Includes new microbiology AST-related changesets. |
| src/main/resources/liquibase/3.5.x.x/056-microbiology-ast-breakpoint-standard.xml | Adds nullable breakpoint standard FK on AST run. |
| src/main/resources/liquibase/3.5.x.x/053-microbiology-manual-ast.xml | Creates AST run/reading tables + constraints and rollback. |
| src/main/java/org/openelisglobal/microbiology/valueholder/MicroCaseActivityType.java | Adds activity types for AST events. |
| src/main/java/org/openelisglobal/microbiology/valueholder/MicroAstRunStatus.java | Introduces AST run status enum. |
| src/main/java/org/openelisglobal/microbiology/valueholder/MicroAstRun.java | Adds JPA entity for AST runs. |
| src/main/java/org/openelisglobal/microbiology/valueholder/MicroAstReading.java | Adds JPA entity for AST readings + overrides. |
| src/main/java/org/openelisglobal/microbiology/valueholder/MicroAstMethod.java | Adds MIC/ZONE method enum. |
| src/main/java/org/openelisglobal/microbiology/valueholder/MicroAstInterpretation.java | Adds interpretation enum including NO_BREAKPOINT. |
| src/main/java/org/openelisglobal/microbiology/service/MicroIsolateServiceImpl.java | Adds mutation guard + organismId normalization. |
| src/main/java/org/openelisglobal/microbiology/service/MicroCaseReadinessServiceImpl.java | Implements readiness calculation based on isolate AST review status. |
| src/main/java/org/openelisglobal/microbiology/service/MicroCaseReadinessService.java | Adds readiness service contract. |
| src/main/java/org/openelisglobal/microbiology/service/MicroCaseMutationGuard.java | Centralizes final-release immutability rule for microbiology cases. |
| src/main/java/org/openelisglobal/microbiology/service/MicroCaseLockedException.java | Adds domain exception surfaced as 409 for locked cases. |
| src/main/java/org/openelisglobal/microbiology/service/MicroAstServiceImpl.java | Implements AST run/read/override/review behavior + activity recording. |
| src/main/java/org/openelisglobal/microbiology/service/MicroAstService.java | Adds AST service contract. |
| src/main/java/org/openelisglobal/microbiology/service/MicroAstInterpretationServiceImpl.java | Implements MIC/ZONE breakpoint interpretation + override validation. |
| src/main/java/org/openelisglobal/microbiology/service/MicroAstInterpretationService.java | Adds interpretation service contract. |
| src/main/java/org/openelisglobal/microbiology/form/MicroIsolateRequestForm.java | Adds isolate request DTO for REST endpoints. |
| src/main/java/org/openelisglobal/microbiology/form/MicroCaseReadinessForm.java | Adds readiness DTO (blockers + ready flag). |
| src/main/java/org/openelisglobal/microbiology/form/MicroCaseActivityRequestForm.java | Adds case activity mutation request DTO. |
| src/main/java/org/openelisglobal/microbiology/form/MicroAstRunRequestForm.java | Adds AST run request DTO. |
| src/main/java/org/openelisglobal/microbiology/form/MicroAstRunForm.java | Adds AST run response DTO including readings. |
| src/main/java/org/openelisglobal/microbiology/form/MicroAstReadingRequestForm.java | Adds AST reading request DTO. |
| src/main/java/org/openelisglobal/microbiology/form/MicroAstReadingForm.java | Adds AST reading response DTO. |
| src/main/java/org/openelisglobal/microbiology/form/MicroAstOverrideRequestForm.java | Adds AST override request DTO. |
| src/main/java/org/openelisglobal/microbiology/daoimpl/MicroAstRunDAOImpl.java | Adds DAO query for runs by isolate. |
| src/main/java/org/openelisglobal/microbiology/daoimpl/MicroAstReadingDAOImpl.java | Adds DAO query for readings by run. |
| src/main/java/org/openelisglobal/microbiology/dao/MicroAstRunDAO.java | Adds AST run DAO contract. |
| src/main/java/org/openelisglobal/microbiology/dao/MicroAstReadingDAO.java | Adds AST reading DAO contract. |
| src/main/java/org/openelisglobal/microbiology/controller/rest/MicroIsolateRestController.java | Adds isolate create/update endpoints with authenticated actor. |
| src/main/java/org/openelisglobal/microbiology/controller/rest/MicroCaseRestController.java | Adds case activity and order-detail endpoints; uses authenticated actor helper. |
| src/main/java/org/openelisglobal/microbiology/controller/rest/MicroCaseReadinessRestController.java | Adds readiness endpoint for microbiology cases. |
| src/main/java/org/openelisglobal/microbiology/controller/rest/MicrobiologyRestExceptionHandler.java | Maps locked-case domain exception to 409 with structured error body. |
| src/main/java/org/openelisglobal/microbiology/controller/rest/MicrobiologyRestControllerSupport.java | Adds shared authenticated actor lookup + 401 enforcement. |
| src/main/java/org/openelisglobal/microbiology/controller/rest/MicroAstRestController.java | Adds AST endpoints (runs, readings, override, review). |
| specs/782-ogc-782-microbiology-mvp-spec/tasks.md | Marks M4/M5 tasks complete in the spec workflow. |
| specs/782-ogc-782-microbiology-mvp-spec/playwright-plan.md | Adds Playwright flow plan and evidence references. |
| frontend/src/pages/MicrobiologyPage.jsx | Adds microbiology page wrapper around case workbench. |
| frontend/src/languages/en.json | Adds microbiology UI message IDs for workbench + AST. |
| frontend/src/components/microbiology/MicrobiologyService.js | Adds frontend API functions for cases, isolates, readiness, and AST. |
| frontend/src/components/microbiology/MicrobiologyCaseView.jsx | Adds case workbench shell: header + timeline + isolates + AST panels. |
| frontend/src/components/microbiology/IsolatePanel.jsx | Adds isolate create/edit UI and organism selection integration. |
| frontend/src/components/microbiology/CaseTimelinePanel.jsx | Adds activity timeline display + stage advancement UI. |
| frontend/src/components/microbiology/AstEntryPanel.jsx | Adds AST run setup, reading entry, override, review, and readiness display. |
| frontend/src/components/microbiology/tests/MicrobiologyCaseView.test.jsx | Adds component tests for workbench loading and activity/isolate refresh behaviors. |
| frontend/src/components/microbiology/tests/IsolatePanel.test.jsx | Adds tests for isolate creation/edit flow and read-only state. |
| frontend/src/components/microbiology/tests/AstEntryPanel.test.jsx | Adds tests covering AST start/read/override/review UI behavior. |
| frontend/src/App.jsx | Adds route for microbiology case workbench page. |
| frontend/playwright/tests/foundational/core/microbiology-case-workbench.spec.ts | Adds foundational Playwright coverage for setup + isolate creation. |
| frontend/playwright/tests/demo/core/ogc-782-microbiology-mvp.spec.ts | Adds demo Playwright journey for end-to-end microbiology MVP flow. |
| frontend/playwright/helpers/seed-microbiology-data.ts | Adds helper to seed/cleanup microbiology data for Playwright scenarios. |
| frontend/playwright.config.ts | Registers new microbiology tests in the Playwright project allowlists. |
Suppressed comments (1)
src/main/java/org/openelisglobal/microbiology/controller/rest/MicroAstRestController.java:71
MicroAstInterpretation.valueOf(request.overrideInterpretation)will throw (NPE/IllegalArgumentException) for missing/invalid input and currently results in a 500. This should be treated as a client error (400) with a clear message.
public ResponseEntity<MicroAstReadingForm> overrideReading(@PathVariable String readingId,
@RequestBody MicroAstOverrideRequestForm request, HttpServletRequest httpRequest) {
MicroAstReading reading = astService.overrideReading(readingId,
MicroAstInterpretation.valueOf(request.overrideInterpretation), request.overrideReason,
authenticatedUserId(httpRequest));
return ResponseEntity.ok(toReadingForm(reading));
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
9b740e5 to
6f2e90e
Compare
6f2e90e to
c54e9df
Compare
Summary
Second OGC-782 implementation layer: the routine bacteriology case workbench and manual antimicrobial susceptibility testing workflow.
Scope
Validation
core-app.Stack And Documents