-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(profiling): PNTS selections store null not false; routeToNextProfileQuestion skips after-field #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -226,12 +226,15 @@ const needProfileRequirements: Record<string, string[]> = { | |
| * and what profile data has already been collected. | ||
| * Returns terminal output when all required fields are collected. | ||
| */ | ||
| function routeToNextProfileQuestion(session: SessionState): RoutingResult { | ||
| function routeToNextProfileQuestion(session: SessionState, after?: string): RoutingResult { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not blocking, but worth considering: typing Small change, and it protects the fix going forward. Up to you. |
||
| const need = session.supportNeed || ''; | ||
| const required = needProfileRequirements[need] || []; | ||
|
|
||
| // Check each required field in order | ||
| for (const field of required) { | ||
| // Skip the field we just answered — prevents re-triggering on null PNTS values | ||
| if (after && field === after) continue; | ||
|
|
||
| // Age | ||
| if (field === 'age' && !session.ageCategory) { | ||
| return { | ||
|
|
@@ -1292,26 +1295,26 @@ export function processInput(session: SessionState, input: string): RoutingResul | |
| if (profAge === '25 or over') mappedProfAge = '25+'; | ||
|
|
||
| const sessionWithAge = { ...session, ageCategory: mappedProfAge }; | ||
| return routeToNextProfileQuestion(sessionWithAge); | ||
| return routeToNextProfileQuestion(sessionWithAge, 'age'); | ||
|
|
||
| case 'B5_PROFILE_GENDER': | ||
| const profGenderOptions = ['Male', 'Female', 'Non-binary or other', 'Prefer not to say']; | ||
| const profGender = choice ? profGenderOptions[choice - 1] : null; | ||
|
|
||
| const sessionWithGender = { ...session, gender: profGender }; | ||
| return routeToNextProfileQuestion(sessionWithGender); | ||
| return routeToNextProfileQuestion(sessionWithGender, 'gender'); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might be missing something, but it looks like this handler returns I can see from the PR description you're already tracking this as backlog item 59. Would mind adding the backlog to tracking and commit it with the next PR? It would be helpful for me to see what's known and what isn't. |
||
|
|
||
| case 'B5_PROFILE_LGBTQ': { | ||
| // 1 = Yes, 2 = No, 3 = Prefer not to say | ||
| const lgbtqValue = choice === 1 ? true : false; | ||
| const lgbtqValue = choice === 1 ? true : (choice === 2 ? false : null); | ||
| if (lgbtqValue === true) { | ||
| return { | ||
| ...phrase('LGBTQ_SPECIALIST_ASK', session.isSupporter), | ||
| stateUpdates: { currentGate: 'LGBTQ_SPECIALIST_ASK', lgbtq: true } | ||
| }; | ||
| } | ||
| const sessionWithLgbtq = { ...session, lgbtq: lgbtqValue }; | ||
| const lgbtqResult = routeToNextProfileQuestion(sessionWithLgbtq); | ||
| const lgbtqResult = routeToNextProfileQuestion(sessionWithLgbtq, 'lgbtq'); | ||
| return { | ||
| ...lgbtqResult, | ||
| stateUpdates: { | ||
|
|
@@ -1324,7 +1327,7 @@ export function processInput(session: SessionState, input: string): RoutingResul | |
| case 'LGBTQ_SPECIALIST_ASK': { | ||
| const lgbtqPref = choice === 1 ? 'Specialist first' : 'Show both'; | ||
| const sessionWithSpec = { ...session, lgbtqServicePreference: lgbtqPref }; | ||
| const specResult = routeToNextProfileQuestion(sessionWithSpec); | ||
| const specResult = routeToNextProfileQuestion(sessionWithSpec, 'lgbtq'); | ||
| return { | ||
| ...specResult, | ||
| stateUpdates: { | ||
|
|
@@ -1339,7 +1342,7 @@ export function processInput(session: SessionState, input: string): RoutingResul | |
| const convictions = choice ? convictionOptions[choice - 1] : null; | ||
|
|
||
| const sessionWithConvictions = { ...session, criminalConvictions: convictions }; | ||
| return routeToNextProfileQuestion(sessionWithConvictions); | ||
| return routeToNextProfileQuestion(sessionWithConvictions, 'convictions'); | ||
|
|
||
| case 'IMMIGRATION_STATUS_ASK': { | ||
| const immigrationMap: Record<number, { status: string; funds: string | null }> = { | ||
|
|
@@ -1362,7 +1365,7 @@ export function processInput(session: SessionState, input: string): RoutingResul | |
| immigrationStatus, | ||
| publicFunds, | ||
| }; | ||
| const result = routeToNextProfileQuestion(sessionWithImmigration); | ||
| const result = routeToNextProfileQuestion(sessionWithImmigration, 'nrpf'); | ||
| return { | ||
| ...result, | ||
| stateUpdates: { | ||
|
|
@@ -1375,10 +1378,10 @@ export function processInput(session: SessionState, input: string): RoutingResul | |
|
|
||
| case 'B5_PROFILE_CHILDREN': | ||
| // 1 = Yes, 2 = No, 3 = Prefer not to say | ||
| const childrenValue = choice === 1 ? true : false; | ||
| const childrenValue = choice === 1 ? true : (choice === 2 ? false : null); | ||
|
|
||
| const sessionWithChildren = { ...session, hasChildren: childrenValue }; | ||
| return routeToNextProfileQuestion(sessionWithChildren); | ||
| return routeToNextProfileQuestion(sessionWithChildren, 'children'); | ||
|
|
||
| case 'B6_HOMELESSNESS_STATUS': | ||
| const homeless = choice === 1; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed this new regression test looks like it's covering the same ground as the existing test at line 798 ("User who prefers not to say on children question progresses past the question"). Same session setup, same option selected, same assertion that the state doesn't come back to
B5_PROFILE_CHILDREN.Not a problem for correctness, but two identical tests means if the behaviour ever changes we'll get two failures for the same thing, and one of them will need updating anyway. Could we drop one? Either is fine, though I'd keep the new one if it has a clearer name tied to the fix in this PR.