refactor(breadcrumbs): extract callback route renderers into mapped groups

2026-04-09 14:57:47 +01:00
parent 0cd78e6f46
commit 0b905f9374
6 changed files with 159 additions and 50 deletions
@@ -214,6 +214,7 @@ const Breadcrumbs = (props) => {
        simpleLinkTextPairRenderersByPath,
        simpleMyPortalRouteRenderersByPath,
        newAppealRouteRenderersByPath,
+        callbackRouteRenderersByPath,
        caseDetailRouteRenderersByPath,
        detailAndAccountRouteRenderersByPath
    } = createMappedRouteRendererGroups({
@@ -230,6 +231,9 @@ const Breadcrumbs = (props) => {
        currentReference,
        nestedSearchString,
        caseReferenceDisplay,
+        onBack: () => {
+            router.back();
+        },
        renderAnchorCrumb,
        renderLinkCrumb,
        renderTextCrumb,
@@ -244,6 +248,7 @@ const Breadcrumbs = (props) => {
        simpleLinkTextPairRenderersByPath,
        simpleMyPortalRouteRenderersByPath,
        newAppealRouteRenderersByPath,
+        callbackRouteRenderersByPath,
        caseDetailRouteRenderersByPath,
        detailAndAccountRouteRenderersByPath
    });
@@ -308,45 +313,6 @@ const Breadcrumbs = (props) => {
                            </>
                        )}

-                        {isPath("/myportal/case/id/[incident]") && (
-                            <>
-                                {renderMyPortalCrumb()}
-                                {renderLinkCrumb(
-                                    isWelsh
-                                        ? `/${router.locale}/fymhorth/chwiliadcyfeiriadau`
-                                        : "/myportal/addresssearch",
-                                    t("common:breadcrumb-address-search")
-                                )}
-                                {renderLinkCrumb(
-                                    isWelsh
-                                        ? `/${router.locale}/fymhorth/canlyniadaucyfeiriadau?${nestedSearchString}`
-                                        : `/myportal/addresssearchresults?${nestedSearchString}`,
-                                    t(
-                                        "common:breadcrumb-address-search-results"
-                                    ),
-                                    () => {
-                                        router.back();
-                                    }
-                                )}
-                                {renderCaseReferenceCrumb(currentReference)}
-                            </>
-                        )}
-
-                        {isPath("/case") && (
-                            <>
-                                {renderLinkCrumb(
-                                    isWelsh
-                                        ? `${router.locale}/searchresults?q=${router.query.q}&page=${router.query.page}`
-                                        : `/searchresults?q=${router.query.q}&page=${router.query.page}`,
-                                    t("common:breadcrumb-search-results"),
-                                    () => {
-                                        router.back();
-                                    }
-                                )}
-                                {renderCaseReferenceCrumb(currentReference)}
-                            </>
-                        )}
-
                        {isPath("/myportal/representation") && (
                            <>
                                {currentView.representationSubmit === true &&
@@ -122,6 +122,51 @@ export const createNewAppealRouteRenderers = ({
    )
 });

+export const createCallbackRouteRenderers = ({
+    t,
+    router,
+    isWelsh,
+    nestedSearchString,
+    currentReference,
+    renderMyPortalCrumb,
+    renderLinkCrumb,
+    renderCaseReferenceCrumb,
+    onBack
+}) => ({
+    "/myportal/case/id/[incident]": () => (
+        <>
+            {renderMyPortalCrumb()}
+            {renderLinkCrumb(
+                isWelsh
+                    ? `/${router.locale}/fymhorth/chwiliadcyfeiriadau`
+                    : "/myportal/addresssearch",
+                t("common:breadcrumb-address-search")
+            )}
+            {renderLinkCrumb(
+                isWelsh
+                    ? `/${router.locale}/fymhorth/canlyniadaucyfeiriadau?${nestedSearchString}`
+                    : `/myportal/addresssearchresults?${nestedSearchString}`,
+                t("common:breadcrumb-address-search-results"),
+                onBack
+            )}
+            {renderCaseReferenceCrumb(currentReference)}
+        </>
+    ),
+
+    "/case": () => (
+        <>
+            {renderLinkCrumb(
+                isWelsh
+                    ? `${router.locale}/searchresults?q=${router.query.q}&page=${router.query.page}`
+                    : `/searchresults?q=${router.query.q}&page=${router.query.page}`,
+                t("common:breadcrumb-search-results"),
+                onBack
+            )}
+            {renderCaseReferenceCrumb(currentReference)}
+        </>
+    )
+});
+
 export const createCaseDetailRouteRenderers = ({
    t,
    router,
@@ -372,6 +417,7 @@ export const createMappedRouteRendererGroups = ({
    currentReference,
    nestedSearchString,
    caseReferenceDisplay,
+    onBack,
    renderAnchorCrumb,
    renderLinkCrumb,
    renderTextCrumb,
@@ -418,6 +464,18 @@ export const createMappedRouteRendererGroups = ({
        renderTextCrumb
    });

+    const callbackRouteRenderersByPath = createCallbackRouteRenderers({
+        t,
+        router,
+        isWelsh,
+        nestedSearchString,
+        currentReference,
+        renderMyPortalCrumb,
+        renderLinkCrumb,
+        renderCaseReferenceCrumb,
+        onBack
+    });
+
    const caseDetailRouteRenderersByPath = createCaseDetailRouteRenderers({
        t,
        router,
@@ -453,6 +511,7 @@ export const createMappedRouteRendererGroups = ({
        simpleLinkTextPairRenderersByPath,
        simpleMyPortalRouteRenderersByPath,
        newAppealRouteRenderersByPath,
+        callbackRouteRenderersByPath,
        caseDetailRouteRenderersByPath,
        detailAndAccountRouteRenderersByPath
    };
@@ -23,6 +23,7 @@ const buildBreadcrumbRendererMaps = ({
    simpleLinkTextPairRenderersByPath,
    simpleMyPortalRouteRenderersByPath,
    newAppealRouteRenderersByPath,
+    callbackRouteRenderersByPath,
    caseDetailRouteRenderersByPath,
    detailAndAccountRouteRenderersByPath
 }) => [
@@ -30,6 +31,7 @@ const buildBreadcrumbRendererMaps = ({
    simpleLinkTextPairRenderersByPath,
    simpleMyPortalRouteRenderersByPath,
    newAppealRouteRenderersByPath,
+    callbackRouteRenderersByPath,
    caseDetailRouteRenderersByPath,
    detailAndAccountRouteRenderersByPath
 ];
@@ -668,6 +668,47 @@ Follow-ups:

 - Next larger slice: extract remaining callback-bearing deterministic branches into mapped route groups with callback injection, then update structure guards accordingly.

+### CL-22541-V: breadcrumbs larger slice — callback route-group extraction (`/myportal/case/id/[incident]`, `/case`)
+
+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 callback-bearing breadcrumb branches into a dedicated callback renderer map while preserving `router.back()` behavior through explicit callback injection.
+impact: Structural refactor with preserved callback semantics; no intended route/auth/session/API/EN-CY/a11y behavior change.
+status: completed
+
+Summary:
+
+- Added `createCallbackRouteRenderers(...)` in `lib/routing/breadcrumbRendererFactories.js` to map:
+    - `/myportal/case/id/[incident]`
+    - `/case`
+- Introduced explicit callback injection (`onBack`) into grouped factory composition and route renderers, preserving `router.back()` behavior via injected callback.
+- Extended `createMappedRouteRendererGroups(...)` return with `callbackRouteRenderersByPath`.
+- Updated map builder in `lib/routing/breadcrumbRouteMaps.js` to include callback map in explicit precedence order:
+    - simple -> link-text -> myportal -> new-appeal -> callback -> case detail -> detail/account.
+- Updated `components/breadcrumbs.js`:
+    - pass `onBack: () => { router.back(); }` into grouped factory composition
+    - include callback route map in `buildBreadcrumbRendererMaps(...)`
+    - remove now-redundant inline `isPath("/myportal/case/id/[incident]")` and `isPath("/case")` branches.
+- Expanded tests:
+    - `tests/phase22/breadcrumb-route-maps-helper.test.cjs`
+        - update grouped map order assertions to seven maps including callback group.
+    - `tests/phase22/breadcrumbs-route-map-structure.test.cjs`
+        - assert callback factory export and mapped callback route presence
+        - assert callback map inclusion in component grouped destructuring and map-order invariant
+        - assert inline callback branches are removed while `router.back();` remains preserved.
+
+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 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-00X: 22500 `components/elements/index.js` Phase 1 helper extraction

 date: 2026-04-07  
@@ -60,6 +60,9 @@ test("buildBreadcrumbRendererMaps returns grouped maps in explicit precedence or
    const newAppealRouteRenderersByPath = {
        "/eta": () => "eta"
    };
+    const callbackRouteRenderersByPath = {
+        "/theta": () => "theta"
+    };
    const caseDetailRouteRenderersByPath = {
        "/delta": () => "delta"
    };
@@ -72,12 +75,13 @@ test("buildBreadcrumbRendererMaps returns grouped maps in explicit precedence or
        simpleLinkTextPairRenderersByPath,
        simpleMyPortalRouteRenderersByPath,
        newAppealRouteRenderersByPath,
+        callbackRouteRenderersByPath,
        caseDetailRouteRenderersByPath,
        detailAndAccountRouteRenderersByPath
    });

    assert.ok(Array.isArray(result), "Expected grouped maps array");
-    assert.strictEqual(result.length, 6, "Expected six grouped route maps");
+    assert.strictEqual(result.length, 7, "Expected seven grouped route maps");
    assert.strictEqual(
        result[0],
        simpleRouteRenderersByPath,
@@ -100,13 +104,18 @@ test("buildBreadcrumbRendererMaps returns grouped maps in explicit precedence or
    );
    assert.strictEqual(
        result[4],
-        caseDetailRouteRenderersByPath,
-        "Expected case detail route renderers fifth"
+        callbackRouteRenderersByPath,
+        "Expected callback route renderers fifth"
    );
    assert.strictEqual(
        result[5],
+        caseDetailRouteRenderersByPath,
+        "Expected case detail route renderers sixth"
+    );
+    assert.strictEqual(
+        result[6],
        detailAndAccountRouteRenderersByPath,
-        "Expected detail and account route renderers sixth"
+        "Expected detail and account route renderers seventh"
    );
 });

@@ -126,6 +135,7 @@ test("resolveMappedRouteRenderer returns renderer from first matching grouped ma
        },
        simpleMyPortalRouteRenderersByPath: {},
        newAppealRouteRenderersByPath: {},
+        callbackRouteRenderersByPath: {},
        caseDetailRouteRenderersByPath: {},
        detailAndAccountRouteRenderersByPath: {}
    });
@@ -153,6 +163,7 @@ test("resolveMappedRouteRenderer returns renderer from later grouped map when ea
        },
        simpleMyPortalRouteRenderersByPath: {},
        caseDetailRouteRenderersByPath: {},
+        callbackRouteRenderersByPath: {},
        detailAndAccountRouteRenderersByPath: {
            "/account/personaldetails": detailRenderer
        }
@@ -178,6 +189,7 @@ test("resolveMappedRouteRenderer returns null for unmapped path", () => {
        simpleLinkTextPairRenderersByPath: {},
        simpleMyPortalRouteRenderersByPath: {},
        newAppealRouteRenderersByPath: {},
+        callbackRouteRenderersByPath: {},
        caseDetailRouteRenderersByPath: {},
        detailAndAccountRouteRenderersByPath: {}
    });
@@ -206,6 +218,7 @@ test("resolveMappedRouteRenderer skips invalid map entries and non-function rend
            "/known": validRenderer
        },
        newAppealRouteRenderersByPath: undefined,
+        callbackRouteRenderersByPath: {},
        caseDetailRouteRenderersByPath: {},
        detailAndAccountRouteRenderersByPath: {}
    });
@@ -42,6 +42,12 @@ test("breadcrumbs/factory module includes expected mapped route definitions", as
        "Expected new-appeal route renderer factory export"
    );

+    assert.strictEqual(
+        source.includes("export const createCallbackRouteRenderers ="),
+        true,
+        "Expected callback route renderer factory export"
+    );
+
    assert.strictEqual(
        source.includes("export const createCaseDetailRouteRenderers ="),
        true,
@@ -96,6 +102,18 @@ test("breadcrumbs/factory module includes expected mapped route definitions", as
        "Expected /newappeal/selectappeal to be mapped in createNewAppealRouteRenderers"
    );

+    assert.strictEqual(
+        source.includes('"/myportal/case/id/[incident]": () => ('),
+        true,
+        "Expected /myportal/case/id/[incident] to be mapped in createCallbackRouteRenderers"
+    );
+
+    assert.strictEqual(
+        source.includes('"/case": () => ('),
+        true,
+        "Expected /case to be mapped in createCallbackRouteRenderers"
+    );
+
    assert.strictEqual(
        source.includes('"/case/[ticketnumber]": () =>'),
        true,
@@ -185,7 +203,7 @@ test("breadcrumbs/component composes mapped routes via imported factories and sh
    );

    assert.strictEqual(
-        /const\s+\{[\s\S]*simpleRouteRenderersByPath[\s\S]*newAppealRouteRenderersByPath[\s\S]*detailAndAccountRouteRenderersByPath[\s\S]*\}\s*=\s*createMappedRouteRendererGroups\s*\(/.test(
+        /const\s+\{[\s\S]*simpleRouteRenderersByPath[\s\S]*newAppealRouteRenderersByPath[\s\S]*callbackRouteRenderersByPath[\s\S]*detailAndAccountRouteRenderersByPath[\s\S]*\}\s*=\s*createMappedRouteRendererGroups\s*\(/.test(
            source
        ),
        true,
@@ -351,6 +369,9 @@ test("breadcrumbs/map builder preserves route map ordering", async () => {
    const newAppealOrderIndex = mapBuilderSlice.indexOf(
        "newAppealRouteRenderersByPath"
    );
+    const callbackOrderIndex = mapBuilderSlice.indexOf(
+        "callbackRouteRenderersByPath"
+    );
    const caseDetailOrderIndex = mapBuilderSlice.indexOf(
        "caseDetailRouteRenderersByPath"
    );
@@ -363,26 +384,33 @@ test("breadcrumbs/map builder preserves route map ordering", async () => {
            simpleLinkTextPairOrderIndex > simpleRouteOrderIndex &&
            simpleMyPortalOrderIndex > simpleLinkTextPairOrderIndex &&
            newAppealOrderIndex > simpleMyPortalOrderIndex &&
-            caseDetailOrderIndex > newAppealOrderIndex &&
+            callbackOrderIndex > newAppealOrderIndex &&
+            caseDetailOrderIndex > callbackOrderIndex &&
            detailAndAccountOrderIndex > caseDetailOrderIndex,
        true,
-        "Expected map builder call to preserve explicit route-map ordering (simple -> link-text -> myportal -> new-appeal -> case detail -> detail/account)"
+        "Expected map builder call to preserve explicit route-map ordering (simple -> link-text -> myportal -> new-appeal -> callback -> case detail -> detail/account)"
    );
 });

-test("breadcrumbs/dynamic callback branch remains explicit for myportal case incident route", async () => {
+test("breadcrumbs/callback-mapped branches preserve router.back behavior", async () => {
    const source = loadBreadcrumbSource();

    assert.strictEqual(
        source.includes('isPath("/myportal/case/id/[incident]")'),
-        true,
-        "Expected dynamic /myportal/case/id/[incident] breadcrumb branch to remain explicit"
+        false,
+        "Expected /myportal/case/id/[incident] explicit branch to be removed after callback mapping"
+    );
+
+    assert.strictEqual(
+        source.includes('isPath("/case")'),
+        false,
+        "Expected /case explicit branch to be removed after callback mapping"
    );

    assert.strictEqual(
        source.includes("router.back();"),
        true,
-        "Expected callback-driven router.back() behavior to remain explicit"
+        "Expected callback-driven router.back() behavior to remain preserved"
    );
 });