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

Separate user intent from planner output via day assertions

Browse Source 
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
This commit is contained in:
Jacob Hagstedt
2026-09-21 07:20:14 +02:00
parent fc90cf9a5f
commit 782415122d
101 changed files with 9542 additions and 3930 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/services/leave_wish_service.py
+125 -38
View File
@@ -23,10 +23,13 @@ caller the occurrence belongs to a repeating series.
from datetime import date, datetime
from uuid import UUID, uuid4
from app.entities.day_assertion import DAY_TYPE_NONE
from app.entities.leave_wish import LeaveWish, project_window
from app.repositories.child_repository import ChildRepository
from app.repositories.day_assertion_repository import DayAssertionRepository
from app.repositories.leave_wish_repository import LeaveWishRepository
from app.repositories.planned_leave_repository import PlannedLeaveRepository
from app.repositories.unit_of_work import NoOpUnitOfWork, UnitOfWork
from app.utils.errors import NotFoundError, ValidationError
from app.utils.holidays import cached_year_bounds
@@ -67,6 +70,8 @@ class LeaveWishService:
repo: LeaveWishRepository,
planned_leave_repo: PlannedLeaveRepository,
child_repo: ChildRepository | None = None,
day_assertion_repo: DayAssertionRepository | None = None,
uow: UnitOfWork | None = None,
) -> None:
self._repo = repo
self._planned_leave_repo = planned_leave_repo
@@ -75,6 +80,26 @@ class LeaveWishService:
# open-ended ("until the child turns 4") series falls back to the
# childless horizon instead of the child's actual 4th birthday.
self._child_repo = child_repo
# Optional, like `child_repo` above — `None` means existing
# direct-construction call sites (older tests) keep today's
# behaviour: `delete()` still removes every `planned_leave_day` row,
# it just doesn't also clear the `DayAssertion` at each row's key.
# See `delete()`'s docstring for why that cleanup matters now.
self._day_assertion_repo = day_assertion_repo
# `delete()`/`_materialise_series()` are each a multi-statement
# sequence over possibly several rows (a series delete/re-projection
# touches every occurrence's `planned_leave_day` rows, `DayAssertion`
# rows, and the `LeaveWish` rows themselves) with no atomicity of
# their own — this service has never had a `uow`, so a crash midway
# left a partially-deleted/partially-reprojected series. Defaults to
# a no-op so existing direct-construction call sites (tests,
# `PlanningService.sync_with_wish`'s own nested call — nesting a
# `unit_of_work()` inside one already open is a safe passthrough,
# see `app.db.connection.unit_of_work`) keep working unchanged;
# `app.dependencies.get_leave_wish_service` wires a real
# `SQLiteUnitOfWork` for the standalone `/api/leave-wishes` router,
# which previously had none at all.
self._uow = uow or NoOpUnitOfWork()
async def create(
self,
@@ -165,11 +190,12 @@ class LeaveWishService:
created_at=now,
updated_at=now,
)
created = await self._repo.create(anchor)
if repeats and series_id is None:
end_year = await self._resolve_series_end_year(child_id, first_year, last_year)
for year in range(first_year + 1, end_year + 1):
await self._repo.create(_project_occurrence(created, year))
async with self._uow():
created = await self._repo.create(anchor)
if repeats and series_id is None:
end_year = await self._resolve_series_end_year(child_id, first_year, last_year)
for year in range(first_year + 1, end_year + 1):
await self._repo.create(_project_occurrence(created, year))
return created
async def get(self, leave_wish_id: UUID, workspace_id: UUID | None = None) -> LeaveWish:
@@ -328,32 +354,33 @@ class LeaveWishService:
wants_dubbeldag=entity.wants_dubbeldag,
)
siblings = (
[s for s in await self._repo.get_by_series(entity.series_id) if s.id != entity.id]
if entity.series_id is not None
else []
)
async with self._uow():
siblings = (
[s for s in await self._repo.get_by_series(entity.series_id) if s.id != entity.id]
if entity.series_id is not None
else []
)
if was_series and not will_repeat:
# "Stop repeating": keep the clicked occurrence, drop the rest.
entity.series_id = None
await self._delete_rows(siblings)
return await self._repo.update(entity.id, entity)
if was_series and not will_repeat:
# "Stop repeating": keep the clicked occurrence, drop the rest.
entity.series_id = None
await self._delete_rows(siblings)
return await self._repo.update(entity.id, entity)
if not will_repeat:
return await self._repo.update(entity.id, entity)
if not will_repeat:
return await self._repo.update(entity.id, entity)
if not was_series:
# "Make this repeat": this occurrence becomes the series anchor.
entity.series_id = uuid4()
if not was_series:
# "Make this repeat": this occurrence becomes the series anchor.
entity.series_id = uuid4()
anchor = await self._repo.update(entity.id, entity)
if was_series and scope != SCOPE_SERIES:
# A single year of an existing series changed; siblings keep
# whatever the user (or the planner) had already given them.
anchor = await self._repo.update(entity.id, entity)
if was_series and scope != SCOPE_SERIES:
# A single year of an existing series changed; siblings keep
# whatever the user (or the planner) had already given them.
return anchor
await self._materialise_series(anchor, siblings)
return anchor
await self._materialise_series(anchor, siblings)
return anchor
async def delete(
self,
@@ -361,8 +388,9 @@ class LeaveWishService:
workspace_id: UUID | None = None,
scope: str = SCOPE_OCCURRENCE,
) -> None:
"""Delete a wish occurrence (or a whole series) and every
`planned_leave_day` row it produced.
"""Delete a wish occurrence (or a whole series), every
`planned_leave_day` row it produced, and any `DayAssertion` locking
one of those rows in place.
Under the calendar-direct-manipulation UX a period *is* its wish: the
band the user drew, the days the allocator filled it with, and any
@@ -380,30 +408,89 @@ class LeaveWishService:
a signal that the day should outlive the period a user explicitly
chose to delete. This also matches the ad-hoc-period delete path
(`Calendar.handleDeletePeriod`'s no-`wishId` branch), which already
deletes every day in the band regardless of `is_pinned` with no
deletes every day in the band regardless of pin state with no
special-casing.
**Restructure step 3b — the `DayAssertion` cleanup.** A pinned or
promoted-and-locked row's assertion (`day_type` concrete — the
'attach'/'pin' day-lock actions, see `PlanningService._assert_pin`)
is cleared at the same key, for every row this call deletes.
Deliberately unconditional: nothing in the stored data distinguishes
an assertion the user made independently and *later* attached to
this wish ('attach') from one made *because* the day was already
under this wish ('pin') — both look identical (concrete `day_type`,
`wish_id` = this wish). Left alone, a surviving assertion would
become invisible to the frontend the moment its mirrored
`planned_leave_day` row is deleted above (nothing in the UI lists an
assertion with no row behind it) while still silently pinning
whatever new row a later sync/hand-pin happens to place at that same
(workspace, child, parent, date) — a slot the user cannot see, let
alone unlock, through the UI. Deleting it is the safe default: the
user can trivially re-pin a day they still want locked, which is a
far smaller cost than a permanently and invisibly stuck slot.
A `day_type='none'` assertion (an exclusion) is the one exception —
left untouched. An exclusion is never actually produced with this
wish's `wish_id` by any current write path (a `planned_leave_day`
row's `day_type` CHECK constraint doesn't allow `'none'`, so 'attach'/
'pin' can never assert one), but even if one existed, "the planner
may not place leave here" is a standing instruction about the DATE,
not about this particular period — see the restructure notes'
already-accepted "exclusions outlive the wish they were made
against" behaviour (keyed without `wish_id` on purpose).
`scope='series'` removes every year of the repeating wish this
occurrence belongs to; `'occurrence'` removes just this year's row,
which is now a plain delete rather than the range-narrowing (and
split-the-wish-in-two) contortion the single-row shape forced on the
frontend.
occurrence belongs to (and every occurrence's own rows/assertions,
not just the clicked year's); `'occurrence'` removes just this
year's row, which is now a plain delete rather than the
range-narrowing (and split-the-wish-in-two) contortion the
single-row shape forced on the frontend.
"""
if scope not in _VALID_SCOPE:
raise ValidationError(f"scope must be one of {_VALID_SCOPE}")
entity = await self.get(leave_wish_id, workspace_id)
targets = [entity]
if scope == SCOPE_SERIES and entity.series_id is not None:
targets = await self._repo.get_by_series(entity.series_id)
await self._delete_rows(targets)
async with self._uow():
entity = await self.get(leave_wish_id, workspace_id)
targets = [entity]
if scope == SCOPE_SERIES and entity.series_id is not None:
targets = await self._repo.get_by_series(entity.series_id)
await self._delete_rows(targets)
# -- internals ---------------------------------------------------------
async def _delete_rows(self, rows: list[LeaveWish]) -> None:
for row in rows:
await self._clear_assertions_for(row.id)
await self._planned_leave_repo.delete_by_wish(row.id)
await self._repo.delete(row.id)
async def _clear_assertions_for(self, wish_id: UUID) -> None:
"""Clear every `DayAssertion` attached to this wish — except a
`'none'` (exclusion) assertion, which outlives the wish on purpose.
See `delete()`'s docstring for the full reasoning.
Adversarial-review fix: queries `DayAssertionRepository.list_by_wish`
directly by `wish_id` rather than walking the wish's surviving
`planned_leave_day` rows and clearing at each row's natural key. The
row-walk was blind to a DANGLING assertion — one whose mirrored
`planned_leave_day` row had already gone missing (e.g. a crash
between `DayAssertionService`'s own assertion write and its mirror
write) — which then outlived the wish as a `wish_id=NULL` ad-hoc
assertion (FK `ON DELETE SET NULL`) that silently, permanently
locked its date with no UI path to find it. Querying by `wish_id`
catches that assertion regardless of whether a mirror row survives.
"""
if self._day_assertion_repo is None:
return
for assertion in await self._day_assertion_repo.list_by_wish(wish_id):
if assertion.day_type != DAY_TYPE_NONE:
await self._day_assertion_repo.delete_by_natural_key(
assertion.workspace_id,
assertion.child_id,
assertion.parent_personal_data_uuid,
assertion.date,
)
async def _materialise_series(self, anchor: LeaveWish, siblings: list[LeaveWish]) -> None:
"""Make the rows of `anchor`'s series agree with `anchor`.