Peardian
commented
5 years ago Thank you for reporting this. The description of the code seems to be a tad inaccurate, but the bug you found is real. I probably neglected to set the shipment ID option when filling the shipment object because I couldn't (and still can't) think of any use cases in which this would be needed. Regardless, the option could still be set for the sake of convenience.
The getShipment() method (like all get methods in this library) only retrieves data that has already been stored to the object by a previous API request, which in this case would be createShipment(). Upon creating the shipment, Amazon provides full information for the shipment, which this library then stores in a shipment object. The only two available actions from that point are to fetch the data again (the same data already loaded) or to cancel the shipment which was just created, which is why having the shipment ID option isn't very useful.
Hopefully that description helps you with using the class, and thanks again.
mehgcap
Hello, I've found what might be a bug, or might just be something I am not fully understanding. I'm working with the AmazonMerchantShipment and AmazonMerchantShipmentCreator classes, trying to get a ship response (including label) from Amazon. I can set everything in the Creator instance, but when I try to call AmazonMerchantShipmentCreator::getShipment(), an error is logged that "Shipment ID must be set in order to fetch it!"
After some poking around the two classes, here's my tentative bug report.
When AmazonMerchantShipmentCreator::getShipment() is called, it gets an XML response which it passes to the class's parseXML function. In that function, the constructor of AmazonMerchantShipment gets the XML as AmazonMerchantShipmentCreator makes a new AmazonMerchantShipment instance. In that constructor, AmazonMerchantShipment calls its own parseXML function, which assigns things from the given XML to the class's $data array. This includes setting $data["ShipmentId"] to the shipment ID in the xml. In other words, when AmazonMerchantShipmentCreator::getShipment() is called, it basically gives XML to AmazonMerchantShipment's constructor, which sets AmazonMerchantShipment::data to the values it needs from that XML. Among these values is ShipmentId.
However, in AmazonMerchantShipment::fetchShipment(), a check is performed to see whether AmazonMerchantShipment::options has a key called ShipmentId. Finally we arrive at the bug: AmazonMerchantShipment::parseXML only sets the ShipmentId in the data array, not the options array. Thus, the check for that ID in the options array always fails, even though the XML that made the instance had the ID set.
Put simply, when given valid XML from AmazonMerchantShipmentCreator, AmazonMerchantShipment sets its data array with ShipmentID, but never sets its options array with that same ID. Then, when it tries to use the ID, it checks in its options array--the array that never got the ID placed in it. It seems like AmazonMerchantShipment::parseXML should set both arrays with a value for ShipmentID to fix this problem. That, or every check should inspect both options and data.
Again, I may be mistaken here. I'm not an expert, and I haven't gone through the entire library. Still, when I call AmazonMerchantShipment::fetchShipment() with the stock code, I get the warning that the shipment ID isn't set. If I add my fix, the warning goes away. Of course, I don't have a label yet, so I might be completely off base here. :) Still, I thought I would bring this up in case it really is a bug. Thanks for reading.