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.
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user