fix(users): show password validation errors (#43191)

Co-authored-by: Claude <noreply@anthropic.com>

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);
       }
     }
   };

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('<html>Bad request</html>', {
+    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.',
+  );
+});

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<never> => {
+  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) {