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

INT B-22169 #14534

Merged
merged 13 commits into from
Jan 13, 2025
Merged

INT B-22169 #14534

merged 13 commits into from
Jan 13, 2025

Conversation

danieljordan-caci
Copy link
Contributor

@danieljordan-caci danieljordan-caci commented Jan 7, 2025

Agility ticket

Summary

We need to consider OCONUS rate area changed when requesting shipment destination address requests. This is a different process than CONUS because we use different sources of truth (re_oconus_rate_areas).

I also noticed that there was some move history blanks and am addressing those in this PR.

This PR does the following:

  • added helper fetch functions to CONUS & OCONUS models
  • since serviceID can be null when getting rate areas with the db func, adjusted that
  • implemented international checks to shipment_address_update_requester.go that checks for rate area changes for OCONUS/CONUS addresses
  • updated AddressUpdatePreview.jsx with international blurbs to be more accurate in informing the TOO when reviewing destination requests for international shipments
  • ensuring that the new address gets populated for the ShipmentForm when the TOO approves the request, previously there was an issue with it not updating if the radio button was checked to No
  • added move history objects for reviewing address updates
  • added move history objects for displaying estimated pricing since those get updated with these changes

How to test

  1. Create a move with an international iHHG shipment
  2. Approve as SC & TOO
  3. Request an update to the destination address as Prime
  4. Should be successful
  5. You can toy around with different ZIPs, but TOO should have to approve if the differences in ZIPs are 50 miles apart
  6. Confirm you can see the difference between modal verbiage for domestic/international shipments
  7. Confirm you see the destination address updated on the edit shipment form after you approve the dest request
  8. If you update the street addresses, but not the ZIPs, it should successfully update the shipment destination address without approval

Screenshots

Screenshot 2025-01-08 at 4 16 22 PM
Screenshot 2025-01-08 at 4 18 06 PM

@danieljordan-caci danieljordan-caci added Mountain Movers Movin' Mountains 1 Sprint at a time INTEGRATION Slated for Integration Testing labels Jan 7, 2025
@danieljordan-caci danieljordan-caci self-assigned this Jan 7, 2025
@danieljordan-caci danieljordan-caci changed the title initial commit, workin on it INT B-22169 Jan 7, 2025
@danieljordan-caci danieljordan-caci marked this pull request as ready for review January 8, 2025 22:19
@danieljordan-caci danieljordan-caci requested review from a team as code owners January 8, 2025 22:19
@danieljordan-caci danieljordan-caci mentioned this pull request Jan 8, 2025
traskowskycaci
traskowskycaci previously approved these changes Jan 9, 2025
Copy link
Contributor

@traskowskycaci traskowskycaci left a comment

Choose a reason for hiding this comment

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

Tested and works as described, seeing the expected different blurbs for a domestic vs international address change request

This BL should also fix the issue described here, added a comment noting that

Copy link
Contributor

@cameroncaci cameroncaci left a comment

Choose a reason for hiding this comment

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

Functional, meets AC. Identified bugs but are not relevant to this backlog item.

First bug: notification counter not updating properly
https://github.com/user-attachments/assets/4d6d0676-43eb-42cb-a78e-d0804de61f28

Second bug: prime can create oconus addresses without a USPRC FK and thus no rate area. This leads to this backlog item encountering a breaking error

Screenshot 2025-01-09 at 12 55 36 PM

2025-01-09T17:54:57.127Z	ERROR	ghcapi/mto_shipment.go:1036	ghcapi.ReviewShipmentAddressUpdateHandler	{"git_branch": "INT-B-22169", "git_commit": "7055ac68700ff7970eb5031dcb4accc2648f5e15", "host": "officelocal:3000", "milmove_trace_id": "b92313d1-0135-4f78-893d-fe81190c587a", "session_id": "tHx8rNupAZa8eDdC1xx34ahWyDLecMG7LPqPkGBX7dI", "error": "error updating estimated pricing for shipment's service items: pq: Rate area not found for address 387ade30-7eae-4495-9a6c-e16aec6f8f4b for service item ID 56e91c2d-015d-4243-9657-3ed34867abaa"}
github.com/transcom/mymove/pkg/handlers/ghcapi.(*ReviewShipmentAddressUpdateHandler).Handle.ReviewShipmentAddressUpdateHandler.Handle.func1.1
	/Users/cameron.jewell_cn/Projects/mymove/pkg/handlers/ghcapi/mto_shipment.go:1036
github.com/transcom/mymove/pkg/handlers/ghcapi.(*ReviewShipmentAddressUpdateHandler).Handle.ReviewShipmentAddressUpdateHandler.Handle.func1
	/Users/cameron.jewell_cn/Projects/mymove/pkg/handlers/ghcapi/mto_shipment.go:1062
github.com/transcom/mymove/pkg/handlers.(*Config).AuditableAppContextFromRequestWithErrors.func1
	/Users/cameron.jewell_cn/Projects/mymove/pkg/handlers/config.go:152

Coming from payload

{
  "contractorRemarks": "New address",
  "newAddress": {
    "streetAddress1": "Storage Facility",
    "city": "Allakaket",
    "state": "AK",
    "postalCode": "99720",
    "country": "US",
    "county": "Yukon-Koyukuk",
    "isOconus": true,
    "destinationGbloc": "NVBJ"
  }
}

on endpoint prime/v1/mto-shipments/:mtoShipmentID/hipment-address-updates

This falls under E-06198

@cameroncaci cameroncaci self-requested a review January 9, 2025 18:23
Copy link
Contributor

@cameroncaci cameroncaci left a comment

Choose a reason for hiding this comment

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

Awaiting feedback from scrum master before approving
https://caci-bits.slack.com/archives/C060PJCD1BQ/p1736447118261759

cameroncaci
cameroncaci previously approved these changes Jan 9, 2025
Copy link
Contributor

@antgmann antgmann left a comment

Choose a reason for hiding this comment

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

Small issue regarding something missing from storybook. About to run testing instructions, if they go well and this is resolved then I'll be good to approve.

Copy link
Contributor

@antgmann antgmann left a comment

Choose a reason for hiding this comment

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

I might have another issue, though I'm not sure if it was introduced here or not. When I denied a destination address change request as the TOO, it showed in the H&AL as "Status: Approved" with no address update.

Edit: For reference, both of the below entries were denied destination address update requests.
image

"added move history objects for reviewing address updates" - After checking on main, these were introduced here so this issue is definitely coming from this, unfortunately

Copy link
Contributor

@antgmann antgmann left a comment

Choose a reason for hiding this comment

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

Denied destination address requests are showing as Status: Approved in the H&AL just without an address, see above comment
image

@traskowskycaci traskowskycaci dismissed stale reviews from cameroncaci and themself via edcd335 January 10, 2025 19:18
@traskowskycaci traskowskycaci requested a review from a team as a code owner January 10, 2025 19:18
@antgmann
Copy link
Contributor

Happo diffs accepted

@antgmann
Copy link
Contributor

Issue 2 is also fixed:
image

antgmann
antgmann previously approved these changes Jan 13, 2025
Copy link
Contributor

@antgmann antgmann left a comment

Choose a reason for hiding this comment

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

Everything looks good now! There might be some blank history logs being printed for us to be wary of, but I think that has been an unrelated ongoing issue for a while. Thanks Daniel for the work and Maria for the fixes! image

cameroncaci
cameroncaci previously approved these changes Jan 13, 2025
@danieljordan-caci
Copy link
Contributor Author

Re-requesting review from @antgmann, @cameroncaci, and @brianmanley-caci following small code change regarding reusable variables

Copy link
Contributor

@brianmanley-caci brianmanley-caci left a comment

Choose a reason for hiding this comment

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

Tried a few different scenarios (intl, domestic, same zip) and everything worked as expected.

Copy link
Contributor

@antgmann antgmann left a comment

Choose a reason for hiding this comment

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

Repeating approval comment above image

@danieljordan-caci
Copy link
Contributor Author

@antgmann can I get a Happo check?

@danieljordan-caci
Copy link
Contributor Author

@antgmann beat me to it nvm haha

@danieljordan-caci danieljordan-caci merged commit 1777a3b into integrationTesting Jan 13, 2025
31 of 34 checks passed
@danieljordan-caci danieljordan-caci deleted the INT-B-22169 branch January 13, 2025 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTEGRATION Slated for Integration Testing Mountain Movers Movin' Mountains 1 Sprint at a time
Development

Successfully merging this pull request may close these issues.

5 participants