Skip to content
This repository has been archived by the owner on Jun 13, 2018. It is now read-only.

Fix bugs in USPS tracking #300

Merged
merged 4 commits into from
Jun 29, 2015
Merged

Fix bugs in USPS tracking #300

merged 4 commits into from
Jun 29, 2015

Conversation

trishume
Copy link
Contributor

While running batch processing on tracking numbers I discovered a few bugs in active_shipping USPS tracking. This PR fixes two of them:

  • Some country names aren't the same as they are in ActiveUtils and have to be mapped separately. I only found 2 out of the 6000 numbers I tested but I have emailed someone at USPS asking if they have a full list.
  • One type of event ("Origin Post is Preparing Shipment") doesn't have a time, even though it really should. This currently causes an exception. I handled this by returning an arbitrary date in the past (Jan 1, 2000).

I'm definitely open to discussion on how I handled the lack of date. I imagine lots of code would assume that all events have a time, which is true for everything except this rare event type, which is why I still return a time rather than nil. I chose an arbitrary past date rather than the current date so that the sort by time would do the right thing and put this event in the past.

@kmcphillips @garethson

@@ -154,6 +154,11 @@ class USPS < Carrier
"WS" => "Western Samoa"
}

TRACKING_ODD_COUNTRY_NAMES = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine. The way we solved a similar problem in shopify is defining a name_alternates array in the countries YAML file (https://github.com/Shopify/shopify/blob/master/db/data/countries.yml#L51), and then we roll that up into another key https://github.com/Shopify/shopify/blob/master/app/models/country_db.rb#L133). It's a bit more of a sustainable long term approach. Ok with this too though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's lots of other hashes like this in the same place so I thought I would do it a consistent way, and there should only be a few of them.

@garethson
Copy link
Contributor

@trishume
Copy link
Contributor Author

I received a list of the country names USPS uses from someone at USPS and reconciled the code so that it should handle any country they can throw at us.

@kmcphillips
Copy link
Member

This is a great approach. Huge. Thanks for digging to find that country list.

trishume added a commit that referenced this pull request Jun 29, 2015
@trishume trishume merged commit 18d9d46 into master Jun 29, 2015
@trishume trishume deleted the fix-track-country branch June 29, 2015 18:39
maartenvg pushed a commit that referenced this pull request Nov 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants