From 06592df87c6f75b28cf14cd9eefdf2e9eca68c2a Mon Sep 17 00:00:00 2001 From: Jacob Hagstedt Date: Mon, 21 Sep 2026 09:49:22 +0200 Subject: [PATCH] Say something when planning a free day changes nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Planning a day through the default "Planera en dag" dialog on a weekend or red day accepted the save, returned 200, and did nothing visible: the allocator marks a free day as 'free' before a type is chosen, the shortfall count excludes it, and planning_service strips it before persisting — so the sync came back accepted with an empty batch and the toast builder had nothing to say. The click looked broken. The modal already had the right words for this; they were only wired to the advanced/pinned path. They now show in the default path too, so the user is told before clicking. And an accepted sync that changed nothing now says so instead of staying silent — opt-in at the one call site that needs it, since the preset rail deliberately stays quiet on a genuine no-op. No change to the allocator, to planning_service, or to the policy that free days stay off. Co-Authored-By: Claude Opus 5 --- .../modals/PlannedDayModal.test.tsx | 45 ++++++++++++++++++- .../src/components/modals/PlannedDayModal.tsx | 33 +++++++++++--- frontend/src/i18n/en/calendar.ts | 5 +++ frontend/src/i18n/sv/calendar.ts | 5 +++ frontend/src/pages/Calendar.test.tsx | 25 +++++++++++ frontend/src/pages/Calendar.tsx | 43 ++++++++++++++---- 6 files changed, 139 insertions(+), 17 deletions(-) diff --git a/frontend/src/components/modals/PlannedDayModal.test.tsx b/frontend/src/components/modals/PlannedDayModal.test.tsx index 5a7886b..88a687b 100644 --- a/frontend/src/components/modals/PlannedDayModal.test.tsx +++ b/frontend/src/components/modals/PlannedDayModal.test.tsx @@ -768,11 +768,52 @@ describe('PlannedDayModal', () => { expect(screen.queryByTestId('pin-exclusion-notice')).not.toBeInTheDocument() }) - it('shows neither notice while the type select is still on "let the planner choose"', () => { + it('still warns free days will stay off while on "let the planner choose" (solo) — the pin-only notice does not apply', () => { + // §silent-no-op fix — a free day is marked 'free' by the allocator's + // `is_free_day` branch (allocation_service.py) before any type is even + // considered, so 'auto' stays off here exactly like a concrete pin + // does. Only the pin-exclusion notice (below) is pin-specific. render() - expect(screen.queryByTestId('free-days-stay-off-notice')).not.toBeInTheDocument() + expect(screen.getByTestId('free-days-stay-off-notice')).toBeInTheDocument() expect(screen.queryByTestId('pin-exclusion-notice')).not.toBeInTheDocument() }) + + it('does not warn on "let the planner choose" once takePlOnRedDays is on', () => { + render( + , + ) + expect(screen.queryByTestId('free-days-stay-off-notice')).not.toBeInTheDocument() + }) + + it('warns in the default (non-advanced) couple path too — a "Planera en dag" save on a free day used to accept silently with nothing changed', () => { + render() + expect(screen.getByTestId('free-days-stay-off-notice')).toBeInTheDocument() + // The advanced-only pin-exclusion notice must not leak into this path. + expect(screen.queryByTestId('pin-exclusion-notice')).not.toBeInTheDocument() + }) + + it('does not warn in the default couple path once takePlOnRedDays is on', () => { + render( + , + ) + expect(screen.queryByTestId('free-days-stay-off-notice')).not.toBeInTheDocument() + }) + + it('does not duplicate the notice once Advanced is opened (same warning, one copy)', async () => { + render() + await openAdvanced() + expect(screen.getAllByTestId('free-days-stay-off-notice')).toHaveLength(1) + }) }) describe("removed days (DayAssertion, dayType: 'none') — the way back", () => { diff --git a/frontend/src/components/modals/PlannedDayModal.tsx b/frontend/src/components/modals/PlannedDayModal.tsx index e62d8dc..724e7e1 100644 --- a/frontend/src/components/modals/PlannedDayModal.tsx +++ b/frontend/src/components/modals/PlannedDayModal.tsx @@ -130,16 +130,24 @@ export function PlannedDayModal({ const showPinnedControls = forcedAdvanced || soloDirect || isAdvancedOpen // A concrete type is about to be pinned (not the 'auto' planner-decides - // choice) — the two notices below only make sense once that's true. + // choice) — the pin-exclusion notice below only makes sense once that's + // true. const isPinningConcreteType = pinnedPath && dayType !== 'auto' - // The chosen type would leave every free day in the selection (weekend/red - // day) as a day off: always true for `vacation`, and true for a PL type - // unless the workspace opted in via `takePlOnRedDays` — mirrors - // `Calendar.tsx`'s `handleSavePinnedDay` filter exactly. + // Would every free day (weekend/red day) in the selection stay OFF under + // the save path this dialog is currently set up to take? Pinning a + // concrete type: always for `vacation`, and for a PL type unless the + // workspace opted in via `takePlOnRedDays` — mirrors `Calendar.tsx`'s + // `handleSavePinnedDay` filter exactly. The default/auto (wish) path: + // `allocation_service.py`'s `is_free_day` branch fires BEFORE any type is + // chosen and marks the day 'free' unless `takePlOnRedDays`, regardless of + // parentMode/dubbeldag — so free days stay off there too, always, unless + // that same opt-in is on. Shown regardless of `isPinningConcreteType` so + // the default path warns just as loudly as the advanced one (see the + // "silent no-op" fix: planning a free day through the default dialog used + // to accept with nothing changed and no explanation). const freeDaysWillStayOff = - isPinningConcreteType && freeDatesInSelection.length > 0 && - (dayType === 'vacation' || !takePlOnRedDays) + (pinnedPath ? dayType === 'vacation' || !takePlOnRedDays : !takePlOnRedDays) // Pinning excludes the day from `sync_plan`'s automatic re-planning — // worth saying prospectively (about to pin) even when the day isn't // already pinned; `existingDay?.isPinned` below already covers the @@ -348,6 +356,17 @@ export function PlannedDayModal({ )} + {/* The default path's own copy of the "will stay off" notice — + * `showPinnedControls` is false here (Advanced isn't open), so + * the copy inside that section below never renders for this + * case. Gated on `!showPinnedControls` rather than re-deriving + * "default path" so the two copies can never both render. */} + {!forcedAdvanced && hasMultipleParents && !showPinnedControls && freeDaysWillStayOff && ( +

+ {t('plannedDayModal.freeDaysStayOffNotice')} +

+ )} + {!forcedAdvanced && hasMultipleParents && (