Fix mail box creation error handling and mail domain addition form

CHANGELOG.md

@@ -11,13 +11,15 @@ and this project adheres to
 ### Added
 
 - 📈(monitoring) configure sentry monitoring #378
+- 🥅(frontend) improve api error handling #355
 
 ### Fixed
 
 - 🐛(dimail) improve handling of dimail errors on failed mailbox creation #377
 -  💬(frontend) fix group member removal text #382
 -  💬(frontend) fix add mail domain text #382
 - 🐛(frontend) fix keyboard navigation #379
+- 🐛(frontend) fix add mail domain form submission #355
 
 ## [1.0.2] - 2024-08-30
 

src/frontend/apps/desk/src/api/__tests__/parseAPIError.test.tsx

@@ -0,0 +1,174 @@
+import { APIError } from '@/api';
+
+import {
+  parseAPIError,
+  parseAPIErrorCause,
+  parseServerAPIError,
+} from '../parseAPIError';
+
+describe('parseAPIError', () => {
+  const handleErrorMock = jest.fn();
+  const handleServerErrorMock = jest.fn();
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should handle specific API error and return no unhandled causes', () => {
+    const error = new APIError('client error', {
+      cause: ['Mail domain with this name already exists.'],
+      status: 400,
+    });
+
+    const result = parseAPIError({
+      error,
+      errorParams: {
+        name: {
+          causes: ['Mail domain with this name already exists.'],
+          handleError: handleErrorMock,
+        },
+      },
+      serverErrorParams: {
+        defaultMessage: 'Server error',
+        handleError: handleServerErrorMock,
+      },
+    });
+
+    expect(handleErrorMock).toHaveBeenCalled();
+    expect(result).toEqual([]);
+  });
+
+  it('should return unhandled causes when no match is found', () => {
+    const error = new APIError('client error', {
+      cause: ['Unhandled error'],
+      status: 400,
+    });
+
+    const result = parseAPIError({
+      error,
+      errorParams: {
+        name: {
+          causes: ['Mail domain with this name already exists.'],
+          handleError: handleErrorMock,
+        },
+      },
+      serverErrorParams: {
+        defaultMessage: 'Server error',
+        handleError: handleServerErrorMock,
+      },
+    });
+
+    expect(handleErrorMock).not.toHaveBeenCalled();
+    expect(result).toEqual(['Unhandled error']);
+  });
+
+  it('should handle server errors correctly and prepend server error message', () => {
+    const error = new APIError('server error', { status: 500 });
+
+    const result = parseAPIError({
+      error,
+      errorParams: undefined,
+      serverErrorParams: {
+        defaultMessage: 'Server error occurred',
+        handleError: handleServerErrorMock,
+      },
+    });
+
+    expect(handleServerErrorMock).toHaveBeenCalled();
+    expect(result).toEqual(['Server error occurred']);
+  });
+
+  it('should handle absence of errors gracefully', () => {
+    const result = parseAPIError({
+      error: null,
+      errorParams: {
+        name: {
+          causes: ['Mail domain with this name already exists.'],
+          handleError: handleErrorMock,
+        },
+      },
+      serverErrorParams: {
+        defaultMessage: 'Server error',
+        handleError: handleServerErrorMock,
+      },
+    });
+
+    expect(result).toBeUndefined();
+  });
+});
+
+describe('parseAPIErrorCause', () => {
+  it('should handle specific errors and call handleError', () => {
+    const handleErrorMock = jest.fn();
+    const causes = ['Mail domain with this name already exists.'];
+
+    const errorParams = {
+      name: {
+        causes: ['Mail domain with this name already exists.'],
+        handleError: handleErrorMock,
+      },
+    };
+
+    const result = parseAPIErrorCause({ causes, errorParams });
+
+    expect(handleErrorMock).toHaveBeenCalled();
+    expect(result).toEqual([]);
+  });
+
+  it('should handle multiple causes and return unhandled causes', () => {
+    const handleErrorMock = jest.fn();
+    const causes = [
+      'Mail domain with this name already exists.',
+      'Unhandled error',
+    ];
+
+    const errorParams = {
+      name: {
+        causes: ['Mail domain with this name already exists.'],
+        handleError: handleErrorMock,
+      },
+    };
+
+    const result = parseAPIErrorCause({ causes, errorParams });
+
+    expect(handleErrorMock).toHaveBeenCalled();
+    expect(result).toEqual(['Unhandled error']);
+  });
+});
+
+describe('parseServerAPIError', () => {
+  it('should prepend the server error message when there are other causes', () => {
+    const causes = ['Some other error'];
+    const serverErrorParams = {
+      defaultMessage: 'Server error',
+    };
+
+    const result = parseServerAPIError({ causes, serverErrorParams });
+
+    expect(result).toEqual(['Server error', 'Some other error']);
+  });
+
+  it('should only return server error message when no other causes exist', () => {
+    const causes: string[] = [];
+    const serverErrorParams = {
+      defaultMessage: 'Server error',
+    };
+
+    const result = parseServerAPIError({ causes, serverErrorParams });
+
+    expect(result).toEqual(['Server error']);
+  });
+
+  it('should call handleError when provided as a param', () => {
+    const handleErrorMock = jest.fn();
+    const causes: string[] = [];
+    const serverErrorParams = {
+      defaultMessage: 'Server error',
+      handleError: handleErrorMock,
+    };
+
+    parseServerAPIError({ causes, serverErrorParams });
+
+    expect(handleErrorMock).toHaveBeenCalled();
+  });
+});

src/frontend/apps/desk/src/api/parseAPIError.ts

@@ -0,0 +1,85 @@
+import { APIError } from '@/api/index';
+
+type ErrorParams = {
+  [fieldName: string]: {
+    causes: string[];
+    causeShown?: string;
+    handleError: () => void;
+  };
+};
+
+type ServerErrorParams = {
+  defaultMessage: string;
+  handleError?: () => void;
+};
+
+export type parseAPIErrorParams = {
+  error: APIError | null;
+  errorParams?: ErrorParams;
+  serverErrorParams: ServerErrorParams;
+};
+export const parseAPIError = ({
+  error,
+  errorParams,
+  serverErrorParams,
+}: parseAPIErrorParams) => {
+  if (!error || !serverErrorParams?.defaultMessage) {
+    return;
+  }
+
+  let causes: string[] =
+    error.cause?.length && errorParams
+      ? parseAPIErrorCause({ causes: error.cause, errorParams })
+      : [];
+
+  if (error?.status === 500 || !error?.status) {
+    causes = parseServerAPIError({ causes, serverErrorParams });
+  }
+
+  return causes;
+};
+
+export const parseAPIErrorCause = ({
+  causes,
+  errorParams,
+}: {
+  causes: string[];
+  errorParams: ErrorParams;
+}): string[] =>
+  causes.reduce((arrayCauses, cause) => {
+    const foundErrorParams = Object.values(errorParams).find((params) =>
+      params.causes.find((knownCause) =>
+        new RegExp(knownCause, 'i').test(cause),
+      ),
+    );
+
+    if (!foundErrorParams) {
+      arrayCauses.push(cause);
+    }
+
+    if (foundErrorParams?.causeShown) {
+      arrayCauses.push(foundErrorParams.causeShown);
+    }
+
+    if (typeof foundErrorParams?.handleError === 'function') {
+      foundErrorParams.handleError();
+    }
+
+    return arrayCauses;
+  }, [] as string[]);
+
+export const parseServerAPIError = ({
+  causes,
+  serverErrorParams,
+}: {
+  causes: string[];
+  serverErrorParams: ServerErrorParams;
+}): string[] => {
+  causes.unshift(serverErrorParams.defaultMessage);
+
+  if (typeof serverErrorParams?.handleError === 'function') {
+    serverErrorParams.handleError();
+  }
+
+  return causes;
+};

src/frontend/apps/desk/src/features/mail-domains/api/index.tsx

@@ -2,4 +2,4 @@ export * from './useMailDomains';
 export * from './useMailDomain';
 export * from './useCreateMailbox';
 export * from './useMailboxes';
-export * from './useCreateMailDomain';
+export * from './useAddMailDomain';

src/frontend/apps/desk/src/features/mail-domains/api/useCreateMailDomain.tsx → src/frontend/apps/desk/src/features/mail-domains/api/useAddMailDomain.tsx

@@ -5,7 +5,13 @@ import { MailDomain } from '@/features/mail-domains';
 
 import { KEY_LIST_MAIL_DOMAIN } from './useMailDomains';
 
-export const createMailDomain = async (name: string): Promise<MailDomain> => {
+export interface AddMailDomainParams {
+  name: string;
+}
+
+export const addMailDomain = async (
+  name: AddMailDomainParams['name'],
+): Promise<MailDomain> => {
   const response = await fetchAPI(`mail-domains/`, {
     method: 'POST',
     body: JSON.stringify({
@@ -23,19 +29,24 @@ export const createMailDomain = async (name: string): Promise<MailDomain> => {
   return response.json() as Promise<MailDomain>;
 };
 
-export function useCreateMailDomain({
+export const useAddMailDomain = ({
   onSuccess,
+  onError,
 }: {
   onSuccess: (data: MailDomain) => void;
-}) {
+  onError: (error: APIError) => void;
+}) => {
   const queryClient = useQueryClient();
   return useMutation<MailDomain, APIError, string>({
-    mutationFn: createMailDomain,
+    mutationFn: addMailDomain,
     onSuccess: (data) => {
       void queryClient.invalidateQueries({
         queryKey: [KEY_LIST_MAIL_DOMAIN],
       });
       onSuccess(data);
     },
+    onError: (error) => {
+      onError(error);
+    },
   });
-}
+};

src/frontend/apps/desk/src/features/mail-domains/api/useCreateMailbox.tsx

@@ -26,7 +26,6 @@ export const createMailbox = async ({
   });
 
   if (!response.ok) {
-    // TODO: extend errorCauses to return the name of the invalid field names to highlight in the form?
     throw new APIError(
       'Failed to create the mailbox',
       await errorCauses(response),
@@ -40,7 +39,7 @@ type UseCreateMailboxParams = { mailDomainSlug: string } & UseMutationOptions<
   CreateMailboxParams
 >;
 
-export function useCreateMailbox(options: UseCreateMailboxParams) {
+export const useCreateMailbox = (options: UseCreateMailboxParams) => {
   const queryClient = useQueryClient();
   return useMutation<void, APIError, CreateMailboxParams>({
     mutationFn: createMailbox,
@@ -61,4 +60,4 @@ export function useCreateMailbox(options: UseCreateMailboxParams) {
       }
     },
   });
-}
+};

src/frontend/apps/desk/src/features/mail-domains/components/MailDomainsContent.tsx

@@ -19,7 +19,7 @@ import { default as MailDomainsLogo } from '../assets/mail-domains-logo.svg';
 import { PAGE_SIZE } from '../conf';
 import { MailDomain, MailDomainMailbox } from '../types';
 
-import { CreateMailboxForm } from './forms/CreateMailboxForm';
+import { ModalCreateMailbox } from './ModalCreateMailbox';
 
 export type ViewMailbox = {
   name: string;
@@ -87,7 +87,7 @@ export function MailDomainsContent({ mailDomain }: { mailDomain: MailDomain }) {
   ) : (
     <>
       {isCreateMailboxFormVisible && mailDomain ? (
-        <CreateMailboxForm
+        <ModalCreateMailbox
           mailDomain={mailDomain}
           closeModal={() => setIsCreateMailboxFormVisible(false)}
         />