-
Notifications
You must be signed in to change notification settings - Fork 259
TimeInTransit Service Codes, InternationalForms EEIFilingOption & AdditionalDocumentIndicator, StatusType Constants, Unit Bug Fix #108
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
Conversation
This mainly focuses on shipments originating in the US. I'm mainly focussed in my code in EU. Should we add more codes + a prefix for the origin location? Think it would benefit more people. |
Of course. I'll add some more. Just being selfish! On Wed, Jun 29, 2016 at 1:05 PM Stefan Doorn notifications@github.com
|
Haha :) No problem, any help is appreciated anyway :) |
@@ -92,7 +92,7 @@ public function setNumber($number) | |||
*/ | |||
public function setValue($value) | |||
{ | |||
$this->value = number_format($value, 6); | |||
$this->value = number_format($value, 6, '.', ''); |
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.
This change here is because the number_format() call automatically pops in a thousands comma separator. The UPS Shipping API considers any values > 999.99 to be invalid because "," isn't a valid character in the node.
Looks good. Let me know when it's ready to be merged. |
All good. Go for it. |
Alright. I can't squash them on GH it seems, can you squash them into 3 commits (services, the number_format fix, and the additional documents thing)? |
add in EU time in transit response codes fix constant naming collisions.
…ection in the bottom right corner of the shipping label. This node is not present by default.
I went ahead and squashed them. The number_format fix is wrapped in with the other good commit we wanted to keep. want me to break it out? |
Oh np, this is perfect, thanks! I'll check it more tomorrow and merge it. Do you have more coming up or should I file a new release? |
As far as I know this is it at the moment. We're doing some work to roll out international shipping in the next week or so. As a result, it's possible I'll find some new tweaks I need to make and have an additional PR. For now this is good to go. Thanks for being so active with respect to reviewing and merging things in. |
Actually go ahead and hold on this a second. I'm going to add the EEIFilingOption to the InternationalForms node. |
…ociated nodes include UPSFiling, ShipperFiling, and POA. Also added a new directory for entity test classes so we can check the constructor, getters, setters, and toNode methods on the entities. Remove one StyleCI PHP Issue be more stringent. don't just assert not empty. make sure it equals what we want
Alright, alright, alright. I'm all ready to rock here. Take a look! |
gonna get squashed also gonna get squashed
const ST_DELIVERED = 'D'; // Delivered | ||
const ST_EXCEPTION = 'X'; // Exception | ||
const ST_PICKUP = 'P'; // Pickup | ||
const ST_MANIFEST_PICKUP = 'M'; // Manifest Pickup |
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.
Cool, great work :) (also the tests!) One last thing, if you could update the changelog to include 0.7.9 with your changes, would be great. You probably know best how to describe them. Then afterwards I will merge it and release 0.7.9 directly. |
@stefandoorn done! |
Thanks! |
This PR adds a handful of Time in Transit Service codes as constants to \Ups\Entity\Service.
The \Ups\Entity\Service class is used for several API calls. In particular, the Shipping, Rating, and Time in Transit requests each make use of Service.php.
Unfortunately, the UPS Service Code for Time in Transit requests is different than the codes used when requesting a rate/attempting to process a shipment.