-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Commercial destination should not be treated as residential #40307
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
base: 2.4-develop
Are you sure you want to change the base?
Conversation
…y value for the field ResidentialAddressIndicator is not correct
|
Hi @dmytrokaplin. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
lbajsarowicz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Great, simple change.
Could you add Integration Test to cover both variants of this scenario?
|
@magento create issue |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
Description (*)
The UPS REST rate request builder in
\Magento\Ups\Model\Carrieralways includes theResidentialAddressIndicatorfield in theShipTo.Addresssection, even when the destination type is commercial (49_residential = "02").According to UPS API behavior, the mere presence of the
ResidentialAddressIndicatortag (even with an empty value) makes the shipment be treated as residential, which results in “Demand Surcharge - Residential” being applied and higher rates being returned for commercial destinations.This pull request updates the REST request payload builder so that:
ResidentialAddressIndicatoris only included when49_residential === "01"(residential)."02"commercial) requests, theResidentialAddressIndicatorfield is not added to the payload at all.This aligns the REST implementation with the existing XML implementation, which already adds
ResidentialAddressIndicatorconditionally based on49_residential.Manual testing scenarios (*)
Scenario 1: Commercial destination should not be treated as residential
49_residential = "02").ShipTo.Address.ResidentialAddressIndicatoris present with an empty value.ShipTo.Addressdoes not contain theResidentialAddressIndicatorfield for49_residential = "02".Scenario 2: Residential destination still behaves as residential
49_residential = "01").ShipTo.Address.ResidentialAddressIndicatoris present in the UPS REST request.Resolved issues: