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.

Sign up for GitHub

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

MB-13125 - handle state for weightTickets #8873

Merged
merged 19 commits into from
Jul 18, 2022
Merged

MB-13125 - handle state for weightTickets #8873

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c992558
WIP, action is not a plain object error
ronaktruss Jul 8, 2022
1311e8d
Fix dispatch issue and remove local logging
ronaktruss Jul 11, 2022
36080d7
Clean up comments
ronaktruss Jul 11, 2022
570c2e9
Update tests to use new selector
ronaktruss Jul 11, 2022
f67297a
Add back accidentally removed code
ronaktruss Jul 11, 2022
59a6581
Remove unnecesary awaits
ronaktruss Jul 11, 2022
8595730
Clean up test diffs
ronaktruss Jul 11, 2022
b8c6131
Add functionality to show error msg when create fails during initial …
ronaktruss Jul 12, 2022
e93a5fb
Add logic for how redux state would be updated during an update
ronaktruss Jul 12, 2022
d5c4a53
Add test for selector
ronaktruss Jul 12, 2022
66de399
Remove unnecessary optional chaining
ronaktruss Jul 13, 2022
12c13e5
Add implementation details for how to handle uploads and deletes in R…
ronaktruss Jul 14, 2022
98c994b
Refactor selector name
ronaktruss Jul 14, 2022
b66fe15
Add minimal updates to fix tests
ronaktruss Jul 14, 2022
2443572
Rename variablesto remove additional check
ronaktruss Jul 14, 2022
8ba24db
Merge branch 'master' into rp-MB-13125-handle-state-weight-tickets
ronaktruss Jul 14, 2022
d389589
Update storybook files with the name changes for document fields
ronaktruss Jul 14, 2022
3d6e244
Update comment to remove uncertainty
ronaktruss Jul 14, 2022
fceb84e
Merge branch 'master' into rp-MB-13125-handle-state-weight-tickets
mergify[bot] Jul 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 64 additions & 24 deletions src/pages/MyMove/PPM/Closeout/WeightTickets/WeightTickets.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React, { useEffect, useState } from 'react';
import { generatePath, useHistory, useParams, useLocation } from 'react-router-dom';
import { useSelector } from 'react-redux';
import { useDispatch, useSelector } from 'react-redux';
import { Alert, Grid, GridContainer } from '@trussworks/react-uswds';
import qs from 'query-string';
import { v4 as uuidv4 } from 'uuid';

import { selectMTOShipmentById } from 'store/entities/selectors';
import { selectMTOShipmentById, selectWeightTicketAndIndexById } from 'store/entities/selectors';
import { customerRoutes, generalRoutes } from 'constants/routes';
import { createUploadForDocument, createWeightTicket, patchWeightTicket } from 'services/internalApi';
import LoadingPlaceholder from 'shared/LoadingPlaceholder';
Expand All @@ -15,28 +15,33 @@ import ShipmentTag from 'components/ShipmentTag/ShipmentTag';
import { shipmentTypes } from 'constants/shipments';
import closingPageStyles from 'pages/MyMove/PPM/Closeout/Closeout.module.scss';
import WeightTicketForm from 'components/Customer/PPM/Closeout/WeightTicketForm/WeightTicketForm';
import { updateMTOShipment } from 'store/entities/actions';

const WeightTickets = () => {
const [errorMessage, setErrorMessage] = useState();

const dispatch = useDispatch();
const history = useHistory();
const { moveId, mtoShipmentId, weightTicketId } = useParams();

const { search } = useLocation();

const { tripNumber } = qs.parse(search);

// TODO remove when replaced by Redux call
const [weightTicket, setWeightTicket] = useState();

const mtoShipment = useSelector((state) => selectMTOShipmentById(state, mtoShipmentId));
// TODO add selector for selecting weight ticket from Redux store when data changes are solidified
const { weightTicket: currentWeightTicket, index: currentIndex } = useSelector((state) =>
selectWeightTicketAndIndexById(state, mtoShipmentId, weightTicketId),
);

useEffect(() => {
if (!weightTicketId) {
createWeightTicket(mtoShipmentId)
.then((resp) => {
// TODO save weight ticket response in Redux and then the selector will assign the weight ticket
setWeightTicket(resp);
if (mtoShipment?.ppmShipment?.weightTickets) {
mtoShipment.ppmShipment.weightTickets.push(resp);
} else {
mtoShipment.ppmShipment.weightTickets = [resp];
}
// I think it's necessary to update the URL so the back button would work and not create
// a new weight ticket on refresh either.
history.replace(
Expand All @@ -46,30 +51,37 @@ const WeightTickets = () => {
weightTicketId: resp.id,
}),
);
dispatch(updateMTOShipment(mtoShipment));
})
.catch(() => {
setErrorMessage('Failed to create trip record');
});
}
}, [weightTicketId, moveId, mtoShipmentId, history]);
}, [weightTicketId, moveId, mtoShipmentId, history, dispatch, mtoShipment]);

const handleCreateUpload = (fieldName, file) => {
const handleCreateUpload = async (fieldName, file) => {
let documentId;
let documentField;
switch (fieldName) {
case 'emptyWeightTickets':
documentId = weightTicket.emptyWeightDocumentId;
documentId = currentWeightTicket.emptyWeightDocumentId;
documentField = 'emptyDocument';
break;
case 'fullWeightTickets':
documentId = weightTicket.fullWeightDocumentId;
documentId = currentWeightTicket.fullWeightDocumentId;
documentField = 'fullDocument';
break;
case 'trailerOwnershipDocs':
documentId = weightTicket.trailerOwnershipDocumentId;
documentId = currentWeightTicket.trailerOwnershipDocumentId;
documentField = 'proofOfTrailerOwnershipDocument';
break;
default:
}

createUploadForDocument(file, documentId)
.then((upload) => {
mtoShipment.ppmShipment.weightTickets[currentIndex][documentField].push(upload);
dispatch(updateMTOShipment(mtoShipment));
return upload;
})
.catch(() => {
Expand All @@ -96,9 +108,27 @@ const WeightTickets = () => {
};

const handleUploadDelete = (uploadId, fieldName, values, setFieldTouched, setFieldValue) => {
const remainingUploads = values[`${fieldName}`]?.filter((upload) => upload.id !== uploadId);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason that something like values[`${fieldName}`] is used over values[fieldName]?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's usually to avoid eslint detect-object-injection errors because fieldName, if provided by the user or other malicious source, might not be a string eslint-community/eslint-plugin-security#21

let documentField;
switch (fieldName) {
case 'emptyWeightTickets':
documentField = 'emptyDocument';
break;
case 'fullWeightTickets':
documentField = 'fullDocument';
break;
case 'trailerOwnershipDocs':
documentField = 'proofOfTrailerOwnershipDocument';
break;
default:
}
ronaktruss marked this conversation as resolved.
Show resolved Hide resolved
const filterdDocuments = mtoShipment.ppmShipment.weightTickets[currentIndex][documentField].filter(
(upload) => upload.id !== uploadId,
);
mtoShipment.ppmShipment.weightTickets[currentIndex][documentField] = filterdDocuments;
const remainingUploads = values[fieldName]?.filter((upload) => upload.id !== uploadId);
setFieldTouched(fieldName, true, true);
setFieldValue(fieldName, remainingUploads, true);
dispatch(updateMTOShipment(mtoShipment));
};

const handleBack = () => {
Expand All @@ -120,19 +150,33 @@ const WeightTickets = () => {
trailerMeetsCriteria,
};

patchWeightTicket(mtoShipment.id, weightTicket.id, payload, weightTicket.eTag)
.then(() => {
patchWeightTicket(mtoShipment.id, currentWeightTicket.id, payload, currentWeightTicket.eTag)
.then((resp) => {
setSubmitting(false);
mtoShipment.ppmShipment.weightTickets[currentIndex] = resp;
history.push(generatePath(customerRoutes.SHIPMENT_PPM_REVIEW_PATH, { moveId, mtoShipmentId }));
dispatch(updateMTOShipment(mtoShipment));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is reachable if it comes after history.push(), it might need moved before the history call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tested this, it seemed to work as expected. I'll double check the behavior during the integration.

})
.catch(() => {
setSubmitting(false);
setErrorMessage('Failed to save updated trip record');
});
};

if (!mtoShipment || !weightTicket) {
return <LoadingPlaceholder />;
const renderError = () => {
if (!errorMessage) {
return null;
}

return (
<Alert slim type="error">
{errorMessage}
</Alert>
);
};

if (!mtoShipment || !currentWeightTicket) {
return renderError() || <LoadingPlaceholder />;
}

return (
Expand All @@ -143,11 +187,7 @@ const WeightTickets = () => {
<Grid col desktop={{ col: 8, offset: 2 }}>
<ShipmentTag shipmentType={shipmentTypes.PPM} />
<h1>Weight Tickets</h1>
{errorMessage && (
<Alert slim type="error">
{errorMessage}
</Alert>
)}
{renderError()}
<div className={closingPageStyles['closing-section']}>
<p>
Weight tickets should include both an empty or full weight ticket for each segment or trip. If you’re
Expand All @@ -157,7 +197,7 @@ const WeightTickets = () => {
<p>You must upload at least one set of weight tickets to get paid for your PPM.</p>
</div>
<WeightTicketForm
weightTicket={weightTicket}
weightTicket={currentWeightTicket}
tripNumber={tripNumber}
onCreateUpload={handleCreateUpload}
onUploadComplete={handleUploadComplete}
Expand Down
40 changes: 25 additions & 15 deletions src/pages/MyMove/PPM/Closeout/WeightTickets/WeightTickets.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import userEvent from '@testing-library/user-event';
import { useParams, generatePath } from 'react-router-dom';
import { v4 } from 'uuid';

import { selectMTOShipmentById } from 'store/entities/selectors';
import { selectMTOShipmentById, selectWeightTicketAndIndexById } from 'store/entities/selectors';
import { customerRoutes, generalRoutes } from 'constants/routes';
import { createWeightTicket, patchWeightTicket } from 'services/internalApi';
import { MockProviders } from 'testUtils';
Expand Down Expand Up @@ -117,6 +117,7 @@ const mockWeightTicketWithUploads = {
jest.mock('store/entities/selectors', () => ({
...jest.requireActual('store/entities/selectors'),
selectMTOShipmentById: jest.fn(() => mockMTOShipment),
selectWeightTicketAndIndexById: jest.fn(),
}));

beforeEach(() => {
Expand Down Expand Up @@ -146,7 +147,7 @@ describe('Weight Tickets page', () => {
});

it('displays an error if the createWeightTicket request fails', async () => {
createWeightTicket.mockRejectedValueOnce('an error occurred');
createWeightTicket.mockRejectedValue('an error occurred');

render(<WeightTickets />, { wrapper: MockProviders });

Expand All @@ -161,6 +162,7 @@ describe('Weight Tickets page', () => {
mtoShipmentId: mockMTOShipmentId,
weightTicketId: mockWeightTicketId,
}));
selectWeightTicketAndIndexById.mockReturnValue(mockWeightTicket);

render(<WeightTickets />, { wrapper: MockProviders });

Expand All @@ -171,6 +173,8 @@ describe('Weight Tickets page', () => {

it('renders the page Content', async () => {
createWeightTicket.mockResolvedValue(mockWeightTicket);
selectWeightTicketAndIndexById.mockReturnValueOnce(null);
selectWeightTicketAndIndexById.mockReturnValue(mockWeightTicket);

render(<WeightTickets />, { wrapper: MockProviders });

Expand All @@ -195,6 +199,8 @@ describe('Weight Tickets page', () => {

it('replaces the router history with newly created weight ticket id', async () => {
createWeightTicket.mockResolvedValue(mockWeightTicket);
selectWeightTicketAndIndexById.mockReturnValueOnce(null);
selectWeightTicketAndIndexById.mockReturnValue(mockWeightTicket);

render(<WeightTickets />, { wrapper: MockProviders });

Expand All @@ -205,6 +211,7 @@ describe('Weight Tickets page', () => {

it('routes back to home when finish later is clicked', async () => {
createWeightTicket.mockResolvedValue(mockWeightTicket);
selectWeightTicketAndIndexById.mockReturnValue(mockWeightTicket);

render(<WeightTickets />, { wrapper: MockProviders });

Expand All @@ -217,20 +224,21 @@ describe('Weight Tickets page', () => {

it('calls patch weight ticket with the appropriate payload', async () => {
createWeightTicket.mockResolvedValue(mockWeightTicketWithUploads);
selectWeightTicketAndIndexById.mockReturnValue(mockWeightTicketWithUploads);
patchWeightTicket.mockResolvedValue({});

render(<WeightTickets />, { wrapper: MockProviders });

await waitFor(() => {
expect(screen.getByRole('heading', { level: 2 })).toHaveTextContent('Trip 2');
});
await userEvent.type(screen.getByLabelText('Vehicle description'), 'DMC Delorean');
await userEvent.type(screen.getByLabelText('Empty weight'), '4999');
await userEvent.type(screen.getByLabelText('Full weight'), '6999');
await userEvent.click(screen.getByLabelText('Yes'));
await userEvent.click(screen.getAllByLabelText('Yes')[1]);
userEvent.type(screen.getByLabelText('Vehicle description'), 'DMC Delorean');
userEvent.type(screen.getByLabelText('Empty weight'), '4999');
userEvent.type(screen.getByLabelText('Full weight'), '6999');
userEvent.click(screen.getByLabelText('Yes'));
userEvent.click(screen.getAllByLabelText('Yes')[1]);

await userEvent.click(screen.getByRole('button', { name: 'Save & Continue' }));
userEvent.click(screen.getByRole('button', { name: 'Save & Continue' }));

await waitFor(() => {
expect(patchWeightTicket).toHaveBeenCalledWith(
Expand All @@ -255,20 +263,21 @@ describe('Weight Tickets page', () => {

it('displays an error if patchWeightTicket fails', async () => {
createWeightTicket.mockResolvedValue(mockWeightTicketWithUploads);
selectWeightTicketAndIndexById.mockReturnValue(mockWeightTicketWithUploads);
patchWeightTicket.mockRejectedValueOnce('an error occurred');

render(<WeightTickets />, { wrapper: MockProviders });

await waitFor(() => {
expect(screen.getByRole('heading', { level: 2 })).toHaveTextContent('Trip 2');
});
await userEvent.type(screen.getByLabelText('Vehicle description'), 'DMC Delorean');
await userEvent.type(screen.getByLabelText('Empty weight'), '4999');
await userEvent.type(screen.getByLabelText('Full weight'), '6999');
await userEvent.click(screen.getByLabelText('Yes'));
await userEvent.click(screen.getAllByLabelText('Yes')[1]);
userEvent.type(screen.getByLabelText('Vehicle description'), 'DMC Delorean');
userEvent.type(screen.getByLabelText('Empty weight'), '4999');
userEvent.type(screen.getByLabelText('Full weight'), '6999');
userEvent.click(screen.getByLabelText('Yes'));
userEvent.click(screen.getAllByLabelText('Yes')[1]);

await userEvent.click(screen.getByRole('button', { name: 'Save & Continue' }));
userEvent.click(screen.getByRole('button', { name: 'Save & Continue' }));

await waitFor(() => {
expect(screen.getByText('Failed to save updated trip record')).toBeInTheDocument();
Expand All @@ -277,6 +286,7 @@ describe('Weight Tickets page', () => {

it('calls the delete handler when removing an existing upload', async () => {
createWeightTicket.mockResolvedValue(mockWeightTicketWithUploads);
selectWeightTicketAndIndexById.mockReturnValue(mockWeightTicketWithUploads);

render(<WeightTickets />, { wrapper: MockProviders });

Expand All @@ -285,7 +295,7 @@ describe('Weight Tickets page', () => {
deleteButtons = screen.getAllByRole('button', { name: 'Delete' });
expect(deleteButtons).toHaveLength(2);
});
await userEvent.click(deleteButtons[0]);
userEvent.click(deleteButtons[0]);
await waitFor(() => {
expect(screen.queryByText('empty_weight.jpg')).not.toBeInTheDocument();
});
Expand Down
16 changes: 16 additions & 0 deletions src/store/entities/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,22 @@ export function selectMTOShipmentById(state, id) {
}

/** PPMs */
export const selectWeightTicketAndIndexById = (state, mtoShipmentId, weightTicketId) => {
let weightTicket = null;
let index = -1;
if (weightTicketId == null) {
return { weightTicket, index };
}

const mtoShipment = selectMTOShipmentById(state, mtoShipmentId);
const weightTickets = mtoShipment?.ppmShipment?.weightTickets;
if (Array.isArray(weightTickets)) {
index = weightTickets.findIndex((ele) => ele.id === weightTicketId);
weightTicket = weightTickets?.[index] || null;
}
return { weightTicket, index };
};

export const selectPPMForMove = (state, moveId) => {
const ppmForMove = Object.values(state.entities.personallyProcuredMoves).find((ppm) => ppm.move_id === moveId);
if (['DRAFT', 'SUBMITTED', 'APPROVED', 'PAYMENT_REQUESTED', 'COMPLETED'].indexOf(ppmForMove?.status) > -1) {
Expand Down
Loading