Give tips a date, a scope per child, and a dismissal that sticks
Three faults, all invisible while the tests passed. Advice was built for the first child only, so a second child got none. The planned days feeding every count were fetched once per parent and appended to one list, so a couple's days were counted twice and every intensity and projection derived from them was wrong. And dismissing a tip forgot the moment the page reloaded. Tips now carry the date they concern, lifted from figures the rules already computed, so a view of a month can show only what applies to it. They are built per child and identified per child, so the same rule firing for two children is two separate pieces of advice. A dismissal is stored against a fingerprint of the facts that were dismissed. When those facts change the advice returns with its new numbers, rather than one click silencing a warning about a forfeiture that keeps growing.
This commit is contained in:
@@ -0,0 +1,305 @@
|
||||
"""Regression tests for `TipsService._build_context` and multi-child scoping.
|
||||
|
||||
`_build_context` used to have two bugs:
|
||||
|
||||
1. Single-child only: it read `children[0]` and fetched leave/planned-day
|
||||
data for that child alone, so a workspace with two children got tips
|
||||
about one and silence about the other.
|
||||
2. Doubled planned days: `list_by_workspace_child` already returns every
|
||||
parent's days for a child, but the old code called it *inside* the
|
||||
per-parent loop and kept appending — an N-parent workspace saw every
|
||||
planned day N times, corrupting every count-sensitive rule (intensity,
|
||||
SGI, age-4 projections).
|
||||
|
||||
These tests go through the real sqlite repositories (via the `tips_service`
|
||||
fixture in conftest.py), not hand-built `TipsContext`s, because the bugs
|
||||
lived in the fetching/wiring code itself.
|
||||
"""
|
||||
|
||||
from datetime import date, datetime, timedelta
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest_asyncio
|
||||
|
||||
from app.entities.child import Child
|
||||
from app.entities.parental_leave import ParentalLeaveFetch, TierData
|
||||
from app.entities.personal_data import PersonalData
|
||||
from app.entities.planned_leave import PlannedLeaveDay
|
||||
from app.entities.workspace import Workspace
|
||||
from app.repositories_impl.sqlite.child_repository import SQLiteChildRepository
|
||||
from app.repositories_impl.sqlite.parental_leave_repository import SQLiteParentalLeaveRepository
|
||||
from app.repositories_impl.sqlite.personal_data_repository import SQLitePersonalDataRepository
|
||||
from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository
|
||||
from app.repositories_impl.sqlite.workspace_repository import SQLiteWorkspaceRepository
|
||||
from app.services.tips_service import TipsService
|
||||
from app.utils.errors import NotFoundError
|
||||
|
||||
|
||||
def _make_fetch(
|
||||
personal_data_uuid,
|
||||
workspace_id,
|
||||
child_id,
|
||||
total: int = 240,
|
||||
used: int = 20,
|
||||
) -> ParentalLeaveFetch:
|
||||
now = datetime.now()
|
||||
tier = TierData(
|
||||
sjukpenning_total=390,
|
||||
sjukpenning_used=used,
|
||||
sjukpenning_left=390 - used,
|
||||
lagsta_niva_total=90,
|
||||
lagsta_niva_used=0,
|
||||
lagsta_niva_left=90,
|
||||
)
|
||||
return ParentalLeaveFetch(
|
||||
id=uuid4(),
|
||||
personal_data_uuid=personal_data_uuid,
|
||||
workspace_id=workspace_id,
|
||||
child_id=child_id,
|
||||
source="manual",
|
||||
fetched_at=now,
|
||||
total_days=total,
|
||||
days_used=used,
|
||||
days_left=total - used,
|
||||
reserved_days_total=90,
|
||||
reserved_days_left=90,
|
||||
transferable_days_left=50,
|
||||
dubbeldagar_left=60,
|
||||
tier_data=tier,
|
||||
created_at=now,
|
||||
updated_at=now,
|
||||
)
|
||||
|
||||
|
||||
def _make_planned_day(
|
||||
workspace_id, child_id, parent_uuid, day: date, day_type: str = "sjukpenning"
|
||||
) -> PlannedLeaveDay:
|
||||
return PlannedLeaveDay(
|
||||
id=uuid4(),
|
||||
workspace_id=workspace_id,
|
||||
child_id=child_id,
|
||||
parent_personal_data_uuid=parent_uuid,
|
||||
date=day,
|
||||
day_type=day_type,
|
||||
)
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def two_parent_one_child_workspace(
|
||||
workspace_repo: SQLiteWorkspaceRepository,
|
||||
personal_data_repo: SQLitePersonalDataRepository,
|
||||
child_repo: SQLiteChildRepository,
|
||||
planned_leave_repo: SQLitePlannedLeaveRepository,
|
||||
):
|
||||
now = datetime.now()
|
||||
ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now))
|
||||
parent1 = await personal_data_repo.create(
|
||||
PersonalData(uuid=uuid4(), workspace_id=ws.id, name="Alice", created_at=now)
|
||||
)
|
||||
parent2 = await personal_data_repo.create(
|
||||
PersonalData(uuid=uuid4(), workspace_id=ws.id, name="Bob", created_at=now)
|
||||
)
|
||||
child = await child_repo.create(
|
||||
Child(
|
||||
id=uuid4(),
|
||||
workspace_id=ws.id,
|
||||
name="Barn",
|
||||
birth_date=date.today().replace(year=date.today().year - 1),
|
||||
created_at=now,
|
||||
)
|
||||
)
|
||||
# A known, small day set: 3 future planned days for parent1 only.
|
||||
today = date.today()
|
||||
for i in range(3):
|
||||
await planned_leave_repo.create(
|
||||
_make_planned_day(ws.id, child.id, parent1.uuid, today + timedelta(days=i + 40))
|
||||
)
|
||||
return ws, parent1, parent2, child
|
||||
|
||||
|
||||
class TestPlannedDaysNotDoubledAcrossParents:
|
||||
async def test_build_context_does_not_double_planned_days(
|
||||
self,
|
||||
tips_service: TipsService,
|
||||
two_parent_one_child_workspace,
|
||||
) -> None:
|
||||
ws, _parent1, _parent2, _child = two_parent_one_child_workspace
|
||||
|
||||
_workspace_ctx, child_contexts = await tips_service._build_context(ws.id)
|
||||
|
||||
assert len(child_contexts) == 1
|
||||
# 3 planned days were seeded once each; with two parents in the
|
||||
# workspace, the old code (which re-fetched `list_by_workspace_child`
|
||||
# inside the per-parent loop and kept appending) would return 6.
|
||||
assert len(child_contexts[0].planned_days) == 3, (
|
||||
"planned days were duplicated across parents — "
|
||||
f"got {len(child_contexts[0].planned_days)} for 2 parents, expected 3"
|
||||
)
|
||||
|
||||
|
||||
class TestMultiChildContexts:
|
||||
async def test_each_child_gets_its_own_scoped_context(
|
||||
self,
|
||||
tips_service: TipsService,
|
||||
workspace_repo: SQLiteWorkspaceRepository,
|
||||
personal_data_repo: SQLitePersonalDataRepository,
|
||||
child_repo: SQLiteChildRepository,
|
||||
parental_leave_repo: SQLiteParentalLeaveRepository,
|
||||
) -> None:
|
||||
now = datetime.now()
|
||||
ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now))
|
||||
parent = await personal_data_repo.create(
|
||||
PersonalData(uuid=uuid4(), workspace_id=ws.id, name="Alice", created_at=now)
|
||||
)
|
||||
child_a = await child_repo.create(
|
||||
Child(
|
||||
id=uuid4(),
|
||||
workspace_id=ws.id,
|
||||
name="A",
|
||||
birth_date=date(2023, 1, 1),
|
||||
created_at=now,
|
||||
)
|
||||
)
|
||||
child_b = await child_repo.create(
|
||||
Child(
|
||||
id=uuid4(),
|
||||
workspace_id=ws.id,
|
||||
name="B",
|
||||
birth_date=date(2025, 1, 1),
|
||||
created_at=now,
|
||||
)
|
||||
)
|
||||
await parental_leave_repo.create(_make_fetch(parent.uuid, ws.id, child_a.id, used=10))
|
||||
await parental_leave_repo.create(_make_fetch(parent.uuid, ws.id, child_b.id, used=200))
|
||||
|
||||
workspace_ctx, child_contexts = await tips_service._build_context(ws.id)
|
||||
|
||||
assert workspace_ctx.children == []
|
||||
assert {c.children[0].id for c in child_contexts} == {child_a.id, child_b.id}
|
||||
|
||||
by_child = {c.children[0].id: c for c in child_contexts}
|
||||
assert by_child[child_a.id].leave_by_parent[parent.uuid].days_used == 10
|
||||
assert by_child[child_b.id].leave_by_parent[parent.uuid].days_used == 200
|
||||
|
||||
async def test_get_tips_scopes_tip_ids_and_data_per_child(
|
||||
self,
|
||||
tips_service: TipsService,
|
||||
workspace_repo: SQLiteWorkspaceRepository,
|
||||
personal_data_repo: SQLitePersonalDataRepository,
|
||||
child_repo: SQLiteChildRepository,
|
||||
parental_leave_repo: SQLiteParentalLeaveRepository,
|
||||
) -> None:
|
||||
"""Two children both eligible for `preserve_for_age_4` must produce
|
||||
two distinct tips, not one deduped/overwritten result — see the task
|
||||
brief: "a tip about child A must not dedupe against the same rule
|
||||
firing for child B".
|
||||
"""
|
||||
now = datetime.now()
|
||||
ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now))
|
||||
parent = await personal_data_repo.create(
|
||||
PersonalData(uuid=uuid4(), workspace_id=ws.id, name="Alice", created_at=now)
|
||||
)
|
||||
child_a = await child_repo.create(
|
||||
Child(
|
||||
id=uuid4(),
|
||||
workspace_id=ws.id,
|
||||
name="Adam",
|
||||
birth_date=date.today().replace(year=date.today().year - 1),
|
||||
created_at=now,
|
||||
)
|
||||
)
|
||||
child_b = await child_repo.create(
|
||||
Child(
|
||||
id=uuid4(),
|
||||
workspace_id=ws.id,
|
||||
name="Berit",
|
||||
birth_date=date.today().replace(year=date.today().year - 2),
|
||||
created_at=now,
|
||||
)
|
||||
)
|
||||
# total=240, used=20 -> projected_left=220, expiring=220-96=124 for both.
|
||||
await parental_leave_repo.create(_make_fetch(parent.uuid, ws.id, child_a.id, used=20))
|
||||
await parental_leave_repo.create(_make_fetch(parent.uuid, ws.id, child_b.id, used=20))
|
||||
|
||||
tips = await tips_service.get_tips(ws.id)
|
||||
|
||||
preserve_tips = [t for t in tips if t.id.startswith("preserve_for_age_4:")]
|
||||
assert len(preserve_tips) == 2, "expected one preserve_for_age_4 tip per child"
|
||||
|
||||
ids = {t.id for t in preserve_tips}
|
||||
assert ids == {
|
||||
f"preserve_for_age_4:{child_a.id}",
|
||||
f"preserve_for_age_4:{child_b.id}",
|
||||
}
|
||||
child_names = {t.data["child_name"] for t in preserve_tips}
|
||||
assert child_names == {"Adam", "Berit"}
|
||||
|
||||
|
||||
class TestDismissals:
|
||||
async def test_dismiss_hides_tip_until_data_changes(
|
||||
self,
|
||||
tips_service: TipsService,
|
||||
workspace_repo: SQLiteWorkspaceRepository,
|
||||
personal_data_repo: SQLitePersonalDataRepository,
|
||||
child_repo: SQLiteChildRepository,
|
||||
parental_leave_repo: SQLiteParentalLeaveRepository,
|
||||
planned_leave_repo: SQLitePlannedLeaveRepository,
|
||||
) -> None:
|
||||
now = datetime.now()
|
||||
ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now))
|
||||
parent = await personal_data_repo.create(
|
||||
PersonalData(uuid=uuid4(), workspace_id=ws.id, name="Alice", created_at=now)
|
||||
)
|
||||
child = await child_repo.create(
|
||||
Child(
|
||||
id=uuid4(),
|
||||
workspace_id=ws.id,
|
||||
name="Barn",
|
||||
birth_date=date.today().replace(year=date.today().year - 1),
|
||||
created_at=now,
|
||||
)
|
||||
)
|
||||
await parental_leave_repo.create(_make_fetch(parent.uuid, ws.id, child.id, used=20))
|
||||
|
||||
tips = await tips_service.get_tips(ws.id)
|
||||
tip = next(t for t in tips if t.id.startswith("preserve_for_age_4:"))
|
||||
original_expiring = tip.data["expiring_days"]
|
||||
|
||||
await tips_service.dismiss(ws.id, tip.id)
|
||||
|
||||
tips_after_dismiss = await tips_service.get_tips(ws.id)
|
||||
assert tip.id not in {t.id for t in tips_after_dismiss}
|
||||
|
||||
# The situation changes materially: add planned FP days before the
|
||||
# 4th birthday, which shrinks `expiring_days`. The stale dismissal
|
||||
# must not silence the new figure.
|
||||
fourth_bday = child.birth_date.replace(year=child.birth_date.year + 4)
|
||||
for i in range(30):
|
||||
await planned_leave_repo.create(
|
||||
_make_planned_day(
|
||||
ws.id,
|
||||
child.id,
|
||||
parent.uuid,
|
||||
fourth_bday - timedelta(days=i + 1),
|
||||
)
|
||||
)
|
||||
|
||||
tips_after_change = await tips_service.get_tips(ws.id)
|
||||
resurfaced = next((t for t in tips_after_change if t.id == tip.id), None)
|
||||
assert resurfaced is not None, "dismissal must expire once expiring_days changes"
|
||||
assert resurfaced.data["expiring_days"] != original_expiring
|
||||
|
||||
async def test_dismiss_unknown_tip_raises_not_found(
|
||||
self,
|
||||
tips_service: TipsService,
|
||||
workspace_repo: SQLiteWorkspaceRepository,
|
||||
) -> None:
|
||||
now = datetime.now()
|
||||
ws = await workspace_repo.create(Workspace(id=uuid4(), name="Familj", created_at=now))
|
||||
|
||||
try:
|
||||
await tips_service.dismiss(ws.id, "not_a_real_tip")
|
||||
raised = False
|
||||
except NotFoundError:
|
||||
raised = True
|
||||
assert raised
|
||||
Reference in New Issue
Block a user