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

FedEx: Pass Residential true unless localtion is commerial #31

Closed
wants to merge 2 commits into from

Conversation

jduff
Copy link
Contributor

@jduff jduff commented Sep 15, 2011

@odorcicd @jaw6 for codereview

@codyfauser
Copy link

How does this currently work with Shipwire, USPS and UPS?

@jduff
Copy link
Contributor Author

jduff commented Sep 15, 2011

UPS submits a residential flag if the address isn't of type commercial. Shipwire, USPS and Canada Post don't submit a residential/commercial flag at all.

@@ -218,6 +218,8 @@ module ActiveMerchant
xml_node << XmlNode.new('Address') do |address_node|
address_node << XmlNode.new('PostalCode', location.postal_code)
address_node << XmlNode.new("CountryCode", location.country_code(:alpha2))

address_node << XmlNode.new("Residential", true) unless location.commercial? || !location.company_name.blank?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should probably only be basing the Residential status on the address_type, and not the company name as well. Users of the library can make that decision on their own.

address_node << XmlNode.new("Residential", !location.commercial?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to only add the node if the location type is commercial

@jaw6
Copy link

jaw6 commented Sep 16, 2011

LGTM

@jduff jduff closed this in 933147c Sep 16, 2011
isaac pushed a commit to isaac/active_shipping that referenced this pull request Oct 20, 2011
kknd113 pushed a commit to dotandbo/active_shipping that referenced this pull request May 12, 2015
kknd113 pushed a commit to dotandbo/active_shipping that referenced this pull request May 12, 2015
maartenvg pushed a commit that referenced this pull request Oct 8, 2017
maartenvg pushed a commit that referenced this pull request Oct 12, 2017
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.

None yet

4 participants