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

Fixed bug in Mage_Usa_Model_Shipping_Carrier_Ups->_doShipmentRequestRest() #4046

Merged

Conversation

ragnese
Copy link
Contributor

@ragnese ragnese commented Jun 19, 2024

Description (*)

The _doShipmentRequest method has a mistake in parsing the JSON response from UPS. Each successful response has a ShipmentResponse object under the root object.

Related Pull Requests

N/A

Fixed Issues (if relevant)

Should I make an issue first for this kind of thing?

Manual testing scenarios (*)

Do anything that requires getting shipping labels from UPS

Questions or comments

The Magento2 code for UPS has this right. When I implemented the OpenMage version, my brain just skipped over the ShipmentResponse key- probably because it looks so similar to the next key, "ShipmentResults". Here's the Magento2 part for reference:

            $responseShipment = json_decode($response, true);
            $result = new DataObject();
            $shippingLabelContent =
                (string)$responseShipment['ShipmentResponse']['ShipmentResults']['PackageResults']['ShippingLabel']
                ['GraphicImage'];
            $trackingNumber =
                (string)$responseShipment['ShipmentResponse']['ShipmentResults']['PackageResults']['TrackingNumber'];
            // phpcs:ignore Magento2.Functions.DiscouragedFunction
            $result->setLabelContent(base64_decode($shippingLabelContent));
            $result->setTrackingNumber($trackingNumber)

We've been running this in production after encountering errors with generating shipping labels.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added the Component: Usa Relates to Mage_Usa label Jun 19, 2024
Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

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

confirmed

@fballiano fballiano changed the title Fix UPS REST shipping request Fixed bug in Mage_Usa_Model_Shipping_Carrier_Ups->_doShipmentRequestRest() Jun 26, 2024
@fballiano fballiano merged commit 878cf7c into OpenMage:main Jun 26, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Usa Relates to Mage_Usa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants