B-19178 MTO shipment risk of excess banner persistence solution #12608
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bundle StatsHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded
Removed
Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
landan-parker-0
added
G-Unit
landan-parker-0
added
MAIN
and removed
INTEGRATION
pambecker
reviewed
May 15, 2024
| expect(mockNavigate).toHaveBeenCalled(); | ||
| }, 10000); | ||
|
|
||
| it('submits the form and tests for unsupported state validation', async () => { |
There was a problem hiding this comment.
I noticed this file had more changes than your INT PR. seems this test is duplicated in this file. maybe a bad merge fix
removed click of duplicate code
pambecker
reviewed
May 15, 2024
|
|
||
| await userEvent.type(getByTestId('cac-user-no'), fakePayload.cac_user); |
There was a problem hiding this comment.
did you mean to delete this line?
|
|
||
| await userEvent.type(getByTestId('create-okta-account-yes'), fakePayload.create_okta_account); |
There was a problem hiding this comment.
Maria updated this line (188) and line 190 in a recent merge to main, be sure you aren't breaking any logic. I think we need to keep these lines.
…, the remote main branch, not the local one, sorry
|
If you are going to revert your changes for CreateCustomerForm.test.jsx, you will need to create a new PR to push that back into integrationTesting. |
pambecker
approved these changes
May 16, 2024
loganwc
approved these changes
May 20, 2024
paulstonebraker
requested changes
May 23, 2024
| } else { | ||
| shouldSaveMoveRecord = false | ||
| } | ||
| if shouldSaveMoveRecord { |
There was a problem hiding this comment.
Were there any logic changes or was this refactored for some reason? I don't see any changes
|
|
||
| useEffect(() => { | ||
| setIsAtExcessWeightRisk(hasRiskOfExcess(estimatedWeightTotal, order?.entitlement?.authorizedWeight)); | ||
| }, [estimatedWeightTotal, order?.entitlement?.authorizedWeight]); |
There was a problem hiding this comment.
I thought this was accurate (the excess weight flag directs your to look review the max billable weight), but the way it was originally is accurate. We should be checking against entitlement.totalWeight.
A move is considered having excess weight if the total move weight exceeds the customer's weight allowance. A move is considered having risk of excess weight if the estimated weight total is within 90% of the customer’s weight allowance.
| @@ -118,6 +123,20 @@ const MoveDetails = ({ | |||
| setAlertType('error'); | |||
| }, | |||
| }); | |||
| useEffect(() => { | |||
| setIsAtExcessWeightRisk(hasRiskOfExcess(estimatedWeightTotal, order?.entitlement?.authorizedWeight)); | |||
| }, [estimatedWeightTotal, order?.entitlement?.authorizedWeight]); | |||
There was a problem hiding this comment.
Same comment as I left in MoveTaskOrder.jsx. I think we need to clear up with PO how we want to handle this
| @@ -136,8 +138,9 @@ export const MoveTaskOrder = (props) => { | |||
|
|
|||
| const { orders = {}, move, mtoShipments, mtoServiceItems, isLoading, isError } = useMoveTaskOrderQueries(moveCode); | |||
| const order = Object.values(orders)?.[0]; | |||
| const nonPPMShipments = mtoShipments?.filter((shipment) => shipment.shipmentType !== 'PPM'); | |||
There was a problem hiding this comment.
not going to die on this hill but I think this is easier to read
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Agility ticket
Summary
Is there anything you would like reviewers to give additional scrutiny?
Continuation of:
#12499
Screenshots