fix: validate username on Profile screen#7425
Conversation
Validate the username field against the server's UTF8_User_Names_Validation setting (with the default pattern as a fallback) before submitting the profile. This prevents invalid characters such as spaces from reaching the server, which previously surfaced only as a generic save error, and shows an inline message explaining the allowed characters. Closes RocketChat#1682
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds client-side username validation to ChangesUsername Validation in ProfileView
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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.
🧹 Nitpick comments (1)
app/views/ProfileView/index.tsx (1)
54-61: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd an explicit return type to
isValidUsername.Please annotate the helper with
: booleanto satisfy the TS guideline for explicit function return types.Suggested change
-const isValidUsername = (username: string, pattern: string) => { +const isValidUsername = (username: string, pattern: string): boolean => {As per coding guidelines,
**/*.{ts,tsx}requires explicit type annotations for function parameters and return types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/views/ProfileView/index.tsx` around lines 54 - 61, The isValidUsername function is missing an explicit return type annotation required by TypeScript coding guidelines. Add `: boolean` to the function signature after the closing parenthesis of the parameters list and before the opening curly brace to explicitly declare that this function returns a boolean value. This satisfies the TypeScript guideline that all functions in .ts and .tsx files must have explicit return type annotations.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/views/ProfileView/index.tsx`:
- Around line 54-61: The isValidUsername function is missing an explicit return
type annotation required by TypeScript coding guidelines. Add `: boolean` to the
function signature after the closing parenthesis of the parameters list and
before the opening curly brace to explicitly declare that this function returns
a boolean value. This satisfies the TypeScript guideline that all functions in
.ts and .tsx files must have explicit return type annotations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 730fbeb5-6860-43c2-b623-a5ecd948b0aa
📒 Files selected for processing (3)
app/i18n/locales/en.jsonapp/views/ProfileView/index.test.tsxapp/views/ProfileView/index.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/i18n/locales/en.jsonapp/views/ProfileView/index.test.tsxapp/views/ProfileView/index.tsx
app/i18n/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Place internationalization (i18n) configuration in 'app/i18n/' directory with support for 40+ locales and RTL
Files:
app/i18n/locales/en.json
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/views/ProfileView/index.test.tsxapp/views/ProfileView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/views/ProfileView/index.test.tsxapp/views/ProfileView/index.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/views/ProfileView/index.test.tsxapp/views/ProfileView/index.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/ProfileView/index.test.tsxapp/views/ProfileView/index.tsx
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/views/ProfileView/index.test.tsxapp/views/ProfileView/index.tsx
🔇 Additional comments (3)
app/i18n/locales/en.json (1)
997-997: LGTM!app/views/ProfileView/index.tsx (1)
78-78: LGTM!Also applies to: 91-103
app/views/ProfileView/index.test.tsx (1)
12-12: LGTM!Also applies to: 74-75, 116-136
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/ProfileView/index.tsx (1)
71-104: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the duplicate
isMasterDetailbinding (build/lint blocker).
isMasterDetailis destructured at Line 80 and then redeclared at Line 104, which triggersno-redeclareand breaks compilation.
Also, the selector object (Lines 83-94) does not provide anisMasterDetailkey, so that destructured field is stale.💡 Suggested fix
const { Accounts_AllowDeleteOwnAccount, Accounts_AllowEmailChange, Accounts_AllowPasswordChange, Accounts_AllowRealNameChange, Accounts_AllowUserAvatarChange, Accounts_AllowUsernameChange, Accounts_CustomFields, UTF8_User_Names_Validation, - isMasterDetail, serverVersion, user } = useAppSelector(state => ({ user: getUserSelector(state),As per coding guidelines,
**/*.{js,jsx,ts,tsx}must enforce ESLint rules from@rocket.chat/eslint-config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/views/ProfileView/index.tsx` around lines 71 - 104, The ProfileView component has a duplicate isMasterDetail binding that causes a no-redeclare lint/build failure. In app/views/ProfileView/index.tsx, remove the stale isMasterDetail from the useAppSelector destructuring since that selector does not return it, and keep only the useMasterDetail() assignment (or otherwise use a single source of truth). Verify the ProfileView function/component scope has just one isMasterDetail declaration.Sources: Coding guidelines, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/views/ProfileView/index.tsx`:
- Around line 71-104: The ProfileView component has a duplicate isMasterDetail
binding that causes a no-redeclare lint/build failure. In
app/views/ProfileView/index.tsx, remove the stale isMasterDetail from the
useAppSelector destructuring since that selector does not return it, and keep
only the useMasterDetail() assignment (or otherwise use a single source of
truth). Verify the ProfileView function/component scope has just one
isMasterDetail declaration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e733dac-fdb3-4777-a17b-49ba8d63d707
📒 Files selected for processing (3)
app/i18n/locales/en.jsonapp/views/ProfileView/index.test.tsxapp/views/ProfileView/index.tsx
💤 Files with no reviewable changes (1)
- app/views/ProfileView/index.test.tsx
✅ Files skipped from review due to trivial changes (1)
- app/i18n/locales/en.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/views/ProfileView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/views/ProfileView/index.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/views/ProfileView/index.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/views/ProfileView/index.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/ProfileView/index.tsx
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/views/ProfileView/index.tsx
🪛 ESLint
app/views/ProfileView/index.tsx
[error] 104-104: 'isMasterDetail' is already defined.
(no-redeclare)
Drop the stale isMasterDetail destructured from useAppSelector (the selector no longer returns it) so only the useMasterDetail() hook declares it, fixing the no-redeclare lint/build failure introduced by merging develop. Also annotate isValidUsername with an explicit boolean return type per the TS guideline.
|
Hi team! I've resolved the compilation/lint blocker on ProfileView by removing the redundant |
|
Hi @Shevilll, This is a great feature! Correctly mirroring the server's username validation ( I have one key suggestion regarding React performance and Yup schema reference stability: Cache the Yup Validation Schema with
|
1 participant
Description
On the Profile screen, the username field had no character validation: an invalid username (e.g. containing spaces or disallowed characters) left the Save button enabled and only failed afterwards with a generic server error.
This adds client-side validation that mirrors the server's
validateUsernamelogic and honors theUTF8_User_Names_Validationsetting, so the Save button is disabled and an inline error is shown for invalid input.Changes
UTF8_User_Names_Validationpattern, with the standard default fallback ([0-9a-zA-Z-_.]+) and a guard for an invalid server-supplied regex.Username_invalidi18n string (English source).Closes #1682
Summary by CodeRabbit