Remove a period's days along with the period

Deleting a period left every day it had funded behind. The link from a day
back to its period was set to null the instant the period was deleted, so the
days became untraceable before anything could clean them up, and the next
recalculation had no way to know they no longer belonged to anything. Undo had
the same fault for the same reason: the period went, the days stayed, and a
reload showed them still there.

The days are now removed with the period, before the link can be broken. A day
pinned by hand goes too — pinning keeps the planner from quietly retyping a
day, not from being deleted when you delete the period it belongs to, which is
already how an unplanned band behaves.
This commit is contained in:
2026-09-18 00:23:34 +02:00
parent 8d2f658061
commit ac3b6a41d7
10 changed files with 337 additions and 37 deletions
+5 -2
View File
@@ -210,8 +210,11 @@ def get_vacation_days_service(repo: VacationDaysRepositoryDep) -> VacationDaysSe
VacationDaysServiceDep = Annotated[VacationDaysService, Depends(get_vacation_days_service)]
def get_leave_wish_service(repo: LeaveWishRepositoryDep) -> LeaveWishService:
return LeaveWishService(repo)
def get_leave_wish_service(
repo: LeaveWishRepositoryDep,
planned_leave_repo: PlannedLeaveRepositoryDep,
) -> LeaveWishService:
return LeaveWishService(repo, planned_leave_repo)
LeaveWishServiceDep = Annotated[LeaveWishService, Depends(get_leave_wish_service)]
@@ -33,3 +33,13 @@ class PlannedLeaveRepository(Repository[PlannedLeaveDay]):
@abstractmethod
async def count_dubbeldagar(self, child_id: UUID, from_date: date | None = None) -> int:
"""Count dubbeldagar already planned for a child (after from_date)."""
@abstractmethod
async def delete_by_wish(self, wish_id: UUID) -> int:
"""Delete every row attributed to this wish (pinned or not) and return the count deleted.
See `LeaveWishService.delete()`: called before the `leave_wish` row is
removed so the FK's `ON DELETE SET NULL` never fires — by the time
that DELETE runs, no `planned_leave_day` row still references this
wish, so there is nothing left to orphan.
"""
@@ -172,3 +172,10 @@ class SQLitePlannedLeaveRepository(PlannedLeaveRepository):
)
row = await cursor.fetchone()
return int(row[0]) if row else 0
async def delete_by_wish(self, wish_id: UUID) -> int:
cursor = await self._conn.execute(
"DELETE FROM planned_leave_day WHERE wish_id = ?", (str(wish_id),)
)
await self._conn.commit()
return cursor.rowcount
+29 -1
View File
@@ -10,6 +10,7 @@ from uuid import UUID, uuid4
from app.entities.leave_wish import LeaveWish
from app.repositories.leave_wish_repository import LeaveWishRepository
from app.repositories.planned_leave_repository import PlannedLeaveRepository
from app.utils.errors import NotFoundError, ValidationError
_VALID_RECURRENCE = {"yearly", "once"}
@@ -21,8 +22,13 @@ _LEAP_REFERENCE_YEAR = 2028
class LeaveWishService:
def __init__(self, repo: LeaveWishRepository) -> None:
def __init__(
self,
repo: LeaveWishRepository,
planned_leave_repo: PlannedLeaveRepository,
) -> None:
self._repo = repo
self._planned_leave_repo = planned_leave_repo
async def create(
self,
@@ -197,7 +203,29 @@ class LeaveWishService:
return await self._repo.update(leave_wish_id, entity)
async def delete(self, leave_wish_id: UUID) -> None:
"""Delete a wish and every `planned_leave_day` row it produced.
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
of those days the user later hand-edited are all one thing. Deleting
the period must remove all of it — including pinned days.
`wish_id` is nullable and `ON DELETE SET NULL` at the schema level
(so a row can still exist detached/ad-hoc), but that "orphan, don't
delete" behavior is wrong for this call path: the rows are deleted
explicitly, *before* the wish row goes, so the FK's SET NULL never
gets a row left to act on. See `PlannedLeaveRepository.delete_by_wish`.
A pinned day is deleted too rather than surviving as ad-hoc. Pinning
only protects a day from being silently retyped/reassigned by a
later `sync_plan` re-plan (see that method's docstring) — it is not
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
special-casing.
"""
await self.get(leave_wish_id) # raises NotFoundError if missing
await self._planned_leave_repo.delete_by_wish(leave_wish_id)
await self._repo.delete(leave_wish_id)
+1 -1
View File
@@ -135,7 +135,7 @@ def _build_services(conn: aiosqlite.Connection) -> DemoServices:
child=ChildService(children),
vacation=VacationDaysService(vacation_repo),
day_override=DayOverrideService(day_overrides),
leave_wish=LeaveWishService(leave_wishes),
leave_wish=LeaveWishService(leave_wishes, planned_leave),
manual_fetch=ManualFetchService(parental_leave),
planning=PlanningService(planned_leave, analytics, parental_leave, vacation_repo),
users=users,
+2 -1
View File
@@ -222,8 +222,9 @@ async def leave_wish_repo(
@pytest_asyncio.fixture
async def leave_wish_service(
leave_wish_repo: SQLiteLeaveWishRepository,
planned_leave_repo: SQLitePlannedLeaveRepository,
) -> LeaveWishService:
return LeaveWishService(leave_wish_repo)
return LeaveWishService(leave_wish_repo, planned_leave_repo)
# ---------------------------------------------------------------------------
@@ -5,13 +5,15 @@ Aim for >=90% coverage on new code (ARCHITECTURE.md §10).
"""
from datetime import date, datetime
from uuid import uuid4
from uuid import UUID, uuid4
import pytest
from app.entities.leave_wish import LeaveWish
from app.entities.planned_leave import PlannedLeaveDay
from app.entities.workspace import Workspace
from app.repositories_impl.sqlite.leave_wish_repository import SQLiteLeaveWishRepository
from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository
from app.repositories_impl.sqlite.workspace_repository import SQLiteWorkspaceRepository
from app.services.leave_wish_service import LeaveWishService
from app.utils.errors import NotFoundError, ValidationError
@@ -1091,3 +1093,121 @@ class TestLeaveWishService:
) -> None:
with pytest.raises(NotFoundError):
await leave_wish_service.delete(uuid4())
class TestLeaveWishServiceDeleteCascade:
"""`LeaveWishService.delete()` must remove the `planned_leave_day` rows
the wish produced, not just null their `wish_id` out from under them
(the schema's `ON DELETE SET NULL` would otherwise leave them behind as
permanent, unattributable ad-hoc rows — see the docstring on
`LeaveWishService.delete()` and `PlannedLeaveRepository.delete_by_wish`).
"""
def _make_day(
self,
seeded_ids: SeededIds,
*,
wish_id: UUID | None = None,
is_pinned: bool = False,
day: str = "2027-02-03",
) -> PlannedLeaveDay:
return PlannedLeaveDay(
id=uuid4(),
workspace_id=seeded_ids.workspace_id,
child_id=seeded_ids.child_id,
parent_personal_data_uuid=seeded_ids.personal_data_uuid,
date=date.fromisoformat(day),
day_type="sjukpenning",
is_dubbeldag=False,
wish_id=wish_id,
is_pinned=is_pinned,
created_at=datetime.now(),
updated_at=datetime.now(),
)
async def test_delete_removes_the_planned_leave_days_it_produced(
self,
leave_wish_service: LeaveWishService,
leave_wish_repo: SQLiteLeaveWishRepository,
planned_leave_repo: SQLitePlannedLeaveRepository,
seeded_ids: SeededIds,
) -> None:
wish = await leave_wish_repo.create(_make(workspace_id=seeded_ids.workspace_id))
day1 = await planned_leave_repo.create(
self._make_day(seeded_ids, wish_id=wish.id, day="2027-02-03")
)
day2 = await planned_leave_repo.create(
self._make_day(seeded_ids, wish_id=wish.id, day="2027-02-04")
)
await leave_wish_service.delete(wish.id)
assert await planned_leave_repo.get_by_id(day1.id) is None
assert await planned_leave_repo.get_by_id(day2.id) is None
async def test_delete_removes_a_pinned_day_too(
self,
leave_wish_service: LeaveWishService,
leave_wish_repo: SQLiteLeaveWishRepository,
planned_leave_repo: SQLitePlannedLeaveRepository,
seeded_ids: SeededIds,
) -> None:
"""A pinned day dies with its wish rather than surviving as ad-hoc.
Pinning only protects a day from being silently retyped/reassigned
by a later `sync_plan` re-plan — it is not a signal that the day
should outlive a period the user explicitly chose to delete. This
also matches the ad-hoc-period delete path in `Calendar.tsx`
(`handleDeletePeriod`'s no-`wishId` branch), which already deletes
every day in the band regardless of `is_pinned`.
"""
wish = await leave_wish_repo.create(_make(workspace_id=seeded_ids.workspace_id))
pinned = await planned_leave_repo.create(
self._make_day(seeded_ids, wish_id=wish.id, is_pinned=True)
)
await leave_wish_service.delete(wish.id)
assert await planned_leave_repo.get_by_id(pinned.id) is None
async def test_delete_leaves_a_different_wishs_days_untouched(
self,
leave_wish_service: LeaveWishService,
leave_wish_repo: SQLiteLeaveWishRepository,
planned_leave_repo: SQLitePlannedLeaveRepository,
seeded_ids: SeededIds,
) -> None:
wish_a = await leave_wish_repo.create(_make(workspace_id=seeded_ids.workspace_id, name="A"))
wish_b = await leave_wish_repo.create(_make(workspace_id=seeded_ids.workspace_id, name="B"))
day_a = await planned_leave_repo.create(
self._make_day(seeded_ids, wish_id=wish_a.id, day="2027-02-03")
)
day_b = await planned_leave_repo.create(
self._make_day(seeded_ids, wish_id=wish_b.id, day="2027-02-04")
)
await leave_wish_service.delete(wish_a.id)
assert await planned_leave_repo.get_by_id(day_a.id) is None
survivor = await planned_leave_repo.get_by_id(day_b.id)
assert survivor is not None
assert survivor.wish_id == wish_b.id
async def test_delete_leaves_genuine_ad_hoc_days_untouched(
self,
leave_wish_service: LeaveWishService,
leave_wish_repo: SQLiteLeaveWishRepository,
planned_leave_repo: SQLitePlannedLeaveRepository,
seeded_ids: SeededIds,
) -> None:
wish = await leave_wish_repo.create(_make(workspace_id=seeded_ids.workspace_id))
ad_hoc = await planned_leave_repo.create(
self._make_day(seeded_ids, wish_id=None, day="2027-02-03")
)
await leave_wish_service.delete(wish.id)
survivor = await planned_leave_repo.get_by_id(ad_hoc.id)
assert survivor is not None
assert survivor.wish_id is None