[ENG-601] feat: allow planned encounters (w/ future start date) to transition to cancelled/discontinued/entered in error#16476
[ENG-601] feat: allow planned encounters (w/ future start date) to transition to cancelled/discontinued/entered in error#16476Jacobjeevan wants to merge 6 commits into
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
ChangesEncounter period.end synchronization
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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. 🔧 OpenGrep (1.23.0)src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.13][ERROR]: unable to find a config; path Comment |
Greptile SummaryThis PR adds special-case handling in the encounter status
Confidence Score: 3/5The change partially achieves its goal but leaves DISCONTINUED broken for future-start encounters. Discontinuing a planned encounter still causes the effect to stamp an end date before the future start, which the backend will reject. The fix is moving DISCONTINUED into the correct branch. src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx — status grouping in the useEffect at line 141. Important Files Changed
Reviews (6): Last reviewed commit: "chore: copilot suggestion" | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Adjusts encounter period handling so that planned encounters (often with future period.start) can be transitioned into terminal “non-completion” statuses (cancelled/discontinued/entered-in-error) without auto-setting an end time that would precede the start time.
Changes:
- Auto-populates
period.endonly fordischarged/completedencounters (when missing). - Special-cases
cancelled/discontinued/entered_in_errorsoperiod.endis not set for planned encounters (and is cleared if present).
Comments suppressed due to low confidence (1)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx:171
- The planned-encounter exception is keyed off
encounterData?.status, which is the server-loaded status. If the user sets the status toplannedin this form and then switches tocancelled/discontinued/entered_in_error, this block will treat it as non-planned and will setperiod.endtonow, recreating the invalidend < startscenario the PR is trying to avoid. Also,encounterDataisn’t in the effect deps, so relying on it here is brittle.
Consider keying this behavior off the encounter timing (future start) instead of the initial server status.
if (encounterData?.status === EncounterStatus.PLANNED) {
if (encounter.period.end) {
handleUpdateEncounter({
period: {
...encounter.period,
end: undefined,
},
});
}
} else if (!encounter.period.end) {
handleUpdateEncounter({
period: {
...encounter.period,
end: new Date().toISOString(),
},
});
}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx`:
- Around line 155-165: The transition logic in EncounterQuestion.tsx is using
the persisted encounterData.status snapshot instead of the previous local
status, which can cause period.end to be cleared and then re-added when
reopening the same record. Update the status-change detection in the
EncounterQuestion component to compare against a previous status stored with
useRef, and only clear period.end for true PLANNED-to-terminal-status
transitions in the handleUpdateEncounter flow.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cf709796-2cf8-43b0-892e-003b9c5633d1
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #9782 |
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)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx (1)
137-173: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEffect misses
period.startin dependencies, so the future-start rule can be skipped.Line 173 only depends on
encounter.status, but this effect’s decision also usesencounter.period.start(andencounter.period.end). If status is changed first and start is later moved to a future date,period.endwon’t be re-synced.Suggested fix
- }, [encounter.status]); + }, [encounter.status, encounter.period.start, encounter.period.end]);🤖 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 `@src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx` around lines 137 - 173, The useEffect in EncounterQuestion is only keyed on encounter.status, but it also reads encounter.period.start and encounter.period.end, so the future-start/end synchronization can become stale when those fields change. Update the dependency list for this effect to include the period fields it relies on, especially encounter.period.start and encounter.period.end, so the logic that calls handleUpdateEncounter always re-runs when the encounter timing changes.
🤖 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 `@src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx`:
- Around line 137-173: The useEffect in EncounterQuestion is only keyed on
encounter.status, but it also reads encounter.period.start and
encounter.period.end, so the future-start/end synchronization can become stale
when those fields change. Update the dependency list for this effect to include
the period fields it relies on, especially encounter.period.start and
encounter.period.end, so the logic that calls handleUpdateEncounter always
re-runs when the encounter timing changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: df74125a-9428-46e1-97cd-ebece23de6a7
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx
Deploying care-preview with
|
| Latest commit: |
2313ed5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c9bcedb8.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-601-planned.care-preview-a7w.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx`:
- Around line 141-157: The EncounterQuestion status handling currently treats
EncounterStatus.DISCONTINUED the same as discharged/completed in the branch that
always sets period.end, which bypasses the new future-start guard. Update the
conditional logic in EncounterQuestion so DISCONTINUED follows the same
restriction path as the other future-start planned encounter transitions, and
only set period.end in the branch that is meant for statuses that are allowed to
auto-close; use the EncounterStatus checks and handleUpdateEncounter flow in
this component to keep the behavior consistent.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: be00cedc-a06c-4b61-8ee9-fa87893d44ac
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx (1)
141-157: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
DISCONTINUEDstill skips the future-start rule (Line 143).
EncounterStatus.DISCONTINUEDis in the always-set-end branch, so future-start planned encounters transitioned to discontinued still getperiod.end, which contradicts the PR behavior for discontinued/cancelled/entered-in-error.Suggested fix
if ( encounter.status === EncounterStatus.DISCHARGED || - encounter.status === EncounterStatus.DISCONTINUED || encounter.status === EncounterStatus.COMPLETED ) { // Always set end date — if start is in the future, BE will reject it if (!encounter.period.end) { handleUpdateEncounter({ @@ } else if ( + encounter.status === EncounterStatus.DISCONTINUED || encounter.status === EncounterStatus.CANCELLED || encounter.status === EncounterStatus.ENTERED_IN_ERROR ) {🤖 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 `@src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx` around lines 141 - 157, `EncounterQuestion` is treating `EncounterStatus.DISCONTINUED` like discharged/completed and always setting `period.end`, which bypasses the future-start handling expected for discontinued encounters. Update the status branching in the encounter update logic so the `DISCONTINUED` path follows the same rule as `CANCELLED` and `ENTERED_IN_ERROR`, and only auto-populates `period.end` for the statuses that should always receive an end date. Keep the fix in the conditional block that checks `encounter.status` and uses `handleUpdateEncounter`.
🤖 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.
Duplicate comments:
In `@src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx`:
- Around line 141-157: `EncounterQuestion` is treating
`EncounterStatus.DISCONTINUED` like discharged/completed and always setting
`period.end`, which bypasses the future-start handling expected for discontinued
encounters. Update the status branching in the encounter update logic so the
`DISCONTINUED` path follows the same rule as `CANCELLED` and `ENTERED_IN_ERROR`,
and only auto-populates `period.end` for the statuses that should always receive
an end date. Keep the fix in the conditional block that checks
`encounter.status` and uses `handleUpdateEncounter`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ff0e6ecf-ba8c-403e-a9fa-98781f2acb21
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx


Proposed Changes
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary of changes