-
Notifications
You must be signed in to change notification settings - Fork 160
#13775 - Moved treatment fields from therapy to cases #13795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
- Corrected some hard-coded strings in the notifier UI
📝 WalkthroughWalkthroughThis PR moves treatment-related fields (treatmentStarted, treatmentNotApplicable, treatmentStartDate) from Therapy to Case across API DTOs, backend entities/facades, DB schema/migration, external-message processing, and notifier UI; it updates mappings, SQL migration, and related UI/processing signatures. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
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/caze/notifier/CaseNotifierSideViewController.java (1)
146-158: Dead code: unusedtherapyvariable.The
therapyvariable is fetched on line 148 but is never used after the refactoring removed the therapy parameter fromopenEditWindow(). This should be removed.🧹 Proposed fix
// We only edit the current version NotifierDto notifier = FacadeProvider.getNotifierFacade().getByUuid(caze.getNotifier().getUuid()); - TherapyDto therapy = caze.getTherapy(); openEditWindow(
🤖 Fix all issues with AI agents
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Around line 15069-15084: The migration leaves treatmentnotapplicable NULL for
existing rows because DEFAULT false doesn't backfill and the UPDATE only touches
rows where therapy columns are non‑NULL; update the migration to backfill and
enforce NOT NULL by (a) using COALESCE when setting treatmentnotapplicable from
the therapy row in the UPDATE cases ... FROM therapy t statement (use
COALESCE(t.treatmentnotapplicable, false)), (b) run equivalent backfill for
cases_history (or a second UPDATE setting treatmentnotapplicable = false WHERE
treatmentnotapplicable IS NULL), and then (c) ALTER TABLE cases and ALTER TABLE
cases_history to set treatmentnotapplicable NOT NULL so the column cannot be
tri‑state going forward.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierForm.java`:
- Around line 240-250: populateFields() reads caze (calls
caze.isTreatmentNotApplicable() and caze.getTreatmentStarted()) without guarding
against null, causing an NPE when setValue(NotifierDto) calls populateFields()
before caze is initialized; fix by adding a null-check for caze at the start of
populateFields() (e.g., if (caze == null) return;) or ensure
setValue(NotifierDto) assigns the NotifierDto's caze to the class field before
invoking populateFields(); update references to treatmentGroup,
TreatmentOption.forValue(...) and YesNoUnknown accordingly so they only execute
when caze is non-null.
🧹 Nitpick comments (2)
sormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.java (1)
3446-3448: Normalize treatment flags to avoid contradictory state.Consider clearing
treatmentStarted/treatmentStartDatewhentreatmentNotApplicableis true to keep persisted data consistent.♻️ Suggested adjustment
target.setTreatmentStarted(source.getTreatmentStarted()); target.setTreatmentNotApplicable(source.isTreatmentNotApplicable()); target.setTreatmentStartDate(source.getTreatmentStartDate()); +if (source.isTreatmentNotApplicable()) { + target.setTreatmentStarted(null); + target.setTreatmentStartDate(null); +}sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java (1)
251-294: Method name and Javadoc are misleading after refactoring.The method
updateTherapyBasedOnSelectionnow updates theCaseDataDto, not aTherapyDto. The name and documentation (line 255: "the case data containing the therapy") should be updated to reflect the new behavior.♻️ Suggested rename
/** - * Updates therapy data based on selected treatment option. + * Updates case treatment data based on selected treatment option. * * `@param` caze - * the case data containing the therapy + * the case data to update * `@param` selectedOption * the selected treatment option from the form */ - private void updateTherapyBasedOnSelection(CaseDataDto caze, TreatmentOption selectedOption) { + private void updateCaseTreatmentBasedOnSelection(CaseDataDto caze, TreatmentOption selectedOption) {
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierForm.java
Show resolved
Hide resolved
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13795 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13795 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13795 |
Fixes #13775
Summary by CodeRabbit
Refactor
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.