From 5e96c352fb9a944991ea44a6a37f6f0dc6973caf Mon Sep 17 00:00:00 2001 From: Jacob Hagstedt Date: Thu, 17 Sep 2026 21:04:44 +0200 Subject: [PATCH] Persist the wish and pinned flag a planned day is created with The create and update handlers for a planned day accepted both fields in their request models and passed neither to the service, so a day hand-edited in the calendar was never marked as pinned and never kept its link back to the period that produced it. The allocator would then feel free to retype it on the next sync, which is precisely what pinning exists to prevent. Two investigations alongside this found no defect. A period that occurs once rather than yearly does get placed, and a window that wraps past New Year keeps the half that falls after today; both are now covered by tests, since neither case had any. Where those looked broken, the wish had simply run out of budget for that year, which the interface does not yet explain. --- backend/app/routers/planned_days.py | 4 + .../tests/integration/test_user_journey.py | 156 +++++++++++++++ .../test_allocation_once_and_wrap.py | 185 ++++++++++++++++++ 3 files changed, 345 insertions(+) create mode 100644 backend/tests/unit/test_services/test_allocation_once_and_wrap.py diff --git a/backend/app/routers/planned_days.py b/backend/app/routers/planned_days.py index 5739c82..3c40631 100644 --- a/backend/app/routers/planned_days.py +++ b/backend/app/routers/planned_days.py @@ -33,6 +33,8 @@ async def create_planned_day( date=payload.date, day_type=payload.day_type, is_dubbeldag=payload.is_dubbeldag, + wish_id=payload.wish_id, + is_pinned=payload.is_pinned, ) return _to_response(day) except ValidationError as exc: @@ -84,6 +86,8 @@ async def update_planned_day( planned_leave_day_id, day_type=payload.day_type, is_dubbeldag=payload.is_dubbeldag, + wish_id=payload.wish_id, + is_pinned=payload.is_pinned, ) return _to_response(day) except NotFoundError as exc: diff --git a/backend/tests/integration/test_user_journey.py b/backend/tests/integration/test_user_journey.py index 0a52eab..37fe2fd 100644 --- a/backend/tests/integration/test_user_journey.py +++ b/backend/tests/integration/test_user_journey.py @@ -193,6 +193,24 @@ async def _apply( return resp.json() +async def _sync(client: AsyncClient, headers: dict[str, str], workspace_id: UUID) -> dict[str, Any]: + resp = await client.post( + "/api/plan/sync", headers=headers, json={"workspace_id": str(workspace_id)} + ) + assert resp.status_code == 200, resp.text + return resp.json() + + +async def _list_planned_days( + client: AsyncClient, headers: dict[str, str], child_id: UUID +) -> list[dict[str, Any]]: + resp = await client.get( + "/api/planned-days", headers=headers, params={"child_id": str(child_id)} + ) + assert resp.status_code == 200, resp.text + return resp.json() + + def _paid_days_for_wish(proposal: dict[str, Any], wish_id: str) -> list[dict[str, Any]]: """Proposed days belonging to `wish_id` that would actually cost a @@ -942,3 +960,141 @@ class TestLeaveWishNameMessageKeyRoundTrip: body = resp.json() assert body["name"] == "Vinterresa" assert body["name_message_key"] is None + + +# --------------------------------------------------------------------------- +# 9) is_pinned / wish_id ROUND-TRIP, AND SURVIVAL THROUGH /api/plan/sync +# (routers/planned_days.py bug — both fields were accepted by the request +# schema and understood by PlanningService, but silently dropped by the +# router before ever reaching the service, so "pin this day" never +# persisted). One shared registration for both scenarios — see +# `TestLeaveWishNameMessageKeyRoundTrip` above for why: `/api/auth/register` +# is rate-limited to 10/minute and this module already registers a user per +# test class. +# --------------------------------------------------------------------------- + + +class TestPlannedDayIsPinnedRoundTripAndSyncSurvival: + """`PlannedLeaveDayCreate`/`Update` both carry `is_pinned`/`wish_id`, and + + `PlanningService.add_planned_day`/`.update` both accept them — but the + router never forwarded either field, so a client setting `is_pinned: + true` (the "pin this day so the allocator won't retype it" gesture) had + it silently discarded. Both scenarios (the raw POST/PUT round trip, and + the end-to-end "survives a re-sync" behaviour it exists for) share a + single registration, same reasoning as `TestLeaveWishNameMessageKeyRoundTrip` + above. + """ + + async def test_is_pinned_round_trips_and_survives_a_subsequent_sync( + self, client: AsyncClient, connection: aiosqlite.Connection + ) -> None: + headers, ws_id, pd_uuid = await _register(client, "pin-roundtrip@example.com", "Alex") + birth_date = date.today() - relativedelta(months=6) + child_id = await _add_child(client, headers, ws_id, "Barn", birth_date) + await _set_full_parental_leave(client, headers, pd_uuid, ws_id, child_id) + + next_year = date.today().year + 1 + await _set_vacation(client, headers, pd_uuid, ws_id, next_year, total_days=25) + + wish = await _create_wish( + client, + headers, + workspace_id=str(ws_id), + child_id=str(child_id), + name="Pin Week", + window_start_month=9, + window_start_day=1, + window_end_month=9, + window_end_day=7, + target_weeks=1, + recurrence="once", + first_year=next_year, + ) + wish_id = wish["id"] + + # --- part 1: raw POST/PUT round trip ----------------------------- + create_resp = await client.post( + "/api/planned-days", + headers=headers, + json={ + "workspace_id": str(ws_id), + "child_id": str(child_id), + "parent_personal_data_uuid": str(pd_uuid), + "date": date.today().isoformat(), + "day_type": "vacation", + "is_dubbeldag": False, + "wish_id": wish_id, + "is_pinned": True, + }, + ) + assert create_resp.status_code == 201, create_resp.text + created = create_resp.json() + assert created["is_pinned"] is True, ( + "is_pinned must round-trip through POST /api/planned-days, not be silently " + "dropped by the router" + ) + assert created["wish_id"] == wish_id + + day_id = created["id"] + + # Flip both fields via PUT — a partial update — and confirm the new + # values, not the create-time ones, come back. + update_resp = await client.put( + f"/api/planned-days/{day_id}", + headers=headers, + json={"is_pinned": False}, + ) + assert update_resp.status_code == 200, update_resp.text + updated = update_resp.json() + assert updated["is_pinned"] is False, ( + "is_pinned must round-trip through PUT /api/planned-days/{id} too" + ) + # wish_id was not mentioned in this PUT — must be left untouched. + assert updated["wish_id"] == wish_id + + # Delete the ad-hoc probe row before part 2 so it can't be mistaken + # for one of the wish's own placed days below. + del_resp = await client.delete(f"/api/planned-days/{day_id}", headers=headers) + assert del_resp.status_code == 204, del_resp.text + + # --- part 2: a pinned day survives a subsequent /api/plan/sync -- + # the end-to-end behaviour users actually rely on `is_pinned` for: + # "changing a day's funding type pins it so the allocator won't + # retype it". + + # First sync places the wish's days as ordinary (unpinned, + # wish-attributed) rows. + await _sync(client, headers, ws_id) + planned = await _list_planned_days(client, headers, child_id) + wish_days = [d for d in planned if d["wish_id"] == wish_id] + assert wish_days, "expected the wish to have been placed by the first sync" + + pinned_day = wish_days[0] + original_day_type = pinned_day["day_type"] + # Retype it to whatever it currently is NOT, mirroring "the user + # hand-edited this day's funding type" — then pin it so a re-sync + # must leave it alone. + new_day_type = "sjukpenning" if original_day_type != "sjukpenning" else "vacation" + + pin_resp = await client.put( + f"/api/planned-days/{pinned_day['id']}", + headers=headers, + json={"day_type": new_day_type, "is_pinned": True}, + ) + assert pin_resp.status_code == 200, pin_resp.text + assert pin_resp.json()["is_pinned"] is True + assert pin_resp.json()["day_type"] == new_day_type + + # Re-sync: a fresh propose would want this date back at + # `original_day_type` (the wish's natural funding choice) — the + # pinned row must survive untouched instead. + await _sync(client, headers, ws_id) + + after = await _list_planned_days(client, headers, child_id) + after_row = next(d for d in after if d["id"] == pinned_day["id"]) + assert after_row["is_pinned"] is True + assert after_row["day_type"] == new_day_type, ( + "a pinned day must survive a subsequent /api/plan/sync without being " + "retyped back by the allocator" + ) diff --git a/backend/tests/unit/test_services/test_allocation_once_and_wrap.py b/backend/tests/unit/test_services/test_allocation_once_and_wrap.py new file mode 100644 index 0000000..19e94b5 --- /dev/null +++ b/backend/tests/unit/test_services/test_allocation_once_and_wrap.py @@ -0,0 +1,185 @@ +"""Regression tests for two behaviours investigated after a live bug report: + +1. `recurrence: 'once'` wishes — reported as producing *zero* planned days + for a drag-created period, with no error/rejection surfaced anywhere. + Investigation (see the task write-up) traced the live symptom to the demo + seed data: the new wish's default (lowest) priority + `parent_mode: + 'specific'` bound to a parent whose vacation budget for that particular + year is deliberately zeroed by `scripts/seed_demo.py`. `LeaveWish + .applies_to_year`/`_candidate_years`/`_expand_all_windows` all handle + `recurrence == 'once'` correctly — proven here with ample resources (the + window is produced and fully funded) and with a past window (correctly + produces nothing, the *good* kind of "nothing"). These tests exist so + that conclusion has a permanent regression guard instead of living only + in an investigation transcript. + +2. A wrapping window (e.g. Jullov 12-20 -> 01-16) whose anchor year's start + is before `today` but whose end is after `today` — does `window_for_year` + + the horizon-clip in `_expand_all_windows` partially clip it (correct) + or drop the whole window (bug)? Proven here: partial clip, future part + funded, past part silently dropped from the window's *dates* — a + distinct, correct mechanism from a *funding* shortfall (which shows up as + `shortfall_days`, not as a shorter window). +""" + +from datetime import date + +from app.services.allocation_service import ( + Allocator, + DayPool, + VacationBudget, +) +from tests.unit.test_services.test_allocation_service import ( + _build_input, + _child, + _parent, + _wish, +) + + +class TestOnceRecurrenceProducesDays: + def test_once_wish_with_ample_resources_is_fully_funded_on_the_right_dates(self) -> None: + """A `once` wish is not, by itself, broken: given a parent with an + + untouched pool/vacation budget, it funds every day in its single + occurrence — exactly like a `yearly` wish would for that same year. + """ + child = _child(birth_date=date(2026, 3, 1)) + parent = _parent(salary_type="monthly") + wish = _wish( + child_id=child.id, + name="Drag-created period", + start=(10, 5), + end=(10, 9), # Mon-Fri, 5 days + weeks=1, + target_days=5, + recurrence="once", + first_year=2026, + parent_mode="specific", + parent_uuid=parent.uuid, + ) + alloc_input = _build_input( + today=date(2026, 9, 17), + horizon_end=date(2027, 12, 31), + planning_mode="solo", + children=[child], + parents=[parent], + pools={(parent.uuid, child.id): DayPool(390, 90, 30)}, + vacation={(parent.uuid, 2026): VacationBudget(25, 0, 0)}, + wishes=[wish], + ) + + proposal = Allocator().propose(alloc_input) + + assert len(proposal.outcomes) == 1 + outcome = proposal.outcomes[0] + assert outcome.start_date == date(2026, 10, 5) + assert outcome.end_date == date(2026, 10, 9) + assert outcome.shortfall_days == 0 + + funded_dates = {d.date for d in proposal.days if d.day_type != "free"} + assert funded_dates == { + date(2026, 10, 5), + date(2026, 10, 6), + date(2026, 10, 7), + date(2026, 10, 8), + date(2026, 10, 9), + } + assert all(d.day_type == "vacation" for d in proposal.days if d.date in funded_dates) + + def test_once_wish_entirely_in_the_past_produces_nothing(self) -> None: + """The correct-clipping counterpart: a `once` wish whose single + + occurrence is entirely before `today` is dropped, not funded — the + same rule that applies to a `yearly` wish's already-elapsed years. + """ + child = _child(birth_date=date(2020, 1, 1)) + parent = _parent(salary_type="monthly") + wish = _wish( + child_id=child.id, + name="Long gone", + start=(3, 1), + end=(3, 5), + weeks=1, + recurrence="once", + first_year=2021, + parent_mode="specific", + parent_uuid=parent.uuid, + ) + alloc_input = _build_input( + today=date(2026, 9, 17), + horizon_end=date(2027, 12, 31), + planning_mode="solo", + children=[child], + parents=[parent], + pools={(parent.uuid, child.id): DayPool(390, 90, 30)}, + vacation={(parent.uuid, 2021): VacationBudget(25, 0, 0)}, + wishes=[wish], + ) + + proposal = Allocator().propose(alloc_input) + + assert proposal.outcomes == [] + assert proposal.days == [] + + +class TestWrappingWindowStraddlingToday: + def test_wrapping_window_straddling_today_clips_past_and_funds_future(self) -> None: + """A yearly wrapping window (Jullov-style, 12-20 -> 01-16) whose + + anchor year's start already elapsed relative to `today`, but whose + (year+1) end has not: `_expand_all_windows` must clip the window down + to `[today, end]`, not drop the whole thing. This is the specific + mechanism the "wrapping window loses its head" bug report asked to + be checked — proven here to be correct: only the already-past + Dec 20-Jan 4 stub disappears from the window's *dates*; the + still-future Jan 5-16 part is present and gets funded. + """ + child = _child(birth_date=date(2020, 1, 1)) + parent = _parent(salary_type="monthly") + wish = _wish( + child_id=child.id, + name="Jullov", + start=(12, 20), + end=(1, 16), # wraps into January + weeks=4, + recurrence="yearly", + first_year=2025, + last_year=2026, # explicit, so this doesn't depend on the child's 4th birthday + parent_mode="specific", + parent_uuid=parent.uuid, + ) + today = date(2026, 1, 5) # inside the 2025-anchored window's January tail + alloc_input = _build_input( + today=today, + horizon_end=date(2026, 12, 31), + planning_mode="solo", + children=[child], + parents=[parent], + pools={(parent.uuid, child.id): DayPool(390, 90, 30)}, + vacation={ + (parent.uuid, 2025): VacationBudget(25, 0, 0), + (parent.uuid, 2026): VacationBudget(25, 0, 0), + }, + wishes=[wish], + ) + + proposal = Allocator().propose(alloc_input) + + outcome_2025 = next(o for o in proposal.outcomes if o.year == 2025) + # Clipped to [today, end] — the already-past Dec 20 2025 -> Jan 4 + # 2026 stub is gone; only the future Jan 5 -> Jan 16 part remains. + assert outcome_2025.start_date == today + assert outcome_2025.end_date == date(2026, 1, 16) + assert outcome_2025.start_date > date(2025, 12, 20), ( + "the window must be clipped to today, not left at its original (now-past) start date" + ) + + funded_dates = { + d.date for d in proposal.days if d.wish_id == wish.id and d.date <= date(2026, 1, 16) + } + assert funded_dates, "the future (Jan 5-16) part of the window must still be funded" + assert all(d >= today for d in funded_dates), ( + "no day before `today` should ever appear — the window was clipped, not just " + "reported as clipped" + )