diff --git a/docs/openapi/index.yaml b/docs/openapi/index.yaml index 97b810fad4..6e0f2c523e 100644 --- a/docs/openapi/index.yaml +++ b/docs/openapi/index.yaml @@ -85,6 +85,8 @@ components: description: Topics covered during the TTA items: type: string + approvingManager: + $ref: '#/components/schemas/user' pageState: type: object enum: ['Not Started', 'In progress', 'Completed'] @@ -100,7 +102,11 @@ components: type: string status: type: string - enum: ['draft', 'submitted'] + enum: + - draft + - submitted + - approved + - needs_action description: The current state of the report ttaType: type: array diff --git a/docs/openapi/paths/activity-reports/review.yaml b/docs/openapi/paths/activity-reports/review.yaml index 0156f6d371..b0df1209ab 100644 --- a/docs/openapi/paths/activity-reports/review.yaml +++ b/docs/openapi/paths/activity-reports/review.yaml @@ -18,8 +18,8 @@ put: type: string description: The status of the report after review enum: - - Approved - - Needs Action + - approved + - needs_action managerNotes: type: string description: Any notes the manager needs to relay to the author/collaborators of the report @@ -40,7 +40,7 @@ put: status: type: string enum: - - Approved - - Needs Action + - approved + - needs_action managerNotes: type: string diff --git a/frontend/src/Constants.js b/frontend/src/Constants.js index df2b978d90..828fa877bc 100644 --- a/frontend/src/Constants.js +++ b/frontend/src/Constants.js @@ -66,3 +66,15 @@ export const REGIONS = [ ]; export const DECIMAL_BASE = 10; + +export const managerReportStatuses = [ + 'needs_action', + 'approved', +]; + +export const REPORT_STATUSES = { + DRAFT: 'draft', + SUBMITTED: 'submitted', + NEEDS_ACTION: 'needs_action', + APPROVED: 'approved', +}; diff --git a/frontend/src/components/Navigator/__tests__/index.js b/frontend/src/components/Navigator/__tests__/index.js index 8c6d6066b8..58365a0530 100644 --- a/frontend/src/components/Navigator/__tests__/index.js +++ b/frontend/src/components/Navigator/__tests__/index.js @@ -61,9 +61,10 @@ describe('Navigator', () => { const renderNavigator = (currentPage = 'first', onSubmit = () => {}, onSave = () => {}) => { render( {}} onReview={() => {}} approvingManager={false} defaultValues={{ first: '', second: '' }} @@ -80,7 +81,7 @@ describe('Navigator', () => { const firstInput = screen.getByTestId('first'); userEvent.click(firstInput); const first = await screen.findByRole('button', { name: 'first page' }); - await waitFor(() => expect(within(first).getByText('In progress')).toBeVisible()); + await waitFor(() => expect(within(first).getByText('In Progress')).toBeVisible()); }); it('onContinue calls onSave with correct page position', async () => { diff --git a/frontend/src/components/Navigator/components/SideNav.js b/frontend/src/components/Navigator/components/SideNav.js index 7de4990ba3..06fb7337cc 100644 --- a/frontend/src/components/Navigator/components/SideNav.js +++ b/frontend/src/components/Navigator/components/SideNav.js @@ -5,6 +5,7 @@ */ import React from 'react'; import PropTypes from 'prop-types'; +import { startCase } from 'lodash'; import Sticky from 'react-stickynode'; import { Button, Tag, Alert } from '@trussworks/react-uswds'; import { useMediaQuery } from 'react-responsive'; @@ -12,8 +13,9 @@ import moment from 'moment'; import Container from '../../Container'; import './SideNav.css'; +import { REPORT_STATUSES } from '../../../Constants'; import { - NOT_STARTED, IN_PROGRESS, COMPLETE, SUBMITTED, APPROVED, NEEDS_ACTION, + NOT_STARTED, IN_PROGRESS, COMPLETE, } from '../constants'; const tagClass = (state) => { @@ -24,11 +26,11 @@ const tagClass = (state) => { return 'smart-hub--tag-in-progress'; case COMPLETE: return 'smart-hub--tag-complete'; - case SUBMITTED: + case REPORT_STATUSES.SUBMITTED: return 'smart-hub--tag-submitted'; - case APPROVED: + case REPORT_STATUSES.APPROVED: return 'smart-hub--tag-submitted'; - case NEEDS_ACTION: + case REPORT_STATUSES.NEEDS_ACTION: return 'smart-hub--tag-needs-action'; default: return ''; @@ -50,10 +52,10 @@ function SideNav({ > {page.label} - {page.state !== 'draft' + {page.state !== REPORT_STATUSES.DRAFT && ( - {page.state} + {startCase(page.state)} )} diff --git a/frontend/src/components/Navigator/components/__tests__/SideNav.js b/frontend/src/components/Navigator/components/__tests__/SideNav.js index d4b53b459c..ae84297d00 100644 --- a/frontend/src/components/Navigator/components/__tests__/SideNav.js +++ b/frontend/src/components/Navigator/components/__tests__/SideNav.js @@ -7,9 +7,11 @@ import { import SideNav from '../SideNav'; import { - NOT_STARTED, IN_PROGRESS, COMPLETE, SUBMITTED, + NOT_STARTED, IN_PROGRESS, COMPLETE, } from '../../constants'; +import { REPORT_STATUSES } from '../../../../Constants'; + describe('SideNav', () => { const renderNav = (state, onNavigation = () => {}, current = false) => { const pages = [ @@ -39,14 +41,14 @@ describe('SideNav', () => { describe('displays the correct status', () => { it('not started', () => { renderNav(NOT_STARTED); - const notStarted = screen.getByText('Not started'); + const notStarted = screen.getByText('Not Started'); expect(notStarted).toHaveClass('smart-hub--tag-not-started'); expect(notStarted).toBeVisible(); }); it('in progress', () => { renderNav(IN_PROGRESS); - const inProgress = screen.getByText('In progress'); + const inProgress = screen.getByText('In Progress'); expect(inProgress).toHaveClass('smart-hub--tag-in-progress'); expect(inProgress).toBeVisible(); }); @@ -59,7 +61,7 @@ describe('SideNav', () => { }); it('submitted', () => { - renderNav(SUBMITTED); + renderNav(REPORT_STATUSES.SUBMITTED); const submitted = screen.getByText('Submitted'); expect(submitted).toHaveClass('smart-hub--tag-submitted'); expect(submitted).toBeVisible(); @@ -69,13 +71,13 @@ describe('SideNav', () => { it('clicking a nav item calls onNavigation', () => { const onNav = jest.fn(); renderNav(NOT_STARTED, onNav); - const notStarted = screen.getByText('Not started'); + const notStarted = screen.getByText('Not Started'); userEvent.click(notStarted); expect(onNav).toHaveBeenCalled(); }); it('the currently selected page has the current class', () => { - renderNav(SUBMITTED, () => {}, true); + renderNav(REPORT_STATUSES.SUBMITTED, () => {}, true); const submitted = screen.getByRole('button', { name: 'test' }); expect(submitted).toHaveClass('smart-hub--navigator-link-active'); }); diff --git a/frontend/src/components/Navigator/constants.js b/frontend/src/components/Navigator/constants.js index b644ef86cb..7c98318b85 100644 --- a/frontend/src/components/Navigator/constants.js +++ b/frontend/src/components/Navigator/constants.js @@ -1,6 +1,3 @@ export const NOT_STARTED = 'Not started'; export const IN_PROGRESS = 'In progress'; export const COMPLETE = 'Complete'; -export const SUBMITTED = 'Submitted'; -export const APPROVED = 'Approved'; -export const NEEDS_ACTION = 'Needs Action'; diff --git a/frontend/src/components/Navigator/index.js b/frontend/src/components/Navigator/index.js index 67ac22cd75..494cd9afbc 100644 --- a/frontend/src/components/Navigator/index.js +++ b/frontend/src/components/Navigator/index.js @@ -22,7 +22,8 @@ import SideNav from './components/SideNav'; import NavigatorHeader from './components/NavigatorHeader'; function Navigator({ - initialData, + formData, + updateFormData, initialLastUpdated, pages, onFormSubmit, @@ -32,10 +33,8 @@ function Navigator({ onSave, autoSaveInterval, approvingManager, - status, reportId, }) { - const [formData, updateFormData] = useState(initialData); const [errorMessage, updateErrorMessage] = useState(); const [lastSaveTime, updateLastSaveTime] = useState(initialLastUpdated); const { pageState } = formData; @@ -105,7 +104,7 @@ function Navigator({ const navigatorPages = pages.map((p) => { const current = p.position === page.position; const stateOfPage = current ? IN_PROGRESS : pageState[p.position]; - const state = p.review ? status : stateOfPage; + const state = p.review ? formData.status : stateOfPage; return { label: p.label, onNavigation: () => onSaveForm(false, p.position), @@ -136,7 +135,6 @@ function Navigator({ additionalData, onReview, approvingManager, - reportId, )} {!page.review && ( @@ -161,11 +159,14 @@ function Navigator({ } Navigator.propTypes = { - initialData: PropTypes.shape({}), + formData: PropTypes.shape({ + status: PropTypes.string, + pageState: PropTypes.shape({}), + }).isRequired, + updateFormData: PropTypes.func.isRequired, initialLastUpdated: PropTypes.instanceOf(moment), onFormSubmit: PropTypes.func.isRequired, onSave: PropTypes.func.isRequired, - status: PropTypes.string.isRequired, onReview: PropTypes.func.isRequired, approvingManager: PropTypes.bool.isRequired, pages: PropTypes.arrayOf( @@ -184,7 +185,6 @@ Navigator.propTypes = { }; Navigator.defaultProps = { - initialData: {}, additionalData: {}, autoSaveInterval: 1000 * 60 * 2, initialLastUpdated: null, diff --git a/frontend/src/pages/ActivityReport/Pages/Review/Approver/Approved.js b/frontend/src/pages/ActivityReport/Pages/Review/Approver/Approved.js new file mode 100644 index 0000000000..3882710861 --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/Approver/Approved.js @@ -0,0 +1,39 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +const Approved = ({ + additionalNotes, + managerNotes, +}) => ( + <> +

Report approved

+
+

+ Creator notes +
+
+ { additionalNotes || 'No creator notes' } +

+
+
+

+ Manager notes +
+
+ { managerNotes || 'No manager notes' } +

+
+ +); + +Approved.propTypes = { + additionalNotes: PropTypes.string, + managerNotes: PropTypes.string, +}; + +Approved.defaultProps = { + additionalNotes: '', + managerNotes: '', +}; + +export default Approved; diff --git a/frontend/src/pages/ActivityReport/Pages/ApproverReviewPage.js b/frontend/src/pages/ActivityReport/Pages/Review/Approver/Review.js similarity index 79% rename from frontend/src/pages/ActivityReport/Pages/ApproverReviewPage.js rename to frontend/src/pages/ActivityReport/Pages/Review/Approver/Review.js index 96856aa8ec..05138c6df1 100644 --- a/frontend/src/pages/ActivityReport/Pages/ApproverReviewPage.js +++ b/frontend/src/pages/ActivityReport/Pages/Review/Approver/Review.js @@ -1,15 +1,13 @@ import React from 'react'; import PropTypes from 'prop-types'; +import _ from 'lodash'; import { Dropdown, Form, Label, Fieldset, Textarea, Alert, Button, } from '@trussworks/react-uswds'; -const possibleStatus = [ - 'Approved', - 'Needs Action', -]; +import { managerReportStatuses } from '../../../../../Constants'; -const ApproverReviewPage = ({ +const Review = ({ reviewed, additionalNotes, register, @@ -27,12 +25,12 @@ const ApproverReviewPage = ({ )}

Review and approve report

-
+

Creator notes

- { additionalNotes || 'No creator notes' } + { additionalNotes || 'No creator notes'}

@@ -43,8 +41,8 @@ const ApproverReviewPage = ({ - {possibleStatus.map((status) => ( - + {managerReportStatuses.map((status) => ( + ))} @@ -52,7 +50,7 @@ const ApproverReviewPage = ({ ); -ApproverReviewPage.propTypes = { +Review.propTypes = { reviewed: PropTypes.bool.isRequired, additionalNotes: PropTypes.string.isRequired, register: PropTypes.func.isRequired, @@ -61,4 +59,4 @@ ApproverReviewPage.propTypes = { onFormReview: PropTypes.func.isRequired, }; -export default ApproverReviewPage; +export default Review; diff --git a/frontend/src/pages/ActivityReport/Pages/Review/Approver/__tests__/index.js b/frontend/src/pages/ActivityReport/Pages/Review/Approver/__tests__/index.js new file mode 100644 index 0000000000..fa09c8966e --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/Approver/__tests__/index.js @@ -0,0 +1,82 @@ +import '@testing-library/jest-dom'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; +import { useForm } from 'react-hook-form'; + +import Approver from '../index'; +import { REPORT_STATUSES } from '../../../../../../Constants'; + +const RenderApprover = ({ + // eslint-disable-next-line react/prop-types + onFormReview, reviewed, valid, formData, +}) => { + const { register, handleSubmit } = useForm({ + mode: 'onChange', + defaultValues: formData, + }); + + return ( + + ); +}; + +const renderReview = (status, onFormReview, reviewed, valid, notes = '') => { + const formData = { + approvingManager: { name: 'name' }, + managerNotes: notes, + additionalNotes: notes, + approvingManagerId: '1', + status, + }; + render( + , + ); +}; + +describe('Approver review page', () => { + describe('when the report is submitted', () => { + it('displays the submit review component', async () => { + renderReview(REPORT_STATUSES.SUBMITTED, () => {}, false, true); + expect(await screen.findByText('Review and approve report')).toBeVisible(); + }); + + it('allows the approver to submit a review', async () => { + const mockSubmit = jest.fn(); + renderReview(REPORT_STATUSES.SUBMITTED, mockSubmit, true, true); + const dropdown = await screen.findByTestId('dropdown'); + userEvent.selectOptions(dropdown, 'approved'); + const button = await screen.findByRole('button'); + userEvent.click(button); + const alert = await screen.findByTestId('alert'); + expect(alert.textContent).toContain('Success'); + expect(mockSubmit).toHaveBeenCalled(); + }); + + it('handles empty notes', async () => { + renderReview(REPORT_STATUSES.SUBMITTED, () => {}, true, true); + const notes = await screen.findByLabelText('additionalNotes'); + expect(notes.textContent).toContain('No creator notes'); + }); + }); + + describe('when the report is approved', () => { + it('displays the approved component', async () => { + renderReview(REPORT_STATUSES.APPROVED, () => {}, false, true); + expect(await screen.findByText('Report approved')).toBeVisible(); + }); + }); +}); diff --git a/frontend/src/pages/ActivityReport/Pages/Review/Approver/index.js b/frontend/src/pages/ActivityReport/Pages/Review/Approver/index.js new file mode 100644 index 0000000000..aee8a7e675 --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/Approver/index.js @@ -0,0 +1,60 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import Review from './Review'; +import Approved from './Approved'; +import { REPORT_STATUSES } from '../../../../../Constants'; + +const Approver = ({ + register, + handleSubmit, + onFormReview, + reviewed, + formData, + valid, +}) => { + const { managerNotes, additionalNotes, status } = formData; + const review = status === REPORT_STATUSES.SUBMITTED || status === REPORT_STATUSES.NEEDS_ACTION; + const approved = status === REPORT_STATUSES.APPROVED; + + return ( + <> + {review + && ( + + )} + {approved + && ( + + )} + + ); +}; + +Approver.propTypes = { + register: PropTypes.func.isRequired, + handleSubmit: PropTypes.func.isRequired, + onFormReview: PropTypes.func.isRequired, + reviewed: PropTypes.bool.isRequired, + valid: PropTypes.bool.isRequired, + formData: PropTypes.shape({ + approvingManager: PropTypes.shape({ + name: PropTypes.string, + }), + managerNotes: PropTypes.string, + additionalNotes: PropTypes.string, + status: PropTypes.string, + }).isRequired, +}; + +export default Approver; diff --git a/frontend/src/pages/ActivityReport/Pages/Review/Submitter/Approved.js b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/Approved.js new file mode 100644 index 0000000000..3882710861 --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/Approved.js @@ -0,0 +1,39 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +const Approved = ({ + additionalNotes, + managerNotes, +}) => ( + <> +

Report approved

+
+

+ Creator notes +
+
+ { additionalNotes || 'No creator notes' } +

+
+
+

+ Manager notes +
+
+ { managerNotes || 'No manager notes' } +

+
+ +); + +Approved.propTypes = { + additionalNotes: PropTypes.string, + managerNotes: PropTypes.string, +}; + +Approved.defaultProps = { + additionalNotes: '', + managerNotes: '', +}; + +export default Approved; diff --git a/frontend/src/pages/ActivityReport/Pages/SubmitterReviewPage.js b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/Draft.js similarity index 80% rename from frontend/src/pages/ActivityReport/Pages/SubmitterReviewPage.js rename to frontend/src/pages/ActivityReport/Pages/Review/Submitter/Draft.js index 62cf6e7609..bf2543689a 100644 --- a/frontend/src/pages/ActivityReport/Pages/SubmitterReviewPage.js +++ b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/Draft.js @@ -5,9 +5,9 @@ import { Dropdown, Form, Label, Fieldset, Textarea, Alert, Button, } from '@trussworks/react-uswds'; -import { DECIMAL_BASE } from '../../../Constants'; +import { DECIMAL_BASE } from '../../../../../Constants'; -const SubmitterReviewPage = ({ +const Draft = ({ submitted, allComplete, register, @@ -25,21 +25,21 @@ const SubmitterReviewPage = ({ return ( <> {submitted - && ( - - Success -
- This report was successfully submitted for approval - - )} + && ( + + Success +
+ This report was successfully submitted for approval + + )} {!allComplete - && ( - - Incomplete report -
- This report cannot be submitted until all sections are complete - - )} + && ( + + Incomplete report +
+ This report cannot be submitted until all sections are complete + + )}

Submit Report

@@ -66,7 +66,7 @@ const SubmitterReviewPage = ({ ); }; -SubmitterReviewPage.propTypes = { +Draft.propTypes = { submitted: PropTypes.bool.isRequired, allComplete: PropTypes.bool.isRequired, register: PropTypes.func.isRequired, @@ -79,4 +79,4 @@ SubmitterReviewPage.propTypes = { onFormSubmit: PropTypes.func.isRequired, }; -export default SubmitterReviewPage; +export default Draft; diff --git a/frontend/src/pages/ActivityReport/Pages/Review/Submitter/NeedsAction.js b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/NeedsAction.js new file mode 100644 index 0000000000..e7da7eeb78 --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/NeedsAction.js @@ -0,0 +1,82 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Alert, Button } from '@trussworks/react-uswds'; +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; +import { faInfoCircle } from '@fortawesome/free-solid-svg-icons'; + +const NeedsAction = ({ + additionalNotes, + managerNotes, + onSubmit, + approvingManager, + valid, +}) => { + const submit = async () => { + await onSubmit({ approvingManagerId: approvingManager.id, additionalNotes }); + }; + + return ( + <> + + + { approvingManager.name } + {' '} + has requested updates to this activity report + +
+ Please review the manager notes below and re-submit for approval. + +

Review and re-submit report

+
+

+ Creator notes +
+
+ { additionalNotes || 'No creator notes' } +

+
+
+

+ Manager notes +
+
+ { managerNotes } +

+
+
+
+ + + + {' '} + Action Requested + {' '} + from + {' '} + { approvingManager.name || 'No manager notes' } +
+
+
+ +
+ + ); +}; + +NeedsAction.propTypes = { + additionalNotes: PropTypes.string, + managerNotes: PropTypes.string, + onSubmit: PropTypes.func.isRequired, + valid: PropTypes.bool.isRequired, + approvingManager: PropTypes.shape({ + id: PropTypes.number, + name: PropTypes.string, + }).isRequired, +}; + +NeedsAction.defaultProps = { + additionalNotes: '', + managerNotes: '', +}; + +export default NeedsAction; diff --git a/frontend/src/pages/ActivityReport/Pages/Review/Submitter/__tests__/index.js b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/__tests__/index.js new file mode 100644 index 0000000000..00904e1a0c --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/__tests__/index.js @@ -0,0 +1,101 @@ +import '@testing-library/jest-dom'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; +import { useForm } from 'react-hook-form'; + +import Submitter from '../index'; +import { REPORT_STATUSES } from '../../../../../../Constants'; + +const RenderSubmitter = ({ + // eslint-disable-next-line react/prop-types + submitted, allComplete, onFormSubmit, formData, valid, +}) => { + const { register, handleSubmit } = useForm({ + mode: 'onChange', + defaultValues: formData, + }); + + return ( + + ); +}; + +const renderReview = (status, submitted, allComplete, onFormSubmit) => { + const formData = { + approvingManager: { name: 'name' }, + approvingManagerId: 1, + status, + }; + + render( + , + ); +}; + +describe('Submitter review page', () => { + describe('when the report is a draft', () => { + it('displays the draft review component', async () => { + renderReview(REPORT_STATUSES.DRAFT, false, true, () => {}); + expect(await screen.findByText('Submit Report')).toBeVisible(); + }); + + it('allows the author to submit for review', async () => { + const mockSubmit = jest.fn(); + renderReview(REPORT_STATUSES.DRAFT, false, true, mockSubmit); + const button = await screen.findByRole('button'); + userEvent.click(button); + await waitFor(() => expect(mockSubmit).toHaveBeenCalled()); + }); + + it('displays an error if the report is not complete', async () => { + renderReview(REPORT_STATUSES.DRAFT, false, false, () => {}); + const alert = await screen.findByTestId('alert'); + expect(alert.textContent).toContain('Incomplete report'); + }); + + it('displays success if the report has been submitted', async () => { + renderReview(REPORT_STATUSES.DRAFT, true, true, () => {}); + const alert = await screen.findByTestId('alert'); + expect(alert.textContent).toContain('Success'); + }); + }); + + describe('when the report is approved', () => { + it('displays the approved component', async () => { + renderReview(REPORT_STATUSES.APPROVED, false, true, () => {}); + expect(await screen.findByText('Report approved')).toBeVisible(); + }); + }); + + describe('when the report needs action', () => { + it('displays the needs action component', async () => { + renderReview(REPORT_STATUSES.NEEDS_ACTION, false, true, () => {}); + expect(await screen.findByText('Review and re-submit report')).toBeVisible(); + }); + + it('allows the user to resubmit the report', async () => { + const mockSubmit = jest.fn(); + renderReview(REPORT_STATUSES.NEEDS_ACTION, false, true, mockSubmit); + const button = await screen.findByRole('button'); + userEvent.click(button); + await waitFor(() => expect(mockSubmit).toHaveBeenCalled()); + }); + }); +}); diff --git a/frontend/src/pages/ActivityReport/Pages/Review/Submitter/index.js b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/index.js new file mode 100644 index 0000000000..a29fa07c3c --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/Submitter/index.js @@ -0,0 +1,86 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import DraftReview from './Draft'; +import NeedsAction from './NeedsAction'; +import Approved from './Approved'; + +import { REPORT_STATUSES } from '../../../../../Constants'; + +const Submitter = ({ + submitted, + allComplete, + register, + approvers, + valid, + handleSubmit, + onFormSubmit, + formData, +}) => { + const { + approvingManager, + managerNotes, + additionalNotes, + status, + } = formData; + const notReviewed = status === REPORT_STATUSES.DRAFT || status === REPORT_STATUSES.SUBMITTED; + const needsAction = status === REPORT_STATUSES.NEEDS_ACTION; + const approved = status === REPORT_STATUSES.APPROVED; + + return ( + <> + {notReviewed + && ( + + )} + {needsAction + && ( + + )} + {approved + && ( + + )} + + ); +}; + +Submitter.propTypes = { + submitted: PropTypes.bool.isRequired, + allComplete: PropTypes.bool.isRequired, + register: PropTypes.func.isRequired, + approvers: PropTypes.arrayOf(PropTypes.shape({ + id: PropTypes.number, + name: PropTypes.string, + })).isRequired, + valid: PropTypes.bool.isRequired, + handleSubmit: PropTypes.func.isRequired, + onFormSubmit: PropTypes.func.isRequired, + formData: PropTypes.shape({ + approvingManager: PropTypes.shape({ + name: PropTypes.string, + }), + managerNotes: PropTypes.string, + additionalNotes: PropTypes.string, + status: PropTypes.string, + }).isRequired, +}; + +export default Submitter; diff --git a/frontend/src/pages/ActivityReport/Pages/__tests__/ReviewSubmit.js b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/index.js similarity index 80% rename from frontend/src/pages/ActivityReport/Pages/__tests__/ReviewSubmit.js rename to frontend/src/pages/ActivityReport/Pages/Review/__tests__/index.js index 2ca6eb2dd6..6ca86152d6 100644 --- a/frontend/src/pages/ActivityReport/Pages/__tests__/ReviewSubmit.js +++ b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/index.js @@ -5,7 +5,8 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; import { FormProvider, useForm } from 'react-hook-form'; -import ReviewSubmit from '../ReviewSubmit'; +import ReviewSubmit from '../index'; +import { REPORT_STATUSES } from '../../../../../Constants'; const approvers = [ { id: 1, name: 'user 1' }, @@ -14,12 +15,13 @@ const approvers = [ const RenderReview = ({ // eslint-disable-next-line react/prop-types - allComplete, initialData, onSubmit, onReview, approvingManagerId, approvingManager, + allComplete, formData, onSubmit, onReview, approvingManagerId, approvingManager, }) => { const hookForm = useForm({ mode: 'onChange', - defaultValues: { ...initialData, approvingManagerId }, + defaultValues: { ...formData, approvingManagerId }, }); + return ( @@ -38,7 +40,8 @@ const RenderReview = ({ const renderReview = ( allComplete, approvingManager = false, - initialData = { additionalNotes: '' }, + status = REPORT_STATUSES.DRAFT, + formData = { additionalNotes: '' }, onSubmit = () => {}, onReview = () => {}, approvingManagerId = null, @@ -47,7 +50,7 @@ const renderReview = ( { describe('when the user is the approving manager', () => { it('shows the manager UI', async () => { - renderReview(true, true); + renderReview(true, true, REPORT_STATUSES.SUBMITTED); const header = await screen.findByText('Review and approve report'); expect(header).toBeVisible(); }); it('allows the manager to review the report', async () => { const onReview = jest.fn(); - renderReview(true, true, { additionalNotes: '', status: 'Approved' }, () => {}, onReview); + renderReview(true, true, REPORT_STATUSES.SUBMITTED, { additionalNotes: '' }, () => {}, onReview); + userEvent.selectOptions(screen.getByTestId('dropdown'), ['approved']); const reviewButton = await screen.findByRole('button'); userEvent.click(reviewButton); await waitFor(() => expect(onReview).toHaveBeenCalled()); @@ -79,7 +83,8 @@ describe('ReviewSubmit', () => { throw new Error(); }); - renderReview(true, true, { additionalNotes: '', status: 'Approved' }, () => {}, onReview); + renderReview(true, true, REPORT_STATUSES.SUBMITTED, { additionalNotes: '' }, () => {}, onReview); + userEvent.selectOptions(screen.getByTestId('dropdown'), ['approved']); const reviewButton = await screen.findByRole('button'); userEvent.click(reviewButton); const error = await screen.findByTestId('alert'); @@ -121,7 +126,7 @@ describe('ReviewSubmit', () => { it('the submit button calls onSubmit', async () => { const onSubmit = jest.fn(); - renderReview(true, false, {}, onSubmit, () => {}, 1); + renderReview(true, false, REPORT_STATUSES.DRAFT, {}, onSubmit, () => {}, 1); const button = await screen.findByRole('button'); expect(button).toBeEnabled(); userEvent.click(button); @@ -134,7 +139,7 @@ describe('ReviewSubmit', () => { throw new Error(); }); - renderReview(true, false, {}, onSubmit, () => {}, 1); + renderReview(true, false, REPORT_STATUSES.DRAFT, {}, onSubmit, () => {}, 1); const button = await screen.findByRole('button'); expect(button).toBeEnabled(); userEvent.click(button); @@ -144,14 +149,14 @@ describe('ReviewSubmit', () => { }); it('a success modal is shown once submitted', async () => { - renderReview(true, false, {}, () => {}, () => {}, 1); + renderReview(true, false, REPORT_STATUSES.DRAFT, {}, () => {}, () => {}, 1); userEvent.click(await screen.findByTestId('button')); const alert = await screen.findByTestId('alert'); expect(alert).toHaveClass('usa-alert--success'); }); it('initializes the form with "initialData"', async () => { - renderReview(true, false, { additionalNotes: 'test' }); + renderReview(true, false, REPORT_STATUSES.DRAFT, { additionalNotes: 'test' }); const textBox = await screen.findByLabelText('Creator notes'); await waitFor(() => expect(textBox).toHaveValue('test')); }); diff --git a/frontend/src/pages/ActivityReport/Pages/__tests__/reviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/reviewItem.js similarity index 100% rename from frontend/src/pages/ActivityReport/Pages/__tests__/reviewItem.js rename to frontend/src/pages/ActivityReport/Pages/Review/__tests__/reviewItem.js diff --git a/frontend/src/pages/ActivityReport/Pages/ReviewSubmit.css b/frontend/src/pages/ActivityReport/Pages/Review/index.css similarity index 100% rename from frontend/src/pages/ActivityReport/Pages/ReviewSubmit.css rename to frontend/src/pages/ActivityReport/Pages/Review/index.css diff --git a/frontend/src/pages/ActivityReport/Pages/ReviewSubmit.js b/frontend/src/pages/ActivityReport/Pages/Review/index.js similarity index 82% rename from frontend/src/pages/ActivityReport/Pages/ReviewSubmit.js rename to frontend/src/pages/ActivityReport/Pages/Review/index.js index d0e7fd6d7e..33e76ef245 100644 --- a/frontend/src/pages/ActivityReport/Pages/ReviewSubmit.js +++ b/frontend/src/pages/ActivityReport/Pages/Review/index.js @@ -6,10 +6,11 @@ import { import { Helmet } from 'react-helmet'; import { useFormContext } from 'react-hook-form'; -import Container from '../../../components/Container'; -import SubmitterReviewPage from './SubmitterReviewPage'; -import ApproverReviewPage from './ApproverReviewPage'; -import './ReviewSubmit.css'; +import Container from '../../../../components/Container'; +import Submitter from './Submitter'; +import Approver from './Approver'; +import './index.css'; +import { REPORT_STATUSES } from '../../../../Constants'; const ReviewSubmit = ({ allComplete, @@ -18,14 +19,14 @@ const ReviewSubmit = ({ reviewItems, approvers, approvingManager, - initialData, + formData, }) => { const { handleSubmit, register, formState } = useFormContext(); - const { additionalNotes } = initialData; + const { additionalNotes, status } = formData; const { isValid } = formState; const valid = allComplete && isValid; - const [submitted, updateSubmitted] = useState(false); + const [submitted, updateSubmitted] = useState(status === REPORT_STATUSES.SUBMITTED); const [reviewed, updateReviewed] = useState(false); const [error, updateError] = useState(); @@ -67,7 +68,8 @@ const ReviewSubmit = ({ )} {!approvingManager && ( - )} {approvingManager && ( - )} @@ -104,8 +109,9 @@ ReviewSubmit.propTypes = { onSubmit: PropTypes.func.isRequired, onReview: PropTypes.func.isRequired, approvingManager: PropTypes.bool.isRequired, - initialData: PropTypes.shape({ + formData: PropTypes.shape({ additionalNotes: PropTypes.string, + status: PropTypes.string, }).isRequired, // eslint-disable-next-line react/forbid-prop-types reviewItems: PropTypes.arrayOf( diff --git a/frontend/src/pages/ActivityReport/Pages/reviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/reviewItem.js similarity index 100% rename from frontend/src/pages/ActivityReport/Pages/reviewItem.js rename to frontend/src/pages/ActivityReport/Pages/Review/reviewItem.js diff --git a/frontend/src/pages/ActivityReport/Pages/index.js b/frontend/src/pages/ActivityReport/Pages/index.js index 6f75e0d6ce..c8f9071c37 100644 --- a/frontend/src/pages/ActivityReport/Pages/index.js +++ b/frontend/src/pages/ActivityReport/Pages/index.js @@ -3,8 +3,8 @@ import activitySummary from './activitySummary'; import topicsResources from './topicsResources'; import nextSteps from './nextSteps'; import goalsObjectives from './goalsObjectives'; -import ReviewSubmit from './ReviewSubmit'; -import reviewItem from './reviewItem'; +import ReviewSubmit from './Review'; +import reviewItem from './Review/reviewItem'; /* Note these are not react nodes but objects used by the navigator to render out @@ -44,7 +44,7 @@ const reviewPage = { reviewItems={ pages.map((p) => reviewItem(p.path, p.label, p.sections, formData)) } - initialData={formData} + formData={formData} /> ), }; diff --git a/frontend/src/pages/ActivityReport/index.js b/frontend/src/pages/ActivityReport/index.js index ef1b785c0b..d784d4c33d 100644 --- a/frontend/src/pages/ActivityReport/index.js +++ b/frontend/src/pages/ActivityReport/index.js @@ -16,6 +16,7 @@ import Navigator from '../../components/Navigator'; import './index.css'; import { NOT_STARTED } from '../../components/Navigator/constants'; +import { REPORT_STATUSES } from '../../Constants'; import { submitReport, saveReport, @@ -59,6 +60,7 @@ const defaultValues = { approvingManagerId: null, additionalNotes: null, goals: fakeGoals, + status: REPORT_STATUSES.DRAFT, }; // FIXME: default region until we have a way of changing on the frontend @@ -69,10 +71,9 @@ const defaultPageState = _.mapValues(pagesByPos, () => NOT_STARTED); function ActivityReport({ match, user, location }) { const { params: { currentPage, activityReportId } } = match; const history = useHistory(); - const [status, updateStatus] = useState(); const [error, updateError] = useState(); const [loading, updateLoading] = useState(true); - const [initialFormData, updateInitialFormData] = useState(defaultValues); + const [formData, updateFormData] = useState(); const [initialAdditionalData, updateAdditionalData] = useState({}); const [approvingManager, updateApprovingManager] = useState(false); const [canWrite, updateCanWrite] = useState(false); @@ -116,8 +117,7 @@ function ActivityReport({ match, user, location }) { const canWriteReport = isCollaborator || isAuthor; updateAdditionalData({ recipients, collaborators, approvers }); - updateInitialFormData(report); - updateStatus(report.status || 'draft'); + updateFormData(report); updateApprovingManager(report.approvingManagerId === user.id); updateCanWrite(canWriteReport); @@ -152,8 +152,9 @@ function ActivityReport({ match, user, location }) { } if (!currentPage) { + const defaultPage = formData.status === REPORT_STATUSES.DRAFT ? 'activity-summary' : 'review'; return ( - + ); } @@ -190,12 +191,12 @@ function ActivityReport({ match, user, location }) { const onFormSubmit = async (data) => { const report = await submitReport(reportId.current, data); - updateStatus(report.status); + updateFormData(report); }; const onReview = async (data) => { const report = await reviewReport(reportId.current, data); - updateStatus(report.status); + updateFormData(report); }; return ( @@ -207,11 +208,11 @@ function ActivityReport({ match, user, location }) { reportId={reportId.current} currentPage={currentPage} additionalData={initialAdditionalData} - initialData={{ ...defaultValues, ...initialFormData }} + formData={formData} + updateFormData={updateFormData} pages={pages} onFormSubmit={onFormSubmit} onSave={onSave} - status={status} approvingManager={approvingManager} onReview={onReview} /> diff --git a/src/constants.js b/src/constants.js index 5470d7886c..0773e94f56 100644 --- a/src/constants.js +++ b/src/constants.js @@ -1,2 +1,7 @@ -// eslint-disable-next-line import/prefer-default-export +export const REPORT_STATUSES = { + DRAFT: 'draft', + SUBMITTED: 'submitted', + APPROVED: 'approved', + NEEDS_ACTION: 'needs_action', +}; export const DECIMAL_BASE = 10; diff --git a/src/migrations/20210205172332-add-activity-report-status-enum.js b/src/migrations/20210205172332-add-activity-report-status-enum.js new file mode 100644 index 0000000000..14fc48a626 --- /dev/null +++ b/src/migrations/20210205172332-add-activity-report-status-enum.js @@ -0,0 +1,32 @@ +const statuses = [ + 'draft', + 'submitted', + 'needs_action', + 'approved', +]; + +module.exports = { + up: async (queryInterface, Sequelize) => { + queryInterface.changeColumn( + 'ActivityReports', + 'status', + { + type: Sequelize.DataTypes.ENUM(...statuses), + }, + ); + }, + + down: async (queryInterface, Sequelize) => { + queryInterface.sequelize.transaction((t) => Promise.all([ + queryInterface.changeColumn( + 'ActivityReports', + 'status', + { + type: Sequelize.STRING, + }, + { transaction: t }, + ), + queryInterface.sequelize.query('DROP TYPE public."enum_ActivityReports_status";', { transaction: t }), + ])); + }, +}; diff --git a/src/models/activityReport.js b/src/models/activityReport.js index e61fcaa1ac..1e6f9d6260 100644 --- a/src/models/activityReport.js +++ b/src/models/activityReport.js @@ -1,5 +1,6 @@ import { Model } from 'sequelize'; import moment from 'moment'; +import { REPORT_STATUSES } from '../constants'; function formatDate(fieldName) { const raw = this.getDataValue(fieldName); @@ -108,7 +109,7 @@ export default (sequelize, DataTypes) => { }, status: { allowNull: false, - type: DataTypes.STRING, + type: DataTypes.ENUM(Object.keys(REPORT_STATUSES).map((k) => REPORT_STATUSES[k])), validate: { checkRequiredForSubmission() { const requiredForSubmission = [ @@ -128,7 +129,7 @@ export default (sequelize, DataTypes) => { this.topics, this.ttaType, ]; - if (this.status !== 'draft') { + if (this.status !== REPORT_STATUSES.DRAFT) { if (requiredForSubmission.includes(null)) { throw new Error('Missing required field(s)'); } diff --git a/src/policies/activityReport.js b/src/policies/activityReport.js index 2c1800bf2c..6e341962f7 100644 --- a/src/policies/activityReport.js +++ b/src/policies/activityReport.js @@ -8,6 +8,7 @@ */ import _ from 'lodash'; import SCOPES from '../middleware/scopeConstants'; +import { REPORT_STATUSES } from '../constants'; export default class ActivityReport { constructor(user, activityReport) { @@ -35,7 +36,7 @@ export default class ActivityReport { return canReadUnapproved; } - if (status === 'approved') { + if (status === REPORT_STATUSES.APPROVED) { return this.canReadInRegion(); } diff --git a/src/policies/activityReport.test.js b/src/policies/activityReport.test.js index d434842ac5..73346dcb6e 100644 --- a/src/policies/activityReport.test.js +++ b/src/policies/activityReport.test.js @@ -1,7 +1,13 @@ import ActivityReport from './activityReport'; import SCOPES from '../middleware/scopeConstants'; - -function activityReport(author, collaborator, status = 'draft', approvingManager = null) { +import { REPORT_STATUSES } from '../constants'; + +function activityReport( + author, + collaborator, + status = REPORT_STATUSES.DRAFT, + approvingManager = null, +) { const report = { userId: author, regionId: 1, @@ -44,7 +50,7 @@ const otherUser = user(false, true, 4); describe('Activity Report policies', () => { describe('canReview', () => { it('is true if the user is the approving manager', () => { - const report = activityReport(author.id, null, 'submitted', manager.id); + const report = activityReport(author.id, null, REPORT_STATUSES.SUBMITTED, manager.id); const policy = new ActivityReport(manager, report); expect(policy.canReview()).toBeTruthy(); }); @@ -113,7 +119,7 @@ describe('Activity Report policies', () => { }); it('is true for the approving manager', () => { - const report = activityReport(author.id, null, 'draft', manager.id); + const report = activityReport(author.id, null, REPORT_STATUSES.DRAFT, manager.id); const policy = new ActivityReport(manager, report); expect(policy.canGet()).toBeTruthy(); }); @@ -127,7 +133,7 @@ describe('Activity Report policies', () => { describe('for approved reports', () => { it('is true for users with read permissions in the region', () => { - const report = activityReport(author.id, null, 'approved'); + const report = activityReport(author.id, null, REPORT_STATUSES.APPROVED); const policy = new ActivityReport(otherUser, report); expect(policy.canGet()).toBeTruthy(); }); diff --git a/src/routes/activityReports/handlers.js b/src/routes/activityReports/handlers.js index 48c55e0156..5afc38e65e 100644 --- a/src/routes/activityReports/handlers.js +++ b/src/routes/activityReports/handlers.js @@ -7,7 +7,7 @@ import { } from '../../services/activityReports'; import { goalsForGrants } from '../../services/goals'; import { userById, usersWithPermissions } from '../../services/users'; -import { DECIMAL_BASE } from '../../constants'; +import { REPORT_STATUSES, DECIMAL_BASE } from '../../constants'; const { APPROVE_REPORTS } = SCOPES; @@ -96,7 +96,7 @@ export async function submitReport(req, res) { const { activityReportId } = req.params; const { approvingManagerId, additionalNotes } = req.body; const newReport = { approvingManagerId, additionalNotes }; - newReport.status = 'Submitted'; + newReport.status = REPORT_STATUSES.SUBMITTED; const user = await userById(req.session.userId); const report = await activityReportById(activityReportId); @@ -194,7 +194,7 @@ export async function createReport(req, res) { return; } const userId = parseInt(req.session.userId, 10); - newReport.status = 'draft'; + newReport.status = REPORT_STATUSES.DRAFT; newReport.userId = userId; newReport.lastUpdatedById = userId; const user = await userById(req.session.userId); diff --git a/src/routes/files/handlers.test.js b/src/routes/files/handlers.test.js index cec3413aa5..0355e87214 100644 --- a/src/routes/files/handlers.test.js +++ b/src/routes/files/handlers.test.js @@ -8,6 +8,7 @@ import db, { import app from '../../app'; import s3Uploader from '../../lib/s3Uploader'; import SCOPES from '../../middleware/scopeConstants'; +import { REPORT_STATUSES } from '../../constants'; import ActivityReportPolicy from '../../policies/activityReport'; jest.mock('../../policies/activityReport'); @@ -45,7 +46,7 @@ mockSession.userId = mockUser.id; const reportObject = { activityRecipientType: 'grantee', - status: 'draft', + status: REPORT_STATUSES.DRAFT, userId: mockUser.id, lastUpdatedById: mockUser.id, resourcesUsed: 'test', diff --git a/src/services/activityReports.js b/src/services/activityReports.js index c96346e403..3235dd0c7a 100644 --- a/src/services/activityReports.js +++ b/src/services/activityReports.js @@ -184,6 +184,11 @@ export function activityReportById(activityReportId) { as: 'otherResources', required: false, }, + { + model: User, + as: 'approvingManager', + required: false, + }, ], }); } @@ -196,6 +201,7 @@ export async function createOrUpdate(newActivityReport, report) { activityRecipients, attachments, otherResources, + approvingManager, ...updatedFields } = newActivityReport; await sequelize.transaction(async (transaction) => { diff --git a/src/services/activityReports.test.js b/src/services/activityReports.test.js index 0ea64e64d9..547dd3b2fb 100644 --- a/src/services/activityReports.test.js +++ b/src/services/activityReports.test.js @@ -4,6 +4,7 @@ import db, { import { createOrUpdate, activityReportById, } from './activityReports'; +import { REPORT_STATUSES } from '../constants'; const mockUser = { id: 1000, @@ -13,7 +14,7 @@ const mockUser = { const reportObject = { activityRecipientType: 'grantee', - status: 'draft', + status: REPORT_STATUSES.DRAFT, userId: mockUser.id, regionId: 1, lastUpdatedById: mockUser.id,