fix: validate username availability against API in Setup Wizard admin step#38991
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 |
|
WalkthroughThis PR adds username availability validation to the setup wizard's admin registration step by introducing a new context-exposed API function. The function queries the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui-client/src/views/setupWizard/providers/SetupWizardProvider.tsx (1)
55-82:⚠️ Potential issue | 🔴 CriticalEndpoint is unreachable during unauthenticated setup—catches auth failures as "username unavailable".
The endpoint requires
authRequired: trueand callscheckUsernameAvailabilityWithValidation(this.userId, username), which throwserror-invalid-userwhen userId is missing. This function is called during form validation (invalidateUsername()) beforeregisterAdminUser()logs in the user. The catch block returnsfalse, which surfaces as'Username_already_exist', blocking setup progression. Either make the endpoint unauthenticated for setup context, or defer the check until after registration/login, or throw/return a union to distinguish auth failures from unavailable usernames.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-client/src/views/setupWizard/providers/SetupWizardProvider.tsx` around lines 55 - 82, The current checkUsernameAvailability callback swallows auth failures from checkUsernameAvailabilityEndpoint and returns false so validateUsername() reports 'Username_already_exist' during unauthenticated setup; update the logic in checkUsernameAvailability (or the endpoint config) so auth-missing errors are distinguished: either make the endpoint unauthenticated (authRequired: false) used for setup, or change checkUsernameAvailability to inspect the thrown error (e.g., error-invalid-user / missing userId) and either rethrow or return a distinct result (or true) so validateUsername() doesn't treat auth failure as "username unavailable"; reference functions: checkUsernameAvailabilityEndpoint, checkUsernameAvailability, validateUsername(), registerAdminUser().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/ui-client/src/views/setupWizard/providers/SetupWizardProvider.tsx`:
- Around line 55-82: The current checkUsernameAvailability callback swallows
auth failures from checkUsernameAvailabilityEndpoint and returns false so
validateUsername() reports 'Username_already_exist' during unauthenticated
setup; update the logic in checkUsernameAvailability (or the endpoint config) so
auth-missing errors are distinguished: either make the endpoint unauthenticated
(authRequired: false) used for setup, or change checkUsernameAvailability to
inspect the thrown error (e.g., error-invalid-user / missing userId) and either
rethrow or return a distinct result (or true) so validateUsername() doesn't
treat auth failure as "username unavailable"; reference functions:
checkUsernameAvailabilityEndpoint, checkUsernameAvailability,
validateUsername(), registerAdminUser().
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/ui-client/src/views/setupWizard/contexts/SetupWizardContext.tsxpackages/ui-client/src/views/setupWizard/providers/SetupWizardProvider.tsxpackages/ui-client/src/views/setupWizard/steps/AdminInfoStep.tsx
📜 Review details
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🧰 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:
packages/ui-client/src/views/setupWizard/providers/SetupWizardProvider.tsxpackages/ui-client/src/views/setupWizard/contexts/SetupWizardContext.tsxpackages/ui-client/src/views/setupWizard/steps/AdminInfoStep.tsx
🧬 Code graph analysis (2)
packages/ui-client/src/views/setupWizard/providers/SetupWizardProvider.tsx (1)
packages/ui-contexts/src/index.ts (1)
useEndpoint(34-34)
packages/ui-client/src/views/setupWizard/steps/AdminInfoStep.tsx (1)
packages/ui-client/src/views/setupWizard/contexts/SetupWizardContext.tsx (1)
useSetupWizardContext(74-74)
🔇 Additional comments (5)
packages/ui-client/src/views/setupWizard/contexts/SetupWizardContext.tsx (2)
19-38: Context type extension is clear and properly typed.
40-71: Default stub forcheckUsernameAvailabilityis fine for non-provider usage.packages/ui-client/src/views/setupWizard/steps/AdminInfoStep.tsx (2)
20-20: Context consumption update looks good.
42-52: No action required.
The asyncvalidateUsernamevalidator is correctly implemented and used. The code compiles without TypeScript errors, confirming thatAdminInfoPagefrom@rocket.chat/onboarding-uiaccepts async validators with thePromise<boolean | string>signature.packages/ui-client/src/views/setupWizard/providers/SetupWizardProvider.tsx (1)
204-242: Context value wiring and memo deps look consistent.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38991 +/- ##
===========================================
- Coverage 70.64% 70.62% -0.03%
===========================================
Files 3189 3189
Lines 112716 112716
Branches 20413 20427 +14
===========================================
- Hits 79632 79606 -26
- Misses 31040 31058 +18
- Partials 2044 2052 +8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes
Replace the missing username availability API check in AdminInfoStep with a proper async validation using the checkUsernameAvailability endpoint exposed through SetupWizardContext.
The following function is now async and calls the API:
validateUsernameThe following method is added to
SetupWizarContextValuetype and default context value:checkUsernameAvailabilityThe following callback is implemented and exposed in context:
checkUsernameAvailabilityviauseEndpoint('GET', '/v1/users.checkUsernameAvailability')Before
After
Issue(s)
Closes #38990
Steps to reproduce
See
packages/ui-client/src/views/setupWizard/steps/AdminInfoStep.tsxSee
packages/ui-client/src/views/setupWizardcontexts/SetupWizardContext.tsSee
packages/ui-client/src/views/setupWizard/providers/SetupWizardProvider.tsxTesting
Summary by CodeRabbit
COMM-144