Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Prod] Merge goal request, fix bug with other entities, display correct options for creator/approver #2133

Merged
merged 10 commits into from
May 3, 2024
4 changes: 3 additions & 1 deletion frontend/src/components/Navigator/ActivityReportNavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const packageGoals = (goals, goal, grantIds, prompts) => [
*/
export const shouldUpdateFormData = (isAutoSave) => {
if (!isAutoSave) {
return false;
return true;
}

const richTextEditors = document.querySelectorAll('.rdw-editor-main');
Expand Down Expand Up @@ -207,6 +207,7 @@ const ActivityReportNavigator = ({
} else {
newPageState[page.position] = isDirty ? IN_PROGRESS : currentPageState;
}

return newPageState;
};
const onSaveForm = async (isAutoSave = false) => {
Expand Down Expand Up @@ -502,6 +503,7 @@ const ActivityReportNavigator = ({
// update form data
const { status, ...values } = getValues();
const data = { ...formData, ...values, pageState: newNavigatorState() };

if (allowUpdateFormData) {
updateFormData(data);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,8 @@ describe('Navigator', () => {
});

describe('shouldUpdateFormData', () => {
it('if isAutoSave is false, returns false', () => {
expect(shouldUpdateFormData(false)).toBe(false);
it('if isAutoSave is false, returns true', () => {
expect(shouldUpdateFormData(false)).toBe(true);
});

it('if we are focused on a rich editor, return false', async () => {
Expand Down
151 changes: 147 additions & 4 deletions frontend/src/pages/ActivityReport/Pages/Review/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const reportCreator = {
};

const user = {
id: 1,
name: '[email protected]',
permissions: [
{
Expand Down Expand Up @@ -89,6 +90,7 @@ const renderReview = (
isPendingApprover = false,
calculatedStatus = REPORT_STATUSES.DRAFT,
formData = {
userId: 1,
additionalNotes: '',
goalsAndObjectives: [],
},
Expand All @@ -97,10 +99,10 @@ const renderReview = (
onResetToDraft = jest.fn(),
complete = true,
approvers = null,
activityReportCollaborators = [],
) => {
const history = createMemoryHistory();
const pages = complete ? completePages : incompletePages;

render(
<Router history={history}>
<UserContext.Provider value={{ user }}>
Expand All @@ -112,7 +114,14 @@ const renderReview = (
onSubmit={onSubmit}
onResetToDraft={onResetToDraft}
formData={{
...formData, calculatedStatus, submissionStatus: calculatedStatus, author: { name: 'user' }, approvers, id: 1, displayId: '1',
...formData,
calculatedStatus,
submissionStatus: calculatedStatus,
author: { name: 'user' },
approvers,
id: 1,
displayId: '1',
activityReportCollaborators,
}}
isApprover={isApprover}
isPendingApprover={isPendingApprover}
Expand Down Expand Up @@ -140,9 +149,137 @@ describe('ReviewSubmit', () => {
renderReview(allComplete, isApprover, isPendingApprover, calculatedStatus);
const header = await screen.findByText('Review and approve report');
expect(header).toBeVisible();
expect(screen.getByRole('button', { name: 'Submit' })).toBeVisible();
expect(screen.queryByRole('button', { name: 'Submit for approval' })).toBeNull();
});

it('allows creator to modify draft when also an approver', async () => {
const allComplete = true;
const isApprover = true;
const isPendingApprover = true;
const calculatedStatus = REPORT_STATUSES.DRAFT;

renderReview(allComplete, isApprover, isPendingApprover, calculatedStatus);
const header = await screen.findByText('Review and submit report');
expect(header).toBeVisible();
expect(screen.queryAllByRole('button', { name: 'Submit' }).length).toBe(0);
expect(screen.queryByRole('button', { name: 'Submit for approval' })).toBeVisible();
});

it('allows collaborator to modify draft when also an approver', async () => {
const allComplete = true;
const isApprover = true;
const isPendingApprover = true;
const calculatedStatus = REPORT_STATUSES.DRAFT;

renderReview(
allComplete,
isApprover,
isPendingApprover,
calculatedStatus,
{
userId: 2,
additionalNotes: '',
goalsAndObjectives: [],
},
jest.fn(),
jest.fn(),
jest.fn(),
true,
null,
[{ userId: 1 }],
);
const header = await screen.findByText('Review and submit report');
expect(header).toBeVisible();
expect(screen.queryAllByRole('button', { name: 'Submit' }).length).toBe(0);
expect(screen.queryByRole('button', { name: 'Submit for approval' })).toBeVisible();
});

it('allows collaborator to modify draft when not an approver', async () => {
const allComplete = true;
const isApprover = false;
const isPendingApprover = true;
const calculatedStatus = REPORT_STATUSES.DRAFT;

renderReview(
allComplete,
isApprover,
isPendingApprover,
calculatedStatus,
{
userId: 2,
additionalNotes: '',
goalsAndObjectives: [],
},
jest.fn(),
jest.fn(),
jest.fn(),
true,
null,
[{ userId: 1 }],
);
const header = await screen.findByText('Review and submit report');
expect(header).toBeVisible();
expect(screen.queryAllByRole('button', { name: 'Submit' }).length).toBe(0);
expect(screen.getByRole('button', { name: 'Submit for approval' })).toBeVisible();
});

it('allows the manager to review the report', async () => {
it('allows collaborator to approve', async () => {
const allComplete = true;
const isApprover = true;
const isPendingApprover = true;
const calculatedStatus = REPORT_STATUSES.SUBMITTED;

renderReview(
allComplete,
isApprover,
isPendingApprover,
calculatedStatus,
{
userId: 2,
additionalNotes: '',
goalsAndObjectives: [],
},
jest.fn(),
jest.fn(),
jest.fn(),
true,
null,
[{ userId: 1 }],
);
const header = await screen.findByText('Review and submit report');
expect(header).toBeVisible();
expect(screen.getByRole('button', { name: 'Submit' })).toBeVisible();
expect(screen.queryByRole('button', { name: 'Submit for approval' })).toBeNull();
});

it('allows collaborator to reset when not approver', async () => {
const allComplete = true;
const isApprover = false;
const isPendingApprover = true;
const calculatedStatus = REPORT_STATUSES.SUBMITTED;

renderReview(
allComplete,
isApprover,
isPendingApprover,
calculatedStatus,
{
userId: 2,
additionalNotes: '',
goalsAndObjectives: [],
},
jest.fn(),
jest.fn(),
jest.fn(),
true,
null,
[{ userId: 1 }],
);
expect(screen.getByRole('button', { name: 'Reset to Draft' })).toBeVisible();
});

it('allows the approving manager to review the report', async () => {
const allComplete = true;
const isApprover = true;
const isPendingApprover = true;
Expand All @@ -152,7 +289,13 @@ describe('ReviewSubmit', () => {
const onReview = jest.fn();

renderReview(
allComplete, isApprover, isPendingApprover, calculatedStatus, formData, onSubmit, onReview,
allComplete,
isApprover,
isPendingApprover,
calculatedStatus,
{ ...formData, userId: 4 },
onSubmit,
onReview,
);
userEvent.selectOptions(screen.getByTestId('dropdown'), ['approved']);
const reviewButton = await screen.findByRole('button', { name: 'Submit' });
Expand Down
19 changes: 17 additions & 2 deletions frontend/src/pages/ActivityReport/Pages/Review/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import PrintSummary from '../PrintSummary';
import './index.scss';
import { Accordion } from '../../../../components/Accordion';
import AppLoadingContext from '../../../../AppLoadingContext';
import UserContext from '../../../../UserContext';

const ReviewSubmit = ({
onSubmit,
Expand All @@ -29,6 +30,15 @@ const ReviewSubmit = ({
const [reviewed, updateReviewed] = useState(false);
const [error, updateError] = useState();

const { user } = useContext(UserContext);

const isCreator = user.id === formData.userId;
const isDraft = formData.calculatedStatus === REPORT_STATUSES.DRAFT;
const isCollaborator = formData.activityReportCollaborators
&& formData.activityReportCollaborators.find((u) => u.userId === user.id);

const creatorOrCollaborator = (isCreator || !!isCollaborator);

const onFormSubmit = async (data) => {
try {
await onSubmit(data);
Expand Down Expand Up @@ -79,7 +89,7 @@ const ReviewSubmit = ({
<title>Review and Submit</title>
</Helmet>
<PrintSummary reportCreator={reportCreator} />
{!isApprover
{(!isApprover || (isDraft && creatorOrCollaborator))
&& (
<Submitter
availableApprovers={availableApprovers}
Expand All @@ -96,7 +106,7 @@ const ReviewSubmit = ({
</>
</Submitter>
)}
{isApprover
{(isApprover && !isDraft)
&& (
<Approver
availableApprovers={availableApprovers}
Expand Down Expand Up @@ -135,6 +145,11 @@ ReviewSubmit.propTypes = {
formData: PropTypes.shape({
additionalNotes: PropTypes.string,
calculatedStatus: PropTypes.string,
submissionStatus: PropTypes.string,
userId: PropTypes.number,
activityReportCollaborators: PropTypes.arrayOf(PropTypes.shape({
userId: PropTypes.number,
})),
}).isRequired,
reportCreator: PropTypes.shape({
name: PropTypes.string.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ export default function OtherEntity({
/>
);
})}
<PlusButton text="Add new objective" onClick={onAddNew} />
{(recipientIds.length > 0) && (
<PlusButton text="Add new objective" onClick={onAddNew} />
)}
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import '@testing-library/jest-dom';
import userEvent from '@testing-library/user-event';
import { render, screen, waitFor } from '@testing-library/react';
import React from 'react';
import PropTypes from 'prop-types';
import fetchMock from 'fetch-mock';
import { FormProvider, useForm } from 'react-hook-form';
import OtherEntity from '../OtherEntity';
import UserContext from '../../../../../UserContext';

let setError;

// eslint-disable-next-line react/prop-types
const RenderOtherEntity = ({ objectivesWithoutGoals }) => {
const RenderOtherEntity = ({ objectivesWithoutGoals, recipientIds }) => {
const hookForm = useForm({
mode: 'onChange',
defaultValues: {
Expand All @@ -24,12 +24,18 @@ const RenderOtherEntity = ({ objectivesWithoutGoals }) => {
return (
<UserContext.Provider value={{ user: { flags: [] } }}>
<FormProvider {...hookForm}>
<OtherEntity recipientIds={[]} onSaveDraft={jest.fn()} reportId="123" />
<OtherEntity recipientIds={recipientIds} onSaveDraft={jest.fn()} reportId="123" />
</FormProvider>
</UserContext.Provider>
);
};

RenderOtherEntity.propTypes = {
// eslint-disable-next-line react/forbid-prop-types
objectivesWithoutGoals: PropTypes.arrayOf(PropTypes.object).isRequired,
recipientIds: PropTypes.arrayOf(PropTypes.number).isRequired,
};

const objectives = [
{
title: 'title',
Expand Down Expand Up @@ -58,20 +64,20 @@ describe('OtherEntity', () => {
<id>https://acf-ohs.atlassian.net/wiki</id></feed>`);
});
it('renders created objectives', async () => {
render(<RenderOtherEntity objectivesWithoutGoals={objectives} />);
render(<RenderOtherEntity objectivesWithoutGoals={objectives} recipientIds={[1]} />);

const title = await screen.findByText('title', { selector: 'textarea' });
expect(title).toBeVisible();
});

it('renders without roles', async () => {
render(<RenderOtherEntity objectivesWithoutGoals={objectives} />);
render(<RenderOtherEntity objectivesWithoutGoals={objectives} recipientIds={[1]} />);
const title = await screen.findByText('title', { selector: 'textarea' });
expect(title).toBeVisible();
});

it('the button adds a new objective', async () => {
render(<RenderOtherEntity objectivesWithoutGoals={[]} />);
render(<RenderOtherEntity objectivesWithoutGoals={[]} recipientIds={[1]} />);
const button = await screen.findByRole('button', { name: /Add new objective/i });
userEvent.click(button);
expect(screen.queryAllByText(/objective status/i).length).toBe(1);
Expand All @@ -80,7 +86,7 @@ describe('OtherEntity', () => {
});

it('displays errors', async () => {
render(<RenderOtherEntity objectivesWithoutGoals={[]} />);
render(<RenderOtherEntity objectivesWithoutGoals={[]} recipientIds={[1]} />);
const button = await screen.findByRole('button', { name: /Add new objective/i });
userEvent.click(button);
expect(screen.queryAllByText(/objective status/i).length).toBe(1);
Expand All @@ -90,4 +96,10 @@ describe('OtherEntity', () => {
setError('objectivesWithoutGoals[0].title', { type: 'required', message: 'ENTER A TITLE' });
await waitFor(() => expect(screen.queryByText(/ENTER A TITLE/i)).toBeVisible());
});

it('hides plus button when there are no recipients selected errors', async () => {
render(<RenderOtherEntity objectivesWithoutGoals={[]} recipientIds={[]} />);
const button = screen.queryByRole('button', { name: /Add new objective/i });
expect(button).toBeNull();
});
});
Loading