B 21904 nts oconus int #14384

KonstanceH · 2024-12-12T15:41:04Z

B-21904

Summary

This PR adds in the pre approved service items for international NTS shipments.

Verification Steps for Reviewers

These are to be checked by a reviewer.

Has the branch been pulled in and checked out?
Have the BL acceptance criteria been met for this change?
Was the CircleCI build successful?
Has the code been reviewed from a standards and best practices point of view?

How to test

Login as a TOO
Add an international NTS shipment to a move. Test the variations of the move:
OCONUS -> OCONUS
OCONUS -> CONUS... etc
Click on Approve and send.
Confirm on the next pageInternational Shipping and Linehaul and International Packing with NTS Pricing Factor are preapproved service items. Fuel Surcharge - PODFSC should only be pre approved if the shipment is OCONUS -> CONUS

Note: OCONUS -> CONUS etc are determined by the shipments pickupaddress and storage facility address.
Example: NTS Shipment
Pickup Address -> Storage Facility Address == Ordering Type
Alaska -> Alaska == OCONUS -> OCONUS
Iowa -> Alaska == CONUS -> OCONUS

Backend

Code follows the guidelines for Logging.
The requirements listed in Querying the Database Safely have been satisfied.

Database

Any new migrations/schema changes:

Follows our guidelines for Zero-Downtime Deploys.
Have been communicated to #g-database.
Secure migrations have been tested following the instructions in our docs.

Screenshots

OCONUS -> CONUS

OCONUS -> OCONUS

taeJungCaci

Just 1 minor thing to check - everything else looks good to me.

migrations/app/schema/20241208080706_add_internatinoal_nts_re_service_items.up.sql

taeJungCaci

LGTM!

JamesHawks224

testing: success
code: lgtm

begrohmann · 2024-12-16T16:13:51Z

migrations/app/schema/20241208080706_add_internatinoal_nts_re_service_items.up.sql

+INSERT INTO re_service_items
+(id, service_id, shipment_type, market_code, is_auto_approved, created_at, updated_at, sort)
+VALUES
+    ('2a560507-db09-4be1-b809-49c0f515b31b', '9f3d551a-0725-430e-897e-80ee9add3ae9' ,'HHG_INTO_NTS_DOMESTIC', 'i', true, now(), now(), 1),


The shipment type should be updated to HHG_INTO_NTS (remove DOMESTIC). I see we have B-21905 to update this so I think that BL should be an upstream to this one.

Yea I saw that after I was already knee deep in this story so I figured since its just adding these records, it'd be okay to do the name change after. Tae's working the other story that's handling that name change and we had already talked about it. BUT I don't mind putting this on hold until his story gets in and then I can make the name change on this pr either.

I should be almost finished with name change story. Just need to test if anything is broken lol. I'd be okay with you holding this story until mine gets in. Then I don't have to do rename changes for your DB stuff.

okay sounds like a plan to me. I have to wait for a response from the gov anyway. So I'll hold this until yours gets in.

The changes for renaming NTS is in IntegrationBranch now and you should be able to merge in MAIN-B-21905-rename-nts-shipment-type to your Story Main branch and then make the name changes to your files -> then merge those changes here. This is the Main PR for ref.

Added in the name change and the migration file from my main branch so this should be good to go for another review.

migrations/app/schema/20241208080706_add_internatinoal_nts_re_service_items.up.sql

pkg/services/mto_shipment/shipment_approver.go

pkg/services/mto_shipment/shipment_approver_test.go

KonstanceH · 2025-01-08T17:52:07Z

@KonstanceH adding this nil check fixed it, but should we consider shipment type in that check?
		if newShipment.MarketCode == models.MarketCodeInternational &&
			newShipment.PrimeEstimatedWeight != nil ||
			newShipment.PickupAddress != nil && dbShipment.PickupAddress != nil && newShipment.PickupAddress.PostalCode != dbShipment.PickupAddress.PostalCode ||
			newShipment.DestinationAddress != nil && dbShipment.DestinationAddress != nil && newShipment.DestinationAddress.PostalCode != dbShipment.DestinationAddress.PostalCode ||
			newShipment.RequestedPickupDate != nil && newShipment.RequestedPickupDate.Format("2006-01-02") != dbShipment.RequestedPickupDate.Format("2006-01-02") {
Yea I think there should be a shipment type check for NTS shipments because they're the only ones that there's a conversion from storage address to destination address that happens in the db. But that makes me think doesn't that happen right after the NTS is created?
Doesn't the TOO input the storage address if the customer created the NTS?

I don't think so? or at least I thought the customer could add the storage address from their creation of the nts.

danieljordan-caci · 2025-01-08T17:52:09Z

I also have a question - when we have a domestic NTS shipment, but it has an OCONUS storage facility... Should that still have domestic service items or the international ones?

Trying to understand this. If it has an OCONUS storage facility, it wouldn't be a domestic NTS right?

Yeah we might have an upstream problem here because, in my head, if either the pickup/destination (in this case, pickup/storage) is OCONUS, the shipment needs to be international

brianmanley-caci · 2025-01-08T17:53:02Z

Yeah we might have an upstream problem here because, in my head, if either the pickup/destination (in this case, pickup/storage) is OCONUS, the shipment needs to be international

Yes, I would agree with that.

danieljordan-caci · 2025-01-08T17:54:17Z

@KonstanceH adding this nil check fixed it, but should we consider shipment type in that check?
		if newShipment.MarketCode == models.MarketCodeInternational &&
			newShipment.PrimeEstimatedWeight != nil ||
			newShipment.PickupAddress != nil && dbShipment.PickupAddress != nil && newShipment.PickupAddress.PostalCode != dbShipment.PickupAddress.PostalCode ||
			newShipment.DestinationAddress != nil && dbShipment.DestinationAddress != nil && newShipment.DestinationAddress.PostalCode != dbShipment.DestinationAddress.PostalCode ||
			newShipment.RequestedPickupDate != nil && newShipment.RequestedPickupDate.Format("2006-01-02") != dbShipment.RequestedPickupDate.Format("2006-01-02") {
Yea I think there should be a shipment type check for NTS shipments because they're the only ones that there's a conversion from storage address to destination address that happens in the db. But that makes me think doesn't that happen right after the NTS is created?
Doesn't the TOO input the storage address if the customer created the NTS?
I don't think so? or at least I thought the customer could add the storage address from their creation of the nts.

Customer only provides the pickup address for NTS

KonstanceH · 2025-01-08T17:55:28Z

I also have a question - when we have a domestic NTS shipment, but it has an OCONUS storage facility... Should that still have domestic service items or the international ones?

They should have international ones. I must've messed something up on the creation I'm looking into now

I believe this is the same issue I raised @KonstanceH where the NTS is registering as dNTS instead of iNTS when the customer creates NTS and the TOO inputs the AK storage facility address.

OOH okay. Well then yea in that case we were just talking about this earlier. Its a bug. And its a special case for ONLY this case on editing dNTS shipments for the first time the shipment market code doesn't flip to international when the addresses are changed to OCONUS. I was waiting on direction from our scrum master to see how to go about getting the process started on reporting the bug.

danieljordan-caci · 2025-01-08T17:56:33Z

I also have a question - when we have a domestic NTS shipment, but it has an OCONUS storage facility... Should that still have domestic service items or the international ones?

They should have international ones. I must've messed something up on the creation I'm looking into now

I believe this is the same issue I raised @KonstanceH where the NTS is registering as dNTS instead of iNTS when the customer creates NTS and the TOO inputs the AK storage facility address.

OOH okay. Well then yea in that case we were just talking about this earlier. Its a bug. And its a special case for ONLY this case on editing dNTS shipments for the first time the shipment market code doesn't flip to international when the addresses are changed to OCONUS. I was waiting on direction from our scrum master to see how to go about getting the process started on reporting the bug.

Wouldn't that be a blocker for this work?

KonstanceH · 2025-01-08T18:01:04Z

I also have a question - when we have a domestic NTS shipment, but it has an OCONUS storage facility... Should that still have domestic service items or the international ones?

They should have international ones. I must've messed something up on the creation I'm looking into now

I believe this is the same issue I raised @KonstanceH where the NTS is registering as dNTS instead of iNTS when the customer creates NTS and the TOO inputs the AK storage facility address.

OOH okay. Well then yea in that case we were just talking about this earlier. Its a bug. And its a special case for ONLY this case on editing dNTS shipments for the first time the shipment market code doesn't flip to international when the addresses are changed to OCONUS. I was waiting on direction from our scrum master to see how to go about getting the process started on reporting the bug.

Wouldn't that be a blocker for this work?

I don't think so because the bug exists now in milmove. The code I'm adding works for this features AC. I think that bug has more to do with updating the shipment in general and that would effect more than the service items. But that's just my lowly dev thoughts lol I think this is a question for the government?

taeJungCaci · 2025-01-08T18:04:33Z

I also have a question - when we have a domestic NTS shipment, but it has an OCONUS storage facility... Should that still have domestic service items or the international ones?

They should have international ones. I must've messed something up on the creation I'm looking into now

I believe this is the same issue I raised @KonstanceH where the NTS is registering as dNTS instead of iNTS when the customer creates NTS and the TOO inputs the AK storage facility address.

OOH okay. Well then yea in that case we were just talking about this earlier. Its a bug. And its a special case for ONLY this case on editing dNTS shipments for the first time the shipment market code doesn't flip to international when the addresses are changed to OCONUS. I was waiting on direction from our scrum master to see how to go about getting the process started on reporting the bug.

Wouldn't that be a blocker for this work?

A workaround - If you "edit shipment" for the 2nd time and then save with AK address - it switches to iNTS

danieljordan-caci · 2025-01-08T18:05:49Z

I also have a question - when we have a domestic NTS shipment, but it has an OCONUS storage facility... Should that still have domestic service items or the international ones?

They should have international ones. I must've messed something up on the creation I'm looking into now

I believe this is the same issue I raised @KonstanceH where the NTS is registering as dNTS instead of iNTS when the customer creates NTS and the TOO inputs the AK storage facility address.

OOH okay. Well then yea in that case we were just talking about this earlier. Its a bug. And its a special case for ONLY this case on editing dNTS shipments for the first time the shipment market code doesn't flip to international when the addresses are changed to OCONUS. I was waiting on direction from our scrum master to see how to go about getting the process started on reporting the bug.

Wouldn't that be a blocker for this work?

A workaround - If you "edit shipment" for the 2nd time and then save with AK address - it switches to iNTS

Is there an issue created for this bug? @KonstanceH @taeJungCaci

KonstanceH · 2025-01-08T18:07:16Z

I also have a question - when we have a domestic NTS shipment, but it has an OCONUS storage facility... Should that still have domestic service items or the international ones?

They should have international ones. I must've messed something up on the creation I'm looking into now

I believe this is the same issue I raised @KonstanceH where the NTS is registering as dNTS instead of iNTS when the customer creates NTS and the TOO inputs the AK storage facility address.

OOH okay. Well then yea in that case we were just talking about this earlier. Its a bug. And its a special case for ONLY this case on editing dNTS shipments for the first time the shipment market code doesn't flip to international when the addresses are changed to OCONUS. I was waiting on direction from our scrum master to see how to go about getting the process started on reporting the bug.

Wouldn't that be a blocker for this work?

A workaround - If you "edit shipment" for the 2nd time and then save with AK address - it switches to iNTS

Yea this^^ also they could create a new shipment for the marketCode change and delete the old one

KonstanceH · 2025-01-08T18:09:22Z

I also have a question - when we have a domestic NTS shipment, but it has an OCONUS storage facility... Should that still have domestic service items or the international ones?

They should have international ones. I must've messed something up on the creation I'm looking into now

I believe this is the same issue I raised @KonstanceH where the NTS is registering as dNTS instead of iNTS when the customer creates NTS and the TOO inputs the AK storage facility address.

OOH okay. Well then yea in that case we were just talking about this earlier. Its a bug. And its a special case for ONLY this case on editing dNTS shipments for the first time the shipment market code doesn't flip to international when the addresses are changed to OCONUS. I was waiting on direction from our scrum master to see how to go about getting the process started on reporting the bug.

Wouldn't that be a blocker for this work?

A workaround - If you "edit shipment" for the 2nd time and then save with AK address - it switches to iNTS

Is there an issue created for this bug? @KonstanceH @taeJungCaci

Yea @danieljordan-caci that was the part about waiting to hear back on how to report it lol
For bug issues can we just create them ourselves/follow the issue template and go on about our day? or is there more to it?
Because if I can just do it myself I'll make one now

danieljordan-caci · 2025-01-08T18:13:49Z

Is there an issue created for this bug? @KonstanceH @taeJungCaci

Yea @danieljordan-caci that was the part about waiting to hear back on how to report it lol For bug issues can we just create them ourselves/follow the issue template and go on about our day? or is there more to it? Because if I can just do it myself I'll make one now

Oh yeah I create issues all the time! Ideal process is to create the issue, provide analysis & fix, testers confirm, get PO to create a BL from the issue -> fix the issue

KonstanceH · 2025-01-08T18:16:12Z

Is there an issue created for this bug? @KonstanceH @taeJungCaci

Yea @danieljordan-caci that was the part about waiting to hear back on how to report it lol For bug issues can we just create them ourselves/follow the issue template and go on about our day? or is there more to it? Because if I can just do it myself I'll make one now

Oh yeah I create issues all the time! Ideal process is to create the issue, provide analysis & fix, testers confirm, get PO to create a BL from the issue -> fix the issue

oh nice! I'll get on it now and tag the issue in this pr

danieljordan-caci · 2025-01-08T18:23:44Z

Is there an issue created for this bug? @KonstanceH @taeJungCaci

Yea @danieljordan-caci that was the part about waiting to hear back on how to report it lol For bug issues can we just create them ourselves/follow the issue template and go on about our day? or is there more to it? Because if I can just do it myself I'll make one now

Oh yeah I create issues all the time! Ideal process is to create the issue, provide analysis & fix, testers confirm, get PO to create a BL from the issue -> fix the issue

oh nice! I'll get on it now and tag the issue in this pr

Cool cool. For now if you can get a fix in for that nil check, rest looks good.

KonstanceH · 2025-01-08T18:44:22Z

Is there an issue created for this bug? @KonstanceH @taeJungCaci

Yea @danieljordan-caci that was the part about waiting to hear back on how to report it lol For bug issues can we just create them ourselves/follow the issue template and go on about our day? or is there more to it? Because if I can just do it myself I'll make one now

Oh yeah I create issues all the time! Ideal process is to create the issue, provide analysis & fix, testers confirm, get PO to create a BL from the issue -> fix the issue

oh nice! I'll get on it now and tag the issue in this pr

Cool cool. For now if you can get a fix in for that nil check, rest looks good.

Okay cool the issue is created adding it here for reference:
https://www13.v1host.com/USTRANSCOM38/Issue.mvc/Summary?oidToken=Issue%3A1050704

danieljordan-caci · 2025-01-08T21:19:44Z

pkg/services/mto_shipment/mto_shipment_updater.go

-		if newShipment.MarketCode == models.MarketCodeInternational &&
-			(newShipment.PrimeEstimatedWeight != nil ||
+		if dbShipment.PickupAddress != nil && dbShipment.DestinationAddress != nil &&
+			newShipment.ShipmentType == models.MTOShipmentTypeHHGIntoNTS || newShipment.ShipmentType == models.MTOShipmentTypeHHGOutOfNTSDom {


This specific function is addressing updating estimated pricing for this shipment type. Has the upstream db functions/procs been updated to accommodate international NTS & NTSr shipments?

You also are eliminating this check for iHHG moves, which is what it was originally created for.

Okay this should be good now. I'll see about these failing circleCI tests tomorrow @_@

KonstanceH added 5 commits December 11, 2024 20:32

merged in latest int, added conflict error, adding tests

4bcf62e

removing frontend for now

0427ed6

syntax fixes and working on tests

c828828

removing added error logic and its handling

89ef13e

testing

b3995e6

KonstanceH self-assigned this Dec 12, 2024

KonstanceH added ByteSize M&Ms Team ByteSized M&Ms INTEGRATION Slated for Integration Testing labels Dec 12, 2024

KonstanceH and others added 5 commits December 12, 2024 09:42

Merge branch 'integrationTesting' into B-21904-NTS-OCONUS-INT

a143a1e

testing

6ad5618

adding invalid check for more coverage

cd85f86

Merge branch 'integrationTesting' into B-21904-NTS-OCONUS-INT

e3998b3

Merge branch 'integrationTesting' into B-21904-NTS-OCONUS-INT

e24834f

KonstanceH requested review from danieljordan-caci, taeJungCaci and JamesHawks224 December 12, 2024 21:43

KonstanceH marked this pull request as ready for review December 12, 2024 21:43

KonstanceH requested review from a team as code owners December 12, 2024 21:43

taeJungCaci reviewed Dec 13, 2024

View reviewed changes

migrations/app/schema/20241208080706_add_internatinoal_nts_re_service_items.up.sql Outdated Show resolved Hide resolved

fix ordering

76845c0

taeJungCaci previously approved these changes Dec 13, 2024

View reviewed changes

Merge branch 'integrationTesting' into B-21904-NTS-OCONUS-INT

aab9454

JamesHawks224 previously approved these changes Dec 13, 2024

View reviewed changes

begrohmann reviewed Dec 16, 2024

View reviewed changes

danieljordan-caci reviewed Dec 16, 2024

View reviewed changes

migrations/app/schema/20241208080706_add_internatinoal_nts_re_service_items.up.sql Outdated Show resolved Hide resolved

danieljordan-caci requested changes Dec 16, 2024

View reviewed changes

pkg/services/mto_shipment/shipment_approver.go Outdated Show resolved Hide resolved

pkg/services/mto_shipment/shipment_approver_test.go Outdated Show resolved Hide resolved

pkg/services/mto_shipment/shipment_approver_test.go Outdated Show resolved Hide resolved

KonstanceH added 3 commits December 16, 2024 18:47

working comments

70387dd

Merge branch 'integrationTesting' into B-21904-NTS-OCONUS-INT

8939bec

merge in latest integration and fix conflicts

2cea7b1

KonstanceH closed this Jan 8, 2025

KonstanceH reopened this Jan 8, 2025

KonstanceH and others added 3 commits January 8, 2025 21:02

add in nil/type check

a1c8dad

Merge branch 'integrationTesting' into B-21904-NTS-OCONUS-INT

73103dc

generate swagger

27d0535

KonstanceH requested a review from danieljordan-caci January 8, 2025 21:15

danieljordan-caci requested changes Jan 8, 2025

View reviewed changes

added nts/nil checks

212af35

KonstanceH requested a review from danieljordan-caci January 8, 2025 22:41

KonstanceH and others added 6 commits January 9, 2025 08:12

Merge branch 'integrationTesting' into B-21904-NTS-OCONUS-INT

802c6c5

Merge branch 'integrationTesting' into B-21904-NTS-OCONUS-INT

35c16af

remove breaking type change

4ee0dbe

trying to find why tests are brekaing?

46a4f80

changing file completely back

214c4ff

this is weird

ba7dc65

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

B 21904 nts oconus int #14384

B 21904 nts oconus int #14384

KonstanceH commented Dec 12, 2024 •

edited

Loading

taeJungCaci left a comment

taeJungCaci left a comment

JamesHawks224 left a comment

begrohmann Dec 16, 2024

KonstanceH Dec 16, 2024

taeJungCaci Dec 16, 2024

KonstanceH Dec 16, 2024 •

edited

Loading

taeJungCaci Dec 18, 2024 •

edited

Loading

KonstanceH Dec 19, 2024

KonstanceH commented Jan 8, 2025

danieljordan-caci commented Jan 8, 2025

brianmanley-caci commented Jan 8, 2025

danieljordan-caci commented Jan 8, 2025

KonstanceH commented Jan 8, 2025 •

edited by taeJungCaci

Loading

danieljordan-caci commented Jan 8, 2025 •

edited by taeJungCaci

Loading

KonstanceH commented Jan 8, 2025 •

edited by taeJungCaci

Loading

taeJungCaci commented Jan 8, 2025 •

edited

Loading

danieljordan-caci commented Jan 8, 2025 •

edited by taeJungCaci

Loading

KonstanceH commented Jan 8, 2025 •

edited by taeJungCaci

Loading

KonstanceH commented Jan 8, 2025 •

edited by taeJungCaci

Loading

danieljordan-caci commented Jan 8, 2025

KonstanceH commented Jan 8, 2025

danieljordan-caci commented Jan 8, 2025

KonstanceH commented Jan 8, 2025 •

edited

Loading

danieljordan-caci Jan 8, 2025

danieljordan-caci Jan 8, 2025

KonstanceH Jan 8, 2025

B 21904 nts oconus int #14384

Are you sure you want to change the base?

B 21904 nts oconus int #14384

Conversation

KonstanceH commented Dec 12, 2024 • edited Loading

Summary

Verification Steps for Reviewers

How to test

Backend

Database

Any new migrations/schema changes:

Screenshots

taeJungCaci left a comment

Choose a reason for hiding this comment

taeJungCaci left a comment

Choose a reason for hiding this comment

JamesHawks224 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KonstanceH Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

taeJungCaci Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KonstanceH commented Jan 8, 2025

danieljordan-caci commented Jan 8, 2025

brianmanley-caci commented Jan 8, 2025

danieljordan-caci commented Jan 8, 2025

KonstanceH commented Jan 8, 2025 • edited by taeJungCaci Loading

danieljordan-caci commented Jan 8, 2025 • edited by taeJungCaci Loading

KonstanceH commented Jan 8, 2025 • edited by taeJungCaci Loading

taeJungCaci commented Jan 8, 2025 • edited Loading

danieljordan-caci commented Jan 8, 2025 • edited by taeJungCaci Loading

KonstanceH commented Jan 8, 2025 • edited by taeJungCaci Loading

KonstanceH commented Jan 8, 2025 • edited by taeJungCaci Loading

danieljordan-caci commented Jan 8, 2025

KonstanceH commented Jan 8, 2025

danieljordan-caci commented Jan 8, 2025

KonstanceH commented Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KonstanceH commented Dec 12, 2024 •

edited

Loading

KonstanceH Dec 16, 2024 •

edited

Loading

taeJungCaci Dec 18, 2024 •

edited

Loading

KonstanceH commented Jan 8, 2025 •

edited by taeJungCaci

Loading

danieljordan-caci commented Jan 8, 2025 •

edited by taeJungCaci

Loading

KonstanceH commented Jan 8, 2025 •

edited by taeJungCaci

Loading

taeJungCaci commented Jan 8, 2025 •

edited

Loading

danieljordan-caci commented Jan 8, 2025 •

edited by taeJungCaci

Loading

KonstanceH commented Jan 8, 2025 •

edited by taeJungCaci

Loading

KonstanceH commented Jan 8, 2025 •

edited by taeJungCaci

Loading

KonstanceH commented Jan 8, 2025 •

edited

Loading