Let a day removed from a period stay removed
Taking one day out of a period the planner placed did nothing that lasted: the period still asked for a full week, so the next sync put the day straight back. The option was disabled rather than lying about it. A removed day is now recorded as its own thing — no leave for this parent, this child, this date — and the planner honours it on every later sync. It is kept apart from the working-day overrides, which answer a different question about the calendar rather than about the plan. The day belongs to the parent whose band was clicked, so removing one parent's day leaves their partner's alone; where both parents were to be off on one funding arrangement, the partner is simply funded on their own instead. A removed day shortens what the period is asking for rather than counting as a day the plan failed to find, so the advice about closing a shortfall does not start arguing for a day nobody wants. Nothing is removed without a way back: the toast undoes it, and the day itself is marked in the calendar and offers to restore. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ei7gCZzdd751uZtPzb1Pep
This commit is contained in:
@@ -32,6 +32,7 @@ from app.repositories.day_override_repository import DayOverrideRepository
|
||||
from app.repositories.leave_wish_repository import LeaveWishRepository
|
||||
from app.repositories.parental_leave_repository import ParentalLeaveRepository
|
||||
from app.repositories.personal_data_repository import PersonalDataRepository
|
||||
from app.repositories.planned_day_exclusion_repository import PlannedDayExclusionRepository
|
||||
from app.repositories.planned_leave_repository import PlannedLeaveRepository
|
||||
from app.repositories.vacation_days_repository import VacationDaysRepository
|
||||
from app.repositories.workspace_repository import WorkspaceRepository
|
||||
@@ -267,6 +268,19 @@ class AllocationInput:
|
||||
# (the wish was reassigned) — matched by `wish_id`, not by parent, for
|
||||
# exactly that reason. Never used for resource netting or `_existing_lookup`.
|
||||
all_existing_planned: list[PlannedLeaveDay] | None = None
|
||||
# §remove-this-day — every (parent_uuid, child_id, date) the user has
|
||||
# explicitly removed from the plan (see `PlannedDayExclusion`). The
|
||||
# allocator must never place leave on one of these, however emphatically
|
||||
# the wish covering it asks: "removing a single day stays permanent, even
|
||||
# on an auto-added period day". Empty by default, so every hand-built
|
||||
# `AllocationInput` in the test suite keeps producing byte-identical
|
||||
# output. Read only via `_Ledger._is_excluded` /
|
||||
# `_Ledger._excluded_window_dates`; deliberately NOT merged into
|
||||
# `day_overrides`/`is_free_day`, which answer the unrelated *calendar*
|
||||
# question "is this date a working day for this workspace" (a removed
|
||||
# Tuesday is still a working Tuesday, for analytics and for everyone
|
||||
# else in the workspace).
|
||||
excluded_days: frozenset[tuple[UUID, UUID, date]] = frozenset()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -929,6 +943,78 @@ class _Ledger:
|
||||
|
||||
# -- resource spending ------------------------------------------------
|
||||
|
||||
# -- user-removed days (§remove-this-day) ------------------------------
|
||||
|
||||
def _is_excluded(self, parent_uuid: UUID, child_id: UUID | None, d: date) -> bool:
|
||||
"""Whether the user removed THIS date from THIS parent's plan.
|
||||
|
||||
The single reader of `AllocationInput.excluded_days`. A wish with no
|
||||
resolved child (`child_id is None`) can never match: an exclusion is
|
||||
always recorded from a real `planned_leave_day`, which always has a
|
||||
child.
|
||||
"""
|
||||
if child_id is None or not self._input.excluded_days:
|
||||
return False
|
||||
return (parent_uuid, child_id, d) in self._input.excluded_days
|
||||
|
||||
def _excluded_window_dates(
|
||||
self, wish: LeaveWish, responsible: _Responsible, dates: list[date]
|
||||
) -> set[date]:
|
||||
"""Dates that leave a window ENTIRELY because no responsible parent may take them.
|
||||
|
||||
The exclusion itself is per parent (see `PlannedDayExclusionService`
|
||||
for why), but what a *window* should do about it depends on how many
|
||||
parents that window puts on leave per date:
|
||||
|
||||
- 'dubbeldag' (both parents off on the same dates): removing one
|
||||
parent's day must leave the partner's standing, so a date only
|
||||
drops out when BOTH parents were removed from it. The single-parent
|
||||
case is handled inside `_fund_both`/`_fund_solo`, which skip just
|
||||
the excluded parent.
|
||||
- 'solo'/'swap' (exactly one parent on leave per date): ANY
|
||||
responsible parent's exclusion empties the date. Without the `any`,
|
||||
`_fund_swap`'s leftover hand-off would hand the removed date
|
||||
straight to the partner — silently refilling the hole the user just
|
||||
made, under a different name, which is precisely the "it came
|
||||
back" failure this whole feature exists to prevent.
|
||||
|
||||
Dropping these dates from the list `fund_window` funds (and from the
|
||||
list it measures) is also the shortfall decision: a day the user
|
||||
removed reduces the window's target rather than counting as a day the
|
||||
plan failed to fund. It is not a gap the planner could do anything
|
||||
about, `_build_remedies` must not propose freeing resources for it,
|
||||
and `sync_plan`'s eviction preview (which fires on a stale row whose
|
||||
own wish still reports `shortfall_days > 0`) must not mistake a
|
||||
deliberate removal for a lost priority race.
|
||||
"""
|
||||
if not self._input.excluded_days or not responsible.parents:
|
||||
return set()
|
||||
child = self._resolve_wish_child(wish)
|
||||
if child is None:
|
||||
return set()
|
||||
quantifier = all if responsible.mode == "dubbeldag" else any
|
||||
return {
|
||||
d
|
||||
for d in dates
|
||||
if quantifier(self._is_excluded(p.uuid, child.id, d) for p in responsible.parents)
|
||||
}
|
||||
|
||||
def _window_dates(
|
||||
self, wish: LeaveWish, responsible: _Responsible, start: date, end: date
|
||||
) -> list[date]:
|
||||
"""`[start, end]` minus the dates the user removed from it.
|
||||
|
||||
Shared by `fund_window` (the real placement) and
|
||||
`_candidate_shortfall` (§Decision C's budget-aware probe) so the two
|
||||
keep measuring the same window — see `_shortfall_and_weeks_funded`'s
|
||||
docstring for why they must never silently disagree.
|
||||
"""
|
||||
all_dates = _dates_in(start, end)
|
||||
excluded = self._excluded_window_dates(wish, responsible, all_dates)
|
||||
if not excluded:
|
||||
return all_dates
|
||||
return [d for d in all_dates if d not in excluded]
|
||||
|
||||
def _spend_vacation(self, parent_uuid: UUID, d: date) -> bool:
|
||||
key = (parent_uuid, d.year)
|
||||
budget = self._vacation.get(key)
|
||||
@@ -1079,6 +1165,17 @@ class _Ledger:
|
||||
pool_key = (parent.uuid, child_id) if child_id is not None else None
|
||||
|
||||
for d in dates:
|
||||
if self._is_excluded(parent.uuid, child_id, d):
|
||||
# §remove-this-day — the user removed this exact date for
|
||||
# this exact parent. Never re-derive it, and emit nothing at
|
||||
# all (not even a 'free' day): `fund_window`/
|
||||
# `_candidate_shortfall` have already taken the date out of
|
||||
# the window's target wherever no responsible parent may take
|
||||
# it, so it is neither funded nor a gap. This is also what
|
||||
# stops `_fund_swap`'s leftover hand-off from handing a
|
||||
# parent a date they were removed from.
|
||||
continue
|
||||
|
||||
key = (parent.uuid, child_id, d)
|
||||
existing_type = self._existing_lookup.get(key)
|
||||
if existing_type is not None:
|
||||
@@ -1336,6 +1433,10 @@ class _Ledger:
|
||||
else REASON_FREE_WEEKEND
|
||||
)
|
||||
for red_parent, red_pool in ((parent_a, pool_a), (parent_b, pool_b)):
|
||||
if self._is_excluded(red_parent.uuid, child.id, d):
|
||||
# Removed for this parent — the partner still gets
|
||||
# their free day for this date.
|
||||
continue
|
||||
fp_on_red_day = (
|
||||
self._spend_fp((red_parent.uuid, child.id), prefer="sjukpenning")
|
||||
if self._input.take_pl_on_red_days and red_pool is not None
|
||||
@@ -1363,6 +1464,19 @@ class _Ledger:
|
||||
)
|
||||
continue
|
||||
|
||||
if self._is_excluded(parent_a.uuid, child.id, d) or self._is_excluded(
|
||||
parent_b.uuid, child.id, d
|
||||
):
|
||||
# §remove-this-day — one parent was removed from this date
|
||||
# (never both: `fund_window` already dropped those dates from
|
||||
# `dates` entirely). Dubbeldagar and co-leave each spend for
|
||||
# BOTH parents, so neither can fund a date only one parent may
|
||||
# take. Hand it to the swap path, which funds it for whichever
|
||||
# parent is still allowed to have it — `_fund_solo` skips the
|
||||
# excluded one — so the partner keeps their day.
|
||||
leftover_for_swap.append(d)
|
||||
continue
|
||||
|
||||
funded = False
|
||||
|
||||
if (
|
||||
@@ -1473,7 +1587,13 @@ class _Ledger:
|
||||
wish = w.wish
|
||||
responsible = self._resolve_parents(wish)
|
||||
|
||||
all_dates = _dates_in(w.start, w.end)
|
||||
# §remove-this-day — the window's dates minus the ones the user
|
||||
# removed outright (see `_excluded_window_dates`, which also explains
|
||||
# why an excluded date shrinks the target instead of counting as a
|
||||
# shortfall). The red-day cluster is still scanned over the FULL
|
||||
# range: klämdag detection is a property of the calendar, not of what
|
||||
# this window happens to fund.
|
||||
all_dates = self._window_dates(wish, responsible, w.start, w.end)
|
||||
red_cluster = _red_day_cluster_dates(
|
||||
w.start, w.end, self._input.holidays, self._input.day_overrides
|
||||
)
|
||||
@@ -1826,7 +1946,7 @@ class _Ledger:
|
||||
candidate is funded for real afterwards through the ordinary
|
||||
`fund_window()` path, exactly as before this fix.
|
||||
"""
|
||||
dates = _dates_in(candidate_start, candidate_end)
|
||||
dates = self._window_dates(wish, responsible, candidate_start, candidate_end)
|
||||
red_cluster = _red_day_cluster_dates(
|
||||
candidate_start, candidate_end, self._input.holidays, self._input.day_overrides
|
||||
)
|
||||
@@ -2490,6 +2610,7 @@ class AllocationInputLoader:
|
||||
planned_leave_repo: PlannedLeaveRepository,
|
||||
leave_wish_repo: LeaveWishRepository,
|
||||
day_override_repo: DayOverrideRepository | None = None,
|
||||
exclusion_repo: PlannedDayExclusionRepository | None = None,
|
||||
) -> None:
|
||||
self._workspace_repo = workspace_repo
|
||||
self._children_repo = child_repo
|
||||
@@ -2499,6 +2620,7 @@ class AllocationInputLoader:
|
||||
self._planned_leave_repo = planned_leave_repo
|
||||
self._leave_wish_repo = leave_wish_repo
|
||||
self._day_override_repo = day_override_repo
|
||||
self._exclusion_repo = exclusion_repo
|
||||
|
||||
async def load(
|
||||
self,
|
||||
@@ -2686,6 +2808,18 @@ class AllocationInputLoader:
|
||||
overrides = await self._day_override_repo.list_by_workspace(workspace_id)
|
||||
day_overrides = {o.date: o.is_working_day for o in overrides}
|
||||
|
||||
# §remove-this-day — the dates the user removed by hand. Loaded
|
||||
# unconditionally (never narrowed by `only_pinned_existing`): a
|
||||
# removal is an instruction about the plan, not a planned row, so a
|
||||
# re-plan must see every one of them or the removed day comes back on
|
||||
# the very next sync — which is the bug this exists to fix.
|
||||
excluded_days: frozenset[tuple[UUID, UUID, date]] = frozenset()
|
||||
if self._exclusion_repo is not None:
|
||||
exclusions = await self._exclusion_repo.list_by_workspace(workspace_id)
|
||||
excluded_days = frozenset(
|
||||
(e.parent_personal_data_uuid, e.child_id, e.date) for e in exclusions
|
||||
)
|
||||
|
||||
return AllocationInput(
|
||||
today=today,
|
||||
horizon_end=horizon_end,
|
||||
@@ -2700,6 +2834,7 @@ class AllocationInputLoader:
|
||||
day_overrides=day_overrides,
|
||||
take_pl_on_red_days=ws.take_pl_on_red_days,
|
||||
all_existing_planned=all_existing_planned,
|
||||
excluded_days=excluded_days,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
"""Business logic for planned-day exclusions.
|
||||
|
||||
The service owns rules; the repository only persists. Routers call services,
|
||||
not repositories directly. Mirrors `DayOverrideService`'s
|
||||
upsert-by-natural-key shape: a workspace has at most one exclusion per
|
||||
(child, parent, date).
|
||||
|
||||
**Why the parent is part of the key.** A both-parents period places one
|
||||
`planned_leave_day` per parent on the same date, and the calendar draws one
|
||||
band per parent. "Remove this day" is always clicked on ONE band, so it
|
||||
removes ONE parent's day; keying the exclusion on (workspace, child, date)
|
||||
alone would make removing a day from one parent's band silently cancel the
|
||||
partner's leave that day too — a change the user never asked for and, on a
|
||||
shared plan, one the other parent would discover only by noticing the hole.
|
||||
The UI says whose day it removed (`calendar.periodDeleteConfirmDay` /
|
||||
`calendar.toastDayExcluded` name the parent). A date only leaves a window
|
||||
entirely when no responsible parent may take it — see
|
||||
`_Ledger._excluded_window_dates`.
|
||||
|
||||
**Removing the day itself.** Creating an exclusion also deletes the matching
|
||||
`planned_leave_day` row, if one exists, so "remove this day" is true the
|
||||
moment the call returns rather than only after the next
|
||||
`POST /api/plan/sync`. The subsequent sync (which the calendar runs anyway)
|
||||
is what keeps the rest of the plan consistent; the allocator will not
|
||||
re-derive this date, which is the whole point of the row.
|
||||
"""
|
||||
|
||||
from datetime import date, datetime
|
||||
from uuid import UUID, uuid4
|
||||
|
||||
from app.entities.planned_day_exclusion import PlannedDayExclusion
|
||||
from app.repositories.planned_day_exclusion_repository import PlannedDayExclusionRepository
|
||||
from app.repositories.planned_leave_repository import PlannedLeaveRepository
|
||||
from app.repositories.unit_of_work import NoOpUnitOfWork, UnitOfWork
|
||||
from app.utils.errors import NotFoundError
|
||||
|
||||
|
||||
class PlannedDayExclusionService:
|
||||
def __init__(
|
||||
self,
|
||||
repo: PlannedDayExclusionRepository,
|
||||
planned_leave_repo: PlannedLeaveRepository | None = None,
|
||||
uow: UnitOfWork | None = None,
|
||||
) -> None:
|
||||
self._repo = repo
|
||||
self._planned_leave_repo = planned_leave_repo
|
||||
self._uow = uow or NoOpUnitOfWork()
|
||||
|
||||
async def exclude(
|
||||
self,
|
||||
workspace_id: UUID,
|
||||
child_id: UUID,
|
||||
parent_personal_data_uuid: UUID,
|
||||
day: date,
|
||||
) -> PlannedDayExclusion:
|
||||
"""Create (or return) the exclusion, and drop the day it removes.
|
||||
|
||||
Idempotent: excluding an already-excluded day returns the existing
|
||||
row rather than raising, so a double click (or a retried request)
|
||||
cannot produce a UNIQUE violation.
|
||||
"""
|
||||
async with self._uow():
|
||||
existing = await self._repo.get_by_natural_key(
|
||||
workspace_id, child_id, parent_personal_data_uuid, day
|
||||
)
|
||||
if existing is not None:
|
||||
await self._delete_planned_day(
|
||||
workspace_id, child_id, parent_personal_data_uuid, day
|
||||
)
|
||||
return existing
|
||||
|
||||
entity = PlannedDayExclusion(
|
||||
id=uuid4(),
|
||||
workspace_id=workspace_id,
|
||||
child_id=child_id,
|
||||
parent_personal_data_uuid=parent_personal_data_uuid,
|
||||
date=day,
|
||||
created_at=datetime.now(),
|
||||
updated_at=datetime.now(),
|
||||
)
|
||||
created = await self._repo.create(entity)
|
||||
await self._delete_planned_day(workspace_id, child_id, parent_personal_data_uuid, day)
|
||||
return created
|
||||
|
||||
async def _delete_planned_day(
|
||||
self,
|
||||
workspace_id: UUID,
|
||||
child_id: UUID,
|
||||
parent_personal_data_uuid: UUID,
|
||||
day: date,
|
||||
) -> None:
|
||||
"""Remove the `planned_leave_day` this exclusion forbids, if it exists.
|
||||
|
||||
A pinned row is deleted too: the user just said, explicitly and for
|
||||
this exact date, that they do not want leave here — that instruction
|
||||
is newer than the pin. Undo removes the exclusion and re-syncs, which
|
||||
restores the day as an ordinary planner-placed (unpinned) day.
|
||||
"""
|
||||
if self._planned_leave_repo is None:
|
||||
return
|
||||
rows = await self._planned_leave_repo.list_by_workspace_child(
|
||||
workspace_id, child_id, from_date=day, to_date=day
|
||||
)
|
||||
ids = [r.id for r in rows if r.parent_personal_data_uuid == parent_personal_data_uuid]
|
||||
if ids:
|
||||
await self._planned_leave_repo.delete_many(ids)
|
||||
|
||||
async def get(
|
||||
self, exclusion_id: UUID, workspace_id: UUID | None = None
|
||||
) -> PlannedDayExclusion:
|
||||
entity = await self._repo.get_by_id(exclusion_id)
|
||||
if entity is None or (workspace_id is not None and entity.workspace_id != workspace_id):
|
||||
raise NotFoundError(f"PlannedDayExclusion {exclusion_id} not found")
|
||||
return entity
|
||||
|
||||
async def list_for_workspace(self, workspace_id: UUID) -> list[PlannedDayExclusion]:
|
||||
return await self._repo.list_by_workspace(workspace_id)
|
||||
|
||||
async def delete(self, exclusion_id: UUID, workspace_id: UUID | None = None) -> None:
|
||||
"""Un-exclude: the planner may place a day here again on the next sync."""
|
||||
await self.get(exclusion_id, workspace_id) # raises NotFoundError if missing/foreign
|
||||
await self._repo.delete(exclusion_id)
|
||||
|
||||
async def get_excluded_keys(self, workspace_id: UUID) -> frozenset[tuple[UUID, UUID, date]]:
|
||||
"""Return {(parent_uuid, child_id, date)} for every exclusion in a workspace.
|
||||
|
||||
This is the shape `AllocationInputLoader` feeds into
|
||||
`AllocationInput.excluded_days` — see `_Ledger._is_excluded`, the
|
||||
single place it is consulted.
|
||||
"""
|
||||
rows = await self._repo.list_by_workspace(workspace_id)
|
||||
return frozenset((r.parent_personal_data_uuid, r.child_id, r.date) for r in rows)
|
||||
Reference in New Issue
Block a user