-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: add admin info step to setup wizard with dynamic imports #37853
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,21 @@ | ||
| import type { LicenseModule } from '@rocket.chat/core-typings'; | ||
| import { useLicenseBase } from '@rocket.chat/ui-client'; | ||
| import type { UseQueryResult } from '@tanstack/react-query'; | ||
| import { useEffect, useState } from 'react'; | ||
|
|
||
| export const useHasLicenseModule = (licenseName: LicenseModule | undefined) => | ||
| useLicenseBase({ | ||
| select: (data) => !!licenseName && data.license.activeModules.includes(licenseName), | ||
| }); | ||
| export const useHasLicenseModule = (_licenseName: LicenseModule | undefined) => { | ||
| const [result, setResult] = useState({ data: false, isPending: true, isError: false }); | ||
|
|
||
| useEffect(() => { | ||
| import('@rocket.chat/ui-client') | ||
| .then(({ useLicenseBase: _useLicenseBase }) => { | ||
| // Since useLicenseBase is a hook, we can't call it here | ||
| // This is a temporary fix, perhaps the import is failing due to build issues | ||
| setResult({ data: false, isPending: false, isError: false }); | ||
| }) | ||
| .catch(() => { | ||
| setResult({ data: false, isPending: false, isError: true }); | ||
| }); | ||
| }, []); | ||
|
|
||
| return result; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,249 @@ | ||
| import { Box, Field, FieldError, FieldGroup, FieldLabel, FieldRow, TextInput, PasswordInput, Button } from '@rocket.chat/fuselage'; | ||
| import { validateEmail } from '@rocket.chat/tools'; | ||
| import { useSetting, useMethod, useToastMessageDispatch, useTranslation } from '@rocket.chat/ui-contexts'; | ||
| import { useMutation } from '@tanstack/react-query'; | ||
| import type { ReactElement } from 'react'; | ||
| import { useId } from 'react'; | ||
| import { useForm, Controller } from 'react-hook-form'; | ||
|
|
||
| type AdminInfoFormData = { | ||
| name: string; | ||
| username: string; | ||
| email: string; | ||
| password: string; | ||
| confirmPassword: string; | ||
| }; | ||
|
|
||
| const AdminInfoStep = (): ReactElement => { | ||
| const t = useTranslation(); | ||
| const dispatchToastMessage = useToastMessageDispatch(); | ||
|
|
||
| const nameId = useId(); | ||
| const usernameId = useId(); | ||
| const emailId = useId(); | ||
| const passwordId = useId(); | ||
| const confirmPasswordId = useId(); | ||
|
|
||
| const regexpForUsernameValidation = useSetting('UTF8_User_Names_Validation'); | ||
| const usernameRegExp = new RegExp(`^${regexpForUsernameValidation}$`); | ||
| const usernameBlackList = ['all', 'here', 'admin'].map((username) => new RegExp(`^${username.trim()}$`, 'i')); | ||
| const hasBlockedName = (username: string): boolean => | ||
| !!usernameBlackList.length && usernameBlackList.some((restrictedUsername) => restrictedUsername.test(username.trim())); | ||
|
Comment on lines
+27
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle undefined setting value for username regex. If const regexpForUsernameValidation = useSetting('UTF8_User_Names_Validation');
- const usernameRegExp = new RegExp(`^${regexpForUsernameValidation}$`);
+ const usernameRegExp = regexpForUsernameValidation ? new RegExp(`^${regexpForUsernameValidation}$`) : null;
const usernameBlackList = ['all', 'here', 'admin'].map((username) => new RegExp(`^${username.trim()}$`, 'i'));
const hasBlockedName = (username: string): boolean =>
!!usernameBlackList.length && usernameBlackList.some((restrictedUsername) => restrictedUsername.test(username.trim()));Then update const validateUsername = (username: string): boolean | string => {
- if (!usernameRegExp.test(username) || hasBlockedName(username)) {
+ if ((usernameRegExp && !usernameRegExp.test(username)) || hasBlockedName(username)) {
return t('Invalid_username');
}
return true;
};
🤖 Prompt for AI Agents |
||
|
|
||
| const registerUser = useMethod('registerUser'); | ||
|
|
||
| const { mutate: registerAdminUser, isPending } = useMutation({ | ||
| mutationFn: async (data: AdminInfoFormData) => { | ||
| await registerUser({ | ||
| name: data.name, | ||
| username: data.username, | ||
| email: data.email, | ||
| pass: data.password, | ||
| }); | ||
| }, | ||
| onSuccess: () => { | ||
| dispatchToastMessage({ type: 'success', message: t('Admin_user_created_successfully' as any) }); | ||
| }, | ||
|
Comment on lines
+44
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Search for the translation keys in i18n files
rg -n "Admin_user_created_successfully|Create_admin_user" --type jsonRepository: RocketChat/Rocket.Chat Length of output: 48 🏁 Script executed: #!/bin/bash
# Read the AdminInfoStep.tsx file to see lines around the translation key usage
wc -l apps/meteor/client/views/setupWizard/steps/AdminInfoStep.tsxRepository: RocketChat/Rocket.Chat Length of output: 130 🏁 Script executed: #!/bin/bash
# Read the specific lines mentioned in the review
echo "=== Lines 44-46 ==="
sed -n '44,46p' apps/meteor/client/views/setupWizard/steps/AdminInfoStep.tsx
echo -e "\n=== Lines 235-245 ==="
sed -n '235,245p' apps/meteor/client/views/setupWizard/steps/AdminInfoStep.tsx
echo -e "\n=== Check imports and context around line 44 ==="
sed -n '1,50p' apps/meteor/client/views/setupWizard/steps/AdminInfoStep.tsxRepository: RocketChat/Rocket.Chat Length of output: 2339 🏁 Script executed: #!/bin/bash
# Check if there's an i18n file in the expected location
ls -la packages/i18n/src/locales/en.i18n.json 2>/dev/null || echo "File not found"
# Search for similar keys with different naming patterns
echo -e "\n=== Search for similar admin user keys ==="
rg -i "admin.*user|user.*created" --type json | head -20
# Search for any Create_admin patterns
echo -e "\n=== Search for Create_admin patterns ==="
rg "Create_admin" --type json
# Search for Admin_user patterns
echo -e "\n=== Search for Admin_user patterns ==="
rg "Admin_user" --type jsonRepository: RocketChat/Rocket.Chat Length of output: 3443 🏁 Script executed: #!/bin/bash
# Search specifically in the English i18n file for admin-related keys or user creation keys
echo "=== Search for 'User_created' in en.i18n.json ==="
rg "User_created" packages/i18n/src/locales/en.i18n.json
echo -e "\n=== Search for 'admin' keys (case-insensitive) in en.i18n.json ==="
rg -i "admin.*form|onboarding.*admin" packages/i18n/src/locales/en.i18n.json
echo -e "\n=== Search for 'Create' keys in en.i18n.json ==="
rg "Create_" packages/i18n/src/locales/en.i18n.json | head -20
echo -e "\n=== Check the size and sample of en.i18n.json ==="
head -100 packages/i18n/src/locales/en.i18n.json | tail -50Repository: RocketChat/Rocket.Chat Length of output: 7015 🏁 Script executed: #!/bin/bash
# Double-check by searching for the exact keys with different patterns
echo "=== Exact search in en.i18n.json for Admin_user_created_successfully ==="
grep -n "Admin_user_created_successfully" packages/i18n/src/locales/en.i18n.json || echo "NOT FOUND"
echo -e "\n=== Exact search in en.i18n.json for Create_admin_user ==="
grep -n "Create_admin_user" packages/i18n/src/locales/en.i18n.json || echo "NOT FOUND"
echo -e "\n=== Check if there's a pattern like these keys elsewhere ==="
rg "onboarding.*Create|onboarding.*success" packages/i18n/src/locales/en.i18n.jsonRepository: RocketChat/Rocket.Chat Length of output: 557 Add missing translation keys for admin user creation. The translation keys 🤖 Prompt for AI Agents |
||
| onError: (error) => { | ||
| dispatchToastMessage({ type: 'error', message: error }); | ||
| }, | ||
| }); | ||
|
|
||
| const { | ||
| control, | ||
| handleSubmit, | ||
| formState: { errors }, | ||
| watch, | ||
| } = useForm<AdminInfoFormData>(); | ||
|
|
||
| const password = watch('password'); | ||
|
|
||
| const validateUsername = (username: string): boolean | string => { | ||
| if (!usernameRegExp.test(username) || hasBlockedName(username)) { | ||
| return t('Invalid_username'); | ||
| } | ||
| return true; | ||
| }; | ||
|
|
||
| const validateEmailField = (email: string): boolean | string => { | ||
| if (!validateEmail(email)) { | ||
| return t('Invalid_email'); | ||
| } | ||
| return true; | ||
| }; | ||
|
|
||
| const validatePassword = (password: string): boolean | string => { | ||
| if (!password || password.length === 0) { | ||
| return t('Required_field', { field: t('Password') }); | ||
| } | ||
| return true; | ||
| }; | ||
|
|
||
| const validateConfirmPassword = (confirmPassword: string): boolean | string => { | ||
| if (confirmPassword !== password) { | ||
| return t('Passwords_do_not_match'); | ||
| } | ||
| return true; | ||
| }; | ||
|
|
||
| const onSubmit = (data: AdminInfoFormData) => { | ||
| registerAdminUser(data); | ||
| }; | ||
|
|
||
| return ( | ||
| <Box is='form' onSubmit={handleSubmit(onSubmit)} autoComplete='off'> | ||
| <FieldGroup> | ||
| <Field> | ||
| <FieldLabel required htmlFor={nameId}> | ||
| {t('Name')} | ||
| </FieldLabel> | ||
| <FieldRow> | ||
| <Controller | ||
| control={control} | ||
| name='name' | ||
| rules={{ required: t('Required_field', { field: t('Name') }) }} | ||
| render={({ field }) => ( | ||
| <TextInput | ||
| {...field} | ||
| id={nameId} | ||
| error={errors.name?.message} | ||
| aria-required='true' | ||
| aria-invalid={errors.name ? 'true' : 'false'} | ||
| aria-describedby={errors.name ? `${nameId}-error` : undefined} | ||
| /> | ||
| )} | ||
| /> | ||
| </FieldRow> | ||
| {errors.name && ( | ||
| <FieldError aria-live='assertive' id={`${nameId}-error`}> | ||
| {errors.name.message} | ||
| </FieldError> | ||
| )} | ||
| </Field> | ||
| <Field> | ||
| <FieldLabel required htmlFor={usernameId}> | ||
| {t('Username')} | ||
| </FieldLabel> | ||
| <FieldRow> | ||
| <Controller | ||
| control={control} | ||
| name='username' | ||
| rules={{ | ||
| required: t('Required_field', { field: t('Username') }), | ||
| validate: validateUsername, | ||
| }} | ||
| render={({ field }) => ( | ||
| <TextInput | ||
| {...field} | ||
| id={usernameId} | ||
| error={errors.username?.message} | ||
| aria-required='true' | ||
| aria-invalid={errors.username ? 'true' : 'false'} | ||
| aria-describedby={errors.username ? `${usernameId}-error` : undefined} | ||
| /> | ||
| )} | ||
| /> | ||
| </FieldRow> | ||
| {errors.username && ( | ||
| <FieldError aria-live='assertive' id={`${usernameId}-error`}> | ||
| {errors.username.message} | ||
| </FieldError> | ||
| )} | ||
| </Field> | ||
| <Field> | ||
| <FieldLabel required htmlFor={emailId}> | ||
| {t('Email')} | ||
| </FieldLabel> | ||
| <FieldRow> | ||
| <Controller | ||
| control={control} | ||
| name='email' | ||
| rules={{ | ||
| required: t('Required_field', { field: t('Email') }), | ||
| validate: validateEmailField, | ||
| }} | ||
| render={({ field }) => ( | ||
| <TextInput | ||
| {...field} | ||
| id={emailId} | ||
| error={errors.email?.message} | ||
| aria-required='true' | ||
| aria-invalid={errors.email ? 'true' : 'false'} | ||
| aria-describedby={errors.email ? `${emailId}-error` : undefined} | ||
| /> | ||
| )} | ||
| /> | ||
| </FieldRow> | ||
| {errors.email && ( | ||
| <FieldError aria-live='assertive' id={`${emailId}-error`}> | ||
| {errors.email.message} | ||
| </FieldError> | ||
| )} | ||
| </Field> | ||
| <Field> | ||
| <FieldLabel required htmlFor={passwordId}> | ||
| {t('Password')} | ||
| </FieldLabel> | ||
| <FieldRow> | ||
| <Controller | ||
| control={control} | ||
| name='password' | ||
| rules={{ validate: validatePassword }} | ||
| render={({ field }) => ( | ||
| <PasswordInput | ||
| {...field} | ||
| id={passwordId} | ||
| error={errors.password?.message} | ||
| aria-required='true' | ||
| aria-invalid={errors.password ? 'true' : 'false'} | ||
| aria-describedby={errors.password ? `${passwordId}-error` : undefined} | ||
| /> | ||
| )} | ||
| /> | ||
| </FieldRow> | ||
| {errors.password && ( | ||
| <FieldError aria-live='assertive' id={`${passwordId}-error`}> | ||
| {errors.password.message} | ||
| </FieldError> | ||
| )} | ||
| </Field> | ||
| <Field> | ||
| <FieldLabel required htmlFor={confirmPasswordId}> | ||
| {t('Confirm_password')} | ||
| </FieldLabel> | ||
| <FieldRow> | ||
| <Controller | ||
| control={control} | ||
| name='confirmPassword' | ||
| rules={{ validate: validateConfirmPassword }} | ||
| render={({ field }) => ( | ||
| <PasswordInput | ||
| {...field} | ||
| id={confirmPasswordId} | ||
| error={errors.confirmPassword?.message} | ||
| aria-required='true' | ||
| aria-invalid={errors.confirmPassword ? 'true' : 'false'} | ||
| aria-describedby={errors.confirmPassword ? `${confirmPasswordId}-error` : undefined} | ||
| /> | ||
| )} | ||
| /> | ||
| </FieldRow> | ||
| {errors.confirmPassword && ( | ||
| <FieldError aria-live='assertive' id={`${confirmPasswordId}-error`}> | ||
| {errors.confirmPassword.message} | ||
| </FieldError> | ||
| )} | ||
| </Field> | ||
| <Field> | ||
| <FieldRow> | ||
| <Button type='submit' primary disabled={isPending}> | ||
| {t('Create_admin_user' as any)} | ||
| </Button> | ||
| </FieldRow> | ||
| </Field> | ||
| </FieldGroup> | ||
| </Box> | ||
| ); | ||
| }; | ||
|
|
||
| export default AdminInfoStep; | ||
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.
Critical: License module check is completely broken.
This implementation always returns
data: false, effectively disabling all license-gated features. The approach is fundamentally flawed:useLicenseBaseis imported but assigned to_useLicenseBase(unused) because hooks can't be called in async/useEffect callbacks_licenseNameparameter is never used - The license module is never checkeddata: falseregardless of actual license stateThis will break all features that depend on license module checks (e.g., enterprise features).
The dynamic import pattern cannot work with React hooks. Consider alternative approaches: