Skip to content

Commit

Permalink
Fix: Allow a default_weight of zero
Browse files Browse the repository at this point in the history
A default weight of zero doesn't throw a divide by zero error anymore.  Added a test condition for it.

Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>
  • Loading branch information
FineLineAutomation committed Jan 5, 2013
1 parent ff9f772 commit 4b5ac55
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 18 deletions.
16 changes: 9 additions & 7 deletions app/models/spree/calculator/active_shipping/base.rb
Expand Up @@ -141,30 +141,32 @@ def retrieve_timings(origin, destination, packages)
else
message = re.message
end

error = Spree::ShippingError.new("#{I18n.t(:shipping_error)}: #{message}")
Rails.cache.write @cache_key+"-timings", error #write error to cache to prevent constant re-lookups
raise error
end
end



private

def convert_order_to_weights_array(order)
multiplier = Spree::ActiveShipping::Config[:unit_multiplier]
default_weight = Spree::ActiveShipping::Config[:default_weight]
max_weight = max_weight_for_country(order.ship_address.country)

weights = order.line_items.map do |line_item|
item_weight = line_item.variant.weight.to_f
item_weight = default_weight if item_weight <= 0
item_weight *= multiplier

quantity = line_item.quantity
if max_weight <= 0
item_weight * quantity
elsif item_weight == 0
0
else
if item_weight < max_weight
max_quantity = (max_weight/item_weight).floor
Expand Down Expand Up @@ -193,7 +195,7 @@ def packages(order)
packages = []
weights = convert_order_to_weights_array(order)
max_weight = max_weight_for_country(order.ship_address.country)

if max_weight <= 0
packages << Package.new(weights.sum, [], :units => units)
else
Expand All @@ -208,7 +210,7 @@ def packages(order)
end
packages << Package.new(package_weight, [], :units => units) if package_weight > 0
end

packages
end

Expand Down
30 changes: 19 additions & 11 deletions spec/models/weight_limits_spec.rb
Expand Up @@ -3,7 +3,7 @@

module ActiveShipping
describe Spree::Calculator do

let(:country) { mock_model Spree::Country, :iso => "CA", :state_name => "Quebec", :state => nil }
let(:address) { mock_model Spree::Address, :country => country, :state_name => country.state_name, :city => "Montreal", :zipcode => "H2B", :state => nil }
let(:usa) { mock_model Spree::Country, :iso => "US", :state => mock_model(Spree::State, :abbr => "MD") }
Expand All @@ -18,45 +18,45 @@ module ActiveShipping
let(:us_order) { mock_model Spree::Order, :number => "R12347", :ship_address => us_address, :line_items => [ line_item_1, line_item_2, line_item_3 ] }
let(:too_heavy_order) { mock_model Spree::Order, :number => "R12349", :ship_address => us_address, :line_items => [ line_item_3, line_item_4 ] }
let(:order_with_invalid_weights) { mock_model Spree::Order, :number => "R12350", :ship_address => us_address, :line_items => [ line_item_5, line_item_6 ] }


let(:international_calculator) { Spree::Calculator::Usps::PriorityMailInternational.new }
let(:domestic_calculator) { Spree::Calculator::Usps::PriorityMail.new }

before(:each) do
Rails.cache.clear
Spree::ActiveShipping::Config.set(:units => "imperial")
Spree::ActiveShipping::Config.set(:unit_multiplier => 16)
Spree::ActiveShipping::Config.set(:default_weight => 1)
end

describe "compute" do
context "for international calculators" do
it "should convert order line items to weights array for non-US countries (ex. Canada [limit = 66 lbs])" do
weights = international_calculator.send :convert_order_to_weights_array, order
weights.should == [20.0, 21.0, 29.0, 60.0, 60.0, 60.0].map{|x| x*Spree::ActiveShipping::Config[:unit_multiplier]}
end

it "should create array of packages" do
packages = international_calculator.send :packages, order
packages.size.should == 5
packages.map{|package| package.weight.amount}.should == [41.0, 29.0, 60.0, 60.0, 60.0].map{|x| x*Spree::ActiveShipping::Config[:unit_multiplier]}
packages.map{|package| package.weight.unit}.uniq.should == [:ounces]
end

context "raise exception if max weight exceeded" do
it "should get Spree::ShippingError" do
expect { international_calculator.compute(too_heavy_order) }.to raise_error(Spree::ShippingError)
end
end
end

context "for domestic calculators" do
it "should not convert order line items to weights array for US" do
weights = domestic_calculator.send :convert_order_to_weights_array, us_order
weights.should == [20.0, 21.0, 29.0, 60.0, 60.0, 60.0].map{|x| x*Spree::ActiveShipping::Config[:unit_multiplier]}
end

it "should create array with one package for US" do
packages = domestic_calculator.send :packages, us_order
packages.size.should == 4
Expand All @@ -65,20 +65,28 @@ module ActiveShipping
end
end
end

describe "weight limits" do
it "should be set for USPS calculators" do
international_calculator.send(:max_weight_for_country, country).should == 66.0*Spree::ActiveShipping::Config[:unit_multiplier] # Canada
domestic_calculator.send(:max_weight_for_country, country).should == 70.0*Spree::ActiveShipping::Config[:unit_multiplier]
end
end

describe "validation of line item weight" do
it "should avoid zero weight or negative weight" do
weights = domestic_calculator.send :convert_order_to_weights_array, order_with_invalid_weights
default_weight = Spree::ActiveShipping::Config[:default_weight] * Spree::ActiveShipping::Config[:unit_multiplier]
weights.should == [default_weight, default_weight]
end
end

describe "validation of default weight of zero" do
it "should accept zero default weight" do
Spree::ActiveShipping::Config.set(:default_weight => 0)
weights = domestic_calculator.send :convert_order_to_weights_array, order_with_invalid_weights
weights.should == [0, 0]
end
end
end
end

0 comments on commit 4b5ac55

Please sign in to comment.