From cebd47bf1ce006b5c912cb80379e5f319f8d2e72 Mon Sep 17 00:00:00 2001 From: robbond Date: Thu, 9 Apr 2026 15:03:19 +0100 Subject: [PATCH] refactor(breadcrumbs): map step-back route branches into renderer group --- components/breadcrumbs.js | 51 ++++-------------- lib/routing/breadcrumbRendererFactories.js | 52 ++++++++++++++++++- lib/routing/breadcrumbRouteMaps.js | 2 + memory-bank/change-log.md | 45 ++++++++++++++++ .../breadcrumb-route-maps-helper.test.cjs | 21 ++++++-- .../breadcrumbs-route-map-structure.test.cjs | 40 ++++++++++++-- 6 files changed, 163 insertions(+), 48 deletions(-) diff --git a/components/breadcrumbs.js b/components/breadcrumbs.js index 379109a7..01c045be 100644 --- a/components/breadcrumbs.js +++ b/components/breadcrumbs.js @@ -215,6 +215,7 @@ const Breadcrumbs = (props) => { simpleMyPortalRouteRenderersByPath, newAppealRouteRenderersByPath, callbackRouteRenderersByPath, + stepBackRouteRenderersByPath, caseDetailRouteRenderersByPath, detailAndAccountRouteRenderersByPath } = createMappedRouteRendererGroups({ @@ -234,13 +235,21 @@ const Breadcrumbs = (props) => { onBack: () => { router.back(); }, + currentSection: appealType.currentSection, + onStepBack: () => { + setCurrentSection(appealType.currentSection - 1); + }, + onStepBackWithInlineClass: () => { + setCurrentSection(appealType.currentSection - 1); + }, renderAnchorCrumb, renderLinkCrumb, renderTextCrumb, renderCaseReferenceCrumb, renderMyPortalCrumb, renderMyPortalSectionCrumbs, - renderDnsCaseReferenceCrumbs + renderDnsCaseReferenceCrumbs, + renderBackCrumb }); const mappedRouteRendererMaps = buildBreadcrumbRendererMaps({ @@ -249,6 +258,7 @@ const Breadcrumbs = (props) => { simpleMyPortalRouteRenderersByPath, newAppealRouteRenderersByPath, callbackRouteRenderersByPath, + stepBackRouteRenderersByPath, caseDetailRouteRenderersByPath, detailAndAccountRouteRenderersByPath }); @@ -274,45 +284,6 @@ const Breadcrumbs = (props) => { {isPath("/newappeal") && <>} - {isPath("/newappeal/[appealtypes]") && ( - <> - {appealType.currentSection > 1 && - (appealType.currentSection === 9999 - ? renderLinkCrumb( - "/", - t( - "common:service-name-breadcrumb" - ) - ) - : renderBackCrumb( - () => - setCurrentSection( - appealType.currentSection - - 1 - ), - "govuk-breadcrumbs__link-item" - ))} - - )} - - {isPath("/myportal/[appealtypes]") && ( - <> - {appealType.currentSection > 1 && - (appealType.currentSection === 9999 - ? renderLinkCrumb( - "/", - t( - "common:service-name-breadcrumb" - ) - ) - : renderBackCrumb(() => - setCurrentSection( - appealType.currentSection - 1 - ) - ))} - - )} - {isPath("/myportal/representation") && ( <> {currentView.representationSubmit === true && diff --git a/lib/routing/breadcrumbRendererFactories.js b/lib/routing/breadcrumbRendererFactories.js index 40048dfc..ae8d6c40 100644 --- a/lib/routing/breadcrumbRendererFactories.js +++ b/lib/routing/breadcrumbRendererFactories.js @@ -167,6 +167,42 @@ export const createCallbackRouteRenderers = ({ ) }); +export const createStepBackRouteRenderers = ({ + t, + currentSection, + onStepBack, + onStepBackWithInlineClass, + renderLinkCrumb, + renderBackCrumb +}) => ({ + "/newappeal/[appealtypes]": () => { + if (currentSection <= 1) { + return null; + } + + if (currentSection === 9999) { + return renderLinkCrumb("/", t("common:service-name-breadcrumb")); + } + + return renderBackCrumb( + onStepBackWithInlineClass, + "govuk-breadcrumbs__link-item" + ); + }, + + "/myportal/[appealtypes]": () => { + if (currentSection <= 1) { + return null; + } + + if (currentSection === 9999) { + return renderLinkCrumb("/", t("common:service-name-breadcrumb")); + } + + return renderBackCrumb(onStepBack); + } +}); + export const createCaseDetailRouteRenderers = ({ t, router, @@ -418,13 +454,17 @@ export const createMappedRouteRendererGroups = ({ nestedSearchString, caseReferenceDisplay, onBack, + currentSection, + onStepBack, + onStepBackWithInlineClass, renderAnchorCrumb, renderLinkCrumb, renderTextCrumb, renderCaseReferenceCrumb, renderMyPortalCrumb, renderMyPortalSectionCrumbs, - renderDnsCaseReferenceCrumbs + renderDnsCaseReferenceCrumbs, + renderBackCrumb }) => { const simpleRouteRenderersByPath = createSimpleRouteRenderers({ t, @@ -476,6 +516,15 @@ export const createMappedRouteRendererGroups = ({ onBack }); + const stepBackRouteRenderersByPath = createStepBackRouteRenderers({ + t, + currentSection, + onStepBack, + onStepBackWithInlineClass, + renderLinkCrumb, + renderBackCrumb + }); + const caseDetailRouteRenderersByPath = createCaseDetailRouteRenderers({ t, router, @@ -512,6 +561,7 @@ export const createMappedRouteRendererGroups = ({ simpleMyPortalRouteRenderersByPath, newAppealRouteRenderersByPath, callbackRouteRenderersByPath, + stepBackRouteRenderersByPath, caseDetailRouteRenderersByPath, detailAndAccountRouteRenderersByPath }; diff --git a/lib/routing/breadcrumbRouteMaps.js b/lib/routing/breadcrumbRouteMaps.js index 977da52e..5666de00 100644 --- a/lib/routing/breadcrumbRouteMaps.js +++ b/lib/routing/breadcrumbRouteMaps.js @@ -24,6 +24,7 @@ const buildBreadcrumbRendererMaps = ({ simpleMyPortalRouteRenderersByPath, newAppealRouteRenderersByPath, callbackRouteRenderersByPath, + stepBackRouteRenderersByPath, caseDetailRouteRenderersByPath, detailAndAccountRouteRenderersByPath }) => [ @@ -32,6 +33,7 @@ const buildBreadcrumbRendererMaps = ({ simpleMyPortalRouteRenderersByPath, newAppealRouteRenderersByPath, callbackRouteRenderersByPath, + stepBackRouteRenderersByPath, caseDetailRouteRenderersByPath, detailAndAccountRouteRenderersByPath ]; diff --git a/memory-bank/change-log.md b/memory-bank/change-log.md index dfd38bcc..124e7eec 100644 --- a/memory-bank/change-log.md +++ b/memory-bank/change-log.md @@ -709,6 +709,51 @@ Follow-ups: - Next smaller slice candidate: add a focused helper test for empty-string/whitespace path lookups to assert strict null behavior for non-exact keys. - Next larger slice candidate: evaluate whether `/newappeal/[appealtypes]` and `/myportal/[appealtypes]` back-link branches can be extracted with explicit callback/setter injection while preserving state-step semantics. +### CL-22541-W: breadcrumbs larger slice — step-back route-group extraction (`/newappeal/[appealtypes]`, `/myportal/[appealtypes]`) + +date: 2026-04-09 +author: Cline +scope: `lib/routing/{breadcrumbRendererFactories,breadcrumbRouteMaps}.js`, `components/breadcrumbs.js`, `tests/phase22/{breadcrumb-route-maps-helper,breadcrumbs-route-map-structure}.test.cjs` +type: change +rationale: Execute the next larger extraction slice by moving step-based back-link branches into a dedicated mapped route group with explicit setter callback injection, preserving section-navigation semantics. +impact: Structural refactor with preserved back-link and step-state behavior; no intended route/auth/session/API/EN-CY/a11y behavior change. +status: completed + +Summary: + +- Added `createStepBackRouteRenderers(...)` in `lib/routing/breadcrumbRendererFactories.js` to map: + - `/newappeal/[appealtypes]` + - `/myportal/[appealtypes]` +- Preserved existing step semantics in mapped handlers: + - no crumb when `currentSection <= 1` + - service-name crumb when `currentSection === 9999` + - back-link crumb with existing class parity (`govuk-breadcrumbs__link-item` for new-appeal flow) +- Introduced explicit injected handlers (`onStepBack`, `onStepBackWithInlineClass`) and injected crumb renderer dependency (`renderBackCrumb`) into grouped factory composition. +- Extended map builder in `lib/routing/breadcrumbRouteMaps.js` with `stepBackRouteRenderersByPath` and updated precedence order: + - simple -> link-text -> myportal -> new-appeal -> callback -> step-back -> case detail -> detail/account. +- Updated `components/breadcrumbs.js`: + - pass `currentSection` and step-back callbacks into `createMappedRouteRendererGroups(...)` + - include step-back map in `buildBreadcrumbRendererMaps(...)` + - remove inline `isPath("/newappeal/[appealtypes]")` and `isPath("/myportal/[appealtypes]")` branches. +- Expanded tests: + - `tests/phase22/breadcrumb-route-maps-helper.test.cjs` + - updated grouped map count/order to include step-back map (8 total) + - `tests/phase22/breadcrumbs-route-map-structure.test.cjs` + - assert step-back factory export + mapped route presence + - assert grouped destructuring includes step-back map + - assert inline step-back branches are removed + - assert updated map-order invariant includes step-back group. + +Validation: + +- `npx eslint lib/routing/breadcrumbRendererFactories.js lib/routing/breadcrumbRouteMaps.js components/breadcrumbs.js tests/phase22/breadcrumb-route-maps-helper.test.cjs tests/phase22/breadcrumbs-route-map-structure.test.cjs tests/phase22/index.test.cjs` -> pass. +- `node tests/phase22/index.test.cjs` -> pass (combined suite). + +Follow-ups: + +- Next smaller slice candidate: add focused helper test for strict null behavior on empty-string/whitespace/non-exact path keys. +- Next larger slice candidate: evaluate extractability of `/myportal/representation` back-link states via explicit callback injection, only if questionnaire/submit side-effects remain parity-safe and readable. + ### CL-00X: 22500 `components/elements/index.js` Phase 1 helper extraction date: 2026-04-07 diff --git a/tests/phase22/breadcrumb-route-maps-helper.test.cjs b/tests/phase22/breadcrumb-route-maps-helper.test.cjs index 57d7187c..3dec5147 100644 --- a/tests/phase22/breadcrumb-route-maps-helper.test.cjs +++ b/tests/phase22/breadcrumb-route-maps-helper.test.cjs @@ -63,6 +63,9 @@ test("buildBreadcrumbRendererMaps returns grouped maps in explicit precedence or const callbackRouteRenderersByPath = { "/theta": () => "theta" }; + const stepBackRouteRenderersByPath = { + "/iota": () => "iota" + }; const caseDetailRouteRenderersByPath = { "/delta": () => "delta" }; @@ -76,12 +79,13 @@ test("buildBreadcrumbRendererMaps returns grouped maps in explicit precedence or simpleMyPortalRouteRenderersByPath, newAppealRouteRenderersByPath, callbackRouteRenderersByPath, + stepBackRouteRenderersByPath, caseDetailRouteRenderersByPath, detailAndAccountRouteRenderersByPath }); assert.ok(Array.isArray(result), "Expected grouped maps array"); - assert.strictEqual(result.length, 7, "Expected seven grouped route maps"); + assert.strictEqual(result.length, 8, "Expected eight grouped route maps"); assert.strictEqual( result[0], simpleRouteRenderersByPath, @@ -109,13 +113,18 @@ test("buildBreadcrumbRendererMaps returns grouped maps in explicit precedence or ); assert.strictEqual( result[5], - caseDetailRouteRenderersByPath, - "Expected case detail route renderers sixth" + stepBackRouteRenderersByPath, + "Expected step-back route renderers sixth" ); assert.strictEqual( result[6], + caseDetailRouteRenderersByPath, + "Expected case detail route renderers seventh" + ); + assert.strictEqual( + result[7], detailAndAccountRouteRenderersByPath, - "Expected detail and account route renderers seventh" + "Expected detail and account route renderers eighth" ); }); @@ -136,6 +145,7 @@ test("resolveMappedRouteRenderer returns renderer from first matching grouped ma simpleMyPortalRouteRenderersByPath: {}, newAppealRouteRenderersByPath: {}, callbackRouteRenderersByPath: {}, + stepBackRouteRenderersByPath: {}, caseDetailRouteRenderersByPath: {}, detailAndAccountRouteRenderersByPath: {} }); @@ -164,6 +174,7 @@ test("resolveMappedRouteRenderer returns renderer from later grouped map when ea simpleMyPortalRouteRenderersByPath: {}, caseDetailRouteRenderersByPath: {}, callbackRouteRenderersByPath: {}, + stepBackRouteRenderersByPath: {}, detailAndAccountRouteRenderersByPath: { "/account/personaldetails": detailRenderer } @@ -190,6 +201,7 @@ test("resolveMappedRouteRenderer returns null for unmapped path", () => { simpleMyPortalRouteRenderersByPath: {}, newAppealRouteRenderersByPath: {}, callbackRouteRenderersByPath: {}, + stepBackRouteRenderersByPath: {}, caseDetailRouteRenderersByPath: {}, detailAndAccountRouteRenderersByPath: {} }); @@ -219,6 +231,7 @@ test("resolveMappedRouteRenderer skips invalid map entries and non-function rend }, newAppealRouteRenderersByPath: undefined, callbackRouteRenderersByPath: {}, + stepBackRouteRenderersByPath: {}, caseDetailRouteRenderersByPath: {}, detailAndAccountRouteRenderersByPath: {} }); diff --git a/tests/phase22/breadcrumbs-route-map-structure.test.cjs b/tests/phase22/breadcrumbs-route-map-structure.test.cjs index 8834073e..429a01e4 100644 --- a/tests/phase22/breadcrumbs-route-map-structure.test.cjs +++ b/tests/phase22/breadcrumbs-route-map-structure.test.cjs @@ -48,6 +48,12 @@ test("breadcrumbs/factory module includes expected mapped route definitions", as "Expected callback route renderer factory export" ); + assert.strictEqual( + source.includes("export const createStepBackRouteRenderers ="), + true, + "Expected step-back route renderer factory export" + ); + assert.strictEqual( source.includes("export const createCaseDetailRouteRenderers ="), true, @@ -114,6 +120,18 @@ test("breadcrumbs/factory module includes expected mapped route definitions", as "Expected /case to be mapped in createCallbackRouteRenderers" ); + assert.strictEqual( + source.includes('"/newappeal/[appealtypes]": () => {'), + true, + "Expected /newappeal/[appealtypes] to be mapped in createStepBackRouteRenderers" + ); + + assert.strictEqual( + source.includes('"/myportal/[appealtypes]": () => {'), + true, + "Expected /myportal/[appealtypes] to be mapped in createStepBackRouteRenderers" + ); + assert.strictEqual( source.includes('"/case/[ticketnumber]": () =>'), true, @@ -203,7 +221,7 @@ test("breadcrumbs/component composes mapped routes via imported factories and sh ); assert.strictEqual( - /const\s+\{[\s\S]*simpleRouteRenderersByPath[\s\S]*newAppealRouteRenderersByPath[\s\S]*callbackRouteRenderersByPath[\s\S]*detailAndAccountRouteRenderersByPath[\s\S]*\}\s*=\s*createMappedRouteRendererGroups\s*\(/.test( + /const\s+\{[\s\S]*simpleRouteRenderersByPath[\s\S]*newAppealRouteRenderersByPath[\s\S]*callbackRouteRenderersByPath[\s\S]*stepBackRouteRenderersByPath[\s\S]*detailAndAccountRouteRenderersByPath[\s\S]*\}\s*=\s*createMappedRouteRendererGroups\s*\(/.test( source ), true, @@ -290,6 +308,18 @@ test("breadcrumbs/component composes mapped routes via imported factories and sh "Expected /newappeal/selectappeal explicit branch to be removed after mapping" ); + assert.strictEqual( + source.includes('isPath("/newappeal/[appealtypes]")'), + false, + "Expected /newappeal/[appealtypes] explicit branch to be removed after step-back mapping" + ); + + assert.strictEqual( + source.includes('isPath("/myportal/[appealtypes]")'), + false, + "Expected /myportal/[appealtypes] explicit branch to be removed after step-back mapping" + ); + assert.strictEqual( source.includes('isPath("/myportal/case/[ticketnumber]")'), false, @@ -372,6 +402,9 @@ test("breadcrumbs/map builder preserves route map ordering", async () => { const callbackOrderIndex = mapBuilderSlice.indexOf( "callbackRouteRenderersByPath" ); + const stepBackOrderIndex = mapBuilderSlice.indexOf( + "stepBackRouteRenderersByPath" + ); const caseDetailOrderIndex = mapBuilderSlice.indexOf( "caseDetailRouteRenderersByPath" ); @@ -385,10 +418,11 @@ test("breadcrumbs/map builder preserves route map ordering", async () => { simpleMyPortalOrderIndex > simpleLinkTextPairOrderIndex && newAppealOrderIndex > simpleMyPortalOrderIndex && callbackOrderIndex > newAppealOrderIndex && - caseDetailOrderIndex > callbackOrderIndex && + stepBackOrderIndex > callbackOrderIndex && + caseDetailOrderIndex > stepBackOrderIndex && detailAndAccountOrderIndex > caseDetailOrderIndex, true, - "Expected map builder call to preserve explicit route-map ordering (simple -> link-text -> myportal -> new-appeal -> callback -> case detail -> detail/account)" + "Expected map builder call to preserve explicit route-map ordering (simple -> link-text -> myportal -> new-appeal -> callback -> step-back -> case detail -> detail/account)" ); });