OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
865 stars 436 forks source link

Fixed bug in Mage_Usa_Model_Shipping_Carrier_Ups->_doShipmentRequestRest() #4046

Closed ragnese closed 3 months ago

ragnese commented 3 months ago

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 (*)