MAIN B-21702 #14528
Merged
MAIN B-21702 #14528
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
…cing needs to be updated
… zip distance looker upper please save me lord
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
|
Merged
danieljordan-caci
requested review from
a team,
deandreJones,
TevinAdams,
cameroncaci and
traskowskycaci
TevinAdams
approved these changes
Jan 8, 2025
There was a problem hiding this comment.
Seeing some diffs between lines changed in between this PR and the INT - would there be a missing upstream or something? I don't see those lines like serviceAreaPriceDUPK := and serviceAreaPriceDPK := in main already pkg/services/mto_service_item/mto_service_item_creator_test.go
Not a missing upstream, but G-Unit has a |
|
Which I'll ping @loganwc here about since he will need to update those functions with my changes when this gets merged |
There was a problem hiding this comment.
Not seeing this block from the INT PR here, do we need to pull it in?
There was a problem hiding this comment.
This block belongs to @CoryKleinjanCACI and his B-21464
cameroncaci
reviewed
Jan 8, 2025
There was a problem hiding this comment.
Found 4 diffs, the one Maria pointed out is the most impactful
---break---
fixing headache failing tests after db truncate changes
Main
489de36
Int
38f2c58
pkg/services/mto_service_item/mto_service_item_creator_test.go has file changes in int but not main. This is just a test file change, does not impact production
---break---
Diff between 1d3144f and b64bd5f is based on function calls needing the parameter, int has more of these function calls hence the diff. I don’t like this diff, but it will not impact a production deployment. Upstream wasn’t set properly
diff <(git show --stat 1d3144f4bc39b240a3a3db95a9019d073da42fb0) <(git show --stat b64bd5f80b6119f8d388096a85191096cab3d322)
File in question mto_service_item_updater_test.go
---break---
adding test for shipment address updates on intl shipments
diff <(git show --stat f0ed4c7ec7fb725aae8582086109f0cb4997867d) <(git show --stat cacd86d2fcda4071889f8a0392ab08743e141206)
Maria pointed it out cacd86d#r1907507796
---break---
updated migration, all tests with added parameter for zip distance calculation
diff <(git show --stat 1d3144f4bc39b240a3a3db95a9019d073da42fb0) <(git show --stat b64bd5f80b6119f8d388096a85191096cab3d322)
Same thing as 1d3144f and b64bd5f, func calls and tests
traskowskycaci
approved these changes
Jan 8, 2025
There was a problem hiding this comment.
If the two files and their changes are being covered by @loganwc for pkg/services/mto_service_item/mto_service_item_creator_test.go and @CoryKleinjanCACI for pkg/services/shipment_address_update/shipment_address_update_requester.go - I am good with this and didn't have concerns for the other files
|
@CoryKleinjanCACI & @loganwc please read up & be aware of code changes that will be required in your |
cameroncaci
approved these changes
Jan 8, 2025
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.
INT PR 1
INT PR 2
Agility ticket
SECOND NOTE
Please test these carefully & thoroughly
Summary
When we order basic iHHG service items, we also want the ability to see the estimated price when we have the estimated weight on the shipment. In order to do this, the UI has to allow the estimated price section show for iHHG service items AND we have to populate the
pricing_estimatedata any time that pertinent data is provided and/or changed.This PR contains a buttload of file changes because I changed the
db-truncateto stop deleting static data we use for pricing & other additional params. This required the creation of a couple factories, and many many many changes in test data because now we are using data pulled from the db instead of data created within tests & then saved to the test db.In a nutshell, we want to calculate estimated pricing & display that estimated pricing when the following things happen:
Some stickler details we need to ensure we have:
poe_locationorpod_locationforPOEFSCorPODFSCbasic service itemsshipment.prime_estimated_weightaddresses.us_post_region_cities_idWithout the above details, pricing ain't gonna happen.
Some additional things happening in this PR with migrations:
get_zip_codefunction to not return an error, this was causing unnecessary errors when we didn't have the port location stuff yetget_rate_area_idfunction to take in thecontract_id- this is useful when there are multiple contractsget_port_location_info_for_shipmentfunction that will return the port ZIP associated with the shipment as well as if it is aPOEFSCorPODFSCget_rate_area_idas well as taking in amileagevalue that we are getting from DTOD prior to the pricing func being called - this is a business requirement and AC on the featureHow to test
iHHGshipment (don't approve yet) - do that however you wish, but please keep in mind we do not have total coverage on CONUS -> AK OCONUS ZIP codes (I have had good luck with ZIPs of major bases and the998ZIPpricing_estimatevalue on any of the service itemspricing_estimatevalue on the service items - this is because we need the mileage from/to the port and since we don't have apoe_location_idorpod_location_idon thePOEFSC/PODFSCservice itemeTagfor the fuel surcharge service item (Prime v3getMoveTaskOrder) & the service itemid- swagger UI makes it easyeTagpricing_estimatevalues on all the basic service itemsPODFSC- which areOCONUS->CONUSshipmentsghc_diesel_fuel_pricesis lower than $2.50 so a lot of the times I get a negative value for that. You could update the last row in that table to be something greater (say400,000instead of243,300and that value will be positive. You can also runmake tasks_save_ghc_fuel_price_datato try and get updated values imported into your dev db.Agility ticket
Issue agility
Previous INT PR
Summary
During testing Dan Schuster decided that he'd rather see the estimated pricing on all the iHHG basic service items even when we don't have the port data so making some adjustments here to allow that to happen.
This PR does the following:
How to test