Skip to content

Commit 717e209

Browse files
committed
CCM-15826: Add user claims popover and header navigation components; update navigation structure and related styles
1 parent 54445ac commit 717e209

26 files changed

Lines changed: 1065 additions & 160 deletions

frontend/.env.template

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ NEXT_PUBLIC_BASE_PATH=''
44
# Optional local override for the backend API base URL.
55
API_BASE_URL=''
66

7-
# Amplify / Cognito values used by `npm run create-amplify-outputs env`.
7+
# Amplify / Cognito values used to generate `frontend/amplify_outputs.json`
8+
# for local development. Frontend package scripts regenerate that file from
9+
# `.env` / `.env.local` before running.
810
AWS_REGION=''
911
USER_POOL_CLIENT_ID=''
1012
USER_POOL_ID=''

frontend/AGENTS.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# AGENTS.md
2+
<!-- vale off -->
3+
4+
## Scope
5+
6+
This file applies to work under `frontend/`.
7+
Follow the root `AGENTS.md` first, then use this file for frontend-specific guidance.
8+
9+
## What this package is
10+
11+
- A Next.js App Router frontend workspace.
12+
- Uses `nhsuk-frontend` Sass/CSS alongside React components and Amplify auth UI.
13+
- Package-local scripts live in `frontend/package.json` and are the source of truth for package verification.
14+
15+
## Change guidance
16+
17+
- Keep changes small and focused.
18+
- Prefer existing package entrypoints over deep imports when bringing in framework assets or styles.
19+
- Treat `.next/` as generated output; do not edit generated files.
20+
- If you change routing, auth, styles, Next config, ESLint config, or dependencies, run the full verification flow below.
21+
22+
## Known fragile areas
23+
24+
### Sass imports
25+
26+
- `frontend/src/styles/app.scss` must import NHS styles from the package entrypoint:
27+
28+
```scss
29+
@use "nhsuk-frontend";
30+
```
31+
32+
- Do not switch this back to a deep path such as `nhsuk-frontend/dist/nhsuk` unless you have verified that the installed package still resolves that path in this workspace.
33+
- After changing Sass imports or style tooling, run `npm run build --workspace frontend` from the repo root to catch stylesheet resolution problems.
34+
35+
### App Router query params and auth pages
36+
37+
- Build-time and prerender-sensitive routes should prefer resolving query params in the server `page.tsx` via `searchParams`, then passing plain props into client components.
38+
- Be careful when introducing `useSearchParams()` in client components used by prerendered routes such as `frontend/src/app/auth/page.tsx`; this can require a suspense boundary or cause build failures.
39+
- If you touch auth flow or redirect handling, test both the production build and a manual auth URL such as `/auth?redirect=%2F`.
40+
41+
### ESLint and generated Next.js files
42+
43+
- `frontend/.next/` must stay ignored by ESLint.
44+
- If you change `frontend/eslint.config.mjs`, verify that `.next/**` is still ignored and run lint after a build, not only before it.
45+
- A clean lint result before `next build` is not sufficient; generated `.next/types/**` files can reveal config regressions only after a build has run.
46+
47+
## Required verification flow
48+
49+
Run these commands from the repository root when making non-trivial changes in `frontend/`:
50+
51+
```sh
52+
pre-commit run --config scripts/config/pre-commit.yaml
53+
rm -rf frontend/.next
54+
npm run build --workspace frontend
55+
npm run lint --workspace frontend
56+
npm run typecheck --workspace frontend
57+
npm run test:unit --workspace frontend
58+
```
59+
60+
## Verification notes
61+
62+
- Use the clean build step to surface Sass resolution and App Router prerender issues.
63+
- Keep the post-build lint step to catch accidental linting of generated `.next` artifacts.
64+
- `typecheck` and `test:unit` both create mock Amplify outputs automatically via package scripts; prefer the existing scripts over ad hoc commands.
65+
- If you change auth, routing, middleware, or base-path handling, add a quick manual browser check in development as an extra validation step.
66+
67+
## Minimum review summary for frontend changes
68+
69+
Include in your handoff or PR summary:
70+
71+
- what changed in `frontend/`
72+
- why the change was needed
73+
- that the root pre-commit checks were run
74+
- that `build`, `lint`, `typecheck`, and `test:unit` were run for the frontend workspace
75+
- any manual route or auth checks you performed

frontend/eslint.config.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export default defineConfig([
1111
},
1212
},
1313
{
14-
ignores: ["jest.config.mjs"],
14+
ignores: [".next/**", "jest.config.mjs", "next-env.d.ts"],
1515
},
1616
{
1717
files: ["next.config.js"],

frontend/jest.config.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const config = {
99
testEnvironment: "node",
1010
testPathIgnorePatterns: ["/node_modules/", ".next/"],
1111
transform: {
12-
"^.+\\.(ts|tsx)$": ["ts-jest", { tsconfig: "<rootDir>/tsconfig.json" }],
12+
"^.+\\.(ts|tsx)$": ["ts-jest", { tsconfig: "<rootDir>/tsconfig.jest.json" }],
1313
},
1414
};
1515

frontend/next-env.d.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
/// <reference types="next" />
2+
/// <reference types="next/image-types/global" />
3+
/// <reference path="./.next/types/routes.d.ts" />
4+
5+
// NOTE: This file should not be edited
6+
// see https://nextjs.org/docs/app/api-reference/config/typescript for more information.

frontend/package.json

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,15 @@
2828
"name": "@supplier-config/frontend",
2929
"private": true,
3030
"scripts": {
31-
"build": "next build",
32-
"dev": "next dev",
33-
"lint": "npm run mock-amplify-outputs && eslint .",
31+
"build": "npm run sync-amplify-outputs && next build",
32+
"dev": "npm run sync-amplify-outputs && next dev",
33+
"lint": "npm run sync-amplify-outputs && eslint .",
3434
"lint:fix": "npm run lint -- --fix",
3535
"mock-amplify-outputs": "if [ ! -f ./amplify_outputs.json ]; then echo \"{}\" > ./amplify_outputs.json ; fi",
36-
"start": "next start",
37-
"test:unit": "npm run mock-amplify-outputs && jest --runInBand",
38-
"typecheck": "npm run mock-amplify-outputs && tsc --noEmit"
36+
"start": "npm run sync-amplify-outputs && next start",
37+
"sync-amplify-outputs": "if [ -f ./.env ] || [ -f ./.env.local ]; then npm --prefix .. run create-amplify-outputs env; else npm run mock-amplify-outputs; fi",
38+
"test:unit": "npm run sync-amplify-outputs && jest --runInBand",
39+
"typecheck": "npm run sync-amplify-outputs && tsc --noEmit"
3940
},
4041
"version": "0.1.0"
4142
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import {
2+
getNavigationLinkForPath,
3+
getNavigationSectionForPath,
4+
overviewNavigationLink,
5+
} from "@/components/navigation-links";
6+
7+
describe("navigation-links", () => {
8+
it("maps supplier pages to the Suppliers section", () => {
9+
expect(getNavigationSectionForPath("/suppliers")?.label).toBe("Suppliers");
10+
expect(getNavigationSectionForPath("/supplier-allocations")?.label).toBe(
11+
"Suppliers",
12+
);
13+
expect(getNavigationSectionForPath("/volume-groups")?.label).toBe(
14+
"Suppliers",
15+
);
16+
});
17+
18+
it("maps specification pages to the Specifications section", () => {
19+
expect(getNavigationSectionForPath("/pack-specifications")?.label).toBe(
20+
"Specifications",
21+
);
22+
expect(getNavigationSectionForPath("/postage")?.label).toBe(
23+
"Specifications",
24+
);
25+
});
26+
27+
it("maps mapping pages to the Mapping section", () => {
28+
expect(getNavigationSectionForPath("/letter-variants")?.label).toBe(
29+
"Mapping",
30+
);
31+
expect(getNavigationSectionForPath("/reports/suppliers")?.label).toBe(
32+
"Mapping",
33+
);
34+
});
35+
36+
it("returns the exact active link for subsection pages", () => {
37+
expect(getNavigationLinkForPath("/reports/suppliers")?.label).toBe(
38+
"Supplier reports",
39+
);
40+
expect(getNavigationLinkForPath("/letter-variants")?.label).toBe(
41+
"Letter variants",
42+
);
43+
});
44+
45+
it("keeps overview outside the grouped sections", () => {
46+
expect(overviewNavigationLink.href).toBe("/");
47+
expect(getNavigationSectionForPath("/")).toBeUndefined();
48+
expect(getNavigationLinkForPath("/")).toBeUndefined();
49+
});
50+
});

frontend/src/__tests__/token-utils.test.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { getDisplayName, hasAdminGroup } from "@/utils/token-utils";
1+
import {
2+
getClaimsFromToken,
3+
getDisplayName,
4+
hasAdminGroup,
5+
} from "@/utils/token-utils";
26

37
function encodeBase64Url(value: unknown) {
48
return Buffer.from(JSON.stringify(value)).toString("base64url");
@@ -11,15 +15,15 @@ function createToken(payload: Record<string, unknown>) {
1115
describe("token-utils", () => {
1216
it("recognises the admin group from Cognito groups", () => {
1317
const token = createToken({
14-
"cognito:groups": ["editor", "admin"],
18+
"cognito:groups": ["Editors", "Admins"],
1519
});
1620

1721
expect(hasAdminGroup(token)).toBe(true);
1822
});
1923

20-
it("returns false when the admin group is not present", () => {
24+
it("returns false when the canonical admin group is not present", () => {
2125
const token = createToken({
22-
"cognito:groups": ["viewer"],
26+
"cognito:groups": ["admin"],
2327
});
2428

2529
expect(hasAdminGroup(token)).toBe(false);
@@ -34,4 +38,21 @@ describe("token-utils", () => {
3438

3539
expect(getDisplayName(token)).toBe("Alex Morgan");
3640
});
41+
42+
it("returns decoded claims when a token is provided", () => {
43+
const token = createToken({
44+
email: "alex@example.nhs.uk",
45+
sub: "user-123",
46+
});
47+
48+
expect(getClaimsFromToken(token)).toEqual({
49+
email: "alex@example.nhs.uk",
50+
sub: "user-123",
51+
});
52+
});
53+
54+
it("returns undefined for a missing or invalid token", () => {
55+
expect(getClaimsFromToken()).toBeUndefined();
56+
expect(getClaimsFromToken("not-a-jwt")).toBeUndefined();
57+
});
3758
});
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { createElement } from "react";
2+
import { renderToStaticMarkup } from "react-dom/server";
3+
import { UserClaimsPopover } from "@/components/user-claims-popover";
4+
import {
5+
isPopoverDismissKey,
6+
isPopoverInteractionOutside,
7+
} from "@/utils/user-claims-popover";
8+
9+
describe("UserClaimsPopover", () => {
10+
it("renders token claims when the popover is open", () => {
11+
const markup = renderToStaticMarkup(
12+
createElement(UserClaimsPopover, {
13+
claims: {
14+
"cognito:groups": ["Admins"],
15+
email: "alex@example.nhs.uk",
16+
sub: "user-123",
17+
},
18+
displayName: "Alex Morgan",
19+
initialOpen: true,
20+
}),
21+
);
22+
23+
expect(markup).toContain("Current user token claims");
24+
expect(markup).toContain("Alex Morgan");
25+
expect(markup).toContain("email");
26+
expect(markup).toContain("alex@example.nhs.uk");
27+
expect(markup).toContain("[&quot;Admins&quot;]");
28+
});
29+
30+
it("does not render token claims when the popover is closed", () => {
31+
const markup = renderToStaticMarkup(
32+
createElement(UserClaimsPopover, {
33+
claims: { email: "alex@example.nhs.uk" },
34+
displayName: "Alex Morgan",
35+
}),
36+
);
37+
38+
expect(markup).toContain("Alex Morgan");
39+
expect(markup).not.toContain("Current user token claims");
40+
});
41+
42+
it("recognises interactions inside the popover container", () => {
43+
const insideTarget = {} as Node;
44+
45+
expect(
46+
isPopoverInteractionOutside(
47+
{
48+
contains: (node) => [insideTarget].includes(node as Node),
49+
},
50+
insideTarget as unknown as EventTarget,
51+
),
52+
).toBe(false);
53+
});
54+
55+
it("recognises interactions outside the popover container", () => {
56+
const outsideTarget = {} as Node;
57+
58+
expect(
59+
isPopoverInteractionOutside(
60+
{
61+
contains: () => false,
62+
},
63+
outsideTarget as unknown as EventTarget,
64+
),
65+
).toBe(true);
66+
});
67+
68+
it("closes the popover when Escape is pressed", () => {
69+
expect(isPopoverDismissKey("Escape")).toBe(true);
70+
expect(isPopoverDismissKey("Enter")).toBe(false);
71+
});
72+
});

frontend/src/app/auth/page.tsx

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,26 @@
11
import { AuthPageContent } from "@/components/auth-page-content";
2+
import { getBasePath } from "@/utils/get-base-path";
23

3-
export default function AuthPage() {
4-
return <AuthPageContent />;
4+
type AuthPageProps = Readonly<{
5+
searchParams: Promise<{
6+
error?: string | string[];
7+
redirect?: string | string[];
8+
}>;
9+
}>;
10+
11+
function getFirstValue(value?: string | string[]) {
12+
return Array.isArray(value) ? value[0] : value;
13+
}
14+
15+
export default async function AuthPage({ searchParams }: AuthPageProps) {
16+
const resolvedSearchParams = await searchParams;
17+
const redirectTarget = getFirstValue(resolvedSearchParams.redirect);
18+
const error = getFirstValue(resolvedSearchParams.error);
19+
20+
return (
21+
<AuthPageContent
22+
error={error}
23+
redirectTarget={redirectTarget || getBasePath() || "/"}
24+
/>
25+
);
526
}

0 commit comments

Comments
 (0)