Check vacation days against the entitlement before persisting them

Applied batches were validated against the remaining föräldrapenning tiers
but never against semester. That held only while the allocator was the sole
writer, since its ledger spends vacation against a per-year budget. The
calendar is about to let a user force any day to semester and pin it, which
writes straight past that ledger, so nothing would have stopped forty pinned
vacation days against a twenty-five day entitlement.

The check mirrors the allocator's own derivation rather than restating it:
available days are total plus days saved in, and a year with no entered row
falls back to the statutory entitlement instead of reading as zero. The
semesterlagen floor stays out of it, being a bound on what may be carried
forward rather than a cap on what may be spent. Vacation is budgeted per
calendar year, so a period crossing New Year is charged to both.
This commit is contained in:
2026-09-17 18:22:55 +02:00
parent c1aa4ae0b0
commit d06c8c8449
6 changed files with 466 additions and 10 deletions
+4 -1
View File
@@ -307,9 +307,12 @@ def get_planning_service(
repo: PlannedLeaveRepositoryDep,
analytics_service: AnalyticsServiceDep,
parental_leave_repo: ParentalLeaveRepositoryDep,
vacation_repo: VacationDaysRepositoryDep,
planning_engine_service: PlanningEngineServiceDep,
) -> PlanningService:
return PlanningService(repo, analytics_service, parental_leave_repo, planning_engine_service)
return PlanningService(
repo, analytics_service, parental_leave_repo, vacation_repo, planning_engine_service
)
PlanningServiceDep = Annotated[PlanningService, Depends(get_planning_service)]
+111
View File
@@ -10,6 +10,7 @@ from app.config import settings
from app.entities.planned_leave import PlannedLeaveDay
from app.repositories.parental_leave_repository import ParentalLeaveRepository
from app.repositories.planned_leave_repository import PlannedLeaveRepository
from app.repositories.vacation_days_repository import VacationDaysRepository
from app.services.allocation_service import PlanningEngineService, PlanProposal
from app.services.analytics_service import AnalyticsService
from app.utils.errors import DubbeldagError, NotFoundError, ValidationError
@@ -136,11 +137,13 @@ class PlanningService:
repo: PlannedLeaveRepository,
analytics_service: AnalyticsService,
parental_leave_repo: ParentalLeaveRepository,
vacation_repo: VacationDaysRepository,
planning_engine_service: PlanningEngineService | None = None,
) -> None:
self._repo = repo
self._analytics = analytics_service
self._parental_leave_repo = parental_leave_repo
self._vacation_repo = vacation_repo
# Only needed by `sync_plan` (POST /api/plan/sync) — optional so
# existing callers (scripts/seed_demo.py, older fixtures) that only
# ever use the per-day CRUD/`apply_proposed_days` paths don't need to
@@ -471,6 +474,9 @@ class PlanningService:
domain_errors.extend(
await self._validate_remaining_balance(pending, to_replace, delete_rows)
)
domain_errors.extend(
await self._validate_remaining_vacation_balance(pending, to_replace, delete_rows)
)
if domain_errors:
return PlanApplyResult(accepted=False, collisions=collisions, rejections=domain_errors)
@@ -858,3 +864,108 @@ class PlanningService:
)
)
return errors
async def _vacation_days_left(self, parent_uuid: UUID, year: int) -> int:
"""Remaining vacation (semester) days for one parent in one calendar year.
Vacation is budgeted per (parent, calendar year) — unlike the FP
tiers above, which are per (parent, child) — because a semesterlagen
entitlement doesn't care which child a day off is attributed to.
Mirrors `AllocationInputLoader.load`'s vacation-budget derivation in
`allocation_service.py` exactly: `available = total_days +
saved_days_in - already_used`, where `total_days`/`saved_days_in`
come from the `VacationDays` row for (parent, year) if one exists.
A year with **no** row does NOT mean zero entitlement — the loader
defaults `total_days` to `rules.statutory_vacation_days_per_year`
(25) and flags the result `is_assumed=True`; this does the same
default (there is no `is_assumed` to carry here since this is a
plain remaining-days count, not a `VacationBudget`).
`PersonalData.min_vacation_days_per_year` is deliberately NOT used
here: per `ParentFacts.min_vacation_days_per_year`'s docstring and
the allocator's own carry-forward remedy, it is a semesterlagen
floor on days that must be *taken* each year (bounding what may be
saved forward), not a cap on how much may be spent — turning it into
a spending cap here would reject perfectly legal plans.
Like `_tier_pool_left` above, "already used" counts every currently
persisted 'vacation' row for the (parent, year), pinned or not — this
intentionally does NOT mirror the pure `_Ledger`'s pinned-day
exclusion (that exclusion exists there only because the `_Ledger`
debits a pinned day's cost itself in a separate step); this check has
no such separate step, so it must count everything actually
persisted, the same "whatever is committed reduces what's left"
shape the sjukpenning/lagsta check above already uses.
"""
rules = settings.rules
row = await self._vacation_repo.get_by_parent_year(parent_uuid, year)
if row is not None:
total_available = row.total_days + row.saved_days_in
else:
total_available = rules.statutory_vacation_days_per_year
committed = [
d
for d in await self._repo.list(parent_personal_data_uuid=parent_uuid)
if d.day_type == "vacation" and d.date.year == year
]
return max(total_available - len(committed), 0)
async def _validate_remaining_vacation_balance(
self,
pending: list[ProposedDayInput],
to_replace: list[tuple[PlannedLeaveDay, ProposedDayInput]],
delete_rows: list[PlannedLeaveDay],
) -> list[Rejection]:
"""The batch must not spend more vacation days than remain for a year.
Grouped by (parent, calendar year) rather than (parent, child, tier)
— see `_vacation_days_left`. Crediting `to_replace`/`delete_rows`
back before checking follows the exact same shape as
`_validate_remaining_balance` above, including for a period that
wraps New Year (e.g. Jullov, 20 Dec - 16 Jan): each `ProposedDayInput`
is grouped under its own `date.year`, so a band that crosses the
boundary is automatically split into two independently-checked
(parent, year) budgets — no special-casing needed here.
"""
errors: list[Rejection] = []
year_pairs = {
(d.parent_personal_data_uuid, d.date.year) for d in pending if d.day_type == "vacation"
}
for parent_uuid, year in year_pairs:
left = await self._vacation_days_left(parent_uuid, year)
added = sum(
1
for d in pending
if d.parent_personal_data_uuid == parent_uuid
and d.day_type == "vacation"
and d.date.year == year
)
removed = sum(
1
for old, new in to_replace
if new.parent_personal_data_uuid == parent_uuid
and old.day_type == "vacation"
and new.date.year == year
) + sum(
1
for row in delete_rows
if row.parent_personal_data_uuid == parent_uuid
and row.day_type == "vacation"
and row.date.year == year
)
net_new = added - removed
if net_new > left:
errors.append(
Rejection(
message_key="planApply.rejection.insufficientVacationBalance",
data={
"parent_personal_data_uuid": str(parent_uuid),
"year": year,
"needed": net_new,
"remaining": left,
},
)
)
return errors
+1 -1
View File
@@ -137,7 +137,7 @@ def _build_services(conn: aiosqlite.Connection) -> DemoServices:
day_override=DayOverrideService(day_overrides),
leave_wish=LeaveWishService(leave_wishes),
manual_fetch=ManualFetchService(parental_leave),
planning=PlanningService(planned_leave, analytics, parental_leave),
planning=PlanningService(planned_leave, analytics, parental_leave, vacation_repo),
users=users,
personal_data=personal_data,
workspaces=workspaces,
+6 -1
View File
@@ -298,8 +298,13 @@ async def planning_service(
planned_leave_repo: SQLitePlannedLeaveRepository,
analytics_service: AnalyticsService,
parental_leave_repo: SQLiteParentalLeaveRepository,
vacation_repo: SQLiteVacationDaysRepository,
planning_engine_service: PlanningEngineService,
) -> PlanningService:
return PlanningService(
planned_leave_repo, analytics_service, parental_leave_repo, planning_engine_service
planned_leave_repo,
analytics_service,
parental_leave_repo,
vacation_repo,
planning_engine_service,
)
@@ -597,14 +597,17 @@ class TestApplyProposedDaysDeleteKeys:
planned_leave_repo: SQLitePlannedLeaveRepository,
seeded_ids: SeededIds,
) -> None:
"""`apply_proposed_days` places no balance cap on 'vacation' today (no
"""This batch nets to zero vacation days for 2026 (1 deleted, 1 added),
`_validate_remaining_balance` tier exists for it), so there is no cap
to "credit back" here the way there is for sjukpenning/lagsta. This
just confirms the general `delete_keys` write/validate machinery
handles a vacation-typed deletion correctly (deleted + new rows both
land right), since nothing else in this file exercises that day_type
through the delete path.
which fits comfortably under the statutory default entitlement (no
`VacationDays` row exists for this parent/year) -- so it exercises
the general `delete_keys` write/validate machinery for a vacation-
typed deletion, but does not by itself prove the vacation balance cap
(`_validate_remaining_vacation_balance`) is enforced or that its
`delete_keys` credit path matters. See
`test_planning_service_vacation.py::TestVacationBalanceDeleteKeysCredit`
for a tight-budget version that actually would reject without the
credit.
"""
ws, child, parent = (
seeded_ids.workspace_id,
@@ -0,0 +1,334 @@
"""Tests for `PlanningService`'s vacation-day balance validation.
Closes the gap where `_validate_remaining_balance` checked `sjukpenning`/
`lagsta` but never `vacation`, letting a caller persist unlimited vacation
days. Vacation is budgeted per (parent, calendar year) -- not per (parent,
child) like the FP tiers -- and mirrors `AllocationInputLoader.load`'s
`VacationBudget` derivation in `allocation_service.py`:
available = total_days + saved_days_in - already_used
with a year that has no `VacationDays` row defaulting `total_days` to
`rules.statutory_vacation_days_per_year` rather than zero ("not entered"
must never be read as "no entitlement").
"""
from datetime import date, datetime, timedelta
from uuid import uuid4
from app.config import settings
from app.entities.vacation_days import VacationDays
from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository
from app.repositories_impl.sqlite.vacation_days_repository import SQLiteVacationDaysRepository
from app.services.planning_service import PlanningService, ProposedDayInput
from tests.conftest import SeededIds
def _make_vacation(
personal_data_uuid,
workspace_id,
year: int,
total_days: int,
saved_days_in: int = 0,
) -> VacationDays:
now = datetime.now()
return VacationDays(
id=uuid4(),
personal_data_uuid=personal_data_uuid,
workspace_id=workspace_id,
year=year,
total_days=total_days,
saved_days_in=saved_days_in,
created_at=now,
updated_at=now,
)
class TestVacationBalanceWithEnteredEntitlement:
async def test_vacation_within_entitlement_accepted(
self,
planning_service: PlanningService,
vacation_repo: SQLiteVacationDaysRepository,
seeded_ids: SeededIds,
) -> None:
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
await vacation_repo.create(_make_vacation(parent, ws, 2026, total_days=5))
days = [ProposedDayInput(date(2026, 6, i), parent, child, "vacation") for i in range(1, 6)]
result = await planning_service.apply_proposed_days(ws, days)
assert result.accepted is True, result.rejections
assert result.created_count == 5
async def test_vacation_beyond_entitlement_rejected_whole(
self,
planning_service: PlanningService,
planned_leave_repo: SQLitePlannedLeaveRepository,
vacation_repo: SQLiteVacationDaysRepository,
seeded_ids: SeededIds,
) -> None:
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
await vacation_repo.create(_make_vacation(parent, ws, 2026, total_days=5))
days = [
ProposedDayInput(date(2026, 6, i), parent, child, "vacation") for i in range(1, 7)
] # 6 days, only 5 entitled
result = await planning_service.apply_proposed_days(ws, days)
assert result.accepted is False
assert any(
r.message_key == "planApply.rejection.insufficientVacationBalance"
and r.data["year"] == 2026
and r.data["parent_personal_data_uuid"] == str(parent)
and r.data["needed"] == 6
and r.data["remaining"] == 5
for r in result.rejections
)
rows = await planned_leave_repo.list_by_workspace_child(
workspace_id=ws, child_id=child, from_date=date(2026, 6, 1), to_date=date(2026, 6, 30)
)
assert rows == [], "a rejected batch must write nothing"
class TestVacationBalanceWithNoEnteredYear:
async def test_missing_year_defaults_to_statutory_entitlement_not_zero(
self,
planning_service: PlanningService,
seeded_ids: SeededIds,
) -> None:
"""No `VacationDays` row for 2026 at all -- must fall back to
`rules.statutory_vacation_days_per_year`, never to zero.
"""
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
statutory = settings.rules.statutory_vacation_days_per_year
exactly_statutory = [
ProposedDayInput(date(2026, 1, 1) + timedelta(days=i), parent, child, "vacation")
for i in range(statutory)
]
result = await planning_service.apply_proposed_days(ws, exactly_statutory)
assert result.accepted is True, result.rejections
async def test_missing_year_one_day_over_statutory_default_rejected(
self,
planning_service: PlanningService,
seeded_ids: SeededIds,
) -> None:
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
statutory = settings.rules.statutory_vacation_days_per_year
one_too_many = [
ProposedDayInput(date(2026, 1, 1) + timedelta(days=i), parent, child, "vacation")
for i in range(statutory + 1)
]
result = await planning_service.apply_proposed_days(ws, one_too_many)
assert result.accepted is False
assert any(
r.message_key == "planApply.rejection.insufficientVacationBalance"
and r.data["year"] == 2026
for r in result.rejections
)
class TestVacationBalanceCountsSavedDaysIn:
"""5 this year's entitlement + 3 carried forward from prior years = 8
available. Each test starts from a fresh (unspent) budget -- unlike
`TestVacationBalanceWithEnteredEntitlement` above, these two are kept
independent (not one accept-then-reject sequence in a single test)
specifically so the second assertion's `remaining` reflects the
*original* 8-day budget, not 8 minus whatever a prior call in the same
test already persisted.
"""
async def test_exactly_available_including_saved_days_in_accepted(
self,
planning_service: PlanningService,
vacation_repo: SQLiteVacationDaysRepository,
seeded_ids: SeededIds,
) -> None:
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
await vacation_repo.create(_make_vacation(parent, ws, 2026, total_days=5, saved_days_in=3))
exactly_available = [
ProposedDayInput(date(2026, 6, i), parent, child, "vacation") for i in range(1, 9)
]
result = await planning_service.apply_proposed_days(ws, exactly_available)
assert result.accepted is True, result.rejections
assert result.created_count == 8
async def test_one_more_than_saved_days_in_allows_rejected(
self,
planning_service: PlanningService,
vacation_repo: SQLiteVacationDaysRepository,
seeded_ids: SeededIds,
) -> None:
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
await vacation_repo.create(_make_vacation(parent, ws, 2026, total_days=5, saved_days_in=3))
one_more = [
ProposedDayInput(date(2026, 6, i), parent, child, "vacation") for i in range(1, 10)
]
result_over = await planning_service.apply_proposed_days(ws, one_more)
assert result_over.accepted is False
assert any(
r.message_key == "planApply.rejection.insufficientVacationBalance"
and r.data["needed"] == 9
and r.data["remaining"] == 8
for r in result_over.rejections
)
class TestVacationBalanceDeleteKeysCredit:
async def test_delete_and_readd_same_count_same_year_accepted(
self,
planning_service: PlanningService,
planned_leave_repo: SQLitePlannedLeaveRepository,
vacation_repo: SQLiteVacationDaysRepository,
seeded_ids: SeededIds,
) -> None:
"""A tight 1-day budget already fully spent -- adding one more
vacation day would be rejected outright, but replacing (delete +
re-add, same count, same year) must be accepted because the deletion
is credited back before the new demand is checked.
"""
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
await vacation_repo.create(_make_vacation(parent, ws, 2026, total_days=1))
old = await planning_service.add_planned_day(
workspace_id=ws,
child_id=child,
parent_personal_data_uuid=parent,
date=date(2026, 6, 1),
day_type="vacation",
)
# Sanity: with the budget already fully spent, a bare addition (no
# delete) of one more vacation day in the same year is rejected.
bare_addition = [ProposedDayInput(date(2026, 6, 2), parent, child, "vacation")]
blocked = await planning_service.apply_proposed_days(ws, bare_addition)
assert blocked.accepted is False
assert any(
r.message_key == "planApply.rejection.insufficientVacationBalance"
for r in blocked.rejections
)
new_days = [ProposedDayInput(date(2026, 6, 2), parent, child, "vacation")]
delete_keys = {(child, parent, date(2026, 6, 1))}
result = await planning_service.apply_proposed_days(ws, new_days, delete_keys=delete_keys)
assert result.accepted is True, result.rejections
assert result.deleted_count == 1
assert result.created_count == 1
rows = await planned_leave_repo.list_by_workspace_child(
workspace_id=ws, child_id=child, from_date=date(2026, 6, 1), to_date=date(2026, 6, 30)
)
assert {r.date for r in rows} == {date(2026, 6, 2)}
assert old.id not in {r.id for r in rows}
class TestVacationBalanceNewYearWrap:
async def test_wrapping_period_charges_each_year_independently(
self,
planning_service: PlanningService,
planned_leave_repo: SQLitePlannedLeaveRepository,
vacation_repo: SQLiteVacationDaysRepository,
seeded_ids: SeededIds,
) -> None:
"""A Jullov-shaped period (20 Dec - 16 Jan) spans two vacation-year
budgets. 2026 has ample room (12 days needed, 15 entitled); 2027 is
deliberately too tight (16 days needed, only 10 entitled) -- the
whole batch must be rejected, and the rejection must name 2027, not
2026 or a summed total across both years.
"""
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
await vacation_repo.create(_make_vacation(parent, ws, 2026, total_days=15))
await vacation_repo.create(_make_vacation(parent, ws, 2027, total_days=10))
dec_days = [
ProposedDayInput(date(2026, 12, d), parent, child, "vacation") for d in range(20, 32)
] # 12 days
jan_days = [
ProposedDayInput(date(2027, 1, d), parent, child, "vacation") for d in range(1, 17)
] # 16 days
batch = dec_days + jan_days
result = await planning_service.apply_proposed_days(ws, batch)
assert result.accepted is False
rejections_by_year = {
r.data["year"]: r
for r in result.rejections
if r.message_key == "planApply.rejection.insufficientVacationBalance"
}
assert 2027 in rejections_by_year
assert rejections_by_year[2027].data["needed"] == 16
assert rejections_by_year[2027].data["remaining"] == 10
assert 2026 not in rejections_by_year, "2026 has room and must not be rejected"
rows = await planned_leave_repo.list_by_workspace_child(
workspace_id=ws, child_id=child, from_date=date(2026, 12, 1), to_date=date(2027, 1, 31)
)
assert rows == [], "a rejected batch must write nothing, including the in-budget year"
async def test_wrapping_period_accepted_when_both_years_have_room(
self,
planning_service: PlanningService,
vacation_repo: SQLiteVacationDaysRepository,
seeded_ids: SeededIds,
) -> None:
ws, child, parent = (
seeded_ids.workspace_id,
seeded_ids.child_id,
seeded_ids.personal_data_uuid,
)
await vacation_repo.create(_make_vacation(parent, ws, 2026, total_days=15))
await vacation_repo.create(_make_vacation(parent, ws, 2027, total_days=20))
dec_days = [
ProposedDayInput(date(2026, 12, d), parent, child, "vacation") for d in range(20, 32)
]
jan_days = [
ProposedDayInput(date(2027, 1, d), parent, child, "vacation") for d in range(1, 17)
]
batch = dec_days + jan_days
result = await planning_service.apply_proposed_days(ws, batch)
assert result.accepted is True, result.rejections
assert result.created_count == 28