Merge pull request #189 from adhocteam/js-313-form-navigation-update

Form navigation more consistent
HHS · Mar 1, 2021 · 90a7211 · 90a7211
2 parents 5e1e303 + 3927696
commit 90a7211
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 55 deletions.
diff --git a/frontend/src/components/Navigator/__tests__/index.js b/frontend/src/components/Navigator/__tests__/index.js
@@ -29,7 +29,7 @@ const pages = [
     label: 'first page',
     review: false,
     render: () => (
-      <Input name="first" />
+      <Input name="first" required />
     ),
   },
   {
@@ -38,7 +38,7 @@ const pages = [
     label: 'second page',
     review: false,
     render: () => (
-      <Input name="second" />
+      <Input name="second" required />
     ),
   },
   {
@@ -109,26 +109,19 @@ describe('Navigator', () => {
     await waitFor(() => expect(onSubmit).toHaveBeenCalled());
   });
 
-  it('shows an error message if the form is not valid', async () => {
-    renderNavigator('third');
-    const button = await screen.findByRole('button', { name: 'Save & Continue' });
-    userEvent.click(button);
-    expect(await screen.findByTestId('alert')).toHaveTextContent('Please complete all required fields before submitting this report.');
-  });
-
   it('onBack calls onUpdatePage', async () => {
     const updatePage = jest.fn();
     renderNavigator('third', () => {}, () => {}, updatePage);
     const button = await screen.findByRole('button', { name: 'Back' });
     userEvent.click(button);
-    expect(updatePage).toHaveBeenCalledWith(2);
+    await waitFor(() => expect(updatePage).toHaveBeenCalledWith(2));
   });
 
   it('calls onSave on navigation', async () => {
     const updatePage = jest.fn();
     const updateForm = jest.fn();
     renderNavigator('second', () => {}, () => {}, updatePage, updateForm);
-    userEvent.click(screen.getByRole('button', { name: 'first page' }));
+    userEvent.click(await screen.findByRole('button', { name: 'first page' }));
     await waitFor(() => expect(updateForm).toHaveBeenCalledWith({ ...initialData, second: null }));
     await waitFor(() => expect(updatePage).toHaveBeenCalledWith(1));
   });

diff --git a/frontend/src/components/Navigator/index.js b/frontend/src/components/Navigator/index.js
@@ -4,7 +4,7 @@
   on the left hand side with each page of the form listed. Clicking on an item in the nav list will
   display that item in the content section. The navigator keeps track of the "state" of each page.
 */
-import React, { useState } from 'react';
+import React, { useEffect, useState } from 'react';
 import PropTypes from 'prop-types';
 import { FormProvider, useForm } from 'react-hook-form';
 import {
@@ -45,6 +45,7 @@ function Navigator({
 }) {
   const [errorMessage, updateErrorMessage] = useState();
   const [lastSaveTime, updateLastSaveTime] = useState(initialLastUpdated);
+  const [showValidationErrors, updateShowValidationErrors] = useState(false);
   const { pageState } = formData;
   const page = pages.find((p) => p.path === currentPage);
 
@@ -55,63 +56,65 @@ function Navigator({
 
   const {
     formState,
-    handleSubmit,
     getValues,
     reset,
+    trigger,
   } = hookForm;
 
-  const { isDirty, errors } = formState;
-
+  const { isDirty, errors, isValid } = formState;
   const hasErrors = Object.keys(errors).length > 0;
 
-  const newNavigatorState = (completed) => {
+  useEffect(() => {
+    if (showValidationErrors && !page.review) {
+      trigger();
+    }
+  }, [page.review, trigger, showValidationErrors]);
+
+  const newNavigatorState = () => {
     if (page.review) {
       return pageState;
     }
 
     const newPageState = { ...pageState };
-    if (completed) {
+    if (isValid) {
       newPageState[page.position] = COMPLETE;
     } else {
       newPageState[page.position] = isDirty ? IN_PROGRESS : pageState[page.position];
     }
     return newPageState;
   };
 
-  const onSaveForm = async (completed) => {
+  const onSaveForm = async () => {
     if (!editable) {
       return;
     }
     const { status, ...values } = getValues();
-    const data = { ...formData, ...values, pageState: newNavigatorState(completed) };
+    const data = { ...formData, ...values, pageState: newNavigatorState() };
+
+    updateFormData(data);
     try {
-      const result = await onSave(data);
-      if (result) {
-        updateLastSaveTime(moment());
-        updateErrorMessage();
-      }
+      await onSave(data);
+      updateLastSaveTime(moment());
+      updateErrorMessage();
     } catch (error) {
       // eslint-disable-next-line no-console
       console.log(error);
       updateErrorMessage('Unable to save activity report');
     }
   };
 
-  const onUpdatePage = (index, completed) => {
+  const onUpdatePage = async (index) => {
     const newIndex = index === page.position ? null : index;
-    const { status, ...values } = getValues();
-    const data = { ...formData, ...values, pageState: newNavigatorState(completed) };
-    updateFormData(data);
+    await onSaveForm();
     updatePage(newIndex);
   };
 
   const onContinue = () => {
-    onSaveForm(true);
-    onUpdatePage(page.position + 1, true);
+    onUpdatePage(page.position + 1);
   };
 
   useInterval(() => {
-    onSaveForm(false);
+    onSaveForm();
   }, autoSaveInterval);
 
   // A new form page is being shown so we need to reset `react-hook-form` so validations are
@@ -163,6 +166,7 @@ function Navigator({
               onSaveForm,
               navigatorPages,
               reportCreator,
+              updateShowValidationErrors,
             )}
             {!page.review
             && (
@@ -177,14 +181,13 @@ function Navigator({
                   </Alert>
                 )}
                 <Form
-                  onSubmit={handleSubmit(onContinue)}
                   className="smart-hub--form-large"
                 >
                   {page.render(additionalData, formData, reportId)}
                   <div className="display-flex">
                     <Button disabled={page.position <= 1} outline type="button" onClick={() => { onUpdatePage(page.position - 1); }}>Back</Button>
-                    <Button type="button" onClick={() => { onSaveForm(false); }}>Save draft</Button>
-                    <Button className="margin-left-auto margin-right-0" type="submit">Save & Continue</Button>
+                    <Button type="button" onClick={() => { onSaveForm(); }}>Save draft</Button>
+                    <Button className="margin-left-auto margin-right-0" type="button" onClick={onContinue}>Save & Continue</Button>
                   </div>
                 </Form>
               </Container>

diff --git a/frontend/src/pages/ActivityReport/Pages/Review/__tests__/index.js b/frontend/src/pages/ActivityReport/Pages/Review/__tests__/index.js
@@ -41,6 +41,7 @@ const RenderReview = ({
         approvingManager={approvingManager}
         pages={pages}
         reportCreator={reportCreator}
+        updateShowValidationErrors={() => {}}
       />
     </FormProvider>
   );

diff --git a/frontend/src/pages/ActivityReport/Pages/Review/index.js b/frontend/src/pages/ActivityReport/Pages/Review/index.js
@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
 import PropTypes from 'prop-types';
 import {
   Accordion,
@@ -21,9 +21,14 @@ const ReviewSubmit = ({
   onResetToDraft,
   onSaveForm,
   pages,
+  updateShowValidationErrors,
 }) => {
   const { additionalNotes, status } = formData;
 
+  useEffect(() => {
+    updateShowValidationErrors(true);
+  }, [updateShowValidationErrors]);
+
   const [reviewed, updateReviewed] = useState(false);
   const [error, updateError] = useState();
 
@@ -95,6 +100,7 @@ const ReviewSubmit = ({
 };
 
 ReviewSubmit.propTypes = {
+  updateShowValidationErrors: PropTypes.func.isRequired,
   onSaveForm: PropTypes.func.isRequired,
   approvers: PropTypes.arrayOf(
     PropTypes.shape({

diff --git a/frontend/src/pages/ActivityReport/Pages/activitySummary.js b/frontend/src/pages/ActivityReport/Pages/activitySummary.js
@@ -2,6 +2,7 @@ import React, { useEffect, useRef } from 'react';
 import PropTypes from 'prop-types';
 import { Helmet } from 'react-helmet';
 import { useFormContext } from 'react-hook-form';
+import { isEmpty } from 'lodash';
 
 import {
   Fieldset, Radio, Grid, TextInput, Checkbox,
@@ -47,7 +48,7 @@ const ActivitySummary = ({
     value: nonGrantee.activityRecipientId,
   }));
 
-  const disableRecipients = activityRecipientType === '';
+  const disableRecipients = isEmpty(activityRecipientType);
   const nonGranteeSelected = activityRecipientType === 'non-grantee';
   const granteeSelected = activityRecipientType === 'grantee';
   const selectedRecipients = nonGranteeSelected ? nonGrantees : grants;

diff --git a/frontend/src/pages/ActivityReport/Pages/index.js b/frontend/src/pages/ActivityReport/Pages/index.js
@@ -37,8 +37,10 @@ const reviewPage = {
       onSaveForm,
       allPages,
       reportCreator,
+      updateShowValidationErrors,
     ) => (
       <ReviewSubmit
+        updateShowValidationErrors={updateShowValidationErrors}
         approvers={additionalData.approvers}
         onSubmit={onSubmit}
         onSaveForm={onSaveForm}

diff --git a/frontend/src/pages/ActivityReport/__tests__/index.js b/frontend/src/pages/ActivityReport/__tests__/index.js
@@ -35,6 +35,7 @@ const formData = () => ({
   numberOfParticipants: '1',
   reason: ['reason 1'],
   activityRecipientType: 'grantee',
+  collaborators: [],
   participants: ['CEO / CFO / Executive'],
   programTypes: ['type 1'],
   requester: 'grantee',

diff --git a/frontend/src/pages/ActivityReport/index.js b/frontend/src/pages/ActivityReport/index.js
@@ -32,17 +32,17 @@ import {
 } from '../../fetchers/activityReports';
 
 const defaultValues = {
-  deliveryMethod: [],
+  deliveryMethod: null,
   activityRecipientType: '',
   activityRecipients: [],
   activityType: [],
   attachments: [],
   context: '',
   collaborators: [],
-  duration: '',
+  duration: null,
   endDate: null,
   grantees: [],
-  numberOfParticipants: '',
+  numberOfParticipants: null,
   participantCategory: '',
   participants: [],
   programTypes: [],
@@ -178,31 +178,23 @@ function ActivityReport({
     if (!editable) {
       return;
     }
-
-    const page = pages.find((p) => p.position === position);
     const state = {};
     if (activityReportId === 'new' && reportId.current !== 'new') {
       state.showLastUpdatedTime = true;
     }
+
+    const page = pages.find((p) => p.position === position);
     history.replace(`/activity-reports/${reportId.current}/${page.path}`, state);
   };
 
   const onSave = async (data) => {
-    const { activityRecipientType, activityRecipients } = data;
-    let updatedReport = false;
     if (reportId.current === 'new') {
-      if (activityRecipientType && activityRecipients && activityRecipients.length > 0) {
-        const savedReport = await createReport({ ...data, regionId: formData.regionId }, {});
-        reportId.current = savedReport.id;
-        const current = pages.find((p) => p.path === currentPage);
-        updatePage(current.position);
-        updatedReport = false;
-      }
+      const savedReport = await createReport({ ...data, regionId: formData.regionId }, {});
+      reportId.current = savedReport.id;
+      window.history.replaceState(null, null, `/activity-reports/${savedReport.id}/${currentPage}`);
     } else {
       await saveReport(reportId.current, data, {});
-      updatedReport = true;
     }
-    return updatedReport;
   };
 
   const onFormSubmit = async (data) => {

diff --git a/src/models/activityReport.js b/src/models/activityReport.js
@@ -80,7 +80,6 @@ export default (sequelize, DataTypes) => {
       get: formatDate,
     },
     activityRecipientType: {
-      allowNull: false,
       type: DataTypes.STRING,
     },
     requester: {

diff --git a/src/services/activityReports.js b/src/services/activityReports.js
@@ -75,7 +75,8 @@ async function saveReportRecipients(
     activityReportId,
   };
 
-  if (activityRecipientType === 'non-grantee') {
+  const empty = activityRecipientIds.length === 0;
+  if (!empty && activityRecipientType === 'non-grantee') {
     where[Op.or] = {
       nonGranteeId: {
         [Op.notIn]: activityRecipientIds,
@@ -84,7 +85,7 @@ async function saveReportRecipients(
         [Op.not]: null,
       },
     };
-  } else if (activityRecipientType === 'grantee') {
+  } else if (!empty && activityRecipientType === 'grantee') {
     where[Op.or] = {
       grantId: {
         [Op.notIn]: activityRecipientIds,