From 8c5751806dcdeec2ae845086173285c607ae8c45 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Fri, 2 Oct 2026 07:49:03 -0400 Subject: [PATCH 1/3] Send explicit UK claimant roles and stop reading an adult dependant as the partner The UK builder puts everyone in one benefit unit and sent only ages, so policyengine-uk inferred the couple from ages: a lone parent aged 50 with a dependant aged 25 was assessed as a couple (PolicyEngine/policyengine-uk#2039). The builder made the same mistake itself: getBuilderPartnerKey fell back to any other adult, so setting a dependant's age to 25 flipped the composition to married, and choosing "single" then deleted the dependant. - getBuilderPartnerKey now returns only an explicit partner: the US marital unit, "your partner", or a member flagged is_claimant_or_partner for the year. Age never makes a partner. This applies to US and UK builders. - Household.getBuilderClaimantRoles / withBuilderClaimantRoles give a UK builder household ("you" present) explicit roles: "you" and the explicit partner true, every other member false. Values already set are kept. - withSupportedBuilderClaimantRoles adds those roles before every save (standalone builder, report-builder modal create and update, report year copies) only when the loaded UK model information defines the variable. policyengine-core rejects a situation naming an unknown variable, and the live API (policyengine-uk 2.90.2) does not have it yet, so saves are unchanged until it ships. - New children are numbered after existing dependants, so adding a child next to an adult dependant gives "your second dependent", not "your first dependent 2". Tests: composition cases for UK and US single parents and couples with a dependant aged 25, role and codec round trips (v1 payload, Python package, saved household), the metadata gate, every save site, and fast-check properties over random builder action sequences. Co-Authored-By: Claude Opus 5.5 --- app/package.json | 1 + app/src/models/Household.ts | 68 ++++++- .../hooks/useModifyReportSubmission.ts | 4 +- .../hooks/useReportSubmission.ts | 3 + .../modals/HouseholdCreationModal.tsx | 15 +- .../utils/createReportSimulations.ts | 15 +- .../views/population/HouseholdBuilderView.tsx | 6 +- .../models/builderClaimantRolesMocks.ts | 99 +++++++++ .../household/UKBuilderClaimantRoles.test.tsx | 108 ++++++++++ app/src/tests/unit/models/Household.test.ts | 123 +++++++++++- .../householdBuilderRoles.property.test.ts | 190 ++++++++++++++++++ .../modals/HouseholdCreationModal.test.tsx | 134 ++++++++++-- .../utils/createReportSimulations.test.ts | 63 +++++- .../unit/utils/builderClaimantRoles.test.ts | 70 +++++++ .../utils/householdBuilderComposition.test.ts | 103 +++++++++- app/src/utils/builderClaimantRoles.ts | 28 +++ bun.lock | 1 + 17 files changed, 1002 insertions(+), 29 deletions(-) create mode 100644 app/src/tests/fixtures/models/builderClaimantRolesMocks.ts create mode 100644 app/src/tests/integration/household/UKBuilderClaimantRoles.test.tsx create mode 100644 app/src/tests/unit/models/householdBuilderRoles.property.test.ts create mode 100644 app/src/tests/unit/utils/builderClaimantRoles.test.ts create mode 100644 app/src/utils/builderClaimantRoles.ts diff --git a/app/package.json b/app/package.json index 1aec39bfe..98dbc1bbf 100644 --- a/app/package.json +++ b/app/package.json @@ -97,6 +97,7 @@ "eslint": "^9.29.0", "eslint-plugin-jsx-a11y": "^6.10.2", "eslint-plugin-react": "^7.37.5", + "fast-check": "^4.10.2", "identity-obj-proxy": "^3.0.0", "jsdom": "^26.1.0", "postcss": "^8.5.5", diff --git a/app/src/models/Household.ts b/app/src/models/Household.ts index 630034c85..80f1d9c43 100644 --- a/app/src/models/Household.ts +++ b/app/src/models/Household.ts @@ -59,6 +59,12 @@ const BUILDER_PRIMARY_PERSON_NAME = 'you'; const BUILDER_DEFAULT_PARTNER_NAME = 'your partner'; const BUILDER_DEPENDENT_ORDINALS = ['first', 'second', 'third', 'fourth', 'fifth']; +/** + * policyengine-uk person input naming the benefit unit's claimant and partner. Without it the + * model infers the couple from ages, so an adult dependant can be read as the partner. + */ +export const UK_CLAIMANT_OR_PARTNER_VARIABLE = 'is_claimant_or_partner'; + export interface PersonWithName extends AppHouseholdInputPerson { name: string; } @@ -884,8 +890,14 @@ export class Household extends BaseModel { return BUILDER_DEFAULT_PARTNER_NAME; } - const adults = this.getAdults(year).map((person) => person.name); - return adults.find((personKey) => personKey !== resolvedPrimaryPersonKey) ?? null; + // Only an explicit partner counts. Age never makes one: a dependant aged 25 stays a dependant. + return ( + this.getSortedPersonNames().find( + (personKey) => + personKey !== resolvedPrimaryPersonKey && + this.getPersonVariableAtYear(personKey, UK_CLAIMANT_OR_PARTNER_VARIABLE, year) === true + ) ?? null + ); } getBuilderChildKeys( @@ -926,6 +938,45 @@ export class Household extends BaseModel { }; } + /** + * Benefit-unit roles of a UK builder household: "you" and the explicit partner are the + * claimant and partner, and every other member (a dependant of any age) is neither. + * Returns null for a household the builder did not make (no "you"), whose relationships the + * app does not know; policyengine-uk then infers them as before. + */ + getBuilderClaimantRoles(year: string): Record | null { + if (this.countryId !== 'uk' || !(BUILDER_PRIMARY_PERSON_NAME in this.appInputData.people)) { + return null; + } + + const { primaryPersonKey, partnerKey } = this.deriveBuilderComposition(year); + return Object.fromEntries( + this.personNames.map((personKey) => [ + personKey, + personKey === primaryPersonKey || personKey === partnerKey, + ]) + ); + } + + /** Records getBuilderClaimantRoles for the year, keeping any value a person already has. */ + withBuilderClaimantRoles(year: string): Household { + const roles = this.getBuilderClaimantRoles(year); + if (!roles) { + return this; + } + + const nextData = cloneAppHouseholdInputData(this.appInputData); + const normalizedYear = normalizeYear(year); + for (const [personKey, isClaimantOrPartner] of Object.entries(roles)) { + const person = nextData.people[personKey]; + if (getYearValue(person[UK_CLAIMANT_OR_PARTNER_VARIABLE], normalizedYear) === undefined) { + setYearValue(person, UK_CLAIMANT_OR_PARTNER_VARIABLE, normalizedYear, isClaimantOrPartner); + } + } + + return this.withHouseholdData(nextData); + } + getHeadPersonName(year: string | null | undefined): string | null { const people = this.appInputData.people; const orderedPeople = this.getSortedPersonNames(); @@ -1175,9 +1226,16 @@ export class Household extends BaseModel { const parentIds = [composition.primaryPersonKey, composition.partnerKey].filter( Boolean ) as string[]; - - for (let index = composition.childKeys.length; index < newCount; index += 1) { - const childKey = getUniqueBuilderPersonName(existingKeys, getBuilderDependentName(index)); + // Number new children after every dependant, including adult dependants the count excludes. + const dependantCount = composition.people.filter( + (personKey) => !parentIds.includes(personKey) + ).length; + + for (let offset = 0; offset < newCount - composition.childKeys.length; offset += 1) { + const childKey = getUniqueBuilderPersonName( + existingKeys, + getBuilderDependentName(dependantCount + offset) + ); existingKeys.add(childKey); nextHousehold = nextHousehold.addChild(childKey, BUILDER_DEFAULT_CHILD_AGE, parentIds, { employment_income: BUILDER_DEFAULT_EMPLOYMENT_INCOME, diff --git a/app/src/pages/reportBuilder/hooks/useModifyReportSubmission.ts b/app/src/pages/reportBuilder/hooks/useModifyReportSubmission.ts index 381932924..5299b15ce 100644 --- a/app/src/pages/reportBuilder/hooks/useModifyReportSubmission.ts +++ b/app/src/pages/reportBuilder/hooks/useModifyReportSubmission.ts @@ -49,6 +49,7 @@ export function useModifyReportSubmission({ onSuccess, }: UseModifyReportSubmissionArgs): UseModifyReportSubmissionReturn { const currentLawId = useSelector((state: RootState) => state.metadata.currentLawId); + const metadata = useSelector((state: RootState) => state.metadata); const manager = useCalcOrchestratorManager(); const updateReportAssociation = useUpdateReportAssociation(); const queryClient = useQueryClient(); @@ -68,6 +69,7 @@ export function useModifyReportSubmission({ countryId, currentLawId, reportYear: reportState.year, + metadata, }); const reportPayload = ReportAdapter.toCreationPayload({ @@ -78,7 +80,7 @@ export function useModifyReportSubmission({ } as Report); return { simulationIds, simulations, simulationStates, reportPayload }; - }, [reportState, countryId, currentLawId]); + }, [reportState, countryId, currentLawId, metadata]); /** * Shared logic: start calculation after a report is created. diff --git a/app/src/pages/reportBuilder/hooks/useReportSubmission.ts b/app/src/pages/reportBuilder/hooks/useReportSubmission.ts index 0545445fb..4e65f3e45 100644 --- a/app/src/pages/reportBuilder/hooks/useReportSubmission.ts +++ b/app/src/pages/reportBuilder/hooks/useReportSubmission.ts @@ -52,6 +52,7 @@ export function useReportSubmission({ onSuccess, }: UseReportSubmissionArgs): UseReportSubmissionReturn { const currentLawId = useSelector((state: RootState) => state.metadata.currentLawId); + const metadata = useSelector((state: RootState) => state.metadata); const [submissionError, setSubmissionError] = useState(null); const [isSubmitting, setIsSubmitting] = useState(false); const { createReport } = useCreateReport(reportState.label || undefined); @@ -75,6 +76,7 @@ export function useReportSubmission({ countryId, currentLawId, reportYear: reportState.year, + metadata, }); journeyProfiler?.markEnd?.('report-submit-simulations', 'api-call'); @@ -138,6 +140,7 @@ export function useReportSubmission({ reportState, countryId, currentLawId, + metadata, createReport, onSuccess, ]); diff --git a/app/src/pages/reportBuilder/modals/HouseholdCreationModal.tsx b/app/src/pages/reportBuilder/modals/HouseholdCreationModal.tsx index 5de15ed5e..a767460e0 100644 --- a/app/src/pages/reportBuilder/modals/HouseholdCreationModal.tsx +++ b/app/src/pages/reportBuilder/modals/HouseholdCreationModal.tsx @@ -18,6 +18,7 @@ import { Household as HouseholdModel } from '@/models/Household'; import { EditableLabel } from '@/pages/reportBuilder/components/EditableLabel'; import { RootState } from '@/store'; import { PopulationStateProps } from '@/types/pathwayState'; +import { withSupportedBuilderClaimantRoles } from '@/utils/builderClaimantRoles'; import { HouseholdValidation } from '@/utils/HouseholdValidation'; import { getModelMetadataError } from '@/utils/spmSelection'; import { BROWSE_MODAL_CONFIG, FONT_SIZES, INGREDIENT_COLORS } from '../constants'; @@ -224,10 +225,15 @@ export function HouseholdCreationModal({ setSaveError(null); try { - const payload = householdToSave.toV1CreationPayload(); + const householdWithRoles = withSupportedBuilderClaimantRoles( + householdToSave, + metadata, + reportYear + ); + const payload = householdWithRoles.toV1CreationPayload(); const result = await createHouseholdWithLabel(payload, householdToSave.label ?? undefined); const householdId = String(result.result.household_id); - const savedHousehold = householdToSave + const savedHousehold = householdWithRoles .withId(householdId) .withLabel(householdToSave.label ?? null); persistCreatedHousehold(savedHousehold); @@ -281,11 +287,12 @@ export function HouseholdCreationModal({ return; } + const householdWithRoles = withSupportedBuilderClaimantRoles(household, metadata, reportYear); let updatedAssociation = await updateHouseholdAssociation.mutateAsync({ userHouseholdId: association.id, updates: {}, association, - nextHousehold: household, + nextHousehold: householdWithRoles, }); const desiredLabel = household.label ?? undefined; @@ -296,7 +303,7 @@ export function HouseholdCreationModal({ }); } - const savedHousehold = household + const savedHousehold = householdWithRoles .withId(updatedAssociation.householdId) .withLabel(updatedAssociation.label ?? desiredLabel ?? null); diff --git a/app/src/pages/reportBuilder/utils/createReportSimulations.ts b/app/src/pages/reportBuilder/utils/createReportSimulations.ts index 22eea69df..16f7b4802 100644 --- a/app/src/pages/reportBuilder/utils/createReportSimulations.ts +++ b/app/src/pages/reportBuilder/utils/createReportSimulations.ts @@ -6,6 +6,10 @@ import { MOCK_USER_ID } from '@/constants'; import type { Household } from '@/models/Household'; import { Simulation } from '@/types/ingredients/Simulation'; import { SimulationStateProps } from '@/types/pathwayState'; +import { + withSupportedBuilderClaimantRoles, + type ClaimantRoleModelMetadata, +} from '@/utils/builderClaimantRoles'; import { getReportPopulationError, hasRequiredSimulationIngredients, @@ -18,6 +22,7 @@ interface CreateReportSimulationsArgs { countryId: 'us' | 'uk'; currentLawId: number; reportYear?: string; + metadata?: ClaimantRoleModelMetadata; } interface CreatedReportSimulations { @@ -31,6 +36,7 @@ export async function createReportSimulations({ countryId, currentLawId, reportYear, + metadata, }: CreateReportSimulationsArgs): Promise { const populationError = getReportPopulationError(simulationStates); if (populationError) { @@ -67,12 +73,17 @@ export async function createReportSimulations({ const identity = JSON.stringify(household.toJSON()); let saved = savedHouseholds.get(identity); if (!saved) { - const created = await createHousehold(household.toV1CreationPayload()); + const householdWithRoles = withSupportedBuilderClaimantRoles( + household, + metadata, + reportYear ?? household.year + ); + const created = await createHousehold(householdWithRoles.toV1CreationPayload()); const id = created.result?.household_id; if (!id) { throw new Error('Household creation returned no ID. Your report has not been replaced.'); } - saved = household.withId(String(id)); + saved = householdWithRoles.withId(String(id)); savedHouseholds.set(identity, saved); } resolvedStates.push({ diff --git a/app/src/pathways/report/views/population/HouseholdBuilderView.tsx b/app/src/pathways/report/views/population/HouseholdBuilderView.tsx index e2a6aa8c5..24ba80883 100644 --- a/app/src/pathways/report/views/population/HouseholdBuilderView.tsx +++ b/app/src/pathways/report/views/population/HouseholdBuilderView.tsx @@ -21,6 +21,7 @@ import { FONT_SIZES, INGREDIENT_COLORS } from '@/pages/reportBuilder/constants'; import { HouseholdCreationContent } from '@/pages/reportBuilder/modals/population'; import { RootState } from '@/store'; import { PopulationStateProps } from '@/types/pathwayState'; +import { withSupportedBuilderClaimantRoles } from '@/utils/builderClaimantRoles'; import { HouseholdValidation } from '@/utils/HouseholdValidation'; import { getModelMetadataError } from '@/utils/spmSelection'; @@ -204,14 +205,15 @@ function HouseholdBuilderForYear({ return; } - const payload = household.toV1CreationPayload(); + const householdToSave = withSupportedBuilderClaimantRoles(household, metadata, reportYear); + const payload = householdToSave.toV1CreationPayload(); setSaveError(null); try { const result = await createHousehold(payload); const householdId = result.result.household_id; - onSubmitSuccess(householdId, household.withId(householdId)); + onSubmitSuccess(householdId, householdToSave.withId(householdId)); } catch (err) { setSaveError(err instanceof Error ? err : new Error('Unable to save household.')); } diff --git a/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts b/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts new file mode 100644 index 000000000..f908d7fbc --- /dev/null +++ b/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts @@ -0,0 +1,99 @@ +import type { CountryId } from '@/libs/countries'; +import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; +import metadataReducer from '@/reducers/metadataReducer'; +import type { MetadataState } from '@/types/metadata'; +import type { ClaimantRoleModelMetadata } from '@/utils/builderClaimantRoles'; + +export const ROLE_TEST_YEAR = '2026'; +export const ROLE_TEST_YEAR_NUMBER = 2026; + +export const BUILDER_PEOPLE = { + YOU: 'you', + PARTNER: 'your partner', + FIRST_DEPENDANT: 'your first dependent', + SECOND_DEPENDANT: 'your second dependent', +} as const; + +export const BUILDER_AGES = { + PARENT: 50, + PARTNER: 48, + ADULT_DEPENDANT: 25, + CHILD: 10, +} as const; + +/** Model information whose UK model defines is_claimant_or_partner. */ +export const UK_METADATA_WITH_CLAIMANT_ROLES: ClaimantRoleModelMetadata = { + currentCountry: 'uk', + variables: { + age: { name: 'age', entity: 'person', valueType: 'float', isInputVariable: true }, + [UK_CLAIMANT_OR_PARTNER_VARIABLE]: { + name: UK_CLAIMANT_OR_PARTNER_VARIABLE, + entity: 'person', + valueType: 'bool', + isInputVariable: false, + definitionPeriod: 'year', + }, + }, +}; + +/** Model information for a UK model that predates is_claimant_or_partner. */ +export const UK_METADATA_WITHOUT_CLAIMANT_ROLES: ClaimantRoleModelMetadata = { + currentCountry: 'uk', + variables: { + age: { name: 'age', entity: 'person', valueType: 'float', isInputVariable: true }, + }, +}; + +/** Loaded UK model information for builder UI tests (synthetic; not a published model). */ +export function ukBuilderMetadata(claimantRoles: ClaimantRoleModelMetadata): MetadataState { + return { + ...metadataReducer(undefined, { type: 'test/init' }), + currentCountry: 'uk', + version: 'test-model', + basicInputs: ['age'], + variables: { + ...claimantRoles.variables, + age: { + name: 'age', + entity: 'person', + label: 'Age', + valueType: 'float', + isInputVariable: true, + }, + }, + entities: { + person: { key: 'person', plural: 'people', label: 'Person', is_person: true }, + benunit: { key: 'benunit', plural: 'benunits', label: 'Benefit unit' }, + household: { key: 'household', plural: 'households', label: 'Household' }, + }, + }; +} + +/** Model information loaded for another country than the household's. */ +export const US_METADATA_WITH_CLAIMANT_ROLES: ClaimantRoleModelMetadata = { + ...UK_METADATA_WITH_CLAIMANT_ROLES, + currentCountry: 'us', +}; + +/** + * A lone parent and their adult child, built the way the builder builds them: one child from the + * child count, whose age is then set to 25. + */ +export function singleParentWithAdultDependant(countryId: CountryId = 'uk'): Household { + return Household.starter(countryId, ROLE_TEST_YEAR) + .withBuilderChildCount(ROLE_TEST_YEAR, 1) + .setPersonVariableAtYear(BUILDER_PEOPLE.YOU, 'age', ROLE_TEST_YEAR, BUILDER_AGES.PARENT) + .setPersonVariableAtYear( + BUILDER_PEOPLE.FIRST_DEPENDANT, + 'age', + ROLE_TEST_YEAR, + BUILDER_AGES.ADULT_DEPENDANT + ); +} + +/** A couple and their adult child, built through the builder's marital status and child count. */ +export function coupleWithAdultDependant(countryId: CountryId = 'uk'): Household { + return singleParentWithAdultDependant(countryId) + .withBuilderMaritalStatus(ROLE_TEST_YEAR, 'married') + .setPersonVariableAtYear(BUILDER_PEOPLE.PARTNER, 'age', ROLE_TEST_YEAR, BUILDER_AGES.PARTNER); +} diff --git a/app/src/tests/integration/household/UKBuilderClaimantRoles.test.tsx b/app/src/tests/integration/household/UKBuilderClaimantRoles.test.tsx new file mode 100644 index 000000000..9b538670a --- /dev/null +++ b/app/src/tests/integration/household/UKBuilderClaimantRoles.test.tsx @@ -0,0 +1,108 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { render, screen, userEvent, waitFor } from '@test-utils'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { LocalStorageHouseholdStore } from '@/api/householdAssociation'; +import { UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; +import HouseholdBuilderView from '@/pathways/report/views/population/HouseholdBuilderView'; +import { + BUILDER_PEOPLE, + ROLE_TEST_YEAR, + singleParentWithAdultDependant, + UK_METADATA_WITH_CLAIMANT_ROLES, + UK_METADATA_WITHOUT_CLAIMANT_ROLES, + ukBuilderMetadata, +} from '@/tests/fixtures/models/builderClaimantRolesMocks'; +import type { MetadataState } from '@/types/metadata'; + +let mockMetadata: MetadataState = ukBuilderMetadata(UK_METADATA_WITH_CLAIMANT_ROLES); + +vi.mock('react-redux', async (importOriginal) => ({ + ...(await importOriginal()), + useSelector: (selector: (state: any) => unknown) => selector({ metadata: mockMetadata }), +})); +vi.mock('@/hooks/useReportYear', () => ({ useReportYear: () => ROLE_TEST_YEAR })); + +const mockFetch = vi.fn(); +const CREATED_HOUSEHOLD_ID = 'created-uk-household'; + +function renderUKBuilder(onSubmitSuccess = vi.fn()) { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const household = singleParentWithAdultDependant().withLabel('Lone parent and adult child'); + render( + + + , + 'uk' + ); + return client; +} + +async function saveAndReadPeople(onSubmitSuccess = vi.fn()) { + const client = renderUKBuilder(onSubmitSuccess); + const save = screen.getByRole('button', { name: 'Save household' }); + await waitFor(() => expect(save).toBeEnabled()); + await userEvent.click(save); + await waitFor(() => expect(onSubmitSuccess).toHaveBeenCalled()); + client.clear(); + const [url, request] = mockFetch.mock.calls[0]; + expect(url).toMatch(/\/uk\/household$/); + return JSON.parse(request.body).data.people; +} + +describe('UK household builder claimant roles through the builder view', () => { + beforeEach(() => { + mockMetadata = ukBuilderMetadata(UK_METADATA_WITH_CLAIMANT_ROLES); + vi.stubGlobal('fetch', mockFetch); + mockFetch.mockReset(); + mockFetch.mockResolvedValue( + new Response(JSON.stringify({ result: { household_id: CREATED_HOUSEHOLD_ID } }), { + status: 200, + }) + ); + vi.spyOn(LocalStorageHouseholdStore.prototype, 'create').mockImplementation( + async (association) => ({ ...association, id: 'uk-association' }) as any + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + test('given a lone parent with a dependant aged 25 then the builder shows them as single', async () => { + renderUKBuilder(); + + expect(await screen.findByText('Single')).toBeInTheDocument(); + expect(screen.queryByText('Married')).not.toBeInTheDocument(); + }); + + test('given the model defines claimant roles then saving sends a single claimant', async () => { + const onSubmitSuccess = vi.fn(); + const people = await saveAndReadPeople(onSubmitSuccess); + + expect(people[BUILDER_PEOPLE.YOU][UK_CLAIMANT_OR_PARTNER_VARIABLE]).toEqual({ + [ROLE_TEST_YEAR]: true, + }); + expect(people[BUILDER_PEOPLE.FIRST_DEPENDANT][UK_CLAIMANT_OR_PARTNER_VARIABLE]).toEqual({ + [ROLE_TEST_YEAR]: false, + }); + const [householdId, savedHousehold] = onSubmitSuccess.mock.calls[0]; + expect(householdId).toBe(CREATED_HOUSEHOLD_ID); + expect(savedHousehold.toV1CreationPayload().data.people).toEqual(people); + }); + + test('given the model predates claimant roles then saving sends the household unchanged', async () => { + mockMetadata = ukBuilderMetadata(UK_METADATA_WITHOUT_CLAIMANT_ROLES); + + const people = await saveAndReadPeople(); + + expect(people).toEqual(singleParentWithAdultDependant().toV1CreationPayload().data.people); + expect(JSON.stringify(people)).not.toContain(UK_CLAIMANT_OR_PARTNER_VARIABLE); + }); +}); diff --git a/app/src/tests/unit/models/Household.test.ts b/app/src/tests/unit/models/Household.test.ts index db4e3f9d7..4ed607934 100644 --- a/app/src/tests/unit/models/Household.test.ts +++ b/app/src/tests/unit/models/Household.test.ts @@ -1,5 +1,12 @@ import { describe, expect, it } from 'vitest'; -import { Household } from '@/models/Household'; +import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; +import { + BUILDER_PEOPLE, + coupleWithAdultDependant, + ROLE_TEST_YEAR, + ROLE_TEST_YEAR_NUMBER, + singleParentWithAdultDependant, +} from '@/tests/fixtures/models/builderClaimantRolesMocks'; import { createMockEmptyHouseholdData, createMockHouseholdData, @@ -755,4 +762,118 @@ describe('Household', () => { expect(householdA.isEqual(householdB)).toBe(false); }); }); + + describe('builder claimant roles', () => { + const roleFor = (household: Household, personName: string) => + household.getPersonVariableAtYear( + personName, + UK_CLAIMANT_OR_PARTNER_VARIABLE, + ROLE_TEST_YEAR + ); + + it('gives a lone parent with a dependant aged 25 a single claimant', () => { + expect(singleParentWithAdultDependant().getBuilderClaimantRoles(ROLE_TEST_YEAR)).toEqual({ + [BUILDER_PEOPLE.YOU]: true, + [BUILDER_PEOPLE.FIRST_DEPENDANT]: false, + }); + }); + + it('gives a couple with a dependant aged 25 a claimant and partner only', () => { + expect(coupleWithAdultDependant().getBuilderClaimantRoles(ROLE_TEST_YEAR)).toEqual({ + [BUILDER_PEOPLE.YOU]: true, + [BUILDER_PEOPLE.PARTNER]: true, + [BUILDER_PEOPLE.FIRST_DEPENDANT]: false, + }); + }); + + it('gives no roles for households outside the UK builder', () => { + const usHousehold = singleParentWithAdultDependant('us'); + const ukHouseholdWithoutYou = Household.empty('uk', ROLE_TEST_YEAR) + .addAdult('claimant', 50) + .addAdult('adult child', 25); + + expect(usHousehold.getBuilderClaimantRoles(ROLE_TEST_YEAR)).toBeNull(); + expect(ukHouseholdWithoutYou.getBuilderClaimantRoles(ROLE_TEST_YEAR)).toBeNull(); + expect(usHousehold.withBuilderClaimantRoles(ROLE_TEST_YEAR)).toBe(usHousehold); + expect(ukHouseholdWithoutYou.withBuilderClaimantRoles(ROLE_TEST_YEAR)).toBe( + ukHouseholdWithoutYou + ); + }); + + it('records the roles for the year without touching other years or inputs', () => { + const household = coupleWithAdultDependant() + .setPersonVariableAtYear(BUILDER_PEOPLE.YOU, UK_CLAIMANT_OR_PARTNER_VARIABLE, '2025', false) + .withBuilderClaimantRoles(ROLE_TEST_YEAR); + + expect(household.householdData.people[BUILDER_PEOPLE.YOU]).toEqual({ + age: { [ROLE_TEST_YEAR]: 50 }, + employment_income: { [ROLE_TEST_YEAR]: 0 }, + [UK_CLAIMANT_OR_PARTNER_VARIABLE]: { 2025: false, [ROLE_TEST_YEAR]: true }, + }); + expect(roleFor(household, BUILDER_PEOPLE.PARTNER)).toBe(true); + expect(roleFor(household, BUILDER_PEOPLE.FIRST_DEPENDANT)).toBe(false); + }); + + it('keeps a role a person already has for the year', () => { + const household = coupleWithAdultDependant() + .setPersonVariableAtYear( + BUILDER_PEOPLE.PARTNER, + UK_CLAIMANT_OR_PARTNER_VARIABLE, + ROLE_TEST_YEAR, + false + ) + .withBuilderClaimantRoles(ROLE_TEST_YEAR); + + expect(roleFor(household, BUILDER_PEOPLE.PARTNER)).toBe(false); + }); + + it('sends the roles in the v1 creation payload', () => { + const payload = singleParentWithAdultDependant() + .withBuilderClaimantRoles(ROLE_TEST_YEAR) + .toV1CreationPayload(); + + expect(payload.data.people[BUILDER_PEOPLE.YOU][UK_CLAIMANT_OR_PARTNER_VARIABLE]).toEqual({ + [ROLE_TEST_YEAR]: true, + }); + expect( + payload.data.people[BUILDER_PEOPLE.FIRST_DEPENDANT][UK_CLAIMANT_OR_PARTNER_VARIABLE] + ).toEqual({ [ROLE_TEST_YEAR]: false }); + expect(payload.data.benunits).toEqual({ + 'your benefit unit': { members: [BUILDER_PEOPLE.YOU, BUILDER_PEOPLE.FIRST_DEPENDANT] }, + }); + }); + + it('sends the same roles in the Python package situation', () => { + const household = coupleWithAdultDependant().withBuilderClaimantRoles(ROLE_TEST_YEAR); + const v1People = household.toV1CreationPayload().data.people; + const pythonPeople = household.toPythonPackage().people; + + for (const personName of household.personNames) { + expect(pythonPeople[personName][UK_CLAIMANT_OR_PARTNER_VARIABLE]).toEqual( + v1People[personName][UK_CLAIMANT_OR_PARTNER_VARIABLE] + ); + } + }); + + it('keeps the roles and the composition through a saved-household round trip', () => { + const household = singleParentWithAdultDependant().withBuilderClaimantRoles(ROLE_TEST_YEAR); + const payload = household.toV1CreationPayload(); + const saved = Household.fromV1Metadata({ + id: 'saved-household', + country_id: payload.country_id, + label: null, + api_version: '0.0.0', + household_json: payload.data, + household_hash: 'hash', + }); + + expect(saved.year).toBe(ROLE_TEST_YEAR_NUMBER); + expect(saved.householdData).toEqual(household.householdData); + expect(saved.toV1CreationPayload()).toEqual(payload); + expect(saved.deriveBuilderComposition(ROLE_TEST_YEAR)).toEqual( + household.deriveBuilderComposition(ROLE_TEST_YEAR) + ); + expect(saved.withBuilderClaimantRoles(ROLE_TEST_YEAR).isEqual(saved)).toBe(true); + }); + }); }); diff --git a/app/src/tests/unit/models/householdBuilderRoles.property.test.ts b/app/src/tests/unit/models/householdBuilderRoles.property.test.ts new file mode 100644 index 000000000..c3180ce1a --- /dev/null +++ b/app/src/tests/unit/models/householdBuilderRoles.property.test.ts @@ -0,0 +1,190 @@ +import fc from 'fast-check'; +import { describe, expect, test } from 'vitest'; +import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; +import { + BUILDER_PEOPLE, + ROLE_TEST_YEAR, + UK_METADATA_WITHOUT_CLAIMANT_ROLES, +} from '@/tests/fixtures/models/builderClaimantRolesMocks'; +import { withSupportedBuilderClaimantRoles } from '@/utils/builderClaimantRoles'; + +type BuilderAction = + | { kind: 'marital'; status: 'single' | 'married' } + | { kind: 'children'; count: number } + | { kind: 'age'; personIndex: number; age: number }; + +const ageArbitrary = fc.integer({ min: 0, max: 100 }); + +const builderActionArbitrary: fc.Arbitrary = fc.oneof( + fc.record({ + kind: fc.constant('marital' as const), + status: fc.constantFrom('single' as const, 'married' as const), + }), + fc.record({ kind: fc.constant('children' as const), count: fc.integer({ min: 0, max: 5 }) }), + fc.record({ + kind: fc.constant('age' as const), + personIndex: fc.nat({ max: 7 }), + age: ageArbitrary, + }) +); + +function personAt(household: Household, personIndex: number): string { + const people = household.getSortedPersonNames(); + return people[personIndex % people.length]; +} + +function applyBuilderAction(household: Household, action: BuilderAction): Household { + switch (action.kind) { + case 'marital': + return household.withBuilderMaritalStatus(ROLE_TEST_YEAR, action.status); + case 'children': + return household.withBuilderChildCount(ROLE_TEST_YEAR, action.count); + case 'age': + return household.setPersonVariableAtYear( + personAt(household, action.personIndex), + 'age', + ROLE_TEST_YEAR, + action.age + ); + } +} + +/** Any household a user can reach through the builder's controls, in either builder country. */ +const builderHouseholdArbitrary = fc + .record({ + countryId: fc.constantFrom('uk' as const, 'us' as const), + actions: fc.array(builderActionArbitrary, { maxLength: 12 }), + }) + .map(({ countryId, actions }) => + actions.reduce(applyBuilderAction, Household.starter(countryId, ROLE_TEST_YEAR)) + ); + +const ukBuilderHouseholdArbitrary = builderHouseholdArbitrary.filter( + (household) => household.countryId === 'uk' +); + +describe('household builder roles properties', () => { + test('the partner is exactly the explicit partner, whatever anyone’s age', () => { + fc.assert( + fc.property(builderHouseholdArbitrary, (household) => { + const composition = household.deriveBuilderComposition(ROLE_TEST_YEAR); + const hasPartner = household.personNames.includes(BUILDER_PEOPLE.PARTNER); + + expect(composition.partnerKey).toBe(hasPartner ? BUILDER_PEOPLE.PARTNER : null); + expect(composition.maritalStatus).toBe(hasPartner ? 'married' : 'single'); + }) + ); + }); + + test('changing a member’s age never changes who the partner is', () => { + fc.assert( + fc.property( + builderHouseholdArbitrary, + fc.nat({ max: 7 }), + ageArbitrary, + (household, personIndex, age) => { + const aged = household.setPersonVariableAtYear( + personAt(household, personIndex), + 'age', + ROLE_TEST_YEAR, + age + ); + + expect(aged.getBuilderPartnerKey(ROLE_TEST_YEAR)).toBe( + household.getBuilderPartnerKey(ROLE_TEST_YEAR) + ); + } + ) + ); + }); + + test('UK roles name "you" and the explicit partner, at most two people, and no one else', () => { + fc.assert( + fc.property(ukBuilderHouseholdArbitrary, (household) => { + const roles = household.getBuilderClaimantRoles(ROLE_TEST_YEAR); + const { partnerKey } = household.deriveBuilderComposition(ROLE_TEST_YEAR); + const expectedClaimants = [BUILDER_PEOPLE.YOU, ...(partnerKey ? [partnerKey] : [])]; + + expect(Object.keys(roles ?? {}).sort()).toEqual([...household.personNames].sort()); + expect( + Object.entries(roles ?? {}) + .filter(([, isClaimantOrPartner]) => isClaimantOrPartner) + .map(([personName]) => personName) + .sort() + ).toEqual(expectedClaimants.sort()); + }) + ); + }); + + test('US builder households get no UK roles', () => { + fc.assert( + fc.property( + builderHouseholdArbitrary.filter((household) => household.countryId === 'us'), + (household) => { + expect(household.getBuilderClaimantRoles(ROLE_TEST_YEAR)).toBeNull(); + expect(household.withBuilderClaimantRoles(ROLE_TEST_YEAR)).toBe(household); + } + ) + ); + }); + + test('recording roles is idempotent and survives the saved-household round trip', () => { + fc.assert( + fc.property(ukBuilderHouseholdArbitrary, (household) => { + const withRoles = household.withBuilderClaimantRoles(ROLE_TEST_YEAR); + const payload = withRoles.toV1CreationPayload(); + const saved = Household.fromV1CreationPayload(payload); + + expect(withRoles.withBuilderClaimantRoles(ROLE_TEST_YEAR).isEqual(withRoles)).toBe(true); + expect(saved.householdData).toEqual(withRoles.householdData); + expect(saved.getBuilderClaimantRoles(ROLE_TEST_YEAR)).toEqual( + withRoles.getBuilderClaimantRoles(ROLE_TEST_YEAR) + ); + }) + ); + }); + + test('the v1 payload and the Python package situation carry the same roles', () => { + fc.assert( + fc.property(ukBuilderHouseholdArbitrary, (household) => { + const withRoles = household.withBuilderClaimantRoles(ROLE_TEST_YEAR); + const v1People = withRoles.toV1CreationPayload().data.people; + const pythonPeople = withRoles.toPythonPackage().people; + + for (const personName of withRoles.personNames) { + expect(pythonPeople[personName][UK_CLAIMANT_OR_PARTNER_VARIABLE]).toEqual( + v1People[personName][UK_CLAIMANT_OR_PARTNER_VARIABLE] + ); + } + }) + ); + }); + + test('without the variable in the model information, every household saves unchanged', () => { + fc.assert( + fc.property(builderHouseholdArbitrary, (household) => { + expect( + withSupportedBuilderClaimantRoles( + household, + UK_METADATA_WITHOUT_CLAIMANT_ROLES, + ROLE_TEST_YEAR + ) + ).toBe(household); + }) + ); + }); + + test('setting the child count gives that many children and keeps every adult and the partner', () => { + fc.assert( + fc.property(builderHouseholdArbitrary, fc.integer({ min: 0, max: 5 }), (household, count) => { + const adults = household.getAdults(ROLE_TEST_YEAR).map(({ name }) => name); + const updated = household.withBuilderChildCount(ROLE_TEST_YEAR, count); + const composition = updated.deriveBuilderComposition(ROLE_TEST_YEAR); + + expect(composition.numChildren).toBe(count); + expect(updated.personNames).toEqual(expect.arrayContaining(adults)); + expect(composition.partnerKey).toBe(household.getBuilderPartnerKey(ROLE_TEST_YEAR)); + }) + ); + }); +}); diff --git a/app/src/tests/unit/pages/reportBuilder/modals/HouseholdCreationModal.test.tsx b/app/src/tests/unit/pages/reportBuilder/modals/HouseholdCreationModal.test.tsx index c0354b254..7ddaa52e8 100644 --- a/app/src/tests/unit/pages/reportBuilder/modals/HouseholdCreationModal.test.tsx +++ b/app/src/tests/unit/pages/reportBuilder/modals/HouseholdCreationModal.test.tsx @@ -1,7 +1,13 @@ import { fireEvent, render, screen, waitFor } from '@test-utils'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; -import { Household } from '@/models/Household'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; import { HouseholdCreationModal } from '@/pages/reportBuilder/modals/HouseholdCreationModal'; +import { + BUILDER_PEOPLE, + ROLE_TEST_YEAR, + singleParentWithAdultDependant, + UK_METADATA_WITH_CLAIMANT_ROLES, +} from '@/tests/fixtures/models/builderClaimantRolesMocks'; import type { PopulationStateProps } from '@/types/pathwayState'; const mockCreateHouseholdWithLabel = vi.hoisted(() => vi.fn()); @@ -9,16 +15,18 @@ const mockUpdateHouseholdAssociation = vi.hoisted(() => vi.fn()); const mockHouseholdAssociationRefetch = vi.hoisted(() => vi.fn()); const MODAL_TEST_TIMEOUT_MS = 20_000; +const US_MODAL_METADATA = { + currentCountry: 'us', + loading: false, + error: null, + version: 'test-model', + basicInputs: [], + variables: {} as Record, + entities: {}, +}; + const mockReduxState = { - metadata: { - currentCountry: 'us', - loading: false, - error: null, - version: 'test-model', - basicInputs: [], - variables: {}, - entities: {}, - }, + metadata: US_MODAL_METADATA, }; vi.mock('react-redux', async () => { @@ -30,7 +38,7 @@ vi.mock('react-redux', async () => { }); vi.mock('@/hooks/useCurrentCountry', () => ({ - useCurrentCountry: () => 'us', + useCurrentCountry: () => mockReduxState.metadata.currentCountry, })); vi.mock('@/hooks/useCreateHousehold', () => ({ @@ -220,4 +228,106 @@ describe('HouseholdCreationModal', () => { }, MODAL_TEST_TIMEOUT_MS ); + + describe('given a UK lone parent with a dependant aged 25', () => { + const ukPopulation: PopulationStateProps = { + type: 'household', + household: singleParentWithAdultDependant().withId('hh-uk').withLabel('Lone parent'), + geography: null, + label: 'Lone parent', + }; + const roleOf = (household: Household, personName: string) => + household.getPersonVariableAtYear( + personName, + UK_CLAIMANT_OR_PARTNER_VARIABLE, + ROLE_TEST_YEAR + ); + + beforeEach(() => { + mockReduxState.metadata = { + ...US_MODAL_METADATA, + currentCountry: 'uk', + variables: UK_METADATA_WITH_CLAIMANT_ROLES.variables, + }; + }); + + afterEach(() => { + mockReduxState.metadata = US_MODAL_METADATA; + }); + + const renderUKModal = (onHouseholdSaved = vi.fn()) => + render( + + ); + + test( + 'given save as new household then saves one claimant and a dependant who is not a partner', + async () => { + const onHouseholdSaved = vi.fn(); + renderUKModal(onHouseholdSaved); + + fireEvent.click(screen.getByRole('button', { name: /make household change/i })); + const saveAsNewButton = screen.getByRole('button', { name: /save as new household/i }); + await waitFor(() => expect(saveAsNewButton).not.toBeDisabled()); + fireEvent.click(saveAsNewButton); + fireEvent.click(screen.getByRole('button', { name: /keep same name/i })); + + await waitFor(() => expect(mockCreateHouseholdWithLabel).toHaveBeenCalled()); + const [payload] = mockCreateHouseholdWithLabel.mock.calls[0]; + expect(payload.data.people[BUILDER_PEOPLE.YOU][UK_CLAIMANT_OR_PARTNER_VARIABLE]).toEqual({ + [ROLE_TEST_YEAR]: true, + }); + expect( + payload.data.people[BUILDER_PEOPLE.FIRST_DEPENDANT][UK_CLAIMANT_OR_PARTNER_VARIABLE] + ).toEqual({ [ROLE_TEST_YEAR]: false }); + const savedHousehold: Household = onHouseholdSaved.mock.calls[0][0].household; + expect(roleOf(savedHousehold, BUILDER_PEOPLE.YOU)).toBe(true); + expect(roleOf(savedHousehold, BUILDER_PEOPLE.FIRST_DEPENDANT)).toBe(false); + }, + MODAL_TEST_TIMEOUT_MS + ); + + test( + 'given update existing household then replaces it with the roles recorded', + async () => { + mockUpdateHouseholdAssociation.mockResolvedValue({ + id: 'uhh-uk', + type: 'household', + userId: 'user-1', + householdId: 'hh-uk-replacement', + countryId: 'uk', + label: 'Changed household', + }); + mockHouseholdAssociationRefetch.mockResolvedValue({ + data: { + id: 'uhh-uk', + type: 'household', + userId: 'user-1', + householdId: 'hh-uk', + countryId: 'uk', + label: 'Lone parent', + }, + }); + renderUKModal(); + + fireEvent.click(screen.getByRole('button', { name: /make household change/i })); + const updateButton = screen.getByRole('button', { name: /update existing household/i }); + await waitFor(() => expect(updateButton).not.toBeDisabled()); + fireEvent.click(updateButton); + + await waitFor(() => expect(mockUpdateHouseholdAssociation).toHaveBeenCalled()); + const { nextHousehold } = mockUpdateHouseholdAssociation.mock.calls[0][0]; + expect(roleOf(nextHousehold, BUILDER_PEOPLE.YOU)).toBe(true); + expect(roleOf(nextHousehold, BUILDER_PEOPLE.FIRST_DEPENDANT)).toBe(false); + }, + MODAL_TEST_TIMEOUT_MS + ); + }); }); diff --git a/app/src/tests/unit/pages/reportBuilder/utils/createReportSimulations.test.ts b/app/src/tests/unit/pages/reportBuilder/utils/createReportSimulations.test.ts index 3b6afa4c6..e9efab21c 100644 --- a/app/src/tests/unit/pages/reportBuilder/utils/createReportSimulations.test.ts +++ b/app/src/tests/unit/pages/reportBuilder/utils/createReportSimulations.test.ts @@ -1,7 +1,14 @@ import { beforeEach, describe, expect, test, vi } from 'vitest'; import { createHousehold } from '@/api/household'; -import { Household } from '@/models/Household'; +import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; import { createReportSimulations } from '@/pages/reportBuilder/utils/createReportSimulations'; +import { + BUILDER_PEOPLE, + ROLE_TEST_YEAR, + singleParentWithAdultDependant, + UK_METADATA_WITH_CLAIMANT_ROLES, + UK_METADATA_WITHOUT_CLAIMANT_ROLES, +} from '@/tests/fixtures/models/builderClaimantRolesMocks'; import { CORRECTED_REPORT_YEAR, CURRENT_LAW_ID, @@ -225,4 +232,58 @@ describe('createReportSimulations', () => { ).rejects.toThrow('Simulation creation returned no ID'); expect(mockLocalStorageCreateFn).not.toHaveBeenCalled(); }); + + describe('given a UK draft household of a lone parent and their adult child', () => { + const CREATED_UK_HOUSEHOLD_ID = 'created-uk-household'; + const ukDraftSimulation = () => ({ + ...mockDraftHouseholdSimulation('uk'), + population: { + ...mockDraftHouseholdSimulation('uk').population, + household: singleParentWithAdultDependant(), + }, + }); + const createdPeople = () => vi.mocked(createHousehold).mock.calls[0][0].data.people; + + beforeEach(() => { + vi.mocked(createHousehold).mockResolvedValue({ + result: { household_id: CREATED_UK_HOUSEHOLD_ID }, + }); + }); + + test('given the model defines claimant roles then the household is saved with a single claimant', async () => { + const result = await createReportSimulations({ + simulationStates: [ukDraftSimulation()] as any, + countryId: 'uk', + currentLawId: CURRENT_LAW_ID, + reportYear: ROLE_TEST_YEAR, + metadata: UK_METADATA_WITH_CLAIMANT_ROLES, + }); + + expect(createdPeople()[BUILDER_PEOPLE.YOU][UK_CLAIMANT_OR_PARTNER_VARIABLE]).toEqual({ + [ROLE_TEST_YEAR]: true, + }); + expect( + createdPeople()[BUILDER_PEOPLE.FIRST_DEPENDANT][UK_CLAIMANT_OR_PARTNER_VARIABLE] + ).toEqual({ [ROLE_TEST_YEAR]: false }); + const savedHousehold = result.simulationStates[0].population.household!; + expect(savedHousehold.id).toBe(CREATED_UK_HOUSEHOLD_ID); + expect(savedHousehold.toV1CreationPayload().data.people).toEqual(createdPeople()); + }); + + test('given the model predates claimant roles then the household is saved unchanged', async () => { + const draft = ukDraftSimulation(); + + await createReportSimulations({ + simulationStates: [draft] as any, + countryId: 'uk', + currentLawId: CURRENT_LAW_ID, + reportYear: ROLE_TEST_YEAR, + metadata: UK_METADATA_WITHOUT_CLAIMANT_ROLES, + }); + + expect(vi.mocked(createHousehold)).toHaveBeenCalledWith( + draft.population.household.toV1CreationPayload() + ); + }); + }); }); diff --git a/app/src/tests/unit/utils/builderClaimantRoles.test.ts b/app/src/tests/unit/utils/builderClaimantRoles.test.ts new file mode 100644 index 000000000..99459988a --- /dev/null +++ b/app/src/tests/unit/utils/builderClaimantRoles.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, test } from 'vitest'; +import { UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; +import { + BUILDER_PEOPLE, + ROLE_TEST_YEAR, + singleParentWithAdultDependant, + UK_METADATA_WITH_CLAIMANT_ROLES, + UK_METADATA_WITHOUT_CLAIMANT_ROLES, + US_METADATA_WITH_CLAIMANT_ROLES, +} from '@/tests/fixtures/models/builderClaimantRolesMocks'; +import { withSupportedBuilderClaimantRoles } from '@/utils/builderClaimantRoles'; + +describe('withSupportedBuilderClaimantRoles', () => { + test('given the UK model defines the variable then a lone parent with an adult dependant is sent as a single claimant', () => { + const household = withSupportedBuilderClaimantRoles( + singleParentWithAdultDependant(), + UK_METADATA_WITH_CLAIMANT_ROLES, + ROLE_TEST_YEAR + ); + + expect(household.toV1CreationPayload().data.people).toEqual( + expect.objectContaining({ + [BUILDER_PEOPLE.YOU]: expect.objectContaining({ + [UK_CLAIMANT_OR_PARTNER_VARIABLE]: { [ROLE_TEST_YEAR]: true }, + }), + [BUILDER_PEOPLE.FIRST_DEPENDANT]: expect.objectContaining({ + [UK_CLAIMANT_OR_PARTNER_VARIABLE]: { [ROLE_TEST_YEAR]: false }, + }), + }) + ); + }); + + test('given no year is passed then the household year is used', () => { + const household = withSupportedBuilderClaimantRoles( + singleParentWithAdultDependant(), + UK_METADATA_WITH_CLAIMANT_ROLES + ); + + expect( + household.getPersonVariableAtYear( + BUILDER_PEOPLE.YOU, + UK_CLAIMANT_OR_PARTNER_VARIABLE, + ROLE_TEST_YEAR + ) + ).toBe(true); + }); + + test.each([ + ['the UK model predates the variable', UK_METADATA_WITHOUT_CLAIMANT_ROLES], + ['model information is for another country', US_METADATA_WITH_CLAIMANT_ROLES], + ['model information has not loaded', undefined], + ])('given %s then the household is saved unchanged', (_case, metadata) => { + const household = singleParentWithAdultDependant(); + + const result = withSupportedBuilderClaimantRoles(household, metadata, ROLE_TEST_YEAR); + + expect(result).toBe(household); + expect(JSON.stringify(result.toV1CreationPayload())).not.toContain( + UK_CLAIMANT_OR_PARTNER_VARIABLE + ); + }); + + test('given a US household then no UK roles are added', () => { + const household = singleParentWithAdultDependant('us'); + + expect( + withSupportedBuilderClaimantRoles(household, US_METADATA_WITH_CLAIMANT_ROLES, ROLE_TEST_YEAR) + ).toBe(household); + }); +}); diff --git a/app/src/tests/unit/utils/householdBuilderComposition.test.ts b/app/src/tests/unit/utils/householdBuilderComposition.test.ts index d7b667e9d..f5803bd51 100644 --- a/app/src/tests/unit/utils/householdBuilderComposition.test.ts +++ b/app/src/tests/unit/utils/householdBuilderComposition.test.ts @@ -1,5 +1,12 @@ import { describe, expect, test } from 'vitest'; -import { Household } from '@/models/Household'; +import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; +import { + BUILDER_AGES, + BUILDER_PEOPLE, + coupleWithAdultDependant, + ROLE_TEST_YEAR, + singleParentWithAdultDependant, +} from '@/tests/fixtures/models/builderClaimantRolesMocks'; const YEAR = '2026'; @@ -48,4 +55,98 @@ describe('Household builder composition methods', () => { expect(people).toContain('your second dependent'); expect(updatedHousehold.deriveBuilderComposition(YEAR).numChildren).toBe(2); }); + + describe.each(['uk', 'us'] as const)('given a %s dependant aged 25', (countryId) => { + test('given a single parent then the adult dependant is not read as a partner', () => { + const composition = + singleParentWithAdultDependant(countryId).deriveBuilderComposition(ROLE_TEST_YEAR); + + expect(composition.primaryPersonKey).toBe(BUILDER_PEOPLE.YOU); + expect(composition.partnerKey).toBeNull(); + expect(composition.maritalStatus).toBe('single'); + }); + + test('given a single parent then the adult dependant stays out of the child count', () => { + const composition = + singleParentWithAdultDependant(countryId).deriveBuilderComposition(ROLE_TEST_YEAR); + + expect(composition.childKeys).toEqual([]); + expect(composition.numChildren).toBe(0); + }); + + test('given a couple then the explicit partner stays the partner', () => { + const household = coupleWithAdultDependant(countryId); + const composition = household.deriveBuilderComposition(ROLE_TEST_YEAR); + + expect(composition.partnerKey).toBe(BUILDER_PEOPLE.PARTNER); + expect(composition.maritalStatus).toBe('married'); + expect(household.personNames).toContain(BUILDER_PEOPLE.FIRST_DEPENDANT); + }); + + test('given a single parent chooses single then the adult dependant is kept', () => { + const household = singleParentWithAdultDependant(countryId).withBuilderMaritalStatus( + ROLE_TEST_YEAR, + 'single' + ); + + expect(household.personNames).toContain(BUILDER_PEOPLE.FIRST_DEPENDANT); + }); + + test('given a couple chooses single then only the partner is removed', () => { + const household = coupleWithAdultDependant(countryId).withBuilderMaritalStatus( + ROLE_TEST_YEAR, + 'single' + ); + + expect(household.getSortedPersonNames()).toEqual([ + BUILDER_PEOPLE.YOU, + BUILDER_PEOPLE.FIRST_DEPENDANT, + ]); + }); + + test('given a child is added then it is numbered after the adult dependant', () => { + const household = singleParentWithAdultDependant(countryId).withBuilderChildCount( + ROLE_TEST_YEAR, + 1 + ); + + expect(household.getSortedPersonNames()).toEqual([ + BUILDER_PEOPLE.YOU, + BUILDER_PEOPLE.FIRST_DEPENDANT, + BUILDER_PEOPLE.SECOND_DEPENDANT, + ]); + expect( + household.getPersonVariableAtYear(BUILDER_PEOPLE.FIRST_DEPENDANT, 'age', ROLE_TEST_YEAR) + ).toBe(BUILDER_AGES.ADULT_DEPENDANT); + expect(household.deriveBuilderComposition(ROLE_TEST_YEAR).numChildren).toBe(1); + }); + }); + + test('given another adult without an explicit partner role then the household is single', () => { + const household = Household.empty('uk', ROLE_TEST_YEAR) + .addAdult(BUILDER_PEOPLE.YOU, BUILDER_AGES.PARENT) + .addAdult('lodger', BUILDER_AGES.PARTNER); + + expect(household.getBuilderPartnerKey(ROLE_TEST_YEAR)).toBeNull(); + }); + + test('given a member flagged as claimant or partner then that member is the partner', () => { + const household = Household.empty('uk', ROLE_TEST_YEAR) + .addAdult(BUILDER_PEOPLE.YOU, BUILDER_AGES.PARENT) + .addAdult('alex', BUILDER_AGES.PARTNER) + .setPersonVariableAtYear('alex', UK_CLAIMANT_OR_PARTNER_VARIABLE, ROLE_TEST_YEAR, true); + + expect(household.deriveBuilderComposition(ROLE_TEST_YEAR)).toEqual( + expect.objectContaining({ partnerKey: 'alex', maritalStatus: 'married' }) + ); + }); + + test('given a member flagged for another year only then that member is not the partner', () => { + const household = Household.empty('uk', ROLE_TEST_YEAR) + .addAdult(BUILDER_PEOPLE.YOU, BUILDER_AGES.PARENT) + .addAdult('alex', BUILDER_AGES.PARTNER) + .setPersonVariableAtYear('alex', UK_CLAIMANT_OR_PARTNER_VARIABLE, '2025', true); + + expect(household.getBuilderPartnerKey(ROLE_TEST_YEAR)).toBeNull(); + }); }); diff --git a/app/src/utils/builderClaimantRoles.ts b/app/src/utils/builderClaimantRoles.ts new file mode 100644 index 000000000..082558274 --- /dev/null +++ b/app/src/utils/builderClaimantRoles.ts @@ -0,0 +1,28 @@ +import { UK_CLAIMANT_OR_PARTNER_VARIABLE, type Household } from '@/models/Household'; +import type { MetadataState } from '@/types/metadata'; + +export type ClaimantRoleModelMetadata = Pick; + +/** + * Adds explicit claimant and partner roles to a UK builder household before it is saved, so + * policyengine-uk does not infer a couple from ages alone. + * + * policyengine-core rejects a situation that names a variable its model lacks, so the roles are + * added only when the loaded model information for the household's country defines the + * variable. Until the API's model does, households save exactly as before. + */ +export function withSupportedBuilderClaimantRoles( + household: Household, + metadata: ClaimantRoleModelMetadata | undefined, + year: string | number | null = household.year +): Household { + if ( + year === null || + metadata?.currentCountry !== household.countryId || + !metadata.variables?.[UK_CLAIMANT_OR_PARTNER_VARIABLE] + ) { + return household; + } + + return household.withBuilderClaimantRoles(String(year)); +} diff --git a/bun.lock b/bun.lock index c61dfc7c8..c1964d756 100644 --- a/bun.lock +++ b/bun.lock @@ -81,6 +81,7 @@ "eslint": "^9.29.0", "eslint-plugin-jsx-a11y": "^6.10.2", "eslint-plugin-react": "^7.37.5", + "fast-check": "^4.10.2", "identity-obj-proxy": "^3.0.0", "jsdom": "^26.1.0", "postcss": "^8.5.5", From 0f4234baa41f280dc380eb9652704ae8927b7540 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Fri, 2 Oct 2026 11:40:08 -0400 Subject: [PATCH 2/3] Scope claimant roles to the builder's benefit unit (review fixes) Independent review of 8c575180 found two reproduced bugs and three smaller issues; all are fixed here. - A claimant-or-partner flag names the partner only inside the primary person's benefit unit. Before, a flagged claimant in another unit could become "the partner", and choosing "single" deleted them. - Roles are generated only for the builder's structure: "you" and one benefit unit holding everyone. policyengine-core gives anyone without an input the variable's default (false), not its formula, so roles for one unit would make another unit's own claimant a non-claimant. Multi-unit and partly-outside households now get no roles, and the model infers them as before. - When the loaded UK model information lacks the variable, roles a household already carries are removed before saving, so a stored flag can never reach a model that rejects it. - New children are numbered after the highest dependant ordinal in use, so a gap no longer gives "your second dependent 2". - The integration test's API response moved to fixtures. Tests: regression cases for each, and fast-check properties over saved multi-unit households, households already carrying roles, and roles recorded before and after further builder edits. Undoing each fix fails 4, 5, 2 and 1 tests respectively. Co-Authored-By: Claude Opus 5.5 --- app/src/models/Household.ts | 61 +++++++++-- .../models/builderClaimantRolesMocks.ts | 50 +++++++++ .../household/UKBuilderClaimantRoles.test.tsx | 9 +- app/src/tests/unit/models/Household.test.ts | 24 +++++ .../householdBuilderRoles.property.test.ts | 102 ++++++++++++++++-- .../utils/createReportSimulations.test.ts | 6 +- .../unit/utils/builderClaimantRoles.test.ts | 32 ++++++ .../utils/householdBuilderComposition.test.ts | 48 +++++++++ app/src/utils/builderClaimantRoles.ts | 34 ++++-- 9 files changed, 331 insertions(+), 35 deletions(-) diff --git a/app/src/models/Household.ts b/app/src/models/Household.ts index 80f1d9c43..e83e9590f 100644 --- a/app/src/models/Household.ts +++ b/app/src/models/Household.ts @@ -357,6 +357,34 @@ function getBuilderDependentName(index: number): string { return `your ${BUILDER_DEPENDENT_ORDINALS[index] || `${index + 1}th`} dependent`; } +/** The index getBuilderDependentName gave this name, or -1 for any other name. */ +function getBuilderDependentIndex(personKey: string): number { + const ordinal = /^your (\S+) dependent$/.exec(personKey)?.[1]; + if (!ordinal) { + return -1; + } + + const namedIndex = BUILDER_DEPENDENT_ORDINALS.indexOf(ordinal); + if (namedIndex !== -1) { + return namedIndex; + } + + const numbered = /^(\d+)th$/.exec(ordinal)?.[1]; + return numbered ? Number(numbered) - 1 : -1; +} + +/** Everyone who shares a group with the person, the person included. */ +function getGroupMatesOf( + groups: AppHouseholdInputGroupMap | undefined, + personKey: string +): Set { + return new Set( + Object.values(groups ?? {}) + .filter((group) => group.members.includes(personKey)) + .flatMap((group) => group.members) + ); +} + function getUniqueBuilderPersonName(existingKeys: Set, preferredName: string): string { if (!existingKeys.has(preferredName)) { return preferredName; @@ -891,10 +919,14 @@ export class Household extends BaseModel { } // Only an explicit partner counts. Age never makes one: a dependant aged 25 stays a dependant. + // A claimant-or-partner flag names the partner only inside the primary person's benefit unit; + // a flagged member of another unit is that unit's own claimant. + const benefitUnitMates = getGroupMatesOf(this.appInputData.benunits, resolvedPrimaryPersonKey); return ( this.getSortedPersonNames().find( (personKey) => personKey !== resolvedPrimaryPersonKey && + benefitUnitMates.has(personKey) && this.getPersonVariableAtYear(personKey, UK_CLAIMANT_OR_PARTNER_VARIABLE, year) === true ) ?? null ); @@ -941,11 +973,21 @@ export class Household extends BaseModel { /** * Benefit-unit roles of a UK builder household: "you" and the explicit partner are the * claimant and partner, and every other member (a dependant of any age) is neither. - * Returns null for a household the builder did not make (no "you"), whose relationships the - * app does not know; policyengine-uk then infers them as before. + * + * Returns null unless the household has the builder's structure: a person named "you" and one + * benefit unit holding everyone. Roles are all or nothing: policyengine-core gives anyone + * without an input the variable's default (false), not its formula, so roles for one unit + * would make another unit's claimant a non-claimant. Without roles, policyengine-uk infers + * them as before. */ getBuilderClaimantRoles(year: string): Record | null { - if (this.countryId !== 'uk' || !(BUILDER_PRIMARY_PERSON_NAME in this.appInputData.people)) { + const benefitUnits = Object.values(this.appInputData.benunits ?? {}); + const hasBuilderStructure = + this.countryId === 'uk' && + BUILDER_PRIMARY_PERSON_NAME in this.appInputData.people && + benefitUnits.length === 1 && + this.personNames.every((personKey) => benefitUnits[0].members.includes(personKey)); + if (!hasBuilderStructure) { return null; } @@ -1226,15 +1268,18 @@ export class Household extends BaseModel { const parentIds = [composition.primaryPersonKey, composition.partnerKey].filter( Boolean ) as string[]; - // Number new children after every dependant, including adult dependants the count excludes. - const dependantCount = composition.people.filter( - (personKey) => !parentIds.includes(personKey) - ).length; + // Number new children after every dependant, including adult dependants the count excludes, + // and after the highest dependant ordinal in use, so a gap never produces "... dependent 2". + const dependants = composition.people.filter((personKey) => !parentIds.includes(personKey)); + const firstNewIndex = Math.max( + dependants.length, + ...dependants.map((personKey) => getBuilderDependentIndex(personKey) + 1) + ); for (let offset = 0; offset < newCount - composition.childKeys.length; offset += 1) { const childKey = getUniqueBuilderPersonName( existingKeys, - getBuilderDependentName(dependantCount + offset) + getBuilderDependentName(firstNewIndex + offset) ); existingKeys.add(childKey); nextHousehold = nextHousehold.addChild(childKey, BUILDER_DEFAULT_CHILD_AGE, parentIds, { diff --git a/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts b/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts index f908d7fbc..59329b74e 100644 --- a/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts +++ b/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts @@ -1,5 +1,6 @@ import type { CountryId } from '@/libs/countries'; import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; +import type { AppHouseholdInputPerson } from '@/models/household/appTypes'; import metadataReducer from '@/reducers/metadataReducer'; import type { MetadataState } from '@/types/metadata'; import type { ClaimantRoleModelMetadata } from '@/utils/builderClaimantRoles'; @@ -97,3 +98,52 @@ export function coupleWithAdultDependant(countryId: CountryId = 'uk'): Household .withBuilderMaritalStatus(ROLE_TEST_YEAR, 'married') .setPersonVariableAtYear(BUILDER_PEOPLE.PARTNER, 'age', ROLE_TEST_YEAR, BUILDER_AGES.PARTNER); } + +export const SEPARATE_CLAIMANT = 'other claimant'; + +/** + * A saved UK household with two benefit units: "you" (50) alone in one, and a separate claimant + * alone in another, as a household made outside the builder can be. + */ +export function householdWithSeparateClaimant( + separateClaimant: { age: number; isClaimantOrPartner?: boolean }, + youIsClaimantOrPartner?: boolean +): Household { + const person = ( + age: number, + isClaimantOrPartner: boolean | undefined + ): AppHouseholdInputPerson => + isClaimantOrPartner === undefined + ? { age: { [ROLE_TEST_YEAR]: age } } + : { + age: { [ROLE_TEST_YEAR]: age }, + [UK_CLAIMANT_OR_PARTNER_VARIABLE]: { [ROLE_TEST_YEAR]: isClaimantOrPartner }, + }; + + return Household.fromAppInput({ + countryId: 'uk', + year: ROLE_TEST_YEAR_NUMBER, + householdData: { + people: { + [BUILDER_PEOPLE.YOU]: person(BUILDER_AGES.PARENT, youIsClaimantOrPartner), + [SEPARATE_CLAIMANT]: person(separateClaimant.age, separateClaimant.isClaimantOrPartner), + }, + households: { + 'your household': { members: [BUILDER_PEOPLE.YOU, SEPARATE_CLAIMANT] }, + }, + benunits: { + 'your benefit unit': { members: [BUILDER_PEOPLE.YOU] }, + 'their benefit unit': { members: [SEPARATE_CLAIMANT] }, + }, + }, + }); +} + +export const CREATED_HOUSEHOLD_ID = 'created-uk-household'; + +/** The v1 API's response to a household creation request. */ +export function mockCreatedHouseholdResponse(): Response { + return new Response(JSON.stringify({ result: { household_id: CREATED_HOUSEHOLD_ID } }), { + status: 200, + }); +} diff --git a/app/src/tests/integration/household/UKBuilderClaimantRoles.test.tsx b/app/src/tests/integration/household/UKBuilderClaimantRoles.test.tsx index 9b538670a..13863c128 100644 --- a/app/src/tests/integration/household/UKBuilderClaimantRoles.test.tsx +++ b/app/src/tests/integration/household/UKBuilderClaimantRoles.test.tsx @@ -6,6 +6,8 @@ import { UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; import HouseholdBuilderView from '@/pathways/report/views/population/HouseholdBuilderView'; import { BUILDER_PEOPLE, + CREATED_HOUSEHOLD_ID, + mockCreatedHouseholdResponse, ROLE_TEST_YEAR, singleParentWithAdultDependant, UK_METADATA_WITH_CLAIMANT_ROLES, @@ -23,7 +25,6 @@ vi.mock('react-redux', async (importOriginal) => ({ vi.mock('@/hooks/useReportYear', () => ({ useReportYear: () => ROLE_TEST_YEAR })); const mockFetch = vi.fn(); -const CREATED_HOUSEHOLD_ID = 'created-uk-household'; function renderUKBuilder(onSubmitSuccess = vi.fn()) { const client = new QueryClient({ @@ -60,11 +61,7 @@ describe('UK household builder claimant roles through the builder view', () => { mockMetadata = ukBuilderMetadata(UK_METADATA_WITH_CLAIMANT_ROLES); vi.stubGlobal('fetch', mockFetch); mockFetch.mockReset(); - mockFetch.mockResolvedValue( - new Response(JSON.stringify({ result: { household_id: CREATED_HOUSEHOLD_ID } }), { - status: 200, - }) - ); + mockFetch.mockImplementation(async () => mockCreatedHouseholdResponse()); vi.spyOn(LocalStorageHouseholdStore.prototype, 'create').mockImplementation( async (association) => ({ ...association, id: 'uk-association' }) as any ); diff --git a/app/src/tests/unit/models/Household.test.ts b/app/src/tests/unit/models/Household.test.ts index 4ed607934..5fc4fd15c 100644 --- a/app/src/tests/unit/models/Household.test.ts +++ b/app/src/tests/unit/models/Household.test.ts @@ -3,6 +3,7 @@ import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; import { BUILDER_PEOPLE, coupleWithAdultDependant, + householdWithSeparateClaimant, ROLE_TEST_YEAR, ROLE_TEST_YEAR_NUMBER, singleParentWithAdultDependant, @@ -800,6 +801,29 @@ describe('Household', () => { ); }); + it.each([ + ['an unflagged adult', { age: 40 }], + ['a 17-year-old claiming separately', { age: 17 }], + ])('gives no roles when %s has their own benefit unit', (_case, separateClaimant) => { + const household = householdWithSeparateClaimant(separateClaimant); + + expect(household.getBuilderClaimantRoles(ROLE_TEST_YEAR)).toBeNull(); + expect(household.withBuilderClaimantRoles(ROLE_TEST_YEAR)).toBe(household); + }); + + it('gives no roles when someone is outside the benefit unit', () => { + const household = singleParentWithAdultDependant(); + const outsideUnit = Household.fromAppInput({ + ...household.toAppInput(), + householdData: { + ...household.householdData, + benunits: { 'your benefit unit': { members: [BUILDER_PEOPLE.YOU] } }, + }, + }); + + expect(outsideUnit.getBuilderClaimantRoles(ROLE_TEST_YEAR)).toBeNull(); + }); + it('records the roles for the year without touching other years or inputs', () => { const household = coupleWithAdultDependant() .setPersonVariableAtYear(BUILDER_PEOPLE.YOU, UK_CLAIMANT_OR_PARTNER_VARIABLE, '2025', false) diff --git a/app/src/tests/unit/models/householdBuilderRoles.property.test.ts b/app/src/tests/unit/models/householdBuilderRoles.property.test.ts index c3180ce1a..9120ffca1 100644 --- a/app/src/tests/unit/models/householdBuilderRoles.property.test.ts +++ b/app/src/tests/unit/models/householdBuilderRoles.property.test.ts @@ -4,6 +4,7 @@ import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; import { BUILDER_PEOPLE, ROLE_TEST_YEAR, + SEPARATE_CLAIMANT, UK_METADATA_WITHOUT_CLAIMANT_ROLES, } from '@/tests/fixtures/models/builderClaimantRolesMocks'; import { withSupportedBuilderClaimantRoles } from '@/utils/builderClaimantRoles'; @@ -63,6 +64,41 @@ const ukBuilderHouseholdArbitrary = builderHouseholdArbitrary.filter( (household) => household.countryId === 'uk' ); +const optionalRoleArbitrary = fc.option(fc.boolean(), { nil: undefined }); + +/** Adds a claimant in a benefit unit of their own, as a household saved outside the builder can have. */ +function withSeparateClaimant( + household: Household, + age: number, + isClaimantOrPartner: boolean | undefined +): Household { + const data = household.householdData; + data.people[SEPARATE_CLAIMANT] = { + age: { [ROLE_TEST_YEAR]: age }, + ...(isClaimantOrPartner === undefined + ? {} + : { [UK_CLAIMANT_OR_PARTNER_VARIABLE]: { [ROLE_TEST_YEAR]: isClaimantOrPartner } }), + }; + data.benunits = { ...data.benunits, 'their benefit unit': { members: [SEPARATE_CLAIMANT] } }; + return Household.fromAppInput({ ...household.toAppInput(), householdData: data }); +} + +/** UK builder households beside a separate claimant, any of whom may already carry a role. */ +const multiUnitHouseholdArbitrary = fc + .record({ + household: ukBuilderHouseholdArbitrary, + recordRoles: fc.boolean(), + age: ageArbitrary, + isClaimantOrPartner: optionalRoleArbitrary, + }) + .map(({ household, recordRoles, age, isClaimantOrPartner }) => + withSeparateClaimant( + recordRoles ? household.withBuilderClaimantRoles(ROLE_TEST_YEAR) : household, + age, + isClaimantOrPartner + ) + ); + describe('household builder roles properties', () => { test('the partner is exactly the explicit partner, whatever anyone’s age', () => { fc.assert( @@ -160,20 +196,72 @@ describe('household builder roles properties', () => { ); }); - test('without the variable in the model information, every household saves unchanged', () => { + test('without the variable in the model information, no household sends a role', () => { fc.assert( - fc.property(builderHouseholdArbitrary, (household) => { - expect( - withSupportedBuilderClaimantRoles( - household, + fc.property( + fc.oneof(ukBuilderHouseholdArbitrary, multiUnitHouseholdArbitrary), + fc.boolean(), + (household, recordRoles) => { + const saved = withSupportedBuilderClaimantRoles( + recordRoles ? household.withBuilderClaimantRoles(ROLE_TEST_YEAR) : household, UK_METADATA_WITHOUT_CLAIMANT_ROLES, ROLE_TEST_YEAR - ) - ).toBe(household); + ); + + expect(JSON.stringify(saved.toV1CreationPayload())).not.toContain( + UK_CLAIMANT_OR_PARTNER_VARIABLE + ); + } + ) + ); + }); + + test('a claimant in another benefit unit is never the partner, and choosing single keeps them', () => { + fc.assert( + fc.property(multiUnitHouseholdArbitrary, (household) => { + const single = household.withBuilderMaritalStatus(ROLE_TEST_YEAR, 'single'); + + expect(household.getBuilderPartnerKey(ROLE_TEST_YEAR)).not.toBe(SEPARATE_CLAIMANT); + expect(single.personNames).toContain(SEPARATE_CLAIMANT); }) ); }); + test('a household with more than one benefit unit gets no generated roles', () => { + fc.assert( + fc.property(multiUnitHouseholdArbitrary, (household) => { + expect(household.getBuilderClaimantRoles(ROLE_TEST_YEAR)).toBeNull(); + expect(household.withBuilderClaimantRoles(ROLE_TEST_YEAR)).toBe(household); + }) + ); + }); + + test('roles recorded, then edited in the builder, then recorded again match the composition', () => { + fc.assert( + fc.property( + ukBuilderHouseholdArbitrary, + fc.array(builderActionArbitrary, { maxLength: 12 }), + (household, laterActions) => { + const edited = laterActions + .reduce(applyBuilderAction, household.withBuilderClaimantRoles(ROLE_TEST_YEAR)) + .withBuilderClaimantRoles(ROLE_TEST_YEAR); + const recorded = Object.fromEntries( + edited.personNames.map((personName) => [ + personName, + edited.getPersonVariableAtYear( + personName, + UK_CLAIMANT_OR_PARTNER_VARIABLE, + ROLE_TEST_YEAR + ), + ]) + ); + + expect(recorded).toEqual(edited.getBuilderClaimantRoles(ROLE_TEST_YEAR)); + } + ) + ); + }); + test('setting the child count gives that many children and keeps every adult and the partner', () => { fc.assert( fc.property(builderHouseholdArbitrary, fc.integer({ min: 0, max: 5 }), (household, count) => { diff --git a/app/src/tests/unit/pages/reportBuilder/utils/createReportSimulations.test.ts b/app/src/tests/unit/pages/reportBuilder/utils/createReportSimulations.test.ts index e9efab21c..b9dc37952 100644 --- a/app/src/tests/unit/pages/reportBuilder/utils/createReportSimulations.test.ts +++ b/app/src/tests/unit/pages/reportBuilder/utils/createReportSimulations.test.ts @@ -4,6 +4,7 @@ import { Household, UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; import { createReportSimulations } from '@/pages/reportBuilder/utils/createReportSimulations'; import { BUILDER_PEOPLE, + CREATED_HOUSEHOLD_ID, ROLE_TEST_YEAR, singleParentWithAdultDependant, UK_METADATA_WITH_CLAIMANT_ROLES, @@ -234,7 +235,6 @@ describe('createReportSimulations', () => { }); describe('given a UK draft household of a lone parent and their adult child', () => { - const CREATED_UK_HOUSEHOLD_ID = 'created-uk-household'; const ukDraftSimulation = () => ({ ...mockDraftHouseholdSimulation('uk'), population: { @@ -246,7 +246,7 @@ describe('createReportSimulations', () => { beforeEach(() => { vi.mocked(createHousehold).mockResolvedValue({ - result: { household_id: CREATED_UK_HOUSEHOLD_ID }, + result: { household_id: CREATED_HOUSEHOLD_ID }, }); }); @@ -266,7 +266,7 @@ describe('createReportSimulations', () => { createdPeople()[BUILDER_PEOPLE.FIRST_DEPENDANT][UK_CLAIMANT_OR_PARTNER_VARIABLE] ).toEqual({ [ROLE_TEST_YEAR]: false }); const savedHousehold = result.simulationStates[0].population.household!; - expect(savedHousehold.id).toBe(CREATED_UK_HOUSEHOLD_ID); + expect(savedHousehold.id).toBe(CREATED_HOUSEHOLD_ID); expect(savedHousehold.toV1CreationPayload().data.people).toEqual(createdPeople()); }); diff --git a/app/src/tests/unit/utils/builderClaimantRoles.test.ts b/app/src/tests/unit/utils/builderClaimantRoles.test.ts index 99459988a..f06cfed44 100644 --- a/app/src/tests/unit/utils/builderClaimantRoles.test.ts +++ b/app/src/tests/unit/utils/builderClaimantRoles.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'vitest'; import { UK_CLAIMANT_OR_PARTNER_VARIABLE } from '@/models/Household'; import { BUILDER_PEOPLE, + householdWithSeparateClaimant, ROLE_TEST_YEAR, singleParentWithAdultDependant, UK_METADATA_WITH_CLAIMANT_ROLES, @@ -60,6 +61,37 @@ describe('withSupportedBuilderClaimantRoles', () => { ); }); + test('given the UK model predates the variable then roles a household already carries are removed', () => { + const household = singleParentWithAdultDependant().withBuilderClaimantRoles(ROLE_TEST_YEAR); + + const result = withSupportedBuilderClaimantRoles( + household, + UK_METADATA_WITHOUT_CLAIMANT_ROLES, + ROLE_TEST_YEAR + ); + + expect(JSON.stringify(result.toV1CreationPayload())).not.toContain( + UK_CLAIMANT_OR_PARTNER_VARIABLE + ); + expect(result.householdData).toEqual(singleParentWithAdultDependant().householdData); + }); + + test('given model information for another country then roles a household carries are kept', () => { + const household = singleParentWithAdultDependant().withBuilderClaimantRoles(ROLE_TEST_YEAR); + + expect( + withSupportedBuilderClaimantRoles(household, US_METADATA_WITH_CLAIMANT_ROLES, ROLE_TEST_YEAR) + ).toBe(household); + }); + + test('given a household with two benefit units then no roles are added', () => { + const household = householdWithSeparateClaimant({ age: 40 }); + + expect( + withSupportedBuilderClaimantRoles(household, UK_METADATA_WITH_CLAIMANT_ROLES, ROLE_TEST_YEAR) + ).toBe(household); + }); + test('given a US household then no UK roles are added', () => { const household = singleParentWithAdultDependant('us'); diff --git a/app/src/tests/unit/utils/householdBuilderComposition.test.ts b/app/src/tests/unit/utils/householdBuilderComposition.test.ts index f5803bd51..339c95e19 100644 --- a/app/src/tests/unit/utils/householdBuilderComposition.test.ts +++ b/app/src/tests/unit/utils/householdBuilderComposition.test.ts @@ -4,7 +4,9 @@ import { BUILDER_AGES, BUILDER_PEOPLE, coupleWithAdultDependant, + householdWithSeparateClaimant, ROLE_TEST_YEAR, + SEPARATE_CLAIMANT, singleParentWithAdultDependant, } from '@/tests/fixtures/models/builderClaimantRolesMocks'; @@ -149,4 +151,50 @@ describe('Household builder composition methods', () => { expect(household.getBuilderPartnerKey(ROLE_TEST_YEAR)).toBeNull(); }); + + describe('given a flagged claimant in another benefit unit', () => { + const household = () => + householdWithSeparateClaimant({ age: 17, isClaimantOrPartner: true }, true); + + test('then they are not read as the partner', () => { + expect(household().deriveBuilderComposition(ROLE_TEST_YEAR)).toEqual( + expect.objectContaining({ partnerKey: null, maritalStatus: 'single' }) + ); + }); + + test('then choosing single keeps them and their benefit unit', () => { + const updated = household().withBuilderMaritalStatus(ROLE_TEST_YEAR, 'single'); + + expect(updated.personNames).toContain(SEPARATE_CLAIMANT); + expect(updated.getGroupMembers('benunits', 'their benefit unit')).toEqual([ + SEPARATE_CLAIMANT, + ]); + }); + + test('then choosing married adds a partner beside "you" and keeps them', () => { + const updated = household().withBuilderMaritalStatus(ROLE_TEST_YEAR, 'married'); + + expect(updated.getBuilderPartnerKey(ROLE_TEST_YEAR)).toBe(BUILDER_PEOPLE.PARTNER); + expect(updated.personNames).toContain(SEPARATE_CLAIMANT); + }); + }); + + test('given a dependant ordinal is skipped then a new child is numbered after the highest one', () => { + const household = Household.starter('uk', ROLE_TEST_YEAR) + .withBuilderChildCount(ROLE_TEST_YEAR, 2) + .setPersonVariableAtYear( + BUILDER_PEOPLE.SECOND_DEPENDANT, + 'age', + ROLE_TEST_YEAR, + BUILDER_AGES.ADULT_DEPENDANT + ) + .withBuilderChildCount(ROLE_TEST_YEAR, 0) + .withBuilderChildCount(ROLE_TEST_YEAR, 1); + + expect(household.getSortedPersonNames()).toEqual([ + BUILDER_PEOPLE.YOU, + BUILDER_PEOPLE.SECOND_DEPENDANT, + 'your third dependent', + ]); + }); }); diff --git a/app/src/utils/builderClaimantRoles.ts b/app/src/utils/builderClaimantRoles.ts index 082558274..ac1f6e2a1 100644 --- a/app/src/utils/builderClaimantRoles.ts +++ b/app/src/utils/builderClaimantRoles.ts @@ -3,26 +3,38 @@ import type { MetadataState } from '@/types/metadata'; export type ClaimantRoleModelMetadata = Pick; +function withoutClaimantRoles(household: Household): Household { + const people = household.people; + return Object.keys(people) + .filter((personKey) => people[personKey][UK_CLAIMANT_OR_PARTNER_VARIABLE] !== undefined) + .reduce( + (next, personKey) => + next.removeEntityVariable('people', personKey, UK_CLAIMANT_OR_PARTNER_VARIABLE), + household + ); +} + /** - * Adds explicit claimant and partner roles to a UK builder household before it is saved, so - * policyengine-uk does not infer a couple from ages alone. + * Prepares a household's claimant and partner roles for the loaded model before it is saved, + * so policyengine-uk does not infer a couple from ages alone. * - * policyengine-core rejects a situation that names a variable its model lacks, so the roles are - * added only when the loaded model information for the household's country defines the - * variable. Until the API's model does, households save exactly as before. + * policyengine-core rejects a situation that names a variable its model lacks. So when the loaded + * model information for the household's country defines the variable, a UK builder household + * gets its roles; when it does not, any roles the household already carries are removed. Until + * model information for the household's country has loaded, the household is left unchanged. */ export function withSupportedBuilderClaimantRoles( household: Household, metadata: ClaimantRoleModelMetadata | undefined, year: string | number | null = household.year ): Household { - if ( - year === null || - metadata?.currentCountry !== household.countryId || - !metadata.variables?.[UK_CLAIMANT_OR_PARTNER_VARIABLE] - ) { + if (metadata?.currentCountry !== household.countryId) { return household; } - return household.withBuilderClaimantRoles(String(year)); + if (!metadata.variables?.[UK_CLAIMANT_OR_PARTNER_VARIABLE]) { + return withoutClaimantRoles(household); + } + + return year === null ? household : household.withBuilderClaimantRoles(String(year)); } From 3768c7f7117828953bf1600a1d46356bb052cd34 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 3 Oct 2026 07:33:02 -0400 Subject: [PATCH 3/3] Keep builder edits inside the family unit of "you" (round-2 review fixes) The second independent review (of 0f4234ba) confirmed the round-1 fixes and found two more cases; both are fixed here, with three smaller points. - The partner and child controls manage only the primary person's own family unit (UK benefit unit, US tax unit). A 17-year-old claiming in another benefit unit was counted as a child, so setting the child count to zero deleted them and their unit. Main had the same behaviour. - A claimant-or-partner flag names a partner only when "you" are flagged as a claimant too. Before, "you" flagged false (or unflagged, which policyengine-core reads as false once anyone has the input) still acquired a flagged member as partner, and "single" deleted them. - New partners and children join the groups that hold "you", not whichever group is listed first, so a saved household whose other unit comes first no longer gets the new partner in the wrong unit. - The role helper treats model information as loaded only when it has finished loading without error (the app's getModelMetadataError), so roles are never stripped while it is still loading. Tests: regression cases for each, a US tax-unit case, and a fast-check property that builder edits on a multi-unit household never touch the other unit and that new members join the unit of "you". Undoing each fix fails 5, 2, 2 and 1 tests. Co-Authored-By: Claude Opus 5.5 --- app/src/models/Household.ts | 84 ++++++++++++++---- .../models/builderClaimantRolesMocks.ts | 25 ++++-- .../householdBuilderRoles.property.test.ts | 39 ++++++++- .../unit/utils/builderClaimantRoles.test.ts | 9 ++ .../utils/householdBuilderComposition.test.ts | 86 ++++++++++++++++++- app/src/utils/builderClaimantRoles.ts | 11 ++- 6 files changed, 222 insertions(+), 32 deletions(-) diff --git a/app/src/models/Household.ts b/app/src/models/Household.ts index e83e9590f..f84637c3a 100644 --- a/app/src/models/Household.ts +++ b/app/src/models/Household.ts @@ -65,6 +65,12 @@ const BUILDER_DEPENDENT_ORDINALS = ['first', 'second', 'third', 'fourth', 'fifth */ export const UK_CLAIMANT_OR_PARTNER_VARIABLE = 'is_claimant_or_partner'; +/** The unit whose members the builder's partner and child controls manage, by country. */ +const BUILDER_FAMILY_GROUPS: Partial> = { + uk: 'benunits', + us: 'taxUnits', +}; + export interface PersonWithName extends AppHouseholdInputPerson { name: string; } @@ -153,7 +159,8 @@ function ensureGroupCollection( return householdData[entityName] as Record; } -function addPersonToFirstGroup( +/** Adds the person to the group holding "you" (the builder's own unit), else the first group. */ +function addPersonToBuilderGroup( groupMap: Record, defaultGroupName: string, personKey: string @@ -162,9 +169,11 @@ function addPersonToFirstGroup( groupMap[defaultGroupName] = { members: [] }; } - const firstGroup = Object.values(groupMap)[0]; - if (!firstGroup.members.includes(personKey)) { - firstGroup.members.push(personKey); + const groups = Object.values(groupMap); + const builderGroup = + groups.find((group) => group.members.includes(BUILDER_PRIMARY_PERSON_NAME)) ?? groups[0]; + if (!builderGroup.members.includes(personKey)) { + builderGroup.members.push(personKey); } } @@ -426,7 +435,7 @@ function applyDefaultHouseholdGroups( householdData: AppHouseholdInputData, personKey: string ): void { - addPersonToFirstGroup( + addPersonToBuilderGroup( ensureGroupCollection(householdData, 'households'), 'your household', personKey @@ -438,13 +447,17 @@ function applyUSDefaultGroups( personKey: string, personType: 'adult' | 'child' ): void { - addPersonToFirstGroup( + addPersonToBuilderGroup( ensureGroupCollection(householdData, 'taxUnits'), 'your tax unit', personKey ); - addPersonToFirstGroup(ensureGroupCollection(householdData, 'families'), 'your family', personKey); - addPersonToFirstGroup( + addPersonToBuilderGroup( + ensureGroupCollection(householdData, 'families'), + 'your family', + personKey + ); + addPersonToBuilderGroup( ensureGroupCollection(householdData, 'spmUnits'), 'your household', personKey @@ -459,7 +472,7 @@ function applyUSDefaultGroups( } function applyUKDefaultGroups(householdData: AppHouseholdInputData, personKey: string): void { - addPersonToFirstGroup( + addPersonToBuilderGroup( ensureGroupCollection(householdData, 'benunits'), 'your benefit unit', personKey @@ -919,15 +932,22 @@ export class Household extends BaseModel { } // Only an explicit partner counts. Age never makes one: a dependant aged 25 stays a dependant. - // A claimant-or-partner flag names the partner only inside the primary person's benefit unit; - // a flagged member of another unit is that unit's own claimant. - const benefitUnitMates = getGroupMatesOf(this.appInputData.benunits, resolvedPrimaryPersonKey); + // A claimant-or-partner flag names a partner only when the primary person is flagged as a + // claimant too, and only inside their own family unit: a flagged member of another unit is + // that unit's own claimant. + const familyMembers = this.getBuilderFamilyMembers(resolvedPrimaryPersonKey); + const isFlaggedClaimantOrPartner = (personKey: string) => + this.getPersonVariableAtYear(personKey, UK_CLAIMANT_OR_PARTNER_VARIABLE, year) === true; + if (!familyMembers || !isFlaggedClaimantOrPartner(resolvedPrimaryPersonKey)) { + return null; + } + return ( this.getSortedPersonNames().find( (personKey) => personKey !== resolvedPrimaryPersonKey && - benefitUnitMates.has(personKey) && - this.getPersonVariableAtYear(personKey, UK_CLAIMANT_OR_PARTNER_VARIABLE, year) === true + familyMembers.has(personKey) && + isFlaggedClaimantOrPartner(personKey) ) ?? null ); } @@ -940,6 +960,9 @@ export class Household extends BaseModel { const resolvedPrimaryPersonKey = primaryPersonKey ?? this.getBuilderPrimaryPersonKey(year); const resolvedPartnerKey = partnerKey ?? this.getBuilderPartnerKey(year, resolvedPrimaryPersonKey); + const familyMembers = resolvedPrimaryPersonKey + ? this.getBuilderFamilyMembers(resolvedPrimaryPersonKey) + : null; return Household.sortPersonNames( Object.entries(this.appInputData.people) @@ -947,6 +970,9 @@ export class Household extends BaseModel { if (personKey === resolvedPrimaryPersonKey || personKey === resolvedPartnerKey) { return false; } + if (familyMembers && !familyMembers.has(personKey)) { + return false; + } return isManagedBuilderChild(person, year); }) @@ -954,6 +980,20 @@ export class Household extends BaseModel { ); } + /** + * Members of the primary person's own family unit (UK benefit unit, US tax unit). The builder's + * partner and child controls manage only these; members of another unit belong to that unit's + * own claimant. Null when the country has no such unit or the primary person is in none, and + * then the whole household counts. + */ + private getBuilderFamilyMembers(primaryPersonKey: string): Set | null { + const groupKey = BUILDER_FAMILY_GROUPS[this.countryId]; + const members = groupKey + ? getGroupMatesOf(getGroupCollectionFromData(this.appInputData, groupKey), primaryPersonKey) + : new Set(); + return members.size > 0 ? members : null; + } + deriveBuilderComposition(year: string): HouseholdBuilderComposition { const people = this.getSortedPersonNames(); const primaryPersonKey = this.getBuilderPrimaryPersonKey(year); @@ -1268,12 +1308,18 @@ export class Household extends BaseModel { const parentIds = [composition.primaryPersonKey, composition.partnerKey].filter( Boolean ) as string[]; - // Number new children after every dependant, including adult dependants the count excludes, - // and after the highest dependant ordinal in use, so a gap never produces "... dependent 2". - const dependants = composition.people.filter((personKey) => !parentIds.includes(personKey)); + // Number new children after every dependant in the family unit, adult dependants included, + // and after the highest dependant ordinal anyone uses, so names never collide or repeat. + const familyMembers = composition.primaryPersonKey + ? this.getBuilderFamilyMembers(composition.primaryPersonKey) + : null; + const dependantCount = composition.people.filter( + (personKey) => + !parentIds.includes(personKey) && (!familyMembers || familyMembers.has(personKey)) + ).length; const firstNewIndex = Math.max( - dependants.length, - ...dependants.map((personKey) => getBuilderDependentIndex(personKey) + 1) + dependantCount, + ...composition.people.map((personKey) => getBuilderDependentIndex(personKey) + 1) ); for (let offset = 0; offset < newCount - composition.childKeys.length; offset += 1) { diff --git a/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts b/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts index 59329b74e..df566c731 100644 --- a/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts +++ b/app/src/tests/fixtures/models/builderClaimantRolesMocks.ts @@ -25,6 +25,9 @@ export const BUILDER_AGES = { /** Model information whose UK model defines is_claimant_or_partner. */ export const UK_METADATA_WITH_CLAIMANT_ROLES: ClaimantRoleModelMetadata = { currentCountry: 'uk', + loading: false, + error: null, + version: 'test-model', variables: { age: { name: 'age', entity: 'person', valueType: 'float', isInputVariable: true }, [UK_CLAIMANT_OR_PARTNER_VARIABLE]: { @@ -40,6 +43,9 @@ export const UK_METADATA_WITH_CLAIMANT_ROLES: ClaimantRoleModelMetadata = { /** Model information for a UK model that predates is_claimant_or_partner. */ export const UK_METADATA_WITHOUT_CLAIMANT_ROLES: ClaimantRoleModelMetadata = { currentCountry: 'uk', + loading: false, + error: null, + version: 'test-model', variables: { age: { name: 'age', entity: 'person', valueType: 'float', isInputVariable: true }, }, @@ -70,6 +76,15 @@ export function ukBuilderMetadata(claimantRoles: ClaimantRoleModelMetadata): Met }; } +/** UK model information still loading: the store holds no variables yet. */ +export const UK_METADATA_LOADING: ClaimantRoleModelMetadata = { + currentCountry: 'uk', + loading: true, + error: null, + version: null, + variables: {}, +}; + /** Model information loaded for another country than the household's. */ export const US_METADATA_WITH_CLAIMANT_ROLES: ClaimantRoleModelMetadata = { ...UK_METADATA_WITH_CLAIMANT_ROLES, @@ -107,8 +122,11 @@ export const SEPARATE_CLAIMANT = 'other claimant'; */ export function householdWithSeparateClaimant( separateClaimant: { age: number; isClaimantOrPartner?: boolean }, - youIsClaimantOrPartner?: boolean + youIsClaimantOrPartner?: boolean, + separateUnitFirst = false ): Household { + const yourUnit = { 'your benefit unit': { members: [BUILDER_PEOPLE.YOU] } }; + const theirUnit = { 'their benefit unit': { members: [SEPARATE_CLAIMANT] } }; const person = ( age: number, isClaimantOrPartner: boolean | undefined @@ -131,10 +149,7 @@ export function householdWithSeparateClaimant( households: { 'your household': { members: [BUILDER_PEOPLE.YOU, SEPARATE_CLAIMANT] }, }, - benunits: { - 'your benefit unit': { members: [BUILDER_PEOPLE.YOU] }, - 'their benefit unit': { members: [SEPARATE_CLAIMANT] }, - }, + benunits: separateUnitFirst ? { ...theirUnit, ...yourUnit } : { ...yourUnit, ...theirUnit }, }, }); } diff --git a/app/src/tests/unit/models/householdBuilderRoles.property.test.ts b/app/src/tests/unit/models/householdBuilderRoles.property.test.ts index 9120ffca1..7b6bb00d7 100644 --- a/app/src/tests/unit/models/householdBuilderRoles.property.test.ts +++ b/app/src/tests/unit/models/householdBuilderRoles.property.test.ts @@ -70,7 +70,8 @@ const optionalRoleArbitrary = fc.option(fc.boolean(), { nil: undefined }); function withSeparateClaimant( household: Household, age: number, - isClaimantOrPartner: boolean | undefined + isClaimantOrPartner: boolean | undefined, + separateUnitFirst: boolean ): Household { const data = household.householdData; data.people[SEPARATE_CLAIMANT] = { @@ -79,7 +80,14 @@ function withSeparateClaimant( ? {} : { [UK_CLAIMANT_OR_PARTNER_VARIABLE]: { [ROLE_TEST_YEAR]: isClaimantOrPartner } }), }; - data.benunits = { ...data.benunits, 'their benefit unit': { members: [SEPARATE_CLAIMANT] } }; + const theirUnit = { 'their benefit unit': { members: [SEPARATE_CLAIMANT] } }; + data.benunits = separateUnitFirst + ? { ...theirUnit, ...data.benunits } + : { ...data.benunits, ...theirUnit }; + const [householdName, householdGroup] = Object.entries(data.households ?? {})[0]; + data.households = { + [householdName]: { ...householdGroup, members: [...householdGroup.members, SEPARATE_CLAIMANT] }, + }; return Household.fromAppInput({ ...household.toAppInput(), householdData: data }); } @@ -90,12 +98,14 @@ const multiUnitHouseholdArbitrary = fc recordRoles: fc.boolean(), age: ageArbitrary, isClaimantOrPartner: optionalRoleArbitrary, + separateUnitFirst: fc.boolean(), }) - .map(({ household, recordRoles, age, isClaimantOrPartner }) => + .map(({ household, recordRoles, age, isClaimantOrPartner, separateUnitFirst }) => withSeparateClaimant( recordRoles ? household.withBuilderClaimantRoles(ROLE_TEST_YEAR) : household, age, - isClaimantOrPartner + isClaimantOrPartner, + separateUnitFirst ) ); @@ -227,6 +237,27 @@ describe('household builder roles properties', () => { ); }); + test('builder edits never touch another benefit unit, and new members join the unit of "you"', () => { + fc.assert( + fc.property( + multiUnitHouseholdArbitrary, + fc.array(builderActionArbitrary, { maxLength: 12 }), + (household, actions) => { + const edited = actions.reduce(applyBuilderAction, household); + const yourUnit = edited.getGroupMembers('benunits', 'your benefit unit'); + + expect(edited.getGroupMembers('benunits', 'their benefit unit')).toEqual([ + SEPARATE_CLAIMANT, + ]); + expect([...yourUnit, SEPARATE_CLAIMANT].sort()).toEqual([...edited.personNames].sort()); + expect(edited.deriveBuilderComposition(ROLE_TEST_YEAR).childKeys).not.toContain( + SEPARATE_CLAIMANT + ); + } + ) + ); + }); + test('a household with more than one benefit unit gets no generated roles', () => { fc.assert( fc.property(multiUnitHouseholdArbitrary, (household) => { diff --git a/app/src/tests/unit/utils/builderClaimantRoles.test.ts b/app/src/tests/unit/utils/builderClaimantRoles.test.ts index f06cfed44..4c7a5ce48 100644 --- a/app/src/tests/unit/utils/builderClaimantRoles.test.ts +++ b/app/src/tests/unit/utils/builderClaimantRoles.test.ts @@ -5,6 +5,7 @@ import { householdWithSeparateClaimant, ROLE_TEST_YEAR, singleParentWithAdultDependant, + UK_METADATA_LOADING, UK_METADATA_WITH_CLAIMANT_ROLES, UK_METADATA_WITHOUT_CLAIMANT_ROLES, US_METADATA_WITH_CLAIMANT_ROLES, @@ -76,6 +77,14 @@ describe('withSupportedBuilderClaimantRoles', () => { expect(result.householdData).toEqual(singleParentWithAdultDependant().householdData); }); + test('given UK model information is still loading then roles a household carries are kept', () => { + const household = singleParentWithAdultDependant().withBuilderClaimantRoles(ROLE_TEST_YEAR); + + expect(withSupportedBuilderClaimantRoles(household, UK_METADATA_LOADING, ROLE_TEST_YEAR)).toBe( + household + ); + }); + test('given model information for another country then roles a household carries are kept', () => { const household = singleParentWithAdultDependant().withBuilderClaimantRoles(ROLE_TEST_YEAR); diff --git a/app/src/tests/unit/utils/householdBuilderComposition.test.ts b/app/src/tests/unit/utils/householdBuilderComposition.test.ts index 339c95e19..a1294325c 100644 --- a/app/src/tests/unit/utils/householdBuilderComposition.test.ts +++ b/app/src/tests/unit/utils/householdBuilderComposition.test.ts @@ -132,10 +132,16 @@ describe('Household builder composition methods', () => { expect(household.getBuilderPartnerKey(ROLE_TEST_YEAR)).toBeNull(); }); - test('given a member flagged as claimant or partner then that member is the partner', () => { + test('given "you" and another member are both flagged as claimant or partner then that member is the partner', () => { const household = Household.empty('uk', ROLE_TEST_YEAR) .addAdult(BUILDER_PEOPLE.YOU, BUILDER_AGES.PARENT) .addAdult('alex', BUILDER_AGES.PARTNER) + .setPersonVariableAtYear( + BUILDER_PEOPLE.YOU, + UK_CLAIMANT_OR_PARTNER_VARIABLE, + ROLE_TEST_YEAR, + true + ) .setPersonVariableAtYear('alex', UK_CLAIMANT_OR_PARTNER_VARIABLE, ROLE_TEST_YEAR, true); expect(household.deriveBuilderComposition(ROLE_TEST_YEAR)).toEqual( @@ -143,6 +149,39 @@ describe('Household builder composition methods', () => { ); }); + test.each([ + ['flagged false', false], + ['not flagged', undefined], + ])( + 'given "you" are %s then flagged members of the unit are not your partner and "single" keeps them', + (_case, youIsClaimantOrPartner) => { + const flagged = (household: Household, personKey: string, value: boolean | undefined) => + value === undefined + ? household + : household.setPersonVariableAtYear( + personKey, + UK_CLAIMANT_OR_PARTNER_VARIABLE, + ROLE_TEST_YEAR, + value + ); + const household = flagged( + Household.empty('uk', ROLE_TEST_YEAR) + .addAdult(BUILDER_PEOPLE.YOU, 1) + .addAdult('mum', 17) + .addAdult('dad', 17) + .setPersonVariableAtYear('mum', UK_CLAIMANT_OR_PARTNER_VARIABLE, ROLE_TEST_YEAR, true) + .setPersonVariableAtYear('dad', UK_CLAIMANT_OR_PARTNER_VARIABLE, ROLE_TEST_YEAR, true), + BUILDER_PEOPLE.YOU, + youIsClaimantOrPartner + ); + + expect(household.getBuilderPartnerKey(ROLE_TEST_YEAR)).toBeNull(); + expect(household.withBuilderMaritalStatus(ROLE_TEST_YEAR, 'single').personNames).toEqual( + expect.arrayContaining(['mum', 'dad']) + ); + } + ); + test('given a member flagged for another year only then that member is not the partner', () => { const household = Household.empty('uk', ROLE_TEST_YEAR) .addAdult(BUILDER_PEOPLE.YOU, BUILDER_AGES.PARENT) @@ -177,6 +216,51 @@ describe('Household builder composition methods', () => { expect(updated.getBuilderPartnerKey(ROLE_TEST_YEAR)).toBe(BUILDER_PEOPLE.PARTNER); expect(updated.personNames).toContain(SEPARATE_CLAIMANT); }); + + test('then the child count does not count them and setting it to zero keeps them', () => { + const updated = household().withBuilderChildCount(ROLE_TEST_YEAR, 0); + + expect(household().deriveBuilderComposition(ROLE_TEST_YEAR).numChildren).toBe(0); + expect(updated.personNames).toContain(SEPARATE_CLAIMANT); + expect(updated.getGroupMembers('benunits', 'their benefit unit')).toEqual([ + SEPARATE_CLAIMANT, + ]); + }); + + test.each([ + ['their unit is listed second', false], + ['their unit is listed first', true], + ])('then a new partner and child join the benefit unit of "you" when %s', (_case, first) => { + const updated = householdWithSeparateClaimant( + { age: 17, isClaimantOrPartner: true }, + true, + first + ) + .withBuilderMaritalStatus(ROLE_TEST_YEAR, 'married') + .withBuilderChildCount(ROLE_TEST_YEAR, 1); + + expect(updated.getGroupMembers('benunits', 'your benefit unit')).toEqual([ + BUILDER_PEOPLE.YOU, + BUILDER_PEOPLE.PARTNER, + BUILDER_PEOPLE.FIRST_DEPENDANT, + ]); + expect(updated.getGroupMembers('benunits', 'their benefit unit')).toEqual([ + SEPARATE_CLAIMANT, + ]); + }); + }); + + test('given a US child in another tax unit then the child count leaves them alone', () => { + const base = Household.starter('us', ROLE_TEST_YEAR).addChild('nephew', 16); + const data = base.householdData; + data.taxUnits = { + 'your tax unit': { members: [BUILDER_PEOPLE.YOU] }, + "nephew's tax unit": { members: ['nephew'] }, + }; + const household = Household.fromAppInput({ ...base.toAppInput(), householdData: data }); + + expect(household.deriveBuilderComposition(ROLE_TEST_YEAR).numChildren).toBe(0); + expect(household.withBuilderChildCount(ROLE_TEST_YEAR, 0).personNames).toContain('nephew'); }); test('given a dependant ordinal is skipped then a new child is numbered after the highest one', () => { diff --git a/app/src/utils/builderClaimantRoles.ts b/app/src/utils/builderClaimantRoles.ts index ac1f6e2a1..7bcab833c 100644 --- a/app/src/utils/builderClaimantRoles.ts +++ b/app/src/utils/builderClaimantRoles.ts @@ -1,7 +1,11 @@ import { UK_CLAIMANT_OR_PARTNER_VARIABLE, type Household } from '@/models/Household'; import type { MetadataState } from '@/types/metadata'; +import { getModelMetadataError } from './spmSelection'; -export type ClaimantRoleModelMetadata = Pick; +export type ClaimantRoleModelMetadata = Pick< + MetadataState, + 'currentCountry' | 'loading' | 'error' | 'version' | 'variables' +>; function withoutClaimantRoles(household: Household): Household { const people = household.people; @@ -21,14 +25,15 @@ function withoutClaimantRoles(household: Household): Household { * policyengine-core rejects a situation that names a variable its model lacks. So when the loaded * model information for the household's country defines the variable, a UK builder household * gets its roles; when it does not, any roles the household already carries are removed. Until - * model information for the household's country has loaded, the household is left unchanged. + * model information for the household's country has loaded without error, the household is left + * unchanged. */ export function withSupportedBuilderClaimantRoles( household: Household, metadata: ClaimantRoleModelMetadata | undefined, year: string | number | null = household.year ): Household { - if (metadata?.currentCountry !== household.countryId) { + if (!metadata || getModelMetadataError(household.countryId, metadata)) { return household; }