Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for default_weight of 0 error and added support for a max package weight #48

Closed
wants to merge 4 commits into from

2 participants

@FineLineAutomation

A default weight of zero doesn't throw a divide by zero error anymore. Added a test condition for it.

The max weight per package is useful if you want to

a) limit package weights so they don't go above carrier limits like UPS's 150lb limit.
b) have a company rule that splits boxes after a certain weight.

It is implemented to respect the countries max weight if that happens to be lower.

@FineLineAutomation FineLineAutomation Add configuration page for active_shipping settings
The page shows up in the configuration tab and allows you to set all of the settings available in the extension.

Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>
ff9f772
@FineLineAutomation FineLineAutomation Fix: Allow a default_weight of zero
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>
4b5ac55
@FineLineAutomation FineLineAutomation Added the ability to define a max weight per package.
The max weight per package is useful if you want to

a) limit package weights so they don't go above carrier limits like UPS's 150lb limit.
b) have a company rule that splits boxes after a certain weight.

It is implemented to respect the countries max weight if that happens to be lower.

Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>
882ec0c
@FineLineAutomation FineLineAutomation Merge branch 'master' of https://github.com/spree/spree_active_shipping
Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>

Conflicts:
	app/views/spree/admin/active_shipping_settings/edit.html.erb
	app/views/spree/admin/active_shipping_settings/show.html.erb
d2e264b
@radar radar closed this pull request from a commit
@FineLineAutomation FineLineAutomation Added the ability to define a max weight per package.
The max weight per package is useful if you want to

a) limit package weights so they don't go above carrier limits like UPS's 150lb limit.
b) have a company rule that splits boxes after a certain weight.

It is implemented to respect the countries max weight if that happens to be lower.

Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>

Fixes #48
5d82b77
@radar radar closed this in 5d82b77
@radar

Added to 1-3-stable and master. Thanks!

@radar radar referenced this pull request from a commit
@FineLineAutomation FineLineAutomation Added the ability to define a max weight per package.
The max weight per package is useful if you want to

a) limit package weights so they don't go above carrier limits like UPS's 150lb limit.
b) have a company rule that splits boxes after a certain weight.

It is implemented to respect the countries max weight if that happens to be lower.

Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>

Fixes #48
01b98fd
@FineLineAutomation
@FineLineAutomation

Ryan,

I think you missed commit 4b5acc5, which fixes a divide by zero error when the default_weight is 0 and a item_weight is 0 or negative.

Regards,

Nate

@radar

Right you are, Nate. I've added that commit now to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 22, 2012
  1. @FineLineAutomation

    Add configuration page for active_shipping settings

    FineLineAutomation authored
    The page shows up in the configuration tab and allows you to set all of the settings available in the extension.
    
    Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>
Commits on Jan 5, 2013
  1. @FineLineAutomation

    Fix: Allow a default_weight of zero

    FineLineAutomation authored
    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>
Commits on Jan 7, 2013
  1. @FineLineAutomation

    Added the ability to define a max weight per package.

    FineLineAutomation authored
    The max weight per package is useful if you want to
    
    a) limit package weights so they don't go above carrier limits like UPS's 150lb limit.
    b) have a company rule that splits boxes after a certain weight.
    
    It is implemented to respect the countries max weight if that happens to be lower.
    
    Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>
  2. @FineLineAutomation

    Merge branch 'master' of https://github.com/spree/spree_active_shipping

    FineLineAutomation authored
    Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>
    
    Conflicts:
    	app/views/spree/admin/active_shipping_settings/edit.html.erb
    	app/views/spree/admin/active_shipping_settings/show.html.erb
This page is out of date. Refresh to see the latest.
View
32 app/models/spree/calculator/active_shipping/base.rb
@@ -141,7 +141,7 @@ 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
@@ -149,22 +149,24 @@ def retrieve_timings(origin, destination, packages)
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)
-
+ max_weight = get_max_weight(order)
+
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
@@ -192,8 +194,8 @@ def packages(order)
units = Spree::ActiveShipping::Config[:units].to_sym
packages = []
weights = convert_order_to_weights_array(order)
- max_weight = max_weight_for_country(order.ship_address.country)
-
+ max_weight = get_max_weight(order)
+
if max_weight <= 0
packages << Package.new(weights.sum, [], :units => units)
else
@@ -208,10 +210,22 @@ def packages(order)
end
packages << Package.new(package_weight, [], :units => units) if package_weight > 0
end
-
+
packages
end
+ def get_max_weight(order)
+ max_weight = max_weight_for_country(order.ship_address.country)
+ max_weight_per_package = Spree::ActiveShipping::Config[:max_weight_per_package] * Spree::ActiveShipping::Config[:unit_multiplier]
+ if max_weight == 0 and max_weight_per_package > 0
+ max_weight = max_weight_per_package
+ elsif max_weight > 0 and max_weight_per_package < max_weight and max_weight_per_package > 0
+ max_weight = max_weight_per_package
+ end
+
+ max_weight
+ end
+
def cache_key(order)
addr = order.ship_address
line_items_hash = Digest::MD5.hexdigest(order.line_items.map {|li| li.variant_id.to_s + "_" + li.quantity.to_s }.join("|"))
View
4 app/views/spree/admin/active_shipping_settings/edit.html.erb
@@ -76,6 +76,10 @@
<%= form.label(:handling_fee, "Handling Fee: ") + tag(:br) %>
<%= preference_field_tag(:handling_fee, @config[:handling_fee], :type => :integer) %>
<% end %>
+ <%= form.field_container :max_weight_per_package do %>
+ <%= form.label(:max_weight_per_package, "Max Weight Per Package: ") + tag(:br) %>
+ <%= preference_field_tag(:max_weight_per_package, @config[:max_weight_per_package], :type => :integer) %>
+ <% end %>
<%= form.field_container :test_mode do %>
<%= form.label(:test_mode, "Test Mode: ") + tag(:br) %>
<%= preference_field_tag(:test_mode, @config[:test_mode], :type => :boolean) %>
View
4 app/views/spree/admin/active_shipping_settings/show.html.erb
@@ -39,6 +39,10 @@
<td><%= @config[:handling_fee] %></td>
</tr>
<tr>
+ <td>Max Weight Package:</td>
+ <td><%= @config[:max_weight_per_package] %></td>
+ </tr>
+ <tr>
<td>Test Mode:</td>
<td><%= @config[:test_mode] %></td>
</tr>
View
3  lib/spree/active_shipping_configuration.rb
@@ -23,6 +23,7 @@ class Spree::ActiveShippingConfiguration < Spree::Preferences::Configuration
preference :unit_multiplier, :integer, :default => 16 # 16 oz./lb - assumes variant weights are in lbs
preference :default_weight, :integer, :default => 0 # 16 oz./lb - assumes variant weights are in lbs
preference :handling_fee, :integer
-
+ preference :max_weight_per_package, :integer, :default => 0 # 0 means no limit
+
preference :test_mode, :boolean, :default => false
end
View
41 spec/models/weight_limits_spec.rb
@@ -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") }
@@ -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
@@ -65,14 +65,25 @@ 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
+
+ it "should respect the max weight per package" do
+ Spree::ActiveShipping::Config.set(:max_weight_per_package => 30)
+ weights = international_calculator.send :convert_order_to_weights_array, order
+ weights.should == [20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 21.0, 29.0].map{|x| x*Spree::ActiveShipping::Config[:unit_multiplier]}
+
+ packages = international_calculator.send :packages, order
+ packages.size.should == 12
+ packages.map{|package| package.weight.amount}.should == [20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 20.0, 21.0, 29.0].map{|x| x*Spree::ActiveShipping::Config[:unit_multiplier]}
+ packages.map{|package| package.weight.unit}.uniq.should == [:ounces]
+ 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
@@ -80,5 +91,13 @@ module ActiveShipping
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
Something went wrong with that request. Please try again.