Merge pull request #21 from solidusio-contrib/extract_service

Extract the packaging code into a service object

app/models/spree/calculator/shipping/usps/base.rb

@@ -11,11 +11,12 @@ class Base < Spree::Calculator::Shipping::ActiveShipping::Base
         def compute_package(package)
           order = package.order
           stock_location = package.stock_location
+          max_weight = get_max_weight(package)
 
           origin = build_location(stock_location)
           destination = build_location(order.ship_address)
 
-          rates_result = retrieve_rates_from_cache(package, origin, destination)
+          rates_result = retrieve_rates_from_cache(package, origin, destination, max_weight)
 
           return nil if rates_result.kind_of?(Spree::ShippingError)
           return nil if rates_result.empty?

app/models/spree/content_item_decorator.rb

@@ -0,0 +1,7 @@
+module Spree
+  module Stock
+    ContentItem.class_eval do
+      delegate :has_product_packages?, to: :variant, prefix: true
+    end
+  end
+end

app/models/spree/package_builder.rb

@@ -0,0 +1,114 @@
+module Spree
+  class PackageBuilder
+    attr_accessor :max_weight
+
+    def process(solidus_package, max_weight)
+      # We compute and set the max_weight once, at the beginning of the process
+      @max_weight = max_weight
+      to_packages(solidus_package)
+    end
+
+    private
+
+    # Configuration
+    def units
+      Spree::ActiveShipping::Config[:units].to_sym
+    end
+
+    def multiplier
+      Spree::ActiveShipping::Config[:unit_multiplier]
+    end
+
+    def default_weight
+      Spree::ActiveShipping::Config[:default_weight]
+    end
+
+    def convert_package_to_weights_array(product_with_product_packages)
+      weights = product_with_product_packages.map do |content_item|
+        item_weight = content_item.weight
+        item_weight = default_weight if item_weight <= 0
+        item_weight *= multiplier
+
+        if (item_weight > max_weight) && max_weight > 0
+          raise Spree::ShippingError, "#{I18n.t(:shipping_error)}: The maximum per package weight for the selected service from the selected country is #{max_weight} ounces."
+        end
+
+        item_weight
+      end
+
+      weights.compact
+    end
+
+    # Used for calculating Dimensional Weight pricing.
+    # Override in your own extensions to compute package dimensions,
+    # or just leave this alone to keep the default behavior.
+    # Sample output: [9, 6, 3]
+    def convert_package_to_dimensions_array(_package)
+      []
+    end
+
+    def convert_package_to_item_packages_array(product_with_no_product_packages)
+      packages = []
+
+      product_with_no_product_packages.each do |content_item|
+        variant  = content_item.variant
+        quantity = content_item.quantity
+        product  = variant.product
+
+        product.product_packages.each do |product_package|
+          if product_package.weight.to_f <= max_weight || max_weight == 0
+            quantity.times do
+              packages << product_package
+            end
+          else
+            raise Spree::ShippingError, "#{I18n.t(:shipping_error)}: The maximum per package weight for the selected service from the selected country is #{max_weight} ounces."
+          end
+        end
+      end
+
+      packages
+    end
+
+    # Generates an array of Package objects based on the quantities and weights of the variants in the line items
+    def to_packages(solidus_package)
+      active_shipping_packages = []
+
+      product_with_product_packages = []
+      product_with_no_product_packages = []
+      # Product with no associated product packages will be combined in packages based on weight
+      # Product with associated product packages will be added individually based on their attributes
+      solidus_package.contents.each do |content_item|
+        if content_item.variant_has_product_packages?
+          product_with_product_packages << content_item
+        else
+          product_with_no_product_packages << content_item
+        end
+      end
+
+      weights = convert_package_to_weights_array(product_with_no_product_packages)
+      dimensions = convert_package_to_dimensions_array(solidus_package)
+      item_specific_packages = convert_package_to_item_packages_array(product_with_product_packages)
+
+      if max_weight <= 0
+        active_shipping_packages << ::ActiveShipping::Package.new(weights.sum, dimensions, units: units) unless weights.empty?
+      else
+        package_weight = 0
+        weights.each do |content_weight|
+          if package_weight + content_weight <= max_weight
+            package_weight += content_weight
+          else
+            active_shipping_packages << ::ActiveShipping::Package.new(package_weight, dimensions, units: units)
+            package_weight = content_weight
+          end
+        end
+        active_shipping_packages << ::ActiveShipping::Package.new(package_weight, dimensions, units: units) if package_weight > 0
+      end
+
+      item_specific_packages.each do |product_package|
+        active_shipping_packages << ::ActiveShipping::Package.new(product_package.weight * multiplier, [product_package.length, product_package.width, product_package.height], units: units)
+      end
+
+      active_shipping_packages
+    end
+  end
+end

app/models/spree/product_decorator.rb

@@ -1,6 +1,10 @@
 # Add product packages relation
 Spree::Product.class_eval do
-  has_many :product_packages, :dependent => :destroy
+  has_many :product_packages, dependent: :destroy
 
-  accepts_nested_attributes_for :product_packages, :allow_destroy => true, :reject_if => lambda { |pp| pp[:weight].blank? or Integer(pp[:weight]) < 1 }
+  accepts_nested_attributes_for :product_packages, allow_destroy: true, reject_if: ->(pp) { pp[:weight].blank? || Integer(pp[:weight]) < 1 }
+
+  def has_product_packages?
+    product_packages.any?
+  end
 end

app/models/spree/variant_decorator.rb

@@ -0,0 +1,3 @@
+Spree::Variant.class_eval do
+  delegate :has_product_packages?, to: :product
+end

spec/models/active_shipping_calculator_spec.rb

@@ -9,7 +9,7 @@
   let(:variant_2) { FactoryGirl.create(:variant, weight: 2) }
   let!(:order) do
     FactoryGirl.create(:order_with_line_items, ship_address: address, line_items_count: 2,
-                       line_items_attributes: [{ quantity: 2, variant: variant_1}, { quantity: 2, variant: variant_2 }] )
+                                               line_items_attributes: [{ quantity: 2, variant: variant_1 }, { quantity: 2, variant: variant_2 }])
   end
 
   let(:calculator) { Spree::Calculator::Shipping::ActiveShipping::BogusCalculator.new }
@@ -36,7 +36,7 @@
 
     context 'when rates are not available' do
       let(:invalid_response) do
-        ::ActiveShipping::RateResponse.new(true, "success!", {}, :rates => [], :xml => "")
+        ::ActiveShipping::RateResponse.new(true, 'success!', {}, rates: [], xml: '')
       end
 
       before do
@@ -72,7 +72,7 @@
 
   describe 'compute' do
     subject { calculator.compute(package) }
-    
+
     # It's passing but probably because it's not checking anything
     xit 'should ignore variants that have a nil weight' do
       variant = order.line_items.first.variant
@@ -87,8 +87,7 @@
       subject
     end
 
-    context "when the cache is warm" do
-
+    context 'when the cache is warm' do
       it 'should check the cache first before finding rates' do
         # Since the cache is cleared between the tests, cache.fetch will return a miss,
         # but by passing a block { Hash.new }, the return value of the block will be
@@ -99,7 +98,7 @@
       end
     end
 
-    context "when the cache is empty" do
+    context 'when the cache is empty' do
       before do
         # We're stubbing the carrier method because we
         # need to check that a specific instance of carrier
@@ -126,7 +125,7 @@
       end
 
       it 'should return nil if service_name is not found in rate_hash' do
-        allow(calculator.class).to receive(:description) {'invalid service_name'}
+        allow(calculator.class).to receive(:description) { 'invalid service_name' }
         expect(subject).to be_nil
       end
     end
@@ -137,7 +136,7 @@
       end
 
       it 'should raise a Spree::ShippingError' do
-        expect{ subject }.to raise_exception(Spree::ShippingError)
+        expect { subject }.to raise_exception(Spree::ShippingError)
       end
     end
   end
@@ -147,4 +146,51 @@
       expect(calculator.class.service_name).to eq calculator.description
     end
   end
+
+  # We make an exception and tests this the private method because max_weight values
+  # are difficult to tests conclusively through the
+  describe 'get_max_weight' do
+    include_context 'US stock location'
+    include_context 'package setup'
+
+    context 'when the max_weight from the calculator is non-zero and max_weight_per_package is zero' do
+      before do
+        allow(calculator).to receive(:max_weight_for_country).and_return(1)
+        allow(calculator).to receive(:max_weight_per_package).and_return(0)
+      end
+
+      it 'uses the max_weight_for_country as a max_weight' do
+        expect(calculator.send(:get_max_weight, package)).to eq calculator.send(:max_weight_for_country)
+      end
+    end
+
+    context 'when the max_weight from the calculator is zero and max_weight_per_package is non-zero' do
+      before do
+        allow(calculator).to receive(:max_weight_for_country).and_return(0)
+        allow(calculator).to receive(:max_weight_per_package).and_return(1)
+      end
+
+      it 'uses the max_weight_per_package as a max_weight' do
+        expect(calculator.send(:get_max_weight, package)).to eq calculator.send(:max_weight_per_package)
+      end
+    end
+
+    context 'when the max_weight from the calculator is non-zero and max_weight_per_package is non-zero' do
+      before do
+        allow(calculator).to receive(:max_weight_per_package).and_return(SecureRandom.random_number(19) + 1)
+        allow(calculator).to receive(:max_weight_for_country).and_return(SecureRandom.random_number(19) + 1)
+      end
+
+      it 'uses the lesser one of the two values' do
+        min = [calculator.send(:max_weight_for_country), calculator.send(:max_weight_per_package)].min
+        expect(calculator.send(:get_max_weight, package)).to eq min
+      end
+    end
+
+    context 'when the max_weight is zero and max_weight_per_package is zero' do
+      it 'uses 0 as a max_eight' do
+        expect(calculator.send(:get_max_weight, package)).to be_zero
+      end
+    end
+  end
 end