-
Notifications
You must be signed in to change notification settings - Fork 2
VIBE-175 Bug Fix in Create Media Application About Error Messages #164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces granular form validation error messages for the create-media-account feature, replacing generic error keys with specific ones for blank fields, whitespace issues, and format validation. Updates include new locale strings in English and Welsh, enhanced validation logic with helper functions, and comprehensive test coverage for the expanded validation scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/public-pages/src/pages/validation.test.ts (2)
46-65: Incorrect error message for length validation.The test expects
errorFullNameBlank("Full name field must be populated") when the name exceeds 100 characters. This message is misleading since the field is populated - it's just too long. The validation should return a distinct error likeerrorFullNameTooLongwith a message indicating the length constraint.Same issue applies to the invalid characters test on line 84.
- expect(errors[0].text).toBe(en.errorFullNameBlank); + // Consider adding a distinct error key for length validation: + // expect(errors[0].text).toBe(en.errorFullNameTooLong);
255-274: Similar issue: using blank error for length constraint.Like the full name case,
errorEmployerBlankis used when employer exceeds 120 characters. This is semantically incorrect - the field is populated, just too long.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
libs/public-pages/src/pages/create-media-account/cy.ts(1 hunks)libs/public-pages/src/pages/create-media-account/en.ts(1 hunks)libs/public-pages/src/pages/create-media-account/index.njk.test.ts(3 hunks)libs/public-pages/src/pages/validation.test.ts(7 hunks)libs/public-pages/src/pages/validation.ts(3 hunks)
🧰 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/public-pages/src/pages/validation.tslibs/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/create-media-account/cy.tslibs/public-pages/src/pages/validation.test.tslibs/public-pages/src/pages/create-media-account/en.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/public-pages/src/pages/validation.tslibs/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/create-media-account/cy.tslibs/public-pages/src/pages/validation.test.tslibs/public-pages/src/pages/create-media-account/en.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/public-pages/src/pages/validation.tslibs/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/create-media-account/cy.tslibs/public-pages/src/pages/validation.test.tslibs/public-pages/src/pages/create-media-account/en.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/public-pages/src/pages/validation.tslibs/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/create-media-account/cy.tslibs/public-pages/src/pages/validation.test.tslibs/public-pages/src/pages/create-media-account/en.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/public-pages/src/pages/validation.tslibs/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/create-media-account/cy.tslibs/public-pages/src/pages/validation.test.tslibs/public-pages/src/pages/create-media-account/en.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/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/validation.test.ts
🧠 Learnings (5)
📓 Common learnings
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.
📚 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/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/create-media-account/cy.tslibs/public-pages/src/pages/validation.test.tslibs/public-pages/src/pages/create-media-account/en.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} : Welsh translations are required for all user-facing text. Do not skip Welsh support.
Applied to files:
libs/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/create-media-account/cy.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/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/create-media-account/cy.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/public-pages/src/pages/create-media-account/index.njk.test.tslibs/public-pages/src/pages/validation.test.ts
🧬 Code graph analysis (1)
libs/public-pages/src/pages/create-media-account/index.njk.test.ts (2)
libs/public-pages/src/pages/create-media-account/en.ts (1)
en(1-36)libs/public-pages/src/pages/create-media-account/cy.ts (1)
cy(1-36)
⏰ 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 (11)
libs/public-pages/src/pages/validation.test.ts (3)
87-148: Well-structured whitespace validation tests.The new test cases for full name whitespace scenarios (leading space, double spaces, missing surname space) are comprehensive and properly validate both the error message and the target field href.
171-232: Good coverage for email validation edge cases.The new tests properly cover email whitespace scenarios (leading space, double spaces) and blank email handling with appropriate error messages and field references.
318-344: File validation tests look good.The distinction between
errorFileBlank(missing file) anderrorFileType(wrong file format) appropriately separates these error conditions.libs/public-pages/src/pages/validation.ts (3)
15-21: Clean helper predicates.The
hasDoubleWhiteSpaceandstartsWithWhiteSpacefunctions are well-implemented, pure functions with clear semantics.
62-83: Email validation logic is well-structured.The validation chain properly separates blank, whitespace, and format errors with distinct messages for each condition.
119-126: File extension validation looks correct.The extension extraction handles edge cases appropriately - files without extensions or with unusual names will fail validation as expected since they won't match the allowed extensions list.
libs/public-pages/src/pages/create-media-account/index.njk.test.ts (3)
64-80: Comprehensive English error message assertions.The updated assertions cover the full range of granular validation error messages, ensuring the English locale contains all expected error keys with correct text.
127-143: Welsh translations properly verified.The Welsh error message assertions ensure bilingual support is complete, with all new granular error keys having corresponding Welsh translations.
146-189: Locale consistency validation is thorough.The test verifies that both English and Welsh locales have identical key sets, which is essential for preventing missing translation issues at runtime. As per coding guidelines, every page must support both English and Welsh.
libs/public-pages/src/pages/create-media-account/cy.ts (1)
21-35: Welsh translations are complete and consistent.All new granular error keys have corresponding Welsh translations. The key naming matches the English locale (
en.ts), ensuring consistency across both languages as required by coding guidelines.libs/public-pages/src/pages/create-media-account/en.ts (1)
21-35: Granular error messages improve user experience.The new error keys provide specific, actionable feedback for each validation failure. The email invalid message correctly uses the format guidance ("like [email protected]") as expected. Based on learnings, this matches the expected format for the create-media-account form.
🎭 Playwright E2E Test Results145 tests 145 ✅ 9m 49s ⏱️ Results for commit be10181. |



Jira link
https://tools.hmcts.net/jira/browse/VIBE-175
Change description
Create Media Application Error Messages Fix
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.