diff --git a/components/breadcrumbs.js b/components/breadcrumbs.js index 94dffcc5..8cad3c16 100644 --- a/components/breadcrumbs.js +++ b/components/breadcrumbs.js @@ -12,6 +12,10 @@ import { setRepresentationCapacity, setRepresentationSubmit } from "../store/currentView/action"; +import { + normalizeRouteStateQuery, + resolveSearchResultsHref +} from "../lib/routing/routeState"; const Breadcrumbs = (props) => { const { currentView, @@ -69,9 +73,8 @@ const Breadcrumbs = (props) => { ? 9 : ""; - const { va, adv, ads, key } = router.query; - const isAdv = adv === "true"; - const isAds = ads === "true"; + const routeState = normalizeRouteStateQuery(router.query); + const { viewAll, advanced, address, key } = routeState; const hideServiceNamePaths = [ "/myportal/[appealtypes]", @@ -95,55 +98,33 @@ const Breadcrumbs = (props) => { return labelsByKey[viewKey] || null; }; - const breadcrumbHref = (() => { - if (va === "true") { - return { - pathname: "/myportal/viewall", - query: { key } - }; - } + const breadcrumbHref = resolveSearchResultsHref({ + query: router.query, + hasSession: Boolean(session), + includeViewAll: true, + fallbackToMyPortalWhenNoFlags: true + }); - const base = session ? "/myportal" : ""; - - if (!va && !adv && !ads) { - return { - pathname: "/myportal" - }; - } - - if (adv === "true") { - return { - pathname: `${base}/advancedsearchresults`, - query: router.query - }; - } - - if (ads === "true") { - return { - pathname: `${base}/addresssearchresults`, - query: router.query - }; - } - - return { - pathname: `${base}/searchresults`, - query: router.query - }; - })(); + const caseResultsHref = resolveSearchResultsHref({ + query: router.query, + hasSession: Boolean(session), + includeViewAll: false, + fallbackToMyPortalWhenNoFlags: false + }); const breadcrumbLabel = (() => { - if (va === "true") { + if (viewAll) { return getViewAllLabel(key); } const keyedLabel = getViewAllLabel(key); if (keyedLabel) return keyedLabel; - if (isAdv) { + if (advanced) { return t("common:breadcrumb-advanced-search-results"); } - if (isAds) { + if (address) { return t("common:breadcrumb-address-search-results"); } @@ -518,16 +499,7 @@ const Breadcrumbs = (props) => { <>
  • {breadcrumbLabel} diff --git a/components/case/summary.js b/components/case/summary.js index 9958de55..cbbe4b7c 100644 --- a/components/case/summary.js +++ b/components/case/summary.js @@ -45,6 +45,7 @@ import { showRepsEnded, getBilingualText } from "./summary/utils/helpers"; +import { resolveSearchResultsHref } from "../../lib/routing/routeState"; import CaseNoticeBanner from "./caseNoticeBanner"; import WatchModal from "./watchmodal"; @@ -324,52 +325,19 @@ const CaseSummary = (props) => { const zoom = isNaN(parseFloat(siteCoords.latitude)) ? 7 : 12; - const buildSearchHref = (router, session) => { - const { va, adv, ads, key } = router.query; + const isDnsRoute = + router.pathname === "/dns" || + router.pathname.startsWith("/dns/") || + router.pathname === "/myportal/dns" || + router.pathname.startsWith("/myportal/dns/"); - const isDnsRoute = - router.pathname === "/dns" || - router.pathname.startsWith("/dns/") || - router.pathname === "/myportal/dns" || - router.pathname.startsWith("/myportal/dns/"); - - const base = session ? "/myportal" : ""; - - if (!va && !adv && !ads) { - return { - pathname: "/myportal" - }; - } - - if (isDnsRoute) { - return { - pathname: `${base}/dnsapplications` - }; - } - - if (va === "true") { - return { - pathname: "/myportal/viewall", - query: { key } - }; - } - - if (adv === "true") { - return { - pathname: `${base}/advancedsearchresults`, - query: router.query - }; - } - - if (ads === "true") { - return { - pathname: `${base}/addresssearchresults`, - query: router.query - }; - } - - return { pathname: `${base}/searchresults`, query: router.query }; - }; + const searchHref = resolveSearchResultsHref({ + query: router.query, + hasSession: Boolean(session), + includeViewAll: true, + fallbackToMyPortalWhenNoFlags: true, + isDnsRoute + }); let showDetailsBlock = showDetails == true ? ( @@ -1043,10 +1011,7 @@ const CaseSummary = (props) => { {" "} {t( diff --git a/lib/routing/routeState.js b/lib/routing/routeState.js new file mode 100644 index 00000000..3784751d --- /dev/null +++ b/lib/routing/routeState.js @@ -0,0 +1,62 @@ +export const normalizeRouteStateQuery = (query = {}) => { + const viewAll = query?.va === "true"; + const advanced = query?.adv === "true"; + const address = query?.ads === "true"; + + return { + viewAll, + advanced, + address, + key: query?.key + }; +}; + +export const resolveSearchResultsHref = ({ + query = {}, + hasSession = false, + includeViewAll = true, + fallbackToMyPortalWhenNoFlags = false, + isDnsRoute = false +} = {}) => { + const routeState = normalizeRouteStateQuery(query); + const { viewAll, advanced, address, key } = routeState; + const base = hasSession ? "/myportal" : ""; + + if (fallbackToMyPortalWhenNoFlags && !viewAll && !advanced && !address) { + return { + pathname: "/myportal" + }; + } + + if (isDnsRoute) { + return { + pathname: `${base}/dnsapplications` + }; + } + + if (includeViewAll && viewAll) { + return { + pathname: "/myportal/viewall", + query: { key } + }; + } + + if (advanced) { + return { + pathname: `${base}/advancedsearchresults`, + query + }; + } + + if (address) { + return { + pathname: `${base}/addresssearchresults`, + query + }; + } + + return { + pathname: `${base}/searchresults`, + query + }; +}; diff --git a/memory-bank/change-log.md b/memory-bank/change-log.md index 28f12758..ee5ba512 100644 --- a/memory-bank/change-log.md +++ b/memory-bank/change-log.md @@ -18,6 +18,40 @@ Follow-ups: --- +### CL-22541: breadcrumb/back-link route-state helper extraction (`va/adv/ads/key`) + +date: 2026-04-09 +author: Cline +scope: `lib/routing/routeState.js`, `components/breadcrumbs.js`, `components/case/summary.js`, `tests/phase22/{route-state-helper.test.cjs,index.test.cjs}` +type: change +rationale: Action Priority 3 refactor backlog item by extracting duplicated route-state decision logic (`va/adv/ads/key`) into a shared pure helper and reusing it from breadcrumb and case-summary back-link flows to reduce drift/regression risk. +impact: Refactor-only decision-layer consolidation for search/case/myportal navigation; no auth/session/CSP/API contract changes; EN/CY behavior intended unchanged as helper only resolves route targets and preserves existing query forwarding behavior. +status: completed + +Summary: + +- Created branch from `origin/SIPS-Development`: `22541-breadcrumb-route-state-helper`. +- Added `lib/routing/routeState.js` with: + - `normalizeRouteStateQuery(query)` + - `resolveSearchResultsHref(options)` +- Updated `components/breadcrumbs.js` to consume the shared helper for: + - breadcrumb href resolution (`/myportal/viewall`, advanced/address/default results) + - case-results breadcrumb target on `/case/[ticketnumber]` + - preserving existing label selection behavior via normalized flags. +- Updated `components/case/summary.js` to replace local `buildSearchHref` with shared helper output (`searchHref`), including DNS-route override and no-flag fallback behavior. +- Added table-driven unit coverage in `tests/phase22/route-state-helper.test.cjs` and registered it in `tests/phase22/index.test.cjs`. + +Validation: + +- `npx eslint lib/routing/routeState.js components/breadcrumbs.js components/case/summary.js tests/phase22/route-state-helper.test.cjs tests/phase22/index.test.cjs` -> pass with 1 pre-existing warning in `components/case/summary.js` (`react-hooks/exhaustive-deps`). +- `node tests/phase22/route-state-helper.test.cjs` -> pass (5/5). +- `node tests/phase22/index.test.cjs` -> fails in existing auth suite (`tests/phase22/auth-redirect-safety.test.cjs`) with `ReferenceError: resolveLocale is not defined` in `pages/api/auth/[...nextauth].js`; unrelated to route-state changes. + +Follow-ups: + +- If needed, triage/fix the existing phase22 auth redirect test harness failure separately to restore full combined suite execution. +- Expand helper adoption in future slices to other navigation decision call sites if additional duplication emerges. + ### CL-00X: 22500 `components/elements/index.js` Phase 1 helper extraction date: 2026-04-07 diff --git a/tests/phase22/auth-redirect-safety.test.cjs b/tests/phase22/auth-redirect-safety.test.cjs index 7020109c..9cbf80d8 100644 --- a/tests/phase22/auth-redirect-safety.test.cjs +++ b/tests/phase22/auth-redirect-safety.test.cjs @@ -21,7 +21,7 @@ const loadAuthInternals = () => { source = source.replace(/import[\s\S]*?from\s+"[^"]+";\n?/g, ""); source = source.replace( /export default NextAuthPEDW;\s*$/, - "module.exports = { appendParamsAndPathToNewUrl, resolveLocale, authOptions, NextAuthPEDW };" + "module.exports = { appendParamsAndPathToNewUrl, resolveRequestLocale, authOptions, NextAuthPEDW };" ); const context = { @@ -68,11 +68,11 @@ const loadAuthInternals = () => { return context.module.exports; }; -test("auth/resolveLocale prefers query then body then cookie then default", async () => { +test("auth/resolveRequestLocale prefers query then body then cookie then default", async () => { const mod = loadAuthInternals(); assert.strictEqual( - mod.resolveLocale({ + mod.resolveRequestLocale({ query: { locale: "cy" }, body: { locale: "en" }, cookies: { pedw_locale: "en" } @@ -81,7 +81,7 @@ test("auth/resolveLocale prefers query then body then cookie then default", asyn ); assert.strictEqual( - mod.resolveLocale({ + mod.resolveRequestLocale({ body: { locale: "cy" }, cookies: { pedw_locale: "en" } }), @@ -89,13 +89,13 @@ test("auth/resolveLocale prefers query then body then cookie then default", asyn ); assert.strictEqual( - mod.resolveLocale({ + mod.resolveRequestLocale({ cookies: { pedw_locale: "cy" } }), "cy" ); - assert.strictEqual(mod.resolveLocale({}), "en"); + assert.strictEqual(mod.resolveRequestLocale({}), "en"); }); test("auth/redirect callback keeps relative URLs on same base", async () => { diff --git a/tests/phase22/index.test.cjs b/tests/phase22/index.test.cjs index 02d4341c..a8e2ea0f 100644 --- a/tests/phase22/index.test.cjs +++ b/tests/phase22/index.test.cjs @@ -6,6 +6,7 @@ const runPortalServiceTests = require("./portal-service-behaviour.test.cjs"); const runAuthRedirectSafetyTests = require("./auth-redirect-safety.test.cjs"); const runI18nRouteParityTests = require("./i18n-route-parity.test.cjs"); const runAzurestorageHelperTests = require("./azurestorage-helper-behaviour.test.cjs"); +const runRouteStateHelperTests = require("./route-state-helper.test.cjs"); const run = async () => { await runCoreTokenTests(); @@ -16,6 +17,7 @@ const run = async () => { await runAuthRedirectSafetyTests(); await runI18nRouteParityTests(); await runAzurestorageHelperTests(); + await runRouteStateHelperTests(); console.log("Phase 22 combined suite passed."); }; diff --git a/tests/phase22/route-state-helper.test.cjs b/tests/phase22/route-state-helper.test.cjs new file mode 100644 index 00000000..6b2ac398 --- /dev/null +++ b/tests/phase22/route-state-helper.test.cjs @@ -0,0 +1,139 @@ +const fs = require("fs"); +const path = require("path"); +const vm = require("vm"); +const assert = require("assert"); + +const rootDir = path.resolve(__dirname, "..", ".."); + +const loadRouteStateModule = () => { + const filePath = path.join(rootDir, "lib", "routing", "routeState.js"); + let source = fs.readFileSync(filePath, "utf8"); + + source = source.replace(/export const\s+/g, "const "); + source += + "\nmodule.exports = { normalizeRouteStateQuery, resolveSearchResultsHref };\n"; + + const context = { + module: { exports: {} }, + exports: {}, + require + }; + + vm.runInNewContext(source, context, { filename: filePath }); + return context.module.exports; +}; + +const tests = []; +const test = (name, fn) => tests.push({ name, fn }); + +test("routing/routeState normalizes va/adv/ads flags and key", async () => { + const mod = loadRouteStateModule(); + + const normalized = mod.normalizeRouteStateQuery({ + va: "true", + adv: "false", + ads: "true", + key: "myCases" + }); + + assert.deepStrictEqual(JSON.parse(JSON.stringify(normalized)), { + viewAll: true, + advanced: false, + address: true, + key: "myCases" + }); +}); + +test("routing/routeState resolves view-all href when va=true", async () => { + const mod = loadRouteStateModule(); + + const href = mod.resolveSearchResultsHref({ + query: { va: "true", key: "myCases", adv: "true" }, + hasSession: true + }); + + assert.deepStrictEqual(JSON.parse(JSON.stringify(href)), { + pathname: "/myportal/viewall", + query: { key: "myCases" } + }); +}); + +test("routing/routeState resolves advanced/address/default result hrefs", async () => { + const mod = loadRouteStateModule(); + + const advancedHref = mod.resolveSearchResultsHref({ + query: { adv: "true", q: "abc" }, + hasSession: false + }); + const addressHref = mod.resolveSearchResultsHref({ + query: { ads: "true", q: "abc" }, + hasSession: true + }); + const defaultHref = mod.resolveSearchResultsHref({ + query: { q: "abc" }, + hasSession: true + }); + + assert.deepStrictEqual(JSON.parse(JSON.stringify(advancedHref)), { + pathname: "/advancedsearchresults", + query: { adv: "true", q: "abc" } + }); + assert.deepStrictEqual(JSON.parse(JSON.stringify(addressHref)), { + pathname: "/myportal/addresssearchresults", + query: { ads: "true", q: "abc" } + }); + assert.deepStrictEqual(JSON.parse(JSON.stringify(defaultHref)), { + pathname: "/myportal/searchresults", + query: { q: "abc" } + }); +}); + +test("routing/routeState supports fallback to /myportal when no route flags", async () => { + const mod = loadRouteStateModule(); + + const href = mod.resolveSearchResultsHref({ + query: { q: "abc" }, + hasSession: false, + fallbackToMyPortalWhenNoFlags: true + }); + + assert.deepStrictEqual(JSON.parse(JSON.stringify(href)), { + pathname: "/myportal" + }); +}); + +test("routing/routeState supports DNS route override", async () => { + const mod = loadRouteStateModule(); + + const href = mod.resolveSearchResultsHref({ + query: { adv: "true", q: "abc" }, + hasSession: true, + isDnsRoute: true + }); + + assert.deepStrictEqual(JSON.parse(JSON.stringify(href)), { + pathname: "/myportal/dnsapplications" + }); +}); + +const run = async () => { + let passed = 0; + + for (const currentTest of tests) { + await currentTest.fn(); + passed += 1; + } + + console.log( + `Phase 22 route-state helper tests passed (${passed}/${tests.length}).` + ); +}; + +module.exports = run; + +if (require.main === module) { + run().catch((error) => { + console.error(error); + process.exit(1); + }); +}