diff --git a/backend/app/db/connection.py b/backend/app/db/connection.py index 0a76ecf..acc4dd8 100644 --- a/backend/app/db/connection.py +++ b/backend/app/db/connection.py @@ -173,6 +173,16 @@ CREATE TABLE IF NOT EXISTS day_override ( UNIQUE(workspace_id, date), FOREIGN KEY (workspace_id) REFERENCES workspace(id) ON DELETE CASCADE ); + +CREATE TABLE IF NOT EXISTS tip_dismissal ( + id TEXT PRIMARY KEY, + workspace_id TEXT NOT NULL, + tip_id TEXT NOT NULL, + data_hash TEXT NOT NULL, + dismissed_at TIMESTAMP NOT NULL, + UNIQUE(workspace_id, tip_id), + FOREIGN KEY (workspace_id) REFERENCES workspace(id) ON DELETE CASCADE +); """ diff --git a/backend/app/dependencies.py b/backend/app/dependencies.py index ec39373..b7a12c2 100644 --- a/backend/app/dependencies.py +++ b/backend/app/dependencies.py @@ -24,6 +24,7 @@ from app.repositories.leave_wish_repository import LeaveWishRepository from app.repositories.parental_leave_repository import ParentalLeaveRepository from app.repositories.personal_data_repository import PersonalDataRepository from app.repositories.planned_leave_repository import PlannedLeaveRepository +from app.repositories.tip_dismissal_repository import TipDismissalRepository from app.repositories.user_repository import UserRepository from app.repositories.vacation_days_repository import VacationDaysRepository from app.repositories.workspace_repository import ( @@ -36,6 +37,7 @@ from app.repositories_impl.sqlite.leave_wish_repository import SQLiteLeaveWishRe 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.tip_dismissal_repository import SQLiteTipDismissalRepository from app.repositories_impl.sqlite.user_repository import SQLiteUserRepository from app.repositories_impl.sqlite.vacation_days_repository import SQLiteVacationDaysRepository from app.repositories_impl.sqlite.workspace_repository import ( @@ -51,6 +53,7 @@ from app.services.day_override_service import DayOverrideService from app.services.leave_wish_service import LeaveWishService from app.services.personal_data_service import PersonalDataService from app.services.planning_service import PlanningService +from app.services.tips_service import TipsService from app.services.vacation_days_service import VacationDaysService from app.services.workspace_service import WorkspaceService from app.utils.errors import UnauthorizedError @@ -175,6 +178,15 @@ def get_day_override_repository(conn: ConnectionDep) -> DayOverrideRepository: DayOverrideRepositoryDep = Annotated[DayOverrideRepository, Depends(get_day_override_repository)] +def get_tip_dismissal_repository(conn: ConnectionDep) -> TipDismissalRepository: + if settings.database_type == "sqlite": + return SQLiteTipDismissalRepository(conn) + raise ValueError(f"Unsupported database_type: {settings.database_type}") + + +TipDismissalRepositoryDep = Annotated[TipDismissalRepository, Depends(get_tip_dismissal_repository)] + + # --------------------------------------------------------------------------- # Service factories # --------------------------------------------------------------------------- @@ -219,6 +231,31 @@ def get_calendar_service(day_override_service: DayOverrideServiceDep) -> Calenda CalendarServiceDep = Annotated[CalendarService, Depends(get_calendar_service)] +def get_tips_service( + leave_repo: ParentalLeaveRepositoryDep, + planned_repo: PlannedLeaveRepositoryDep, + child_repo: ChildRepositoryDep, + pd_repo: PersonalDataRepositoryDep, + vacation_repo: VacationDaysRepositoryDep, + leave_wish_repo: LeaveWishRepositoryDep, + ws_repo: WorkspaceRepositoryDep, + dismissal_repo: TipDismissalRepositoryDep, +) -> TipsService: + return TipsService( + leave_repo, + planned_repo, + child_repo, + pd_repo, + vacation_repo, + leave_wish_repo, + ws_repo, + dismissal_repo, + ) + + +TipsServiceDep = Annotated[TipsService, Depends(get_tips_service)] + + def get_child_service(repo: ChildRepositoryDep) -> ChildService: return ChildService(repo) diff --git a/backend/app/entities/tip_dismissal.py b/backend/app/entities/tip_dismissal.py new file mode 100644 index 0000000..9ec6b50 --- /dev/null +++ b/backend/app/entities/tip_dismissal.py @@ -0,0 +1,30 @@ +"""Domain entity: TipDismissal. + +A workspace-scoped record that a specific tip (`Tip.id`, see +`app.services.tips_service`) has been acknowledged/dismissed by the user. +Keyed by (workspace_id, tip_id) — see the UNIQUE constraint on the +`tip_dismissal` table — so re-dismissing the same tip refreshes `data_hash` +and `dismissed_at` instead of creating a duplicate row. + +`data_hash` snapshots `app.utils.tip_predicates.hash_tip_data(tip.data)` at +dismiss time. `TipsService` only treats a dismissal as active while the +rule's *current* output hashes to the same value — once the underlying +numbers move (more days now projected to expire at age 4, a different +`fourth_birthday`, ...), the dismissal is stale and the tip resurfaces. See +`TipsService._filter_dismissed` for why: permanently silencing a warning like +`preserve_for_age_4` because its *old* figures were once dismissed would be +dangerous. +""" + +from dataclasses import dataclass +from datetime import datetime +from uuid import UUID + + +@dataclass +class TipDismissal: + id: UUID + workspace_id: UUID + tip_id: str + data_hash: str + dismissed_at: datetime | None = None diff --git a/backend/app/models/tips.py b/backend/app/models/tips.py new file mode 100644 index 0000000..9a009a5 --- /dev/null +++ b/backend/app/models/tips.py @@ -0,0 +1,43 @@ +"""API schemas (Pydantic) for the tips engine. + +The route used to return `list[dict]` — no shape guarantee at all. `Tip.data` +stays a free-form bag on purpose (§ tips_service.py docstring: it is +interpolation context for the frontend, and every rule shapes it +differently), but everything around it is now a real, typed contract. +""" + +from datetime import date + +from pydantic import BaseModel + +from app.services.tips_service import Tip + +# Every value any rule currently puts in `Tip.data` is a JSON scalar (str, +# int, float, bool) or None — never a nested dict/list. Typing it this way +# (rather than `dict[str, Any]`) still catches a rule accidentally stuffing +# something un-renderable into `data`. +TipDataValue = str | int | float | bool | None + + +class TipResponse(BaseModel): + id: str + severity: str + message_key: str + data: dict[str, TipDataValue] + anchor_date: date | None + anchor_range: tuple[date, date] | None + + @classmethod + def from_domain(cls, tip: Tip) -> "TipResponse": + return cls( + id=tip.id, + severity=tip.severity, + message_key=tip.message_key, + data=tip.data, + anchor_date=tip.anchor_date, + anchor_range=tip.anchor_range, + ) + + +class TipDismissalCreate(BaseModel): + tip_id: str diff --git a/backend/app/repositories/tip_dismissal_repository.py b/backend/app/repositories/tip_dismissal_repository.py new file mode 100644 index 0000000..a6cb9cd --- /dev/null +++ b/backend/app/repositories/tip_dismissal_repository.py @@ -0,0 +1,21 @@ +"""Abstract repository interface for TipDismissal. + +Adds entity-specific queries on top of the generic CRUD contract. Concrete +implementations live in app/repositories_impl//. +""" + +from abc import abstractmethod +from uuid import UUID + +from app.entities.tip_dismissal import TipDismissal +from app.repositories.base import Repository + + +class TipDismissalRepository(Repository[TipDismissal]): + @abstractmethod + async def get_by_workspace_tip(self, workspace_id: UUID, tip_id: str) -> TipDismissal | None: + """Return the dismissal for a given workspace + tip id (UNIQUE), or None.""" + + @abstractmethod + async def list_by_workspace(self, workspace_id: UUID) -> list[TipDismissal]: + """Return all dismissals for a workspace.""" diff --git a/backend/app/repositories_impl/sqlite/tip_dismissal_repository.py b/backend/app/repositories_impl/sqlite/tip_dismissal_repository.py new file mode 100644 index 0000000..575f0ba --- /dev/null +++ b/backend/app/repositories_impl/sqlite/tip_dismissal_repository.py @@ -0,0 +1,101 @@ +"""SQLite implementation of TipDismissalRepository. + +Follows the DayOverrideRepository reference shape: upsert-by-natural-key +((workspace_id, tip_id) is UNIQUE — see the `tip_dismissal` table), map rows +<-> entities explicitly in `_row_to_entity`, always parameterize queries, +raise NotFoundError from update/delete when the row is missing. +""" + +from datetime import datetime +from uuid import UUID + +import aiosqlite + +from app.entities.tip_dismissal import TipDismissal +from app.repositories.filter_validator import validate_filters +from app.repositories.tip_dismissal_repository import TipDismissalRepository +from app.utils.errors import NotFoundError + + +class SQLiteTipDismissalRepository(TipDismissalRepository): + def __init__(self, connection: aiosqlite.Connection) -> None: + self._conn = connection + + @staticmethod + def _row_to_entity(row: aiosqlite.Row) -> TipDismissal: + return TipDismissal( + id=UUID(row["id"]), + workspace_id=UUID(row["workspace_id"]), + tip_id=row["tip_id"], + data_hash=row["data_hash"], + dismissed_at=datetime.fromisoformat(row["dismissed_at"]), + ) + + async def create(self, item: TipDismissal) -> TipDismissal: + item.dismissed_at = item.dismissed_at or datetime.now() + await self._conn.execute( + """ + INSERT INTO tip_dismissal (id, workspace_id, tip_id, data_hash, dismissed_at) + VALUES (?, ?, ?, ?, ?) + """, + ( + str(item.id), + str(item.workspace_id), + item.tip_id, + item.data_hash, + item.dismissed_at.isoformat(), + ), + ) + await self._conn.commit() + return item + + async def get_by_id(self, id: UUID) -> TipDismissal | None: + cursor = await self._conn.execute("SELECT * FROM tip_dismissal WHERE id = ?", (str(id),)) + row = await cursor.fetchone() + return self._row_to_entity(row) if row else None + + async def get_by_workspace_tip(self, workspace_id: UUID, tip_id: str) -> TipDismissal | None: + cursor = await self._conn.execute( + "SELECT * FROM tip_dismissal WHERE workspace_id = ? AND tip_id = ?", + (str(workspace_id), tip_id), + ) + row = await cursor.fetchone() + return self._row_to_entity(row) if row else None + + async def update(self, id: UUID, item: TipDismissal) -> TipDismissal: + item.dismissed_at = datetime.now() + cursor = await self._conn.execute( + "UPDATE tip_dismissal SET data_hash = ?, dismissed_at = ? WHERE id = ?", + (item.data_hash, item.dismissed_at.isoformat(), str(id)), + ) + await self._conn.commit() + if cursor.rowcount == 0: + raise NotFoundError(f"TipDismissal {id} not found") + refreshed = await self.get_by_id(id) + assert refreshed is not None # just updated + return refreshed + + async def delete(self, id: UUID) -> None: + cursor = await self._conn.execute("DELETE FROM tip_dismissal WHERE id = ?", (str(id),)) + await self._conn.commit() + if cursor.rowcount == 0: + raise NotFoundError(f"TipDismissal {id} not found") + + async def list(self, **filters: object) -> list[TipDismissal]: + validate_filters(filters, {"workspace_id"}) + clauses: list[str] = [] + params: list[object] = [] + for column in ("workspace_id",): + if column in filters and filters[column] is not None: + value = filters[column] + clauses.append(f"{column} = ?") + params.append(str(value) if isinstance(value, UUID) else value) + where = f" WHERE {' AND '.join(clauses)}" if clauses else "" + cursor = await self._conn.execute( + f"SELECT * FROM tip_dismissal{where} ORDER BY dismissed_at ASC", params + ) + rows = await cursor.fetchall() + return [self._row_to_entity(row) for row in rows] + + async def list_by_workspace(self, workspace_id: UUID) -> list[TipDismissal]: + return await self.list(workspace_id=workspace_id) diff --git a/backend/app/routers/settings.py b/backend/app/routers/settings.py index b799cf3..8eb864a 100644 --- a/backend/app/routers/settings.py +++ b/backend/app/routers/settings.py @@ -13,20 +13,14 @@ from uuid import UUID from fastapi import APIRouter, HTTPException, status from app.dependencies import ( - ChildRepositoryDep, ChildServiceDep, CurrentUserDep, - LeaveWishRepositoryDep, - ParentalLeaveRepositoryDep, - PersonalDataRepositoryDep, PersonalDataServiceDep, - PlannedLeaveRepositoryDep, - VacationDaysRepositoryDep, - WorkspaceRepositoryDep, + TipsServiceDep, ) from app.models.child import ChildCreate, ChildResponse, ChildUpdate from app.models.personal_data import PersonalDataResponse, PersonalDataUpdate -from app.services.tips_service import TipsService +from app.models.tips import TipDismissalCreate, TipResponse from app.utils.errors import NotFoundError, ValidationError router = APIRouter(tags=["settings"]) @@ -39,26 +33,27 @@ router = APIRouter(tags=["settings"]) tips_router = APIRouter(prefix="/api/tips", tags=["tips"]) -@tips_router.get("/{workspace_id}", response_model=list[dict]) +@tips_router.get("/{workspace_id}", response_model=list[TipResponse]) async def get_tips( workspace_id: UUID, - leave_repo: ParentalLeaveRepositoryDep, - planned_repo: PlannedLeaveRepositoryDep, - child_repo: ChildRepositoryDep, - pd_repo: PersonalDataRepositoryDep, - vacation_repo: VacationDaysRepositoryDep, - leave_wish_repo: LeaveWishRepositoryDep, - ws_repo: WorkspaceRepositoryDep, + service: TipsServiceDep, _current_user: CurrentUserDep, -) -> list[dict]: - service = TipsService( - leave_repo, planned_repo, child_repo, pd_repo, vacation_repo, leave_wish_repo, ws_repo - ) +) -> list[TipResponse]: tips = await service.get_tips(workspace_id) - return [ - {"id": t.id, "severity": t.severity, "message_key": t.message_key, "data": t.data} - for t in tips - ] + return [TipResponse.from_domain(tip) for tip in tips] + + +@tips_router.post("/{workspace_id}/dismissals", status_code=status.HTTP_204_NO_CONTENT) +async def dismiss_tip( + workspace_id: UUID, + payload: TipDismissalCreate, + service: TipsServiceDep, + _current_user: CurrentUserDep, +) -> None: + try: + await service.dismiss(workspace_id, payload.tip_id) + except NotFoundError as exc: + raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc # --------------------------------------------------------------------------- diff --git a/backend/app/services/tips_service.py b/backend/app/services/tips_service.py index fc9ade8..10ae01d 100644 --- a/backend/app/services/tips_service.py +++ b/backend/app/services/tips_service.py @@ -22,7 +22,7 @@ from collections import defaultdict from dataclasses import dataclass from datetime import date, timedelta from typing import Callable, Coroutine, TypeAlias -from uuid import UUID +from uuid import UUID, uuid4 from dateutil.relativedelta import relativedelta as _relativedelta @@ -32,17 +32,22 @@ from app.entities.leave_wish import LeaveWish from app.entities.parental_leave import ParentalLeaveFetch from app.entities.personal_data import PersonalData from app.entities.planned_leave import PlannedLeaveDay +from app.entities.tip_dismissal import TipDismissal from app.entities.vacation_days import VacationDays from app.repositories.child_repository import ChildRepository from app.repositories.leave_wish_repository import LeaveWishRepository from app.repositories.parental_leave_repository import ParentalLeaveRepository from app.repositories.personal_data_repository import PersonalDataRepository from app.repositories.planned_leave_repository import PlannedLeaveRepository +from app.repositories.tip_dismissal_repository import TipDismissalRepository from app.repositories.vacation_days_repository import VacationDaysRepository from app.repositories.workspace_repository import WorkspaceRepository +from app.utils.errors import NotFoundError from app.utils.tip_predicates import ( + age4_pooling_loss, days_left_at_age_4_projection, has_vacation_near_expiry, + hash_tip_data, is_child_over_age, preschool_buffer_needed, red_day_clusters_in_range, @@ -58,15 +63,31 @@ class Tip: severity: str # 'info' | 'warning' | 'action' message_key: str # i18n key for frontend data: dict # context for frontend rendering (e.g. {days: 5, period: "Jul–Aug"}) + # Where this tip is anchored on a calendar: a single day (e.g. "use this + # vacation day"), a span (e.g. a break period), or neither. Lifted out of + # `data` — where the value already lived — rather than invented; see the + # individual rules for which `data` key each maps to. Rules with no + # meaningful date (balance_between_parents, lagsta_niva_coverage, + # stretch_time_at_home, sgi_minimum_after_age_1, + # reserve_for_preschool_planning_days) leave both null. + anchor_date: date | None = None + anchor_range: tuple[date, date] | None = None @dataclass class TipsContext: workspace_id: UUID + # Either empty (the workspace-level context — see `_WORKSPACE_SCOPED_RULES` + # in `TipsService`) or a single child (a per-child context, built once per + # child in `_build_context`). Never more than one: entitlement and + # planned days are tracked per child, so a rule scoped to "the" child + # would silently pick an arbitrary one in a multi-child workspace. children: list[Child] parents: list[PersonalData] leave_by_parent: dict[UUID, ParentalLeaveFetch | None] # personal_data_uuid → latest - planned_days: list[PlannedLeaveDay] # all future planned days for workspace + # Future planned days for `children[0]` only (empty for the workspace + # context) — see `_build_context`. + planned_days: list[PlannedLeaveDay] vacation_by_parent: dict[UUID, list[VacationDays]] leave_wishes: list[LeaveWish] today: date # injected for deterministic testing @@ -89,6 +110,11 @@ async def _rule_use_vacation_before_expiry(ctx: TipsContext) -> Tip | None: for parent in ctx.parents: for vd in ctx.vacation_by_parent.get(parent.uuid, []): if has_vacation_near_expiry(vd.total_days, vd.saved_days_in): + # `vd.year` is the only date-shaped fact this rule has; anchor + # to that vacation year's last day — the natural forcing point + # for "use it before it's gone" — rather than leaving the rail + # with nothing to place this tip on. + year_end = date(vd.year, 12, 31) return Tip( id="use_vacation_before_expiry", severity="warning", @@ -98,6 +124,7 @@ async def _rule_use_vacation_before_expiry(ctx: TipsContext) -> Tip | None: "saved_days": vd.saved_days_in, "year": vd.year, }, + anchor_date=year_end, ) return None @@ -159,10 +186,120 @@ async def _rule_preserve_for_age_4(ctx: TipsContext) -> Tip | None: "expiring_days": expiring_at_b4, "fourth_birthday": fourth_bday.isoformat(), }, + anchor_date=fourth_bday, ) return None +async def _rule_age4_pooling_loss(ctx: TipsContext) -> Tip | None: + """Warn a couple when one parent is over the per-parent age-4 cap while + the other has unused headroom — days that could have been pooled but + won't be, because `resolve_age4_savable_cap` splits the ceiling per + parent once a workspace is in `couple` mode (see `age4_pooling_loss`). + + Advice order is the substance of this tip: taking more leave before the + birthday converts entitlement into actual time at home (strictly + better), so it is offered first. Transferring days at Försäkringskassan + only preserves the entitlement — it is offered second, and the transfer + figure is bounded by what `ParentalLeaveFetch.transferable_days_left` + (not the raw days-left total, which includes non-transferable reserved + days) says is actually movable. When the partner has no FK snapshot the + exact transfer figure can't be known, so the tip degrades to a hedged + "if you both have days left" phrasing instead of guessing a number. + """ + if ctx.workspace_planning_mode != "couple" or not ctx.children: + return None + + child = ctx.children[0] + if not is_child_over_age(child.birth_date, settings.rules.age4_pooling_tip_min_child_age_years): + return None # first couple of years: too early to usefully act on this + + fourth_bday = child.birth_date + _relativedelta(years=4) + per_parent_cap = resolve_age4_savable_cap(ctx.workspace_planning_mode, len(ctx.parents)) + per_child_cap = settings.rules.max_savable_days_at_age_4 + + projected_left_by_parent: dict[UUID, int] = {} + excess_by_parent: dict[UUID, int] = {} + for parent in ctx.parents: + leave = ctx.leave_by_parent.get(parent.uuid) + if leave is None: + continue + planned_fp_before_b4 = sum( + 1 + for d in ctx.planned_days + if d.parent_personal_data_uuid == parent.uuid + and d.day_type in ("sjukpenning", "lagsta") + and d.date < fourth_bday + ) + projected_left, expiring = days_left_at_age_4_projection( + leave.days_used, planned_fp_before_b4, leave.total_days, per_parent_cap + ) + projected_left_by_parent[parent.uuid] = projected_left + excess_by_parent[parent.uuid] = expiring + + if not projected_left_by_parent: + return None + + family_loss = age4_pooling_loss( + list(projected_left_by_parent.values()), per_parent_cap, per_child_cap + ) + if family_loss <= 0: + return None + + surplus_uuid = max(excess_by_parent, key=lambda u: excess_by_parent[u]) + surplus_parent = next(p for p in ctx.parents if p.uuid == surplus_uuid) + surplus_leave = ctx.leave_by_parent[surplus_uuid] + excess_days = excess_by_parent[surplus_uuid] + + other_parents = [p for p in ctx.parents if p.uuid != surplus_uuid] + partner_has_snapshot = False + lagging_parent: PersonalData | None = None + lagging_headroom = 0 + for other in other_parents: + other_leave = ctx.leave_by_parent.get(other.uuid) + if other_leave is None: + continue + partner_has_snapshot = True + headroom = max(per_parent_cap - projected_left_by_parent[other.uuid], 0) + if headroom > lagging_headroom: + lagging_headroom = headroom + lagging_parent = other + + if lagging_parent is not None and lagging_headroom > 0: + transfer_days = min(excess_days, lagging_headroom, surplus_leave.transferable_days_left) + transfer_days = max(transfer_days, 0) + return Tip( + id="age4_pooling_loss", + severity="warning", + message_key="tips.age4_pooling_loss_exact", + data={ + "surplus_parent": surplus_parent.name, + "lagging_parent": lagging_parent.name, + "excess_days": excess_days, + "transfer_days": transfer_days, + "family_loss_days": family_loss, + "parent_cap": per_parent_cap, + "fourth_birthday": fourth_bday.isoformat(), + }, + ) + + if not partner_has_snapshot and other_parents: + return Tip( + id="age4_pooling_loss", + severity="warning", + message_key="tips.age4_pooling_loss_hedged", + data={ + "surplus_parent": surplus_parent.name, + "excess_days": excess_days, + "family_loss_days": family_loss, + "parent_cap": per_parent_cap, + "fourth_birthday": fourth_bday.isoformat(), + }, + ) + + return None + + async def _rule_use_dubbeldagar_strategically(ctx: TipsContext) -> Tip | None: if len(ctx.parents) < 2 or not ctx.children: return None @@ -189,6 +326,7 @@ async def _rule_use_dubbeldagar_strategically(ctx: TipsContext) -> Tip | None: severity="action", message_key="tips.use_dubbeldagar_strategically", data={"date": day_date.isoformat()}, + anchor_date=day_date, ) return None @@ -304,6 +442,7 @@ async def _rule_wrap_weekend_with_vacation(ctx: TipsContext) -> Tip | None: "friday": fp_date.isoformat(), "monday": monday.isoformat(), }, + anchor_range=(fp_date, monday), ) return None @@ -337,6 +476,7 @@ async def _rule_wrap_red_days_with_vacation(ctx: TipsContext) -> Tip | None: "cluster_start": cluster_start.isoformat(), "cluster_end": cluster_end.isoformat(), }, + anchor_range=(cluster_start, cluster_end), ) return None @@ -393,6 +533,7 @@ async def _rule_long_break_parent_swap(ctx: TipsContext) -> Tip | None: "parent_name": parent.name, "other_parent_name": other.name, }, + anchor_range=(start_date, end_date), ) return None @@ -430,6 +571,7 @@ RULE_REGISTRY: list[RuleFn] = [ _rule_use_vacation_before_expiry, _rule_balance_between_parents, _rule_preserve_for_age_4, + _rule_age4_pooling_loss, _rule_use_dubbeldagar_strategically, _rule_lagsta_niva_coverage, _rule_stretch_time_at_home, @@ -440,6 +582,18 @@ RULE_REGISTRY: list[RuleFn] = [ _rule_reserve_for_preschool_planning_days, ] +# Parental-leave entitlement (`parental_leave_fetch`), and every planned day +# that draws on it, is tracked per CHILD (see `ParentalLeaveRepository. +# get_latest_by_parent` / `PlannedLeaveRepository.list_by_workspace_child`). +# `TipsService.get_tips` therefore runs most rules once per child, against a +# context scoped to that child's own leave/planned-day data (see +# `_build_context`). `use_vacation_before_expiry` is the one rule that reads +# neither — vacation days belong to the parent, not any child — so running it +# per child would just print the same warning N times for an N-child +# workspace; it runs once instead, against a childless workspace-level +# context. +_WORKSPACE_SCOPED_RULES: frozenset[RuleFn] = frozenset({_rule_use_vacation_before_expiry}) + class TipsService: def __init__( @@ -451,6 +605,7 @@ class TipsService: vacation_repo: VacationDaysRepository, leave_wish_repo: LeaveWishRepository, workspace_repo: WorkspaceRepository, + dismissal_repo: TipDismissalRepository, ) -> None: self._leave = leave_repo self._planned = planned_repo @@ -459,47 +614,152 @@ class TipsService: self._vacation = vacation_repo self._leave_wishes = leave_wish_repo self._workspace_repo = workspace_repo + self._dismissals = dismissal_repo async def get_tips(self, workspace_id: UUID) -> list[Tip]: - """Load workspace context then run all rules, returning non-None tips.""" - ctx = await self._build_context(workspace_id) - results = await asyncio.gather(*(rule(ctx) for rule in RULE_REGISTRY)) - return [tip for tip in results if tip is not None] + """Load workspace context(s), run all rules, drop stale-dismissed tips. - async def _build_context(self, workspace_id: UUID) -> TipsContext: + One workspace-level context feeds `_WORKSPACE_SCOPED_RULES`; one + per-child context feeds every other rule (see `_build_context` and + `_WORKSPACE_SCOPED_RULES` for why). A child-scoped tip's `id` is + suffixed with its child's id so that e.g. `preserve_for_age_4` firing + for two different children never collides — neither in the returned + list nor in `TipDismissal` (dismissing child A's warning must not + dismiss child B's). + """ + workspace_ctx, child_contexts = await self._build_context(workspace_id) + tips: list[Tip] = [] + workspace_rules = [rule for rule in RULE_REGISTRY if rule in _WORKSPACE_SCOPED_RULES] + child_rules = [rule for rule in RULE_REGISTRY if rule not in _WORKSPACE_SCOPED_RULES] + + workspace_results = await asyncio.gather(*(rule(workspace_ctx) for rule in workspace_rules)) + tips.extend(tip for tip in workspace_results if tip is not None) + + for ctx in child_contexts: + child = ctx.children[0] + results = await asyncio.gather(*(rule(ctx) for rule in child_rules)) + for tip in results: + if tip is None: + continue + tip.data = {**tip.data, "child_id": str(child.id), "child_name": child.name} + tip.id = f"{tip.id}:{child.id}" + tips.append(tip) + + return await self._drop_stale_dismissed(workspace_id, tips) + + async def dismiss(self, workspace_id: UUID, tip_id: str) -> None: + """Record that `tip_id` is dismissed, fingerprinted to its current `data`. + + Recomputes the live tip list (rather than trusting a client-supplied + `data` payload) so the stored fingerprint is always the engine's own + truth. Raises NotFoundError if `tip_id` doesn't match any + currently-firing tip — there is nothing meaningful to dismiss. + """ + tips = await self.get_tips(workspace_id) + tip = next((t for t in tips if t.id == tip_id), None) + if tip is None: + raise NotFoundError(f"Tip {tip_id!r} is not currently active for this workspace") + + data_hash = hash_tip_data(tip.data) + existing = await self._dismissals.get_by_workspace_tip(workspace_id, tip_id) + if existing is not None: + existing.data_hash = data_hash + await self._dismissals.update(existing.id, existing) + return + + await self._dismissals.create( + TipDismissal( + id=uuid4(), + workspace_id=workspace_id, + tip_id=tip_id, + data_hash=data_hash, + ) + ) + + async def _drop_stale_dismissed(self, workspace_id: UUID, tips: list[Tip]) -> list[Tip]: + """Filter out tips whose dismissal is still fresh. + + A dismissal only silences the *exact* situation the user acknowledged + (same tip id, same `data`). If the rule now fires with different data + — a bigger `expiring_days` count, a moved `fourth_birthday`, ... — the + old dismissal describes a fact that's no longer true, so the tip + resurfaces instead of staying silenced forever. See module docstring + on `app.entities.tip_dismissal.TipDismissal` for why an unconditional + (non-expiring) dismissal would be dangerous for warnings like + `preserve_for_age_4`. + """ + dismissals = await self._dismissals.list_by_workspace(workspace_id) + if not dismissals: + return tips + dismissed_hash_by_id = {d.tip_id: d.data_hash for d in dismissals} + return [tip for tip in tips if dismissed_hash_by_id.get(tip.id) != hash_tip_data(tip.data)] + + async def _build_context(self, workspace_id: UUID) -> tuple[TipsContext, list[TipsContext]]: + """Build the workspace-level context plus one context per child. + + Two bugs used to live here: + + 1. **Single-child only.** Only `children[0]` was ever fetched, so a + workspace with two children got advice about one and silence about + the other — despite `list_by_workspace` already returning every + child. Now every child gets its own context (see the loop below). + 2. **Doubled planned days.** `list_by_workspace_child` already + returns every parent's days for a given child; the old code called + it again *inside* the per-parent loop and kept appending to the + same list, so an N-parent workspace saw each planned day N times. + Every count-sensitive rule (intensity, SGI, age-4 projections) was + silently working from doubled input. Fetched once per child now, + outside any per-parent loop. + """ parents = await self._parents.list_by_workspace(workspace_id) children = await self._children.list_by_workspace(workspace_id) leave_wishes = await self._leave_wishes.get_by_workspace(workspace_id) - today = date.today() - all_planned: list[PlannedLeaveDay] = [] - leave_by_parent: dict[UUID, ParentalLeaveFetch | None] = {} - vacation_by_parent: dict[UUID, list[VacationDays]] = {} - if children: - child = children[0] # MVP: single child - for pd in parents: - leave_by_parent[pd.uuid] = await self._leave.get_latest_by_parent(pd.uuid, child.id) - future = await self._planned.list_by_workspace_child( - workspace_id, child.id, from_date=today - ) - all_planned.extend(future) - vacation_by_parent[pd.uuid] = await self._vacation.list( - personal_data_uuid=pd.uuid, workspace_id=workspace_id - ) + # Vacation days belong to the parent, not any child — fetched once, + # shared by the workspace context and every per-child context. + vacation_by_parent: dict[UUID, list[VacationDays]] = {} + for pd in parents: + vacation_by_parent[pd.uuid] = await self._vacation.list( + personal_data_uuid=pd.uuid, workspace_id=workspace_id + ) ws = await self._workspace_repo.get_by_id(workspace_id) planning_mode = ws.planning_mode if ws else "solo" - return TipsContext( + workspace_ctx = TipsContext( workspace_id=workspace_id, - children=children, + children=[], parents=parents, - leave_by_parent=leave_by_parent, - planned_days=all_planned, + leave_by_parent={}, + planned_days=[], vacation_by_parent=vacation_by_parent, leave_wishes=leave_wishes, today=today, workspace_planning_mode=planning_mode, ) + + child_contexts: list[TipsContext] = [] + for child in children: + leave_by_parent: dict[UUID, ParentalLeaveFetch | None] = {} + for pd in parents: + leave_by_parent[pd.uuid] = await self._leave.get_latest_by_parent(pd.uuid, child.id) + planned_days = await self._planned.list_by_workspace_child( + workspace_id, child.id, from_date=today + ) + child_contexts.append( + TipsContext( + workspace_id=workspace_id, + children=[child], + parents=parents, + leave_by_parent=leave_by_parent, + planned_days=planned_days, + vacation_by_parent=vacation_by_parent, + leave_wishes=leave_wishes, + today=today, + workspace_planning_mode=planning_mode, + ) + ) + + return workspace_ctx, child_contexts diff --git a/backend/migrations/versions/010_add_tip_dismissal.py b/backend/migrations/versions/010_add_tip_dismissal.py new file mode 100644 index 0000000..e5a4575 --- /dev/null +++ b/backend/migrations/versions/010_add_tip_dismissal.py @@ -0,0 +1,59 @@ +"""Add tip_dismissal table for persisting tip dismissals. + +`useOptimizationTips` only kept dismissed tip ids in a local `Set` — they came +back on every reload. This adds a workspace-scoped dismissal record keyed by +`(workspace_id, tip_id)` so a dismissed tip stays dismissed across reloads, +until the underlying situation changes materially. + +`data_hash` is a fingerprint of the tip's `data` payload at dismiss time +(see `app.utils.tip_predicates.hash_tip_data`). `TipsService` only honors a +dismissal while the rule's current output still hashes the same; once the +numbers move (e.g. more days now projected to expire at age 4), the +dismissal is stale and the tip resurfaces automatically. A dismissal that +never expired could permanently silence a safety-relevant warning such as +`preserve_for_age_4` — that would be worse than not persisting dismissals at +all, so expiry-on-change is not optional here. + +Revision ID: 010 +Revises: 009 +Create Date: 2026-09-17 +""" + +from __future__ import annotations + +from alembic import op + +revision: str = "010" +down_revision: str | None = "009" +branch_labels: str | None = None +depends_on: str | None = None + +_UPGRADE_SQL = """ +CREATE TABLE tip_dismissal ( + id TEXT PRIMARY KEY, + workspace_id TEXT NOT NULL, + tip_id TEXT NOT NULL, + data_hash TEXT NOT NULL, + dismissed_at TIMESTAMP NOT NULL, + UNIQUE(workspace_id, tip_id), + FOREIGN KEY (workspace_id) REFERENCES workspace(id) ON DELETE CASCADE +); +""" + +_DOWNGRADE_SQL = """ +DROP TABLE tip_dismissal; +""" + + +def upgrade() -> None: + from sqlalchemy import text + + for stmt in [s.strip() for s in _UPGRADE_SQL.split(";") if s.strip()]: + op.execute(text(stmt)) + + +def downgrade() -> None: + from sqlalchemy import text + + for stmt in [s.strip() for s in _DOWNGRADE_SQL.split(";") if s.strip()]: + op.execute(text(stmt)) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index e8291fd..93092bd 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -24,6 +24,7 @@ from app.repositories_impl.sqlite.leave_wish_repository import SQLiteLeaveWishRe 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.tip_dismissal_repository import SQLiteTipDismissalRepository from app.repositories_impl.sqlite.user_repository import SQLiteUserRepository from app.repositories_impl.sqlite.vacation_days_repository import ( SQLiteVacationDaysRepository, @@ -39,6 +40,7 @@ from app.services.child_service import ChildService from app.services.day_override_service import DayOverrideService from app.services.leave_wish_service import LeaveWishService from app.services.planning_service import PlanningService +from app.services.tips_service import TipsService from app.services.vacation_days_service import VacationDaysService from app.services.workspace_service import WorkspaceService @@ -308,3 +310,38 @@ async def planning_service( vacation_repo, planning_engine_service, ) + + +# --------------------------------------------------------------------------- +# Tips +# --------------------------------------------------------------------------- + + +@pytest_asyncio.fixture +async def tip_dismissal_repo( + connection: aiosqlite.Connection, +) -> SQLiteTipDismissalRepository: + return SQLiteTipDismissalRepository(connection) + + +@pytest_asyncio.fixture +async def tips_service( + parental_leave_repo: SQLiteParentalLeaveRepository, + planned_leave_repo: SQLitePlannedLeaveRepository, + child_repo: SQLiteChildRepository, + personal_data_repo: SQLitePersonalDataRepository, + vacation_repo: SQLiteVacationDaysRepository, + leave_wish_repo: SQLiteLeaveWishRepository, + workspace_repo: SQLiteWorkspaceRepository, + tip_dismissal_repo: SQLiteTipDismissalRepository, +) -> TipsService: + return TipsService( + parental_leave_repo, + planned_leave_repo, + child_repo, + personal_data_repo, + vacation_repo, + leave_wish_repo, + workspace_repo, + tip_dismissal_repo, + ) diff --git a/backend/tests/unit/test_services/test_tips_service_context.py b/backend/tests/unit/test_services/test_tips_service_context.py new file mode 100644 index 0000000..1923fd9 --- /dev/null +++ b/backend/tests/unit/test_services/test_tips_service_context.py @@ -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 diff --git a/backend/tests/unit/test_tip_dismissal_repository.py b/backend/tests/unit/test_tip_dismissal_repository.py new file mode 100644 index 0000000..a3f3e6a --- /dev/null +++ b/backend/tests/unit/test_tip_dismissal_repository.py @@ -0,0 +1,123 @@ +"""Tests for the TipDismissal repository. + +Follows the DayOverride reference pattern (upsert-by-natural-key, UNIQUE +constraint on the natural key): arrange an entity, exercise the repo, assert +on the result. +""" + +from datetime import datetime +from uuid import uuid4 + +import pytest + +from app.entities.tip_dismissal import TipDismissal +from app.entities.workspace import Workspace +from app.repositories_impl.sqlite.tip_dismissal_repository import SQLiteTipDismissalRepository +from app.repositories_impl.sqlite.workspace_repository import SQLiteWorkspaceRepository +from app.utils.errors import NotFoundError +from tests.conftest import SeededIds + + +def _make(workspace_id=None, **overrides) -> TipDismissal: + base = dict( + id=uuid4(), + workspace_id=workspace_id or uuid4(), + tip_id="preserve_for_age_4:child-1", + data_hash="deadbeef", + dismissed_at=datetime.now(), + ) + base.update(overrides) + return TipDismissal(**base) + + +class TestSQLiteTipDismissalRepository: + async def test_create_and_get_by_id( + self, + tip_dismissal_repo: SQLiteTipDismissalRepository, + seeded_ids: SeededIds, + ) -> None: + entity = _make(workspace_id=seeded_ids.workspace_id) + await tip_dismissal_repo.create(entity) + + fetched = await tip_dismissal_repo.get_by_id(entity.id) + assert fetched is not None + assert fetched.tip_id == "preserve_for_age_4:child-1" + assert fetched.data_hash == "deadbeef" + + async def test_get_by_workspace_tip( + self, + tip_dismissal_repo: SQLiteTipDismissalRepository, + seeded_ids: SeededIds, + ) -> None: + entity = _make(workspace_id=seeded_ids.workspace_id) + await tip_dismissal_repo.create(entity) + + found = await tip_dismissal_repo.get_by_workspace_tip( + seeded_ids.workspace_id, "preserve_for_age_4:child-1" + ) + assert found is not None + assert found.id == entity.id + + missing = await tip_dismissal_repo.get_by_workspace_tip(uuid4(), "no_such_tip") + assert missing is None + + async def test_update_refreshes_hash_and_timestamp( + self, + tip_dismissal_repo: SQLiteTipDismissalRepository, + seeded_ids: SeededIds, + ) -> None: + entity = _make(workspace_id=seeded_ids.workspace_id) + await tip_dismissal_repo.create(entity) + + entity.data_hash = "newhash" + updated = await tip_dismissal_repo.update(entity.id, entity) + assert updated.data_hash == "newhash" + + async def test_update_missing_raises( + self, tip_dismissal_repo: SQLiteTipDismissalRepository + ) -> None: + with pytest.raises(NotFoundError): + await tip_dismissal_repo.update(uuid4(), _make()) + + async def test_delete( + self, + tip_dismissal_repo: SQLiteTipDismissalRepository, + seeded_ids: SeededIds, + ) -> None: + entity = _make(workspace_id=seeded_ids.workspace_id) + await tip_dismissal_repo.create(entity) + await tip_dismissal_repo.delete(entity.id) + assert await tip_dismissal_repo.get_by_id(entity.id) is None + + async def test_delete_missing_raises( + self, tip_dismissal_repo: SQLiteTipDismissalRepository + ) -> None: + with pytest.raises(NotFoundError): + await tip_dismissal_repo.delete(uuid4()) + + async def test_list_by_workspace_filters( + self, + tip_dismissal_repo: SQLiteTipDismissalRepository, + seeded_ids: SeededIds, + workspace_repo: SQLiteWorkspaceRepository, + ) -> None: + ws = seeded_ids.workspace_id + ws2 = await workspace_repo.create( + Workspace(id=uuid4(), name="WS2", created_at=datetime.now()) + ) + + await tip_dismissal_repo.create(_make(workspace_id=ws, tip_id="a")) + await tip_dismissal_repo.create(_make(workspace_id=ws, tip_id="b")) + await tip_dismissal_repo.create(_make(workspace_id=ws2.id, tip_id="c")) + + rows = await tip_dismissal_repo.list_by_workspace(ws) + assert {r.tip_id for r in rows} == {"a", "b"} + + async def test_unique_constraint_per_workspace_tip( + self, + tip_dismissal_repo: SQLiteTipDismissalRepository, + seeded_ids: SeededIds, + ) -> None: + await tip_dismissal_repo.create(_make(workspace_id=seeded_ids.workspace_id, tip_id="x")) + with pytest.raises(Exception): # sqlite3.IntegrityError via aiosqlite + await tip_dismissal_repo.create(_make(workspace_id=seeded_ids.workspace_id, tip_id="x")) diff --git a/backend/tests/unit/test_tip_predicates_hash.py b/backend/tests/unit/test_tip_predicates_hash.py new file mode 100644 index 0000000..4284ea1 --- /dev/null +++ b/backend/tests/unit/test_tip_predicates_hash.py @@ -0,0 +1,30 @@ +"""Tests for `hash_tip_data` — the dismissal-expiry fingerprint. + +See `app.entities.tip_dismissal.TipDismissal` and +`TipsService._drop_stale_dismissed`: a dismissal only stays silent while the +rule's `data` still hashes the same. These tests pin the two properties that +matter for that contract: stability (same data → same hash, regardless of +key order) and sensitivity (any value change → a different hash). +""" + +from app.utils.tip_predicates import hash_tip_data + + +class TestHashTipData: + def test_stable_regardless_of_key_order(self) -> None: + a = hash_tip_data({"parent_name": "Alice", "expiring_days": 12}) + b = hash_tip_data({"expiring_days": 12, "parent_name": "Alice"}) + assert a == b + + def test_changes_when_a_value_changes(self) -> None: + a = hash_tip_data({"parent_name": "Alice", "expiring_days": 12}) + b = hash_tip_data({"parent_name": "Alice", "expiring_days": 13}) + assert a != b + + def test_changes_when_a_key_is_added_or_removed(self) -> None: + a = hash_tip_data({"parent_name": "Alice"}) + b = hash_tip_data({"parent_name": "Alice", "expiring_days": 12}) + assert a != b + + def test_empty_data_hashes_consistently(self) -> None: + assert hash_tip_data({}) == hash_tip_data({}) diff --git a/backend/tests/unit/test_tips_router.py b/backend/tests/unit/test_tips_router.py new file mode 100644 index 0000000..11f3cc6 --- /dev/null +++ b/backend/tests/unit/test_tips_router.py @@ -0,0 +1,159 @@ +"""API-level tests for `GET /api/tips/{workspace_id}` and its dismiss route. + +Guards two things a unit test on the service can't see: + - the wire shape (`TipResponse`) actually carries `anchor_date`/ + `anchor_range` and `message_key` + `data` — not rendered prose; + - the dismiss endpoint round-trips through the real DB and a second GET + no longer returns the dismissed tip. +""" + +from datetime import date, datetime +from uuid import uuid4 + +import aiosqlite +import pytest_asyncio +from httpx import ASGITransport, AsyncClient + +from app.dependencies import get_connection, get_current_user +from app.entities.child import Child +from app.entities.parental_leave import ParentalLeaveFetch, TierData +from app.entities.personal_data import PersonalData +from app.entities.user import User +from app.entities.workspace import Workspace +from app.main import app +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.workspace_repository import SQLiteWorkspaceRepository + + +def _make_fetch(personal_data_uuid, workspace_id, child_id, 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=240, + days_used=used, + days_left=240 - 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, + ) + + +@pytest_asyncio.fixture +async def client(connection: aiosqlite.Connection) -> AsyncClient: + app.dependency_overrides[get_connection] = lambda: connection + app.dependency_overrides[get_current_user] = lambda: User( + id=uuid4(), + personal_data_uuid=uuid4(), + email="test@example.com", + hashed_password="not-a-real-hash", + created_at=datetime.now(), + ) + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: + yield ac + app.dependency_overrides.pop(get_connection, None) + app.dependency_overrides.pop(get_current_user, None) + + +@pytest_asyncio.fixture +async def workspace_with_expiring_days(connection: aiosqlite.Connection): + now = datetime.now() + workspace_repo = SQLiteWorkspaceRepository(connection) + personal_data_repo = SQLitePersonalDataRepository(connection) + child_repo = SQLiteChildRepository(connection) + parental_leave_repo = SQLiteParentalLeaveRepository(connection) + + 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)) + return ws, child + + +class TestGetTipsOverTheWire: + async def test_response_carries_anchor_date_and_no_rendered_prose( + self, + client: AsyncClient, + workspace_with_expiring_days, + ) -> None: + ws, child = workspace_with_expiring_days + + response = await client.get(f"/api/tips/{ws.id}") + assert response.status_code == 200 + body = response.json() + + preserve = next(t for t in body if t["id"].startswith("preserve_for_age_4:")) + assert preserve["id"] == f"preserve_for_age_4:{child.id}" + assert preserve["message_key"] == "tips.preserve_for_age_4" + assert preserve["anchor_date"] == ( + child.birth_date.replace(year=child.birth_date.year + 4).isoformat() + ) + assert preserve["anchor_range"] is None + assert preserve["data"]["child_id"] == str(child.id) + assert preserve["data"]["child_name"] == "Barn" + + # Never rendered Swedish prose — only message_key + data. + for tip in body: + assert isinstance(tip["message_key"], str) and tip["message_key"].startswith("tips.") + for value in tip["data"].values(): + assert not isinstance(value, dict) + + +class TestDismissTipOverTheWire: + async def test_dismiss_then_get_excludes_the_tip( + self, + client: AsyncClient, + workspace_with_expiring_days, + ) -> None: + ws, _child = workspace_with_expiring_days + + before = await client.get(f"/api/tips/{ws.id}") + tip = next(t for t in before.json() if t["id"].startswith("preserve_for_age_4:")) + + dismiss_response = await client.post( + f"/api/tips/{ws.id}/dismissals", json={"tip_id": tip["id"]} + ) + assert dismiss_response.status_code == 204 + + after = await client.get(f"/api/tips/{ws.id}") + assert tip["id"] not in {t["id"] for t in after.json()} + + async def test_dismiss_unknown_tip_returns_404( + self, + client: AsyncClient, + workspace_with_expiring_days, + ) -> None: + ws, _child = workspace_with_expiring_days + + response = await client.post( + f"/api/tips/{ws.id}/dismissals", json={"tip_id": "not_a_real_tip"} + ) + assert response.status_code == 404 diff --git a/frontend/src/hooks/useOptimizationTips.test.ts b/frontend/src/hooks/useOptimizationTips.test.ts index a9f3f83..05130ea 100644 --- a/frontend/src/hooks/useOptimizationTips.test.ts +++ b/frontend/src/hooks/useOptimizationTips.test.ts @@ -61,13 +61,14 @@ describe('useOptimizationTips', () => { expect(result.current.error).toBe('Failed to fetch tips') }) - it('dismisses tips locally', async () => { + it('dismisses tips locally and persists the dismissal', async () => { const mockTips = [ makeTip({ id: 'tip1', severity: 'info', messageKey: 'msg1' }), makeTip({ id: 'tip2', severity: 'warning', messageKey: 'msg2' }), ] vi.mocked(api.getTips).mockResolvedValue(mockTips) + vi.mocked(api.dismissTip).mockResolvedValue(undefined) const { result } = renderHook(() => useOptimizationTips('ws1')) @@ -77,12 +78,45 @@ describe('useOptimizationTips', () => { expect(result.current.data?.length).toBe(2) - act(() => { - result.current.dismiss('tip1') + await act(async () => { + await result.current.dismiss('tip1') }) expect(result.current.data?.length).toBe(1) expect(result.current.data?.[0]?.id).toBe('tip2') + expect(api.dismissTip).toHaveBeenCalledWith('ws1', 'tip1') + }) + + it('keeps a tip hidden locally even if persisting the dismissal fails', async () => { + const mockTips = [makeTip({ id: 'tip1', severity: 'info', messageKey: 'msg1' })] + + vi.mocked(api.getTips).mockResolvedValue(mockTips) + vi.mocked(api.dismissTip).mockRejectedValue(new Error('network down')) + + const { result } = renderHook(() => useOptimizationTips('ws1')) + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + await act(async () => { + await result.current.dismiss('tip1') + }) + + expect(result.current.data?.length).toBe(0) + expect(result.current.error).toContain('network down') + }) + + it('does not call the API when dismissing with no workspace', async () => { + vi.mocked(api.getTips).mockResolvedValue([]) + + const { result } = renderHook(() => useOptimizationTips(null)) + + await act(async () => { + await result.current.dismiss('tip1') + }) + + expect(api.dismissTip).not.toHaveBeenCalled() }) it('provides refresh function', async () => { diff --git a/frontend/src/hooks/useOptimizationTips.ts b/frontend/src/hooks/useOptimizationTips.ts index 6f2e4f9..6e48758 100644 --- a/frontend/src/hooks/useOptimizationTips.ts +++ b/frontend/src/hooks/useOptimizationTips.ts @@ -7,7 +7,7 @@ interface UseOptimizationTipsResult { isLoading: boolean error: string | null refresh: () => Promise - dismiss: (tipId: string) => void + dismiss: (tipId: string) => Promise } export function useOptimizationTips(workspaceId: string | null): UseOptimizationTipsResult { @@ -31,9 +31,24 @@ export function useOptimizationTips(workspaceId: string | null): UseOptimization } }, [workspaceId]) - const dismiss = useCallback((tipId: string) => { - setDismissed((prev) => new Set([...prev, tipId])) - }, []) + const dismiss = useCallback( + async (tipId: string) => { + // Hide it immediately (optimistic) — a dismiss button that waits on + // the network before reacting feels broken. + setDismissed((prev) => new Set([...prev, tipId])) + if (!workspaceId) return + try { + await api.dismissTip(workspaceId, tipId) + } catch (err) { + // Persistence failed: it stays hidden for this session (the local + // Set above), but the next reload will show it again since the + // server never recorded it. Surface the failure rather than + // swallowing it — see AGENTS.md's "no empty catch blocks". + setError(err instanceof Error ? err.message : 'Failed to dismiss tip') + } + }, + [workspaceId], + ) useEffect(() => { fetch() diff --git a/frontend/src/services/api.test.ts b/frontend/src/services/api.test.ts index 91923b5..f6d7bbc 100644 --- a/frontend/src/services/api.test.ts +++ b/frontend/src/services/api.test.ts @@ -248,6 +248,68 @@ describe('API response mappers (snake_case → camelCase)', () => { }) }) + describe('getTips', () => { + it('maps snake_case tips (incl. anchor fields) to camelCase', async () => { + const rawResponse = [ + { + id: 'preserve_for_age_4:child-1', + severity: 'warning' as const, + message_key: 'tips.preserve_for_age_4', + data: { parent_name: 'Alice', expiring_days: 12, child_id: 'child-1' }, + anchor_date: '2030-01-01', + anchor_range: null, + }, + { + id: 'wrap_weekend_with_vacation:child-1', + severity: 'info' as const, + message_key: 'tips.wrap_weekend_with_vacation', + data: { parent_name: 'Bob' }, + anchor_date: null, + anchor_range: ['2026-07-03', '2026-07-06'] as [string, string], + }, + ] + + fetchMock.mockResolvedValue({ + ok: true, + status: 200, + json: async () => rawResponse, + }) + + const result = await api.getTips('ws1') + + expect(result).toHaveLength(2) + expect(result[0]!.id).toBe('preserve_for_age_4:child-1') + expect(result[0]!.messageKey).toBe('tips.preserve_for_age_4') + expect(result[0]!.anchorDate).toBe('2030-01-01') + expect(result[0]!.anchorRange).toBeNull() + expect(result[0]!.data.expiring_days).toBe(12) + + expect(result[1]!.anchorDate).toBeNull() + expect(result[1]!.anchorRange).toEqual(['2026-07-03', '2026-07-06']) + }) + }) + + describe('dismissTip', () => { + it('posts tip_id to the dismissals endpoint', async () => { + fetchMock.mockResolvedValue({ + ok: true, + status: 204, + json: async () => null, + }) + + await api.dismissTip('ws1', 'preserve_for_age_4:child-1') + + const calls = vi.mocked(fetchMock).mock.calls + if (calls.length === 0) throw new Error('No fetch calls made') + const [url, options] = calls[0]! + const body = JSON.parse(options.body) + + expect(String(url)).toContain('/tips/ws1/dismissals') + expect(options.method).toBe('POST') + expect(body.tip_id).toBe('preserve_for_age_4:child-1') + }) + }) + describe('createLeaveWish', () => { it('sends camelCase request as snake_case and maps the response back', async () => { const rawResponse = { diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 7ee95ac..fd7fce5 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -214,6 +214,8 @@ export interface RawTip { severity: 'info' | 'warning' | 'action' message_key: string data: Record + anchor_date: string | null + anchor_range: [string, string] | null } function toTip(raw: RawTip): Tip { @@ -222,6 +224,8 @@ function toTip(raw: RawTip): Tip { severity: raw.severity, messageKey: raw.message_key, data: raw.data, + anchorDate: raw.anchor_date, + anchorRange: raw.anchor_range, } } @@ -1007,6 +1011,13 @@ export async function getTips(workspaceId: string): Promise { return raw.map(toTip) } +export async function dismissTip(workspaceId: string, tipId: string): Promise { + return apiFetch(`/tips/${workspaceId}/dismissals`, { + method: 'POST', + body: JSON.stringify({ tip_id: tipId }), + }) +} + // --------------------------------------------------------------------------- // Parental Leave Fetch // --------------------------------------------------------------------------- diff --git a/frontend/src/types/entities.ts b/frontend/src/types/entities.ts index 864d391..fb09403 100644 --- a/frontend/src/types/entities.ts +++ b/frontend/src/types/entities.ts @@ -216,6 +216,11 @@ export interface Tip { severity: 'info' | 'warning' | 'action' messageKey: string data: Record + // Where this tip is anchored on a calendar: a single day, a span, or + // neither (many rules — e.g. balance_between_parents — have no date that + // makes sense to place on a calendar rail). Never both set at once. + anchorDate: string | null // ISO-8601 date + anchorRange: [string, string] | null // [ISO-8601 date, ISO-8601 date] } // Planning engine — POST /api/plan/propose (read-only preview, nothing is saved) diff --git a/frontend/tests/fixtures/mockData.ts b/frontend/tests/fixtures/mockData.ts index c1487c3..5fb190c 100644 --- a/frontend/tests/fixtures/mockData.ts +++ b/frontend/tests/fixtures/mockData.ts @@ -202,6 +202,8 @@ export function makeTip(overrides: Partial = {}): Tip { severity: 'info', messageKey: 'tips.example', data: {}, + anchorDate: null, + anchorRange: null, ...overrides, } }