Fix four bugs found by browser verification
First real browser walkthrough of the app, against seeded demo data. All 1140 tests were passing; none of these four were visible to them. Age-4 entitlement (high severity, domain bug). The no-snapshot fallback gave a single parent rules.sjukpenning_days + rules.lagsta_niva_days = 390 + 90 = 480 days, but those config values are the whole CHILD's combined tier totals across both parents, not one parent's share. The correct fallback is days_per_parent (240 = 195 + 45), which only AnalyticsService used. So a parent without an FK snapshot was credited with roughly double their real entitlement, and the Dashboard and Plan tab disagreed about expiring_at_b4 for the same committed state. The wrong default existed in three places; allocation_service.py and planning_service.py are corrected, and analytics_service.py now calls the shared pure days_left_at_age_4_projection instead of re-deriving the arithmetic, so there is one source of truth. A test that had encoded the buggy 390/90 default as expected behaviour is corrected too. The Dashboard stat is workspace-wide while the Plan tab breaks down per parent, which is legitimate but was unlabelled and contributed to the confusion; the dashboard label now says it is a combined all-parents total. Nonsensical remedy text. lower_priority_wish computed max(other.target_weeks // 2, 1), which for a 1-week wish clamped back to 1 and rendered "Lowering Var from 1 to 1 weeks frees up 32 days". target_weeks has a domain floor of 1 and the planner works in whole days, so there is no smaller whole-week suggestion to make: the remedy is now suppressed for 1-week wishes. Wishes of 2+ weeks still get a genuine, strictly smaller suggestion. Day override invisible until reload. Calendar refetched the overrides list but not GET /api/calendar/days, which backs the grid's shading and badges, so a just-applied override did not render. useCalendarDays now exposes refetch and Calendar calls it after a successful upsert or remove. Dashboard showed the previous user's data after a login swap. WorkspaceContext sits above the router and outlives login/logout, and its refetch was gated on !workspaceId || children.length === 0 - so precisely because the old session's data was still present, the guard blocked the refetch. It now keys off an explicit isAuthenticated transition and clears state on the logout edge itself, including the persisted localStorage workspaceId, so one account's figures can never be briefly visible under another's session. useAnalytics, useParentalLeave and usePlannedDays clear cached data on identity loss as defence in depth. Backend 435 -> 439 tests, frontend 705 -> 710. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXMhc9nT6sNz1SL4j8dznL
This commit is contained in:
@@ -1116,29 +1116,40 @@ class _Ledger:
|
||||
]
|
||||
if siblings:
|
||||
other = min(siblings, key=lambda o: o.priority)
|
||||
other_start, other_end = other.window_for_year(other.first_year)
|
||||
other_dates = _dates_in(other_start, other_end)
|
||||
spendable = sum(
|
||||
1
|
||||
for d in other_dates
|
||||
if not is_free_day(d, self._input.holidays, self._input.day_overrides)
|
||||
)
|
||||
half_weeks = max(other.target_weeks // 2, 1)
|
||||
freed_estimate = round(spendable / 2)
|
||||
if freed_estimate > 0:
|
||||
remedies.append(
|
||||
ShortfallRemedy(
|
||||
kind=REMEDY_LOWER_PRIORITY_WISH,
|
||||
message_key=f"planRemedies.{REMEDY_LOWER_PRIORITY_WISH}",
|
||||
data={
|
||||
"wish_name": other.name,
|
||||
"from_weeks": other.target_weeks,
|
||||
"to_weeks": half_weeks,
|
||||
"freed_days": freed_estimate,
|
||||
},
|
||||
days_freed=freed_estimate,
|
||||
)
|
||||
# A whole-week target can only be halved to a genuinely
|
||||
# smaller whole week count when it's at least 2 weeks to
|
||||
# begin with (`target_weeks` has a domain floor of 1 — see
|
||||
# `LeaveWishCreate.target_weeks`). At exactly 1 week,
|
||||
# `target_weeks // 2 == 0`, which used to get clamped back
|
||||
# up to 1, producing a nonsensical "from 1 to 1 weeks"
|
||||
# no-op remedy. Suppress the suggestion entirely rather
|
||||
# than claim a change that isn't one — the planner works in
|
||||
# whole days/weeks, so there's no fractional week to offer
|
||||
# instead.
|
||||
half_weeks = other.target_weeks // 2
|
||||
if half_weeks >= 1:
|
||||
other_start, other_end = other.window_for_year(other.first_year)
|
||||
other_dates = _dates_in(other_start, other_end)
|
||||
spendable = sum(
|
||||
1
|
||||
for d in other_dates
|
||||
if not is_free_day(d, self._input.holidays, self._input.day_overrides)
|
||||
)
|
||||
freed_estimate = round(spendable / 2)
|
||||
if freed_estimate > 0:
|
||||
remedies.append(
|
||||
ShortfallRemedy(
|
||||
kind=REMEDY_LOWER_PRIORITY_WISH,
|
||||
message_key=f"planRemedies.{REMEDY_LOWER_PRIORITY_WISH}",
|
||||
data={
|
||||
"wish_name": other.name,
|
||||
"from_weeks": other.target_weeks,
|
||||
"to_weeks": half_weeks,
|
||||
"freed_days": freed_estimate,
|
||||
},
|
||||
days_freed=freed_estimate,
|
||||
)
|
||||
)
|
||||
|
||||
shift_gain = 0
|
||||
probe = w.start - timedelta(days=1)
|
||||
@@ -1890,8 +1901,16 @@ class AllocationInputLoader:
|
||||
lagsta_left = tier.lagsta_niva_left
|
||||
dubbel_left = latest.dubbeldagar_left
|
||||
else:
|
||||
sjuk_left = rules.sjukpenning_days
|
||||
lagsta_left = rules.lagsta_niva_days
|
||||
# No FK snapshot yet: `rules.sjukpenning_days`/`lagsta_niva_days`
|
||||
# are the WHOLE CHILD's combined tier totals (390 + 90, split
|
||||
# across both parents) — assuming that full amount for a single
|
||||
# parent double-counts the pool. Halve it to get this parent's
|
||||
# default share, matching AnalyticsService's no-snapshot
|
||||
# fallback (`rules.days_per_parent` == 195 + 45 == 240) and the
|
||||
# manual-entry convention (see scripts/seed_demo.py, and
|
||||
# `fk_scraper.py`'s "= rules.lagsta_niva_days/2" comment).
|
||||
sjuk_left = rules.sjukpenning_days // 2
|
||||
lagsta_left = rules.lagsta_niva_days // 2
|
||||
dubbel_left = rules.max_dubbeldagar
|
||||
|
||||
# Net out days already committed locally (planned_leave_day)
|
||||
|
||||
@@ -25,6 +25,7 @@ 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.utils.errors import NotFoundError
|
||||
from app.utils.tip_predicates import days_left_at_age_4_projection
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -107,10 +108,15 @@ class AnalyticsService:
|
||||
)
|
||||
planned_fp_before_b4 = sjukpenning_before_b4 + lagsta_before_b4
|
||||
|
||||
projected_used = days_used + planned_fp_before_b4
|
||||
projected_left = total_days - projected_used
|
||||
# Shared with the plan-proposal side (`allocation_service._Ledger.
|
||||
# build_age4_projections`) so the two surfaces can never re-derive
|
||||
# this arithmetic differently — see `days_left_at_age_4_projection`.
|
||||
projected_left, expiring = days_left_at_age_4_projection(
|
||||
days_used=days_used,
|
||||
planned_fp_before_b4=planned_fp_before_b4,
|
||||
total_days=total_days,
|
||||
)
|
||||
savable = min(projected_left, rules.max_savable_days_at_age_4)
|
||||
expiring = max(projected_left - rules.max_savable_days_at_age_4, 0)
|
||||
|
||||
return Age4Projection(
|
||||
personal_data_uuid=personal_data_uuid,
|
||||
|
||||
@@ -469,8 +469,13 @@ class PlanningService:
|
||||
sjuk_total_left = latest.tier_data.sjukpenning_left
|
||||
lagsta_total_left = latest.tier_data.lagsta_niva_left
|
||||
else:
|
||||
sjuk_total_left = rules.sjukpenning_days
|
||||
lagsta_total_left = rules.lagsta_niva_days
|
||||
# `rules.sjukpenning_days`/`lagsta_niva_days` are the WHOLE
|
||||
# CHILD's combined tier totals across both parents — halve them
|
||||
# for this parent's default share, matching
|
||||
# `AllocationInputLoader.load`'s no-snapshot fallback (Bug A
|
||||
# fix: the two used to disagree, 390/90 vs. 195/45).
|
||||
sjuk_total_left = rules.sjukpenning_days // 2
|
||||
lagsta_total_left = rules.lagsta_niva_days // 2
|
||||
|
||||
committed_sjuk = await self._repo.count_by_parent_type(parent_uuid, child_id, "sjukpenning")
|
||||
committed_lagsta = await self._repo.count_by_parent_type(parent_uuid, child_id, "lagsta")
|
||||
|
||||
@@ -9,7 +9,7 @@ The "nothing written on refusal" assertions check actual repository state
|
||||
property this test file cares about most.
|
||||
"""
|
||||
|
||||
from datetime import date, datetime
|
||||
from datetime import date, datetime, timedelta
|
||||
from uuid import uuid4
|
||||
|
||||
from app.entities.parental_leave import ParentalLeaveFetch, TierData
|
||||
@@ -253,6 +253,48 @@ class TestApplyProposedDaysValidation:
|
||||
)
|
||||
assert rows == []
|
||||
|
||||
async def test_no_snapshot_default_balance_matches_allocation_engine(
|
||||
self,
|
||||
planning_service: PlanningService,
|
||||
planned_leave_repo: SQLitePlannedLeaveRepository,
|
||||
seeded_ids: SeededIds,
|
||||
) -> None:
|
||||
"""Bug A regression, third copy: with no FK snapshot at all, this
|
||||
parent's default tier balance must be their per-parent SHARE (195
|
||||
sjukpenning / 45 lagsta) of `rules.sjukpenning_days`/`lagsta_niva_days`
|
||||
(390/90, the WHOLE CHILD's combined total across both parents) — not
|
||||
the full 390/90, which `AllocationInputLoader.load` also used to
|
||||
wrongly assume for a single parent before the fix. A batch of exactly
|
||||
195 sjukpenning days must be accepted; one more must be rejected.
|
||||
"""
|
||||
ws, child, parent = (
|
||||
seeded_ids.workspace_id,
|
||||
seeded_ids.child_id,
|
||||
seeded_ids.personal_data_uuid,
|
||||
)
|
||||
# No ParentalLeaveFetch created -> exercises the default fallback.
|
||||
one_too_many = [
|
||||
ProposedDayInput(date(2026, 1, 1) + timedelta(days=i), parent, child, "sjukpenning")
|
||||
for i in range(196) # 195 default share + 1 over
|
||||
]
|
||||
|
||||
result = await planning_service.apply_proposed_days(ws, one_too_many)
|
||||
|
||||
assert result.accepted is False
|
||||
assert any(
|
||||
r.message_key == "planApply.rejection.insufficientBalance"
|
||||
and r.data["tier"] == "sjukpenning"
|
||||
for r in result.rejections
|
||||
)
|
||||
rows = await planned_leave_repo.list_by_workspace_child(
|
||||
workspace_id=ws, child_id=child, from_date=date(2026, 1, 1), to_date=date(2027, 1, 1)
|
||||
)
|
||||
assert rows == []
|
||||
|
||||
exactly_the_share = one_too_many[:195]
|
||||
result_ok = await planning_service.apply_proposed_days(ws, exactly_the_share)
|
||||
assert result_ok.accepted is True
|
||||
|
||||
async def test_batch_exceeding_remaining_balance_rejected_whole(
|
||||
self,
|
||||
planning_service: PlanningService,
|
||||
|
||||
@@ -210,9 +210,14 @@ class TestAllocationInputLoader:
|
||||
assert pool_with_fetch.lagsta_left == 90
|
||||
|
||||
# No FK snapshot at all for parent_without_fetch -> default entitlement.
|
||||
# `rules.sjukpenning_days`/`lagsta_niva_days` (390/90) are the WHOLE
|
||||
# CHILD's combined tier totals across both parents, so a single
|
||||
# parent's default share is half of each (195/45, summing to
|
||||
# `rules.days_per_parent` == 240) — matching AnalyticsService's own
|
||||
# no-snapshot fallback (Bug A regression: these two must agree).
|
||||
pool_defaulted = alloc_input.pools[(parent_without_fetch.uuid, child.id)]
|
||||
assert pool_defaulted.sjukpenning_left == 390
|
||||
assert pool_defaulted.lagsta_left == 90
|
||||
assert pool_defaulted.sjukpenning_left == 195
|
||||
assert pool_defaulted.lagsta_left == 45
|
||||
|
||||
vac = alloc_input.vacation[(parent_with_fetch.uuid, 2027)]
|
||||
assert vac.total_days == 25
|
||||
|
||||
@@ -420,6 +420,70 @@ class TestRemedies:
|
||||
carry = next(r for r in outcome.remedies if r.kind == "carry_vacation_forward")
|
||||
assert carry.days_freed == 13
|
||||
|
||||
def test_lower_priority_wish_suppressed_when_target_is_already_one_week(self) -> None:
|
||||
"""Bug B regression: a 1-week sibling wish has no smaller whole-week
|
||||
target to suggest (`target_weeks` floors at 1 — see
|
||||
`LeaveWishCreate.target_weeks`). Halving used to clamp back up to 1,
|
||||
producing a nonsensical "lower Vår from 1 to 1 weeks" remedy. It must
|
||||
be suppressed instead of claiming a change that isn't one."""
|
||||
child = _child(birth_date=date(2025, 1, 1))
|
||||
parent = _parent(salary_type="hourly", min_vacation_days_per_year=20)
|
||||
wish_high = _wish(
|
||||
child_id=child.id, name="Feb", start=FEB_2027[0], end=FEB_2027[1], weeks=4, priority=1
|
||||
)
|
||||
wish_low = _wish(
|
||||
child_id=child.id, name="Vår", start=AUG_2027[0], end=AUG_2027[1], weeks=1, priority=2
|
||||
)
|
||||
alloc_input = _build_input(
|
||||
today=date(2027, 1, 1),
|
||||
horizon_end=date(2027, 12, 31),
|
||||
planning_goal="max_money",
|
||||
planning_mode="solo",
|
||||
children=[child],
|
||||
parents=[parent],
|
||||
pools={(parent.uuid, child.id): DayPool(5, 0, 60)},
|
||||
vacation={(parent.uuid, 2026): VacationBudget(0, 0, 0)}, # no carry-forward remedy
|
||||
wishes=[wish_high, wish_low],
|
||||
)
|
||||
|
||||
proposal = Allocator().propose(alloc_input)
|
||||
|
||||
outcome_high = next(o for o in proposal.outcomes if o.wish_name == "Feb")
|
||||
assert outcome_high.shortfall_days > 0
|
||||
assert "lower_priority_wish" not in {r.kind for r in outcome_high.remedies}
|
||||
|
||||
def test_lower_priority_wish_reports_a_genuine_reduction(self) -> None:
|
||||
"""A sibling wish of >= 2 weeks halves to a strictly smaller whole
|
||||
week count, so the remedy fires with a real (not no-op) suggestion."""
|
||||
child = _child(birth_date=date(2025, 1, 1))
|
||||
parent = _parent(salary_type="hourly", min_vacation_days_per_year=20)
|
||||
wish_high = _wish(
|
||||
child_id=child.id, name="Feb", start=FEB_2027[0], end=FEB_2027[1], weeks=4, priority=1
|
||||
)
|
||||
wish_low = _wish(
|
||||
child_id=child.id, name="Aug", start=AUG_2027[0], end=AUG_2027[1], weeks=2, priority=2
|
||||
)
|
||||
alloc_input = _build_input(
|
||||
today=date(2027, 1, 1),
|
||||
horizon_end=date(2027, 12, 31),
|
||||
planning_goal="max_money",
|
||||
planning_mode="solo",
|
||||
children=[child],
|
||||
parents=[parent],
|
||||
pools={(parent.uuid, child.id): DayPool(5, 0, 60)},
|
||||
vacation={(parent.uuid, 2026): VacationBudget(0, 0, 0)},
|
||||
wishes=[wish_high, wish_low],
|
||||
)
|
||||
|
||||
proposal = Allocator().propose(alloc_input)
|
||||
|
||||
outcome_high = next(o for o in proposal.outcomes if o.wish_name == "Feb")
|
||||
remedy = next(r for r in outcome_high.remedies if r.kind == "lower_priority_wish")
|
||||
assert remedy.data["from_weeks"] == 2
|
||||
assert remedy.data["to_weeks"] == 1
|
||||
assert remedy.data["from_weeks"] != remedy.data["to_weeks"]
|
||||
assert remedy.days_freed > 0
|
||||
|
||||
|
||||
class TestDayOverrides:
|
||||
"""`AllocationInput.day_overrides` — see `DayOverride` / `is_free_day`.
|
||||
|
||||
@@ -15,7 +15,10 @@ from app.entities.parental_leave import ParentalLeaveFetch, TierData
|
||||
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.leave_wish_repository import SQLiteLeaveWishRepository
|
||||
from app.repositories_impl.sqlite.planned_leave_repository import SQLitePlannedLeaveRepository
|
||||
from app.repositories_impl.sqlite.vacation_days_repository import SQLiteVacationDaysRepository
|
||||
from app.repositories_impl.sqlite.workspace_repository import SQLiteWorkspaceRepository
|
||||
from app.services.analytics_service import AnalyticsService
|
||||
from tests.conftest import SeededIds
|
||||
|
||||
@@ -317,3 +320,87 @@ class TestGetOverview:
|
||||
)
|
||||
with pytest.raises(NotFoundError):
|
||||
await service.get_overview(uuid4(), uuid4())
|
||||
|
||||
|
||||
class TestAgreesWithPlanProposalForTheSameCommittedState:
|
||||
"""Bug A regression: the Dashboard (`AnalyticsService`) and the Plan tab's
|
||||
propose-time projection (`allocation_service._Ledger.build_age4_projections`,
|
||||
via `Allocator`/`AllocationInputLoader`) must compute the same
|
||||
`expiring_at_b4` for a parent with no FK snapshot yet and no leave wishes
|
||||
(so the proposal adds nothing new) — i.e. the *committed* state.
|
||||
|
||||
Before the fix, `AllocationInputLoader`'s no-snapshot fallback assumed a
|
||||
single parent was entitled to the WHOLE child's combined tier totals
|
||||
(`rules.sjukpenning_days` + `rules.lagsta_niva_days` == 390 + 90 == 480)
|
||||
instead of that parent's fair half-share (195 + 45 == 240, matching
|
||||
`AnalyticsService`'s own fallback, `rules.days_per_parent`). That made the
|
||||
Plan tab report roughly double what the Dashboard showed for the exact
|
||||
same (empty) committed plan — this test pins the two together.
|
||||
"""
|
||||
|
||||
async def test_no_snapshot_no_wishes_matches_dashboard(
|
||||
self,
|
||||
workspace_repo: SQLiteWorkspaceRepository,
|
||||
child_repo: SQLiteChildRepository,
|
||||
personal_data_repo: SQLitePersonalDataRepository,
|
||||
parental_leave_repo: SQLiteParentalLeaveRepository,
|
||||
planned_leave_repo: SQLitePlannedLeaveRepository,
|
||||
vacation_repo: SQLiteVacationDaysRepository,
|
||||
leave_wish_repo: SQLiteLeaveWishRepository,
|
||||
) -> None:
|
||||
from app.entities.personal_data import PersonalData
|
||||
from app.entities.workspace import Workspace
|
||||
from app.services.allocation_service import AllocationInputLoader, Allocator
|
||||
|
||||
now = datetime.now()
|
||||
ws = await workspace_repo.create(
|
||||
Workspace(id=uuid4(), name="Familj", planning_mode="couple", created_at=now)
|
||||
)
|
||||
parent = await personal_data_repo.create(
|
||||
PersonalData(
|
||||
uuid=uuid4(),
|
||||
workspace_id=ws.id,
|
||||
name="Sam",
|
||||
salary_type="monthly",
|
||||
employment_status="employed",
|
||||
created_at=now,
|
||||
)
|
||||
)
|
||||
birth = date.today() + relativedelta(years=2)
|
||||
child = _make_child(ws.id, birth)
|
||||
await child_repo.create(child)
|
||||
# Deliberately no ParentalLeaveFetch and no leave wishes for `parent`
|
||||
# -> both surfaces must fall back to default entitlement.
|
||||
|
||||
# -- Dashboard side --
|
||||
analytics = await _make_service(
|
||||
child_repo, parental_leave_repo, planned_leave_repo, personal_data_repo
|
||||
)
|
||||
dashboard_proj = await analytics.get_days_left_at_age_4(parent.uuid, child.id)
|
||||
|
||||
# -- Plan-tab side --
|
||||
loader = AllocationInputLoader(
|
||||
workspace_repo=workspace_repo,
|
||||
child_repo=child_repo,
|
||||
personal_data_repo=personal_data_repo,
|
||||
parental_leave_repo=parental_leave_repo,
|
||||
vacation_repo=vacation_repo,
|
||||
planned_leave_repo=planned_leave_repo,
|
||||
leave_wish_repo=leave_wish_repo,
|
||||
)
|
||||
alloc_input = await loader.load(ws.id, today=date.today())
|
||||
proposal = Allocator().propose(alloc_input)
|
||||
assert proposal.days == [] # no wishes -> nothing new proposed
|
||||
plan_proj = next(
|
||||
p
|
||||
for p in proposal.age4_projections
|
||||
if p.parent_personal_data_uuid == parent.uuid and p.child_id == child.id
|
||||
)
|
||||
|
||||
assert dashboard_proj.projected_left_at_b4 == plan_proj.projected_left_at_b4
|
||||
assert dashboard_proj.savable_after_b4 == plan_proj.savable_after_b4
|
||||
assert dashboard_proj.expiring_at_b4 == plan_proj.expiring_at_b4
|
||||
# Pin the actual figures too, so a future regression of either
|
||||
# fallback (240 vs. 480) fails loudly rather than merely disagreeing.
|
||||
assert dashboard_proj.projected_left_at_b4 == 240
|
||||
assert dashboard_proj.expiring_at_b4 == 144
|
||||
|
||||
Reference in New Issue
Block a user