Skip to content

Commit

Permalink
Merge pull request #3864 from opengovsg/form-v2/feat/refactor-collab-…
Browse files Browse the repository at this point in the history
…error-messages

feat(v2): add clearer error messages when updating collaborators
  • Loading branch information
siddarth2824 authored May 19, 2022
2 parents b946cce + 133fcfa commit c8e77f6
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Meta, Story } from '@storybook/react'
import {
createFormBuilderMocks,
getAdminFormCollaborators,
updateFormCollaborators,
} from '~/mocks/msw/handlers/admin-form'
import { getUser, MOCK_USER } from '~/mocks/msw/handlers/user'

Expand Down Expand Up @@ -164,3 +165,11 @@ ViewerViewLoading.parameters = {
}),
],
}

export const EditCollaboratorBadRequestError = Template.bind({})
EditCollaboratorBadRequestError.parameters = {
msw: [
updateFormCollaborators({ delay: 0, errorCode: 400 }),
...baseMswRoutes,
],
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { composeStories } from '@storybook/testing-react'
import { act, render, screen, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'

import * as stories from './CollaboratorModal.stories'

const { EditCollaboratorBadRequestError } = composeStories(stories)

describe('400 bad request error', () => {
it('should throw the appropriate 400 error message when user attempts to add collaborator', async () => {
await act(async () => {
render(<EditCollaboratorBadRequestError />)
})

// Wait until msw has fetched all the necessary resources
await waitFor(() =>
expect(screen.getByText(/manage collaborators/i)).toBeVisible(),
)

const input = screen.getByPlaceholderText(/me@example.com/i)
const submitButton = screen.getByRole('button', {
name: /add collaborator/i,
})
const mockEmail = '[email protected]'

await act(async () => userEvent.type(input, mockEmail))
await act(async () => userEvent.click(submitButton))

await screen.findByRole('alert')

expect(screen.getByRole('alert').textContent).toBe(
'The collaborator was unable to be added or edited. Please try again or refresh the page.',
)
})
})
109 changes: 102 additions & 7 deletions frontend/src/features/admin-form/common/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {

import { ROOT_ROUTE } from '~constants/routes'
import { useToast } from '~hooks/useToast'
import { HttpError } from '~services/ApiService'

import { permissionsToRole } from './components/CollaboratorModal/utils'
import {
Expand All @@ -29,6 +30,14 @@ export type MutateRemoveCollaboratorArgs = {
currentPermissions: FormPermissionsDto
}

enum FormCollaboratorAction {
UPDATE,
ADD,
REMOVE,
TRANSFER_OWNERSHIP,
REMOVE_SELF,
}

export const useMutateCollaborators = () => {
const { formId } = useParams()
if (!formId) throw new Error('No formId provided')
Expand All @@ -55,6 +64,74 @@ export const useMutateCollaborators = () => {
[formId, queryClient],
)

const getMappedBadRequestErrorMessage = (
formCollaboratorAction: FormCollaboratorAction,
): string => {
let badRequestErrorMessage
switch (formCollaboratorAction) {
case FormCollaboratorAction.ADD:
badRequestErrorMessage = `The collaborator was unable to be added or edited. Please try again or refresh the page.`
break
default:
badRequestErrorMessage = `Sorry, an error occurred. Please refresh the page and try again later.`
}

return badRequestErrorMessage
}

const getMappedDefaultErrorMessage = (
formCollaboratorAction: FormCollaboratorAction,
): string => {
let defaultErrorMessage
switch (formCollaboratorAction) {
case FormCollaboratorAction.ADD:
defaultErrorMessage = 'Error adding collaborator.'
break
case FormCollaboratorAction.UPDATE:
defaultErrorMessage = 'Error updating collaborator.'
break
case FormCollaboratorAction.REMOVE:
defaultErrorMessage = 'Error removing collaborator.'
break
case FormCollaboratorAction.REMOVE_SELF:
defaultErrorMessage = 'Error removing self.'
break
case FormCollaboratorAction.TRANSFER_OWNERSHIP:
defaultErrorMessage = 'Error transfering form ownership.'
break
//should not reach
default:
defaultErrorMessage = 'Error.'
}
return defaultErrorMessage
}

const getMappedErrorMessage = (
error: Error,
formCollaboratorAction: FormCollaboratorAction,
requestEmail?: string,
): string => {
// check if error is an instance of HttpError to be able to access status code of error
if (error instanceof HttpError) {
let errorMessage
switch (error.code) {
case 422:
errorMessage = requestEmail
? `${requestEmail} is not part of a whitelisted agency`
: `An unexpected error 422 happened`
break
case 400:
errorMessage = getMappedBadRequestErrorMessage(formCollaboratorAction)
break
default:
errorMessage = getMappedDefaultErrorMessage(formCollaboratorAction)
}
return errorMessage
}
// if error is not of type HttpError return the error message encapsulated in Error object
return error.message
}

const handleSuccess = useCallback(
({
newData,
Expand All @@ -75,10 +152,18 @@ export const useMutateCollaborators = () => {
)

const handleError = useCallback(
(error: Error) => {
(
error: Error,
formCollaboratorAction: FormCollaboratorAction,
requestEmail?: string,
) => {
toast.closeAll()
toast({
description: error.message,
description: getMappedErrorMessage(
error,
formCollaboratorAction,
requestEmail,
),
status: 'danger',
})
},
Expand Down Expand Up @@ -113,7 +198,9 @@ export const useMutateCollaborators = () => {
} has been updated to the ${permissionsToRole(permissionToUpdate)} role`
handleSuccess({ newData, toastDescription })
},
onError: handleError,
onError: (error: Error) => {
handleError(error, FormCollaboratorAction.UPDATE)
},
},
)

Expand All @@ -129,7 +216,9 @@ export const useMutateCollaborators = () => {
} has been added as a ${permissionsToRole(newPermission)}`
handleSuccess({ newData, toastDescription })
},
onError: handleError,
onError: (error: Error, { newPermission }) => {
handleError(error, FormCollaboratorAction.ADD, newPermission.email)
},
},
)

Expand All @@ -149,7 +238,9 @@ export const useMutateCollaborators = () => {
const toastDescription = `${permissionToRemove.email} has been removed as a collaborator`
handleSuccess({ newData, toastDescription })
},
onError: handleError,
onError: (error: Error) => {
handleError(error, FormCollaboratorAction.REMOVE)
},
},
)

Expand All @@ -173,7 +264,9 @@ export const useMutateCollaborators = () => {
newData.form,
)
},
onError: handleError,
onError: (error: Error) => {
handleError(error, FormCollaboratorAction.TRANSFER_OWNERSHIP)
},
},
)

Expand All @@ -189,7 +282,9 @@ export const useMutateCollaborators = () => {
// Remove all related queries from cache.
queryClient.removeQueries(adminFormKeys.id(formId))
},
onError: handleError,
onError: (error: Error) => {
handleError(error, FormCollaboratorAction.REMOVE_SELF)
},
},
)

Expand Down
15 changes: 15 additions & 0 deletions frontend/src/mocks/msw/handlers/admin-form/collaborators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,18 @@ export const getAdminFormCollaborators = ({
},
)
}

export const updateFormCollaborators = ({
delay = 0,
errorCode,
}: {
delay?: number | 'infinite'
errorCode: number
}): ReturnType<typeof rest['put']> => {
return rest.put<FormPermissionsDto>(
'/api/v3/admin/forms/:formId/collaborators',
(_req, res, ctx) => {
return res(ctx.delay(delay), ctx.status(errorCode), ctx.json([]))
},
)
}

0 comments on commit c8e77f6

Please sign in to comment.