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

adds address validation to usps.rb #410

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions lib/active_shipping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,4 @@
require 'active_shipping/errors'
require 'active_shipping/external_return_label_request'
require 'active_shipping/external_return_label_response'
require 'active_shipping/address_standardization_response'
26 changes: 26 additions & 0 deletions lib/active_shipping/address_standardization_response.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module ActiveShipping

# The `AddressStandarizationResponse` object is returned by the {ActiveShipping::Carrier#????}
# call. The most important method is {#standardize_addresses}, which will return a list of
# standardized, correctly-spelled, validated, full addresses.
#
# @note This only works for USPS and in the US
#
# @!attribute addresses
# The correct addresses returned from USPS
class AddressStandardizationResponse < Response

attr_reader :locations
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll need an attr_reader for :addresses too.


# Initializes a new AddressStandarizationResponse instance.
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation a little off here.

#
# @param success (see ActiveShipping::Response#initialize)
# @param message (see ActiveShipping::Response#initialize)
# @param params (see ActiveShipping::Response#initialize)
# @option options (see ActiveShipping::Response#initialize)
Copy link
Contributor

Choose a reason for hiding this comment

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

def initialize(success, message, params = {}, options = {})
@locations = options[:locations]
super
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

A newline is preferred at the end of all files.

75 changes: 73 additions & 2 deletions lib/active_shipping/carriers/usps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class USPS < Carrier
:us_rates => 'RateV4',
:world_rates => 'IntlRateV2',
:test => 'CarrierPickupAvailability',
:track => 'TrackV2'
:track => 'TrackV2',
:address_standarization => 'Verify'
}
USE_SSL = {
:us_rates => false,
Expand Down Expand Up @@ -229,6 +230,12 @@ def find_rates(origin, destination, packages, options = {})
end
end

def standardize_addresses(addresses, options = {})
request = build_address_standardization_request(addresses)

parse_address_standardize_response(commit(:address_standarization, request, false), addresses, options)
end

def valid_credentials?
# Cannot test with find_rates because USPS doesn't allow that in test mode
test_mode? ? canned_address_verification_works? : super
Expand Down Expand Up @@ -607,7 +614,6 @@ def parse_tracking_response(response, options = {})
end
end


def parse_tracking_info(response, node)
success = !has_error?(node)
message = response_message(node)
Expand Down Expand Up @@ -654,6 +660,71 @@ def parse_tracking_info(response, node)
)
end

def build_address_standardization_request(addresses)
xml_builder = Nokogiri::XML::Builder.new do |xml|
xml.AddressValidateRequest('USERID' => @options[:login]) do
xml.IncludeOptionalElements(true)
xml.ReturnCarrierRoute(true)
Array(addresses).each_with_index do |address, id|
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe you need to wrap addresses in an Array() constructor; do we anticipate it ever being not-Array-like?

xml.Address('ID' => id) do
xml.FirmName(address[:firm_name])
xml.Address1(address[:address1])
Copy link
Contributor

Choose a reason for hiding this comment

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

From the documentation, it looks like Address1 and Address2 are flipped for USPS.

image

We use Address1 as the street address in ActiveShipping but USPS uses it as the suite number, with Address2 being their street address.

xml.Address2(address[:address2])
xml.City(address[:city])
xml.State(address[:state])
xml.Urbanization(address[:urbanization])
xml.Zip5(address[:postal_code])
xml.Zip4(address[:zip4])
end
end
end
end
save_request(xml_builder.to_xml)
end

def parse_address_standardize_response(response, addresses, options = {})
success = true
message = ''
address_hash = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks unused to me.

locations = []
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use the map assignment suggestion below, you won't need to initialize an empty array variable.


xml = Nokogiri.XML(response)

if error = xml.at_xpath('/Error | //ServiceErrors/ServiceError')
success = false
message = error.at('Description').text
else
xml.root.xpath('Address').each do |address|
if address.at('Error')
success = false
message = address.at('Error/Description').text
break
end
end
end

if success
addresses = Hash.from_xml(response)["AddressValidateResponse"]["Address"]
if addresses.is_a?(Array)
Copy link
Contributor

Choose a reason for hiding this comment

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

Took a look at the response, you should be able to do this to avoid the Array check entirely:

locations = [addresses].flatten.map do |address|
  Location.new(#etc etc)
end

addresses.each do |address|
locations << Location.new(:address1 => address["Address1"],
:address2 => address["Address2"],
:postal_code => address["Zip5"],
:city => address["City"],
:state => address["State"])
end
else
locations << Location.new(:address1 => addresses["Address1"],
:address2 => addresses["Address2"],
:postal_code => addresses["Zip5"],
:city => addresses["City"],
:state => addresses["State"])
end
end

AddressStandardizationResponse.new(success, message, Hash.from_xml(response), :locations => locations, :addresses => addresses, :xml => response, :request => last_request)
end

def error_description_node(node)
node.xpath('Error/Description')
end
Expand Down