jacobh2/F-r-ldraledigplannerare
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Persist the wish and pinned flag a planned day is created with

Browse Source 
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.
This commit is contained in:
Jacob Hagstedt
2026-09-17 21:04:44 +02:00
parent 9e27fef78a
commit 5e96c352fb
3 changed files with 345 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/routers/planned_days.py
+4
View File
@@ -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:
backend/tests/integration/test_user_journey.py
+156
View File
@@ -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"
)
backend/tests/unit/test_services/test_allocation_once_and_wrap.py
+185
View File
@@ -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"
)