Only send non-zero package values to USPS

khoan · Feb 24, 2011 · d430d28 · d430d28
1 parent 5996253
commit d430d28
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 1 deletion.
diff --git a/lib/active_shipping/shipping/carriers/usps.rb b/lib/active_shipping/shipping/carriers/usps.rb
@@ -250,7 +250,9 @@ def build_world_rate_request(packages, destination_country)
               package << XmlNode.new('Pounds', 0)
               package << XmlNode.new('Ounces', [p.ounces,1].max.ceil) #takes an integer for some reason, must be rounded UP
               package << XmlNode.new('MailType', MAIL_TYPES[p.options[:mail_type]] || 'Package')
-              package << XmlNode.new('ValueOfContents', p.value / 100.0) if p.value && p.currency == 'USD'
+              if p.value && (p.value > 0) && (p.currency == 'USD')
+                package << XmlNode.new('ValueOfContents', p.value / 100.0)
+              end
               package << XmlNode.new('Country') do |node|
                 node.cdata = country
               end

diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -71,6 +71,8 @@ module TestFixtures
         :all_metric => Package.new(1000, [2,20,40]),
         :book => Package.new(250, [14, 19, 2]),
         :wii => Package.new((7.5 * 16), [15, 10, 4.5], :units => :imperial, :value => 269.99, :currency => 'GBP'),
+        :american_wii => Package.new((7.5 * 16), [15, 10, 4.5], :units => :imperial, :value => 269.99, :currency => 'USD'),
+        :worthless_wii => Package.new((7.5 * 16), [15, 10, 4.5], :units => :imperial, :value => 0.0, :currency => 'USD'),
         :poster => Package.new(100, [93,10], :cylinder => true),
         :small_half_pound => Package.new(8, [1,1,1], :units => :imperial),
         :big_half_pound => Package.new((16 * 50), [24,24,36], :units => :imperial),

diff --git a/test/unit/carriers/usps_test.rb b/test/unit/carriers/usps_test.rb
@@ -16,6 +16,27 @@ def setup
   # TODO: test_build_us_rate_request
   # TODO: test_build_world_rate_request
 
+  def test_build_world_rate_request
+    expected_request = "<IntlRateRequest USERID='login'><Package ID='0'><Pounds>0</Pounds><Ounces>9</Ounces><MailType>Package</MailType><Country><![CDATA[Canada]]></Country></Package></IntlRateRequest>"
+    @carrier.expects(:commit).with(:world_rates, URI.encode(expected_request), false).returns(expected_request)
+    @carrier.expects(:parse_rate_response)
+    @carrier.find_rates(@locations[:beverly_hills], @locations[:ottawa], @packages[:book], :test => true)
+  end
+
+  def test_build_world_rate_request_with_package_value
+    expected_request = "<IntlRateRequest USERID='login'><Package ID='0'><Pounds>0</Pounds><Ounces>120</Ounces><MailType>Package</MailType><ValueOfContents>269.99</ValueOfContents><Country><![CDATA[Canada]]></Country></Package></IntlRateRequest>"
+    @carrier.expects(:commit).with(:world_rates, URI.encode(expected_request), false).returns(expected_request)
+    @carrier.expects(:parse_rate_response)
+    @carrier.find_rates(@locations[:beverly_hills], @locations[:ottawa], @packages[:american_wii], :test => true)
+  end
+
+  def test_build_world_rate_request_does_not_send_zero_values
+    expected_request = "<IntlRateRequest USERID='login'><Package ID='0'><Pounds>0</Pounds><Ounces>120</Ounces><MailType>Package</MailType><Country><![CDATA[Canada]]></Country></Package></IntlRateRequest>"
+    @carrier.expects(:commit).with(:world_rates, URI.encode(expected_request), false).returns(expected_request)
+    @carrier.expects(:parse_rate_response)
+    @carrier.find_rates(@locations[:beverly_hills], @locations[:ottawa], @packages[:worthless_wii], :test => true)
+  end
+
   def test_initialize_options_requirements
     assert_raises ArgumentError do USPS.new end
     assert_nothing_raised { USPS.new(:login => 'blah')}