From c1aa4ae0b042d5882672ef24dece01eb447b12dd Mon Sep 17 00:00:00 2001 From: Jacob Hagstedt Date: Thu, 17 Sep 2026 18:12:31 +0200 Subject: [PATCH] Credit days freed by the same sync before checking the balance A sync deleted the rows a re-plan had dropped only after apply_proposed_days had already validated, so capacity being freed in the very same call was never counted. Shortening one period and lengthening another while near the pool cap read as pure net-new demand and was refused outright, writing nothing and losing the edit silently. apply_proposed_days now takes the keys it should delete, credits those rows back to their tier and to the dubbeldagar quota during the up-front checks, and performs the deletions in its own write phase. Validation still runs fully before any write, which is what SQLite's lack of a cross-request transaction requires. --- backend/app/models/plan.py | 2 + backend/app/services/planning_service.py | 140 +++++++-- .../tests/unit/test_planning_service_apply.py | 295 ++++++++++++++++++ .../tests/unit/test_planning_service_sync.py | 99 ++++++ 4 files changed, 505 insertions(+), 31 deletions(-) diff --git a/backend/app/models/plan.py b/backend/app/models/plan.py index 433189d..e4d3edb 100644 --- a/backend/app/models/plan.py +++ b/backend/app/models/plan.py @@ -314,6 +314,7 @@ class PlanApplyResponse(BaseModel): skipped_identical_count: int skipped_free_count: int replaced_count: int + deleted_count: int collisions: list[DayCollisionResponse] rejections: list[RejectionResponse] created_days: list[PlannedLeaveDayResponse] @@ -326,6 +327,7 @@ class PlanApplyResponse(BaseModel): skipped_identical_count=result.skipped_identical_count, skipped_free_count=result.skipped_free_count, replaced_count=result.replaced_count, + deleted_count=result.deleted_count, collisions=[DayCollisionResponse.from_domain(c) for c in result.collisions], rejections=[RejectionResponse.from_domain(r) for r in result.rejections], created_days=[PlannedLeaveDayResponse.from_entity(d) for d in result.created], diff --git a/backend/app/services/planning_service.py b/backend/app/services/planning_service.py index bec4b03..94172e9 100644 --- a/backend/app/services/planning_service.py +++ b/backend/app/services/planning_service.py @@ -84,6 +84,7 @@ class PlanApplyResult: skipped_identical_count: int = 0 skipped_free_count: int = 0 replaced_count: int = 0 + deleted_count: int = 0 collisions: list[DayCollision] = field(default_factory=list) rejections: list[Rejection] = field(default_factory=list) @@ -277,6 +278,7 @@ class PlanningService: workspace_id: UUID, days: list[ProposedDayInput], replace: bool = False, + delete_keys: set[tuple[UUID, UUID, date]] | None = None, ) -> PlanApplyResult: """Persist a batch of proposed days as `planned_leave_day` rows. @@ -292,13 +294,29 @@ class PlanningService: `replace=True` to overwrite those specific collisions (every other collision-free day in the batch is written too). + `delete_keys` names rows (by their `(child_id, parent_personal_ + data_uuid, date)` identity) to delete as part of this same call — + e.g. `sync_plan` uses it for rows the freshly-computed plan no longer + accounts for at all. A key must not also appear in `days`; that is a + structural error (a batch cannot both write and delete the same + slot). Deletions are folded into the same up-front validation as the + creates/replaces: the tier (or dubbeldagar slot) a deleted row frees + is credited back before checking whether the batch's new demand + fits, so a sync that frees N days and re-spends N days in the same + call is validated against the *post-sync* balance, not a stale + pre-write snapshot. A key with no matching row (already gone) is + silently ignored rather than treated as an error. + Validates the entire batch up front (day types, per-day dubbeldagar window, batch-wide dubbeldagar quota, child eligibility window, - remaining sjukpenning/lagsta balance) and only writes if everything - passes — SQLite gives us no cross-request transaction here, so this - explicit validate-then-write split is what keeps a rejection from - ever leaving partial writes behind. + remaining sjukpenning/lagsta balance, credited for anything in + `delete_keys`) and only writes if everything passes — SQLite gives us + no cross-request transaction here, so this explicit validate-then- + write split is what keeps a rejection from ever leaving partial + writes behind. """ + delete_keys = delete_keys or set() + # -- stage 1: structural validation ------------------------------ structural_errors: list[Rejection] = [] for i, d in enumerate(days): @@ -336,17 +354,33 @@ class PlanningService: ) deduped[key] = d + for key in delete_keys: + if key in deduped: + child_id, parent_uuid, day = key + structural_errors.append( + Rejection( + message_key="planApply.rejection.deleteKeyConflict", + data={ + "date": day.isoformat(), + "child_id": str(child_id), + "parent_personal_data_uuid": str(parent_uuid), + }, + ) + ) + if structural_errors: return PlanApplyResult(accepted=False, rejections=structural_errors) unique_days = list(deduped.values()) - if not unique_days: + if not unique_days and not delete_keys: return PlanApplyResult(accepted=True, skipped_free_count=skipped_free_count) # -- stage 2: classify against existing rows (collision detection) -- by_child: dict[UUID, list[date]] = {} for d in unique_days: by_child.setdefault(d.child_id, []).append(d.date) + for child_id, _parent_uuid, day in delete_keys: + by_child.setdefault(child_id, []).append(day) existing_by_key: dict[tuple[UUID, UUID, date], PlannedLeaveDay] = {} for child_id, dates in by_child.items(): @@ -359,6 +393,12 @@ class PlanningService: for row in existing_rows: existing_by_key[(row.child_id, row.parent_personal_data_uuid, row.date)] = row + # Rows named by `delete_keys` that actually exist — a key with no + # matching row (already gone) is silently ignored. + delete_rows: list[PlannedLeaveDay] = [ + existing_by_key[key] for key in delete_keys if key in existing_by_key + ] + to_create: list[ProposedDayInput] = [] to_replace: list[tuple[PlannedLeaveDay, ProposedDayInput]] = [] to_update_metadata: list[tuple[PlannedLeaveDay, ProposedDayInput]] = [] @@ -407,7 +447,7 @@ class PlanningService: ) pending = to_create + [new for _, new in to_replace] - if not pending and not to_update_metadata: + if not pending and not to_update_metadata and not delete_rows: # Nothing left to write — either an empty batch or a fully # idempotent re-apply. return PlanApplyResult( @@ -417,10 +457,20 @@ class PlanningService: ) # -- stage 3: domain validation on the batch that would actually write + # + # `delete_rows` is threaded into both checks below so a row this same + # call is about to delete is credited back to its tier/quota BEFORE + # the new demand is checked against what's left — the bug this + # `delete_keys` parameter exists to fix was validating against a + # pre-write snapshot that hadn't yet had that capacity freed. domain_errors: list[str] = [] - domain_errors.extend(await self._validate_dubbeldagar_batch(pending, to_replace)) + domain_errors.extend( + await self._validate_dubbeldagar_batch(pending, to_replace, delete_rows) + ) domain_errors.extend(await self._validate_eligible_range(pending)) - domain_errors.extend(await self._validate_remaining_balance(pending, to_replace)) + domain_errors.extend( + await self._validate_remaining_balance(pending, to_replace, delete_rows) + ) if domain_errors: return PlanApplyResult(accepted=False, collisions=collisions, rejections=domain_errors) @@ -429,6 +479,9 @@ class PlanningService: for old, _ in to_replace: await self._repo.delete(old.id) + for row in delete_rows: + await self._repo.delete(row.id) + now = datetime.now() created: list[PlannedLeaveDay] = [] for d in pending: @@ -459,6 +512,7 @@ class PlanningService: skipped_identical_count=skipped_identical_count, skipped_free_count=skipped_free_count, replaced_count=len(to_replace), + deleted_count=len(delete_rows), ) # -- sync (POST /api/plan/sync) --------------------------------------- @@ -477,11 +531,16 @@ class PlanningService: # place (the allocator never emits anything for an already-occupied # key) -- "no collision ever arises" is a property of the allocator, not # something this method has to special-case in the write path. The one - # extra step this method DOES own is deleting rows that fall out of the - # freshly-computed plan entirely (a wish's window moved away from a date - # it used to occupy, or the wish was deleted) -- `apply_proposed_days` - # only ever creates/replaces/updates keys present in its own batch, so - # nothing else would ever clean those up. + # extra thing this method computes beyond `proposal.days` is which rows + # fall out of the freshly-computed plan entirely (a wish's window moved + # away from a date it used to occupy, or the wish was deleted) -- + # `apply_proposed_days` only ever creates/replaces/updates keys present + # in its own batch, so nothing else would identify those. Those keys are + # handed to `apply_proposed_days` as `delete_keys` rather than deleted + # here directly, so the capacity they free is credited back during that + # same call's up-front validation — deleting them first (or after) would + # let a sync that frees N days and re-spends N days read the freed days + # as still-spent net-new demand and reject the whole batch. async def sync_plan(self, workspace_id: UUID, today: date | None = None) -> PlanSyncResult: """Propose, persist, and diff — the single call the direct- @@ -523,26 +582,25 @@ class PlanningService: and (d.child_id, d.parent_personal_data_uuid, d.date) not in pinned_keys ] + batch_keys = {(d.child_id, d.parent_personal_data_uuid, d.date) for d in batch} + # Rows the freshly-computed plan no longer accounts for at all: + # unpinned, wish-attributed, and absent from this run's batch — e.g. + # the wish was deleted, or its window moved elsewhere. Passed as + # `delete_keys` so `apply_proposed_days` validates the deletions + # together with the creates/replaces (see the comment above this + # method) rather than deleting them itself afterwards. + stale_keys = { + key + for key, row in before_by_key.items() + if not row.is_pinned and row.wish_id is not None and key not in batch_keys + } + apply_result = await self.apply_proposed_days( - workspace_id=workspace_id, days=batch, replace=True + workspace_id=workspace_id, days=batch, replace=True, delete_keys=stale_keys ) changes: list[PlanChange] = [] if apply_result.accepted: - batch_keys = {(d.child_id, d.parent_personal_data_uuid, d.date) for d in batch} - # Rows the freshly-computed plan no longer accounts for at all: - # unpinned, wish-attributed, and absent from this run's batch — - # e.g. the wish was deleted, or its window moved elsewhere. - # `apply_proposed_days` never deletes a key it wasn't handed, so - # this is the one bit of write logic sync owns beyond it. - stale = [ - row - for key, row in before_by_key.items() - if not row.is_pinned and row.wish_id is not None and key not in batch_keys - ] - for row in stale: - await self._repo.delete(row.id) - after = await self._repo.list(workspace_id=workspace_id) after_by_key: dict[tuple[UUID, UUID, date], PlannedLeaveDay] = { (d.child_id, d.parent_personal_data_uuid, d.date): d for d in after @@ -623,13 +681,17 @@ class PlanningService: self, pending: list[ProposedDayInput], to_replace: list[tuple[PlannedLeaveDay, ProposedDayInput]], + delete_rows: list[PlannedLeaveDay], ) -> list[Rejection]: """Per-day window check (reused from AnalyticsService) + batch-wide quota. `AnalyticsService.validate_dubbeldagar` only sees rows already committed to the DB, so it cannot by itself catch a batch that introduces more than `max_dubbeldagar` new dubbeldagar in one go — - that aggregate check is done here, on top of it. + that aggregate check is done here, on top of it. `delete_rows` + (rows this same call is about to delete, via `delete_keys`) free + their dubbeldagar slot just like a `to_replace` old row does, so + they're credited back the same way when projecting the total. """ errors: list[Rejection] = [] dubbeldag_days = [d for d in pending if d.is_dubbeldag] @@ -650,6 +712,11 @@ class PlanningService: replaced_out_dubbeldag_by_child[old.child_id] = ( replaced_out_dubbeldag_by_child.get(old.child_id, 0) + 1 ) + for row in delete_rows: + if row.is_dubbeldag: + replaced_out_dubbeldag_by_child[row.child_id] = ( + replaced_out_dubbeldag_by_child.get(row.child_id, 0) + 1 + ) by_child: dict[UUID, int] = {} for d in dubbeldag_days: @@ -738,12 +805,17 @@ class PlanningService: self, pending: list[ProposedDayInput], to_replace: list[tuple[PlannedLeaveDay, ProposedDayInput]], + delete_rows: list[PlannedLeaveDay], ) -> list[Rejection]: """The batch must not spend more sjukpenning/lagsta days than remain. Per (parent, child, tier): net_new = (new days of that tier the batch - adds) - (days of that tier being replaced away, which free up a - slot). Rejects if net_new exceeds what's left. + adds) - (days of that tier being freed: replaced away, via + `to_replace`, or deleted outright via `delete_rows`/`delete_keys`). + Rejects if net_new exceeds what's left. Crediting `delete_rows` here + is what lets a `sync_plan` call that frees N days and re-spends N + days elsewhere pass validation against the balance *after* those + deletions, instead of rejecting against a stale pre-write snapshot. """ errors: list[Rejection] = [] pairs = {(d.parent_personal_data_uuid, d.child_id) for d in pending} @@ -764,6 +836,12 @@ class PlanningService: if new.parent_personal_data_uuid == parent_uuid and new.child_id == child_id and old.day_type == tier + ) + sum( + 1 + for row in delete_rows + if row.parent_personal_data_uuid == parent_uuid + and row.child_id == child_id + and row.day_type == tier ) net_new = added - removed if net_new > left: diff --git a/backend/tests/unit/test_planning_service_apply.py b/backend/tests/unit/test_planning_service_apply.py index 47203d9..92abfab 100644 --- a/backend/tests/unit/test_planning_service_apply.py +++ b/backend/tests/unit/test_planning_service_apply.py @@ -14,6 +14,7 @@ from uuid import uuid4 from app.entities.leave_wish import LeaveWish from app.entities.parental_leave import ParentalLeaveFetch, TierData +from app.entities.planned_leave import PlannedLeaveDay from app.repositories_impl.sqlite.leave_wish_repository import SQLiteLeaveWishRepository from app.repositories_impl.sqlite.parental_leave_repository import SQLiteParentalLeaveRepository from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository @@ -497,3 +498,297 @@ class TestApplyProposedDaysValidation: workspace_id=ws, child_id=child, from_date=date(2026, 6, 1), to_date=date(2026, 6, 30) ) assert rows == [] + + +class TestApplyProposedDaysDeleteKeys: + """`delete_keys` folds explicit deletions into the same validated batch as + + the creates/replaces -- this is the fix for the bug where `sync_plan` + deleted stale rows only AFTER `apply_proposed_days` had already validated + (and written) the fresh batch, so capacity a deletion frees was never + credited back before the balance/quota check ran. + """ + + async def test_delete_keys_credit_sjukpenning_tier_before_checking_balance( + self, + planning_service: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + parental_leave_repo: SQLiteParentalLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + ws, child, parent = ( + seeded_ids.workspace_id, + seeded_ids.child_id, + seeded_ids.personal_data_uuid, + ) + await parental_leave_repo.create( + _make_fetch(parent, ws, child, sjukpenning_left=5, lagsta_left=90) + ) + 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="sjukpenning", + ) + # 5 left, 1 already committed -> 4 free right now. Ask for 5 new + # sjukpenning days while deleting the 1 old one in the SAME call: + # net_new = 5 - 1 = 4, which fits -- but only if the deletion is + # credited before the check runs. + new_days = [ + ProposedDayInput(date(2026, 6, i), parent, child, "sjukpenning") for i in range(2, 7) + ] + 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 == 5 + + 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, i) for i in range(2, 7)} + assert old.id not in {r.id for r in rows} + + async def test_delete_keys_credit_lagsta_tier_before_checking_balance( + self, + planning_service: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + parental_leave_repo: SQLiteParentalLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + ws, child, parent = ( + seeded_ids.workspace_id, + seeded_ids.child_id, + seeded_ids.personal_data_uuid, + ) + await parental_leave_repo.create( + _make_fetch(parent, ws, child, sjukpenning_left=390, lagsta_left=5) + ) + 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="lagsta", + ) + new_days = [ + ProposedDayInput(date(2026, 6, i), parent, child, "lagsta") for i in range(2, 7) + ] + 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 == 5 + + 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, i) for i in range(2, 7)} + assert old.id not in {r.id for r in rows} + + async def test_delete_keys_credit_vacation_deletion_alongside_new_vacation_days( + self, + planning_service: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + """`apply_proposed_days` places no balance cap on 'vacation' today (no + + `_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. + """ + ws, child, parent = ( + seeded_ids.workspace_id, + seeded_ids.child_id, + seeded_ids.personal_data_uuid, + ) + 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", + ) + 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 + 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} + + async def test_delete_keys_credit_frees_dubbeldagar_quota( + self, + planning_service: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + """58 dubbeldagar already planned (below the 60 cap), 7 more asked + + for while 5 are deleted in the same call: 58 - 5 + 7 = 60, exactly at + cap -> must be accepted. Without crediting the 5 deletions the + projected total reads as 58 + 7 = 65, over cap, and would be + (wrongly) refused. + """ + ws, child, parent = ( + seeded_ids.workspace_id, + seeded_ids.child_id, + seeded_ids.personal_data_uuid, + ) + now = datetime.now() + existing_dates = [date(2024, 2, 1) + timedelta(days=i) for i in range(58)] + for d in existing_dates: + await planned_leave_repo.create( + PlannedLeaveDay( + id=uuid4(), + workspace_id=ws, + child_id=child, + parent_personal_data_uuid=parent, + date=d, + day_type="sjukpenning", + is_dubbeldag=True, + created_at=now, + updated_at=now, + ) + ) + + to_delete = existing_dates[:5] + delete_keys = {(child, parent, d) for d in to_delete} + new_dates = [date(2024, 4, 1) + timedelta(days=i) for i in range(7)] + new_days = [ + ProposedDayInput(d, parent, child, "sjukpenning", is_dubbeldag=True) for d in new_dates + ] + + 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 == 5 + assert result.created_count == 7 + + total_dubbeldagar = await planned_leave_repo.count_dubbeldagar(child) + assert total_dubbeldagar == 60 + + async def test_delete_keys_none_is_unchanged_behaviour( + self, + planning_service: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + """No `delete_keys` (the default) behaves exactly as before: nothing + + deleted beyond ordinary `replace` semantics, `deleted_count` stays 0. + """ + ws, child, parent = ( + seeded_ids.workspace_id, + seeded_ids.child_id, + seeded_ids.personal_data_uuid, + ) + days = [ProposedDayInput(date(2026, 6, 15), parent, child, "vacation")] + + result = await planning_service.apply_proposed_days(ws, days) + + assert result.accepted is True + assert result.deleted_count == 0 + assert result.created_count == 1 + + async def test_rejected_batch_with_delete_keys_deletes_nothing( + self, + planning_service: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + parental_leave_repo: SQLiteParentalLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + """Even with `delete_keys` credited, a batch that still exceeds the + + remaining balance is refused whole, and the row named by + `delete_keys` must survive untouched -- the all-or-nothing contract + applies to deletions too. + """ + ws, child, parent = ( + seeded_ids.workspace_id, + seeded_ids.child_id, + seeded_ids.personal_data_uuid, + ) + await parental_leave_repo.create( + _make_fetch(parent, ws, child, sjukpenning_left=5, lagsta_left=90) + ) + 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="sjukpenning", + ) + # 5 left, 1 committed -> 4 free. Deleting the 1 old row credits back + # 1, so net_new must be <= 5 to pass; ask for 7 instead (net_new = 6). + new_days = [ + ProposedDayInput(date(2026, 6, i), parent, child, "sjukpenning") for i in range(2, 9) + ] + 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 False + assert any( + r.message_key == "planApply.rejection.insufficientBalance" 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 len(rows) == 1 + assert rows[0].id == old.id # the row `delete_keys` named was NOT deleted + + async def test_delete_key_colliding_with_a_write_key_is_a_structural_rejection( + self, + planning_service: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + seeded_ids: SeededIds, + ) -> None: + """A batch cannot both write and delete the same (child, parent, + + date) slot -- that's a structural error caught before any DB read, + same as the other structural checks in this file. + """ + ws, child, parent = ( + seeded_ids.workspace_id, + seeded_ids.child_id, + seeded_ids.personal_data_uuid, + ) + existing = await planning_service.add_planned_day( + workspace_id=ws, + child_id=child, + parent_personal_data_uuid=parent, + date=date(2026, 6, 15), + day_type="lagsta", + ) + days = [ProposedDayInput(date(2026, 6, 15), parent, child, "vacation")] + delete_keys = {(child, parent, date(2026, 6, 15))} + + result = await planning_service.apply_proposed_days(ws, days, delete_keys=delete_keys) + + assert result.accepted is False + assert any( + r.message_key == "planApply.rejection.deleteKeyConflict" 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 len(rows) == 1 + assert rows[0].id == existing.id + assert rows[0].day_type == "lagsta" diff --git a/backend/tests/unit/test_planning_service_sync.py b/backend/tests/unit/test_planning_service_sync.py index e16a9bc..14a0b00 100644 --- a/backend/tests/unit/test_planning_service_sync.py +++ b/backend/tests/unit/test_planning_service_sync.py @@ -320,6 +320,105 @@ class TestSyncIdempotent: assert second.changes == [] +class TestSyncCreditsFreedCapacityWithinSameCall: + """Regression for the bug this change fixes: `sync_plan` used to delete + + stale (shrunk-away) rows only AFTER `apply_proposed_days` had already + validated and written the fresh batch, so a wish shrinking never + credited the capacity it freed to a same-call growing wish -- a family + near their sjukpenning cap who shortened one window and lengthened + another would have the whole sync rejected with `insufficientBalance` + even though the freed days more than covered the new demand. + """ + + async def test_shrinking_one_window_frees_capacity_for_a_new_one_in_the_same_sync( + self, + planning_service: PlanningService, + planned_leave_repo: SQLitePlannedLeaveRepository, + vacation_repo: SQLiteVacationDaysRepository, + parental_leave_repo: SQLiteParentalLeaveRepository, + leave_wish_repo: SQLiteLeaveWishRepository, + seeded_ids: SeededIds, + ) -> None: + ws, child, parent = ( + seeded_ids.workspace_id, + seeded_ids.child_id, + seeded_ids.personal_data_uuid, + ) + # No vacation budget -> forces sjukpenning/lagsta funding, so this + # test only has to reason about the sjukpenning pool. + await vacation_repo.create( + VacationDays( + id=uuid4(), personal_data_uuid=parent, workspace_id=ws, year=2027, total_days=0 + ) + ) + await parental_leave_repo.create( + _make_fetch(parent, ws, child, datetime(2027, 1, 1), sjuk_left=390, lagsta_left=90) + ) + + summer = await leave_wish_repo.create( + _make_wish( + ws, + child, + name="Summer", + window_start_month=6, + window_start_day=1, + window_end_month=6, + window_end_day=30, + target_weeks=4, + ) + ) + + first = await planning_service.sync_plan(ws, today=date(2027, 1, 1)) + assert first.apply_result.accepted is True + + summer_rows = await planned_leave_repo.list_by_workspace_child( + ws, child, from_date=date(2027, 6, 1), to_date=date(2027, 6, 30) + ) + sjuk_used = sum(1 for r in summer_rows if r.day_type == "sjukpenning") + assert sjuk_used > 10, "expected the 4-week summer window to fund several sjukpenning days" + + # Pin the balance to exactly what's already spent -> zero slack left, + # simulating a family sitting right at their sjukpenning cap. + await parental_leave_repo.create( + _make_fetch( + parent, ws, child, datetime(2027, 1, 2), sjuk_left=sjuk_used, lagsta_left=90 + ) + ) + + # Shrink summer to one week and add a smaller winter wish elsewhere. + # The capacity freed by shrinking summer comfortably covers winter's + # demand, but only if the freed days are credited within this call. + summer.window_end_day = 7 + summer.target_weeks = 1 + await leave_wish_repo.update(summer.id, summer) + await leave_wish_repo.create( + _make_wish( + ws, + child, + name="Winter", + window_start_month=12, + window_start_day=1, + window_end_month=12, + window_end_day=7, + target_weeks=1, + ) + ) + + second = await planning_service.sync_plan(ws, today=date(2027, 1, 1)) + + assert second.apply_result.accepted is True, second.apply_result.rejections + assert not any( + r.message_key == "planApply.rejection.insufficientBalance" + for r in second.apply_result.rejections + ) + + winter_rows = await planned_leave_repo.list_by_workspace_child( + ws, child, from_date=date(2027, 12, 1), to_date=date(2027, 12, 7) + ) + assert any(r.day_type == "sjukpenning" for r in winter_rows) + + class TestProposeStaysReadOnly: async def test_propose_never_writes_a_row( self,