782415122d
The calendar mixed two kinds of truth in one table. `planned_leave_day` was simultaneously the allocator's output cache, the user's override store (`is_pinned`, plus ad-hoc rows with no `wish_id`), and the resource ledger that analytics and the age-4 maths read. The planner therefore had to ask "am I allowed to touch this row?" on every sync, and got it wrong at the edges. Every user assertion is now one `day_assertion` row — a concrete `day_type` replaces a pinned day, a row with no `wish_id` replaces an ad-hoc day, and `day_type='none'` replaces `planned_day_exclusion`. `planned_leave_day` becomes purely derived with a single writer, and `PlanningService.pinned_keys()` is the single answer to "is this day pinned", used by the allocator's loader, by `sync_plan` and by the read router alike. On top of that, every calendar band is now a real event: an ad-hoc band opens the same full editor as a wish-backed one and saving it promotes it to a `LeaveWish`, so it can be named, repeated yearly, assigned to either or both parents, and handed to the planner. "Planering" is a `moveable` toggle plus a from/to month bound plus a separate lock/unlock action on the days themselves, replacing a three-rung ladder whose "planner decides" rung set a search window equal to the drawn dates and so could never move anything. Bugs found and fixed along the way: - The plan erased its own history. Fixed windows clip to today, so an elapsed auto-placed day fell into `stale_keys` and was deleted on the next sync — and a background sync runs on every workspace load. Past rows are now frozen and counted as spent. - Migration 010's downgrade silently destroyed every pinned day and exclusion (`is_pinned` returned as DEFAULT 0, the exclusion table came back empty, exit 0, no warning). It now reconstructs both from `day_assertion`, and 009 itemises what it cannot recover before dropping the table. - Deleting a period left its assertions behind, locking those dates forever with no UI path to clear them. Wish deletion now clears them by `wish_id`. - `DayAssertionService` and `LeaveWishService` committed multi-statement writes without a transaction; a crash mid-write diverged the tables or half-deleted a series. Both now run in a unit of work. - A sibling year's shortfall was never surfaced: the toast only inspected the edited occurrence's outcome, so a fully unfunded later year rendered as an empty band with no warning. - A flexible window whose bound had entirely elapsed was dropped from the proposal with no outcome at all. It now reports as `unplaced`. - Unlocking a period deleted its days instead of returning them to the planner. - The e2e global setup posted to a route deleted in this change, which would have failed the whole suite before its first test. Gates: backend 938 tests / 91% coverage, frontend 1351 / 97.4 | 92.2 | 94.37 | 97.4. Migrations 009 and 010 audited against a hand-built realistic database: the upgrade reconciles exactly and a constraint violation fails safe, leaving the schema at 008. Not complete. Step 4's toggle, month picker and lock/unlock action are verified by browser walk and unit tests, but the partial-lock readout was not exercised. `ideas.md` §7 adds a full plan for the vacation-forfeiture defect (the planner detects expiring days and then does nothing about them) — not implemented here. Remaining smaller items are listed in the session's restructure notes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4wjgbVL4HAC5Rdaw75VbV
242 lines
9.4 KiB
Python
242 lines
9.4 KiB
Python
"""Tests for the unit-of-work mechanism (`app.db.connection`) and the
|
|
batched `PlannedLeaveRepository` methods it exists to support.
|
|
|
|
See `scratchpad/audit/backend-perf.md` §3: the app runs on one shared
|
|
connection, so a multi-statement write needs (a) mutual exclusion for the
|
|
whole unit of work and (b) the per-row `commit()` calls suppressed while one
|
|
is in progress. These tests exercise that mechanism directly against an
|
|
in-memory connection, plus the `PlanningService.apply_proposed_days`
|
|
atomicity it is meant to guarantee end-to-end.
|
|
"""
|
|
|
|
import asyncio
|
|
from datetime import date, datetime
|
|
from uuid import uuid4
|
|
|
|
import aiosqlite
|
|
import pytest
|
|
|
|
from app.db.connection import commit, ensure_schema, unit_of_work
|
|
from app.entities.leave_wish import LeaveWish
|
|
from app.entities.planned_leave import PlannedLeaveDay
|
|
from app.repositories_impl.sqlite.leave_wish_repository import SQLiteLeaveWishRepository
|
|
from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository
|
|
from app.services.planning_service import PlanningService, ProposedDayInput
|
|
from tests.conftest import SeededIds
|
|
|
|
|
|
@pytest.fixture
|
|
async def uow_connection() -> aiosqlite.Connection:
|
|
conn = await aiosqlite.connect(":memory:")
|
|
conn.row_factory = aiosqlite.Row
|
|
await conn.execute("PRAGMA foreign_keys = ON;")
|
|
await ensure_schema(conn)
|
|
try:
|
|
yield conn
|
|
finally:
|
|
await conn.close()
|
|
|
|
|
|
async def _workspace_count(conn: aiosqlite.Connection) -> int:
|
|
cursor = await conn.execute("SELECT COUNT(*) FROM workspace")
|
|
row = await cursor.fetchone()
|
|
return int(row[0])
|
|
|
|
|
|
async def _insert_workspace(conn: aiosqlite.Connection, workspace_id: str) -> None:
|
|
await conn.execute(
|
|
"INSERT INTO workspace (id, name, created_at) VALUES (?, ?, ?)",
|
|
(workspace_id, "Test", datetime.now().isoformat()),
|
|
)
|
|
|
|
|
|
class TestUnitOfWork:
|
|
async def test_commits_on_success(self, uow_connection: aiosqlite.Connection) -> None:
|
|
async with unit_of_work(uow_connection):
|
|
await _insert_workspace(uow_connection, "ws-commit")
|
|
assert await _workspace_count(uow_connection) == 1
|
|
|
|
async def test_rolls_back_on_exception(self, uow_connection: aiosqlite.Connection) -> None:
|
|
with pytest.raises(ValueError):
|
|
async with unit_of_work(uow_connection):
|
|
await _insert_workspace(uow_connection, "ws-rollback")
|
|
raise ValueError("simulated failure")
|
|
assert await _workspace_count(uow_connection) == 0
|
|
|
|
async def test_commit_helper_is_noop_inside_uow(
|
|
self, uow_connection: aiosqlite.Connection
|
|
) -> None:
|
|
"""`commit()` must not durably commit inside a unit of work — otherwise
|
|
a later rollback in the same block could only undo writes made after
|
|
that no-op `commit()` call, not before it (see backend-perf audit §3,
|
|
"Is BEGIN/COMMIT ... safe on the shared connection? No, not alone.").
|
|
"""
|
|
with pytest.raises(RuntimeError):
|
|
async with unit_of_work(uow_connection):
|
|
await _insert_workspace(uow_connection, "ws-noop-commit")
|
|
await commit(uow_connection) # must be a no-op here
|
|
raise RuntimeError("still rolls back everything, including the row above")
|
|
assert await _workspace_count(uow_connection) == 0
|
|
|
|
async def test_concurrent_calls_serialize(self, uow_connection: aiosqlite.Connection) -> None:
|
|
"""Two concurrent `unit_of_work()` blocks on the same connection never
|
|
interleave — the second blocks at entry until the first fully exits.
|
|
"""
|
|
events: list[str] = []
|
|
|
|
async def worker(name: str, hold: float) -> None:
|
|
async with unit_of_work(uow_connection):
|
|
events.append(f"{name}-start")
|
|
await asyncio.sleep(hold)
|
|
events.append(f"{name}-end")
|
|
|
|
await asyncio.gather(worker("a", 0.05), worker("b", 0))
|
|
|
|
assert events in (
|
|
["a-start", "a-end", "b-start", "b-end"],
|
|
["b-start", "b-end", "a-start", "a-end"],
|
|
)
|
|
|
|
|
|
class TestPlannedLeaveRepositoryBatchMethods:
|
|
async def test_create_many_then_delete_many(
|
|
self,
|
|
planned_leave_repo: SQLitePlannedLeaveRepository,
|
|
seeded_ids: SeededIds,
|
|
) -> None:
|
|
now = datetime.now()
|
|
entities = [
|
|
PlannedLeaveDay(
|
|
id=uuid4(),
|
|
workspace_id=seeded_ids.workspace_id,
|
|
child_id=seeded_ids.child_id,
|
|
parent_personal_data_uuid=seeded_ids.personal_data_uuid,
|
|
date=date(2026, 3, day),
|
|
day_type="sjukpenning",
|
|
created_at=now,
|
|
updated_at=now,
|
|
)
|
|
for day in (1, 2, 3)
|
|
]
|
|
|
|
await planned_leave_repo.create_many(entities)
|
|
rows = await planned_leave_repo.list(workspace_id=seeded_ids.workspace_id)
|
|
assert len(rows) == 3
|
|
|
|
deleted = await planned_leave_repo.delete_many([entities[0].id, entities[1].id])
|
|
assert deleted == 2
|
|
remaining = await planned_leave_repo.list(workspace_id=seeded_ids.workspace_id)
|
|
assert [r.id for r in remaining] == [entities[2].id]
|
|
|
|
async def test_create_many_empty_list_is_noop(
|
|
self,
|
|
planned_leave_repo: SQLitePlannedLeaveRepository,
|
|
seeded_ids: SeededIds,
|
|
) -> None:
|
|
await planned_leave_repo.create_many([])
|
|
assert await planned_leave_repo.list(workspace_id=seeded_ids.workspace_id) == []
|
|
|
|
async def test_delete_many_is_idempotent(
|
|
self, planned_leave_repo: SQLitePlannedLeaveRepository
|
|
) -> None:
|
|
"""Unlike `delete()`, a missing id is silently ignored rather than raising."""
|
|
deleted = await planned_leave_repo.delete_many([uuid4(), uuid4()])
|
|
assert deleted == 0
|
|
|
|
async def test_update_metadata_many(
|
|
self,
|
|
planned_leave_repo: SQLitePlannedLeaveRepository,
|
|
leave_wish_repo: SQLiteLeaveWishRepository,
|
|
seeded_ids: SeededIds,
|
|
) -> None:
|
|
now = datetime.now()
|
|
entity = PlannedLeaveDay(
|
|
id=uuid4(),
|
|
workspace_id=seeded_ids.workspace_id,
|
|
child_id=seeded_ids.child_id,
|
|
parent_personal_data_uuid=seeded_ids.personal_data_uuid,
|
|
date=date(2026, 4, 1),
|
|
day_type="vacation",
|
|
created_at=now,
|
|
updated_at=now,
|
|
)
|
|
await planned_leave_repo.create(entity)
|
|
|
|
wish = await leave_wish_repo.create(
|
|
LeaveWish(
|
|
id=uuid4(),
|
|
workspace_id=seeded_ids.workspace_id,
|
|
child_id=seeded_ids.child_id,
|
|
name="Test wish",
|
|
window_start_month=4,
|
|
window_start_day=1,
|
|
window_end_month=4,
|
|
window_end_day=30,
|
|
target_weeks=1,
|
|
first_year=2026,
|
|
last_year=None,
|
|
priority=1,
|
|
parent_mode="either",
|
|
parent_personal_data_uuid=None,
|
|
window_mode="fixed",
|
|
created_at=now,
|
|
updated_at=now,
|
|
)
|
|
)
|
|
|
|
await planned_leave_repo.update_metadata_many([(entity.id, wish.id)])
|
|
|
|
refreshed = await planned_leave_repo.get_by_id(entity.id)
|
|
assert refreshed is not None
|
|
assert refreshed.wish_id == wish.id
|
|
# day_type/is_dubbeldag are untouched by the metadata-only update.
|
|
assert refreshed.day_type == "vacation"
|
|
|
|
|
|
class TestApplyProposedDaysAtomicity:
|
|
async def test_mid_batch_failure_leaves_nothing_persisted(
|
|
self,
|
|
planning_service_with_uow: PlanningService,
|
|
planned_leave_repo: SQLitePlannedLeaveRepository,
|
|
seeded_ids: SeededIds,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""A batch that replaces an existing row (delete + create, both inside
|
|
stage 4) must not leave the delete applied if the create half fails —
|
|
the whole `async with self._uow():` block must roll back together.
|
|
Uses `planning_service_with_uow` (a real `SQLiteUnitOfWork`), not the
|
|
default `planning_service` fixture, which wires a no-op unit of work
|
|
and would (correctly, given a no-op) leave the delete committed.
|
|
"""
|
|
existing = await planning_service_with_uow.add_planned_day(
|
|
workspace_id=seeded_ids.workspace_id,
|
|
child_id=seeded_ids.child_id,
|
|
parent_personal_data_uuid=seeded_ids.personal_data_uuid,
|
|
date=date(2026, 5, 1),
|
|
day_type="sjukpenning",
|
|
)
|
|
|
|
async def boom(_items: list[PlannedLeaveDay]) -> None:
|
|
raise RuntimeError("simulated mid-batch failure")
|
|
|
|
monkeypatch.setattr(planned_leave_repo, "create_many", boom)
|
|
|
|
days = [
|
|
ProposedDayInput(
|
|
date=date(2026, 5, 1),
|
|
parent_personal_data_uuid=seeded_ids.personal_data_uuid,
|
|
child_id=seeded_ids.child_id,
|
|
day_type="lagsta", # differs from the existing row -> collision -> to_replace
|
|
)
|
|
]
|
|
|
|
with pytest.raises(RuntimeError):
|
|
await planning_service_with_uow.apply_proposed_days(
|
|
workspace_id=seeded_ids.workspace_id, days=days, replace=True
|
|
)
|
|
|
|
remaining = await planned_leave_repo.list(workspace_id=seeded_ids.workspace_id)
|
|
assert len(remaining) == 1
|
|
assert remaining[0].id == existing.id
|
|
assert remaining[0].day_type == "sjukpenning"
|