Improve FedEx handling of some error conditions

CHANGELOG

@@ -1,3 +1,4 @@
+* Improve FedEx handling of some error conditions [cody]
 * Add support for validating credentials to Shipwire [cody]
 * Add support for ssl_get to PostsData. Update Carriers to use PostsData module. Turn on retry safety for carriers [cody]
 * Add support for Shipwire Shipping Rate API [cody]

lib/active_shipping/shipping/carriers/fedex.rb

@@ -156,7 +156,7 @@ def build_location_node(name, location)
         location_node = XmlNode.new(name) do |xml_node|
           xml_node << XmlNode.new('StateOrProvinceCode', location.state)
           xml_node << XmlNode.new('PostalCode', location.postal_code)
-          xml_node << XmlNode.new("CountryCode", location.country_code(:alpha2)) unless location.country_code(:alpha2).blank?
+          xml_node << XmlNode.new("CountryCode", location.country_code(:alpha2))
         end
       end
 
@@ -169,9 +169,11 @@ def parse_rate_response(origin, destination, packages, response, options)
         xml_hash = Hash.from_xml(response)['FDXRateAvailableServicesReply']
         success = response_hash_success?(xml_hash)
         message = response_hash_message(xml_hash)
+
         if success
           entries << xml_hash['Entry']
           entries.flatten!
+          entries.compact!
         end
 
         entries.each do |rated_shipment|
@@ -183,6 +185,11 @@ def parse_rate_response(origin, destination, packages, response, options)
                               :packages => packages)
         end
 
+        if rate_estimates.empty?
+          success = false
+          message = "No shipping rates could be found for the destination address" if message.blank?
+        end
+
         RateResponse.new(success, message, xml_hash, :rates => rate_estimates, :xml => response, :request => last_request, :log_xml => options[:log_xml])
       end
 
@@ -239,15 +246,20 @@ def parse_tracking_response(response, options)
           :request => last_request,
           :shipment_events => shipment_events,
           :destination => destination,
-          :tracking_number => tracking_number)
+          :tracking_number => tracking_number
+        )
       end
 
       def response_hash_success?(xml_hash)
         ! xml_hash['Error'] && ! xml_hash['SoftError']
       end
 
       def response_hash_message(xml_hash)
-        response_hash_success?(xml_hash) ? '' : "FedEx Error Code: #{xml_hash['Error']['Code'] || xml_hash['SoftError']['Code']}: #{xml_hash['Error']['Message'] || xml_hash['SoftError']['Message']}"
+        if error = xml_hash['Error'] || xml_hash['SoftError']
+         "FedEx Error Code: #{error['Code']}: #{error['Message']}"
+        else
+          ''
+        end
       end
 
       def commit(request, test = false)

test/fixtures/xml/fedex/empty_response.xml

@@ -0,0 +1 @@
+?xml version="1.0" encoding="UTF-8"?><FDXRateAvailableServicesReply xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><ReplyHeader></ReplyHeader></FDXRateAvailableServicesReply>

test/fixtures/xml/fedex/invalid_recipient_country_response.xml

@@ -0,0 +1 @@
+<?xml version="1.0" encoding="UTF-8"?><FDXRateAvailableServicesReply xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><ReplyHeader></ReplyHeader><SoftError><Type>01</Type><Code>61451</Code><Message>Invalid recipient country</Message></SoftError></FDXRateAvailableServicesReply>

test/remote/fedex_test.rb

@@ -7,15 +7,14 @@ def setup
     @locations = TestFixtures.locations
     @carrier   = FedEx.new(fixtures(:fedex))
   end
-
+    
   def test_us_to_canada
     response = nil
     assert_nothing_raised do
       response = @carrier.find_rates(
                    @locations[:beverly_hills],
                    @locations[:ottawa],
-                   @packages.values_at(:wii),
-                   :test => true
+                   @packages.values_at(:wii)
                  )
       assert !response.rates.blank?
       response.rates.each do |rate|
@@ -25,14 +24,62 @@ def test_us_to_canada
     end
   end
 
+  def test_zip_to_zip_fails
+    begin
+      @carrier.find_rates(
+        Location.new(:zip => 40524),
+        Location.new(:zip => 40515),
+        @packages[:wii]
+      )
+    rescue ResponseError => e
+      assert_equal 'FedEx Error Code: 64354: Required element OriginAddress/CountryCode missing.', e.message
+    end
+  end
+
+  # FedEx requires a valid origin and destination postal code
+  def test_rates_for_locations_with_only_zip_and_country  
+    response = @carrier.find_rates(
+                 @locations[:bare_beverly_hills],
+                 @locations[:bare_ottawa],
+                 @packages.values_at(:wii)
+               )
+
+    assert response.rates.size > 0
+  end
+
+  def test_rates_for_location_with_only_country_code
+    begin
+      response = @carrier.find_rates(
+                   @locations[:bare_beverly_hills],
+                   Location.new(:country => 'CA'),
+                   @packages.values_at(:wii),
+                   :items => @items
+                 )
+    rescue ResponseError => e
+      assert_equal 'FedEx Error Code: 61530: Invalid Destination/Address/PostalCode.', e.message
+    end
+  end
+
+  def test_invalid_recipient_country
+    begin
+      response = @carrier.find_rates(
+                   @locations[:bare_beverly_hills],
+                   Location.new(:country => 'JP', :zip => '108-8361'),
+                   @packages.values_at(:wii),
+                   :items => @items
+                 )
+    rescue ResponseError => e
+      assert_equal 'FedEx Error Code: 61451: Invalid recipient country', e.message
+    end
+  end
+
   def test_canada_to_us
     response = nil
     assert_nothing_raised do
       response = @carrier.find_rates(
                    @locations[:ottawa],
                    @locations[:beverly_hills],
-                   @packages.values_at(:wii),
-                   :test => true
+                   @packages.values_at(:wii)
                  )
       assert !response.rates.blank?
       response.rates.each do |rate|
@@ -48,8 +95,7 @@ def test_ottawa_to_beverly_hills
       response = @carrier.find_rates(
                    @locations[:ottawa],
                    @locations[:beverly_hills],
-                   @packages.values_at(:book, :wii),
-                   :test => true
+                   @packages.values_at(:book, :wii)
                  )
       assert !response.rates.blank?
       response.rates.each do |rate|

test/unit/carriers/fedex_test.rb

@@ -1,8 +1,6 @@
 require File.dirname(__FILE__) + '/../../test_helper'
 
 class FedExTest < Test::Unit::TestCase
-  include ActiveMerchant::Shipping
-
   def setup
     @packages               = TestFixtures.packages
     @locations              = TestFixtures.locations
@@ -21,9 +19,37 @@ def test_initialize_options_requirements
     assert_nothing_raised { FedEx.new(:login => '999999999', :password => '7777777')}
   end
 
+  def test_invalid_recipient_country
+    @carrier.expects(:commit).returns(xml_fixture('fedex/invalid_recipient_country_response'))
+
+    begin
+      @carrier.find_rates(
+        @locations[:ottawa],                                
+        @locations[:beverly_hills],            
+        @packages.values_at(:book, :wii)
+      )
+    rescue ResponseError => e
+      assert_equal "FedEx Error Code: 61451: Invalid recipient country", e.message
+    end
+  end
+
+  def test_no_rates_response
+    @carrier.expects(:commit).returns(xml_fixture('fedex/empty_response'))
+
+    begin
+      response = @carrier.find_rates(
+        @locations[:ottawa],                                
+        @locations[:beverly_hills],            
+        @packages.values_at(:book, :wii)
+      )
+    rescue ResponseError => e
+      assert_equal "No shipping rates could be found for the destination address", e.message
+    end
+  end
+
   def test_find_tracking_info_should_return_a_tracking_response
     @carrier.expects(:commit).returns(@tracking_response)
-    assert_equal 'ActiveMerchant::Shipping::TrackingResponse', @carrier.find_tracking_info('077973360390581').class.name
+    assert_instance_of ActiveMerchant::Shipping::TrackingResponse, @carrier.find_tracking_info('077973360390581')
   end
 
   def test_find_tracking_info_should_parse_response_into_correct_number_of_shipment_events