diff --git a/CHANGELOG b/CHANGELOG index 1cfb97349..3ee26b775 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ * Fix extra slash in UPS endpoints [cody] +* Add name to Shipwire class [cody] +* 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] diff --git a/lib/active_shipping/shipping/carriers/fedex.rb b/lib/active_shipping/shipping/carriers/fedex.rb index 4b6554881..674a976ca 100644 --- a/lib/active_shipping/shipping/carriers/fedex.rb +++ b/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 @@ -181,6 +181,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, Hash.from_xml(response), :rates => rate_estimates, :xml => response, :request => last_request, :log_xml => options[:log_xml]) end @@ -234,7 +239,8 @@ 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_error_node(document) @@ -248,8 +254,6 @@ def response_success?(document) def response_message(document) error_node = response_error_node(document) if error_node - - debugger "FedEx Error Code: #{error_node.get_text('Code').to_s}: #{error_node.get_text('Message').to_s}" else '' diff --git a/lib/active_shipping/shipping/carriers/shipwire.rb b/lib/active_shipping/shipping/carriers/shipwire.rb index 8b3816415..658cc1bd5 100644 --- a/lib/active_shipping/shipping/carriers/shipwire.rb +++ b/lib/active_shipping/shipping/carriers/shipwire.rb @@ -6,6 +6,9 @@ module Shipping class Shipwire < Carrier self.retry_safe = true + cattr_reader :name + @@name = "Shipwire" + URL = 'https://www.shipwire.com/exec/RateServices.php' SCHEMA_URL = 'http://www.shipwire.com/exec/download/RateRequest.dtd' WAREHOUSES = { 'CHI' => 'Chicago', diff --git a/test/fixtures/xml/fedex/empty_response.xml b/test/fixtures/xml/fedex/empty_response.xml new file mode 100644 index 000000000..26d65816b --- /dev/null +++ b/test/fixtures/xml/fedex/empty_response.xml @@ -0,0 +1 @@ +?xml version="1.0" encoding="UTF-8"?> \ No newline at end of file diff --git a/test/fixtures/xml/fedex/invalid_recipient_country_response.xml b/test/fixtures/xml/fedex/invalid_recipient_country_response.xml new file mode 100644 index 000000000..3845daeb0 --- /dev/null +++ b/test/fixtures/xml/fedex/invalid_recipient_country_response.xml @@ -0,0 +1 @@ +0161451Invalid recipient country \ No newline at end of file diff --git a/test/remote/fedex_test.rb b/test/remote/fedex_test.rb index ff71e8a89..95b428bc8 100644 --- a/test/remote/fedex_test.rb +++ b/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| diff --git a/test/unit/carriers/fedex_test.rb b/test/unit/carriers/fedex_test.rb index aba4f7602..91e1c5e18 100644 --- a/test/unit/carriers/fedex_test.rb +++ b/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