fix: Map internal error-field-unavailable key to localized messages in Admin User Creation #38053
Conversation
|
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe AdminUserCreated component was replaced by UserCreate with a new public type ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UserCreate (client)
participant Form as react-hook-form
participant API as /v1/users.create (server)
participant Toast as Toasts
participant Caller as Parent (onReload/onClose)
rect rgb(240,248,255)
note right of UI: User fills form\n(name, username, email, password,\nsend-welcome-email)
end
UI->>Form: validate inputs
Form-->>UI: validation OK / field errors
alt validation OK
UI->>API: POST /v1/users.create (payload)
API-->>UI: 200 success / 4xx error
alt success
UI->>Toast: show success
UI->>Caller: onReload()
UI->>Caller: onClose()
else error
API-->>UI: error payload (may include error-field-unavailable)
UI->>UI: map server errors to messages
UI->>Toast: show mapped or generic error
end
else validation errors
UI->>Toast: show field-level errors
end
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/client/views/admin/users/AdminUserCreated.tsx">
<violation number="1" location="apps/meteor/client/views/admin/users/AdminUserCreated.tsx:116">
P2: Missing validation error display for the password field. Unlike the username and email fields, the password field doesn't show an error message when validation fails.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (1)
packages/i18n/src/locales/en.i18n.json (1)
1945-1955: Add matchingUsername_already_exists(or reuse existing key) to avoid missing translation
Email_NotificationsandEmail_already_existslook fine, but there is onlyUsername_already_exist(without the trailings) defined later in this file. If the updated Admin user creation flow looks upUsername_already_exists, that key will be missing here and the UI will likely show the raw key name.Either:
- add a new alias key with the expected name, reusing the existing copy; or
- change the frontend to use the existing
Username_already_existkey and avoid introducing a near-duplicate.For example, if you go with an alias key:
Proposed addition
@@ "Email_address_to_send_offline_messages": "Email Address to Send Offline Messages", "Email_already_exists": "Email already exists", + "Username_already_exists": "Username already exists. Please try another username.",Also consider matching the copy to the UX spec (“Email address already exists”) if that’s the desired phrasing.
🧹 Nitpick comments (3)
packages/i18n/src/locales/en.i18n.json (1)
2064-2067: Confirm intended split betweenerror-*andError-*keys and harmonize messagesThe new keys:
Error-domain-blocked: "Domain is blocked"Error-field-unavailable: "The provided value is already in use"Error-file-too-large: "File is too large"mirror existing lowercase
error-domain-blocked,error-field-unavailable, anderror-file-too-largekeys, which are already present with slightly different texts.This is consistent with the pattern of using:
error-*for backend error identifiers, andError-*for user-facing toasts.Two quick checks to avoid confusion:
- Ensure the new Admin User Create code indeed calls the capitalized
Error-*keys, not the lowercaseerror-*ones.- If you want a more specific message, you might align the text with the existing “email domain is blacklisted” wording, e.g. “Email domain is blocked,” to make the context clearer.
Functionally this looks fine; this is more about naming and copy consistency.
apps/meteor/client/views/admin/users/AdminUserCreated.tsx (2)
13-13: Remove unused import.The
useMemoimport is not used anywhere in the component.🔎 Proposed fix
-import React, { useState, useMemo } from 'react'; +import React, { useState } from 'react';
54-54: Consider usingunknowninstead ofanyfor better type safety.Using
anybypasses TypeScript's type checking. Consider usingunknownand narrowing the type with type guards for safer error handling.🔎 Proposed refactor
- } catch (error: any) { + } catch (error: unknown) { + const err = error as { error?: string; details?: { field?: string }; message?: string };Then use
errinstead oferrorin the catch block.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/meteor/client/views/admin/users/AdminUserCreated.tsxpackages/i18n/src/locales/en.i18n.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/admin/users/AdminUserCreated.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/i18n/src/locales/en.i18n.json
⏰ 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). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (3)
apps/meteor/client/views/admin/users/AdminUserCreated.tsx (3)
19-43: LGTM!The props definition and form setup are well-structured. The default values and form initialization follow react-hook-form best practices.
93-99: LGTM!Username field correctly implements required validation with proper error display.
119-141: LGTM!The welcome email toggle, button group, and export statement are correctly implemented. The form submission is properly disabled when no changes are made.
| if (error.error === 'error-field-unavailable') { | ||
| const field = error.details?.field; | ||
|
|
||
| if (field === 'email') { | ||
| return dispatchToastMessage({ | ||
| type: 'error', | ||
| message: t('Email_already_exists'), | ||
| }); | ||
| } | ||
| if (field === 'username') { | ||
| return dispatchToastMessage({ | ||
| type: 'error', | ||
| message: t('Username_already_exists'), | ||
| }); | ||
| } | ||
|
|
||
| return dispatchToastMessage({ | ||
| type: 'error', | ||
| message: t('error-field-unavailable'), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify i18n key locations for error-field-unavailable, Email_already_exists, Username_already_exists
echo "=== Checking packages/i18n/src/locales/en.i18n.json ==="
if [ -f "packages/i18n/src/locales/en.i18n.json" ]; then
jq 'keys[] | select(. == "error-field-unavailable" or . == "Email_already_exists" or . == "Username_already_exists")' packages/i18n/src/locales/en.i18n.json 2>/dev/null || echo "Could not parse JSON"
else
echo "File not found"
fi
echo ""
echo "=== Checking apps/meteor/public/i18n/en.i18n.json ==="
if [ -f "apps/meteor/public/i18n/en.i18n.json" ]; then
jq 'keys[] | select(. == "error-field-unavailable" or . == "Email_already_exists" or . == "Username_already_exists")' apps/meteor/public/i18n/en.i18n.json 2>/dev/null || echo "Could not parse JSON"
else
echo "File not found"
fi
echo ""
echo "=== Searching for keys across all en.i18n.json files ==="
rg -n "error-field-unavailable|Email_already_exists|Username_already_exists" --type json -g '**/en.i18n.json' 2>/dev/null | head -20Repository: RocketChat/Rocket.Chat
Length of output: 516
🏁 Script executed:
#!/bin/bash
# Search explicitly for Username_already_exists
echo "=== Searching for Username_already_exists ==="
rg "Username_already_exists" packages/i18n/src/locales/en.i18n.json
echo ""
echo "=== Checking if key is referenced in the code ==="
rg "Username_already_exists" apps/meteor/client/views/admin/users/AdminUserCreated.tsxRepository: RocketChat/Rocket.Chat
Length of output: 207
Add the missing Username_already_exists i18n key to packages/i18n/src/locales/en.i18n.json
The code references the i18n key Username_already_exists (line 62), but this key does not exist in packages/i18n/src/locales/en.i18n.json. The keys Email_already_exists and error-field-unavailable are present in the correct location, but Username_already_exists is missing and must be added to complete the implementation.
🤖 Prompt for AI Agents
In apps/meteor/client/views/admin/users/AdminUserCreated.tsx around lines 56–76,
the code references the i18n key "Username_already_exists" which is missing from
the English locale; open packages/i18n/src/locales/en.i18n.json and add the
"Username_already_exists" entry with the English message (e.g., "Username
already exists") alongside the existing Email_already_exists and
error-field-unavailable keys so the dispatchToastMessage call resolves
correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/client/views/admin/users/AdminUserCreated.tsx (1)
119-119: Remove the developer comment.The comment
/* P2 FIX: Added missing validation error display for password */should be removed per coding guidelines, which specify avoiding code comments in implementation.🔎 Proposed fix
- {/* P2 FIX: Added missing validation error display for password */} {errors.password && <Field.Error>{t('error-the-field-is-required', { field: t('Password') })}</Field.Error>}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/client/views/admin/users/AdminUserCreated.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/admin/users/AdminUserCreated.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-09-18T17:32:33.969Z
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
Applied to files:
apps/meteor/client/views/admin/users/AdminUserCreated.tsx
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
apps/meteor/client/views/admin/users/AdminUserCreated.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/admin/users/AdminUserCreated.tsx
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/client/views/admin/users/AdminUserCreated.tsx
⏰ 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). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (5)
apps/meteor/client/views/admin/users/AdminUserCreated.tsx (5)
1-17: LGTM!All imports are properly used throughout the component. The dependencies align with the form-based implementation using react-hook-form and Fuselage UI components.
19-43: LGTM!The type definition and form setup are clean. The
UserCreatePropstype properly exposes the public API, and theuseFormconfiguration withdefaultValuesensures controlled form state.
78-81: LGTM!The generic error fallback correctly extracts a readable message using a fallback chain (
error.message || error.error || String(error)), preventing[object Object]from being displayed to users. This properly addresses the previous critical review comment.
94-121: LGTM!All form fields now have consistent validation and error feedback. The password field validation error (line 120) properly addresses the previous review comment. The
isDirtycheck on the Save button (line 134) provides good UX by preventing accidental submissions.
144-144: LGTM!Clean default export of the
UserCreatecomponent.
| if (error.error === 'error-field-unavailable') { | ||
| const field = error.details?.field; | ||
|
|
||
| if (field === 'email') { | ||
| return dispatchToastMessage({ | ||
| type: 'error', | ||
| message: t('Email_already_exists'), | ||
| }); | ||
| } | ||
| if (field === 'username') { | ||
| return dispatchToastMessage({ | ||
| type: 'error', | ||
| message: t('Username_already_exists'), | ||
| }); | ||
| } | ||
|
|
||
| return dispatchToastMessage({ | ||
| type: 'error', | ||
| message: t('error-field-unavailable'), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Verify i18n keys in the English locale file
if [ -f "packages/i18n/src/locales/en.i18n.json" ]; then
echo "=== Checking for i18n keys ==="
echo "Email_already_exists:"
jq -r 'if has("Email_already_exists") then "✓ Found: " + .Email_already_exists else "✗ Missing" end' packages/i18n/src/locales/en.i18n.json
echo "Username_already_exists:"
jq -r 'if has("Username_already_exists") then "✓ Found: " + .Username_already_exists else "✗ Missing" end' packages/i18n/src/locales/en.i18n.json
echo "error-field-unavailable:"
jq -r 'if has("error-field-unavailable") then "✓ Found: " + ."error-field-unavailable" else "✗ Missing" end' packages/i18n/src/locales/en.i18n.json
else
echo "✗ File not found"
fiRepository: RocketChat/Rocket.Chat
Length of output: 265
🏁 Script executed:
# Verify the actual code in AdminUserCreated.tsx
if [ -f "apps/meteor/client/views/admin/users/AdminUserCreated.tsx" ]; then
echo "=== Code snippet around lines 56-76 ==="
sed -n '56,76p' apps/meteor/client/views/admin/users/AdminUserCreated.tsx
else
echo "File not found"
fiRepository: RocketChat/Rocket.Chat
Length of output: 864
Add the missing Username_already_exists i18n key to packages/i18n/src/locales/en.i18n.json.
The code on line 68 references t('Username_already_exists'), but this key is missing from the English locale file. While Email_already_exists and error-field-unavailable are defined, Username_already_exists will cause a translation failure at runtime. Add this key to packages/i18n/src/locales/en.i18n.json with an appropriate value (e.g., "Username already exists").
🤖 Prompt for AI Agents
In apps/meteor/client/views/admin/users/AdminUserCreated.tsx around lines 56 to
76, the code references t('Username_already_exists') but the English locale is
missing that key; open packages/i18n/src/locales/en.i18n.json and add the
"Username_already_exists" entry with an appropriate English string such as
"Username already exists" (ensure JSON syntax is valid, keep key naming
consistent with other entries, and run a quick lint/format check).
1 participant
PR Description
Summary
This PR fixes issue #37992. Previously, the user creation form in the Administration panel leaked internal backend error identifiers (e.g., [error-field-unavailable]) when an administrator attempted to use a duplicate email or username. This resulted in poor UX and exposed internal logic.
Changes
1. Internationalization (i18n)
2. Frontend Logic
Test Plan
To verify this fix, follow these steps:
5.Test Username Conflict: Enter a username that already exists. Click Save.
Checklist
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.