From 9e553cdd30c0153b88d77c938de8e8976e8ae813 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Thu, 18 Feb 2021 15:12:08 -0600 Subject: [PATCH 01/12] Refactor activity report review pages * Each page has more control over what shows up on it's review page * Topics and resources shows download links to the files that have been uploaded to the report --- frontend/src/components/Navigator/index.js | 1 + .../Pages/Review/FileReviewItem.js | 32 ++++ .../ActivityReport/Pages/Review/ReviewItem.js | 49 ++++++ .../ActivityReport/Pages/Review/ReviewPage.js | 53 ++++++ .../Pages/Review/ReviewSection.js | 44 +++++ .../Pages/Review/__tests__/FileReviewItem.js | 35 ++++ .../{reviewItem.js => ReviewPage.js} | 27 +++- .../ActivityReport/Pages/Review/reviewItem.js | 151 ------------------ .../ActivityReport/Pages/activitySummary.js | 7 +- .../ActivityReport/Pages/goalsObjectives.js | 7 +- .../src/pages/ActivityReport/Pages/index.js | 7 +- .../pages/ActivityReport/Pages/nextSteps.js | 7 +- .../ActivityReport/Pages/topicsResources.js | 92 +++++++---- 13 files changed, 324 insertions(+), 188 deletions(-) create mode 100644 frontend/src/pages/ActivityReport/Pages/Review/FileReviewItem.js create mode 100644 frontend/src/pages/ActivityReport/Pages/Review/ReviewItem.js create mode 100644 frontend/src/pages/ActivityReport/Pages/Review/ReviewPage.js create mode 100644 frontend/src/pages/ActivityReport/Pages/Review/ReviewSection.js create mode 100644 frontend/src/pages/ActivityReport/Pages/Review/__tests__/FileReviewItem.js rename frontend/src/pages/ActivityReport/Pages/Review/__tests__/{reviewItem.js => ReviewPage.js} (78%) delete mode 100644 frontend/src/pages/ActivityReport/Pages/Review/reviewItem.js diff --git a/frontend/src/components/Navigator/index.js b/frontend/src/components/Navigator/index.js index b3ead09932..1f6503975a 100644 --- a/frontend/src/components/Navigator/index.js +++ b/frontend/src/components/Navigator/index.js @@ -46,6 +46,7 @@ function Navigator({ const hookForm = useForm({ mode: 'onChange', defaultValues: formData, + shouldUnregister: false, }); const { diff --git a/frontend/src/pages/ActivityReport/Pages/Review/FileReviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/FileReviewItem.js new file mode 100644 index 0000000000..e0123bc186 --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/FileReviewItem.js @@ -0,0 +1,32 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Grid } from '@trussworks/react-uswds'; + +const APPROVED = 'APPROVED'; + +const FileReviewItem = ({ filename, url, status }) => { + const approved = status === APPROVED; + return ( + + + {filename} + + +
+ {approved + && Download} + {!approved + &&
Not Approved
} +
+ + + ); +}; + +FileReviewItem.propTypes = { + filename: PropTypes.string.isRequired, + status: PropTypes.string.isRequired, + url: PropTypes.string.isRequired, +}; + +export default FileReviewItem; diff --git a/frontend/src/pages/ActivityReport/Pages/Review/ReviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/ReviewItem.js new file mode 100644 index 0000000000..747e5b879d --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/ReviewItem.js @@ -0,0 +1,49 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Grid } from '@trussworks/react-uswds'; +import _ from 'lodash'; +import { useFormContext } from 'react-hook-form'; + +const ReviewItem = ({ label, name, path }) => { + const { getValues } = useFormContext(); + const value = getValues(name); + let values = value; + + if (!Array.isArray(value)) { + values = [value]; + } + + if (path) { + values = values.map((v) => _.get(v, path)); + } + + const emptySelector = value && value.length > 0 ? '' : 'smart-hub-review-item--empty'; + const classes = ['margin-top-1', emptySelector].filter((x) => x !== '').join(' '); + + return ( + + + {label} + + + {values.map((v, index) => ( + + {v} + + ))} + + + ); +}; + +ReviewItem.propTypes = { + label: PropTypes.string.isRequired, + name: PropTypes.string.isRequired, + path: PropTypes.string, +}; + +ReviewItem.defaultProps = { + path: '', +}; + +export default ReviewItem; diff --git a/frontend/src/pages/ActivityReport/Pages/Review/ReviewPage.js b/frontend/src/pages/ActivityReport/Pages/Review/ReviewPage.js new file mode 100644 index 0000000000..0be343f134 --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/ReviewPage.js @@ -0,0 +1,53 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { some } from 'lodash'; +import { useFormContext } from 'react-hook-form'; + +import Section from './ReviewSection'; +import ReviewItem from './ReviewItem'; + +const ReviewPage = ({ sections, path }) => { + const { getValues } = useFormContext(); + return ( + <> + {sections.map((section) => { + const names = section.items.map((item) => item.name); + const values = getValues(names); + const isEmpty = !some(values, (value) => value && value.length); + return ( +
+ {section.items.map((item) => ( + + ))} +
+ ); + })} + + ); +}; + +ReviewPage.propTypes = { + path: PropTypes.string.isRequired, + sections: PropTypes.arrayOf(PropTypes.shape({ + title: PropTypes.string, + anchor: PropTypes.string, + items: PropTypes.arrayOf(PropTypes.shape({ + label: PropTypes.string, + path: PropTypes.string, + name: PropTypes.string, + })), + })).isRequired, +}; + +export default ReviewPage; diff --git a/frontend/src/pages/ActivityReport/Pages/Review/ReviewSection.js b/frontend/src/pages/ActivityReport/Pages/Review/ReviewSection.js new file mode 100644 index 0000000000..4511000032 --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/ReviewSection.js @@ -0,0 +1,44 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Grid } from '@trussworks/react-uswds'; +import { HashLink } from 'react-router-hash-link'; + +const Section = ({ + title, children, basePath, anchor, hidePrint, +}) => { + const classes = [ + 'smart-hub-review-section', + hidePrint ? 'smart-hub-review-section--empty no-print' : '', + 'margin-bottom-3', + ].filter((x) => x).join(' '); + + return ( +
+ + + {title} + + + + Edit + + + + {children} +
+ ); +}; + +Section.propTypes = { + hidePrint: PropTypes.bool.isRequired, + title: PropTypes.string.isRequired, + anchor: PropTypes.string.isRequired, + children: PropTypes.node.isRequired, + basePath: PropTypes.string.isRequired, +}; + +export default Section; diff --git a/frontend/src/pages/ActivityReport/Pages/Review/__tests__/FileReviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/FileReviewItem.js new file mode 100644 index 0000000000..69b6ca2736 --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/FileReviewItem.js @@ -0,0 +1,35 @@ +/* eslint-disable react/jsx-props-no-spreading */ +import React from 'react'; +import '@testing-library/jest-dom'; +import { render, screen } from '@testing-library/react'; + +import FileReviewItem from '../FileReviewItem'; + +// eslint-disable-next-line react/prop-types +const RenderFileReviewItem = ({ status = 'APPROVED' }) => ( + +); + +describe('ReviewPage', () => { + it('displays the filename', async () => { + render(); + const item = await screen.findByText('filename'); + expect(item).toBeVisible(); + }); + + it('displays the download link', async () => { + render(); + const link = await screen.findByRole('link'); + expect(link).toHaveAttribute('href', 'http://localhost:3000'); + }); + + it('displays "not approved" if the file has not been approved', async () => { + render(); + const status = await screen.findByText('Not Approved'); + expect(status).toBeVisible(); + }); +}); diff --git a/frontend/src/pages/ActivityReport/Pages/Review/__tests__/reviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/ReviewPage.js similarity index 78% rename from frontend/src/pages/ActivityReport/Pages/Review/__tests__/reviewItem.js rename to frontend/src/pages/ActivityReport/Pages/Review/__tests__/ReviewPage.js index f7279c8c9f..7d71a33106 100644 --- a/frontend/src/pages/ActivityReport/Pages/Review/__tests__/reviewItem.js +++ b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/ReviewPage.js @@ -1,8 +1,10 @@ +/* eslint-disable react/jsx-props-no-spreading */ import React from 'react'; import '@testing-library/jest-dom'; import { render, screen } from '@testing-library/react'; import { BrowserRouter } from 'react-router-dom'; -import accordionItem from '../reviewItem'; +import { FormProvider, useForm } from 'react-hook-form'; +import ReviewPage from '../ReviewPage'; const sections = [ { @@ -37,13 +39,28 @@ const values = { object: { test: 'test' }, }; +const RenderReviewPage = () => { + const hookForm = useForm({ + mode: 'onChange', + defaultValues: values, + shouldUnregister: false, + }); + return ( + + + + + + ); +}; + describe('AccordionItem', () => { beforeEach(() => { - const item = accordionItem('id', 'title', sections, values); render( - - {item.content} - , + , ); }); diff --git a/frontend/src/pages/ActivityReport/Pages/Review/reviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/reviewItem.js deleted file mode 100644 index 73689c475c..0000000000 --- a/frontend/src/pages/ActivityReport/Pages/Review/reviewItem.js +++ /dev/null @@ -1,151 +0,0 @@ -/* - Review items for activity report review section - - These items are for use with the USWDS accordion component. This module is split in the - following way: - - 1. Review Item: this represents a page in the activity report, and contains multiple - review sections. - 2. Sections: this represents a "fieldset" in the report, or the major sections within a page - of the activity report, they contain items. - 3. Items: these are the individual form fields for the activity report -*/ -import { Grid } from '@trussworks/react-uswds'; -import PropTypes from 'prop-types'; -import { HashLink } from 'react-router-hash-link'; -import _ from 'lodash'; -import React from 'react'; - -const itemType = { - label: PropTypes.string.isRequired, - path: PropTypes.string, - value: PropTypes.oneOfType([ - PropTypes.string, - PropTypes.number, - PropTypes.shape({}), - PropTypes.arrayOf(PropTypes.string), - PropTypes.arrayOf(PropTypes.shape({})), - ]), -}; - -const sectionType = { - title: PropTypes.string.isRequired, - anchor: PropTypes.string.isRequired, - items: PropTypes.arrayOf(PropTypes.shape(itemType)).isRequired, - basePath: PropTypes.string.isRequired, -}; - -const Section = ({ - title, items, basePath, anchor, -}) => { - const isEmpty = !items.some(({ value }) => value && value.length); - const classes = [ - 'smart-hub-review-section', - isEmpty ? 'smart-hub-review-section--empty no-print' : '', - 'margin-bottom-3', - ].filter((x) => x).join(' '); - - return ( -
- - - {title} - - - - Edit - - - - {items.map(({ label, value, path }) => ( - - ))} -
- ); -}; - -Section.propTypes = sectionType; - -const Item = ({ label, value, path }) => { - let values = value; - - if (!Array.isArray(value)) { - values = [value]; - } - - if (path) { - values = values.map((v) => _.get(v, path)); - } - - const emptySelector = value && value.length > 0 ? '' : 'smart-hub-review-item--empty'; - const classes = ['margin-top-1', emptySelector].filter((x) => x !== '').join(' '); - - return ( - - - {label} - - - {values.map((v, index) => ( - - {v} - - ))} - - - ); -}; - -Item.propTypes = itemType; - -Item.defaultProps = { - path: '', - value: null, -}; - -const ReviewItem = ({ formData, sections, basePath }) => ( - <> - {sections.map((section) => ( -
{ - const { path, label } = item; - const value = _.get(formData, item.name, ''); - return { - label, - value, - path, - }; - })} - /> - ))} - -); - -ReviewItem.propTypes = { - formData: PropTypes.shape({}).isRequired, - basePath: PropTypes.string.isRequired, - sections: PropTypes.arrayOf( - PropTypes.shape({ - title: PropTypes.string.isRequired, - anchor: PropTypes.string.isRequired, - items: PropTypes.arrayOf(PropTypes.shape(itemType)).isRequired, - }), - ).isRequired, -}; - -/* - This is the format the USWDS accordion expects of accordion items -*/ -export default (id, title, sections, formData) => ({ - id, - title, - content: , -}); diff --git a/frontend/src/pages/ActivityReport/Pages/activitySummary.js b/frontend/src/pages/ActivityReport/Pages/activitySummary.js index 8189f24828..6374112f98 100644 --- a/frontend/src/pages/ActivityReport/Pages/activitySummary.js +++ b/frontend/src/pages/ActivityReport/Pages/activitySummary.js @@ -7,6 +7,7 @@ import { Fieldset, Radio, Label, Grid, TextInput, Checkbox, } from '@trussworks/react-uswds'; +import ReviewPage from './Review/ReviewPage'; import DatePicker from '../../../components/DatePicker'; import MultiSelect from '../../../components/MultiSelect'; import { @@ -352,11 +353,15 @@ const sections = [ }, ]; +const ReviewSection = () => ( + +); + export default { position: 1, label: 'Activity summary', path: 'activity-summary', - sections, + reviewSection: () => , review: false, render: (additionalData) => { const { recipients, collaborators } = additionalData; diff --git a/frontend/src/pages/ActivityReport/Pages/goalsObjectives.js b/frontend/src/pages/ActivityReport/Pages/goalsObjectives.js index a1fec8776a..4e32dd64f4 100644 --- a/frontend/src/pages/ActivityReport/Pages/goalsObjectives.js +++ b/frontend/src/pages/ActivityReport/Pages/goalsObjectives.js @@ -6,6 +6,7 @@ import { } from '@trussworks/react-uswds'; import useDeepCompareEffect from 'use-deep-compare-effect'; import { useFormContext } from 'react-hook-form'; +import ReviewPage from './Review/ReviewPage'; import GoalPicker from './components/GoalPicker'; import { getGoals } from '../../../fetchers/activityReports'; @@ -90,12 +91,16 @@ const sections = [ }, ]; +const ReviewSection = () => ( + +); + export default { position: 3, label: 'Goals and objectives', path: 'goals-objectives', review: false, - sections, + reviewSection: () => , render: (additionalData, formData) => { const recipients = formData.activityRecipients || []; const { activityRecipientType } = formData; diff --git a/frontend/src/pages/ActivityReport/Pages/index.js b/frontend/src/pages/ActivityReport/Pages/index.js index b6ecec9ecf..33f710741b 100644 --- a/frontend/src/pages/ActivityReport/Pages/index.js +++ b/frontend/src/pages/ActivityReport/Pages/index.js @@ -4,7 +4,6 @@ import topicsResources from './topicsResources'; import nextSteps from './nextSteps'; import goalsObjectives from './goalsObjectives'; import ReviewSubmit from './Review'; -import reviewItem from './Review/reviewItem'; /* Note these are not react nodes but objects used by the navigator to render out @@ -43,7 +42,11 @@ const reviewPage = { onReview={onReview} approvingManager={approvingManager} reviewItems={ - pages.map((p) => reviewItem(p.path, p.label, p.sections, formData)) + pages.map((p) => ({ + id: p.path, + title: p.label, + content: p.reviewSection(), + })) } formData={formData} reportCreator={reportCreator} diff --git a/frontend/src/pages/ActivityReport/Pages/nextSteps.js b/frontend/src/pages/ActivityReport/Pages/nextSteps.js index 4f05a4d7a5..8b3a74aaf5 100644 --- a/frontend/src/pages/ActivityReport/Pages/nextSteps.js +++ b/frontend/src/pages/ActivityReport/Pages/nextSteps.js @@ -1,5 +1,6 @@ import React from 'react'; import { Helmet } from 'react-helmet'; +import ReviewPage from './Review/ReviewPage'; const NextSteps = () => ( <> @@ -16,12 +17,16 @@ NextSteps.propTypes = {}; const sections = []; +const ReviewSection = () => ( + +); + export default { position: 4, label: 'Next steps', path: 'next-steps', review: false, - sections, + reviewSection: () => , render: () => ( ), diff --git a/frontend/src/pages/ActivityReport/Pages/topicsResources.js b/frontend/src/pages/ActivityReport/Pages/topicsResources.js index 9b657d2d75..ed6be351e5 100644 --- a/frontend/src/pages/ActivityReport/Pages/topicsResources.js +++ b/frontend/src/pages/ActivityReport/Pages/topicsResources.js @@ -1,13 +1,15 @@ import React from 'react'; import PropTypes from 'prop-types'; - import { Controller, useFormContext } from 'react-hook-form'; import { Helmet } from 'react-helmet'; - import { Fieldset, Label, TextInput, } from '@trussworks/react-uswds'; +import { isUndefined } from 'lodash'; +import Section from './Review/ReviewSection'; +import ReviewItem from './Review/ReviewItem'; +import FileReviewItem from './Review/FileReviewItem'; import MultiSelect from '../../../components/MultiSelect'; import FileUploader from '../../../components/FileUploader'; import { topics } from '../constants'; @@ -82,36 +84,72 @@ TopicsResources.propTypes = { reportId: PropTypes.node.isRequired, }; -const sections = [ - { - title: 'Topics covered', - anchor: 'topics-resources', - items: [ - { label: 'Topics', name: 'topics' }, - ], - }, - { - title: 'Resources', - anchor: 'resources', - items: [ - { label: 'Resources used', name: 'resourcesUsed' }, - { label: 'Other resources', name: 'otherResources', path: 'originalFileName' }, - ], - }, - { - title: 'Attachments', - anchor: 'attachments', - items: [ - { label: 'Attachments', name: 'attachments', path: 'originalFileName' }, - ], - }, -]; +const ReviewSection = () => { + const { getValues } = useFormContext(); + const { + otherResources, + resourcesUsed, + attachments, + topics: formTopics, + } = getValues(); + + return ( + <> +
+ +
+
+ + {otherResources.map((file) => ( + + ))} +
+
+ {attachments.map((file) => ( + + ))} +
+ + ); +}; export default { position: 2, label: 'Topics and resources', path: 'topics-resources', - sections, + reviewSection: () => , review: false, render: (additionalData, formData, reportId) => ( Date: Thu, 18 Feb 2021 15:31:27 -0600 Subject: [PATCH 02/12] Fix review page for new reports --- .../pages/ActivityReport/Pages/Review/FileReviewItem.js | 2 +- .../ActivityReport/Pages/Review/__tests__/ReviewPage.js | 2 +- frontend/src/pages/ActivityReport/Pages/topicsResources.js | 7 +++++-- frontend/src/pages/ActivityReport/index.js | 1 + 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/frontend/src/pages/ActivityReport/Pages/Review/FileReviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/FileReviewItem.js index e0123bc186..1b1a591639 100644 --- a/frontend/src/pages/ActivityReport/Pages/Review/FileReviewItem.js +++ b/frontend/src/pages/ActivityReport/Pages/Review/FileReviewItem.js @@ -14,7 +14,7 @@ const FileReviewItem = ({ filename, url, status }) => {
{approved - && Download} + && Download} {!approved &&
Not Approved
}
diff --git a/frontend/src/pages/ActivityReport/Pages/Review/__tests__/ReviewPage.js b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/ReviewPage.js index 7d71a33106..cbc0182181 100644 --- a/frontend/src/pages/ActivityReport/Pages/Review/__tests__/ReviewPage.js +++ b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/ReviewPage.js @@ -57,7 +57,7 @@ const RenderReviewPage = () => { ); }; -describe('AccordionItem', () => { +describe('ReviewPage', () => { beforeEach(() => { render( , diff --git a/frontend/src/pages/ActivityReport/Pages/topicsResources.js b/frontend/src/pages/ActivityReport/Pages/topicsResources.js index ed6be351e5..53d674c6db 100644 --- a/frontend/src/pages/ActivityReport/Pages/topicsResources.js +++ b/frontend/src/pages/ActivityReport/Pages/topicsResources.js @@ -93,6 +93,9 @@ const ReviewSection = () => { topics: formTopics, } = getValues(); + const hasOtherResources = otherResources && otherResources.length <= 0; + const hasAttachments = attachments && attachments.length <= 0; + return ( <>
{ />
{ ))}
Date: Thu, 18 Feb 2021 15:57:56 -0600 Subject: [PATCH 03/12] Increase coverage --- .../Navigator/components/__tests__/SideNav.js | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/Navigator/components/__tests__/SideNav.js b/frontend/src/components/Navigator/components/__tests__/SideNav.js index ae84297d00..051beb07c1 100644 --- a/frontend/src/components/Navigator/components/__tests__/SideNav.js +++ b/frontend/src/components/Navigator/components/__tests__/SideNav.js @@ -13,7 +13,7 @@ import { import { REPORT_STATUSES } from '../../../../Constants'; describe('SideNav', () => { - const renderNav = (state, onNavigation = () => {}, current = false) => { + const renderNav = (state, onNavigation = () => {}, current = false, errorMessage = null) => { const pages = [ { label: 'test', @@ -34,6 +34,7 @@ describe('SideNav', () => { pages={pages} skipTo="skip" skipToMessage="message" + errorMessage={errorMessage} />, ); }; @@ -60,6 +61,20 @@ describe('SideNav', () => { expect(complete).toBeVisible(); }); + it('approved', () => { + renderNav(REPORT_STATUSES.APPROVED); + const complete = screen.getByText('Approved'); + expect(complete).toHaveClass('smart-hub--tag-submitted'); + expect(complete).toBeVisible(); + }); + + it('needs action', () => { + renderNav(REPORT_STATUSES.NEEDS_ACTION); + const complete = screen.getByText('Needs Action'); + expect(complete).toHaveClass('smart-hub--tag-needs-action'); + expect(complete).toBeVisible(); + }); + it('submitted', () => { renderNav(REPORT_STATUSES.SUBMITTED); const submitted = screen.getByText('Submitted'); @@ -68,6 +83,12 @@ describe('SideNav', () => { }); }); + it('displays error message', async () => { + renderNav(REPORT_STATUSES.SUBMITTED, () => {}, false, 'error'); + const alert = await screen.findByTestId('alert'); + expect(alert).toBeVisible(); + }); + it('clicking a nav item calls onNavigation', () => { const onNav = jest.fn(); renderNav(NOT_STARTED, onNav); From ae12340f9c718be7d500df9e25fed26b8ed6be76 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Mon, 1 Mar 2021 12:01:40 -0600 Subject: [PATCH 04/12] Add some more frontend tests --- .../src/fetchers/__tests__/activityReports.js | 35 +++++++++++++++ .../ActivityReport/Pages/Review/ReviewItem.js | 4 +- .../Pages/__tests__/topicsResources.js | 45 +++++++++++++++++++ .../ActivityReport/Pages/topicsResources.js | 10 +++-- 4 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 frontend/src/fetchers/__tests__/activityReports.js create mode 100644 frontend/src/pages/ActivityReport/Pages/__tests__/topicsResources.js diff --git a/frontend/src/fetchers/__tests__/activityReports.js b/frontend/src/fetchers/__tests__/activityReports.js new file mode 100644 index 0000000000..409473d26a --- /dev/null +++ b/frontend/src/fetchers/__tests__/activityReports.js @@ -0,0 +1,35 @@ +import join from 'url-join'; +import fetchMock from 'fetch-mock'; + +import { submitReport, saveReport, reviewReport } from '../activityReports'; + +describe('activityReports fetcher', () => { + afterEach(() => fetchMock.restore()); + + describe('submitReport', () => { + it('returns the report', async () => { + const report = { id: 1 }; + fetchMock.post(join('api', 'activity-reports', '1', 'submit'), report); + const savedReport = await submitReport(1, report); + expect(savedReport).toEqual(report); + }); + }); + + describe('saveReport', () => { + it('returns the report', async () => { + const report = { id: 1 }; + fetchMock.put(join('api', 'activity-reports', '1'), report); + const savedReport = await saveReport(1, report); + expect(savedReport).toEqual(report); + }); + }); + + describe('reviewReport', () => { + it('returns the report', async () => { + const report = { id: 1 }; + fetchMock.put(join('api', 'activity-reports', '1', 'review'), report); + const savedReport = await reviewReport(1, report); + expect(savedReport).toEqual(report); + }); + }); +}); diff --git a/frontend/src/pages/ActivityReport/Pages/Review/ReviewItem.js b/frontend/src/pages/ActivityReport/Pages/Review/ReviewItem.js index 4193bfe0a1..dc287d97f2 100644 --- a/frontend/src/pages/ActivityReport/Pages/Review/ReviewItem.js +++ b/frontend/src/pages/ActivityReport/Pages/Review/ReviewItem.js @@ -4,8 +4,8 @@ import _ from 'lodash'; import { useFormContext } from 'react-hook-form'; const ReviewItem = ({ label, name, path }) => { - const { getValues } = useFormContext(); - const value = getValues(name); + const { watch } = useFormContext(); + const value = watch(name); let values = value; if (!Array.isArray(value)) { diff --git a/frontend/src/pages/ActivityReport/Pages/__tests__/topicsResources.js b/frontend/src/pages/ActivityReport/Pages/__tests__/topicsResources.js new file mode 100644 index 0000000000..8a965dc01b --- /dev/null +++ b/frontend/src/pages/ActivityReport/Pages/__tests__/topicsResources.js @@ -0,0 +1,45 @@ +/* eslint-disable react/jsx-props-no-spreading */ +import '@testing-library/jest-dom'; +import { render, screen } from '@testing-library/react'; +import React from 'react'; +import { FormProvider, useForm } from 'react-hook-form'; +import { Router } from 'react-router-dom'; +import { createMemoryHistory } from 'history'; + +import topics from '../topicsResources'; + +// eslint-disable-next-line react/prop-types +const RenderTopicsResourcesReview = ({ data }) => { + const history = createMemoryHistory(); + const hookForm = useForm({ + mode: 'onChange', + defaultValues: data, + }); + // eslint-disable-next-line react/prop-types + return ( + + + {topics.reviewSection()} + + + ); +}; + +describe('Topics & resources', () => { + const data = { + otherResources: [{ originalFileName: 'other', url: { url: 'http://localhost/other' }, status: 'APPROVED' }], + attachments: [{ originalFileName: 'attachment', url: { url: 'http://localhost/attachment' }, status: 'APPROVED' }], + resourcesUsed: 'resources', + topics: 'topics', + }; + + describe('review page', () => { + it('displays attachments and other resources', async () => { + render(); + expect(await screen.findByText('other')).toBeVisible(); + expect(await screen.findByText('attachment')).toBeVisible(); + expect(await screen.findByText('resources')).toBeVisible(); + expect(await screen.findByText('topics')).toBeVisible(); + }); + }); +}); diff --git a/frontend/src/pages/ActivityReport/Pages/topicsResources.js b/frontend/src/pages/ActivityReport/Pages/topicsResources.js index 2fbf990097..3953a86f09 100644 --- a/frontend/src/pages/ActivityReport/Pages/topicsResources.js +++ b/frontend/src/pages/ActivityReport/Pages/topicsResources.js @@ -94,13 +94,13 @@ TopicsResources.propTypes = { }; const ReviewSection = () => { - const { getValues } = useFormContext(); + const { watch } = useFormContext(); const { otherResources, resourcesUsed, attachments, topics: formTopics, - } = getValues(); + } = watch(); const hasOtherResources = otherResources && otherResources.length <= 0; const hasAttachments = attachments && attachments.length <= 0; @@ -132,9 +132,10 @@ const ReviewSection = () => { /> {otherResources.map((file) => ( ))}
@@ -147,9 +148,10 @@ const ReviewSection = () => { > {attachments.map((file) => ( ))}
From 12f8e1012081d4617296830dd2e587c4d798a773 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Mon, 1 Mar 2021 15:35:10 -0600 Subject: [PATCH 05/12] Uploaded files have a presigned url after upload --- frontend/src/components/FileUploader.js | 6 ++++-- frontend/src/fetchers/File.js | 2 +- frontend/src/fetchers/__tests__/File.js | 7 +++++-- src/routes/files/handlers.js | 7 ++++--- src/routes/files/handlers.test.js | 4 +++- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/frontend/src/components/FileUploader.js b/frontend/src/components/FileUploader.js index 9d8455527e..dbbc1042ea 100644 --- a/frontend/src/components/FileUploader.js +++ b/frontend/src/components/FileUploader.js @@ -29,13 +29,15 @@ function Dropzone(props) { } else if (id === 'otherResources') { attachmentType = 'RESOURCE'; } + let url; const upload = async (file) => { try { const data = new FormData(); data.append('reportId', reportId); data.append('attachmentType', attachmentType); data.append('file', file); - await uploadFile(data); + const uploadedFile = await uploadFile(data); + url = uploadedFile.url; } catch (error) { setErrorMessage(`${file.name} failed to upload`); // eslint-disable-next-line no-console @@ -44,7 +46,7 @@ function Dropzone(props) { } setErrorMessage(null); return { - key: file.name, originalFileName: file.name, fileSize: file.size, status: 'UPLOADED', + key: file.name, originalFileName: file.name, fileSize: file.size, status: 'UPLOADED', url, }; }; const newFiles = e.map((file) => upload(file)); diff --git a/frontend/src/fetchers/File.js b/frontend/src/fetchers/File.js index a1a2f176b4..512a700b9b 100644 --- a/frontend/src/fetchers/File.js +++ b/frontend/src/fetchers/File.js @@ -11,5 +11,5 @@ export default async function uploadFile(data) { if (!res.ok) { throw new Error(res.statusText); } - return res; + return res.json(); } diff --git a/frontend/src/fetchers/__tests__/File.js b/frontend/src/fetchers/__tests__/File.js index 65767d18f9..5a7653d158 100644 --- a/frontend/src/fetchers/__tests__/File.js +++ b/frontend/src/fetchers/__tests__/File.js @@ -7,8 +7,11 @@ const fakeFile = new File(['testing'], 'test.txt'); describe('File fetcher', () => { it('test that the file gets uploaded', async () => { - fetchMock.mock(activityReportUrl, 200); + fetchMock.mock(activityReportUrl, { + body: { id: 1 }, + status: 200, + }); const res = await uploadFile(fakeFile); - expect(res.status).toBe(200); + expect(res).toEqual({ id: 1 }); }); }); diff --git a/src/routes/files/handlers.js b/src/routes/files/handlers.js index f120bbb811..25d92b0f94 100644 --- a/src/routes/files/handlers.js +++ b/src/routes/files/handlers.js @@ -2,7 +2,7 @@ import { v4 as uuidv4 } from 'uuid'; import * as fs from 'fs'; import handleErrors from '../../lib/apiErrorHandler'; import { File } from '../../models'; -import { uploadFile } from '../../lib/s3'; +import { uploadFile, getPresignedURL } from '../../lib/s3'; import addToScanQueue from '../../services/scanQueue'; import ActivityReportPolicy from '../../policies/activityReport'; @@ -121,9 +121,10 @@ export default async function uploadHandler(req, res) { return; } try { - await uploadFile(buffer, fileName, type); + const uploadedFile = await uploadFile(buffer, fileName, type); + const url = getPresignedURL(uploadedFile.key); await updateStatus(metadata.id, UPLOADED); - res.status(200).send({ id: metadata.id }); + res.status(200).send({ id: metadata.id, url }); } catch (err) { if (metadata) { await updateStatus(metadata.id, UPLOAD_FAILED); diff --git a/src/routes/files/handlers.test.js b/src/routes/files/handlers.test.js index d68ed456c2..0a4226b608 100644 --- a/src/routes/files/handlers.test.js +++ b/src/routes/files/handlers.test.js @@ -6,7 +6,7 @@ import db, { Permission, } from '../../models'; import app from '../../app'; -import { uploadFile } from '../../lib/s3'; +import { uploadFile, getPresignedURL } from '../../lib/s3'; import * as queue from '../../services/scanQueue'; import SCOPES from '../../middleware/scopeConstants'; import { REPORT_STATUSES } from '../../constants'; @@ -80,11 +80,13 @@ describe('File Upload', () => { describe('File Upload Handlers happy path', () => { beforeEach(() => { uploadFile.mockReset(); + getPresignedURL.mockReset(); }); it('tests a file upload', async () => { ActivityReportPolicy.mockImplementation(() => ({ canUpdate: () => true, })); + uploadFile.mockResolvedValue({ key: 'key' }); await request(app) .post('/api/files') .field('reportId', report.dataValues.id) From 4c8baebbfd1e195e24868bf221509cda63d9cd03 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Mon, 1 Mar 2021 17:13:22 -0600 Subject: [PATCH 06/12] Possible recipients test update To try and make the possible recipient "all recipients" test more resilient it now checks the length of "possibleRecipients" with an empty region against all ActivityRecipients --- src/services/activityReports.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/activityReports.test.js b/src/services/activityReports.test.js index e04a972247..a3f49f8667 100644 --- a/src/services/activityReports.test.js +++ b/src/services/activityReports.test.js @@ -251,10 +251,10 @@ describe('Activity Reports DB service', () => { expect(recipients.grants.length).toBe(0); }); - it('retrieves all recipients when not specifying region ', async () => { + it('retrieves all recipients when not specifying region', async () => { + const allRecipients = await ActivityRecipient.findAll(); const recipients = await possibleRecipients(); - // 11 From db being seeded + 1 that we create for this test suite = 12 - expect(recipients.grants.length).toBe(12); + expect(allRecipients.length).toEqual(recipients.grants.length); }); }); }); From fdb894590d3fbd93a11f2b30283010e18169d874 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Mon, 1 Mar 2021 20:46:18 -0600 Subject: [PATCH 07/12] Fix test --- src/services/activityReports.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/activityReports.test.js b/src/services/activityReports.test.js index a3f49f8667..8202416c2e 100644 --- a/src/services/activityReports.test.js +++ b/src/services/activityReports.test.js @@ -252,9 +252,9 @@ describe('Activity Reports DB service', () => { }); it('retrieves all recipients when not specifying region', async () => { - const allRecipients = await ActivityRecipient.findAll(); const recipients = await possibleRecipients(); - expect(allRecipients.length).toEqual(recipients.grants.length); + const grantees = await Grantee.findAll(); + expect(recipients.grants.length).toBe(grantees.length); }); }); }); From 386996ae50b2cc4bca9ec706f8fd8b1bf4f92f67 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Wed, 3 Mar 2021 09:48:15 -0600 Subject: [PATCH 08/12] Topics resource print fixes * Header is positioned relative when printing. This prevents it from repeating and being placed above content * Fix attachment "no-print" logic --- frontend/src/App.css | 3 +++ frontend/src/pages/ActivityReport/Pages/topicsResources.js | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frontend/src/App.css b/frontend/src/App.css index 82f6a408b0..2ffa03179d 100644 --- a/frontend/src/App.css +++ b/frontend/src/App.css @@ -73,6 +73,9 @@ body { } @media print { + .smart-hub-header { + position: relative; + } * { color: #000 !important; diff --git a/frontend/src/pages/ActivityReport/Pages/topicsResources.js b/frontend/src/pages/ActivityReport/Pages/topicsResources.js index 3953a86f09..f49ceabb00 100644 --- a/frontend/src/pages/ActivityReport/Pages/topicsResources.js +++ b/frontend/src/pages/ActivityReport/Pages/topicsResources.js @@ -102,8 +102,8 @@ const ReviewSection = () => { topics: formTopics, } = watch(); - const hasOtherResources = otherResources && otherResources.length <= 0; - const hasAttachments = attachments && attachments.length <= 0; + const hasOtherResources = otherResources && otherResources.length > 0; + const hasAttachments = attachments && attachments.length > 0; return ( <> From 8de76515e8c4591f04c45d9fff4ba618dad64d36 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Wed, 3 Mar 2021 09:54:34 -0600 Subject: [PATCH 09/12] Fix file uploader test --- frontend/src/components/__tests__/FileUploader.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/__tests__/FileUploader.js b/frontend/src/components/__tests__/FileUploader.js index 481f47d26f..961714bb06 100644 --- a/frontend/src/components/__tests__/FileUploader.js +++ b/frontend/src/components/__tests__/FileUploader.js @@ -32,10 +32,10 @@ describe('upload tests', () => { const mockFile = { name: 'MockFile', size: 2000 }; const mockSetErrorMessage = jest.fn(); it('can upload a file and return the correct information', async () => { - const mockFileUpload = jest.spyOn(fileFetcher, 'uploadFile').mockImplementation(async () => ({ id: 1 })); + const mockFileUpload = jest.spyOn(fileFetcher, 'uploadFile').mockImplementation(async () => ({ id: 1, url: 'url' })); const got = await upload(mockFile, 1, 'fakeAttachment', mockSetErrorMessage); expect(got).toStrictEqual({ - id: 1, originalFileName: mockFile.name, fileSize: mockFile.size, status: 'UPLOADED', + id: 1, originalFileName: mockFile.name, fileSize: mockFile.size, status: 'UPLOADED', url: 'url', }); expect(mockFileUpload).toHaveBeenCalled(); expect(mockSetErrorMessage).toHaveBeenCalledWith(null); From 2b3288aad061ff1df178c94d4780dee1b444ec33 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Wed, 3 Mar 2021 15:33:37 -0600 Subject: [PATCH 10/12] Add additional fetcher test --- frontend/src/fetchers/__tests__/activityReports.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/frontend/src/fetchers/__tests__/activityReports.js b/frontend/src/fetchers/__tests__/activityReports.js index 409473d26a..d4f12c6fb4 100644 --- a/frontend/src/fetchers/__tests__/activityReports.js +++ b/frontend/src/fetchers/__tests__/activityReports.js @@ -1,7 +1,7 @@ import join from 'url-join'; import fetchMock from 'fetch-mock'; -import { submitReport, saveReport, reviewReport } from '../activityReports'; +import { submitReport, saveReport, reviewReport, resetToDraft } from '../activityReports'; describe('activityReports fetcher', () => { afterEach(() => fetchMock.restore()); @@ -24,6 +24,15 @@ describe('activityReports fetcher', () => { }); }); + describe('resetToDraft', () => { + it('calls reset and returns the result', async () => { + const report = { id: 1 }; + fetchMock.put(join('api', 'activity-reports', '1', 'reset'), report); + const savedReport = await resetToDraft(1); + expect(savedReport).toEqual(report); + }); + }); + describe('reviewReport', () => { it('returns the report', async () => { const report = { id: 1 }; From 7f0b482d7856fdfc2823a54ea8364547b8d98f24 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Wed, 3 Mar 2021 15:35:38 -0600 Subject: [PATCH 11/12] Lint fixes --- frontend/src/fetchers/__tests__/activityReports.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/src/fetchers/__tests__/activityReports.js b/frontend/src/fetchers/__tests__/activityReports.js index d4f12c6fb4..b9ae5bca27 100644 --- a/frontend/src/fetchers/__tests__/activityReports.js +++ b/frontend/src/fetchers/__tests__/activityReports.js @@ -1,7 +1,9 @@ import join from 'url-join'; import fetchMock from 'fetch-mock'; -import { submitReport, saveReport, reviewReport, resetToDraft } from '../activityReports'; +import { + submitReport, saveReport, reviewReport, resetToDraft, +} from '../activityReports'; describe('activityReports fetcher', () => { afterEach(() => fetchMock.restore()); From 40f8a8c21378e8574bb3def9040b131e80da72c7 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Wed, 3 Mar 2021 17:00:08 -0500 Subject: [PATCH 12/12] Always access minio via the docker hostname when creating a dev bucket When $S3_ENDPOINT was set to localhost aws-cli isn't able to run properly --- docker-compose.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index f366b61f31..e48af8837c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -27,7 +27,9 @@ services: aws-cli: image: amazon/aws-cli env_file: .env - command: ["--endpoint-url", "$S3_ENDPOINT", "s3api", "create-bucket", "--bucket", "$S3_BUCKET"] + command: ["--endpoint-url", "http://minio:9000", "s3api", "create-bucket", "--bucket", "$S3_BUCKET"] + depends_on: + - minio clamav-rest: image: ajilaag/clamav-rest ports: