From 913162340a15f84ea28dad713476ac1437acace0 Mon Sep 17 00:00:00 2001 From: AimeurAmin Date: Wed, 12 Feb 2025 15:12:05 +0000 Subject: [PATCH 1/3] makes title field unique in contentful --- ...0250212150918-update-manuscript-title-to-unique.js | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 packages/contentful/migrations/crn/manuscripts/20250212150918-update-manuscript-title-to-unique.js diff --git a/packages/contentful/migrations/crn/manuscripts/20250212150918-update-manuscript-title-to-unique.js b/packages/contentful/migrations/crn/manuscripts/20250212150918-update-manuscript-title-to-unique.js new file mode 100644 index 0000000000..c5cc3eee65 --- /dev/null +++ b/packages/contentful/migrations/crn/manuscripts/20250212150918-update-manuscript-title-to-unique.js @@ -0,0 +1,11 @@ +module.exports.description = 'Makes title field of Manuscripts unique'; + +module.exports.up = (migration) => { + const manuscripts = migration.editContentType('manuscripts'); + manuscripts.editField('title').validations([{ unique: true }]); +}; + +module.exports.down = (migration) => { + const manuscripts = migration.editContentType('manuscripts'); + manuscripts.editField('title').validations([]); +}; From 151162360d26a74d837307f272143f69e39f24d5 Mon Sep 17 00:00:00 2001 From: AimeurAmin Date: Tue, 18 Feb 2025 15:55:39 +0000 Subject: [PATCH 2/3] handles manuscript duplicate title error --- .../network/teams/ManuscriptToastProvider.tsx | 4 + .../src/network/teams/TeamManuscript.tsx | 32 ++++++ .../crn-server/src/routes/manuscript.route.ts | 27 ++++- .../test/routes/manuscript.route.test.ts | 106 ++++++++++++++++++ .../src/templates/ManuscriptForm.tsx | 91 +++++++++------ .../__tests__/ManuscriptForm.test.tsx | 1 + 6 files changed, 221 insertions(+), 40 deletions(-) diff --git a/apps/crn-frontend/src/network/teams/ManuscriptToastProvider.tsx b/apps/crn-frontend/src/network/teams/ManuscriptToastProvider.tsx index e1af4e9b8b..2c9e997d65 100644 --- a/apps/crn-frontend/src/network/teams/ManuscriptToastProvider.tsx +++ b/apps/crn-frontend/src/network/teams/ManuscriptToastProvider.tsx @@ -4,6 +4,8 @@ import React, { createContext, useState } from 'react'; type FormType = | 'assigned-users' | 'manuscript' + | 'duplicate-manuscript' + | 'default-error' | 'compliance-report' | 'quick-check' | 'compliance-report-discussion' @@ -39,6 +41,8 @@ export const ManuscriptToastProvider = ({ const formTypeMapping = { 'assigned-users': 'User(s) assigned to a manuscript successfully.', manuscript: 'Manuscript submitted successfully.', + 'duplicate-manuscript': 'A manuscript with the same title already exists', + 'default-error': 'An error has occurred. Please try again later.', 'compliance-report': 'Compliance Report submitted successfully.', 'quick-check': 'Replied to quick check successfully.', 'compliance-report-discussion': 'Discussion started successfully.', diff --git a/apps/crn-frontend/src/network/teams/TeamManuscript.tsx b/apps/crn-frontend/src/network/teams/TeamManuscript.tsx index d9c87f1fd3..070a117b8a 100644 --- a/apps/crn-frontend/src/network/teams/TeamManuscript.tsx +++ b/apps/crn-frontend/src/network/teams/TeamManuscript.tsx @@ -30,6 +30,22 @@ type TeamManuscriptProps = { teamId: string; resubmitManuscript?: boolean; }; + +type ErrorDetails = { + details: string; + path: string[]; + value: string; + name: string; +}; + +type ManuscriptError = { + statusCode: number; + response?: { + errors?: ErrorDetails[]; + message: string; + }; +}; + const TeamManuscript: React.FC = ({ teamId, resubmitManuscript = false, @@ -60,6 +76,21 @@ const TeamManuscript: React.FC = ({ pushFromHere(path); }; + const onError = (error: ManuscriptError | Error) => { + if ( + 'statusCode' in error && + error.statusCode === 422 && + error.response?.errors?.length && + error.response?.errors.find( + (err: ErrorDetails) => err.path[1] === 'title' && err.name === 'unique', + ) + ) { + setFormType({ type: 'duplicate-manuscript', accent: 'error' }); + } else { + setFormType({ type: 'default-error', accent: 'error' }); + } + }; + const { teams: manuscriptTeams, labs: manuscriptLabs, @@ -103,6 +134,7 @@ const TeamManuscript: React.FC = ({ manuscriptId={manuscriptId} onSuccess={onSuccess} onCreate={createManuscript} + onError={onError} onUpdate={updateManuscript} onResubmit={handleResubmitManuscript} teamId={teamId} diff --git a/apps/crn-server/src/routes/manuscript.route.ts b/apps/crn-server/src/routes/manuscript.route.ts index cdfbed07d4..aabc4d51d3 100644 --- a/apps/crn-server/src/routes/manuscript.route.ts +++ b/apps/crn-server/src/routes/manuscript.route.ts @@ -91,12 +91,29 @@ export const manuscriptRouteFactory = ( if (!loggedInUser || !userBelongsToTeam) throw Boom.forbidden(); - const manuscript = await manuscriptController.create({ - ...createRequest, - userId: loggedInUser.id, - }); + try { + const manuscript = await manuscriptController.create({ + ...createRequest, + userId: loggedInUser.id, + }); - res.status(201).json(manuscript); + res.status(201).json(manuscript); + } catch (error) { + if (error instanceof Error) { + try { + const errorMessage = error.message as string; + const contentfulError = JSON.parse(errorMessage); + res.status(Number(contentfulError.status) || 500).json({ + message: contentfulError.message || 'An error occurred', + errors: contentfulError.details?.errors || [], + }); + return; + } catch { + throw error; + } + } + res.status(500).json({ message: 'Unexpected error' }); + } }); manuscriptRoutes.post<{ manuscriptId: string }>( diff --git a/apps/crn-server/test/routes/manuscript.route.test.ts b/apps/crn-server/test/routes/manuscript.route.test.ts index 09e645fce6..1f05a29772 100644 --- a/apps/crn-server/test/routes/manuscript.route.test.ts +++ b/apps/crn-server/test/routes/manuscript.route.test.ts @@ -127,6 +127,112 @@ describe('/manuscripts/ route', () => { expect(response.status).toEqual(403); }); + test('Should return a Contentful error when Contentful API fails', async () => { + const teamId = 'team-1'; + + const createManuscriptRequest: ManuscriptPostRequest = { + ...getManuscriptPostBody(), + teamId, + }; + + userMockFactory.mockReturnValueOnce({ + ...createUserResponse(), + teams: [ + { + role: 'Key Personnel', + displayName: 'Test 1', + id: teamId, + }, + ], + }); + const contentfulError = { + status: 400, + message: 'Invalid request to Contentful', + details: { + errors: [{ message: 'Field X is required' }], + }, + }; + + manuscriptControllerMock.create.mockRejectedValueOnce( + new Error(JSON.stringify(contentfulError)), + ); + + const response = await supertest(app) + .post('/manuscripts') + .send(createManuscriptRequest) + .set('Accept', 'application/json'); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ + message: 'Invalid request to Contentful', + errors: contentfulError.details.errors, + }); + }); + + test('Should throw a generic error when an unexpected error occurs', async () => { + const teamId = 'team-1'; + + const createManuscriptRequest: ManuscriptPostRequest = { + ...getManuscriptPostBody(), + teamId, + }; + + userMockFactory.mockReturnValueOnce({ + ...createUserResponse(), + teams: [ + { + role: 'Key Personnel', + displayName: 'Test 1', + id: teamId, + }, + ], + }); + + manuscriptControllerMock.create.mockRejectedValueOnce( + new Error('Unexpected error'), + ); + + const response = await supertest(app) + .post('/manuscripts') + .send(createManuscriptRequest) + .set('Accept', 'application/json'); + + expect(response.status).toBe(500); + expect(response.body).toHaveProperty('message', 'Unexpected error'); + }); + + test('Should throw an unexpected error if it is not an instance of Error', async () => { + const teamId = 'team-1'; + + const createManuscriptRequest: ManuscriptPostRequest = { + ...getManuscriptPostBody(), + teamId, + }; + + userMockFactory.mockReturnValueOnce({ + ...createUserResponse(), + teams: [ + { + role: 'Key Personnel', + displayName: 'Test 1', + id: teamId, + }, + ], + }); + + manuscriptControllerMock.create.mockRejectedValueOnce( + 'NonErrorException error', + ); + + const response = await supertest(app) + .post('/manuscripts') + .send(createManuscriptRequest) + .set('Accept', 'application/json'); + + expect(response.status).toBe(500); + expect(response.body).toHaveProperty('message', 'Unexpected error'); + }); + test('Should return a 201 and pass input to the controller', async () => { const teamId = 'team-1'; diff --git a/packages/react-components/src/templates/ManuscriptForm.tsx b/packages/react-components/src/templates/ManuscriptForm.tsx index 87856b53b2..12e14d16e5 100644 --- a/packages/react-components/src/templates/ManuscriptForm.tsx +++ b/packages/react-components/src/templates/ManuscriptForm.tsx @@ -83,6 +83,21 @@ const apcCoverageLifecycles = [ 'Publication with addendum or corrigendum', ]; +type ErrorDetails = { + details: string; + path: string[]; + value: string; + name: string; +}; + +type ManuscriptError = { + statusCode: number; + response?: { + errors?: ErrorDetails[]; + message: string; + }; +}; + type OptionalVersionFields = Array< keyof Omit< ManuscriptVersion, @@ -292,6 +307,7 @@ type ManuscriptFormProps = Omit< firstAuthors?: AuthorSelectOption[]; correspondingAuthor?: AuthorSelectOption[]; additionalAuthors?: AuthorSelectOption[]; + onError: (error: ManuscriptError | Error) => void; }; const ManuscriptForm: React.FC = ({ @@ -300,6 +316,7 @@ const ManuscriptForm: React.FC = ({ onUpdate, onResubmit, onSuccess, + onError, handleFileUpload, teamId, title, @@ -577,42 +594,46 @@ const ManuscriptForm: React.FC = ({ additionalAuthorsEmails, ), }; - if (!manuscriptId) { - await onCreate({ - ...data, - teamId, - eligibilityReasons: [...eligibilityReasons], - versions: [ - { - ...requestVersionData, - ...versionDataPayload, - }, - ], - }); - } else if (resubmitManuscript) { - await onResubmit(manuscriptId, { - title: data.title, - teamId, - versions: [ - { - ...requestVersionData, - ...versionDataPayload, - }, - ], - }); - } else { - await onUpdate(manuscriptId, { - title: data.title, - teamId, - versions: [ - { - ...requestVersionData, - ...versionDataPayload, - }, - ], - }); + try { + if (!manuscriptId) { + await onCreate({ + ...data, + teamId, + eligibilityReasons: [...eligibilityReasons], + versions: [ + { + ...requestVersionData, + ...versionDataPayload, + }, + ], + }); + } else if (resubmitManuscript) { + await onResubmit(manuscriptId, { + title: data.title, + teamId, + versions: [ + { + ...requestVersionData, + ...versionDataPayload, + }, + ], + }); + } else { + await onUpdate(manuscriptId, { + title: data.title, + teamId, + versions: [ + { + ...requestVersionData, + ...versionDataPayload, + }, + ], + }); + } + onSuccess(); + } catch (error) { + onError(error as ManuscriptError | Error); } - onSuccess(); } }; diff --git a/packages/react-components/src/templates/__tests__/ManuscriptForm.test.tsx b/packages/react-components/src/templates/__tests__/ManuscriptForm.test.tsx index 5b45f1e1d7..c7317fd6ef 100644 --- a/packages/react-components/src/templates/__tests__/ManuscriptForm.test.tsx +++ b/packages/react-components/src/templates/__tests__/ManuscriptForm.test.tsx @@ -86,6 +86,7 @@ const defaultProps: ComponentProps = { additionalAuthors: [], submitterName: 'John Doe', submissionDate: new Date('2024-10-01'), + onError: jest.fn(), }; const submitForm = async () => { From b52c28711cd6f472ad4ed9297464d3fdc9324d45 Mon Sep 17 00:00:00 2001 From: AimeurAmin Date: Wed, 19 Feb 2025 15:19:36 +0000 Subject: [PATCH 3/3] adds missing tests --- .../teams/__tests__/TeamManuscript.test.tsx | 207 +++++++++++++++++- 1 file changed, 206 insertions(+), 1 deletion(-) diff --git a/apps/crn-frontend/src/network/teams/__tests__/TeamManuscript.test.tsx b/apps/crn-frontend/src/network/teams/__tests__/TeamManuscript.test.tsx index e818e6db7a..084e3496b2 100644 --- a/apps/crn-frontend/src/network/teams/__tests__/TeamManuscript.test.tsx +++ b/apps/crn-frontend/src/network/teams/__tests__/TeamManuscript.test.tsx @@ -28,7 +28,7 @@ import TeamManuscript from '../TeamManuscript'; const manuscriptResponse = { id: '1', title: 'The Manuscript' }; const teamId = '42'; -const history = createMemoryHistory({ +let history = createMemoryHistory({ initialEntries: [ network({}).teams({}).team({ teamId }).workspace({}).createManuscript({}).$, ], @@ -56,6 +56,13 @@ beforeEach(() => { jest.resetModules(); jest.spyOn(console, 'error').mockImplementation(); + + history = createMemoryHistory({ + initialEntries: [ + network({}).teams({}).team({ teamId }).workspace({}).createManuscript({}) + .$, + ], + }); }); const renderPage = async ( @@ -248,6 +255,204 @@ it('can publish a form when the data is valid and navigates to team workspace', }); }, 180_000); +it('shows duplicate manuscript toast when submitting with duplicate title', async () => { + const duplicateTitleError = { + statusCode: 422, + response: { + errors: [ + { + path: ['versions', 'title'], + name: 'unique', + details: 'Title must be unique', + value: 'The Manuscript', + }, + ], + message: 'Validation Error', + }, + }; + + (createManuscript as jest.Mock).mockRejectedValueOnce(duplicateTitleError); + const title = 'The Manuscript'; + + await renderPage(); + + userEvent.type( + screen.getByRole('textbox', { name: /title of manuscript/i }), + title, + ); + const typeTextbox = screen.getByRole('textbox', { + name: /Type of Manuscript/i, + }); + userEvent.type(typeTextbox, 'Original'); + userEvent.type(typeTextbox, specialChars.enter); + typeTextbox.blur(); + + const lifecycleTextbox = screen.getByRole('textbox', { + name: /Where is the manuscript in the life cycle/i, + }); + userEvent.type(lifecycleTextbox, 'Typeset proof'); + userEvent.type(lifecycleTextbox, specialChars.enter); + lifecycleTextbox.blur(); + + const apcCoverage = screen.getByRole('group', { + name: /Will you be requesting APC coverage/i, + }); + + userEvent.click(within(apcCoverage).getByRole('radio', { name: /no/i })); + + const testFile = new File(['file content'], 'file.txt', { + type: 'text/plain', + }); + const manuscriptFileInput = screen.getByLabelText(/Upload Manuscript File/i); + const keyResourceTableInput = screen.getByLabelText( + /Upload Key Resource Table/i, + ); + + const descriptionTextbox = screen.getByRole('textbox', { + name: /Manuscript Description/i, + }); + userEvent.type(descriptionTextbox, 'Some description'); + + userEvent.type(screen.getByLabelText(/First Authors/i), 'Jane Doe'); + + await waitFor(() => + expect(screen.queryByText(/loading/i)).not.toBeInTheDocument(), + ); + + userEvent.click(screen.getByText(/Non CRN/i)); + + expect(screen.getByText(/Jane Doe Email/i)).toBeInTheDocument(); + userEvent.type(screen.getByLabelText(/Jane Doe Email/i), 'jane@doe.com'); + + userEvent.upload(manuscriptFileInput, testFile); + userEvent.upload(keyResourceTableInput, testFile); + + const submitButton = screen.getByRole('button', { name: /Submit/ }); + + await waitFor(() => { + expect(submitButton).toBeEnabled(); + }); + + const quickChecks = screen.getByRole('region', { name: /quick checks/i }); + + within(quickChecks) + .getAllByText('Yes') + .forEach((button) => { + userEvent.click(button); + }); + + await act(async () => { + await userEvent.click( + await screen.findByRole('button', { name: /Submit/ }), + ); + }); + + await userEvent.click( + screen.getByRole('button', { name: /Submit Manuscript/ }), + ); + + await waitFor(() => { + expect( + screen.getByText('A manuscript with the same title already exists'), + ).toBeInTheDocument(); + }); +}, 180_000); + +it('shows default error toast when submitting with any other error', async () => { + const genericError = { + statusCode: 500, + response: { + message: 'Internal Server Error', + }, + }; + + (createManuscript as jest.Mock).mockRejectedValueOnce(genericError); + const title = 'The Manuscript'; + + await renderPage(); + + userEvent.type( + screen.getByRole('textbox', { name: /title of manuscript/i }), + title, + ); + const typeTextbox = screen.getByRole('textbox', { + name: /Type of Manuscript/i, + }); + userEvent.type(typeTextbox, 'Original'); + userEvent.type(typeTextbox, specialChars.enter); + typeTextbox.blur(); + + const lifecycleTextbox = screen.getByRole('textbox', { + name: /Where is the manuscript in the life cycle/i, + }); + userEvent.type(lifecycleTextbox, 'Typeset proof'); + userEvent.type(lifecycleTextbox, specialChars.enter); + lifecycleTextbox.blur(); + + const apcCoverage = screen.getByRole('group', { + name: /Will you be requesting APC coverage/i, + }); + + userEvent.click(within(apcCoverage).getByRole('radio', { name: /no/i })); + + const testFile = new File(['file content'], 'file.txt', { + type: 'text/plain', + }); + const manuscriptFileInput = screen.getByLabelText(/Upload Manuscript File/i); + const keyResourceTableInput = screen.getByLabelText( + /Upload Key Resource Table/i, + ); + + const descriptionTextbox = screen.getByRole('textbox', { + name: /Manuscript Description/i, + }); + userEvent.type(descriptionTextbox, 'Some description'); + + userEvent.type(screen.getByLabelText(/First Authors/i), 'Jane Doe'); + + await waitFor(() => + expect(screen.queryByText(/loading/i)).not.toBeInTheDocument(), + ); + + userEvent.click(screen.getByText(/Non CRN/i)); + + expect(screen.getByText(/Jane Doe Email/i)).toBeInTheDocument(); + userEvent.type(screen.getByLabelText(/Jane Doe Email/i), 'jane@doe.com'); + + userEvent.upload(manuscriptFileInput, testFile); + userEvent.upload(keyResourceTableInput, testFile); + + const submitButton = screen.getByRole('button', { name: /Submit/ }); + + await waitFor(() => { + expect(submitButton).toBeEnabled(); + }); + + const quickChecks = screen.getByRole('region', { name: /quick checks/i }); + + within(quickChecks) + .getAllByText('Yes') + .forEach((button) => { + userEvent.click(button); + }); + + await act(async () => { + await userEvent.click( + await screen.findByRole('button', { name: /Submit/ }), + ); + }); + + await userEvent.click( + screen.getByRole('button', { name: /Submit Manuscript/ }), + ); + + await waitFor(() => { + expect( + screen.getByText('An error has occurred. Please try again later.'), + ).toBeInTheDocument(); + }); +}, 180_000); + it('can resubmit a manuscript and navigates to team workspace', async () => { const mockResubmitManuscript = resubmitManuscript as jest.MockedFunction< typeof resubmitManuscript