From 41cf29d8411e64957e1c2d75219a1e5ab2f84bd9 Mon Sep 17 00:00:00 2001 From: Ethan Tison <97046400+EJTison@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:12:41 -0400 Subject: [PATCH 1/2] Split NSO and IBS training costs into two sections The Edit Training Costs modal captured NSO and IBS as one combined "NSO & IBS Cost" section, so a cohort could not be priced for an attendee who goes to NSO but not IBS. Splits the section into "NSO Cost" (unchanged four room/household types) and a new "IBS Cost" (Single, Couple), taking the modal from 11 to 13 fields. The keys now mirror NewStaffCohort::COST_FIELDS in mpdx_api one-for-one, so the modal can populate ibs_single_cost and ibs_couple_cost once the cohort API lands. Requested by Caleb Cox in Figma comment 127 on node 789-32532, after MPDX-9701 had already merged. Co-Authored-By: Claude Opus 5 --- public/locales/en/translation.json | 3 +- .../EditTrainingCostsModal.test.tsx | 52 +++++++++++-------- .../EditTrainingCostsModal.tsx | 37 +++++++++---- .../MpdGoalAdmin/MpdGoalAdminContext.test.tsx | 24 +++++---- .../MpdGoalAdmin/mpdGoalAdminHelpers.ts | 20 ++++--- 5 files changed, 88 insertions(+), 48 deletions(-) diff --git a/public/locales/en/translation.json b/public/locales/en/translation.json index f9c319fbbc..cfaac52622 100644 --- a/public/locales/en/translation.json +++ b/public/locales/en/translation.json @@ -183,9 +183,10 @@ "Enter a valid amount": "Enter a valid amount", "Faith and Finance": "Faith and Finance", "Family": "Family", + "IBS Cost": "IBS Cost", "Individual (1 in room)": "Individual (1 in room)", "Individual (2 in room)": "Individual (2 in room)", - "NSO & IBS Cost": "NSO & IBS Cost", + "NSO Cost": "NSO Cost", "Please enter the cost details that apply to this training. All fields are required.": "Please enter the cost details that apply to this training. All fields are required.", "Refresh Retreat": "Refresh Retreat", "Training Costs": "Training Costs", diff --git a/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.test.tsx b/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.test.tsx index fcb68fed03..4f0867be11 100644 --- a/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.test.tsx +++ b/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.test.tsx @@ -14,17 +14,19 @@ beforeEach(() => { }); const filledCosts: TrainingCosts = { - nsoIbsIndividual1InRoom: 100, - nsoIbsIndividual2InRoom: 200, - nsoIbsCouple: 300, - nsoIbsFamily: 400, - refreshRetreatSingle: 500, - refreshRetreatCouple: 600, - faithAndFinanceSingle: 700, - faithAndFinanceCouple: 800, - cruConferenceSingle: 900, - cruConferenceCouple: 1000, - cruConferenceFamily: 1100, + nsoIndividual1InRoom: 100, + nsoIndividual2InRoom: 200, + nsoCouple: 300, + nsoFamily: 400, + ibsSingle: 500, + ibsCouple: 600, + refreshRetreatSingle: 700, + refreshRetreatCouple: 800, + faithAndFinanceSingle: 900, + faithAndFinanceCouple: 1000, + cruConferenceSingle: 1100, + cruConferenceCouple: 1200, + cruConferenceFamily: 1300, }; /** @@ -39,18 +41,25 @@ const fieldsBySection: { fields: { label: string; value: number }[]; }[] = [ { - title: 'NSO & IBS Cost', + title: 'NSO Cost', fields: [ { label: 'Individual (1 in room)', - value: filledCosts.nsoIbsIndividual1InRoom, + value: filledCosts.nsoIndividual1InRoom, }, { label: 'Individual (2 in room)', - value: filledCosts.nsoIbsIndividual2InRoom, + value: filledCosts.nsoIndividual2InRoom, }, - { label: 'Couple', value: filledCosts.nsoIbsCouple }, - { label: 'Family', value: filledCosts.nsoIbsFamily }, + { label: 'Couple', value: filledCosts.nsoCouple }, + { label: 'Family', value: filledCosts.nsoFamily }, + ], + }, + { + title: 'IBS Cost', + fields: [ + { label: 'Single', value: filledCosts.ibsSingle }, + { label: 'Couple', value: filledCosts.ibsCouple }, ], }, { @@ -116,7 +125,8 @@ describe('EditTrainingCostsModal', () => { 'Please enter the cost details that apply to this training. All fields are required.', ), ).toBeInTheDocument(); - expect(getByText('NSO & IBS Cost')).toBeInTheDocument(); + expect(getByText('NSO Cost')).toBeInTheDocument(); + expect(getByText('IBS Cost')).toBeInTheDocument(); expect(getByText('Refresh Retreat')).toBeInTheDocument(); expect(getByText('Faith and Finance')).toBeInTheDocument(); expect(getByText('Cru Conference')).toBeInTheDocument(); @@ -129,10 +139,10 @@ describe('EditTrainingCostsModal', () => { ).toBeInTheDocument(); }); - it('renders all eleven cost inputs, initially blank', () => { + it('renders all thirteen cost inputs, initially blank', () => { const { getAllByRole } = render(); const inputs = getAllByRole('spinbutton'); - expect(inputs).toHaveLength(11); + expect(inputs).toHaveLength(13); inputs.forEach((input) => expect(input).toHaveValue(null)); }); @@ -208,9 +218,9 @@ describe('EditTrainingCostsModal', () => { await userEvent.click(apply); await waitFor(() => expect(onSave).toHaveBeenCalledWith(filledCosts)); - // Typing into all eleven fields exceeds the default 5s timeout when the + // Typing into all thirteen fields exceeds the default 5s timeout when the // full suite runs in parallel, so give this test extra headroom. - }, 15000); + }, 20000); it('closes via the Cancel button', async () => { const { getByRole } = render(); diff --git a/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.tsx b/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.tsx index 9c59e9133c..6d8aa15a7f 100644 --- a/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.tsx +++ b/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.tsx @@ -53,10 +53,12 @@ export interface EditTrainingCostsModalProps { } const FIELD_NAMES: FieldName[] = [ - 'nsoIbsIndividual1InRoom', - 'nsoIbsIndividual2InRoom', - 'nsoIbsCouple', - 'nsoIbsFamily', + 'nsoIndividual1InRoom', + 'nsoIndividual2InRoom', + 'nsoCouple', + 'nsoFamily', + 'ibsSingle', + 'ibsCouple', 'refreshRetreatSingle', 'refreshRetreatCouple', 'faithAndFinanceSingle', @@ -103,30 +105,47 @@ export const EditTrainingCostsModal: React.FC = ({ const sections = useMemo( () => [ { - title: t('NSO & IBS Cost'), + title: t('NSO Cost'), fields: [ { - name: 'nsoIbsIndividual1InRoom', + name: 'nsoIndividual1InRoom', label: t('Individual (1 in room)'), size: { xs: 12, sm: 6, md: 3 }, }, { - name: 'nsoIbsIndividual2InRoom', + name: 'nsoIndividual2InRoom', label: t('Individual (2 in room)'), size: { xs: 12, sm: 6, md: 3 }, }, { - name: 'nsoIbsCouple', + name: 'nsoCouple', label: t('Couple'), size: { xs: 12, sm: 6, md: 3 }, }, { - name: 'nsoIbsFamily', + name: 'nsoFamily', label: t('Family'), size: { xs: 12, sm: 6, md: 3 }, }, ], }, + // IBS is costed separately from NSO so an attendee who goes to NSO but not + // IBS can be priced without the IBS portion. + { + title: t('IBS Cost'), + fields: [ + { + name: 'ibsSingle', + label: t('Single'), + size: { xs: 12, sm: 6 }, + }, + { + name: 'ibsCouple', + label: t('Couple'), + size: { xs: 12, sm: 6 }, + }, + ], + }, { title: t('Refresh Retreat'), fields: [ diff --git a/src/components/HrTools/MpdGoalAdmin/MpdGoalAdminContext.test.tsx b/src/components/HrTools/MpdGoalAdmin/MpdGoalAdminContext.test.tsx index 99321a4658..3b0bdeb65d 100644 --- a/src/components/HrTools/MpdGoalAdmin/MpdGoalAdminContext.test.tsx +++ b/src/components/HrTools/MpdGoalAdmin/MpdGoalAdminContext.test.tsx @@ -4,17 +4,19 @@ import { mockCohorts } from './mockData'; import { MpdGoalAdminTabEnum, TrainingCosts } from './mpdGoalAdminHelpers'; const trainingCosts: TrainingCosts = { - nsoIbsIndividual1InRoom: 100, - nsoIbsIndividual2InRoom: 200, - nsoIbsCouple: 300, - nsoIbsFamily: 400, - refreshRetreatSingle: 500, - refreshRetreatCouple: 600, - faithAndFinanceSingle: 700, - faithAndFinanceCouple: 800, - cruConferenceSingle: 900, - cruConferenceCouple: 1000, - cruConferenceFamily: 1100, + nsoIndividual1InRoom: 100, + nsoIndividual2InRoom: 200, + nsoCouple: 300, + nsoFamily: 400, + ibsSingle: 500, + ibsCouple: 600, + refreshRetreatSingle: 700, + refreshRetreatCouple: 800, + faithAndFinanceSingle: 900, + faithAndFinanceCouple: 1000, + cruConferenceSingle: 1100, + cruConferenceCouple: 1200, + cruConferenceFamily: 1300, }; const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( diff --git a/src/components/HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts b/src/components/HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts index dfd042134e..e08c910402 100644 --- a/src/components/HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts +++ b/src/components/HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts @@ -25,15 +25,23 @@ export interface StaffGoalRow { /** * Per-training cost figures captured in the "Edit Training Costs" modal. Every - * value is a USD amount; the keys mirror the modal's four cost sections. All + * value is a USD amount; the keys mirror the modal's five cost sections. All * fields are required in the UI, so a saved `TrainingCosts` is fully populated. + * + * These keys mirror `NewStaffCohort::COST_FIELDS` in mpdx_api one-for-one, minus + * the columns' `_cost` suffix (which the pre-existing keys here never carried). + * NSO and IBS are separate sections so an attendee going to NSO but not IBS can + * be costed — see Caleb Cox's Figma comment 127 on node 789-32532. */ export interface TrainingCosts { - /** NSO & IBS Cost */ - nsoIbsIndividual1InRoom: number; - nsoIbsIndividual2InRoom: number; - nsoIbsCouple: number; - nsoIbsFamily: number; + /** NSO Cost */ + nsoIndividual1InRoom: number; + nsoIndividual2InRoom: number; + nsoCouple: number; + nsoFamily: number; + /** IBS Cost */ + ibsSingle: number; + ibsCouple: number; /** Refresh Retreat */ refreshRetreatSingle: number; refreshRetreatCouple: number; From ed7eef044cad97c37eacfe1f2b3c2e0041e823b7 Mon Sep 17 00:00:00 2001 From: Ethan Tison <97046400+EJTison@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:40:52 -0400 Subject: [PATCH 2/2] Assert section order and tighten cost field docs Section order is what this change is actually about, so assert the headings as an ordered list rather than checking each is merely present. Verified the assertion fails when a heading is reordered or renamed. Also drops a stale "11 cost fields" count and rewords the TrainingCosts doc so it no longer implies the frontend keys are a mechanical suffix-stripping of the columns, and notes the columns are nullable while these fields are required. Co-Authored-By: Claude Opus 5 --- .../EditTrainingCostsModal.test.tsx | 17 ++++++++++------- .../EditTrainingCostsModal.tsx | 2 +- .../HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts | 9 +++++---- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.test.tsx b/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.test.tsx index 4f0867be11..035042b1c2 100644 --- a/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.test.tsx +++ b/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.test.tsx @@ -115,8 +115,8 @@ const TestComponent: React.FC< ); describe('EditTrainingCostsModal', () => { - it('renders the cohort-specific title, subtitle and every section', () => { - const { getByText, getByRole } = render(); + it('renders the cohort-specific title, subtitle and every section in order', () => { + const { getByText, getByRole, getAllByRole } = render(); expect( getByRole('heading', { name: 'Training Costs for Fall NSO 2026' }), ).toBeInTheDocument(); @@ -125,11 +125,14 @@ describe('EditTrainingCostsModal', () => { 'Please enter the cost details that apply to this training. All fields are required.', ), ).toBeInTheDocument(); - expect(getByText('NSO Cost')).toBeInTheDocument(); - expect(getByText('IBS Cost')).toBeInTheDocument(); - expect(getByText('Refresh Retreat')).toBeInTheDocument(); - expect(getByText('Faith and Finance')).toBeInTheDocument(); - expect(getByText('Cru Conference')).toBeInTheDocument(); + // Assert the section headings as an ordered list: NSO and IBS being distinct + // sections in this order is the point of the layout, so order is asserted + // rather than mere presence. + expect( + getAllByRole('heading', { level: 3 }).map( + (heading) => heading.textContent, + ), + ).toEqual(fieldsBySection.map((section) => section.title)); }); it('falls back to a generic title when no cohort name is given', () => { diff --git a/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.tsx b/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.tsx index 6d8aa15a7f..58460136dc 100644 --- a/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.tsx +++ b/src/components/HrTools/MpdGoalAdmin/EditTrainingCostsModal/EditTrainingCostsModal.tsx @@ -23,7 +23,7 @@ import { TrainingCosts } from '../mpdGoalAdminHelpers'; type FieldName = keyof TrainingCosts; /** - * The 11 cost fields as held by Formik. Empty inputs are `''`; because these + * The cost fields as held by Formik. Empty inputs are `''`; because these * are `type="number"` inputs, Formik's `handleChange` coerces entered text to * a `number`, so a populated field is a `number`. */ diff --git a/src/components/HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts b/src/components/HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts index e08c910402..2281208ceb 100644 --- a/src/components/HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts +++ b/src/components/HrTools/MpdGoalAdmin/mpdGoalAdminHelpers.ts @@ -28,10 +28,11 @@ export interface StaffGoalRow { * value is a USD amount; the keys mirror the modal's five cost sections. All * fields are required in the UI, so a saved `TrainingCosts` is fully populated. * - * These keys mirror `NewStaffCohort::COST_FIELDS` in mpdx_api one-for-one, minus - * the columns' `_cost` suffix (which the pre-existing keys here never carried). - * NSO and IBS are separate sections so an attendee going to NSO but not IBS can - * be costed — see Caleb Cox's Figma comment 127 on node 789-32532. + * There is one key per column in `NewStaffCohort::COST_FIELDS` in mpdx_api, named + * to match. Note the columns are nullable while these are required, so wiring this + * up will need to decide how an unentered cost arrives. NSO and IBS are separate + * so an attendee going to NSO but not IBS can be costed (MPDX-9811; Figma node + * 789-32532). */ export interface TrainingCosts { /** NSO Cost */