From 22164c6c8b6024fcbe7dedf55dfc7c559dde1391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 28 May 2026 10:10:48 +0200 Subject: [PATCH] fix(meds): honest step counter and a single source of one-shot truth in the wizard The progress chip now matches the path the user actually walks (5 steps for one-shot, 6 for recurring after dropping the empty "day detail" step). The cadence picker no longer offers a one-shot radio once the mode picker has settled on recurring. The summary line on the final step interpolates the actual weekdays / day-of-month / yearly date rather than reading a category label. Co-Authored-By: Claude Opus 4.7 (1M context) --- messages/de.json | 4 + messages/en.json | 4 + messages/es.json | 4 + messages/fr.json | 4 + messages/it.json | 4 + messages/pl.json | 4 + .../medications/scheduling/CreationWizard.tsx | 236 ++++++++++++------ .../__tests__/CreationWizard.test.tsx | 98 +++++++- 8 files changed, 278 insertions(+), 80 deletions(-) diff --git a/messages/de.json b/messages/de.json index beb203711..c781e36c6 100644 --- a/messages/de.json +++ b/messages/de.json @@ -841,6 +841,10 @@ "everyNWeeks": "Alle {n} Wochen", "everyNMonths": "Alle {n} Monate" }, + "weekdaysDetail": ", an {days}", + "dayOfMonthDetail": ", am {day}.", + "everyNMonthsDetail": ", alle {months} Monate am {day}.", + "yearlyDetail": ", am {date}.", "times": "um {times}", "startsOn": "Beginn: {date}", "endsOn": "Ende: {date}", diff --git a/messages/en.json b/messages/en.json index 8f569895a..cc0d2ddc7 100644 --- a/messages/en.json +++ b/messages/en.json @@ -841,6 +841,10 @@ "everyNWeeks": "Every {n} weeks", "everyNMonths": "Every {n} months" }, + "weekdaysDetail": ", on {days}", + "dayOfMonthDetail": ", on day {day}.", + "everyNMonthsDetail": ", every {months} months on day {day}.", + "yearlyDetail": ", on {date}.", "times": "at {times}", "startsOn": "Starts: {date}", "endsOn": "Ends: {date}", diff --git a/messages/es.json b/messages/es.json index ff9ac0ed9..dd2584254 100644 --- a/messages/es.json +++ b/messages/es.json @@ -841,6 +841,10 @@ "everyNWeeks": "Every {n} weeks", "everyNMonths": "Every {n} months" }, + "weekdaysDetail": ", on {days}", + "dayOfMonthDetail": ", on day {day}.", + "everyNMonthsDetail": ", every {months} months on day {day}.", + "yearlyDetail": ", on {date}.", "times": "at {times}", "startsOn": "Starts: {date}", "endsOn": "Ends: {date}", diff --git a/messages/fr.json b/messages/fr.json index 2b6a34a5e..c218116a1 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -841,6 +841,10 @@ "everyNWeeks": "Every {n} weeks", "everyNMonths": "Every {n} months" }, + "weekdaysDetail": ", on {days}", + "dayOfMonthDetail": ", on day {day}.", + "everyNMonthsDetail": ", every {months} months on day {day}.", + "yearlyDetail": ", on {date}.", "times": "at {times}", "startsOn": "Starts: {date}", "endsOn": "Ends: {date}", diff --git a/messages/it.json b/messages/it.json index dde792f0f..d9e1c5777 100644 --- a/messages/it.json +++ b/messages/it.json @@ -841,6 +841,10 @@ "everyNWeeks": "Every {n} weeks", "everyNMonths": "Every {n} months" }, + "weekdaysDetail": ", on {days}", + "dayOfMonthDetail": ", on day {day}.", + "everyNMonthsDetail": ", every {months} months on day {day}.", + "yearlyDetail": ", on {date}.", "times": "at {times}", "startsOn": "Starts: {date}", "endsOn": "Ends: {date}", diff --git a/messages/pl.json b/messages/pl.json index a08a186d5..9617ff810 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -841,6 +841,10 @@ "everyNWeeks": "Every {n} weeks", "everyNMonths": "Every {n} months" }, + "weekdaysDetail": ", on {days}", + "dayOfMonthDetail": ", on day {day}.", + "everyNMonthsDetail": ", every {months} months on day {day}.", + "yearlyDetail": ", on {date}.", "times": "at {times}", "startsOn": "Starts: {date}", "endsOn": "Ends: {date}", diff --git a/src/components/medications/scheduling/CreationWizard.tsx b/src/components/medications/scheduling/CreationWizard.tsx index d137b0493..acac32ecb 100644 --- a/src/components/medications/scheduling/CreationWizard.tsx +++ b/src/components/medications/scheduling/CreationWizard.tsx @@ -3,7 +3,7 @@ /** * v1.5.0 — Medication CreationWizard. * - * Seven-step wizard for cold-start medication creation. Composes the + * Multi-step wizard for cold-start medication creation. Composes the * v1.5 picker primitives (`CadencePicker`, `TimesOfDayChips`, * `CourseWindowRow`) into a guided flow so a user models a cadence, * a window of intake, and a single course window without facing the @@ -14,23 +14,29 @@ * 1. Name + dose (text + unit). NL-extraction button surfaces a * placeholder slot — the parent supplies an overlay later. * 2. One-shot vs Recurring (two-card radio). - * 3. Cadence picker — skipped when step 2 = one-shot. - * 4. Day-detail re-cap (sub-controls already render inline in the - * picker; the step renders a confirmation summary). + * 3. Cadence picker — only shown for recurring; the picker's + * conditional sub-controls (weekday chips / day-of-month / + * yearly date) render inline. One-shot path skips this step. * 5. Times of day — re-uses TimesOfDayChips. One-shot path also * shows a date picker for the single dose date. * 6. Course window — CourseWindowRow. One-shot pins endsOn to * startsOn (`lockEndsToStart`). * 7. Reminders toggle + plain-language summary + "Create". * - * Pure helpers `validateStep`, `buildCreateBody`, and - * `summariseCadence` carry the test surface; the interactive flow is - * covered by Playwright in a later commit. + * The recurring path walks 6 displayed steps (1, 2, 3, 5, 6, 7 in raw + * numbering — step 4 was retired as the picker covers day-detail + * inline). The one-shot path walks 5 displayed steps (1, 2, 5, 6, 7). + * The displayed counter compresses to the path the user actually + * walks so the progress chip stays honest. + * + * Pure helpers `validateStep`, `buildCreateBody`, + * `summariseCadence`, and `progressIndices` carry the test surface; + * the interactive flow is covered by Playwright in a later commit. * * i18n keys consumed (namespace `medications.create.wizard.*`): * * .header.stepOf — "Schritt {current} von {total}" - * .header.title.{step1|step2|step3|step4|step5|step6|step7} + * .header.title.{step1|step2|step3|step5|step6|step7} * .step1.name.label — "Name" * .step1.name.placeholder — name input placeholder * .step1.dose.amount.label — "Dose" @@ -43,8 +49,6 @@ * .step2.oneShot.title / .oneShot.description * .step2.recurring.title / .recurring.description * .step3.helper — cadence picker helper text - * .step4.recap — re-cap copy - * .step4.oneShotSkipped — "(skipped — one-shot)" * .step5.helper — times-of-day helper text * .step5.oneShotDate.label — "Date of the single dose" * .step5.oneShotDate.helper — caption under the picker @@ -53,6 +57,10 @@ * .step7.reminders.description — helper text under switch * .step7.summary.title — "Summary" * .step7.summary.cadence.{daily|weekdays|weekly|biweekly|monthly|quarterly|yearly|rolling|oneShot|everyNWeeks|everyNMonths} + * .step7.summary.weekdaysDetail — ", on {days}" + * .step7.summary.dayOfMonthDetail — ", on day {day}." + * .step7.summary.everyNMonthsDetail — ", every {months} months on day {day}." + * .step7.summary.yearlyDetail — ", on {date}." * .step7.summary.times — "at {times}" * .step7.summary.startsOn — "Starts: {date}" * .step7.summary.endsOn — "Ends: {date}" @@ -157,7 +165,25 @@ type DoseUnitKey = (typeof ALL_DOSE_UNIT_KEYS)[number]; const TIME_RE = /^([01]\d|2[0-3]):([0-5]\d)$/; -const TOTAL_STEPS = 7; +/** + * Raw step numbers the wizard renders. Step 4 (day detail) was retired + * — the cadence picker covers it inline. The displayed step index is + * derived from `progressIndices(payload.mode)` so a one-shot user sees + * "Schritt 3 von 5" rather than "Schritt 5 von 7". + */ +const RECURRING_STEPS: readonly number[] = [1, 2, 3, 5, 6, 7]; +const ONE_SHOT_STEPS: readonly number[] = [1, 2, 5, 6, 7]; + +/** + * The ordered list of raw step numbers for the active mode. When the + * mode is unset the wizard hasn't passed step 2 yet, so we conservatively + * surface the recurring list — both branches share steps 1 + 2. + */ +export function progressIndices( + mode: WizardPayload["mode"], +): readonly number[] { + return mode === "oneShot" ? ONE_SHOT_STEPS : RECURRING_STEPS; +} /** Skeleton of the POST /api/medications request body the wizard emits. */ export interface CreateMedicationBody { @@ -210,14 +236,18 @@ function todayUtc(): Date { * The matrix: * - Step 1: name + doseAmount populated. * - Step 2: a mode picked. - * - Step 3: skipped for one-shot. For recurring, the picker always - * emits a valid `CadenceValue`; the gate only re-checks that the - * yearly date is populated when the cadence kind is `yearly`. - * - Step 4: pass-through (re-cap step has no required input). + * - Step 3: only walked for recurring. Picker emits a valid + * `CadenceValue` on every interaction; the gate only re-checks + * that the yearly date is populated when the cadence kind is + * `yearly`. * - Step 5: at least one valid HH:mm time; one-shot requires startsOn. * - Step 6: range valid (endsOn null OR endsOn >= startsOn); * startsOn required for recurring (one-shot already pinned in 5). * - Step 7: gate matches step 6 (the toggle has no failure mode). + * + * Raw step 4 was retired — the cadence picker carries its day-detail + * sub-controls inline. The case stays absent from the switch so a + * stale caller that passes `4` lands on the default-false branch. */ export function validateStep(payload: WizardPayload, step: number): boolean { switch (step) { @@ -241,8 +271,6 @@ export function validateStep(payload: WizardPayload, step: number): boolean { } return true; } - case 4: - return true; case 5: { const times = payload.timesOfDay.filter((t) => TIME_RE.test(t)); if (times.length === 0) return false; @@ -362,6 +390,8 @@ export function summariseCadence( ? (payload.cadence.rollingIntervalDays ?? 0) : 0, }); + const cadenceDetail = cadenceDetailPhrase(payload, t); + const cadenceLine = cadencePhrase + cadenceDetail; const timesPhrase = payload.timesOfDay.length > 0 ? t(k("step7.summary.times"), { @@ -381,11 +411,62 @@ export function summariseCadence( date: dateToIsoString(payload.endsOn), }) : t(k("step7.summary.noEndDate")); - return [cadencePhrase, timesPhrase, startPhrase, endPhrase] + return [cadenceLine, timesPhrase, startPhrase, endPhrase] .filter(Boolean) .join(" · "); } +/** + * Trailing phrase that interpolates the picker's sub-control values + * into the cadence summary so the user sees the actual weekdays / + * day-of-month / yearly date rather than the category label. Returns + * an empty string for cadences whose category label already carries + * the full information (daily / rolling / oneShot). + */ +function cadenceDetailPhrase( + payload: WizardPayload, + t: (key: string, params?: Record) => string, +): string { + if (payload.mode === "oneShot") return ""; + switch (payload.cadence.kind) { + case "weekdays": + return weekdaysDetail(payload.subControls.weekdays, t); + case "everyNWeeks": + return weekdaysDetail(payload.subControls.weekdays, t); + case "monthly": + return t(k("step7.summary.dayOfMonthDetail"), { + day: payload.subControls.dayOfMonth, + }); + case "everyNMonths": + return t(k("step7.summary.everyNMonthsDetail"), { + months: payload.subControls.intervalMonths, + day: payload.subControls.dayOfMonth, + }); + case "yearly": + return payload.subControls.yearlyDate + ? t(k("step7.summary.yearlyDetail"), { + date: payload.subControls.yearlyDate, + }) + : ""; + default: + return ""; + } +} + +function weekdaysDetail( + tokens: readonly string[], + t: (key: string, params?: Record) => string, +): string { + if (tokens.length === 0) return ""; + const names = tokens + .map((tok) => + t(`medications.scheduling.cadence.weekdays.long.${tok.toLowerCase()}`), + ) + .filter(Boolean); + if (names.length === 0) return ""; + return t(k("step7.summary.weekdaysDetail"), { days: names.join(", ") }); +} + function summaryKeyForCadence(payload: WizardPayload): string { if (payload.mode === "oneShot") return "oneShot"; switch (payload.cadence.kind) { @@ -450,25 +531,38 @@ export function CreationWizard({ [payload, step], ); + const stepList = useMemo( + () => progressIndices(payload.mode), + [payload.mode], + ); + const totalSteps = stepList.length; + const currentIndex = stepList.indexOf(step); + // currentIndex can be -1 in the transient case where the user is on + // raw step 3 and toggles back to one-shot at step 2. Defensive + // re-anchoring keeps the indicator monotone. + const safeIndex = currentIndex >= 0 ? currentIndex : 0; + const displayStep = safeIndex + 1; + const isLastStep = displayStep === totalSteps; + const goNext = useCallback(() => { if (!canContinue) return; - // Skip cadence + day-detail when one-shot. - if (step === 2 && payload.mode === "oneShot") { - setStep(5); - return; - } - setStep((s) => Math.min(s + 1, TOTAL_STEPS)); - }, [canContinue, payload.mode, step]); + setStep((s) => { + const list = progressIndices(payload.mode); + const idx = list.indexOf(s); + if (idx < 0) return list[0]; + const nextIdx = Math.min(idx + 1, list.length - 1); + return list[nextIdx]; + }); + }, [canContinue, payload.mode]); const goBack = useCallback(() => { - // Mirror the skip: jumping back from step 5 lands on step 2 in - // one-shot mode (steps 3 + 4 never rendered). - if (step === 5 && payload.mode === "oneShot") { - setStep(2); - return; - } - setStep((s) => Math.max(s - 1, 1)); - }, [payload.mode, step]); + setStep((s) => { + const list = progressIndices(payload.mode); + const idx = list.indexOf(s); + if (idx <= 0) return list[0]; + return list[idx - 1]; + }); + }, [payload.mode]); const onSubmit = useCallback(async () => { setSubmitting(true); @@ -517,12 +611,18 @@ export function CreationWizard({ const stepTitle = t(`medications.create.wizard.header.title.step${step}`); const stepOf = t(k("header.stepOf"), { - current: step, - total: TOTAL_STEPS, + current: displayStep, + total: totalSteps, }); return ( - +

{stepOf}

{stepTitle} @@ -543,9 +643,6 @@ export function CreationWizard({ {step === 3 && ( )} - {step === 4 && ( - - )} {step === 5 && ( {t(k("nav.back"))} - {step < TOTAL_STEPS ? ( + {!isLastStep ? ( @@ -595,6 +694,7 @@ export function CreationWizard({ disabled={!canContinue || submitting} data-slot="wizard-create" aria-busy={submitting || undefined} + className="h-11" > {submitting && ( @@ -786,6 +886,23 @@ function ModeCard({ value, selected, title, description, onSelect }: ModeCardPro ); } +/** + * The cadence kinds offered by the wizard's Step 3. One-shot is the + * canonical responsibility of Step 2's mode picker — exposing it here + * would invite a contradictory `mode = "recurring"` + `cadence.kind = + * "oneShot"` payload. The picker accepts an optional `allowedKinds` + * filter that we always pass with the recurring-only list. + */ +const STEP3_ALLOWED_KINDS = [ + "daily", + "weekdays", + "everyNWeeks", + "monthly", + "everyNMonths", + "yearly", + "rolling", +] as const; + function Step3Cadence({ payload, applyPartial, t }: StepProps) { return (
@@ -795,6 +912,7 @@ function Step3Cadence({ payload, applyPartial, t }: StepProps) { applyPartial({ cadence, subControls }) } @@ -803,42 +921,6 @@ function Step3Cadence({ payload, applyPartial, t }: StepProps) { ); } -function Step4DayDetail({ payload, t }: Pick) { - // Day-detail follow-ups already render inline in the picker (the - // sub-controls are conditional on the selected kind). Step 4 is a - // confirmation re-cap that surfaces what got picked so the user can - // tap Back if it looks wrong. - if (payload.mode === "oneShot") { - return ( -
-

- {t(k("step4.oneShotSkipped"))} -

-
- ); - } - return ( -
-

- {t(k("step4.recap"))} -

-
- {recapCadence(payload, t)} -
-
- ); -} - -function recapCadence( - payload: WizardPayload, - t: (key: string, params?: Record) => string, -): string { - return summariseCadence(payload, t); -} - function Step5Times({ payload, applyPartial, t }: StepProps) { const isOneShot = payload.mode === "oneShot"; const startsIso = dateToIsoString(payload.startsOn); diff --git a/src/components/medications/scheduling/__tests__/CreationWizard.test.tsx b/src/components/medications/scheduling/__tests__/CreationWizard.test.tsx index 3ab630e3d..a56d4c5c8 100644 --- a/src/components/medications/scheduling/__tests__/CreationWizard.test.tsx +++ b/src/components/medications/scheduling/__tests__/CreationWizard.test.tsx @@ -4,6 +4,7 @@ import { buildCreateBody, type CreateMedicationBody, emptyWizardPayload, + progressIndices, summariseCadence, validateStep, type WizardPayload, @@ -100,9 +101,9 @@ describe("validateStep", () => { ).toBe(true); }); - it("step 4: always passes (re-cap step)", () => { - expect(validateStep(withCadence("daily"), 4)).toBe(true); - expect(validateStep(withCadence("oneShot"), 4)).toBe(true); + it("step 4: retired — the picker covers day-detail inline, so the gate returns false", () => { + expect(validateStep(withCadence("daily"), 4)).toBe(false); + expect(validateStep(withCadence("oneShot"), 4)).toBe(false); }); it("step 5: requires at least one valid HH:mm time", () => { @@ -467,4 +468,95 @@ describe("summariseCadence", () => { const out = summariseCadence(p, t); expect(out).toMatch(/step7\.summary\.times[^|]*"08:00, 20:00"/); }); + + it("appends a weekdays-detail phrase with the picked long names", () => { + const p = withCadence("weekdays", { weekdays: ["MO", "WE", "FR"] }); + const out = summariseCadence(p, t); + expect(out).toContain( + "medications.create.wizard.step7.summary.weekdaysDetail", + ); + // Each token resolves to its long-name key under the cadence + // namespace; the stub-t echoes the keys joined by ", ". + expect(out).toMatch(/weekdays\.long\.mo[^|]*weekdays\.long\.we[^|]*weekdays\.long\.fr/); + }); + + it("appends a weekdays-detail phrase for everyNWeeks (interval + picked days)", () => { + const p = withCadence("everyNWeeks", { + intervalWeeks: 3, + weekdays: ["TU"], + }); + const out = summariseCadence(p, t); + expect(out).toContain( + "medications.create.wizard.step7.summary.weekdaysDetail", + ); + expect(out).toMatch(/weekdays\.long\.tu/); + }); + + it("appends a day-of-month-detail phrase for monthly cadence", () => { + const p = withCadence("monthly", { dayOfMonth: 15 }); + const out = summariseCadence(p, t); + expect(out).toContain( + "medications.create.wizard.step7.summary.dayOfMonthDetail", + ); + expect(out).toMatch(/dayOfMonthDetail[^|]*"day":15/); + }); + + it("appends an everyNMonths-detail phrase with months + day", () => { + const p = withCadence("everyNMonths", { + intervalMonths: 3, + dayOfMonth: 10, + }); + const out = summariseCadence(p, t); + expect(out).toContain( + "medications.create.wizard.step7.summary.everyNMonthsDetail", + ); + expect(out).toMatch(/everyNMonthsDetail[^|]*"months":3[^|]*"day":10/); + }); + + it("appends a yearly-detail phrase with the picked date when set", () => { + const p = withCadence("yearly", { yearlyDate: "2026-10-15" }); + const out = summariseCadence(p, t); + expect(out).toContain( + "medications.create.wizard.step7.summary.yearlyDetail", + ); + expect(out).toMatch(/yearlyDetail[^|]*"date":"2026-10-15"/); + }); + + it("omits the yearly-detail phrase when no date has been picked yet", () => { + const p = withCadence("yearly", { yearlyDate: "" }); + const out = summariseCadence(p, t); + expect(out).not.toContain("yearlyDetail"); + }); + + it("emits no trailing detail for daily / rolling / oneShot cadences", () => { + const trailers = [ + "weekdaysDetail", + "dayOfMonthDetail", + "everyNMonthsDetail", + "yearlyDetail", + ]; + for (const kind of ["daily", "rolling", "oneShot"] as const) { + const out = summariseCadence(withCadence(kind), t); + for (const trailer of trailers) { + expect(out, `${kind} surfaced ${trailer}`).not.toContain(trailer); + } + } + }); +}); + +describe("progressIndices", () => { + it("returns the recurring 6-step path when the mode is recurring", () => { + expect(progressIndices("recurring")).toEqual([1, 2, 3, 5, 6, 7]); + }); + + it("returns the one-shot 5-step path when the mode is oneShot", () => { + expect(progressIndices("oneShot")).toEqual([1, 2, 5, 6, 7]); + }); + + it("falls back to the recurring path while the mode is still unset", () => { + // Step 1 / step 2 both render under either path; the recurring + // list keeps the displayed counter monotone before the user picks + // a mode. + expect(progressIndices(null)).toEqual([1, 2, 3, 5, 6, 7]); + }); });