014503e89b
The planner knew exactly how many semesterdagar a parent was about to forfeit and never placed a single day to prevent it. It also had no weekend-wrap logic at all: REASON_VACATION_WRAP_WEEKEND was stamped on every tier-3 interior vacation day, including a lone Tuesday in February. Two changes. Spend better. A workday adjacent to a free stretch, or in the middle of a gap of three or fewer fenced by free days, now takes vacation -- _vacation_bridge_reason, consulted for every parent type. This departs from spec 6.1's "wrapping suppressed for hourly": the FP-preservation argument is salary-type independent, only the pay argument was hourly-moot. In a both-parents window, both now take vacation on such a date rather than one paying with FP. After funding, reallocate_vacation_by_value swaps vacation onto higher-ranked dates 1-for-1 with FP days, so which dates hold vacation improves while how many never drops -- greedy first, re-timed after, never withheld. An invariant test asserts the count can only rise. Name the losses. A new pass proposes concrete dates for vacation that would otherwise expire, current year only, ranked red-day bridge then weekend bridge then interior, later dates first. They ride on PlanProposal.rescue_days, kept off `days` so nothing downstream commits them, and surface on the Dashboard with accept-one, accept-all (one batched /api/plan/apply, not N) and dismiss. Dismissal reuses the tip_dismissal table, keyed on (parent, year) identity rather than the group's dates -- those are recomputed from the whole plan's residual on every propose, so date-hashing made dismissal self-defeating. Dismiss does not bump the plan version; it changes no data and must not trigger a write. Measured on the seeded demo: 12 forfeiting days for one parent now yield 12 proposed dates, all Fridays or Mondays. Total vacation placed is unchanged at 203 days, confirming the reallocation only re-times. Three defects here were found by driving a browser, while 2273 unit tests passed -- all three lived in the seam between layers that unit tests mock away. Adds API-level tests over the real request sequence. Rate limiting is now off in tests (one file consumed the whole registration budget) with the suite's first real 429 assertion. PROPOSE_VS_SYNC_BUG.md documents a pre-existing, unfixed issue found along the way: Dashboard figures swing on the first sync with no user action. Backend 946 tests, frontend 1341, coverage above all gates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZaX6xbuMEMXuyKA1fq2Mm
299 lines
14 KiB
Python
299 lines
14 KiB
Python
"""API-level flow tests for §vacation-rescue — the accept/dismiss/re-sync
|
|
|
|
sequence a real browser session drives, which no unit test walks (a unit
|
|
test mocks the seam between layers where all three defects below lived).
|
|
|
|
Three defects shipped in the vacation-rescue feature and were only caught by
|
|
manual browser testing, while 2273 unit tests stayed green:
|
|
|
|
1. Dismissing a rescue group did not stick — the dismissal fingerprinted the
|
|
group's volatile date list, and dismissing triggered a plan sync that
|
|
changed those dates, so the group always returned on the next propose.
|
|
Fixed by hashing the group's IDENTITY (`parent_personal_data_uuid`,
|
|
`year`) instead — see `allocation_service._rescue_group_hash_data`.
|
|
2. A dismiss triggered `POST /api/plan/sync`, which *writes* the plan, from
|
|
an action that should change no data.
|
|
3. The same parent-year vacation expiry was reported by three different
|
|
cards with three different numbers.
|
|
|
|
This file drives the real FastAPI app over HTTP (`ASGITransport`), reusing
|
|
`test_user_journey.py`'s `client` fixture and setup helpers — see that
|
|
file's own module docstring for why (every existing test below the API
|
|
layer exercises one layer in isolation and so never walked this sequence).
|
|
|
|
Rather than hand-building an `AllocationInput` with `vacation_forfeiting`
|
|
pre-populated (as the allocator's own unit tests do), every scenario here
|
|
gets a REAL forfeiture forecast for free: a workspace with a child and no
|
|
`vacation_days` row entered for the current year defaults to the statutory
|
|
entitlement (`settings.rules.statutory_vacation_days_per_year`, 25) with
|
|
`is_assumed=True`, and with zero vacation used so far this year, the
|
|
FIFO carry-forward simulation (`AnalyticsService._vacation_year_series`)
|
|
forfeits `earned - min_vacation_days_per_year` (25 - 20 = 20) days for
|
|
*every* parent with no explicit override — see
|
|
`AllocationInputLoader.load`'s `vacation_forfeiting` computation and
|
|
`_Ledger.build_rescue_days`. No `_set_vacation` call for the current year is
|
|
needed (or wanted) anywhere in this file for that reason.
|
|
|
|
Two independent (parent, year) rescue groups are needed — one to accept
|
|
down to zero (proving "shrinks then disappears" via a live, never-dismissed
|
|
propose response) and one to dismiss (proving persistence-across-recompute
|
|
and the no-write guarantee) without the two actions masking each other's
|
|
observable state. A single-parent workspace can't do that (there's only one
|
|
group), so the main scenario adds a placeholder partner (`Sam`) — the same
|
|
`POST /api/workspace/{id}/parents` endpoint `test_user_journey.py`'s
|
|
`TestWorkspaceParentsRenameAndRemove` exercises — giving two independent
|
|
forfeiture forecasts (Alex's and Sam's) off ONE registration.
|
|
`/api/auth/register` is rate-limited to 10/minute and shares its in-memory
|
|
counter with every other test in the process (see `test_user_journey.py`'s
|
|
own docstrings on this) — that file alone already uses all 10 registrations
|
|
in a single run, leaving no budget for a second file to register anyone
|
|
else, so this file makes exactly ONE registration total (the workspace-
|
|
isolation guard below uses fresh random uuids instead of a second tenant —
|
|
see that section's comment for why that's equally valid).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from datetime import date
|
|
from typing import Any
|
|
from uuid import UUID, uuid4
|
|
|
|
import aiosqlite
|
|
from dateutil.relativedelta import relativedelta
|
|
from httpx import AsyncClient
|
|
|
|
from tests.integration.test_user_journey import ( # noqa: F401 (client is a fixture)
|
|
_add_child,
|
|
_list_planned_days,
|
|
_propose,
|
|
_register,
|
|
_sync,
|
|
client,
|
|
)
|
|
|
|
|
|
async def _dismiss(
|
|
http_client: AsyncClient,
|
|
headers: dict[str, str],
|
|
workspace_id: UUID,
|
|
parent_personal_data_uuid: UUID,
|
|
year: int,
|
|
) -> Any:
|
|
return await http_client.post(
|
|
"/api/plan/rescue-days/dismiss",
|
|
headers=headers,
|
|
json={
|
|
"workspace_id": str(workspace_id),
|
|
"parent_personal_data_uuid": str(parent_personal_data_uuid),
|
|
"year": year,
|
|
},
|
|
)
|
|
|
|
|
|
async def _apply_rescue_days(
|
|
http_client: AsyncClient,
|
|
headers: dict[str, str],
|
|
workspace_id: UUID,
|
|
rescue_days: list[dict[str, Any]],
|
|
) -> dict[str, Any]:
|
|
"""Accept a batch of `RescueDayResponse` entries as real vacation days —
|
|
|
|
`wish_id: None` (no wish produced these) and `is_pinned: True` (the
|
|
allocator must never retype a hand-accepted rescue day on a later sync).
|
|
"""
|
|
days = [
|
|
{
|
|
"date": r["date"],
|
|
"parent_personal_data_uuid": r["parent_personal_data_uuid"],
|
|
"child_id": r["child_id"],
|
|
"day_type": "vacation",
|
|
"is_dubbeldag": False,
|
|
"wish_id": None,
|
|
"is_pinned": True,
|
|
}
|
|
for r in rescue_days
|
|
]
|
|
resp = await http_client.post(
|
|
"/api/plan/apply",
|
|
headers=headers,
|
|
json={"workspace_id": str(workspace_id), "days": days, "replace": False},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
return resp.json()
|
|
|
|
|
|
def _rescue_group(proposal: dict[str, Any], parent_uuid: UUID, year: int) -> list[dict[str, Any]]:
|
|
return [
|
|
r
|
|
for r in proposal["rescue_days"]
|
|
if r["parent_personal_data_uuid"] == str(parent_uuid) and r["year"] == year
|
|
]
|
|
|
|
|
|
async def _planned_leave_rows(connection: aiosqlite.Connection) -> list[tuple[Any, ...]]:
|
|
"""Every `planned_leave_day` row, as a stable, comparable tuple list —
|
|
|
|
used to assert a dismiss call is byte-identical no-op at the DB layer
|
|
(defect 2), not just "the response looked like a no-op".
|
|
"""
|
|
cursor = await connection.execute("SELECT * FROM planned_leave_day ORDER BY id")
|
|
rows = await cursor.fetchall()
|
|
return [tuple(row) for row in rows]
|
|
|
|
|
|
class TestVacationRescueApiFlow:
|
|
"""One registration (Alex + placeholder Sam + one child) walks every
|
|
|
|
rescue-day sequence a real browser session would: two independent
|
|
(parent, year) forfeiture groups let the accept-to-zero flow (Sam) and
|
|
the dismiss flow (Alex) run without either masking the other's
|
|
observable state — see this module's docstring for why both are needed
|
|
and why neither `_set_vacation` call for the current year appears.
|
|
"""
|
|
|
|
async def test_accept_dismiss_and_isolation_sequence(
|
|
self,
|
|
client: AsyncClient, # noqa: F811 (pytest fixture injection needs this exact name)
|
|
connection: aiosqlite.Connection,
|
|
) -> None:
|
|
headers, ws_id, pd_alex = await _register(client, "vacation-rescue@example.com", "Alex")
|
|
birth_date = date.today() - relativedelta(months=6)
|
|
child_id = await _add_child(client, headers, ws_id, "Barn", birth_date)
|
|
|
|
add_resp = await client.post(
|
|
f"/api/workspace/{ws_id}/parents", headers=headers, json={"display_name": "Sam"}
|
|
)
|
|
assert add_resp.status_code == 201, add_resp.text
|
|
pd_sam = UUID(add_resp.json()["uuid"])
|
|
|
|
current_year = date.today().year
|
|
|
|
proposal = await _propose(client, headers, ws_id)
|
|
group_alex = _rescue_group(proposal, pd_alex, current_year)
|
|
group_sam = _rescue_group(proposal, pd_sam, current_year)
|
|
assert group_alex, "expected a real current-year vacation-rescue group for Alex"
|
|
assert group_sam, "expected a real current-year vacation-rescue group for Sam"
|
|
# Vacation is taken per-parent, not a shared family resource, so the
|
|
# two groups' dates may legitimately coincide (same ranking, same
|
|
# eligible calendar) — only the (parent, year) IDENTITY need be
|
|
# independent, which `_rescue_group`'s own filtering already proves.
|
|
|
|
# -----------------------------------------------------------------
|
|
# 1) Accept, batched; the group shrinks; it disappears at residual 0;
|
|
# an accepted day survives a later sync. Uses Sam's group, which
|
|
# is NEVER dismissed, so its live propose count directly reflects
|
|
# the real residual throughout (a dismissed group reads empty
|
|
# regardless of residual — see the dismiss flow below — so it
|
|
# can't be used to observe shrinking).
|
|
# -----------------------------------------------------------------
|
|
sam_total = len(group_sam)
|
|
assert sam_total >= 2, (
|
|
f"need at least 2 eligible rescue-day candidates for Sam to exercise a partial "
|
|
f"accept before full acceptance, got {sam_total} "
|
|
f"(too few weekdays left in {current_year}?)"
|
|
)
|
|
first_batch = group_sam[: sam_total // 2]
|
|
assert first_batch
|
|
|
|
apply_1 = await _apply_rescue_days(client, headers, ws_id, first_batch)
|
|
assert apply_1["accepted"] is True, apply_1
|
|
assert apply_1["created_count"] == len(first_batch)
|
|
|
|
proposal_2 = await _propose(client, headers, ws_id)
|
|
group_sam_2 = _rescue_group(proposal_2, pd_sam, current_year)
|
|
assert len(group_sam_2) == sam_total - len(first_batch), (
|
|
"the group must shrink by exactly the number of accepted days"
|
|
)
|
|
assert {r["date"] for r in group_sam_2}.isdisjoint({r["date"] for r in first_batch})
|
|
# Alex's group is untouched by anything done to Sam's.
|
|
assert len(_rescue_group(proposal_2, pd_alex, current_year)) == len(group_alex)
|
|
|
|
apply_2 = await _apply_rescue_days(client, headers, ws_id, group_sam_2)
|
|
assert apply_2["accepted"] is True, apply_2
|
|
assert apply_2["created_count"] == len(group_sam_2)
|
|
|
|
proposal_3 = await _propose(client, headers, ws_id)
|
|
assert _rescue_group(proposal_3, pd_sam, current_year) == [], (
|
|
"the group must disappear entirely once its residual reaches 0"
|
|
)
|
|
|
|
all_sam_dates = {r["date"] for r in group_sam}
|
|
planned = await _list_planned_days(client, headers, child_id)
|
|
sam_planned = {
|
|
d["date"]: d for d in planned if d["parent_personal_data_uuid"] == str(pd_sam)
|
|
}
|
|
assert set(sam_planned) == all_sam_dates, "every accepted rescue date must be persisted"
|
|
for row in sam_planned.values():
|
|
assert row["day_type"] == "vacation"
|
|
assert row["wish_id"] is None
|
|
assert row["is_pinned"] is True
|
|
|
|
# A following sync must leave the accepted (pinned) days untouched.
|
|
await _sync(client, headers, ws_id)
|
|
planned_after_sync = await _list_planned_days(client, headers, child_id)
|
|
sam_after_sync = {
|
|
d["date"]: d
|
|
for d in planned_after_sync
|
|
if d["parent_personal_data_uuid"] == str(pd_sam)
|
|
}
|
|
assert set(sam_after_sync) == all_sam_dates
|
|
for row in sam_after_sync.values():
|
|
assert row["day_type"] == "vacation"
|
|
assert row["is_pinned"] is True
|
|
|
|
# -----------------------------------------------------------------
|
|
# 2) Workspace isolation: a foreign workspace id or parent uuid is
|
|
# rejected and never touches the real (still fully intact,
|
|
# never-dismissed) Alex group. `check_workspace`/`check_member`
|
|
# (app/dependencies.py) 404 identically on a missing OR a
|
|
# genuinely foreign id BY DESIGN ("a foreign id can never be
|
|
# distinguished from a nonexistent one" — see their docstrings),
|
|
# so a fresh random uuid exercises exactly the same guard a real
|
|
# second tenant's id would, without a second `/api/auth/register`
|
|
# call: that endpoint is rate-limited to 10/minute and shares its
|
|
# in-memory counter with every other test in the process —
|
|
# `test_user_journey.py` alone already uses all 10 in one run, so
|
|
# this file keeps its own total to the ONE registration above.
|
|
# -----------------------------------------------------------------
|
|
foreign_workspace_resp = await _dismiss(client, headers, uuid4(), pd_alex, current_year)
|
|
assert foreign_workspace_resp.status_code == 404, foreign_workspace_resp.text
|
|
|
|
foreign_parent_resp = await _dismiss(client, headers, ws_id, uuid4(), current_year)
|
|
assert foreign_parent_resp.status_code == 404, foreign_parent_resp.text
|
|
|
|
proposal_after_guard = await _propose(client, headers, ws_id)
|
|
group_alex_after_guard = _rescue_group(proposal_after_guard, pd_alex, current_year)
|
|
assert {r["date"] for r in group_alex_after_guard} == {r["date"] for r in group_alex}, (
|
|
"a rejected dismiss attempt (foreign workspace or parent) must have no effect on "
|
|
"the real group"
|
|
)
|
|
|
|
# -----------------------------------------------------------------
|
|
# 3) A dismiss writes nothing at the DB layer (defect 2), and it
|
|
# persists across a recompute — including the exact repro step,
|
|
# a second propose after a sync (defect 1). Uses Alex's group.
|
|
# -----------------------------------------------------------------
|
|
before_rows = await _planned_leave_rows(connection)
|
|
|
|
legit_dismiss_resp = await _dismiss(client, headers, ws_id, pd_alex, current_year)
|
|
assert legit_dismiss_resp.status_code == 204, legit_dismiss_resp.text
|
|
assert legit_dismiss_resp.content == b""
|
|
|
|
after_rows = await _planned_leave_rows(connection)
|
|
assert after_rows == before_rows, (
|
|
"dismissing a rescue-day group must write NOTHING to planned_leave_day"
|
|
)
|
|
|
|
proposal_4 = await _propose(client, headers, ws_id)
|
|
assert _rescue_group(proposal_4, pd_alex, current_year) == [], (
|
|
"the dismissed group must not reappear on the very next propose"
|
|
)
|
|
|
|
await _sync(client, headers, ws_id)
|
|
|
|
proposal_5 = await _propose(client, headers, ws_id)
|
|
assert _rescue_group(proposal_5, pd_alex, current_year) == [], (
|
|
"Defect 1 repro: the dismissed group must stay gone through a propose -> sync -> "
|
|
"propose cycle, not resurface once the plan is recomputed"
|
|
)
|