Say something when planning a free day changes nothing
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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(<PlannedDayModal {...defaultProps} isOpen={true} freeDatesInSelection={['2026-06-13']} />)
|
||||
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(
|
||||
<PlannedDayModal
|
||||
{...defaultProps}
|
||||
isOpen={true}
|
||||
freeDatesInSelection={['2026-06-13']}
|
||||
takePlOnRedDays={true}
|
||||
/>,
|
||||
)
|
||||
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(<PlannedDayModal {...coupleProps} isOpen={true} freeDatesInSelection={['2026-06-13']} />)
|
||||
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(
|
||||
<PlannedDayModal
|
||||
{...coupleProps}
|
||||
isOpen={true}
|
||||
freeDatesInSelection={['2026-06-13']}
|
||||
takePlOnRedDays={true}
|
||||
/>,
|
||||
)
|
||||
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(<PlannedDayModal {...coupleProps} isOpen={true} freeDatesInSelection={['2026-06-13']} />)
|
||||
await openAdvanced()
|
||||
expect(screen.getAllByTestId('free-days-stay-off-notice')).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe("removed days (DayAssertion, dayType: 'none') — the way back", () => {
|
||||
|
||||
@@ -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({
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* 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 && (
|
||||
<p className="planned-day-modal__hint" data-testid="free-days-stay-off-notice">
|
||||
{t('plannedDayModal.freeDaysStayOffNotice')}
|
||||
</p>
|
||||
)}
|
||||
|
||||
{!forcedAdvanced && hasMultipleParents && (
|
||||
<button
|
||||
type="button"
|
||||
|
||||
@@ -101,6 +101,11 @@ export const calendar = {
|
||||
// dates alone when no single period can be named — see vacationSwaps.ts.
|
||||
'calendar.toastVacationSwap':
|
||||
'{count} {count|vacation day|vacation days} moved from {from} to {to} — replaced with parental leave',
|
||||
// §silent-no-op fix — the generic fallback for a sync that was accepted
|
||||
// but changed nothing at all (e.g. every selected date resolved to a free
|
||||
// day the planner leaves off): without this, no toast rendered, and the
|
||||
// click looked like it had done nothing.
|
||||
'calendar.toastNoChanges': 'No changes were made.',
|
||||
'calendar.undoButton': 'Undo',
|
||||
'calendar.undoError': 'Failed to undo the change',
|
||||
'calendar.dismissToastAria': 'Dismiss the message',
|
||||
|
||||
@@ -98,6 +98,11 @@ export const calendar = {
|
||||
// dates alone when no single period can be named — see vacationSwaps.ts.
|
||||
'calendar.toastVacationSwap':
|
||||
'{count} {count|semesterdag|semesterdagar} flyttades från {from} till {to} — ersattes med föräldrapenning',
|
||||
// §silent-no-op fix — den generiska reservtexten när en synk accepterades
|
||||
// men inte ändrade något alls (t.ex. varje vald dag var en ledig dag som
|
||||
// planeraren lämnar ledig): utan den här texten visades ingen toast, och
|
||||
// klicket såg ut att inte ha gjort något.
|
||||
'calendar.toastNoChanges': 'Inga ändringar gjordes.',
|
||||
'calendar.undoButton': 'Ångra',
|
||||
'calendar.undoError': 'Det gick inte att ångra ändringen',
|
||||
'calendar.dismissToastAria': 'Stäng meddelandet',
|
||||
|
||||
@@ -3783,6 +3783,31 @@ describe('Calendar', () => {
|
||||
expect(remove).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// §silent-no-op fix — planning a day that resolves entirely to free days
|
||||
// (weekend/red day, `takePlOnRedDays` off) is accepted with an empty
|
||||
// `changes` batch and `shortfallDays: 0` (a free day is excluded from the
|
||||
// shortfall count entirely — see `_shortfall_and_weeks_funded`,
|
||||
// allocation_service.py), which is exactly what the default
|
||||
// `makePlanSyncWithWishResponse()`/`makePlanProposal()` fixtures already
|
||||
// produce. Before this fix, `buildSyncToastMessage` returned `null` for
|
||||
// that shape and `applyWishSyncResult` rendered no toast at all — a click
|
||||
// that visibly did nothing. It must now say so.
|
||||
it('shows a toast when an accepted sync-with-wish changes nothing (e.g. every selected date was a free day)', async () => {
|
||||
const created = makeLeaveWish({ id: 'lw-manual' })
|
||||
const syncWithWish = vi.fn().mockResolvedValue(makePlanSyncWithWishResponse({ wish: created }))
|
||||
vi.mocked(useLeaveWishes).mockReturnValue(makeLeaveWishesResult({ data: [] }))
|
||||
vi.mocked(usePlanSync).mockReturnValue(makePlanSyncResult({ syncWithWish }))
|
||||
|
||||
render(<Calendar />)
|
||||
const calendarDays = screen.getAllByTestId('calendar-day')
|
||||
const currentMonthDay = calendarDays.find((el) => !el.classList.contains('calendar-day--disabled'))!
|
||||
fireEvent.click(currentMonthDay)
|
||||
fireEvent.click(screen.getByTestId('modal-save-wish-both'))
|
||||
|
||||
const toast = await screen.findByTestId('calendar-toast')
|
||||
expect(toast).toHaveTextContent('Inga ändringar gjordes.')
|
||||
})
|
||||
|
||||
// Step 2 (intent-vs-derived-output restructure) — displacement is now an
|
||||
// informational outcome, never a refusal: `sync_plan` always writes, and
|
||||
// a genuinely evicted row (a DIFFERENT wish's edit won a shared pool
|
||||
|
||||
@@ -679,6 +679,14 @@ export function Calendar(): ReactNode {
|
||||
// shown on a rejection; `buildSyncToastMessage`'s own rendering of
|
||||
// `applyResult.rejections` already covers that case.
|
||||
fallbackMessage?: string,
|
||||
// §silent-no-op fix — opt-in only (see below), scoped to `handleSaveWish`
|
||||
// (the reported defect: the default "Planera en dag" dialog on a free
|
||||
// day). The preset rail and period-popover saves have their own
|
||||
// pre-existing "genuine no-op" tests asserting silence on a truly
|
||||
// no-op sync (e.g. re-clicking an already fully-placed preset) — this
|
||||
// stays opt-in rather than global so that intentional silence is
|
||||
// undisturbed.
|
||||
noChangeMessage?: string,
|
||||
): Promise<PlanSyncWithWishResponse> => {
|
||||
// Same last-writer-wins source as `runSync`'s own response — every
|
||||
// wish save on this page comes through here, not `runSync`, so the
|
||||
@@ -696,8 +704,16 @@ export function Calendar(): ReactNode {
|
||||
locale,
|
||||
result.wish,
|
||||
)
|
||||
// §silent-no-op fix — an accepted sync that changed nothing (e.g. every
|
||||
// selected date resolved to a free day the planner leaves off, see
|
||||
// `PlannedDayModal`'s `freeDaysStayOffNotice`) used to fall all the way
|
||||
// through to `message === null` here and render no toast at all: a
|
||||
// click that visibly did nothing. Only applied when the caller opted
|
||||
// in via `noChangeMessage` — see that param's doc comment.
|
||||
const message = accepted
|
||||
? [fallbackMessage, syncMessage].filter((part): part is string => Boolean(part)).join(' · ') || null
|
||||
? [fallbackMessage, syncMessage].filter((part): part is string => Boolean(part)).join(' · ') ||
|
||||
noChangeMessage ||
|
||||
null
|
||||
: syncMessage
|
||||
if (message) showToast(message, accepted && buildUndo ? buildUndo(result.wish) : null)
|
||||
return result
|
||||
@@ -1002,13 +1018,24 @@ export function Calendar(): ReactNode {
|
||||
setIsModalOpen(false)
|
||||
setSelectedDates(new Set())
|
||||
|
||||
await applyWishSyncResult(result, (wish) => ({
|
||||
label: t('calendar.undoButton'),
|
||||
run: async () => {
|
||||
await wishesHook.remove(wish.id)
|
||||
await runSync(workspaceId, { silent: true })
|
||||
},
|
||||
}))
|
||||
await applyWishSyncResult(
|
||||
result,
|
||||
(wish) => ({
|
||||
label: t('calendar.undoButton'),
|
||||
run: async () => {
|
||||
await wishesHook.remove(wish.id)
|
||||
await runSync(workspaceId, { silent: true })
|
||||
},
|
||||
}),
|
||||
undefined,
|
||||
// §silent-no-op fix — this is the reported defect: the default
|
||||
// "Planera en dag" dialog on a free day (weekend/red day, no
|
||||
// `takePlOnRedDays` opt-in) is accepted with an empty `changes`
|
||||
// batch and `shortfallDays: 0` (a free day is excluded from the
|
||||
// shortfall count entirely, see `_shortfall_and_weeks_funded` in
|
||||
// allocation_service.py) — previously rendering no toast at all.
|
||||
t('calendar.toastNoChanges'),
|
||||
)
|
||||
} catch (err) {
|
||||
setModalError(describePlannedDayError(err, t, membersHook.data, t('calendar.saveError')))
|
||||
} finally {
|
||||
|
||||
Reference in New Issue
Block a user