VIBE-241 - CaTH General Information - Cookie Policy #157
Conversation
Created specification and implementation plan for GOV.UK-compliant cookie policy page with user-configurable consent management. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Created specification document with requirements and acceptance criteria - Created tasks document with implementation checklist - Includes cookie policy page, settings form, cookie management logic, and testing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Specification document with requirements and acceptance criteria - Technical implementation plan with 5 phases - Detailed task breakdown with 14 actionable tasks - Estimated effort: 4.5 hours 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- specification.md: Technical specification and requirements analysis - plan.md: 8-phase implementation plan with timelines - tasks.md: 14-task breakdown with code templates - Downloaded Cookie Policy.docx attachment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Created comprehensive technical implementation plan (plan.md): - Analysis of existing cookie management infrastructure - New cookie policy page design (content + embedded settings form) - Cookie category documentation (essential, analytics, performance, settings) - Footer integration and Welsh language support - Testing strategy with GDPR/PECR compliance - 9-phase implementation plan (7.5 days estimated) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Generated comprehensive technical specification and task breakdown for implementing the Cookie Policy page with bilingual support. Specification covers: - Architecture and component breakdown - Frontend/backend requirements - Cookie management integration - Accessibility requirements (WCAG 2.2 AA) - Security considerations - Testing strategy Tasks include: - 28 tasks across 8 phases - Categorized by priority (P0-P2) and complexity (S/M/L) - Estimated 55 hours total effort - Clear dependencies and critical path defined Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughAdds a bilingual Cookie Policy feature (page, templates, controllers, locale content), reorganizes cookie categories (analytics → GA only; new performance category for Dynatrace cookies), gates analytics/performance script injection on consent, fixes cookie encoding/storage, updates footer link behavior, and adds extensive tests and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant CookiePolicyHandler as Cookie Policy<br/>Controller
participant CookieHelpers as Cookie<br/>Helpers/Manager
participant ServerResponse as Response
User->>Browser: navigate to /cookie-policy
Browser->>CookiePolicyHandler: GET request
CookiePolicyHandler->>CookieHelpers: parseCookiePolicy(req.cookies.cookie_policy)
CookieHelpers-->>CookiePolicyHandler: preferences (analytics/performance)
CookiePolicyHandler->>ServerResponse: render page (template + prefs)
ServerResponse-->>Browser: HTML
Browser->>User: user views page
User->>Browser: select prefs and submit form
Browser->>CookiePolicyHandler: POST /cookie-policy (form body)
CookiePolicyHandler->>CookieHelpers: setCookiePolicy(preferences, { path: "/" })
CookieHelpers-->>CookiePolicyHandler: cookie set
CookiePolicyHandler->>CookieHelpers: setCookieBannerSeen({ path: "/" })
CookieHelpers-->>CookiePolicyHandler: banner cookie set
CookiePolicyHandler->>ServerResponse: redirect to /cookie-policy?saved=true[&locale=cy]
ServerResponse-->>Browser: 302 Redirect
Browser->>CookiePolicyHandler: GET /cookie-policy?saved=true
CookiePolicyHandler->>ServerResponse: render page (with success banner)
ServerResponse-->>Browser: HTML (success state)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/package.json📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
.ai/plans/VIBE-241/plan.md (1)
18-155: Planning paths vs. actualcookies-policyimplementation (same note as other plan docs)This plan also references
libs/web-core/src/pages/cookie-policy/...and/cookie-policy, while the implemented code uses acookies-policydirectory and corresponding render path. For consistency with the codebase (and the other VIBE‑241 docs), it would help to update those examples, and optionally add languages to fenced code blocks to satisfy markdown linters.
🧹 Nitpick comments (12)
libs/web-core/src/middleware/cookies/cookie-helpers.ts (1)
7-36: Encoding/path behaviour looks sound; consider reusing cookie-name constantThe updated helpers look consistent with how
req.cookiesis populated:
parseCookiePolicydoing a straightJSON.parseon the value is correct if cookie parsing has already handled URL decoding; invalid JSON falling back to{}is a sensible failure mode for consent.setCookiePolicynow serialises once withJSON.stringifyand relies on the cookie library to do any required encoding, which avoids double-encoding and simplifies the round trip.- Adding
path: "/"(and mirroring it forcookies_preferences_set) fixes scope issues so these cookies apply across the service, and the 1‑yearmaxAgematches the requirements.One small maintainability improvement: elsewhere (for example in
libs/web-core/src/pages/cookies-policy/index.ts)req.cookies?.cookie_policyis referenced via a string literal. Importing and usingCOOKIE_POLICY_NAMEthere would avoid accidental divergence if the cookie name ever changes..ai/plans/VIBE-241/tasks.md (1)
1-561: Align documented paths with implementedcookies-policyroute, and tidy URL formattingThe task examples still refer to
libs/web-core/src/pages/cookie-policy/...and acookie-policy/indextemplate, while the implemented code and routes usecookies-policy(plural). It would help future readers if these paths and template names were updated to match the real structure.While you’re here, you could also wrap
http://localhost:3000/cookie-policyin backticks or a markdown link to avoid the bare-URL markdown warning.libs/web-core/src/pages/cookies-policy/polisi-cwcis.ts (1)
1-7: Welsh redirect handler cleanly preserves state and matches controller conventionsThis
GEThandler correctly:
- Uses Express
Request/Responsetypes and a namedGETexport as per the page-controller guidelines.- Redirects
/polisi-cwcisto/cookies-policy?lng=cy, appending&saved=truewhen relevant so the “saved” state is preserved across the redirect.The
asynckeyword isn’t needed here since there’s noawait, but it doesn’t affect behaviour if you’d rather leave it for consistency. Based on learnings, this nicely complements the bilingual cookies-policy controller that serves bothenandcycontent.VIBE-241-plan.md (1)
60-80: Update cookie storage details to match current implementationThis plan still describes a
cookie_preferencescookie withSameSite: Lax, but the helper now uses acookie_policycookie withsameSite: "strict"and a 1‑year max age. It would be worth updating the “Cookie Storage Format” section so future readers don’t have to reconcile the differences between this plan and the actual implementation.libs/web-core/src/views/components/head-analytics.test.ts (1)
48-58: Consider adding test for undefined cookieManager.The tests cover empty
cookiePreferences: {}but don't test whencookieManageritself is undefined or whencookieManager.cookiePreferencesis undefined. This could cause template errors if the middleware doesn't set the cookieManager.Consider adding a test case:
it("should not load GTM when cookieManager is undefined", () => { const html = env.render("components/head-analytics.njk", { gtm: { containerId: "GTM-TEST123" } }); expect(html).not.toContain("GTM-TEST123"); expect(html).not.toContain("Google Tag Manager"); });libs/web-core/src/pages/cookies-policy/index.njk (1)
198-202: Avoid inline styles; use a CSS class instead.The
style="text-decoration: none;"should be moved to a CSS class for better maintainability and separation of concerns.- <a href="#" class="govuk-link back-to-top-link" style="text-decoration: none;" aria-label="{{ backToTop.ariaLabel }}"> + <a href="#" class="govuk-link back-to-top-link" aria-label="{{ backToTop.ariaLabel }}">Add in your CSS:
.back-to-top-link { text-decoration: none; }e2e-tests/tests/cookie-policy.spec.ts (2)
128-146: ReplacewaitForTimeoutwith condition-based waits to reduce flakiness.Using
waitForTimeoutwith fixed delays (lines 131, 141) can cause flaky tests. Consider waiting for the scroll position to stabilize or usingpage.waitForFunction.- // Wait a moment for scroll to complete - await page.waitForTimeout(100); - - // Verify we scrolled - let scrollY = await page.evaluate(() => window.scrollY); - expect(scrollY).toBeGreaterThan(500); + // Wait for scroll to complete + await page.waitForFunction(() => window.scrollY > 500); // Click back to top link await page.locator('a:has-text("Back to top")').click(); - // Wait for scroll animation - await page.waitForTimeout(300); - - // Verify we're at top - scrollY = await page.evaluate(() => window.scrollY); - expect(scrollY).toBeLessThan(100); + // Wait for scroll to top + await page.waitForFunction(() => window.scrollY < 100);
380-394: Clarify that this tests responsive layout, not browser zoom.Setting viewport to 640x480 tests responsive/mobile layout, not actual 200% browser zoom. The comment is misleading. WCAG zoom requirements relate to browser zoom, which maintains the original viewport width while scaling content.
Consider updating the test name and comment for accuracy, or note this as a proxy test for reflow at narrow widths.
- test("should support browser zoom up to 200%", async ({ page }) => { + test("should support narrow viewport (reflow at 320px CSS equivalent)", async ({ page }) => { await page.goto("/cookies-policy"); - // Set viewport to simulate 200% zoom (half the width) + // Test responsive layout at narrow width (similar to mobile or zoomed view) await page.setViewportSize({ width: 640, height: 480 });VIBE-241-specification.md (2)
28-28: Use proper markdown headings instead of bold emphasis.Lines 28 and 32 use
**text**(bold emphasis) for section headers, which should be markdown headings (##or###) for better document structure and accessibility.-**Category A: Website Usage Measurement** +### Category A: Website Usage Measurement -**Category B: Performance Monitoring** +### Category B: Performance MonitoringAlso applies to: 32-32
52-52: Specify language for fenced code block.Line 52 contains a code block without a language identifier, which reduces syntax highlighting and clarity.
-``` +``` libs/cookie-policy/ ├── src/ │ ├── pages/ │ │ └── cookie-policy.ts # Controller │ │ └── cookie-policy.njk # Nunjucks template │ ├── middleware/ │ │ └── cookie-consent-middleware.ts # Consent checking │ ├── services/ │ │ └── cookie-service.ts # Cookie management logic │ ├── locales/ │ │ ├── en.ts # English translations │ │ └── cy.ts # Welsh translations │ └── config.ts # Module exports ├── package.json └── tsconfig.json -``` +```Note: Since this is a directory tree (not code), you may consider using no language identifier, but adding
textis also acceptable for clarity.docs/tickets/VIBE-241/plan.md (2)
53-53: Add language identifier to directory tree code block.Line 53 code block lacks a language identifier. Consider adding
textortreefor consistency.-``` +```text libs/web-core/src/ ├── pages/ │ ├── cookie-preferences/ # Existing (keep as-is) ... -``` +```
1070-1072: Convert bare URLs to markdown link syntax.Lines 1070–1072 contain bare URLs that should be wrapped in markdown link syntax for better documentation formatting.
## References -- **Specification**: `/home/runner/work/cath-service/cath-service/docs/tickets/VIBE-241/specification.md` -- **Tasks**: `/home/runner/work/cath-service/cath-service/docs/tickets/VIBE-241/tasks.md` +- **Specification**: [VIBE-241 Specification](/docs/tickets/VIBE-241/specification.md) +- **Tasks**: [VIBE-241 Tasks](/docs/tickets/VIBE-241/tasks.md) - **Existing Implementation**: - Cookie preferences page: `libs/web-core/src/pages/cookie-preferences/` - Cookie manager middleware: `libs/web-core/src/middleware/cookies/` - Accessibility statement (reference pattern): `libs/web-core/src/pages/accessibility-statement/` -- **GOV.UK Design System**: https://design-system.service.gov.uk/ -- **WCAG 2.2 Guidelines**: https://www.w3.org/WAI/WCAG22/quickref/ -- **GDPR/PECR Cookie Guidance**: https://ico.org.uk/for-organisations/guide-to-pecr/cookies-and-similar-technologies/ +- **GOV.UK Design System**: [GOV.UK Design System](https://design-system.service.gov.uk/) +- **WCAG 2.2 Guidelines**: [WCAG 2.2 Quick Reference](https://www.w3.org/WAI/WCAG22/quickref/) +- **GDPR/PECR Cookie Guidance**: [ICO Cookie Guidance](https://ico.org.uk/for-organisations/guide-to-pecr/cookies-and-similar-technologies/)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/VIBE-241/Cookie Policy.docxis excluded by!**/*.docx
📒 Files selected for processing (31)
.ai/plans/VIBE-241/plan.md(1 hunks).ai/plans/VIBE-241/specification.md(1 hunks).ai/plans/VIBE-241/tasks.md(1 hunks).jira/VIBE-241.md(1 hunks).jira/specification.md(1 hunks).jira/tasks.md(1 hunks)VIBE-241-plan.md(1 hunks)VIBE-241-specification.md(1 hunks)apps/web/src/app.ts(1 hunks)docs/VIBE-241/plan.md(1 hunks)docs/VIBE-241/specification.md(1 hunks)docs/VIBE-241/tasks.md(1 hunks)docs/tickets/VIBE-241/plan.md(1 hunks)docs/tickets/VIBE-241/specification.md(1 hunks)docs/tickets/VIBE-241/tasks.md(1 hunks)e2e-tests/tests/cookie-policy.spec.ts(1 hunks)e2e-tests/tests/page-structure.spec.ts(2 hunks)libs/web-core/src/middleware/cookies/cookie-helpers.ts(1 hunks)libs/web-core/src/middleware/cookies/cookie-manager-middleware.test.ts(1 hunks)libs/web-core/src/pages/cookies-policy/cy.ts(1 hunks)libs/web-core/src/pages/cookies-policy/en.ts(1 hunks)libs/web-core/src/pages/cookies-policy/index.njk(1 hunks)libs/web-core/src/pages/cookies-policy/index.test.ts(1 hunks)libs/web-core/src/pages/cookies-policy/index.ts(1 hunks)libs/web-core/src/pages/cookies-policy/polisi-cwcis.ts(1 hunks)libs/web-core/src/views/components/body-start-analytics.njk(1 hunks)libs/web-core/src/views/components/body-start-analytics.test.ts(1 hunks)libs/web-core/src/views/components/head-analytics.njk(2 hunks)libs/web-core/src/views/components/head-analytics.test.ts(1 hunks)libs/web-core/src/views/components/site-footer.njk(1 hunks)libs/web-core/src/views/layouts/base-template.njk(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and interfaces must use PascalCase (e.g.,UserService,CaseRepository). DO NOT useIprefix for interfaces.
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: constants at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
Use workspace aliases (@hmcts/*) for imports instead of relative paths across packages.
Database queries must be parameterized using Prisma (never raw SQL with string concatenation).
Never put sensitive data in logs.
Don't add comments unless meaningful - explain why something is done, not what is done.
Favour functional style - use simple functions. Don't use a class unless you have shared state.
Data should be immutable by default - use const and avoid mutations to ensure predictable state.
Functions should have no side effects - avoid modifying external state or relying on mutable data.
Files:
libs/web-core/src/views/components/body-start-analytics.test.tslibs/web-core/src/pages/cookies-policy/en.tslibs/web-core/src/middleware/cookies/cookie-helpers.tslibs/web-core/src/views/components/head-analytics.test.tsapps/web/src/app.tslibs/web-core/src/middleware/cookies/cookie-manager-middleware.test.tslibs/web-core/src/pages/cookies-policy/cy.tslibs/web-core/src/pages/cookies-policy/polisi-cwcis.tslibs/web-core/src/pages/cookies-policy/index.tse2e-tests/tests/page-structure.spec.tslibs/web-core/src/pages/cookies-policy/index.test.tse2e-tests/tests/cookie-policy.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Files and directories must use kebab-case (e.g.,user-service.ts,case-management/).
Never use CommonJS (require(),module.exports). Use ES modules (import/export) exclusively.
Files:
libs/web-core/src/views/components/body-start-analytics.test.tslibs/web-core/src/pages/cookies-policy/en.tslibs/web-core/src/middleware/cookies/cookie-helpers.tslibs/web-core/src/views/components/head-analytics.test.tsapps/web/src/app.tslibs/web-core/src/middleware/cookies/cookie-manager-middleware.test.tslibs/web-core/src/pages/cookies-policy/cy.tslibs/web-core/src/pages/cookies-policy/polisi-cwcis.tslibs/web-core/src/pages/cookies-policy/index.tse2e-tests/tests/page-structure.spec.tslibs/web-core/src/pages/cookies-policy/index.test.tse2e-tests/tests/cookie-policy.spec.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source files using the pattern
*.test.tsor*.spec.tsand use Vitest.
Files:
libs/web-core/src/views/components/body-start-analytics.test.tslibs/web-core/src/views/components/head-analytics.test.tslibs/web-core/src/middleware/cookies/cookie-manager-middleware.test.tse2e-tests/tests/page-structure.spec.tslibs/web-core/src/pages/cookies-policy/index.test.tse2e-tests/tests/cookie-policy.spec.ts
**/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/pages/**/*.ts: Page controllers must export namedGETand/orPOSTfunctions with Express Request and Response types.
Every page must support both English and Welsh with separateenandcycontent objects in the controller, and Welsh content must be tested with?lng=cyquery parameter.
Files:
libs/web-core/src/pages/cookies-policy/en.tslibs/web-core/src/pages/cookies-policy/cy.tslibs/web-core/src/pages/cookies-policy/polisi-cwcis.tslibs/web-core/src/pages/cookies-policy/index.tslibs/web-core/src/pages/cookies-policy/index.test.ts
**/pages/**/*.njk
📄 CodeRabbit inference engine (CLAUDE.md)
Nunjucks templates must extend
layouts/base-templates.njkand use GOV.UK component macros (govukButton, govukInput, govukErrorSummary, etc.).
Files:
libs/web-core/src/pages/cookies-policy/index.njk
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/*.{test,spec}.ts : Test files must be co-located with source files using the pattern `*.test.ts` or `*.spec.ts` and use Vitest.
Applied to files:
libs/web-core/src/views/components/body-start-analytics.test.tslibs/web-core/src/views/components/head-analytics.test.tslibs/web-core/src/pages/cookies-policy/index.test.tse2e-tests/tests/cookie-policy.spec.ts
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to libs/**/locales/**/*.ts : Shared content (common button text, phase banners, service names, common error messages) goes in locale files (`libs/[module]/src/locales/en.ts` and `cy.ts`). Page-specific content goes in controllers.
Applied to files:
libs/web-core/src/pages/cookies-policy/en.tslibs/web-core/src/pages/cookies-policy/cy.ts
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.njk : Nunjucks templates must extend `layouts/base-templates.njk` and use GOV.UK component macros (govukButton, govukInput, govukErrorSummary, etc.).
Applied to files:
libs/web-core/src/pages/cookies-policy/index.njklibs/web-core/src/views/layouts/base-template.njk
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
Applied to files:
docs/VIBE-241/tasks.mdlibs/web-core/src/pages/cookies-policy/cy.tslibs/web-core/src/pages/cookies-policy/polisi-cwcis.tsdocs/tickets/VIBE-241/specification.mde2e-tests/tests/cookie-policy.spec.ts
📚 Learning: 2025-11-20T09:59:16.776Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 106
File: libs/system-admin-pages/src/pages/reference-data-upload/index.test.ts:84-160
Timestamp: 2025-11-20T09:59:16.776Z
Learning: In the cath-service repository, Welsh localization (lng=cy) is not required for admin screens (system-admin-pages), so locale preservation in admin screen redirects is not necessary.
Applied to files:
libs/web-core/src/pages/cookies-policy/cy.ts
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Page controllers must export named `GET` and/or `POST` functions with Express Request and Response types.
Applied to files:
libs/web-core/src/pages/cookies-policy/polisi-cwcis.tslibs/web-core/src/pages/cookies-policy/index.ts
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Don't skip accessibility testing - WCAG 2.2 AA is mandatory.
Applied to files:
docs/tickets/VIBE-241/plan.md
🧬 Code graph analysis (4)
libs/web-core/src/middleware/cookies/cookie-helpers.ts (2)
libs/web-core/src/index.ts (1)
CookiePreferences(5-5)libs/web-core/src/middleware/cookies/cookie-manager-middleware.ts (1)
CookiePreferences(39-43)
libs/web-core/src/pages/cookies-policy/polisi-cwcis.ts (1)
libs/web-core/src/pages/cookies-policy/index.ts (1)
GET(7-17)
libs/web-core/src/pages/cookies-policy/index.ts (1)
libs/web-core/src/middleware/cookies/cookie-helpers.ts (3)
parseCookiePolicy(7-17)setCookiePolicy(19-28)setCookieBannerSeen(30-37)
libs/web-core/src/pages/cookies-policy/index.test.ts (2)
libs/web-core/src/pages/cookies-policy/index.ts (2)
GET(7-17)POST(19-33)libs/web-core/src/pages/cookies-policy/polisi-cwcis.ts (1)
GET(3-7)
🪛 LanguageTool
.jira/VIBE-241.md
[style] ~21-~21: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... Policy in CaTH SO THAT CaTH Users are able to access the Cookies Policy ## Acceptanc...
(BE_ABLE_TO)
.jira/tasks.md
[style] ~144-~144: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...nces using existing helpers - Redirects with success parameter - Sets cookie banner seen fla...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
[grammar] ~335-~335: Use a hyphen to join words.
Context: ...sk 3.1 Description: Create the back to top link with smooth scroll functiona...
(QB_NEW_EN_HYPHEN)
[grammar] ~335-~335: Use a hyphen to join words.
Context: ...3.1 Description: Create the back to top link with smooth scroll functionalit...
(QB_NEW_EN_HYPHEN)
[style] ~457-~457: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...ets cookie banner seen - POST redirects with success parameter - POST returns errors for mis...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
[style] ~525-~525: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...s with errors displayed - No violations with success banner - Keyboard navigation test - Foc...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
[grammar] ~589-~589: Use a hyphen to join words.
Context: ...tion**: Add JavaScript for smooth scroll to top functionality. **Acceptance Crit...
(QB_NEW_EN_HYPHEN)
[grammar] ~589-~589: Use a hyphen to join words.
Context: ...n**: Add JavaScript for smooth scroll to top functionality. **Acceptance Criteri...
(QB_NEW_EN_HYPHEN)
docs/tickets/VIBE-241/plan.md
[grammar] ~1044-~1044: Use a hyphen to join words.
Context: ...mpliance Requirements** - ✅ GDPR/PECR compliant cookie consent - ✅ Clear ex...
(QB_NEW_EN_HYPHEN)
.ai/plans/VIBE-241/plan.md
[grammar] ~224-~224: Use a hyphen to join words.
Context: ...d release process ## Rollback Plan Low risk change. If issues arise: 1. Revert ...
(QB_NEW_EN_HYPHEN)
.jira/specification.md
[grammar] ~171-~171: Use a hyphen to join words.
Context: ...Styling CSS Requirements: - Back to top link styling (arrow icon + text) - P...
(QB_NEW_EN_HYPHEN)
[grammar] ~300-~300: Use a hyphen to join words.
Context: ...ripts must use nonce attribute - Back to top functionality should use CSP-complia...
(QB_NEW_EN_HYPHEN)
[style] ~385-~385: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ... preferences correctly - POST redirects with success parameter - Locale switching works corr...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
.ai/plans/VIBE-241/specification.md
[grammar] ~81-~81: Use a hyphen to join words.
Context: ... Proper focus indicators - Screen reader friendly descriptions - Language attribu...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
.jira/VIBE-241.md
69-69: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
94-94: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
119-119: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
.ai/plans/VIBE-241/tasks.md
271-271: Bare URL used
(MD034, no-bare-urls)
docs/VIBE-241/tasks.md
1634-1634: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
1653-1653: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/VIBE-241/specification.md
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/tickets/VIBE-241/plan.md
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
118-118: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1070-1070: Bare URL used
(MD034, no-bare-urls)
1071-1071: Bare URL used
(MD034, no-bare-urls)
1072-1072: Bare URL used
(MD034, no-bare-urls)
docs/VIBE-241/plan.md
811-811: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
VIBE-241-specification.md
28-28: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
32-32: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.ai/plans/VIBE-241/plan.md
39-39: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
87-87: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.jira/specification.md
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.ai/plans/VIBE-241/specification.md
39-39: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
87-87: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (22)
e2e-tests/tests/page-structure.spec.ts (1)
78-106: LGTM! Test correctly validates new-tab behavior for Cookies link.The test accurately separates footer links by their target behavior and explicitly validates that the Cookies link opens in a new tab with proper security attributes (
target="_blank"andrel="noopener noreferrer").libs/web-core/src/views/components/body-start-analytics.njk (1)
1-1: LGTM! Proper consent gating for analytics.The addition of
cookieManager.cookiePreferences.analyticsto the condition correctly ensures that Google Tag Manager only loads when users have explicitly consented to analytics cookies, aligning with GDPR/PECR requirements.libs/web-core/src/middleware/cookies/cookie-manager-middleware.test.ts (1)
91-91: No action needed—cookie format is correct.The test data format
'{"analytics":true}'(plain JSON) is the correct format. TheparseCookiePolicyfunction usesJSON.parse()directly without any URL decoding, andsetCookiePolicystores cookies usingJSON.stringify()with no encoding. The change from URL-encoded format aligns with the actual application behavior.libs/web-core/src/views/components/head-analytics.njk (1)
1-15: Consent-based gating for GTM/Dynatrace looks correctRequiring
cookieManager.cookiePreferences.analyticsfor GTM and.performancefor Dynatrace aligns well with the split consent model and will prevent loading these scripts without explicit opt‑in, assumingcookieManager.cookiePreferencesis always initialised inres.locals. No changes needed here.libs/web-core/src/views/layouts/base-template.njk (1)
52-62: Back-link handler is now safely guardedThe new
backLinklookup plus null‑check avoids errors on pages without a.govuk-back-linkelement while preserving the intendedhistory.back()behaviour. Wiring only the first back link is consistent with the single back-link block in this layout.docs/tickets/VIBE-241/tasks.md (1)
1-152: Task checklist is clear and consistent with the implementationThe task breakdown aligns with the implemented behaviour (policy page, footer link, consent‑gated GA/Dynatrace, tests) and clearly marks remaining work (e.g. formal Welsh approval, some documentation and cross‑browser testing). Nothing to change from a code/architecture perspective.
libs/web-core/src/views/components/body-start-analytics.test.ts (1)
1-72: LGTM! Comprehensive test coverage for consent-gated GTM rendering.The test suite thoroughly validates the conditional rendering logic for Google Tag Manager's noscript block based on cookie consent. All major scenarios are covered: consent granted, denied, missing preferences, and missing GTM configuration.
libs/web-core/src/pages/cookies-policy/index.test.ts (1)
1-206: LGTM! Thorough test coverage for cookie policy page handlers.The test suite comprehensively validates both GET and POST handlers with excellent edge case coverage. Particularly good:
- Essential cookies properly excluded from user preferences
- Both string ("on"/"off") and boolean (true/false) values handled
- Graceful handling of missing cookie configuration
- Saved state query parameter flow tested
libs/web-core/src/pages/cookies-policy/index.ts (2)
7-17: LGTM! Clean GET handler with proper bilingual support.The GET handler correctly parses existing cookie preferences and provides all necessary data to the template, including bilingual content (en/cy) as per coding guidelines.
Based on learnings, every page must support both English and Welsh with separate en and cy content objects.
19-33: LGTM! POST handler correctly processes cookie preferences.The POST implementation properly:
- Skips essential cookies (line 24)
- Handles both "on" string and boolean true values (line 25)
- Sets cookie preferences and marks banner as seen
- Redirects with saved=true for success banner display
libs/web-core/src/pages/cookies-policy/cy.ts (1)
1-130: Welsh locale structure looks complete.The Welsh content structure matches the specification requirements and provides translations for all sections including analytics, session, authentication, security, and performance cookies. The cookie table data format is correctly structured for the GOV.UK table macro.
docs/tickets/VIBE-241/specification.md (1)
1-192: Comprehensive specification document.The specification thoroughly covers all aspects of the Cookie Policy feature including acceptance criteria, URL structure, page specifications, functional requirements, validation rules, accessibility requirements, and test scenarios. This provides excellent guidance for implementation and testing.
libs/web-core/src/views/components/head-analytics.test.ts (1)
1-176: Comprehensive test coverage for consent-based script loading.The test suite effectively covers all key scenarios:
- GTM loading/blocking based on analytics consent
- Dynatrace loading/blocking based on performance consent
- Independent consent handling (one enabled, other disabled)
- Missing configuration edge cases
Good use of Vitest and proper test organization with nested
describeblocks.libs/web-core/src/pages/cookies-policy/index.njk (1)
1-206: Well-structured template following GOV.UK patterns.The template correctly:
- Extends the base template
- Uses GOV.UK component macros for consistency
- Includes hreflang alternate links for bilingual support
- Implements CSRF protection on the form
- Uses proper fieldset/legend structure for radio groups via govukRadios
- Includes accessibility attributes on interactive elements
e2e-tests/tests/cookie-policy.spec.ts (1)
1-395: Comprehensive E2E test suite for the Cookie Policy feature.Excellent coverage including:
- Content verification for all sections and cookie tables
- Preference saving and persistence
- Welsh language support and preservation after save
- Footer integration with new tab behavior
- Accessibility scanning with Axe (WCAG 2.x AA)
- Heading hierarchy, form controls, and table accessibility
- Keyboard navigation
- Responsive layout
The tests align well with the acceptance criteria in the specification.
VIBE-241-specification.md (1)
148-155: Accessibility requirements are well-documented.The WCAG 2.2 AA requirements section (lines 148–155) appropriately covers semantic HTML, keyboard navigation, screen reader support, focus management, color contrast, and language toggle. This aligns with the retrieved learning that WCAG 2.2 AA is mandatory and should not be skipped.
docs/tickets/VIBE-241/plan.md (6)
6-79: Clear gap analysis distinguishes Cookie Policy from existing Cookie Preferences page.The current state analysis (lines 6–79) is excellent. It correctly identifies that the existing
/cookie-preferencespage provides settings-only functionality, while the new/cookies-policypage should provide comprehensive policy information with an embedded settings form. This clear differentiation prevents feature duplication and confusion.
82-99: Bilingual routing strategy is well-defined.The URL routing strategy (lines 82–99) correctly outlines both query-parameter-based (
?lng=cy) and dedicated route (/polisi-cwcis) approaches. This aligns with the retrieved learning that Welsh content must be testable with?lng=cyquery parameters. The implementation plan to support both approaches provides flexibility.
589-789: Comprehensive testing strategy with strong accessibility coverage.The testing strategy (lines 589–789) is thorough and appropriately prioritizes accessibility testing. The use of Axe with WCAG 2.2 AA tags, keyboard navigation tests, and screen reader compatibility checks aligns with the retrieved learning that accessibility testing is mandatory and must not be skipped. The E2E test examples cover key user journeys (save preferences, language toggle, accordion expansion, back-to-top scrolling, new tab behavior, keyboard navigation).
556-587: WCAG 2.2 AA accessibility compliance section is complete.Accessibility compliance requirements (lines 556–587) properly cover semantic HTML, keyboard navigation, screen reader support, focus management, color contrast, and language toggle preservation. This comprehensive approach ensures the implementation will meet mandatory WCAG 2.2 AA standards.
150-201: Verify implementation files align with planned controller and route handler patterns.The plan provides detailed code examples for the controller (
index.ts), Welsh route handler (polisi-cwcis.ts), template, and content files. However, these implementation files are not included in the provided code for review. Verify that the actual implementation files created inlibs/web-core/src/pages/cookies-policy/follow these patterns, particularly:
- GET handler renders with both
enandcycontent objects (required per learnings)- POST handler persists preferences and redirects with
?saved=truequery parameter- Welsh route properly forces
lng=cyfor bilingual support- Form includes both analytics and performance radio groups
913-974: Implementation checklist spans 9 well-defined phases with clear milestones.The implementation checklist (lines 913–974) provides a structured roadmap across 9 phases from core page implementation through documentation. Phase 7 explicitly includes accessibility testing with Axe and keyboard navigation, ensuring the mandatory WCAG 2.2 AA requirements from the learnings are not overlooked.
| analytics: ["_ga", "_gid"], | ||
| performance: ["dtCookie", "dtSa", "rxVisitor", "rxvt"], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find all TypeScript and template files that might reference cookie categories
git ls-files | grep -E "\.(ts|tsx|njk|js)$" | head -20Repository: hmcts/cath-service
Length of output: 671
🏁 Script executed:
# Find the web app structure and cookie-related files
git ls-files | grep -E "apps/web" | head -30Repository: hmcts/cath-service
Length of output: 920
🏁 Script executed:
# Read the app.ts file to see the cookie categories
cat -n apps/web/src/app.ts | sed -n '70,95p'Repository: hmcts/cath-service
Length of output: 1110
🏁 Script executed:
# Search for performance category references
echo "=== Searching for 'performance' category references ==="
rg -n "performance" apps/web --type ts --type njk -C2
echo ""
echo "=== Searching for Dynatrace cookies ==="
rg -n "dtCookie|dtSa|rxVisitor|rxvt" --type ts --type njk -C2Repository: hmcts/cath-service
Length of output: 231
🏁 Script executed:
# Search without file type filter
echo "=== Searching for 'performance' category references ==="
rg -n "performance" apps/web -C2
echo ""
echo "=== Searching for Dynatrace cookies ==="
rg -n "dtCookie|dtSa|rxVisitor|rxvt" -C2
echo ""
echo "=== Checking locale files for cookie categories ==="
grep -n "analytics\|performance" apps/web/src/locales/*.tsRepository: hmcts/cath-service
Length of output: 10181
🏁 Script executed:
# Check the full context of apps/web/src/assets/js/index.ts around line 60
cat -n apps/web/src/assets/js/index.ts | sed -n '50,70p'
echo ""
echo "=== Check test file for performance category ==="
cat -n libs/web-core/src/pages/cookies-policy/index.test.ts | sed -n '25,40p'Repository: hmcts/cath-service
Length of output: 1228
🏁 Script executed:
# Double-check if there's a performance category defined anywhere in assets/js/index.ts
grep -n "performance" apps/web/src/assets/js/index.tsRepository: hmcts/cath-service
Length of output: 44
Update cookie category references in client-side cookie manifest.
The reorganization of cookies into separate analytics (Google Analytics) and performance (Dynatrace) categories in apps/web/src/app.ts is not consistently applied across the codebase:
apps/web/src/assets/js/index.tsline 60: Still groups all Dynatrace cookies (dtCookie,dtSa,rxVisitor,rxvt) under theanalyticscategory instead of the newperformancecategory.libs/web-core/src/pages/cookies-policy/index.test.tsline 32: Test mock only includesdtCookieanddtSain theperformancecategory, missingrxVisitorandrxvt.
Add a performance category to the client-side cookie manifest in apps/web/src/assets/js/index.ts and ensure all Dynatrace cookies are moved from analytics to performance. Update the test mock to include all four Dynatrace cookies.
🤖 Prompt for AI Agents
In apps/web/src/assets/js/index.ts around line 60 and in
libs/web-core/src/pages/cookies-policy/index.test.ts around line 32, the
Dynatrace cookies were not updated to the new performance category: modify the
client-side cookie manifest in apps/web/src/assets/js/index.ts by adding a
performance category and moving dtCookie, dtSa, rxVisitor and rxvt out of
analytics into performance (leaving analytics for GA cookies like _ga/_gid), and
update the test mock in libs/web-core/src/pages/cookies-policy/index.test.ts to
include all four Dynatrace cookies (dtCookie, dtSa, rxVisitor, rxvt) under the
performance category so the manifest and tests stay consistent.
| | Do not use cookies that measure my website use | Peidio â defnyddio cwcis sy'n mesur fy nefnydd o'r wefan | | ||
| | Allow cookies that measure website application performance monitoring | Caniatáu cwcis sy'n mesur y broses o fonitro perfformiad gwefannau? | | ||
| | Use cookies that measure website application performance monitoring | Defnyddio cwcis sy'n mesur y broses o fonitro perfformiad gwefannau | | ||
| | Do not cookies that measure website application performance monitoring | Peidio â defnyddio cwcis sy'n mesur y broses o fonitro perfformiad gwefannau | |
There was a problem hiding this comment.
Typo in Welsh translations table.
The English text "Do not cookies that measure website application performance monitoring" is missing the word "use". Should be "Do not use cookies that measure website application performance monitoring".
-| Do not cookies that measure website application performance monitoring | Peidio â defnyddio cwcis sy'n mesur y broses o fonitro perfformiad gwefannau |
+| Do not use cookies that measure website application performance monitoring | Peidio â defnyddio cwcis sy'n mesur y broses o fonitro perfformiad gwefannau |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Do not cookies that measure website application performance monitoring | Peidio â defnyddio cwcis sy'n mesur y broses o fonitro perfformiad gwefannau | | |
| | Do not use cookies that measure website application performance monitoring | Peidio â defnyddio cwcis sy'n mesur y broses o fonitro perfformiad gwefannau | |
🤖 Prompt for AI Agents
In docs/tickets/VIBE-241/specification.md around line 181, the English sentence
in the translations table is missing the word "use"; change "Do not cookies that
measure website application performance monitoring" to "Do not use cookies that
measure website application performance monitoring" (leave the Welsh translation
unchanged unless you also need to correct its wording/encoding).
| manageCookiesText: "Mwy o wybodaeth am ", | ||
| manageCookiesLink: "sut i reoli cwcis", | ||
| manageCookiesUrl: "https://www.aboutcookies.org/" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for manageCookiesUrl in TypeScript files to see all occurrences
rg -n "manageCookiesUrl" --type tsRepository: hmcts/cath-service
Length of output: 265
🏁 Script executed:
# Check the E2E test file for what URL it expects at line 232
fd "cookie-policy.spec.ts" --type f -x cat -n {} | grep -A 5 -B 5 "232:"Repository: hmcts/cath-service
Length of output: 44
🏁 Script executed:
# Find and examine both en.ts and cy.ts files in the cookies-policy directory
fd "cookies-policy" --type dRepository: hmcts/cath-service
Length of output: 101
🏁 Script executed:
# Find the E2E test file for cookie-policy
fd "cookie-policy.spec.ts" -type fRepository: hmcts/cath-service
Length of output: 232
🏁 Script executed:
# Search for the E2E test file in e2e-tests directory
find . -path "*/e2e-tests/*" -name "*cookie*" -type fRepository: hmcts/cath-service
Length of output: 145
🏁 Script executed:
# Search for ico.org.uk URL to see if it's expected anywhere
rg "ico.org.uk" --type tsRepository: hmcts/cath-service
Length of output: 213
🏁 Script executed:
# Get the exact line from cookie-policy.spec.ts to see full context
sed -n '225,235p' ./e2e-tests/tests/cookie-policy.spec.tsRepository: hmcts/cath-service
Length of output: 527
Update manageCookiesUrl to match E2E test expectations.
Both en.ts and cy.ts use https://www.aboutcookies.org/, but the E2E test expects https://ico.org.uk/for-the-public/online/cookies/. Update both locale files to use the correct URL.
🤖 Prompt for AI Agents
In libs/web-core/src/pages/cookies-policy/cy.ts around lines 13 to 16, the
manageCookiesUrl points to https://www.aboutcookies.org/ but the E2E tests
expect https://ico.org.uk/for-the-public/online/cookies/; update
manageCookiesUrl in this file to the ICO URL, and also update the same key in
the corresponding libs/web-core/src/pages/cookies-policy/en.ts to use
https://ico.org.uk/for-the-public/online/cookies/ so both locales match test
expectations.
| doNotUseAnalytics: "Do not use cookies that measure my website use", | ||
| performanceLegend: "Allow cookies that measure website application performance monitoring?", | ||
| usePerformance: "Use cookies that measure website application performance monitoring", | ||
| doNotUsePerformance: "Do not cookies that measure website application performance monitoring", |
There was a problem hiding this comment.
Fix typo: missing "use" in doNotUsePerformance text.
Line 102 has a grammatical error: "Do not cookies that measure..." should be "Do not use cookies that measure..."
Apply this diff:
- doNotUsePerformance: "Do not cookies that measure website application performance monitoring",
+ doNotUsePerformance: "Do not use cookies that measure website application performance monitoring",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| doNotUsePerformance: "Do not cookies that measure website application performance monitoring", | |
| doNotUsePerformance: "Do not use cookies that measure website application performance monitoring", |
🤖 Prompt for AI Agents
In libs/web-core/src/pages/cookies-policy/en.ts around line 102, the string for
doNotUsePerformance contains a typo: "Do not cookies that measure website
application performance monitoring"; update the text to read "Do not use cookies
that measure website application performance monitoring" so the sentence is
grammatically correct.
| href: "/cookies-policy", | ||
| text: footer.cookies, | ||
| attributes: { | ||
| target: "_blank", | ||
| rel: "noopener noreferrer", | ||
| "aria-label": "Cookie policy (opens in new tab)" | ||
| } |
There was a problem hiding this comment.
Fix hardcoded English aria-label for bilingual support.
The aria-label attribute is hardcoded in English and will not switch to Welsh when the user toggles language. This violates the bilingual requirement.
Based on learnings, every page must support both English and Welsh content.
Apply this diff to use a locale-aware aria-label:
{
href: "/cookies-policy",
text: footer.cookies,
attributes: {
target: "_blank",
rel: "noopener noreferrer",
- "aria-label": "Cookie policy (opens in new tab)"
+ "aria-label": footer.cookiesAriaLabel
}
},Then add the corresponding locale strings to libs/web-core/src/locales/en.ts and cy.ts:
// en.ts
footer: {
// ...existing footer strings
cookiesAriaLabel: "Cookie policy (opens in new tab)"
}
// cy.ts
footer: {
// ...existing footer strings
cookiesAriaLabel: "Polisi cwcis (yn agor mewn tab newydd)"
}🤖 Prompt for AI Agents
In libs/web-core/src/views/components/site-footer.njk around lines 16 to 22, the
aria-label for the cookies link is hardcoded in English; replace the hardcoded
string with a locale-aware property (e.g. use the footer.cookiesAriaLabel value)
so the label changes with language, and then add cookiesAriaLabel entries to
libs/web-core/src/locales/en.ts and libs/web-core/src/locales/cy.ts under the
footer block with the English and Welsh texts shown in the review so the
template can render the localized aria-label.
🎭 Playwright E2E Test Results169 tests 169 ✅ 10m 37s ⏱️ Results for commit b0a29d8. ♻️ This comment has been updated with latest results. |
…h-service into feature/VIBE-241-cookie-policy
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/tickets/VIBE-241/review.md (1)
151-199: Path references use absolute workspace paths instead of repository-relative paths.Lines 151 and 199 reference
/workspaces/cath-service3/docs/...which appears to be a local development environment path. Consider using repository-relative paths (e.g.,docs/tickets/VIBE-241/specification.md) for portability.e2e-tests/tests/cookie-policy.spec.ts (1)
124-146: ReplacewaitForTimeoutwith deterministic waits to avoid flaky tests.Using
waitForTimeout(lines 131, 141) is an anti-pattern in Playwright that can lead to flaky tests. UsewaitForFunctionto wait for the actual scroll position instead.// Scroll down the page await page.evaluate(() => window.scrollTo(0, 1000)); - // Wait a moment for scroll to complete - await page.waitForTimeout(100); - - // Verify we scrolled - let scrollY = await page.evaluate(() => window.scrollY); - expect(scrollY).toBeGreaterThan(500); + // Wait for scroll to complete + await page.waitForFunction(() => window.scrollY > 500); // Click back to top link await page.locator('a:has-text("Back to top")').click(); - // Wait for scroll animation - await page.waitForTimeout(300); - - // Verify we're at top - scrollY = await page.evaluate(() => window.scrollY); - expect(scrollY).toBeLessThan(100); + // Wait for scroll to top + await page.waitForFunction(() => window.scrollY < 100);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/tickets/VIBE-241/review.md(1 hunks)e2e-tests/tests/cookie-policy.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and interfaces must use PascalCase (e.g.,UserService,CaseRepository). DO NOT useIprefix for interfaces.
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: constants at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
Use workspace aliases (@hmcts/*) for imports instead of relative paths across packages.
Database queries must be parameterized using Prisma (never raw SQL with string concatenation).
Never put sensitive data in logs.
Don't add comments unless meaningful - explain why something is done, not what is done.
Favour functional style - use simple functions. Don't use a class unless you have shared state.
Data should be immutable by default - use const and avoid mutations to ensure predictable state.
Functions should have no side effects - avoid modifying external state or relying on mutable data.
Files:
e2e-tests/tests/cookie-policy.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Files and directories must use kebab-case (e.g.,user-service.ts,case-management/).
Never use CommonJS (require(),module.exports). Use ES modules (import/export) exclusively.
Files:
e2e-tests/tests/cookie-policy.spec.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source files using the pattern
*.test.tsor*.spec.tsand use Vitest.
Files:
e2e-tests/tests/cookie-policy.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
Applied to files:
e2e-tests/tests/cookie-policy.spec.ts
🪛 LanguageTool
docs/tickets/VIBE-241/review.md
[uncategorized] ~9-~9: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... resolved during implementation. ##
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~99-~99: Use a hyphen to join words.
Context: ...erences persist across page refreshes (1 year expiry) - ✅ Integration with existi...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarQube Analysis
- GitHub Check: E2E Tests
🔇 Additional comments (5)
docs/tickets/VIBE-241/review.md (1)
1-281: Comprehensive review documentation for the Cookie Policy feature.The review document thoroughly covers all aspects of the implementation including test coverage, accessibility, security practices, and acceptance criteria verification. The bilingual support with Welsh language testing via
?lng=cyis properly documented.e2e-tests/tests/cookie-policy.spec.ts (4)
380-394: Browser zoom simulation approach is a reasonable approximation.Setting viewport to 640x480 approximates 200% zoom behavior for responsive layout testing, but note this tests responsive breakpoints rather than actual browser zoom rendering. True zoom testing would require manual verification or browser-specific APIs.
Consider adding a comment to clarify this tests responsive layout at smaller viewport rather than true browser zoom behavior.
88-99: Welsh language route and content testing is properly implemented.The test correctly verifies the Welsh redirect from
/polisi-cwcisto/cookies-policy?lng=cyand checks Welsh content is displayed. This aligns with the requirement that Welsh content must be tested with?lng=cyquery parameter. Based on learnings.
274-378: Excellent accessibility test coverage with Axe and structural validation.The accessibility tests comprehensively cover:
- WCAG 2.0, 2.1, and 2.2 AA automated scanning
- Heading hierarchy verification
- Form control label associations
- Table header presence
- Fieldset/legend structure for radio groups
- Welsh language accessibility parity
1-3: LGTM!Imports are correctly structured using ES modules with Playwright Test and Axe accessibility testing library.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
e2e-tests/tests/cookie-policy.spec.ts (1)
370-378: Use the dedicated Welsh route for consistency.The test navigates to
/cookies-policy?lng=cy, but other Welsh tests in this file (line 88-99, 198-209) correctly use the dedicated/polisi-cwcisroute. For consistency with the codebase pattern and the learnings guidance, use the dedicated Welsh route here as well.Apply this diff:
- await page.goto("/cookies-policy?lng=cy"); + await page.goto("/polisi-cwcis");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
e2e-tests/tests/cookie-policy.spec.ts(1 hunks)libs/web-core/src/pages/cookies-policy/polisi-cwcis.test.ts(1 hunks)libs/web-core/src/pages/cookies-policy/polisi-cwcis.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/web-core/src/pages/cookies-policy/polisi-cwcis.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and interfaces must use PascalCase (e.g.,UserService,CaseRepository). DO NOT useIprefix for interfaces.
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: constants at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
Use workspace aliases (@hmcts/*) for imports instead of relative paths across packages.
Database queries must be parameterized using Prisma (never raw SQL with string concatenation).
Never put sensitive data in logs.
Don't add comments unless meaningful - explain why something is done, not what is done.
Favour functional style - use simple functions. Don't use a class unless you have shared state.
Data should be immutable by default - use const and avoid mutations to ensure predictable state.
Functions should have no side effects - avoid modifying external state or relying on mutable data.
Files:
libs/web-core/src/pages/cookies-policy/polisi-cwcis.test.tse2e-tests/tests/cookie-policy.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Files and directories must use kebab-case (e.g.,user-service.ts,case-management/).
Never use CommonJS (require(),module.exports). Use ES modules (import/export) exclusively.
Files:
libs/web-core/src/pages/cookies-policy/polisi-cwcis.test.tse2e-tests/tests/cookie-policy.spec.ts
**/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/pages/**/*.ts: Page controllers must export namedGETand/orPOSTfunctions with Express Request and Response types.
Every page must support both English and Welsh with separateenandcycontent objects in the controller, and Welsh content must be tested with?lng=cyquery parameter.
Files:
libs/web-core/src/pages/cookies-policy/polisi-cwcis.test.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source files using the pattern
*.test.tsor*.spec.tsand use Vitest.
Files:
libs/web-core/src/pages/cookies-policy/polisi-cwcis.test.tse2e-tests/tests/cookie-policy.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
Applied to files:
libs/web-core/src/pages/cookies-policy/polisi-cwcis.test.tse2e-tests/tests/cookie-policy.spec.ts
🧬 Code graph analysis (1)
libs/web-core/src/pages/cookies-policy/polisi-cwcis.test.ts (1)
libs/web-core/src/pages/cookies-policy/polisi-cwcis.ts (2)
GET(7-18)POST(20-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarQube Analysis
- GitHub Check: E2E Tests
🔇 Additional comments (10)
libs/web-core/src/pages/cookies-policy/polisi-cwcis.test.ts (4)
1-11: LGTM!The imports follow ESM standards with
.jsextensions, and the mock setup for cookie helpers is appropriate for isolating the controller logic.
13-40: LGTM!The test setup properly mocks Express request/response objects with all necessary properties, includes realistic cookie categories, and correctly clears mocks between tests.
42-91: LGTM!The GET handler tests comprehensively verify Welsh language rendering, cookie preference parsing, and success banner display logic. All assertions correctly check for Welsh language context (
lng: "cy").
93-126: LGTM!The POST handler tests correctly verify preference saving for both enabled and disabled states, proper cookie setting, banner dismissal, and redirect behavior to the Welsh route. The expected preference objects match the controller implementation logic.
e2e-tests/tests/cookie-policy.spec.ts (6)
1-27: LGTM!The imports are correct, and the initial page rendering test comprehensively verifies all major sections and headings of the cookie policy page.
29-90: LGTM!The tests thoroughly verify cookie table content, preference saving flow, success banner display, and preference persistence after save.
88-169: LGTM!The Welsh route test correctly uses
/polisi-cwcis(as per learnings), and the interaction tests properly verify expandable details, scroll behavior, keyboard navigation, and form defaults.
171-244: LGTM!These tests properly verify ARIA structure, success banner conditional display, Welsh language persistence (correctly using
/polisi-cwcis), layout expectations, and secure external link attributes.
247-272: LGTM!The footer integration tests properly verify the presence, attributes, and new-tab behavior of the Cookies link with appropriate security attributes.
274-369: LGTM!The accessibility tests are comprehensive, covering WCAG compliance with Axe, heading hierarchy, form control labeling, table accessibility, fieldset/legend structure, and responsive behavior under zoom conditions.
Also applies to: 380-395
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
e2e-tests/tests/cookie-policy.spec.ts (3)
374-380: Add explicit Welsh content assertion for?lng=cyrouteYou already run Axe on
/cookies-policy?lng=cy, which is good, but per the bilingual requirement it would be useful to assert that Welsh content actually renders (e.g. heading text) to catch translation regressions, not just accessibility ones.For example:
await page.goto("/cookies-policy?lng=cy"); - const accessibilityScanResults = await new AxeBuilder({ page }).withTags(["wcag2a", "wcag2aa", "wcag21a", "wcag21aa", "wcag22aa"]).analyze(); + await expect(page.locator("h1")).toHaveText("Polisi Cwcis"); + + const accessibilityScanResults = await new AxeBuilder({ page }) + .withTags(["wcag2a", "wcag2aa", "wcag21a", "wcag21aa", "wcag22aa"]) + .analyze();Based on learnings, this strengthens the required
?lng=cyWelsh coverage.
233-247: Consider assertingrelon external_blanklinks for consistencyFor external links opened in a new tab (
how to manage cookies,Privacy Policy,opt out of Google Analytics), you only asserttarget="_blank". For security and consistency with the footer test, consider also assertingrel="noopener noreferrer"here once the markup includes it.Example:
- await expect(manageCookiesLink).toHaveAttribute("target", "_blank"); + await expect(manageCookiesLink).toHaveAttribute("target", "_blank"); + await expect(manageCookiesLink).toHaveAttribute("rel", "noopener noreferrer");Repeat similarly for the other external links.
101-122: UsetoHaveJSPropertyto assert the<details>element's runtime state
await expect(details).toHaveAttribute("open")checks attribute presence in the DOM, but when testing user interactions that toggle the<details>element open/closed, asserting the boolean property is more semantically correct and robust.Use
toHaveJSPropertyinstead:- // Details should be collapsed initially - const details = page.locator('details:has(summary:has-text("Contact us for help"))'); - await expect(details).not.toHaveAttribute("open"); + // Details should be collapsed initially + const details = page.locator('details:has(summary:has-text("Contact us for help"))'); + await expect(details).toHaveJSProperty("open", false); @@ - // Click to expand + // Click to expand await detailsSummary.click(); - await expect(details).toHaveAttribute("open"); + await expect(details).toHaveJSProperty("open", true); @@ - // Click to collapse + // Click to collapse await detailsSummary.click(); - await expect(details).not.toHaveAttribute("open"); + await expect(details).toHaveJSProperty("open", false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e-tests/tests/cookie-policy.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and interfaces must use PascalCase (e.g.,UserService,CaseRepository). DO NOT useIprefix for interfaces.
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: constants at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
Use workspace aliases (@hmcts/*) for imports instead of relative paths across packages.
Database queries must be parameterized using Prisma (never raw SQL with string concatenation).
Never put sensitive data in logs.
Don't add comments unless meaningful - explain why something is done, not what is done.
Favour functional style - use simple functions. Don't use a class unless you have shared state.
Data should be immutable by default - use const and avoid mutations to ensure predictable state.
Functions should have no side effects - avoid modifying external state or relying on mutable data.
Files:
e2e-tests/tests/cookie-policy.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Files and directories must use kebab-case (e.g.,user-service.ts,case-management/).
Never use CommonJS (require(),module.exports). Use ES modules (import/export) exclusively.
Files:
e2e-tests/tests/cookie-policy.spec.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source files using the pattern
*.test.tsor*.spec.tsand use Vitest.
Files:
e2e-tests/tests/cookie-policy.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
📚 Learning: 2025-12-01T11:31:12.342Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T11:31:12.342Z
Learning: Applies to **/pages/**/*.ts : Every page must support both English and Welsh with separate `en` and `cy` content objects in the controller, and Welsh content must be tested with `?lng=cy` query parameter.
Applied to files:
e2e-tests/tests/cookie-policy.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarQube Analysis
- GitHub Check: E2E Tests
🔇 Additional comments (1)
e2e-tests/tests/cookie-policy.spec.ts (1)
4-397: Strong, comprehensive coverage of cookie policy behaviour and accessibilityThe suite gives broad coverage: EN/CY routes, preference saving/persistence, footer integration, keyboard and scroll behaviour, structural a11y (radiogroups, fieldsets, legends, tables), Axe scans, and simple zoom resilience. Aside from the small issues noted elsewhere, this is a solid, maintainable e2e layer.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
libs/web-core/src/pages/cookies-policy/index.ts (1)
3-3: Consider using type-only import for CookiePreferences.Since
CookiePreferencesis only used as a type annotation, you can useimport typefor clarity and potentially better tree-shaking.-import type { Request, Response } from "express"; -import { parseCookiePolicy, setCookieBannerSeen, setCookiePolicy } from "../../middleware/cookies/cookie-helpers.js"; -import type { CookiePreferences } from "../../middleware/cookies/cookie-manager-middleware.js"; +import type { Request, Response } from "express"; +import { parseCookiePolicy, setCookieBannerSeen, setCookiePolicy } from "../../middleware/cookies/cookie-helpers.js"; +import type { CookiePreferences } from "../../middleware/cookies/cookie-manager-middleware.js";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
e2e-tests/tests/cookie-policy.spec.ts(1 hunks)libs/web-core/src/pages/cookies-policy/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e-tests/tests/cookie-policy.spec.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans must useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and interfaces must use PascalCase (e.g.,UserService,CaseRepository). DO NOT useIprefix for interfaces.
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: constants outside function scope at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom.
TypeScript strict mode must be enabled. Noanytype without justification. Use explicit types for all variables and function parameters.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"), even when importing TypeScript files. This is required for ESM with Node.js 'nodenext' module resolution.
Use workspace aliases (@hmcts/*) for imports between packages instead of relative paths.
Only export functions that are intended to be used outside the module. Do not export functions solely for testing purposes.
Only add comments when they provide meaningful explanation of why something is done, not what is done. Code should be self-documenting.
Favor functional style. Don't use classes unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Files:
libs/web-core/src/pages/cookies-policy/index.ts
**/src/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/pages/**/*.ts: Pages are registered through explicit imports inapps/web/src/app.ts. Routes are created based on file names within thepages/directory (e.g.,my-page.tsbecomes/my-page, nested routes via subdirectories).
Page controller files must exportGETand/orPOSTfunctions that accept Express Request and Response, render usingres.render(), and handle form submissions.
Files:
libs/web-core/src/pages/cookies-policy/index.ts
**/src/pages/**/*.{ts,njk}
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/pages/**/*.{ts,njk}: Every page must support both English and Welsh. Controllers must provide bothenandcyobjects with page content.
Welsh translations are required for all user-facing text. Do not skip Welsh support.
Files:
libs/web-core/src/pages/cookies-policy/index.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
DO NOT use CommonJS. Use
import/export, neverrequire()/module.exports. Only ES modules are allowed.
Files:
libs/web-core/src/pages/cookies-policy/index.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Do not create generictypes.tsfiles. Colocate types with the appropriate code file where they are used.
Do not create generic files likeutils.ts. Be specific with naming (e.g.,object-properties.ts,date-formatting.ts).
Files:
libs/web-core/src/pages/cookies-policy/index.ts
🧠 Learnings (1)
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to **/src/pages/**/*.ts : Page controller files must export `GET` and/or `POST` functions that accept Express Request and Response, render using `res.render()`, and handle form submissions.
Applied to files:
libs/web-core/src/pages/cookies-policy/index.ts
🔇 Additional comments (1)
libs/web-core/src/pages/cookies-policy/index.ts (1)
23-27: LGTM! Clean handling of cookie preferences.The logic correctly iterates over non-essential categories and handles both HTML checkbox values (
"on") and boolean values. The explicit skip of the "essential" category is appropriate since essential cookies cannot be disabled.
| en, | ||
| cy, | ||
| cookiePreferences: cookiePolicy, | ||
| categories: res.locals.cookieConfig?.categories, |
There was a problem hiding this comment.
Ensure consistent handling of undefined categories.
The GET handler passes res.locals.cookieConfig?.categories which could be undefined, while the POST handler (line 21) provides a fallback to an empty object. For consistency and to avoid potential template errors, consider adding the same fallback in the GET handler.
res.render("cookies-policy/index", {
en,
cy,
cookiePreferences: cookiePolicy,
- categories: res.locals.cookieConfig?.categories,
+ categories: res.locals.cookieConfig?.categories || {},
saved: req.query.saved === "true"
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| categories: res.locals.cookieConfig?.categories, | |
| res.render("cookies-policy/index", { | |
| en, | |
| cy, | |
| cookiePreferences: cookiePolicy, | |
| categories: res.locals.cookieConfig?.categories || {}, | |
| saved: req.query.saved === "true" | |
| }); |
🤖 Prompt for AI Agents
In libs/web-core/src/pages/cookies-policy/index.ts around line 14, the GET
handler passes res.locals.cookieConfig?.categories which can be undefined while
the POST handler uses a fallback to an empty object; update the GET handler to
provide the same fallback (e.g., res.locals.cookieConfig?.categories ?? {}) so
templates always receive an object and avoid runtime/template errors.
| const lng = req.query.lng; | ||
| const redirectUrl = lng ? `/cookies-policy?lng=${lng}&saved=true` : "/cookies-policy?saved=true"; | ||
| res.redirect(redirectUrl); |
There was a problem hiding this comment.
Validate lng query parameter to prevent malformed redirect URLs.
The req.query.lng parameter can be string | string[] | undefined in Express. If an array is passed (e.g., ?lng=en&lng=cy), the redirect URL will be malformed. Validate that lng is a string and optionally check it against valid locale codes.
setCookiePolicy(res, preferences);
setCookieBannerSeen(res);
- const lng = req.query.lng;
- const redirectUrl = lng ? `/cookies-policy?lng=${lng}&saved=true` : "/cookies-policy?saved=true";
+ const lng = typeof req.query.lng === "string" ? req.query.lng : undefined;
+ const redirectUrl = lng ? `/cookies-policy?lng=${encodeURIComponent(lng)}&saved=true` : "/cookies-policy?saved=true";
res.redirect(redirectUrl);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const lng = req.query.lng; | |
| const redirectUrl = lng ? `/cookies-policy?lng=${lng}&saved=true` : "/cookies-policy?saved=true"; | |
| res.redirect(redirectUrl); | |
| const lng = typeof req.query.lng === "string" ? req.query.lng : undefined; | |
| const redirectUrl = lng ? `/cookies-policy?lng=${encodeURIComponent(lng)}&saved=true` : "/cookies-policy?saved=true"; | |
| res.redirect(redirectUrl); |
🤖 Prompt for AI Agents
In libs/web-core/src/pages/cookies-policy/index.ts around lines 32 to 34,
validate req.query.lng before building the redirect URL: check that lng is a
string (not string[]), optionally validate it against your allowed locale list
(e.g., ['en','cy',...]) and only include it in the query when valid; if it's an
array or invalid, ignore it or pick the first valid value; also URL-encode the
value when appending to the redirect URL and fall back to the safe default
"/cookies-policy?saved=true" when no valid locale is present.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/web-core/src/pages/cookies-policy/index.ts (1)
14-14: Ensure consistent handling of undefined categories.The GET handler passes
res.locals.cookieConfig?.categorieswhich could be undefined, while the POST handler (line 21) provides a fallback to an empty object. For consistency and to avoid potential template errors, consider adding the same fallback in the GET handler.Apply this diff:
res.render("cookies-policy/index", { en, cy, cookiePreferences: cookiePolicy, - categories: res.locals.cookieConfig?.categories, + categories: res.locals.cookieConfig?.categories || {}, saved: req.query.saved === "true" });
🧹 Nitpick comments (2)
libs/web-core/src/pages/cookies-policy/index.ts (2)
7-7: Remove unnecessaryasynckeywords.Both the GET and POST handlers are marked as
asyncbut contain noawaitstatements. This adds unnecessary overhead and can be misleading.Apply this diff:
-export const GET = async (req: Request, res: Response) => { +export const GET = (req: Request, res: Response) => { const cookiePolicy = parseCookiePolicy(req.cookies?.cookie_policy); res.render("cookies-policy/index", {-export const POST = async (req: Request, res: Response) => { +export const POST = (req: Request, res: Response) => { const preferences: CookiePreferences = {}; const categories = res.locals.cookieConfig?.categories || {};Also applies to: 19-19
33-33: URL-encode locale parameter for safety.The
res.locals.localevalue is inserted directly into the redirect URL without encoding. While this is likely safe if set by trusted middleware, URL encoding is best practice to prevent potential injection issues.Apply this diff:
const locale = res.locals.locale; - const redirectUrl = locale && locale !== "en" ? `/cookies-policy?lng=${locale}&saved=true` : "/cookies-policy?saved=true"; + const redirectUrl = locale && locale !== "en" ? `/cookies-policy?lng=${encodeURIComponent(locale)}&saved=true` : "/cookies-policy?saved=true"; res.redirect(redirectUrl);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
e2e-tests/tests/cookie-policy.spec.ts(1 hunks)e2e-tests/tests/page-structure.spec.ts(3 hunks)libs/web-core/src/pages/cookies-policy/index.test.ts(1 hunks)libs/web-core/src/pages/cookies-policy/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e-tests/tests/page-structure.spec.ts
- e2e-tests/tests/cookie-policy.spec.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans must useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and interfaces must use PascalCase (e.g.,UserService,CaseRepository). DO NOT useIprefix for interfaces.
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: constants outside function scope at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom.
TypeScript strict mode must be enabled. Noanytype without justification. Use explicit types for all variables and function parameters.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"), even when importing TypeScript files. This is required for ESM with Node.js 'nodenext' module resolution.
Use workspace aliases (@hmcts/*) for imports between packages instead of relative paths.
Only export functions that are intended to be used outside the module. Do not export functions solely for testing purposes.
Only add comments when they provide meaningful explanation of why something is done, not what is done. Code should be self-documenting.
Favor functional style. Don't use classes unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Files:
libs/web-core/src/pages/cookies-policy/index.tslibs/web-core/src/pages/cookies-policy/index.test.ts
**/src/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/pages/**/*.ts: Pages are registered through explicit imports inapps/web/src/app.ts. Routes are created based on file names within thepages/directory (e.g.,my-page.tsbecomes/my-page, nested routes via subdirectories).
Page controller files must exportGETand/orPOSTfunctions that accept Express Request and Response, render usingres.render(), and handle form submissions.
Files:
libs/web-core/src/pages/cookies-policy/index.tslibs/web-core/src/pages/cookies-policy/index.test.ts
**/src/pages/**/*.{ts,njk}
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/pages/**/*.{ts,njk}: Every page must support both English and Welsh. Controllers must provide bothenandcyobjects with page content.
Welsh translations are required for all user-facing text. Do not skip Welsh support.
Files:
libs/web-core/src/pages/cookies-policy/index.tslibs/web-core/src/pages/cookies-policy/index.test.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
DO NOT use CommonJS. Use
import/export, neverrequire()/module.exports. Only ES modules are allowed.
Files:
libs/web-core/src/pages/cookies-policy/index.tslibs/web-core/src/pages/cookies-policy/index.test.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Do not create generictypes.tsfiles. Colocate types with the appropriate code file where they are used.
Do not create generic files likeutils.ts. Be specific with naming (e.g.,object-properties.ts,date-formatting.ts).
Files:
libs/web-core/src/pages/cookies-policy/index.tslibs/web-core/src/pages/cookies-policy/index.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Unit/integration test files must be co-located with source files as
*.test.tsand use Vitest withdescribe,it, andexpect.
Files:
libs/web-core/src/pages/cookies-policy/index.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to **/src/pages/**/*.ts : Page controller files must export `GET` and/or `POST` functions that accept Express Request and Response, render using `res.render()`, and handle form submissions.
Applied to files:
libs/web-core/src/pages/cookies-policy/index.tslibs/web-core/src/pages/cookies-policy/index.test.ts
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to e2e-tests/**/*.spec.ts : E2E test files must be in `e2e-tests/` directory named `*.spec.ts`, use Playwright, include complete user journeys with validations, Welsh translations, accessibility checks, and keyboard navigation all within a single test.
Applied to files:
libs/web-core/src/pages/cookies-policy/index.test.ts
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to **/*.test.ts : Unit/integration test files must be co-located with source files as `*.test.ts` and use Vitest with `describe`, `it`, and `expect`.
Applied to files:
libs/web-core/src/pages/cookies-policy/index.test.ts
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to e2e-tests/**/*.spec.ts : WCAG 2.2 AA accessibility compliance is mandatory. Include accessibility testing in E2E tests using Axe-core.
Applied to files:
libs/web-core/src/pages/cookies-policy/index.test.ts
🧬 Code graph analysis (2)
libs/web-core/src/pages/cookies-policy/index.ts (1)
libs/web-core/src/middleware/cookies/cookie-helpers.ts (3)
parseCookiePolicy(7-17)setCookiePolicy(19-28)setCookieBannerSeen(30-37)
libs/web-core/src/pages/cookies-policy/index.test.ts (1)
libs/web-core/src/pages/cookies-policy/index.ts (2)
GET(7-17)POST(19-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarQube Analysis
- GitHub Check: E2E Tests
🔇 Additional comments (1)
libs/web-core/src/pages/cookies-policy/index.test.ts (1)
1-230: LGTM - Comprehensive test coverage!The test suite is well-structured with thorough coverage of both GET and POST handlers. It properly tests cookie preference parsing, saving, locale handling, edge cases (missing config, essential cookies exclusion), and redirect behavior. The mock setup is appropriate and assertions verify the expected behavior accurately.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
libs/web-core/src/views/components/site-footer.njk (1)
16-22: Fix hardcoded English aria-label for bilingual support.The
aria-labelattribute is hardcoded in English and will not switch to Welsh when the user toggles language. This violates the bilingual requirement.Based on learnings, every page must support both English and Welsh content.
libs/web-core/src/pages/cookie-policy/index.ts (1)
7-17: Ensure consistent handling of undefined categories.The GET handler passes
res.locals.cookieConfig?.categorieswhich could be undefined, while the POST handler (line 21) provides a fallback to an empty object. For consistency and to avoid potential template errors, consider adding the same fallback in the GET handler.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
e2e-tests/tests/cookie-policy.spec.ts(1 hunks)e2e-tests/tests/page-structure.spec.ts(3 hunks)libs/web-core/src/pages/cookie-policy/cy.ts(1 hunks)libs/web-core/src/pages/cookie-policy/en.ts(1 hunks)libs/web-core/src/pages/cookie-policy/index.njk(1 hunks)libs/web-core/src/pages/cookie-policy/index.test.ts(1 hunks)libs/web-core/src/pages/cookie-policy/index.ts(1 hunks)libs/web-core/src/views/components/site-footer.njk(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/web-core/src/pages/cookie-policy/index.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans must useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and interfaces must use PascalCase (e.g.,UserService,CaseRepository). DO NOT useIprefix for interfaces.
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: constants outside function scope at the top, exported functions next, other functions ordered by usage, interfaces and types at the bottom.
TypeScript strict mode must be enabled. Noanytype without justification. Use explicit types for all variables and function parameters.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"), even when importing TypeScript files. This is required for ESM with Node.js 'nodenext' module resolution.
Use workspace aliases (@hmcts/*) for imports between packages instead of relative paths.
Only export functions that are intended to be used outside the module. Do not export functions solely for testing purposes.
Only add comments when they provide meaningful explanation of why something is done, not what is done. Code should be self-documenting.
Favor functional style. Don't use classes unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Files:
libs/web-core/src/pages/cookie-policy/cy.tslibs/web-core/src/pages/cookie-policy/en.tse2e-tests/tests/page-structure.spec.tslibs/web-core/src/pages/cookie-policy/index.tse2e-tests/tests/cookie-policy.spec.ts
**/src/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/pages/**/*.ts: Pages are registered through explicit imports inapps/web/src/app.ts. Routes are created based on file names within thepages/directory (e.g.,my-page.tsbecomes/my-page, nested routes via subdirectories).
Page controller files must exportGETand/orPOSTfunctions that accept Express Request and Response, render usingres.render(), and handle form submissions.
Files:
libs/web-core/src/pages/cookie-policy/cy.tslibs/web-core/src/pages/cookie-policy/en.tslibs/web-core/src/pages/cookie-policy/index.ts
**/src/pages/**/*.{ts,njk}
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/pages/**/*.{ts,njk}: Every page must support both English and Welsh. Controllers must provide bothenandcyobjects with page content.
Welsh translations are required for all user-facing text. Do not skip Welsh support.
Files:
libs/web-core/src/pages/cookie-policy/cy.tslibs/web-core/src/pages/cookie-policy/en.tslibs/web-core/src/pages/cookie-policy/index.tslibs/web-core/src/pages/cookie-policy/index.njk
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
DO NOT use CommonJS. Use
import/export, neverrequire()/module.exports. Only ES modules are allowed.
Files:
libs/web-core/src/pages/cookie-policy/cy.tslibs/web-core/src/pages/cookie-policy/en.tse2e-tests/tests/page-structure.spec.tslibs/web-core/src/pages/cookie-policy/index.tse2e-tests/tests/cookie-policy.spec.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Do not create generictypes.tsfiles. Colocate types with the appropriate code file where they are used.
Do not create generic files likeutils.ts. Be specific with naming (e.g.,object-properties.ts,date-formatting.ts).
Files:
libs/web-core/src/pages/cookie-policy/cy.tslibs/web-core/src/pages/cookie-policy/en.tse2e-tests/tests/page-structure.spec.tslibs/web-core/src/pages/cookie-policy/index.tse2e-tests/tests/cookie-policy.spec.ts
e2e-tests/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
e2e-tests/**/*.spec.ts: E2E test files must be ine2e-tests/directory named*.spec.ts, use Playwright, include complete user journeys with validations, Welsh translations, accessibility checks, and keyboard navigation all within a single test.
WCAG 2.2 AA accessibility compliance is mandatory. Include accessibility testing in E2E tests using Axe-core.
Files:
e2e-tests/tests/page-structure.spec.tse2e-tests/tests/cookie-policy.spec.ts
**/src/pages/**/*.njk
📄 CodeRabbit inference engine (CLAUDE.md)
Nunjucks templates must extend
layouts/base-templates.njk, use govuk macros for components, include error summaries, and support conditional rendering based on language variables.
Files:
libs/web-core/src/pages/cookie-policy/index.njk
🧠 Learnings (8)
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to **/src/pages/**/*.{ts,njk} : Welsh translations are required for all user-facing text. Do not skip Welsh support.
Applied to files:
libs/web-core/src/pages/cookie-policy/cy.tslibs/web-core/src/pages/cookie-policy/en.tse2e-tests/tests/page-structure.spec.tslibs/web-core/src/views/components/site-footer.njke2e-tests/tests/cookie-policy.spec.ts
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to **/src/pages/**/*.{ts,njk} : Every page must support both English and Welsh. Controllers must provide both `en` and `cy` objects with page content.
Applied to files:
libs/web-core/src/pages/cookie-policy/cy.tslibs/web-core/src/pages/cookie-policy/en.tse2e-tests/tests/page-structure.spec.tslibs/web-core/src/views/components/site-footer.njke2e-tests/tests/cookie-policy.spec.ts
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to e2e-tests/**/*.spec.ts : E2E test files must be in `e2e-tests/` directory named `*.spec.ts`, use Playwright, include complete user journeys with validations, Welsh translations, accessibility checks, and keyboard navigation all within a single test.
Applied to files:
e2e-tests/tests/page-structure.spec.tse2e-tests/tests/cookie-policy.spec.ts
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to e2e-tests/**/*.spec.ts : WCAG 2.2 AA accessibility compliance is mandatory. Include accessibility testing in E2E tests using Axe-core.
Applied to files:
e2e-tests/tests/page-structure.spec.tse2e-tests/tests/cookie-policy.spec.ts
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to **/src/pages/**/*.ts : Page controller files must export `GET` and/or `POST` functions that accept Express Request and Response, render using `res.render()`, and handle form submissions.
Applied to files:
libs/web-core/src/pages/cookie-policy/index.ts
📚 Learning: 2025-12-03T13:55:34.702Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T13:55:34.702Z
Learning: Applies to **/src/pages/**/*.njk : Nunjucks templates must extend `layouts/base-templates.njk`, use govuk macros for components, include error summaries, and support conditional rendering based on language variables.
Applied to files:
libs/web-core/src/pages/cookie-policy/index.njk
📚 Learning: 2025-11-27T14:18:22.932Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 137
File: e2e-tests/tests/create-media-account.spec.ts:51-64
Timestamp: 2025-11-27T14:18:22.932Z
Learning: For the create-media-account form in libs/public-pages, the English email validation error message (errorEmailInvalid) should be: "There is a problem - Enter a valid email address, e.g. nameexample.com" to match the Welsh translation and clearly indicate the format requirement rather than suggesting the field is empty.
Applied to files:
libs/web-core/src/views/components/site-footer.njk
📚 Learning: 2025-11-20T09:59:16.776Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 106
File: libs/system-admin-pages/src/pages/reference-data-upload/index.test.ts:84-160
Timestamp: 2025-11-20T09:59:16.776Z
Learning: In the cath-service repository, Welsh localization (lng=cy) is not required for admin screens (system-admin-pages), so locale preservation in admin screen redirects is not necessary.
Applied to files:
libs/web-core/src/views/components/site-footer.njk
🧬 Code graph analysis (2)
libs/web-core/src/pages/cookie-policy/index.ts (1)
libs/web-core/src/middleware/cookies/cookie-helpers.ts (3)
parseCookiePolicy(7-17)setCookiePolicy(19-28)setCookieBannerSeen(30-37)
e2e-tests/tests/cookie-policy.spec.ts (1)
e2e-tests/tests/cookie-management.spec.ts (1)
test(112-246)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarQube Analysis
- GitHub Check: E2E Tests
🔇 Additional comments (6)
libs/web-core/src/pages/cookie-policy/cy.ts (1)
1-130: LGTM!The Welsh locale file is well-structured with comprehensive translations covering all sections: intro, main heading, sections (analytics, introMessage, session, authentication, security, performance), table headers, change settings, contact information, and success messages. The structure mirrors the English locale appropriately.
e2e-tests/tests/page-structure.spec.ts (2)
78-107: LGTM!The test updates correctly separate footer links that open in the same tab from the Cookies link that opens in a new tab. The verification includes proper security attributes (
target="_blank"andrel="noopener noreferrer").
165-166: LGTM!The Welsh footer link verification correctly references the updated cookie policy path and uses precise text matching for the Welsh translation.
libs/web-core/src/pages/cookie-policy/index.ts (1)
19-35: LGTM!The POST handler correctly builds cookie preferences from form submission, excludes essential cookies from user control, persists preferences, and redirects with locale preservation and a success flag.
libs/web-core/src/pages/cookie-policy/index.njk (1)
1-206: LGTM!The template is well-structured with proper bilingual support, accessibility attributes, GOV.UK component usage, and CSRF protection. Key highlights:
- Alternate language links for SEO (hreflang)
- Conditional success banner rendering
- Properly structured form with fieldsets, legends, and radio groups
- CSRF token included
- Accessible back-to-top link with proper ARIA labels
e2e-tests/tests/cookie-policy.spec.ts (1)
1-396: LGTM!Comprehensive E2E test suite with excellent coverage:
- Page structure and content rendering (English and Welsh)
- Cookie preference form interactions and persistence
- Language persistence across user actions
- Footer integration and new tab behavior
- Accessibility compliance (WCAG 2.2 AA) using Axe
- Form accessibility (labels, fieldsets, legends)
- Table accessibility
- Keyboard navigation
- Responsive design and browser zoom support
The tests align with coding guidelines for E2E tests requiring complete user journeys, Welsh translations, accessibility checks, and keyboard navigation.
junaidiqbalmoj
left a comment
junaidiqbalmoj
left a comment
There was a problem hiding this comment.
There are extra .md files in this PR:
.ai/plans/VIBE-241
.jira
docs/VIBE-241
VIBE-241-plan.md
VIBE-241-specification.md
Remove all these files.
.md files only need in docs/tickets/VIBE-241/ folder
|
3 participants
Jira link
https://tools.hmcts.net/jira/browse/VIBE-241
Change description
Testing done
Security Vulnerability Assessment
CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?
Checklist
Summary by CodeRabbit
New Features
Privacy & Consent
UI
Tests & Docs
✏️ Tip: You can customize this high-level summary in your review settings.