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

MB-13125 - handle state for weightTickets #8873

Merged
merged 19 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
54 changes: 33 additions & 21 deletions src/pages/MyMove/PPM/Closeout/WeightTickets/WeightTickets.jsx
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, selectWeightTicketById } 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,31 @@ 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 currentWeightTicket = useSelector((state) => selectWeightTicketById(state, mtoShipmentId, weightTicketId));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it may be worth having a selector for getting all weightTickets and then another selector for getting the current weight ticket


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);
ronaktruss marked this conversation as resolved.
Show resolved Hide resolved
} else {
mtoShipment.ppmShipment.weightTickets = [resp];
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic may not be necessary if weightTickets defaults to returning an empty array

Copy link
Contributor

Choose a reason for hiding this comment

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

thought: We return an empty array for other things like orders and MTOShipments, so I feel like we should probably remain consistent with that for weight tickets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave this here for now, since we can't ensure that within this PR. But we can likely remove this during the integration story.

// 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,24 +49,25 @@ 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) => {
let documentId;
switch (fieldName) {
case 'emptyWeightTickets':
documentId = weightTicket.emptyWeightDocumentId;
documentId = currentWeightTicket.emptyWeightDocumentId;
break;
case 'fullWeightTickets':
documentId = weightTicket.fullWeightDocumentId;
documentId = currentWeightTicket.fullWeightDocumentId;
break;
case 'trailerOwnershipDocs':
documentId = weightTicket.trailerOwnershipDocumentId;
documentId = currentWeightTicket.trailerOwnershipDocumentId;
break;
default:
}
Expand Down Expand Up @@ -120,7 +124,7 @@ const WeightTickets = () => {
trailerMeetsCriteria,
};

patchWeightTicket(mtoShipment.id, weightTicket.id, payload, weightTicket.eTag)
patchWeightTicket(mtoShipment.id, currentWeightTicket.id, payload, currentWeightTicket.eTag)
.then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add in what the updating of the shipment weight ticket would look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. There are quite a few ways to handle this, happy to discuss alternatives.

To test locally relax the UI restrictions(remove yup validation/disabled condition) and ensure that the create and patch call return promises with the same ID.

setSubmitting(false);
history.push(generatePath(customerRoutes.SHIPMENT_PPM_REVIEW_PATH, { moveId, mtoShipmentId }));
Expand All @@ -131,8 +135,20 @@ const WeightTickets = () => {
});
};

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 +159,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 +169,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
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, selectWeightTicketById } 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),
selectWeightTicketById: 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,
}));
selectWeightTicketById.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);
selectWeightTicketById.mockReturnValueOnce(null);
selectWeightTicketById.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);
selectWeightTicketById.mockReturnValueOnce(null);
selectWeightTicketById.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);
selectWeightTicketById.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);
selectWeightTicketById.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);
selectWeightTicketById.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);
selectWeightTicketById.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
15 changes: 15 additions & 0 deletions src/store/entities/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,21 @@ export function selectMTOShipmentById(state, id) {
}

/** PPMs */
export const selectWeightTicketById = (state, mtoShipmentId, weightTicketId) => {
if (weightTicketId == null) {
return null;
}

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

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