New plan for new feature
This commit is contained in:
@@ -0,0 +1,102 @@
|
||||
# Free days, vacation forfeiture, and auto-replanning
|
||||
|
||||
## Context
|
||||
|
||||
Three feedback points from `todo.md` (human-owned file — do not edit it; item 1 there, the yearly-repeat bug, is owned by another worker and is out of scope here):
|
||||
|
||||
1. **A full-week selection must not burn days on Sat/Sun/red days.** You can never take *vacation* on a free day; you *can* take PL on one (more money, fewer days banked) — that must be an opt-in setting defaulting to off.
|
||||
2. **Vacation days at risk of being forfeited should push the planner toward spending vacation** instead of PL days.
|
||||
3. **Every event that changes the days off must trigger a re-plan** of the day *types*, not just a refetch.
|
||||
|
||||
Exploration shows the backend already implements most of 1 and 2. The real gaps are: the hand-pin path bypasses the free-day invariant entirely, recalculation only fires from the Calendar page for some events, and vacation forfeiture is invisible in the UI.
|
||||
|
||||
### What already exists (reuse, do not rebuild)
|
||||
|
||||
| Piece | Where | State |
|
||||
|---|---|---|
|
||||
| `take_pl_on_red_days` workspace column + migration `003` + `PUT /api/workspace/:id` | `backend/app/entities/workspace.py`, `app/services/workspace_service.py` | done, defaults `False` |
|
||||
| Settings toggle + sv/en strings | `frontend/src/pages/Settings.tsx:496`, `src/i18n/{sv,en}/settings.ts:108-110` | done |
|
||||
| Allocator free-day handling (free day ⇒ `'free'`, or FP via `REASON_FP_RED_DAY` only when the flag is on; vacation **never**) | `allocation_service.py` `_fund_solo:1271`, `_fund_both:~1518` | done, tested at `tests/unit/test_services/test_allocation_service.py:2616+` |
|
||||
| `is_free_day(d, holidays_map, overrides)` — the single definition | `allocation_service.py:470` | reuse everywhere |
|
||||
| `vacation_forfeiting` bias in `_assign_tier3` | `allocation_service.py:1171-1225`, fed from `AnalyticsService._vacation_year_series` at `:2935` | done |
|
||||
| `VacationYearBalance.expiring / expiring_age / expiring_over_cap` on the wire | `analytics_service.py:47`, `frontend/src/types/entities.ts:251` | computed, **never rendered** |
|
||||
| `POST /api/plan/sync` (full re-plan + persist + diff) | `planning_service.py:662`, `usePlanSync`, `Calendar.tsx` `runSync:365` | done |
|
||||
| `WorkspaceContext.planVersion` / `bumpPlanVersion()` cross-instance invalidation | `frontend/src/context/WorkspaceContext.tsx:25-36,179` | done; bumped only by `usePlannedDays`, `useDayOverrides`, `usePlannedDayExclusions` |
|
||||
|
||||
---
|
||||
|
||||
## Task 1 — Free days must never consume vacation (hand-pin path)
|
||||
|
||||
Selecting a range that spans Sat/Sun/red days is fine and stays fine. The planner already skips those days correctly. The hole is the **"advanced" day popover**: `Calendar.tsx` `handleSavePinnedDay:571` writes one `planned_leave_day` row per selected date, free days included, and `PlanningService.add_planned_day:241` accepts `day_type='vacation'` on a Saturday without complaint.
|
||||
|
||||
**Backend (authoritative guard).** In `backend/app/services/planning_service.py`:
|
||||
- Add optional `workspace_repo` + `day_override_repo` constructor deps, following the existing optional-dependency pattern of `_engine` / `_leave_wish_repo` (docstring the reason; keep `scripts/seed_demo.py` and test fixtures working unchanged when unset).
|
||||
- New private helper `_reject_illegal_free_day(workspace_id, date, day_type)` used by both `add_planned_day` and `update`: build the holiday map + `DayOverrideService.get_effective_map(workspace_id)`, call `is_free_day` (import from `allocation_service`, do not re-derive), then raise `ValidationError` when
|
||||
- `day_type == 'vacation'` and the day is free (never legal), or
|
||||
- `day_type in ('sjukpenning','lagsta')` and the day is free and `workspace.take_pl_on_red_days` is `False`.
|
||||
- Skip the check entirely when the deps are unset (`None`), so behaviour is byte-identical for callers that never wire them; `app/dependencies.py` wires the real ones.
|
||||
- Error goes through `app/utils/errors.py`, message keyed so the frontend can translate.
|
||||
|
||||
**Frontend.** In `Calendar.tsx` `handleSavePinnedDay`, filter the write loop by the day's `isFree` from `calendarDaysByDate` (already loaded, `CalendarDayEntry.isFree`):
|
||||
- `vacation` → skip free days always.
|
||||
- `sjukpenning`/`lagsta` → skip free days unless `workspace.takePlOnRedDays`.
|
||||
- If every selected date was skipped, show the existing modal error rather than silently doing nothing.
|
||||
- `PlannedDayModal` shows a one-line note when the selection contains free days that will be left as days off, with a pointer to the setting. New sv/en keys in `src/i18n/{sv,en}/calendar.ts`.
|
||||
|
||||
**Tests.** Backend: `add_planned_day`/`update` reject vacation on a Saturday and on a red day; reject PL on a free day with the flag off; accept PL on a free day with it on. Frontend: pinning a Mon–Sun range with `vacation` creates 5 rows, not 7; with `takePlOnRedDays: true` and `sjukpenning`, creates 7.
|
||||
|
||||
---
|
||||
|
||||
## Task 2 — Surface vacation days at risk
|
||||
|
||||
The numbers are already on the wire and unused.
|
||||
|
||||
- `frontend/src/components/vacation-days/VacationDaysTable.tsx` — add an "at risk" column rendering `VacationYearBalance.expiring`, with a danger style when `> 0` and a tooltip/help line splitting `expiringAge` (too old to carry) from `expiringOverCap` (above the carry-forward ceiling). Follow `Age4Bars.tsx` / `KeyFigures.tsx` for the existing danger-ink pattern; size text by content per the bilingual CSS rule.
|
||||
- `frontend/src/components/dashboard/KeyFigures.tsx` — a tile mirroring the existing `dashboard-figure-expiring` for vacation, so the reason the plan shifted toward vacation days is visible.
|
||||
- `PlanWarning` kind `'vacation_expiring'` already exists; confirm it renders where `days_lost_at_age_4` does and add the missing sv/en strings if not.
|
||||
- Tests: table shows the column and the danger state; zero-expiring renders the neutral state.
|
||||
|
||||
No backend change — `_vacation_year_series` and the `_assign_tier3` bias stay as they are.
|
||||
|
||||
---
|
||||
|
||||
## Task 3 — Re-plan on every plan-input change (global)
|
||||
|
||||
Today `runSync` is Calendar-only and fires once per workspace on mount plus after wish/preset/exclusion edits. Pinned-day saves and deletes, and every Settings change, do not re-plan.
|
||||
|
||||
**Move the trigger into the workspace layer** so it is route-independent:
|
||||
|
||||
- New `frontend/src/context/PlanSyncProvider.tsx` (mounted inside `WorkspaceProvider`, or folded into it — implementer's call, keep `WorkspaceContext`'s surface stable): a `useEffect` on `[workspaceId, planVersion]` that calls `api.syncPlan(workspaceId)` **silently**, guarded by an in-flight ref so overlapping bumps coalesce into one trailing run, and by a "skip the very first render if Calendar is about to sync anyway" check is *not* needed — a duplicate silent sync is idempotent and cheap, but the in-flight guard must still prevent a stampede.
|
||||
- Cycle safety: `bumpPlanVersion` is never called from a fetch effect (`usePlannedDays.ts:84-93` documents this), and `syncPlan` does not bump. Verified no loop.
|
||||
- On an `eviction_preview` coming back (nothing written, needs a user choice), the silent path must **not** discard it — surface it through context so Calendar can prompt on next visit, or at minimum leave the plan untouched and log through `src/services/errors.ts`. Never swallow it into an empty catch.
|
||||
- **Add the missing bumps** so every plan input participates. `useVacationDays.upsert`, `usePreschoolPlanningDays` (after its 800 ms debounce settles), `useChildSettings.update`, `api.updateParentSettings`' caller in `Settings.tsx`, and `handleToggleTakePlOnRedDays` all call `bumpPlanVersion()` on success — they currently do not. `usePlannedDays`, `useDayOverrides`, `usePlannedDayExclusions` already do.
|
||||
- **Calendar keeps its explicit `runSync` calls** for the paths that need a toast + undo (wish create/edit/delete, presets, exclusions). Those write via `useLeaveWishes`, which does *not* bump, so there is no double sync. Replace the `didAutoSyncRef` mount-once effect with reliance on the provider, or keep it — it becomes redundant but harmless; prefer removing it to keep one trigger.
|
||||
- `handleSavePinnedDay` / `handleDeletePlannedDay` need no new call: they write through `usePlannedDays`, which bumps, which now drives the global sync. This is what makes item 2's forfeiture bias actually retype existing unpinned days after a calendar edit.
|
||||
|
||||
**Known limit to state plainly in the UI, not silently:** a *pinned* day is excluded from re-planning by design (`sync_plan` passes `only_pinned_existing=True`). The forfeiture bias can therefore never retype a hand-pinned day. The popover note from Task 1 is the right place to say pinning opts that day out of optimization.
|
||||
|
||||
**Tests.** Frontend: a `bumpPlanVersion()` from a non-Calendar hook triggers exactly one `api.syncPlan`; two rapid bumps coalesce to one trailing call; a failing sync does not wedge the in-flight ref.
|
||||
|
||||
---
|
||||
|
||||
## Verification
|
||||
|
||||
Delegate all full-suite runs to a **Haiku** subagent (per AGENTS.md), serialized — one suite at a time, never parallel:
|
||||
|
||||
```bash
|
||||
# backend/
|
||||
uv run ruff check . && uv run ruff format --check . && uv run pytest
|
||||
# frontend/
|
||||
npm run typecheck && npm run lint && npm run coverage
|
||||
```
|
||||
Require raw verbatim output with exact pass/fail counts and coverage percentages; gates are backend `--cov-fail-under=80` and frontend lines 96 / functions 91 / branches 92 / statements 96. Never lower a gate.
|
||||
|
||||
New migration is **not** needed — no schema change.
|
||||
|
||||
End-to-end, delegated to a **Sonnet** subagent driving Chrome (never the coordinator), on `npm run dev`, checked in both Swedish and English:
|
||||
1. Select Mon–Sun in the calendar, "advanced" → vacation. Assert 5 planned days, Sat/Sun untouched.
|
||||
2. Settings → tick "Ta ut föräldrapenning på röda dagar" → return to calendar, repeat with `sjukpenning`. Assert 7 days.
|
||||
3. Settings → edit a vacation-days row → assert a background `POST /api/plan/sync` fires (network panel) without visiting the Calendar page.
|
||||
4. Enter a vacation balance that forfeits days → assert the at-risk column shows it and the plan's day mix shifts toward vacation.
|
||||
|
||||
Then `graphify update .`.
|
||||
Reference in New Issue
Block a user