Keep plan warnings in step with the plan they describe
Warnings such as "vacation days at risk of being lost" kept showing after the planner had already resolved them. The planner itself was firing correctly -- PlanSyncProvider syncs on every plan-input change -- so the fault was in the warnings, in two independent places. The vacation-expiry warning was computed before allocation. It read `vacation_forfeiting`, which the loader derives from already-committed usage, while `_assign_tier3` deliberately spends at-risk vacation days first precisely to avoid that forfeiture. The figure therefore counted days the plan was in the middle of saving. `_vacation_expiry_warnings` now subtracts the vacation days this run actually placed for that parent and year, clamped at zero -- sound because the bank is FIFO, so a day spent in year Y retires one expiring day one-for-one. The full carry-forward simulation is still not duplicated inside the ledger. The Dashboard then never refreshed them. Its proposal came from a one-shot effect guarded by `proposal === null`, with no planVersion dependency, so the card kept rendering the pre-replan warning until remount -- while `syncPlan` was already returning a fresh proposal that both call sites discarded. PlanSyncProvider now exposes that proposal behind a monotonic counter, and usePlanProposal prefers it once any sync has completed, keeping `propose()` for the first load. Also adds a manual recompute button to Settings, which only bumps planVersion and reports what the resulting sync did, and fixes partner add/remove never bumping planVersion at all -- that gap meant gaining or losing a parent never re-ran the allocator, though it changes who days can be assigned to. Rename stays cosmetic. Verified in the browser: a 2031 warning of 15 days dropped to 12 after three vacation days were planned, without a reload. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014jPFUcCJ5AKXmPVC1anhXe
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { ReactNode, useEffect, useMemo, useRef, useState } from 'react'
|
||||
import { useWorkspace } from '@/context'
|
||||
import { useWorkspace, usePlanSyncContextOptional } from '@/context'
|
||||
import {
|
||||
useVacationDays,
|
||||
useShowWeekNumbers,
|
||||
@@ -108,6 +108,13 @@ function computeVacationYearRange(birthDate: string | undefined, currentYear: nu
|
||||
|
||||
const PRESCHOOL_AUTOSAVE_DELAY_MS = 800
|
||||
|
||||
// How long the recompute button (Kalender & språk panel) waits for
|
||||
// `PlanSyncProvider`'s triggered sync to land before giving up and
|
||||
// reporting a failure — a sync that errors instead of resolving is only
|
||||
// ever logged (see `PlanSyncProvider.tsx`), so without a ceiling here the
|
||||
// button would spin forever on that path.
|
||||
const RECOMPUTE_TIMEOUT_MS = 10_000
|
||||
|
||||
/**
|
||||
* One (child, year) row letting the user record the true number of
|
||||
* preschool planning days actually taken that year — the child-level
|
||||
@@ -258,12 +265,58 @@ export function Settings(): ReactNode {
|
||||
// and vice versa.
|
||||
const [isSavingFamily, setIsSavingFamily] = useState(false)
|
||||
const [isSavingRedDayPref, setIsSavingRedDayPref] = useState(false)
|
||||
const [isRecomputing, setIsRecomputing] = useState(false)
|
||||
const [newPartnerName, setNewPartnerName] = useState('')
|
||||
const [renamingUuid, setRenamingUuid] = useState<string | null>(null)
|
||||
const [renameValue, setRenameValue] = useState('')
|
||||
const [removeConfirmUuid, setRemoveConfirmUuid] = useState<string | null>(null)
|
||||
const [memberActionError, setMemberActionError] = useState<{ uuid: string; text: string } | null>(null)
|
||||
|
||||
// Optional: in the real app `Settings` is always mounted inside
|
||||
// `WorkspaceProvider` (and therefore `PlanSyncProvider`), but this page's
|
||||
// own test suite renders it with `@/context` auto-mocked wholesale — the
|
||||
// optional variant keeps the recompute button below testable without
|
||||
// requiring a real `PlanSyncProvider` in every existing test. See
|
||||
// `PlanSyncProvider.tsx`.
|
||||
const planSyncContext = usePlanSyncContextOptional()
|
||||
const latestProposalVersion = planSyncContext?.latestProposalVersion ?? 0
|
||||
const pendingEvictionPreview = planSyncContext?.pendingEvictionPreview ?? null
|
||||
// The `latestProposalVersion` the recompute button's own triggered sync
|
||||
// should land past — `null` while no recompute is in flight, so a sync
|
||||
// started by something else entirely (e.g. a vacation-day autosave
|
||||
// elsewhere on this same page) never flips this button's spinner off.
|
||||
const recomputeBaselineRef = useRef<number | null>(null)
|
||||
const recomputeTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)
|
||||
|
||||
// Detects the recompute button's own triggered sync landing.
|
||||
// `bumpPlanVersion()` (in `handleRecompute` below) doesn't itself return
|
||||
// anything to await — the sync it causes runs inside `PlanSyncProvider`,
|
||||
// entirely outside this component. `latestProposalVersion` moving past
|
||||
// the baseline captured when the button was clicked is this component's
|
||||
// only signal that *that* sync (as opposed to some unrelated one) has
|
||||
// completed. A `setTimeout` fallback covers a sync that errors instead of
|
||||
// resolving (`PlanSyncProvider` only logs that case, see its doc comment)
|
||||
// — without it a failed sync would leave this button spinning forever.
|
||||
useEffect(() => {
|
||||
if (recomputeBaselineRef.current === null) return
|
||||
if (latestProposalVersion <= recomputeBaselineRef.current) return
|
||||
recomputeBaselineRef.current = null
|
||||
if (recomputeTimeoutRef.current !== null) {
|
||||
clearTimeout(recomputeTimeoutRef.current)
|
||||
recomputeTimeoutRef.current = null
|
||||
}
|
||||
setIsRecomputing(false)
|
||||
if (pendingEvictionPreview) {
|
||||
// `wouldEvict === true` means the sync this button triggered wrote
|
||||
// NOTHING (see `EvictionPreview`'s doc comment) — never report that
|
||||
// as a success.
|
||||
setSaveMessage({ type: 'error', text: t('settings.recomputeEvicted') })
|
||||
} else {
|
||||
setSaveMessage({ type: 'success', text: t('settings.recomputeSuccess') })
|
||||
}
|
||||
setTimeout(() => setSaveMessage(null), 3000)
|
||||
}, [latestProposalVersion, pendingEvictionPreview, t])
|
||||
|
||||
// Settings isn't scoped to a single child the way Onboarding/Dashboard are
|
||||
// — the balance flow needs somewhere to attach a saved balance even before
|
||||
// a child is chosen/created elsewhere in the app.
|
||||
@@ -447,6 +500,10 @@ export function Settings(): ReactNode {
|
||||
setSaveMessage(null)
|
||||
await api.addPartner(workspace.id, newPartnerName)
|
||||
await refreshMembers()
|
||||
// A new partner changes which parent days the allocator can assign
|
||||
// leave to — re-type the rest of the plan's unpinned days. See
|
||||
// `WorkspaceContext.planVersion`.
|
||||
bumpPlanVersion()
|
||||
setNewPartnerName('')
|
||||
showMessage('success', t('settings.addPartnerSuccess'))
|
||||
} catch (err) {
|
||||
@@ -481,6 +538,9 @@ export function Settings(): ReactNode {
|
||||
if (uuid === personalDataUuid) {
|
||||
await refreshContext()
|
||||
}
|
||||
// No `bumpPlanVersion()` here, unlike add/remove below — a rename is
|
||||
// cosmetic and doesn't change which days the allocator can assign to
|
||||
// which parent, so there's nothing for a re-plan to fix.
|
||||
setRenamingUuid(null)
|
||||
setRenameValue('')
|
||||
} catch (err) {
|
||||
@@ -508,6 +568,10 @@ export function Settings(): ReactNode {
|
||||
await api.removeWorkspaceParent(workspace.id, uuid)
|
||||
setRemoveConfirmUuid(null)
|
||||
await refreshMembers()
|
||||
// Removing a partner changes which parent days the allocator can
|
||||
// assign leave to — re-type the rest of the plan's unpinned days.
|
||||
// See `WorkspaceContext.planVersion`.
|
||||
bumpPlanVersion()
|
||||
} catch (err) {
|
||||
setMemberActionError({ uuid, text: describePartnerError(err, t, t('settings.removePartnerError')) })
|
||||
} finally {
|
||||
@@ -532,6 +596,28 @@ export function Settings(): ReactNode {
|
||||
}
|
||||
}
|
||||
|
||||
// Manual recompute: bumping `planVersion` is the whole implementation —
|
||||
// `PlanSyncProvider` (mounted app-wide) turns the bump into a
|
||||
// `POST /api/plan/sync` itself. Calling `api.syncPlan` directly here
|
||||
// instead would bypass that provider's burst-coalescing and its
|
||||
// `pendingEvictionPreview` handling. See the effect above for how the
|
||||
// outcome (success vs. the sync writing nothing) gets reported.
|
||||
const handleRecompute = async () => {
|
||||
recomputeBaselineRef.current = latestProposalVersion
|
||||
setIsRecomputing(true)
|
||||
setSaveMessage(null)
|
||||
if (recomputeTimeoutRef.current !== null) {
|
||||
clearTimeout(recomputeTimeoutRef.current)
|
||||
}
|
||||
recomputeTimeoutRef.current = setTimeout(() => {
|
||||
recomputeBaselineRef.current = null
|
||||
recomputeTimeoutRef.current = null
|
||||
setIsRecomputing(false)
|
||||
showMessage('error', t('settings.recomputeTimeout'))
|
||||
}, RECOMPUTE_TIMEOUT_MS)
|
||||
bumpPlanVersion()
|
||||
}
|
||||
|
||||
return (
|
||||
<Layout>
|
||||
<div data-testid="settings-page" className="settings-page">
|
||||
@@ -819,6 +905,19 @@ export function Settings(): ReactNode {
|
||||
<option value="sv">{t('settings.languageSwedishOption')}</option>
|
||||
<option value="en">{t('settings.languageEnglishOption')}</option>
|
||||
</select>
|
||||
|
||||
<p className="settings-panel__help">{t('settings.recomputeHelp')}</p>
|
||||
<div className="actions settings-panel__actions">
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void handleRecompute()}
|
||||
className="btn btn--secondary"
|
||||
disabled={isRecomputing}
|
||||
data-testid="recompute-plan-button"
|
||||
>
|
||||
{isRecomputing ? t('settings.recomputing') : t('settings.recomputeButton')}
|
||||
</button>
|
||||
</div>
|
||||
</section>
|
||||
|
||||
<section className="settings-panel">
|
||||
|
||||
Reference in New Issue
Block a user