From 8c500ccee1f53e421f4fd5dcdf7c1469eec2fcbb Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Wed, 19 Aug 2026 14:57:46 -0600 Subject: [PATCH] fix(users): show password validation errors (#43191) Co-authored-by: Claude --- .../src/features/users/UserListModal.tsx | 41 ++------ .../src/features/users/utils.test.ts | 99 +++++++++++++++++++ superset-frontend/src/features/users/utils.ts | 41 +++++++- 3 files changed, 147 insertions(+), 34 deletions(-) create mode 100644 superset-frontend/src/features/users/utils.test.ts diff --git a/superset-frontend/src/features/users/UserListModal.tsx b/superset-frontend/src/features/users/UserListModal.tsx index ca398cb7ea7..045b727ac9a 100644 --- a/superset-frontend/src/features/users/UserListModal.tsx +++ b/superset-frontend/src/features/users/UserListModal.tsx @@ -31,7 +31,12 @@ import { import { Group, Role, UserObject } from 'src/pages/UsersList/types'; import { Actions } from 'src/constants'; import { BaseUserListModalProps, FormValues } from './types'; -import { createUser, updateUser, atLeastOneRoleOrGroup } from './utils'; +import { + createUser, + updateUser, + atLeastOneRoleOrGroup, + handleUserError, +} from './utils'; export interface UserModalProps extends BaseUserListModalProps { roles: Role[]; @@ -51,36 +56,6 @@ function UserListModal({ }: UserModalProps) { const { addDangerToast, addSuccessToast } = useToasts(); const handleFormSubmit = async (values: FormValues) => { - const handleError = async ( - err: any, - action: Actions.CREATE | Actions.UPDATE, - ) => { - let errorMessage = - action === Actions.CREATE - ? t('There was an error creating the user. Please, try again.') - : t('There was an error updating the user. Please, try again.'); - - if (err.status === 422) { - const errorData = await err.json(); - const detail = errorData?.message || ''; - - if (detail.includes('duplicate key value')) { - if (detail.includes('ab_user_username_key')) { - errorMessage = t( - 'This username is already taken. Please choose another one.', - ); - } else if (detail.includes('ab_user_email_key')) { - errorMessage = t( - 'This email is already associated with an account. Please choose another one.', - ); - } - } - } - - addDangerToast(errorMessage); - throw err; - }; - if (isEditMode) { if (!user) { throw new Error('User is required in edit mode'); @@ -89,14 +64,14 @@ function UserListModal({ await updateUser(user.id, values); addSuccessToast(t('The user has been updated successfully.')); } catch (err) { - await handleError(err, Actions.UPDATE); + await handleUserError(err as Response, Actions.UPDATE, addDangerToast); } } else { try { await createUser(values); addSuccessToast(t('The user has been created successfully.')); } catch (err) { - await handleError(err, Actions.CREATE); + await handleUserError(err as Response, Actions.CREATE, addDangerToast); } } }; diff --git a/superset-frontend/src/features/users/utils.test.ts b/superset-frontend/src/features/users/utils.test.ts new file mode 100644 index 00000000000..bd9c80efa39 --- /dev/null +++ b/superset-frontend/src/features/users/utils.test.ts @@ -0,0 +1,99 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Actions } from 'src/constants'; +import { handleUserError } from './utils'; + +test('shows the password validation message from a 400 response', async () => { + const error = new Response( + JSON.stringify({ + message: { + password: ['Password must be at least 8 characters long.'], + }, + }), + { status: 400 }, + ); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.CREATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'Password must be at least 8 characters long.', + ); +}); + +test('shows a plain string message from a 400 response', async () => { + const error = new Response( + JSON.stringify({ message: 'User must have at least one role or group!' }), + { status: 400 }, + ); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.UPDATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'User must have at least one role or group!', + ); +}); + +test('keeps the duplicate username message for a 422 response', async () => { + const error = new Response( + JSON.stringify({ + message: + 'duplicate key value violates unique constraint "ab_user_username_key"', + }), + { status: 422 }, + ); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.CREATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'This username is already taken. Please choose another one.', + ); +}); + +test('shows the generic message when a 422 response has no message', async () => { + const error = new Response(JSON.stringify({ foo: 'bar' }), { status: 422 }); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.CREATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'There was an error creating the user. Please, try again.', + ); +}); + +test('shows the generic message when a 400 response is not JSON', async () => { + const error = new Response('Bad request', { + status: 400, + headers: { 'Content-Type': 'text/html' }, + }); + const addDangerToast = jest.fn(); + + await expect( + handleUserError(error, Actions.CREATE, addDangerToast), + ).rejects.toBe(error); + expect(addDangerToast).toHaveBeenCalledWith( + 'There was an error creating the user. Please, try again.', + ); +}); diff --git a/superset-frontend/src/features/users/utils.ts b/superset-frontend/src/features/users/utils.ts index 5450bdca49f..4baa60fbf3c 100644 --- a/superset-frontend/src/features/users/utils.ts +++ b/superset-frontend/src/features/users/utils.ts @@ -17,10 +17,49 @@ * under the License. */ import { t } from '@apache-superset/core/translation'; -import { SupersetClient } from '@superset-ui/core'; +import { getClientErrorObject, SupersetClient } from '@superset-ui/core'; import { SelectOption } from 'src/components/ListView'; +import { Actions } from 'src/constants'; import { FormValues } from './types'; +type AddDangerToast = (message: string) => void; + +export const handleUserError = async ( + err: Response, + action: Actions.CREATE | Actions.UPDATE, + addDangerToast: AddDangerToast, +): Promise => { + let errorMessage = + action === Actions.CREATE + ? t('There was an error creating the user. Please, try again.') + : t('There was an error updating the user. Please, try again.'); + + if (err.status === 400 || err.status === 422) { + const errorData = await getClientErrorObject(err); + const message: unknown = errorData.message; + + if (err.status === 400 && message && errorData.error) { + errorMessage = errorData.error; + } else if ( + err.status === 422 && + errorData.error?.includes('duplicate key value') + ) { + if (errorData.error.includes('ab_user_username_key')) { + errorMessage = t( + 'This username is already taken. Please choose another one.', + ); + } else if (errorData.error.includes('ab_user_email_key')) { + errorMessage = t( + 'This email is already associated with an account. Please choose another one.', + ); + } + } + } + + addDangerToast(errorMessage); + throw err; +}; + export const createUser = async (values: FormValues) => { const { confirmPassword: _confirmPassword, ...payload } = values; if (payload.active == null) {