-
-
Notifications
You must be signed in to change notification settings - Fork 292
feat: surface API validation errors to registration form fields #1902
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: main
Are you sure you want to change the base?
Conversation
Add parseValidationErrors helper to extract field-specific errors from API validation responses. Update Register.vue to display validation errors directly below each form field instead of only showing 'invalid data' in the general error message box. Changes: - Add parseValidationErrors helper with unit tests - Add computed properties for combined client/server validation errors - Update template to use computed error properties - Clear server errors when user types in fields Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
- Extract inline keyup handlers into named functions - Add type guard for API validation error checking - Fix refs value access in keyup handlers Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
kolaente
left a 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.
@copilot please implement the requested changes
frontend/src/views/user/Register.vue
Outdated
| serverValidationErrors.value = fieldErrors | ||
| } else { | ||
| // Fallback to general error message if no field errors | ||
| errorMessage.value = e.message || 'Registration failed' |
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.
This general error should be localized and it needs to be a little more informative than just "registration failed"
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.
Updated in 8dfa927. The error message is now localized using the new translation key user.auth.registrationFailed with the text: "An error occurred during registration. Please check your input and try again."
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughA new validation error parsing utility is introduced with comprehensive tests, enabling structured handling of server-side field errors. The Register component integrates this utility to prioritize client-side validation while surfacing server-provided field-specific error messages. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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
♻️ Duplicate comments (1)
frontend/src/views/user/Register.vue (1)
269-273: Localize the fallback error messages.The hardcoded
'Registration failed'strings should uset()for internationalization.if (Object.keys(fieldErrors).length > 0) { // Apply field-level errors (computed properties will display them) serverValidationErrors.value = fieldErrors } else { // Fallback to general error message if no field errors - errorMessage.value = e.message || 'Registration failed' + errorMessage.value = e.message || t('user.auth.registrationFailed') } } else { - errorMessage.value = 'Registration failed' + errorMessage.value = t('user.auth.registrationFailed') }Note: You'll need to add the
user.auth.registrationFailedtranslation key to your i18n files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
frontend/src/helpers/parseValidationErrors.test.ts(1 hunks)frontend/src/helpers/parseValidationErrors.ts(1 hunks)frontend/src/views/user/Register.vue(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
frontend/**/*.{js,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{js,ts,tsx,vue}: Use single quotes in Vue and JavaScript/TypeScript code
Use trailing commas in Vue and JavaScript/TypeScript code
Do not use semicolons in Vue and JavaScript/TypeScript code
Use tab indentation in Vue and JavaScript/TypeScript code
Files:
frontend/src/helpers/parseValidationErrors.tsfrontend/src/helpers/parseValidationErrors.test.tsfrontend/src/views/user/Register.vue
frontend/src/helpers/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Create utility functions and business logic in
src/helpers/
Files:
frontend/src/helpers/parseValidationErrors.tsfrontend/src/helpers/parseValidationErrors.test.ts
frontend/src/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/**/*.vue: Use Vue 3 Composition API with TypeScript for component scripts
Use camelCase for Vue event names
Use<script lang="ts">in Vue components
Files:
frontend/src/views/user/Register.vue
frontend/src/views/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Create page-level components in
src/views/with proper routing
Files:
frontend/src/views/user/Register.vue
🧠 Learnings (2)
📚 Learning: 2025-11-27T21:25:45.975Z
Learnt from: CR
Repo: go-vikunja/vikunja PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T21:25:45.975Z
Learning: Update TypeScript interfaces in frontend `src/modelTypes/` to match backend model changes
Applied to files:
frontend/src/helpers/parseValidationErrors.ts
📚 Learning: 2025-11-27T21:25:45.975Z
Learnt from: CR
Repo: go-vikunja/vikunja PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T21:25:45.975Z
Learning: Applies to frontend/src/**/*.vue : Use camelCase for Vue event names
Applied to files:
frontend/src/views/user/Register.vue
🧬 Code graph analysis (1)
frontend/src/helpers/parseValidationErrors.test.ts (1)
frontend/src/helpers/parseValidationErrors.ts (1)
parseValidationErrors(21-44)
⏰ 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). (20)
- GitHub Check: Test / test-frontend-e2e-playwright (2, 6)
- GitHub Check: Test / test-frontend-e2e-playwright (1, 6)
- GitHub Check: Test / test-frontend-e2e-cypress
- GitHub Check: Test / test-migration-smoke (mysql)
- GitHub Check: Test / test-migration-smoke (postgres)
- GitHub Check: Test / test-migration-smoke (sqlite)
- GitHub Check: Test / test-api (paradedb, web)
- GitHub Check: Test / test-api (paradedb, feature)
- GitHub Check: Test / test-api (postgres, web)
- GitHub Check: Test / test-api (sqlite, feature)
- GitHub Check: Test / test-api (postgres, feature)
- GitHub Check: Test / test-api (sqlite, web)
- GitHub Check: Test / test-api (mysql, feature)
- GitHub Check: Test / test-api (mysql, web)
- GitHub Check: Test / test-api (sqlite-in-memory, web)
- GitHub Check: Test / test-api (sqlite-in-memory, feature)
- GitHub Check: Test / test-s3-integration
- GitHub Check: copilot
- GitHub Check: docker
- GitHub Check: docker
🔇 Additional comments (4)
frontend/src/helpers/parseValidationErrors.test.ts (1)
1-96: LGTM! Comprehensive test coverage for the validation error parser.The test suite thoroughly covers the expected behavior:
- Empty/missing
invalid_fields- Single and multiple field errors
- Edge cases (no colon, whitespace, null/undefined)
- Last-write-wins for duplicate fields
The tests align well with the implementation in
parseValidationErrors.tsand follow the coding guidelines (single quotes, no semicolons, trailing commas).frontend/src/helpers/parseValidationErrors.ts (1)
21-44: Clean implementation with proper edge-case handling.The function correctly:
- Guards against null/undefined/empty inputs
- Splits only on the first colon (preserving colons in messages)
- Trims whitespace from both field names and messages
- Gracefully skips malformed entries
frontend/src/views/user/Register.vue (2)
200-221: Well-structured computed error properties.The priority logic (client-side validation first, then server-side) is clear and consistent. Returning
nullwhen no error exists integrates cleanly with thev-ifdirectives in the template.
223-235: Clean keyup handlers with server error clearing.The handlers correctly:
- Only trigger validation after the first focusout (via
validateUsernameAfterFirst/validateEmailAfterFirst)- Clear corresponding server errors immediately on user input
| interface ValidationError { | ||
| message?: string | ||
| code?: number | ||
| invalid_fields?: string[] | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Consider exporting the ValidationError interface for reuse.
The Register.vue component defines a similar ApiValidationError interface. Exporting this interface would avoid duplication and provide a single source of truth for the API error shape.
-interface ValidationError {
+export interface ValidationError {
message?: string
code?: number
invalid_fields?: string[]
}📝 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.
| interface ValidationError { | |
| message?: string | |
| code?: number | |
| invalid_fields?: string[] | |
| } | |
| export interface ValidationError { | |
| message?: string | |
| code?: number | |
| invalid_fields?: string[] | |
| } |
🤖 Prompt for AI Agents
In frontend/src/helpers/parseValidationErrors.ts around lines 1 to 5, the
ValidationError interface is currently local; export it so other modules can
reuse it as the single source of truth. Change the declaration to an exported
interface (export interface ValidationError { ... }) and then update
Register.vue (and any other files) to import this interface from
frontend/src/helpers/parseValidationErrors.ts instead of redefining
ApiValidationError, ensuring the property names/types match the exported shape.
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.
@copilot In frontend/src/helpers/parseValidationErrors.ts around lines 1 to 5, the ValidationError interface is currently local; export it so other modules can reuse it as the single source of truth. Change the declaration to an exported interface (export interface ValidationError { ... }) and then update Register.vue (and any other files) to import this interface from frontend/src/helpers/parseValidationErrors.ts instead of redefining ApiValidationError, ensuring the property names/types match the exported shape.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| <p | ||
| v-if="passwordError" | ||
| class="help is-danger" | ||
| > | ||
| {{ passwordError }} | ||
| </p> |
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.
Missing keyup handler to clear password server errors.
The username and email fields clear their server-side errors when the user types (handleUsernameKeyup, handleEmailKeyup), but the Password component doesn't have similar behavior. Users won't see server-side password errors cleared until form resubmission.
Consider adding an @keyup or @update:modelValue handler to clear the password server error:
<Password
:validate-initially="validatePasswordInitially"
@submit="submit"
- @update:modelValue="v => credentials.password = v"
+ @update:modelValue="v => { credentials.password = v; delete serverValidationErrors.password }"
/>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/views/user/Register.vue around lines 80 to 85, the password
error paragraph is shown but there is no keyup/update handler to clear
server-side password errors; add a handler on the password input (e.g., @keyup
or @update:modelValue) that calls the same method used for username/email keyups
(or a new clearPasswordError method) to clear the passwordError reactive
property when the user types so server errors disappear immediately.
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.
@copilot the password does not need server validation, the length validation on the client is enough
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| interface ApiValidationError { | ||
| message?: string | ||
| invalid_fields?: string[] | ||
| } | ||
| function isApiValidationError(error: unknown): error is ApiValidationError { | ||
| return error !== null && | ||
| typeof error === 'object' && | ||
| ('message' in error || 'invalid_fields' in error) | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Duplicate interface and loose type guard.
-
ApiValidationErrorduplicatesValidationErrorfromparseValidationErrors.ts. Consider importing/exporting that interface instead. -
The type guard accepts any object with
messageorinvalid_fields. This is very permissive—nearly any Error object has amessageproperty, so non-API errors could pass.
Consider tightening the guard to specifically match API validation errors:
+import {ValidationError, parseValidationErrors} from '@/helpers/parseValidationErrors'
-interface ApiValidationError {
- message?: string
- invalid_fields?: string[]
-}
-
-function isApiValidationError(error: unknown): error is ApiValidationError {
+function isApiValidationError(error: unknown): error is ValidationError {
return error !== null &&
typeof error === 'object' &&
- ('message' in error || 'invalid_fields' in error)
+ 'invalid_fields' in error &&
+ Array.isArray((error as ValidationError).invalid_fields)
}🤖 Prompt for AI Agents
In frontend/src/views/user/Register.vue around lines 237-246, the local
ApiValidationError duplicates ValidationError from parseValidationErrors.ts and
the isApiValidationError guard is too loose (it will accept any Error with a
message). Import and use the shared ValidationError interface from
parseValidationErrors.ts instead of redefining it, and tighten the type guard to
ensure the value is a non-null object that has an invalid_fields property which
is an array of strings (use Array.isArray and check elements are strings) —
optionally also require the presence of invalid_fields rather than using a
message-only check so generic Errors don’t pass.
Replace hardcoded "Registration failed" with localized message using new translation key 'user.auth.registrationFailed' with more informative text. Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
Original prompt
Surface Registration Validation Errors Implementation Plan
Goal: Display detailed API validation errors (from
invalid_fields) in the registration form's existing field-level error mechanism instead of just showing "invalid data" in the general error message.Architecture: Parse the API's
ValidationHTTPErrorresponse to extract field-specific errors frominvalid_fieldsarray and apply them to Register.vue's existing field validation refs (emailValid,usernameValid). The API returns errors in the format{code: 2002, message: "invalid data", invalid_fields: ["email: email is not a valid email address"]}. Currently, Register.vue shows field-level validation for client-side checks (lines 36-40 for username, 64-68 for email) but only displays server errors in a general error message box.Tech Stack: Vue 3 Composition API, TypeScript
Task 1: Understand Current Error Structure
Files:
frontend/src/views/user/Register.vue(already analyzed)frontend/src/stores/auth.ts:201-228(register method)pkg/routes/api/v1/user_register.go:49-65(API validation)pkg/models/error.go:100-110(ValidationHTTPError structure)Verification: Confirm that:
ValidationHTTPErrorwithcode,message, andinvalid_fields[]authStore.register()throwse.response.datacontaining these fieldsRegister.vue:203only displayse?.message, missinginvalid_fieldsStatus: ✅ Already completed during investigation
Task 2: Write Unit Test for Error Parsing Helper
Files:
frontend/src/helpers/parseValidationErrors.test.tsStep 1: Write failing test for validation error parsing
Create test file that parses
invalid_fieldsinto a field-to-error map:Step 2: Run test to verify it fails
Expected: All tests FAIL with "Cannot find module './parseValidationErrors'"
Step 3: Commit the test
git add frontend/src/helpers/parseValidationErrors.test.ts git commit -m "test: add failing tests for validation error parsing helper"Task 3: Implement Error Parsing Helper
Files:
frontend/src/helpers/parseValidationErrors.tsStep 1: Implement minimal helper to make tests pass