From 4fce50620153e0bf70b3b9d1f2ee38c78ee6f43a Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 15:48:19 +0100
Subject: [PATCH 01/47] Bring splitter/base from spree

---
 app/models/spree/stock/splitter/base.rb       | 29 +++++++++++++++
 spec/models/spree/stock/splitter/base_spec.rb | 37 +++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 app/models/spree/stock/splitter/base.rb
 create mode 100644 spec/models/spree/stock/splitter/base_spec.rb

diff --git a/app/models/spree/stock/splitter/base.rb b/app/models/spree/stock/splitter/base.rb
new file mode 100644
index 0000000000..46ee1499bb
--- /dev/null
+++ b/app/models/spree/stock/splitter/base.rb
@@ -0,0 +1,29 @@
+module Spree
+  module Stock
+    module Splitter
+      class Base
+        attr_reader :packer, :next_splitter
+
+        def initialize(packer, next_splitter=nil)
+          @packer = packer
+          @next_splitter = next_splitter
+        end
+        delegate :stock_location, :order, to: :packer
+
+        def split(packages)
+          return_next(packages)
+        end
+
+        private
+
+        def return_next(packages)
+          next_splitter ? next_splitter.split(packages) : packages
+        end
+
+        def build_package(contents=[])
+          @packer.package_factory.new(stock_location, order, contents)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/stock/splitter/base_spec.rb b/spec/models/spree/stock/splitter/base_spec.rb
new file mode 100644
index 0000000000..a8b58e0e7b
--- /dev/null
+++ b/spec/models/spree/stock/splitter/base_spec.rb
@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+module Spree
+  module Stock
+    module Splitter
+      describe Base do
+        let(:packer) { build(:stock_packer) }
+
+        it 'continues to splitter chain' do
+          splitter1 = Base.new(packer)
+          splitter2 = Base.new(packer, splitter1)
+          packages = []
+
+          splitter1.should_receive(:split).with(packages)
+          splitter2.split(packages)
+        end
+
+        it 'builds package using package factory' do
+          # Basic extension of Base splitter used to test build_package method
+          class ::BasicSplitter < Base
+            def split(packages)
+              build_package
+            end
+          end
+
+          # Custom package used to test setting package factory
+          class ::CustomPackage
+            def initialize(stock_location, order, splitters); end
+          end
+          allow(Spree::Config).to receive(:package_factory) { CustomPackage }
+
+          expect(::BasicSplitter.new(packer).split(nil).class).to eq CustomPackage
+        end
+      end
+    end
+  end
+end

From 735ee1e7ed733d57f1e6f8abe469519b28606517 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 15:50:31 +0100
Subject: [PATCH 02/47] Fix simple rubocop issues

---
 app/models/spree/stock/splitter/base.rb       | 6 ++++--
 spec/models/spree/stock/splitter/base_spec.rb | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/stock/splitter/base.rb b/app/models/spree/stock/splitter/base.rb
index 46ee1499bb..ba2f8b9de9 100644
--- a/app/models/spree/stock/splitter/base.rb
+++ b/app/models/spree/stock/splitter/base.rb
@@ -1,10 +1,12 @@
+# frozen_string_literal: true
+
 module Spree
   module Stock
     module Splitter
       class Base
         attr_reader :packer, :next_splitter
 
-        def initialize(packer, next_splitter=nil)
+        def initialize(packer, next_splitter = nil)
           @packer = packer
           @next_splitter = next_splitter
         end
@@ -20,7 +22,7 @@ module Spree
           next_splitter ? next_splitter.split(packages) : packages
         end
 
-        def build_package(contents=[])
+        def build_package(contents = [])
           @packer.package_factory.new(stock_location, order, contents)
         end
       end
diff --git a/spec/models/spree/stock/splitter/base_spec.rb b/spec/models/spree/stock/splitter/base_spec.rb
index a8b58e0e7b..d1b368494f 100644
--- a/spec/models/spree/stock/splitter/base_spec.rb
+++ b/spec/models/spree/stock/splitter/base_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 module Spree
@@ -18,7 +20,7 @@ module Spree
         it 'builds package using package factory' do
           # Basic extension of Base splitter used to test build_package method
           class ::BasicSplitter < Base
-            def split(packages)
+            def split(_packages)
               build_package
             end
           end

From d18fec71255fb55efdf6b40c0ad70a5c3eef1ce9 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:00:05 +0100
Subject: [PATCH 03/47] Move Base splitter from main app models to order
 management engine services

---
 app/models/spree/stock/splitter/base.rb       | 31 ---------------
 config/application.rb                         |  2 +-
 .../order_management/stock/basic_splitter.rb  | 29 ++++++++++++++
 .../stock/basic_splitter_spec.rb              | 37 ++++++++++++++++++
 spec/config/application_spec.rb               |  4 +-
 spec/models/spree/stock/splitter/base_spec.rb | 39 -------------------
 6 files changed, 69 insertions(+), 73 deletions(-)
 delete mode 100644 app/models/spree/stock/splitter/base.rb
 create mode 100644 engines/order_management/app/services/order_management/stock/basic_splitter.rb
 create mode 100644 engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
 delete mode 100644 spec/models/spree/stock/splitter/base_spec.rb

diff --git a/app/models/spree/stock/splitter/base.rb b/app/models/spree/stock/splitter/base.rb
deleted file mode 100644
index ba2f8b9de9..0000000000
--- a/app/models/spree/stock/splitter/base.rb
+++ /dev/null
@@ -1,31 +0,0 @@
-# frozen_string_literal: true
-
-module Spree
-  module Stock
-    module Splitter
-      class Base
-        attr_reader :packer, :next_splitter
-
-        def initialize(packer, next_splitter = nil)
-          @packer = packer
-          @next_splitter = next_splitter
-        end
-        delegate :stock_location, :order, to: :packer
-
-        def split(packages)
-          return_next(packages)
-        end
-
-        private
-
-        def return_next(packages)
-          next_splitter ? next_splitter.split(packages) : packages
-        end
-
-        def build_package(contents = [])
-          @packer.package_factory.new(stock_location, order, contents)
-        end
-      end
-    end
-  end
-end
diff --git a/config/application.rb b/config/application.rb
index d48b5d2b8b..419638b236 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -83,7 +83,7 @@ module Openfoodnetwork
     #     we must use this splitter and no other
     initializer "spree.register.stock_splitters" do |app|
       app.config.spree.stock_splitters = [
-        Spree::Stock::Splitter::Base
+        OrderManagement::Stock::BasicSplitter
       ]
     end
 
diff --git a/engines/order_management/app/services/order_management/stock/basic_splitter.rb b/engines/order_management/app/services/order_management/stock/basic_splitter.rb
new file mode 100644
index 0000000000..eacbf05704
--- /dev/null
+++ b/engines/order_management/app/services/order_management/stock/basic_splitter.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Stock
+    class BasicSplitter
+      attr_reader :packer, :next_splitter
+
+      def initialize(packer, next_splitter = nil)
+        @packer = packer
+        @next_splitter = next_splitter
+      end
+      delegate :stock_location, :order, to: :packer
+
+      def split(packages)
+        return_next(packages)
+      end
+
+      private
+
+      def return_next(packages)
+        next_splitter ? next_splitter.split(packages) : packages
+      end
+
+      def build_package(contents = [])
+        @packer.package_factory.new(stock_location, order, contents)
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb b/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
new file mode 100644
index 0000000000..9cd68834b8
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+module OrderManagement
+  module Stock
+    describe BasicSplitter do
+      let(:packer) { build(:stock_packer) }
+
+      it 'continues to splitter chain' do
+        splitter1 = BasicSplitter.new(packer)
+        splitter2 = BasicSplitter.new(packer, splitter1)
+        packages = []
+
+        splitter1.should_receive(:split).with(packages)
+        splitter2.split(packages)
+      end
+
+      it 'builds package using package factory' do
+        # Basic extension of Base splitter used to test build_package method
+        class ::RealSplitter < BasicSplitter
+          def split(_packages)
+            build_package
+          end
+        end
+
+        # Custom package used to test setting package factory
+        class ::CustomPackage
+          def initialize(stock_location, order, splitters); end
+        end
+        allow(Spree::Config).to receive(:package_factory) { CustomPackage }
+
+        expect(::RealSplitter.new(packer).split(nil).class).to eq CustomPackage
+      end
+    end
+  end
+end
diff --git a/spec/config/application_spec.rb b/spec/config/application_spec.rb
index 126325a4f1..40c8d1c302 100644
--- a/spec/config/application_spec.rb
+++ b/spec/config/application_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Openfoodnetwork::Application, 'configuration' do
   let(:config) { described_class.config }
 
-  it "sets Spree::Stock::Splitter::Base as the only stock splitter" do
-    expect(config.spree.stock_splitters).to eq [Spree::Stock::Splitter::Base]
+  it "sets OrderManagement::Stock::BasicSplitter as the only stock splitter" do
+    expect(config.spree.stock_splitters).to eq [OrderManagement::Stock::BasicSplitter]
   end
 end
diff --git a/spec/models/spree/stock/splitter/base_spec.rb b/spec/models/spree/stock/splitter/base_spec.rb
deleted file mode 100644
index d1b368494f..0000000000
--- a/spec/models/spree/stock/splitter/base_spec.rb
+++ /dev/null
@@ -1,39 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-module Spree
-  module Stock
-    module Splitter
-      describe Base do
-        let(:packer) { build(:stock_packer) }
-
-        it 'continues to splitter chain' do
-          splitter1 = Base.new(packer)
-          splitter2 = Base.new(packer, splitter1)
-          packages = []
-
-          splitter1.should_receive(:split).with(packages)
-          splitter2.split(packages)
-        end
-
-        it 'builds package using package factory' do
-          # Basic extension of Base splitter used to test build_package method
-          class ::BasicSplitter < Base
-            def split(_packages)
-              build_package
-            end
-          end
-
-          # Custom package used to test setting package factory
-          class ::CustomPackage
-            def initialize(stock_location, order, splitters); end
-          end
-          allow(Spree::Config).to receive(:package_factory) { CustomPackage }
-
-          expect(::BasicSplitter.new(packer).split(nil).class).to eq CustomPackage
-        end
-      end
-    end
-  end
-end

From 3ae2877d4ec21066f007e366d0671b0b186f439b Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:03:46 +0100
Subject: [PATCH 04/47] Bring adjuster and prioritizer from spree_core

---
 app/models/spree/stock/adjuster.rb          |  28 +++++
 app/models/spree/stock/prioritizer.rb       |  47 +++++++++
 spec/models/spree/stock/prioritizer_spec.rb | 111 ++++++++++++++++++++
 3 files changed, 186 insertions(+)
 create mode 100644 app/models/spree/stock/adjuster.rb
 create mode 100644 app/models/spree/stock/prioritizer.rb
 create mode 100644 spec/models/spree/stock/prioritizer_spec.rb

diff --git a/app/models/spree/stock/adjuster.rb b/app/models/spree/stock/adjuster.rb
new file mode 100644
index 0000000000..91e38ed345
--- /dev/null
+++ b/app/models/spree/stock/adjuster.rb
@@ -0,0 +1,28 @@
+# Used by Prioritizer to adjust item quantities
+# see prioritizer_spec for use cases
+module Spree
+  module Stock
+    class Adjuster
+      attr_accessor :variant, :need, :status
+
+      def initialize(variant, quantity, status)
+        @variant = variant
+        @need = quantity
+        @status = status
+      end
+
+      def adjust(item)
+        if item.quantity >= need
+          item.quantity = need
+          @need = 0
+        elsif item.quantity < need
+          @need -= item.quantity
+        end
+      end
+
+      def fulfilled?
+        @need == 0
+      end
+    end
+  end
+end
diff --git a/app/models/spree/stock/prioritizer.rb b/app/models/spree/stock/prioritizer.rb
new file mode 100644
index 0000000000..cdef38e043
--- /dev/null
+++ b/app/models/spree/stock/prioritizer.rb
@@ -0,0 +1,47 @@
+module Spree
+  module Stock
+    class Prioritizer
+      attr_reader :packages, :order
+
+      def initialize(order, packages, adjuster_class=Adjuster)
+        @order = order
+        @packages = packages
+        @adjuster_class = adjuster_class
+      end
+
+      def prioritized_packages
+        sort_packages
+        adjust_packages
+        prune_packages
+        packages
+      end
+
+      private
+      def adjust_packages
+        order.line_items.each do |line_item|
+          adjuster = @adjuster_class.new(line_item.variant, line_item.quantity, :on_hand)
+
+          visit_packages(adjuster)
+
+          adjuster.status = :backordered
+          visit_packages(adjuster)
+        end
+      end
+
+      def visit_packages(adjuster)
+        packages.each do |package|
+          item = package.find_item adjuster.variant, adjuster.status
+          adjuster.adjust(item) if item
+        end
+      end
+
+      def sort_packages
+        # order packages by preferred stock_locations
+      end
+
+      def prune_packages
+        packages.reject! { |pkg| pkg.empty? }
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/stock/prioritizer_spec.rb b/spec/models/spree/stock/prioritizer_spec.rb
new file mode 100644
index 0000000000..52ce3ae97e
--- /dev/null
+++ b/spec/models/spree/stock/prioritizer_spec.rb
@@ -0,0 +1,111 @@
+require 'spec_helper'
+
+module Spree
+  module Stock
+    describe Prioritizer do
+      let(:order) { create(:order_with_line_items, line_items_count: 2) }
+      let(:stock_location) { build(:stock_location) }
+      let(:variant1) { order.line_items[0].variant }
+      let(:variant2) { order.line_items[1].variant }
+
+      def pack
+        package = Package.new(order, stock_location)
+        yield(package) if block_given?
+        package
+      end
+
+      it 'keeps a single package' do
+        package1 = pack do |package|
+          package.add variant1, 1, :on_hand
+          package.add variant2, 1, :on_hand
+        end
+
+        packages = [package1]
+        prioritizer = Prioritizer.new(order, packages)
+        packages = prioritizer.prioritized_packages
+        packages.size.should eq 1
+      end
+
+      it 'removes duplicate packages' do
+        package1 = pack do |package|
+          package.add variant1, 1, :on_hand
+          package.add variant2, 1, :on_hand
+        end
+        package2 = pack do |package|
+          package.add variant1, 1, :on_hand
+          package.add variant2, 1, :on_hand
+        end
+
+        packages = [package1, package2]
+        prioritizer = Prioritizer.new(order, packages)
+        packages = prioritizer.prioritized_packages
+        packages.size.should eq 1
+      end
+
+      it 'split over 2 packages' do
+        package1 = pack do |package|
+          package.add variant1, 1, :on_hand
+        end
+        package2 = pack do |package|
+          package.add variant2, 1, :on_hand
+        end
+
+        packages = [package1, package2]
+        prioritizer = Prioritizer.new(order, packages)
+        packages = prioritizer.prioritized_packages
+        packages.size.should eq 2
+      end
+
+      it '1st has some, 2nd has remaining' do
+        order.line_items[0].stub(:quantity => 5)
+        package1 = pack do |package|
+          package.add variant1, 2, :on_hand
+        end
+        package2 = pack do |package|
+          package.add variant1, 5, :on_hand
+        end
+
+        packages = [package1, package2]
+        prioritizer = Prioritizer.new(order, packages)
+        packages = prioritizer.prioritized_packages
+        packages.count.should eq 2
+        packages[0].quantity.should eq 2
+        packages[1].quantity.should eq 3
+      end
+
+      it '1st has backorder, 2nd has some' do
+        order.line_items[0].stub(:quantity => 5)
+        package1 = pack do |package|
+          package.add variant1, 5, :backordered
+        end
+        package2 = pack do |package|
+          package.add variant1, 2, :on_hand
+        end
+
+        packages = [package1, package2]
+        prioritizer = Prioritizer.new(order, packages)
+        packages = prioritizer.prioritized_packages
+
+        packages[0].quantity(:backordered).should eq 3
+        packages[1].quantity(:on_hand).should eq 2
+      end
+
+      it '1st has backorder, 2nd has all' do
+        order.line_items[0].stub(:quantity => 5)
+        package1 = pack do |package|
+          package.add variant1, 3, :backordered
+          package.add variant2, 1, :on_hand
+        end
+        package2 = pack do |package|
+          package.add variant1, 5, :on_hand
+        end
+
+        packages = [package1, package2]
+        prioritizer = Prioritizer.new(order, packages)
+        packages = prioritizer.prioritized_packages
+        packages[0].quantity(:backordered).should eq 0
+        packages[1].quantity(:on_hand).should eq 5
+      end
+    end
+  end
+end

From eb13595fd3c8ef45f21221e9bbc4ec60d0cee3a9 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:08:05 +0100
Subject: [PATCH 05/47] Fix simple rubocop issues

---
 app/models/spree/stock/adjuster.rb          | 4 +++-
 app/models/spree/stock/prioritizer.rb       | 7 +++++--
 spec/models/spree/stock/prioritizer_spec.rb | 8 +++++---
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/app/models/spree/stock/adjuster.rb b/app/models/spree/stock/adjuster.rb
index 91e38ed345..4926e8a7e6 100644
--- a/app/models/spree/stock/adjuster.rb
+++ b/app/models/spree/stock/adjuster.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 # Used by Prioritizer to adjust item quantities
 # see prioritizer_spec for use cases
 module Spree
@@ -21,7 +23,7 @@ module Spree
       end
 
       def fulfilled?
-        @need == 0
+        @need.zero?
       end
     end
   end
diff --git a/app/models/spree/stock/prioritizer.rb b/app/models/spree/stock/prioritizer.rb
index cdef38e043..41ef339702 100644
--- a/app/models/spree/stock/prioritizer.rb
+++ b/app/models/spree/stock/prioritizer.rb
@@ -1,9 +1,11 @@
+# frozen_string_literal: true
+
 module Spree
   module Stock
     class Prioritizer
       attr_reader :packages, :order
 
-      def initialize(order, packages, adjuster_class=Adjuster)
+      def initialize(order, packages, adjuster_class = Adjuster)
         @order = order
         @packages = packages
         @adjuster_class = adjuster_class
@@ -17,6 +19,7 @@ module Spree
       end
 
       private
+
       def adjust_packages
         order.line_items.each do |line_item|
           adjuster = @adjuster_class.new(line_item.variant, line_item.quantity, :on_hand)
@@ -40,7 +43,7 @@ module Spree
       end
 
       def prune_packages
-        packages.reject! { |pkg| pkg.empty? }
+        packages.reject!(&:empty?)
       end
     end
   end
diff --git a/spec/models/spree/stock/prioritizer_spec.rb b/spec/models/spree/stock/prioritizer_spec.rb
index 52ce3ae97e..c038a6dfeb 100644
--- a/spec/models/spree/stock/prioritizer_spec.rb
+++ b/spec/models/spree/stock/prioritizer_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 module Spree
@@ -57,7 +59,7 @@ module Spree
       end
 
       it '1st has some, 2nd has remaining' do
-        order.line_items[0].stub(:quantity => 5)
+        order.line_items[0].stub(quantity: 5)
         package1 = pack do |package|
           package.add variant1, 2, :on_hand
         end
@@ -74,7 +76,7 @@ module Spree
       end
 
       it '1st has backorder, 2nd has some' do
-        order.line_items[0].stub(:quantity => 5)
+        order.line_items[0].stub(quantity: 5)
         package1 = pack do |package|
           package.add variant1, 5, :backordered
         end
@@ -91,7 +93,7 @@ module Spree
       end
 
       it '1st has backorder, 2nd has all' do
-        order.line_items[0].stub(:quantity => 5)
+        order.line_items[0].stub(quantity: 5)
         package1 = pack do |package|
           package.add variant1, 3, :backordered
           package.add variant2, 1, :on_hand

From a6d7acb6f1c6f5b811168d78a1f35756ba0c9d46 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:12:16 +0100
Subject: [PATCH 06/47] Convert spec to modern rspec syntax

---
 .../stock/basic_splitter_spec.rb              |  2 +-
 spec/models/spree/stock/prioritizer_spec.rb   | 20 +++++++++----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb b/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
index 9cd68834b8..a8265a1d01 100644
--- a/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
@@ -12,7 +12,7 @@ module OrderManagement
         splitter2 = BasicSplitter.new(packer, splitter1)
         packages = []
 
-        splitter1.should_receive(:split).with(packages)
+        expect(splitter1).to receive(:split).with(packages)
         splitter2.split(packages)
       end
 
diff --git a/spec/models/spree/stock/prioritizer_spec.rb b/spec/models/spree/stock/prioritizer_spec.rb
index c038a6dfeb..aee367e7b7 100644
--- a/spec/models/spree/stock/prioritizer_spec.rb
+++ b/spec/models/spree/stock/prioritizer_spec.rb
@@ -25,7 +25,7 @@ module Spree
         packages = [package1]
         prioritizer = Prioritizer.new(order, packages)
         packages = prioritizer.prioritized_packages
-        packages.size.should eq 1
+        expect(packages.size).to eq 1
       end
 
       it 'removes duplicate packages' do
@@ -41,7 +41,7 @@ module Spree
         packages = [package1, package2]
         prioritizer = Prioritizer.new(order, packages)
         packages = prioritizer.prioritized_packages
-        packages.size.should eq 1
+        expect(packages.size).to eq 1
       end
 
       it 'split over 2 packages' do
@@ -55,7 +55,7 @@ module Spree
         packages = [package1, package2]
         prioritizer = Prioritizer.new(order, packages)
         packages = prioritizer.prioritized_packages
-        packages.size.should eq 2
+        expect(packages.size).to eq 2
       end
 
       it '1st has some, 2nd has remaining' do
@@ -70,9 +70,9 @@ module Spree
         packages = [package1, package2]
         prioritizer = Prioritizer.new(order, packages)
         packages = prioritizer.prioritized_packages
-        packages.count.should eq 2
-        packages[0].quantity.should eq 2
-        packages[1].quantity.should eq 3
+        expect(packages.count).to eq 2
+        expect(packages[0].quantity).to eq 2
+        expect(packages[1].quantity).to eq 3
       end
 
       it '1st has backorder, 2nd has some' do
@@ -88,8 +88,8 @@ module Spree
         prioritizer = Prioritizer.new(order, packages)
         packages = prioritizer.prioritized_packages
 
-        packages[0].quantity(:backordered).should eq 3
-        packages[1].quantity(:on_hand).should eq 2
+        expect(packages[0].quantity(:backordered)).to eq 3
+        expect(packages[1].quantity(:on_hand)).to eq 2
       end
 
       it '1st has backorder, 2nd has all' do
@@ -105,8 +105,8 @@ module Spree
         packages = [package1, package2]
         prioritizer = Prioritizer.new(order, packages)
         packages = prioritizer.prioritized_packages
-        packages[0].quantity(:backordered).should eq 0
-        packages[1].quantity(:on_hand).should eq 5
+        expect(packages[0].quantity(:backordered)).to eq 0
+        expect(packages[1].quantity(:on_hand)).to eq 5
       end
     end
   end

From c2ec34ab9f063f41cf46f4c07120646159771958 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:14:02 +0100
Subject: [PATCH 07/47] Bring coordinator from spree_core

---
 app/models/spree/stock/coordinator.rb       | 59 +++++++++++++++++++++
 spec/models/spree/stock/coordinator_spec.rb | 34 ++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100644 app/models/spree/stock/coordinator.rb
 create mode 100644 spec/models/spree/stock/coordinator_spec.rb

diff --git a/app/models/spree/stock/coordinator.rb b/app/models/spree/stock/coordinator.rb
new file mode 100644
index 0000000000..605b9dc343
--- /dev/null
+++ b/app/models/spree/stock/coordinator.rb
@@ -0,0 +1,59 @@
+module Spree
+  module Stock
+    class Coordinator
+      attr_reader :order
+
+      def initialize(order)
+        @order = order
+      end
+
+      def packages
+        packages = build_packages
+        packages = prioritize_packages(packages)
+        packages = estimate_packages(packages)
+      end
+
+      # Build packages as per stock location
+      #
+      # It needs to check whether each stock location holds at least one stock
+      # item for the order. In case none is found it wouldn't make any sense
+      # to build a package because it would be empty. Plus we avoid errors down
+      # the stack because it would assume the stock location has stock items
+      # for the given order
+      # 
+      # Returns an array of Package instances
+      def build_packages(packages = Array.new)
+        StockLocation.active.each do |stock_location|
+          next unless stock_location.stock_items.where(:variant_id => order.line_items.pluck(:variant_id)).exists?
+
+          packer = build_packer(stock_location, order)
+          packages += packer.packages
+        end
+        packages
+      end
+
+      private
+      def prioritize_packages(packages)
+        prioritizer = Prioritizer.new(order, packages)
+        prioritizer.prioritized_packages
+      end
+
+      def estimate_packages(packages)
+        estimator = Estimator.new(order)
+        packages.each do |package|
+          package.shipping_rates = estimator.shipping_rates(package)
+        end
+        packages
+      end
+
+      def build_packer(stock_location, order)
+        Packer.new(stock_location, order, splitters(stock_location))
+      end
+
+      def splitters(stock_location)
+        # extension point to return custom splitters for a location
+        Rails.application.config.spree.stock_splitters
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/stock/coordinator_spec.rb b/spec/models/spree/stock/coordinator_spec.rb
new file mode 100644
index 0000000000..006fd27b8b
--- /dev/null
+++ b/spec/models/spree/stock/coordinator_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+module Spree
+  module Stock
+    describe Coordinator do
+      let!(:order) { create(:order_with_line_items) }
+
+      subject { Coordinator.new(order) }
+
+      context "packages" do
+        it "builds, prioritizes and estimates" do
+          subject.should_receive(:build_packages).ordered
+          subject.should_receive(:prioritize_packages).ordered
+          subject.should_receive(:estimate_packages).ordered
+          subject.packages
+        end
+      end
+
+      context "build packages" do
+        it "builds a package for every stock location" do
+          subject.packages.count == StockLocation.count
+        end
+
+        context "missing stock items in stock location" do
+          let!(:another_location) { create(:stock_location, propagate_all_variants: false) }
+
+          it "builds packages only for valid stock locations" do
+            subject.build_packages.count.should == (StockLocation.count - 1)
+          end
+        end
+      end
+    end
+  end
+end

From ec50a788a6f1d9e61ce38aefb0a80cc0eafd6137 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:15:53 +0100
Subject: [PATCH 08/47] Fix easy rubocop issues

---
 app/models/spree/stock/coordinator.rb       | 14 +++++++++-----
 spec/models/spree/stock/coordinator_spec.rb |  2 ++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/app/models/spree/stock/coordinator.rb b/app/models/spree/stock/coordinator.rb
index 605b9dc343..4b42060bd0 100644
--- a/app/models/spree/stock/coordinator.rb
+++ b/app/models/spree/stock/coordinator.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   module Stock
     class Coordinator
@@ -10,7 +12,7 @@ module Spree
       def packages
         packages = build_packages
         packages = prioritize_packages(packages)
-        packages = estimate_packages(packages)
+        estimate_packages(packages)
       end
 
       # Build packages as per stock location
@@ -20,11 +22,12 @@ module Spree
       # to build a package because it would be empty. Plus we avoid errors down
       # the stack because it would assume the stock location has stock items
       # for the given order
-      # 
+      #
       # Returns an array of Package instances
-      def build_packages(packages = Array.new)
+      def build_packages(packages = [])
         StockLocation.active.each do |stock_location|
-          next unless stock_location.stock_items.where(:variant_id => order.line_items.pluck(:variant_id)).exists?
+          next unless stock_location.stock_items.
+            where(variant_id: order.line_items.pluck(:variant_id)).exists?
 
           packer = build_packer(stock_location, order)
           packages += packer.packages
@@ -33,6 +36,7 @@ module Spree
       end
 
       private
+
       def prioritize_packages(packages)
         prioritizer = Prioritizer.new(order, packages)
         prioritizer.prioritized_packages
@@ -50,7 +54,7 @@ module Spree
         Packer.new(stock_location, order, splitters(stock_location))
       end
 
-      def splitters(stock_location)
+      def splitters(_stock_location)
         # extension point to return custom splitters for a location
         Rails.application.config.spree.stock_splitters
       end
diff --git a/spec/models/spree/stock/coordinator_spec.rb b/spec/models/spree/stock/coordinator_spec.rb
index 006fd27b8b..0349664094 100644
--- a/spec/models/spree/stock/coordinator_spec.rb
+++ b/spec/models/spree/stock/coordinator_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 module Spree

From e0f9894b7a13bb9fd1cd584a278e2dc2df207d44 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:18:02 +0100
Subject: [PATCH 09/47] Bring packer from spree_core

---
 app/models/spree/stock/packer.rb       |  48 ++++++++++++
 spec/models/spree/stock/packer_spec.rb | 102 +++++++++++++++++++++++++
 2 files changed, 150 insertions(+)
 create mode 100644 app/models/spree/stock/packer.rb
 create mode 100644 spec/models/spree/stock/packer_spec.rb

diff --git a/app/models/spree/stock/packer.rb b/app/models/spree/stock/packer.rb
new file mode 100644
index 0000000000..2dc5a67b9c
--- /dev/null
+++ b/app/models/spree/stock/packer.rb
@@ -0,0 +1,48 @@
+module Spree
+  module Stock
+    class Packer
+      attr_reader :stock_location, :order, :splitters, :package_factory
+
+      def initialize(stock_location, order, splitters=[Splitter::Base])
+        @stock_location = stock_location
+        @order = order
+        @splitters = splitters
+        @package_factory = Spree::Config.package_factory
+      end
+
+      def packages
+        if splitters.empty?
+          [default_package]
+        else
+          build_splitter.split [default_package]
+        end
+      end
+
+      def default_package
+        package = package_factory.new(stock_location, order)
+        order.line_items.each do |line_item|
+          if Config.track_inventory_levels
+            next unless stock_location.stock_item(line_item.variant)
+
+            on_hand, backordered = stock_location.fill_status(line_item.variant, line_item.quantity)
+            package.add line_item.variant, on_hand, :on_hand if on_hand > 0
+            package.add line_item.variant, backordered, :backordered if backordered > 0
+          else
+            package.add line_item.variant, line_item.quantity, :on_hand
+          end
+        end
+        package
+      end
+
+      private
+
+      def build_splitter
+        splitter = nil
+        splitters.reverse.each do |klass|
+          splitter = klass.new(self, splitter)
+        end
+        splitter
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/stock/packer_spec.rb b/spec/models/spree/stock/packer_spec.rb
new file mode 100644
index 0000000000..34259e1c43
--- /dev/null
+++ b/spec/models/spree/stock/packer_spec.rb
@@ -0,0 +1,102 @@
+require 'spec_helper'
+
+module Spree
+  module Stock
+    describe Packer do
+      let(:order) { create(:order_with_line_items, line_items_count: 5) }
+      let(:stock_location) { create(:stock_location) }
+
+      subject { Packer.new(stock_location, order) }
+
+      before do
+        Spree::Config.stub(:package_factory) { Package }
+      end
+
+      context 'packages' do
+        it 'builds an array of packages' do
+          packages = subject.packages
+          packages.size.should eq 1
+          packages.first.contents.size.should eq 5
+        end
+
+        it 'allows users to set splitters to an empty array' do
+          packages = Packer.new(stock_location, order, []).packages
+          packages.size.should eq 1
+        end
+      end
+
+      context 'default_package' do
+        it 'contains all the items' do
+          package = subject.default_package
+          package.contents.size.should eq 5
+          package.weight.should > 0
+        end
+
+        it 'variants are added as backordered without enough on_hand' do
+          stock_location.should_receive(:fill_status).exactly(5).times.and_return([2,3])
+
+          package = subject.default_package
+          package.on_hand.size.should eq 5
+          package.backordered.size.should eq 5
+        end
+
+        context 'when a packer factory is not specified' do
+          let(:package) { double(:package, add: true) }
+
+          it 'calls Spree::Stock::Package' do
+            Package
+              .should_receive(:new)
+              .with(stock_location, order)
+              .and_return(package)
+
+            subject.default_package
+          end
+        end
+
+        context 'when a packer factory is specified' do
+          before do
+            Spree::Config.stub(:package_factory) { TestPackageFactory }
+          end
+
+          class TestPackageFactory; end
+
+          let(:package) { double(:package, add: true) }
+
+          it 'calls the specified factory' do
+            TestPackageFactory
+              .should_receive(:new)
+              .with(stock_location, order)
+              .and_return(package)
+
+            subject.default_package
+          end
+        end
+
+        context "location doesn't have order items in stock" do
+          let(:stock_location) { create(:stock_location, propagate_all_variants: false) }
+          let(:packer) { Packer.new(stock_location, order) }
+
+          it "builds an empty package" do
+            packer.default_package.contents.should be_empty
+          end
+        end
+
+        context "doesn't track inventory levels" do
+          let(:order) { Order.create }
+          let!(:line_item) { order.contents.add(create(:variant), 30) }
+
+          before { Config.track_inventory_levels = false }
+
+          it "doesn't bother stock items status in stock location" do
+            expect(subject.stock_location).not_to receive(:fill_status)
+            subject.default_package
+          end
+
+          it "still creates package with proper quantity" do
+            expect(subject.default_package.quantity).to eql 30
+          end
+        end
+      end
+    end
+  end
+end

From ccf928df124c90322e3707c00940a153e29c516c Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:18:59 +0100
Subject: [PATCH 10/47] Fix simple rubocop issues

---
 app/models/spree/stock/packer.rb       | 8 +++++---
 spec/models/spree/stock/packer_spec.rb | 4 +++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/app/models/spree/stock/packer.rb b/app/models/spree/stock/packer.rb
index 2dc5a67b9c..9dab72909c 100644
--- a/app/models/spree/stock/packer.rb
+++ b/app/models/spree/stock/packer.rb
@@ -1,9 +1,11 @@
+# frozen_string_literal: true
+
 module Spree
   module Stock
     class Packer
       attr_reader :stock_location, :order, :splitters, :package_factory
 
-      def initialize(stock_location, order, splitters=[Splitter::Base])
+      def initialize(stock_location, order, splitters = [Splitter::Base])
         @stock_location = stock_location
         @order = order
         @splitters = splitters
@@ -25,8 +27,8 @@ module Spree
             next unless stock_location.stock_item(line_item.variant)
 
             on_hand, backordered = stock_location.fill_status(line_item.variant, line_item.quantity)
-            package.add line_item.variant, on_hand, :on_hand if on_hand > 0
-            package.add line_item.variant, backordered, :backordered if backordered > 0
+            package.add line_item.variant, on_hand, :on_hand if on_hand.positive?
+            package.add line_item.variant, backordered, :backordered if backordered.positive?
           else
             package.add line_item.variant, line_item.quantity, :on_hand
           end
diff --git a/spec/models/spree/stock/packer_spec.rb b/spec/models/spree/stock/packer_spec.rb
index 34259e1c43..e29c7b92f6 100644
--- a/spec/models/spree/stock/packer_spec.rb
+++ b/spec/models/spree/stock/packer_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 module Spree
@@ -33,7 +35,7 @@ module Spree
         end
 
         it 'variants are added as backordered without enough on_hand' do
-          stock_location.should_receive(:fill_status).exactly(5).times.and_return([2,3])
+          stock_location.should_receive(:fill_status).exactly(5).times.and_return([2, 3])
 
           package = subject.default_package
           package.on_hand.size.should eq 5

From fdc085f701a5a0948cb5d731628db9d3705ae81c Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:28:52 +0100
Subject: [PATCH 11/47] Convert to modern rspec and remove specs not applicable
 to ofn

---
 spec/models/spree/stock/packer_spec.rb | 43 ++++++--------------------
 1 file changed, 10 insertions(+), 33 deletions(-)

diff --git a/spec/models/spree/stock/packer_spec.rb b/spec/models/spree/stock/packer_spec.rb
index e29c7b92f6..c392b9788e 100644
--- a/spec/models/spree/stock/packer_spec.rb
+++ b/spec/models/spree/stock/packer_spec.rb
@@ -17,29 +17,31 @@ module Spree
       context 'packages' do
         it 'builds an array of packages' do
           packages = subject.packages
-          packages.size.should eq 1
-          packages.first.contents.size.should eq 5
+          expect(packages.size).to eq 1
+          expect(packages.first.contents.size).to eq 5
         end
 
         it 'allows users to set splitters to an empty array' do
           packages = Packer.new(stock_location, order, []).packages
-          packages.size.should eq 1
+          expect(packages.size).to eq 1
         end
       end
 
       context 'default_package' do
+        before { order.line_items.first.variant.update(weight: 1) }
+
         it 'contains all the items' do
           package = subject.default_package
-          package.contents.size.should eq 5
-          package.weight.should > 0
+          expect(package.contents.size).to eq 5
+          expect(package.weight).to be_positive
         end
 
         it 'variants are added as backordered without enough on_hand' do
-          stock_location.should_receive(:fill_status).exactly(5).times.and_return([2, 3])
+          expect(stock_location).to receive(:fill_status).exactly(5).times.and_return([2, 3])
 
           package = subject.default_package
-          package.on_hand.size.should eq 5
-          package.backordered.size.should eq 5
+          expect(package.on_hand.size).to eq 5
+          expect(package.backordered.size).to eq 5
         end
 
         context 'when a packer factory is not specified' do
@@ -73,31 +75,6 @@ module Spree
             subject.default_package
           end
         end
-
-        context "location doesn't have order items in stock" do
-          let(:stock_location) { create(:stock_location, propagate_all_variants: false) }
-          let(:packer) { Packer.new(stock_location, order) }
-
-          it "builds an empty package" do
-            packer.default_package.contents.should be_empty
-          end
-        end
-
-        context "doesn't track inventory levels" do
-          let(:order) { Order.create }
-          let!(:line_item) { order.contents.add(create(:variant), 30) }
-
-          before { Config.track_inventory_levels = false }
-
-          it "doesn't bother stock items status in stock location" do
-            expect(subject.stock_location).not_to receive(:fill_status)
-            subject.default_package
-          end
-
-          it "still creates package with proper quantity" do
-            expect(subject.default_package.quantity).to eql 30
-          end
-        end
       end
     end
   end

From 69b9cfbad2ac5182e07b6adb20279cf2af649bb8 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:30:18 +0100
Subject: [PATCH 12/47] Make packer use BasicSplitter by default

---
 app/models/spree/stock/packer.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/stock/packer.rb b/app/models/spree/stock/packer.rb
index 9dab72909c..242f46536e 100644
--- a/app/models/spree/stock/packer.rb
+++ b/app/models/spree/stock/packer.rb
@@ -5,7 +5,7 @@ module Spree
     class Packer
       attr_reader :stock_location, :order, :splitters, :package_factory
 
-      def initialize(stock_location, order, splitters = [Splitter::Base])
+      def initialize(stock_location, order, splitters = [OrderManagement::Stock::BasicSplitter])
         @stock_location = stock_location
         @order = order
         @splitters = splitters

From 4711a7469ac3a1930cbe8a357318c04443aa94cc Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:37:23 +0100
Subject: [PATCH 13/47] Adapt coordinator spec to ofn and remove spec that is
 not applicable (multi stock locations)

---
 spec/models/spree/stock/coordinator_spec.rb | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/spec/models/spree/stock/coordinator_spec.rb b/spec/models/spree/stock/coordinator_spec.rb
index 0349664094..adeec6e7cd 100644
--- a/spec/models/spree/stock/coordinator_spec.rb
+++ b/spec/models/spree/stock/coordinator_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 module Spree
   module Stock
     describe Coordinator do
-      let!(:order) { create(:order_with_line_items) }
+      let!(:order) { create(:order_with_line_items, distributor: create(:distributor_enterprise)) }
 
       subject { Coordinator.new(order) }
 
@@ -17,20 +17,6 @@ module Spree
           subject.packages
         end
       end
-
-      context "build packages" do
-        it "builds a package for every stock location" do
-          subject.packages.count == StockLocation.count
-        end
-
-        context "missing stock items in stock location" do
-          let!(:another_location) { create(:stock_location, propagate_all_variants: false) }
-
-          it "builds packages only for valid stock locations" do
-            subject.build_packages.count.should == (StockLocation.count - 1)
-          end
-        end
-      end
     end
   end
 end

From 8c3b8c4db5b7c0145fc2e037efa30e503f08593b Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:38:50 +0100
Subject: [PATCH 14/47] Bring estimator from spree_core

---
 app/models/spree/stock/estimator.rb       |  50 +++++++++++
 spec/models/spree/stock/estimator_spec.rb | 100 ++++++++++++++++++++++
 2 files changed, 150 insertions(+)
 create mode 100644 app/models/spree/stock/estimator.rb
 create mode 100644 spec/models/spree/stock/estimator_spec.rb

diff --git a/app/models/spree/stock/estimator.rb b/app/models/spree/stock/estimator.rb
new file mode 100644
index 0000000000..b9647f6255
--- /dev/null
+++ b/app/models/spree/stock/estimator.rb
@@ -0,0 +1,50 @@
+module Spree
+  module Stock
+    class Estimator
+      attr_reader :order, :currency
+
+      def initialize(order)
+        @order = order
+        @currency = order.currency
+      end
+
+      def shipping_rates(package, frontend_only = true)
+        shipping_rates = Array.new
+        shipping_methods = shipping_methods(package)
+        return [] unless shipping_methods
+
+        shipping_methods.each do |shipping_method|
+          cost = calculate_cost(shipping_method, package)
+          shipping_rates << shipping_method.shipping_rates.new(:cost => cost) unless cost.nil?
+        end
+
+        shipping_rates.sort_by! { |r| r.cost || 0 }
+
+        unless shipping_rates.empty?
+          if frontend_only
+            shipping_rates.each do |rate|
+              rate.selected = true and break if rate.shipping_method.frontend?
+            end
+          else
+            shipping_rates.first.selected = true
+          end
+        end
+
+        shipping_rates
+      end
+
+      private
+      def shipping_methods(package)
+        shipping_methods = package.shipping_methods
+        shipping_methods.delete_if { |ship_method| !ship_method.calculator.available?(package) }
+        shipping_methods.delete_if { |ship_method| !ship_method.include?(order.ship_address) }
+        shipping_methods.delete_if { |ship_method| !(ship_method.calculator.preferences[:currency].nil? || ship_method.calculator.preferences[:currency] == currency) }
+        shipping_methods
+      end
+
+      def calculate_cost(shipping_method, package)
+        shipping_method.calculator.compute(package)
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/stock/estimator_spec.rb b/spec/models/spree/stock/estimator_spec.rb
new file mode 100644
index 0000000000..e5819827a8
--- /dev/null
+++ b/spec/models/spree/stock/estimator_spec.rb
@@ -0,0 +1,100 @@
+require 'spec_helper'
+
+module Spree
+  module Stock
+    describe Estimator do
+      let!(:shipping_method) { create(:shipping_method) }
+      let(:package) { build(:stock_package_fulfilled) }
+      let(:order) { package.order }
+      subject { Estimator.new(order) }
+
+      context "#shipping rates" do
+        before(:each) do
+          shipping_method.zones.first.members.create(:zoneable => order.ship_address.country)
+          ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(true)
+          ShippingMethod.any_instance.stub_chain(:calculator, :compute).and_return(4.00)
+          ShippingMethod.any_instance.stub_chain(:calculator, :preferences).and_return({:currency => "USD"})
+          ShippingMethod.any_instance.stub_chain(:calculator, :marked_for_destruction?)
+
+          package.stub(:shipping_methods => [shipping_method])
+        end
+
+        it "returns shipping rates from a shipping method if the order's ship address is in the same zone" do
+          shipping_rates = subject.shipping_rates(package)
+          shipping_rates.first.cost.should eq 4.00
+        end
+
+        it "does not return shipping rates from a shipping method if the order's ship address is in a different zone" do
+          shipping_method.zones.each{|z| z.members.delete_all}
+          shipping_rates = subject.shipping_rates(package)
+          shipping_rates.should == []
+        end
+
+        it "does not return shipping rates from a shipping method if the calculator is not available for that order" do
+          ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(false)
+          shipping_rates = subject.shipping_rates(package)
+          shipping_rates.should == []
+        end
+
+        it "returns shipping rates from a shipping method if the currency matches the order's currency" do
+          shipping_rates = subject.shipping_rates(package)
+          shipping_rates.first.cost.should eq 4.00
+        end
+
+        it "does not return shipping rates from a shipping method if the currency is different than the order's currency" do
+          order.currency = "GBP"
+          shipping_rates = subject.shipping_rates(package)
+          shipping_rates.should == []
+        end
+
+        it "sorts shipping rates by cost" do
+          shipping_methods = 3.times.map { create(:shipping_method) }
+          shipping_methods[0].stub_chain(:calculator, :compute).and_return(5.00)
+          shipping_methods[1].stub_chain(:calculator, :compute).and_return(3.00)
+          shipping_methods[2].stub_chain(:calculator, :compute).and_return(4.00)
+
+          subject.stub(:shipping_methods).and_return(shipping_methods)
+
+          expect(subject.shipping_rates(package).map(&:cost)).to eq %w[3.00 4.00 5.00].map(&BigDecimal.method(:new))
+        end
+
+        context "general shipping methods" do
+          let(:shipping_methods) { 2.times.map { create(:shipping_method) } }
+
+          it "selects the most affordable shipping rate" do
+            shipping_methods[0].stub_chain(:calculator, :compute).and_return(5.00)
+            shipping_methods[1].stub_chain(:calculator, :compute).and_return(3.00)
+
+            subject.stub(:shipping_methods).and_return(shipping_methods)
+
+            expect(subject.shipping_rates(package).sort_by(&:cost).map(&:selected)).to eq [true, false]
+          end
+
+          it "selects the most affordable shipping rate and doesn't raise exception over nil cost" do
+            shipping_methods[0].stub_chain(:calculator, :compute).and_return(1.00)
+            shipping_methods[1].stub_chain(:calculator, :compute).and_return(nil)
+
+            subject.stub(:shipping_methods).and_return(shipping_methods)
+
+            subject.shipping_rates(package)
+          end
+        end
+
+        context "involves backend only shipping methods" do
+          let(:backend_method) { create(:shipping_method, display_on: "back_end") }
+          let(:generic_method) { create(:shipping_method) }
+
+          # regression for #3287
+          it "doesn't select backend rates even if they're more affordable" do
+            backend_method.stub_chain(:calculator, :compute).and_return(0.00)
+            generic_method.stub_chain(:calculator, :compute).and_return(5.00)
+
+            subject.stub(:shipping_methods).and_return([backend_method, generic_method])
+
+            expect(subject.shipping_rates(package).map(&:selected)).to eq [false, true]
+          end
+        end
+      end
+    end
+  end
+end

From b16db2f40e425b58b7729b15ba1816444c651fc8 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 16:45:38 +0100
Subject: [PATCH 15/47] Fix easy rubocop issues

---
 app/models/spree/stock/estimator.rb       | 14 +++--
 spec/models/spree/stock/estimator_spec.rb | 63 ++++++++++++++---------
 2 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/app/models/spree/stock/estimator.rb b/app/models/spree/stock/estimator.rb
index b9647f6255..7daf7d9d67 100644
--- a/app/models/spree/stock/estimator.rb
+++ b/app/models/spree/stock/estimator.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   module Stock
     class Estimator
@@ -9,13 +11,13 @@ module Spree
       end
 
       def shipping_rates(package, frontend_only = true)
-        shipping_rates = Array.new
+        shipping_rates = []
         shipping_methods = shipping_methods(package)
         return [] unless shipping_methods
 
         shipping_methods.each do |shipping_method|
           cost = calculate_cost(shipping_method, package)
-          shipping_rates << shipping_method.shipping_rates.new(:cost => cost) unless cost.nil?
+          shipping_rates << shipping_method.shipping_rates.new(cost: cost) unless cost.nil?
         end
 
         shipping_rates.sort_by! { |r| r.cost || 0 }
@@ -23,7 +25,7 @@ module Spree
         unless shipping_rates.empty?
           if frontend_only
             shipping_rates.each do |rate|
-              rate.selected = true and break if rate.shipping_method.frontend?
+              rate.selected = true && break if rate.shipping_method.frontend?
             end
           else
             shipping_rates.first.selected = true
@@ -34,11 +36,15 @@ module Spree
       end
 
       private
+
       def shipping_methods(package)
         shipping_methods = package.shipping_methods
         shipping_methods.delete_if { |ship_method| !ship_method.calculator.available?(package) }
         shipping_methods.delete_if { |ship_method| !ship_method.include?(order.ship_address) }
-        shipping_methods.delete_if { |ship_method| !(ship_method.calculator.preferences[:currency].nil? || ship_method.calculator.preferences[:currency] == currency) }
+        shipping_methods.delete_if { |ship_method|
+          !(ship_method.calculator.preferences[:currency].nil? ||
+            ship_method.calculator.preferences[:currency] == currency)
+        }
         shipping_methods
       end
 
diff --git a/spec/models/spree/stock/estimator_spec.rb b/spec/models/spree/stock/estimator_spec.rb
index e5819827a8..e439e85585 100644
--- a/spec/models/spree/stock/estimator_spec.rb
+++ b/spec/models/spree/stock/estimator_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 module Spree
@@ -10,41 +12,52 @@ module Spree
 
       context "#shipping rates" do
         before(:each) do
-          shipping_method.zones.first.members.create(:zoneable => order.ship_address.country)
+          shipping_method.zones.first.members.create(zoneable: order.ship_address.country)
           ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(true)
           ShippingMethod.any_instance.stub_chain(:calculator, :compute).and_return(4.00)
-          ShippingMethod.any_instance.stub_chain(:calculator, :preferences).and_return({:currency => "USD"})
+          ShippingMethod.any_instance.
+            stub_chain(:calculator, :preferences).and_return({ currency: "USD" })
           ShippingMethod.any_instance.stub_chain(:calculator, :marked_for_destruction?)
 
-          package.stub(:shipping_methods => [shipping_method])
+          package.stub(shipping_methods: [shipping_method])
         end
 
-        it "returns shipping rates from a shipping method if the order's ship address is in the same zone" do
-          shipping_rates = subject.shipping_rates(package)
-          shipping_rates.first.cost.should eq 4.00
+        context "the order's ship address is in the same zone" do
+          it "returns shipping rates from a shipping method" do
+            shipping_rates = subject.shipping_rates(package)
+            shipping_rates.first.cost.should eq 4.00
+          end
         end
 
-        it "does not return shipping rates from a shipping method if the order's ship address is in a different zone" do
-          shipping_method.zones.each{|z| z.members.delete_all}
-          shipping_rates = subject.shipping_rates(package)
-          shipping_rates.should == []
+        context "the order's ship address is in a different zone" do
+          it "does not return shipping rates from a shipping method" do
+            shipping_method.zones.each{ |z| z.members.delete_all }
+            shipping_rates = subject.shipping_rates(package)
+            shipping_rates.should == []
+          end
         end
 
-        it "does not return shipping rates from a shipping method if the calculator is not available for that order" do
-          ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(false)
-          shipping_rates = subject.shipping_rates(package)
-          shipping_rates.should == []
+        context "the calculator is not available for that order" do
+          it "does not return shipping rates from a shipping method" do
+            ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(false)
+            shipping_rates = subject.shipping_rates(package)
+            shipping_rates.should == []
+          end
         end
 
-        it "returns shipping rates from a shipping method if the currency matches the order's currency" do
-          shipping_rates = subject.shipping_rates(package)
-          shipping_rates.first.cost.should eq 4.00
+        context "the currency matches the order's currency" do
+          it "returns shipping rates from a shipping method" do
+            shipping_rates = subject.shipping_rates(package)
+            shipping_rates.first.cost.should eq 4.00
+          end
         end
 
-        it "does not return shipping rates from a shipping method if the currency is different than the order's currency" do
-          order.currency = "GBP"
-          shipping_rates = subject.shipping_rates(package)
-          shipping_rates.should == []
+        context "the currency is different than the order's currency" do
+          it "does not return shipping rates from a shipping method" do
+            order.currency = "GBP"
+            shipping_rates = subject.shipping_rates(package)
+            shipping_rates.should == []
+          end
         end
 
         it "sorts shipping rates by cost" do
@@ -55,7 +68,8 @@ module Spree
 
           subject.stub(:shipping_methods).and_return(shipping_methods)
 
-          expect(subject.shipping_rates(package).map(&:cost)).to eq %w[3.00 4.00 5.00].map(&BigDecimal.method(:new))
+          expected_costs = %w[3.00 4.00 5.00].map(&BigDecimal.method(:new))
+          expect(subject.shipping_rates(package).map(&:cost)).to eq expected_costs
         end
 
         context "general shipping methods" do
@@ -67,10 +81,11 @@ module Spree
 
             subject.stub(:shipping_methods).and_return(shipping_methods)
 
-            expect(subject.shipping_rates(package).sort_by(&:cost).map(&:selected)).to eq [true, false]
+            shipping_rates = subject.shipping_rates(package)
+            expect(shipping_rates.sort_by(&:cost).map(&:selected)).to eq [true, false]
           end
 
-          it "selects the most affordable shipping rate and doesn't raise exception over nil cost" do
+          it "selects the cheapest shipping rate and doesn't raise exception over nil cost" do
             shipping_methods[0].stub_chain(:calculator, :compute).and_return(1.00)
             shipping_methods[1].stub_chain(:calculator, :compute).and_return(nil)
 

From feadbb086f5a58bb14b1cb47712378b52868116e Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:00:54 +0100
Subject: [PATCH 16/47] Adapt spec to OFN context

---
 app/models/spree/stock/estimator.rb       |  5 ++++-
 spec/models/spree/stock/estimator_spec.rb | 18 +++++++++---------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/app/models/spree/stock/estimator.rb b/app/models/spree/stock/estimator.rb
index 7daf7d9d67..9f14033590 100644
--- a/app/models/spree/stock/estimator.rb
+++ b/app/models/spree/stock/estimator.rb
@@ -25,7 +25,10 @@ module Spree
         unless shipping_rates.empty?
           if frontend_only
             shipping_rates.each do |rate|
-              rate.selected = true && break if rate.shipping_method.frontend?
+              if rate.shipping_method.frontend?
+                rate.selected = true
+                break
+              end
             end
           else
             shipping_rates.first.selected = true
diff --git a/spec/models/spree/stock/estimator_spec.rb b/spec/models/spree/stock/estimator_spec.rb
index e439e85585..a56172777a 100644
--- a/spec/models/spree/stock/estimator_spec.rb
+++ b/spec/models/spree/stock/estimator_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 module Spree
   module Stock
     describe Estimator do
-      let!(:shipping_method) { create(:shipping_method) }
+      let!(:shipping_method) { create(:shipping_method, zones: [Spree::Zone.global] ) }
       let(:package) { build(:stock_package_fulfilled) }
       let(:order) { package.order }
       subject { Estimator.new(order) }
@@ -16,7 +16,7 @@ module Spree
           ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(true)
           ShippingMethod.any_instance.stub_chain(:calculator, :compute).and_return(4.00)
           ShippingMethod.any_instance.
-            stub_chain(:calculator, :preferences).and_return({ currency: "USD" })
+            stub_chain(:calculator, :preferences).and_return({ currency: order.currency })
           ShippingMethod.any_instance.stub_chain(:calculator, :marked_for_destruction?)
 
           package.stub(shipping_methods: [shipping_method])
@@ -25,15 +25,15 @@ module Spree
         context "the order's ship address is in the same zone" do
           it "returns shipping rates from a shipping method" do
             shipping_rates = subject.shipping_rates(package)
-            shipping_rates.first.cost.should eq 4.00
+            expect(shipping_rates.first.cost).to eq 4.00
           end
         end
 
         context "the order's ship address is in a different zone" do
-          it "does not return shipping rates from a shipping method" do
+          it "still returns shipping rates from a shipping method" do
             shipping_method.zones.each{ |z| z.members.delete_all }
             shipping_rates = subject.shipping_rates(package)
-            shipping_rates.should == []
+            expect(shipping_rates.first.cost).to eq 4.00
           end
         end
 
@@ -41,22 +41,22 @@ module Spree
           it "does not return shipping rates from a shipping method" do
             ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(false)
             shipping_rates = subject.shipping_rates(package)
-            shipping_rates.should == []
+            expect(shipping_rates).to eq []
           end
         end
 
         context "the currency matches the order's currency" do
           it "returns shipping rates from a shipping method" do
             shipping_rates = subject.shipping_rates(package)
-            shipping_rates.first.cost.should eq 4.00
+            expect(shipping_rates.first.cost).to eq 4.00
           end
         end
 
         context "the currency is different than the order's currency" do
           it "does not return shipping rates from a shipping method" do
-            order.currency = "GBP"
+            order.currency = "USD"
             shipping_rates = subject.shipping_rates(package)
-            shipping_rates.should == []
+            expect(shipping_rates).to eq []
           end
         end
 

From 720ad9de0e85f077aa276b7ab46e70a807f21c57 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:01:54 +0100
Subject: [PATCH 17/47] Convert specs to modern rsspec syntax

---
 spec/models/spree/stock/coordinator_spec.rb |  6 ++--
 spec/models/spree/stock/estimator_spec.rb   | 40 ++++++++++-----------
 spec/models/spree/stock/packer_spec.rb      | 12 +++----
 spec/models/spree/stock/prioritizer_spec.rb |  6 ++--
 4 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/spec/models/spree/stock/coordinator_spec.rb b/spec/models/spree/stock/coordinator_spec.rb
index adeec6e7cd..63f81e96ce 100644
--- a/spec/models/spree/stock/coordinator_spec.rb
+++ b/spec/models/spree/stock/coordinator_spec.rb
@@ -11,9 +11,9 @@ module Spree
 
       context "packages" do
         it "builds, prioritizes and estimates" do
-          subject.should_receive(:build_packages).ordered
-          subject.should_receive(:prioritize_packages).ordered
-          subject.should_receive(:estimate_packages).ordered
+          expect(subject).to receive(:build_packages).ordered
+          expect(subject).to receive(:prioritize_packages).ordered
+          expect(subject).to receive(:estimate_packages).ordered
           subject.packages
         end
       end
diff --git a/spec/models/spree/stock/estimator_spec.rb b/spec/models/spree/stock/estimator_spec.rb
index a56172777a..6cdf8106b2 100644
--- a/spec/models/spree/stock/estimator_spec.rb
+++ b/spec/models/spree/stock/estimator_spec.rb
@@ -13,13 +13,13 @@ module Spree
       context "#shipping rates" do
         before(:each) do
           shipping_method.zones.first.members.create(zoneable: order.ship_address.country)
-          ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(true)
-          ShippingMethod.any_instance.stub_chain(:calculator, :compute).and_return(4.00)
-          ShippingMethod.any_instance.
-            stub_chain(:calculator, :preferences).and_return({ currency: order.currency })
-          ShippingMethod.any_instance.stub_chain(:calculator, :marked_for_destruction?)
+          allow_any_instance_of(ShippingMethod).to receive_message_chain(:calculator, :available?).and_return(true)
+          allow_any_instance_of(ShippingMethod).to receive_message_chain(:calculator, :compute).and_return(4.00)
+          allow_any_instance_of(ShippingMethod).
+            to receive_message_chain(:calculator, :preferences).and_return({ currency: order.currency })
+          allow_any_instance_of(ShippingMethod).to receive_message_chain(:calculator, :marked_for_destruction?)
 
-          package.stub(shipping_methods: [shipping_method])
+          allow(package).to receive_messages(shipping_methods: [shipping_method])
         end
 
         context "the order's ship address is in the same zone" do
@@ -39,7 +39,7 @@ module Spree
 
         context "the calculator is not available for that order" do
           it "does not return shipping rates from a shipping method" do
-            ShippingMethod.any_instance.stub_chain(:calculator, :available?).and_return(false)
+            allow_any_instance_of(ShippingMethod).to receive_message_chain(:calculator, :available?).and_return(false)
             shipping_rates = subject.shipping_rates(package)
             expect(shipping_rates).to eq []
           end
@@ -62,11 +62,11 @@ module Spree
 
         it "sorts shipping rates by cost" do
           shipping_methods = 3.times.map { create(:shipping_method) }
-          shipping_methods[0].stub_chain(:calculator, :compute).and_return(5.00)
-          shipping_methods[1].stub_chain(:calculator, :compute).and_return(3.00)
-          shipping_methods[2].stub_chain(:calculator, :compute).and_return(4.00)
+          allow(shipping_methods[0]).to receive_message_chain(:calculator, :compute).and_return(5.00)
+          allow(shipping_methods[1]).to receive_message_chain(:calculator, :compute).and_return(3.00)
+          allow(shipping_methods[2]).to receive_message_chain(:calculator, :compute).and_return(4.00)
 
-          subject.stub(:shipping_methods).and_return(shipping_methods)
+          allow(subject).to receive(:shipping_methods).and_return(shipping_methods)
 
           expected_costs = %w[3.00 4.00 5.00].map(&BigDecimal.method(:new))
           expect(subject.shipping_rates(package).map(&:cost)).to eq expected_costs
@@ -76,20 +76,20 @@ module Spree
           let(:shipping_methods) { 2.times.map { create(:shipping_method) } }
 
           it "selects the most affordable shipping rate" do
-            shipping_methods[0].stub_chain(:calculator, :compute).and_return(5.00)
-            shipping_methods[1].stub_chain(:calculator, :compute).and_return(3.00)
+            allow(shipping_methods[0]).to receive_message_chain(:calculator, :compute).and_return(5.00)
+            allow(shipping_methods[1]).to receive_message_chain(:calculator, :compute).and_return(3.00)
 
-            subject.stub(:shipping_methods).and_return(shipping_methods)
+            allow(subject).to receive(:shipping_methods).and_return(shipping_methods)
 
             shipping_rates = subject.shipping_rates(package)
             expect(shipping_rates.sort_by(&:cost).map(&:selected)).to eq [true, false]
           end
 
           it "selects the cheapest shipping rate and doesn't raise exception over nil cost" do
-            shipping_methods[0].stub_chain(:calculator, :compute).and_return(1.00)
-            shipping_methods[1].stub_chain(:calculator, :compute).and_return(nil)
+            allow(shipping_methods[0]).to receive_message_chain(:calculator, :compute).and_return(1.00)
+            allow(shipping_methods[1]).to receive_message_chain(:calculator, :compute).and_return(nil)
 
-            subject.stub(:shipping_methods).and_return(shipping_methods)
+            allow(subject).to receive(:shipping_methods).and_return(shipping_methods)
 
             subject.shipping_rates(package)
           end
@@ -101,10 +101,10 @@ module Spree
 
           # regression for #3287
           it "doesn't select backend rates even if they're more affordable" do
-            backend_method.stub_chain(:calculator, :compute).and_return(0.00)
-            generic_method.stub_chain(:calculator, :compute).and_return(5.00)
+            allow(backend_method).to receive_message_chain(:calculator, :compute).and_return(0.00)
+            allow(generic_method).to receive_message_chain(:calculator, :compute).and_return(5.00)
 
-            subject.stub(:shipping_methods).and_return([backend_method, generic_method])
+            allow(subject).to receive(:shipping_methods).and_return([backend_method, generic_method])
 
             expect(subject.shipping_rates(package).map(&:selected)).to eq [false, true]
           end
diff --git a/spec/models/spree/stock/packer_spec.rb b/spec/models/spree/stock/packer_spec.rb
index c392b9788e..cd694d87e1 100644
--- a/spec/models/spree/stock/packer_spec.rb
+++ b/spec/models/spree/stock/packer_spec.rb
@@ -11,7 +11,7 @@ module Spree
       subject { Packer.new(stock_location, order) }
 
       before do
-        Spree::Config.stub(:package_factory) { Package }
+        allow(Spree::Config).to receive(:package_factory) { Package }
       end
 
       context 'packages' do
@@ -48,8 +48,8 @@ module Spree
           let(:package) { double(:package, add: true) }
 
           it 'calls Spree::Stock::Package' do
-            Package
-              .should_receive(:new)
+            expect(Package)
+              .to receive(:new)
               .with(stock_location, order)
               .and_return(package)
 
@@ -59,7 +59,7 @@ module Spree
 
         context 'when a packer factory is specified' do
           before do
-            Spree::Config.stub(:package_factory) { TestPackageFactory }
+            allow(Spree::Config).to receive(:package_factory) { TestPackageFactory }
           end
 
           class TestPackageFactory; end
@@ -67,8 +67,8 @@ module Spree
           let(:package) { double(:package, add: true) }
 
           it 'calls the specified factory' do
-            TestPackageFactory
-              .should_receive(:new)
+            expect(TestPackageFactory)
+              .to receive(:new)
               .with(stock_location, order)
               .and_return(package)
 
diff --git a/spec/models/spree/stock/prioritizer_spec.rb b/spec/models/spree/stock/prioritizer_spec.rb
index aee367e7b7..4715da1ab8 100644
--- a/spec/models/spree/stock/prioritizer_spec.rb
+++ b/spec/models/spree/stock/prioritizer_spec.rb
@@ -59,7 +59,7 @@ module Spree
       end
 
       it '1st has some, 2nd has remaining' do
-        order.line_items[0].stub(quantity: 5)
+        allow(order.line_items[0]).to receive_messages(quantity: 5)
         package1 = pack do |package|
           package.add variant1, 2, :on_hand
         end
@@ -76,7 +76,7 @@ module Spree
       end
 
       it '1st has backorder, 2nd has some' do
-        order.line_items[0].stub(quantity: 5)
+        allow(order.line_items[0]).to receive_messages(quantity: 5)
         package1 = pack do |package|
           package.add variant1, 5, :backordered
         end
@@ -93,7 +93,7 @@ module Spree
       end
 
       it '1st has backorder, 2nd has all' do
-        order.line_items[0].stub(quantity: 5)
+        allow(order.line_items[0]).to receive_messages(quantity: 5)
         package1 = pack do |package|
           package.add variant1, 3, :backordered
           package.add variant2, 1, :on_hand

From d505fc213109988f281c6c414adb5bc2fdcc12c3 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:04:00 +0100
Subject: [PATCH 18/47] Bring availability validator from spree_core

---
 .../spree/stock/availability_validator.rb     | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 app/models/spree/stock/availability_validator.rb

diff --git a/app/models/spree/stock/availability_validator.rb b/app/models/spree/stock/availability_validator.rb
new file mode 100644
index 0000000000..fe9bc19442
--- /dev/null
+++ b/app/models/spree/stock/availability_validator.rb
@@ -0,0 +1,25 @@
+module Spree
+  module Stock
+    class AvailabilityValidator < ActiveModel::Validator
+      def validate(line_item)
+        if shipment = line_item.target_shipment
+          units = shipment.inventory_units_for(line_item.variant)
+          return if units.count > line_item.quantity
+          quantity = line_item.quantity - units.count
+        else
+          quantity = line_item.quantity
+        end
+
+        quantifier = Stock::Quantifier.new(line_item.variant_id)
+
+        unless quantifier.can_supply? quantity
+          variant = line_item.variant
+          display_name = %Q{#{variant.name}}
+          display_name += %Q{ (#{variant.options_text})} unless variant.options_text.blank?
+
+          line_item.errors[:quantity] << Spree.t(:out_of_stock, :scope => :order_populator, :item => display_name.inspect)
+        end
+      end
+    end
+  end
+end

From 3e14c9777eb997fd54c6bbbaf25fe00d1992a91b Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:05:48 +0100
Subject: [PATCH 19/47] Merge decorator with class brought from spree_core

---
 .../spree/stock/availability_validator.rb     | 56 ++++++++++++++-----
 .../stock/availability_validator_decorator.rb | 49 ----------------
 2 files changed, 42 insertions(+), 63 deletions(-)
 delete mode 100644 app/models/spree/stock/availability_validator_decorator.rb

diff --git a/app/models/spree/stock/availability_validator.rb b/app/models/spree/stock/availability_validator.rb
index fe9bc19442..1ca304e8d0 100644
--- a/app/models/spree/stock/availability_validator.rb
+++ b/app/models/spree/stock/availability_validator.rb
@@ -2,23 +2,51 @@ module Spree
   module Stock
     class AvailabilityValidator < ActiveModel::Validator
       def validate(line_item)
-        if shipment = line_item.target_shipment
-          units = shipment.inventory_units_for(line_item.variant)
-          return if units.count > line_item.quantity
-          quantity = line_item.quantity - units.count
-        else
-          quantity = line_item.quantity
-        end
+        # OFN specific check for in-memory :skip_stock_check attribute
+        return if line_item.skip_stock_check
 
-        quantifier = Stock::Quantifier.new(line_item.variant_id)
+        quantity_to_validate = line_item.quantity - quantity_in_shipment(line_item)
+        return if quantity_to_validate < 1
 
-        unless quantifier.can_supply? quantity
-          variant = line_item.variant
-          display_name = %Q{#{variant.name}}
-          display_name += %Q{ (#{variant.options_text})} unless variant.options_text.blank?
+        validate_quantity(line_item, quantity_to_validate)
+      end
 
-          line_item.errors[:quantity] << Spree.t(:out_of_stock, :scope => :order_populator, :item => display_name.inspect)
-        end
+      private
+
+      # This is an adapted version of a fix to the inventory_units not being considered here.
+      # See #3090 for details.
+      # This can be removed after upgrading to Spree 2.4.
+      def quantity_in_shipment(line_item)
+        shipment = line_item_shipment(line_item)
+        return 0 unless shipment
+
+        units = shipment.inventory_units_for(line_item.variant)
+        units.count
+      end
+
+      def line_item_shipment(line_item)
+        return line_item.target_shipment if line_item.target_shipment
+        return line_item.order.shipments.first if line_item.order.andand.shipments.any?
+      end
+
+      # Overrides Spree v2.0.4 validate method version to:
+      #   - scope variants to hub and thus acivate variant overrides
+      #   - use calculated quantity instead of the line_item.quantity
+      #   - rely on Variant.can_supply? instead of Stock::Quantified.can_supply?
+      #       so that it works correctly for variant overrides
+      def validate_quantity(line_item, quantity)
+        line_item.scoper.scope(line_item.variant)
+
+        add_out_of_stock_error(line_item) unless line_item.variant.can_supply? quantity
+      end
+
+      def add_out_of_stock_error(line_item)
+        variant = line_item.variant
+        display_name = variant.name.to_s
+        display_name += %{(#{variant.options_text})} if variant.options_text.present?
+        line_item.errors[:quantity] << Spree.t(:out_of_stock,
+                                               scope: :order_populator,
+                                               item: display_name.inspect)
       end
     end
   end
diff --git a/app/models/spree/stock/availability_validator_decorator.rb b/app/models/spree/stock/availability_validator_decorator.rb
deleted file mode 100644
index c3ee4c4818..0000000000
--- a/app/models/spree/stock/availability_validator_decorator.rb
+++ /dev/null
@@ -1,49 +0,0 @@
-Spree::Stock::AvailabilityValidator.class_eval do
-  def validate(line_item)
-    # OFN specific check for in-memory :skip_stock_check attribute
-    return if line_item.skip_stock_check
-
-    quantity_to_validate = line_item.quantity - quantity_in_shipment(line_item)
-    return if quantity_to_validate < 1
-
-    validate_quantity(line_item, quantity_to_validate)
-  end
-
-  private
-
-  # This is an adapted version of a fix to the inventory_units not being considered here.
-  # See #3090 for details.
-  # This can be removed after upgrading to Spree 2.4.
-  def quantity_in_shipment(line_item)
-    shipment = line_item_shipment(line_item)
-    return 0 unless shipment
-
-    units = shipment.inventory_units_for(line_item.variant)
-    units.count
-  end
-
-  def line_item_shipment(line_item)
-    return line_item.target_shipment if line_item.target_shipment
-    return line_item.order.shipments.first if line_item.order.andand.shipments.any?
-  end
-
-  # Overrides Spree v2.0.4 validate method version to:
-  #   - scope variants to hub and thus acivate variant overrides
-  #   - use calculated quantity instead of the line_item.quantity
-  #   - rely on Variant.can_supply? instead of Stock::Quantified.can_supply?
-  #       so that it works correctly for variant overrides
-  def validate_quantity(line_item, quantity)
-    line_item.scoper.scope(line_item.variant)
-
-    add_out_of_stock_error(line_item) unless line_item.variant.can_supply? quantity
-  end
-
-  def add_out_of_stock_error(line_item)
-    variant = line_item.variant
-    display_name = variant.name.to_s
-    display_name += %{(#{variant.options_text})} if variant.options_text.present?
-    line_item.errors[:quantity] << Spree.t(:out_of_stock,
-                                           scope: :order_populator,
-                                           item: display_name.inspect)
-  end
-end

From 0ca8b6aab60af99901bc8e2caaecd8d06ebecbe8 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:06:55 +0100
Subject: [PATCH 20/47] Fix easy rubocop issue

---
 app/models/spree/stock/availability_validator.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/spree/stock/availability_validator.rb b/app/models/spree/stock/availability_validator.rb
index 1ca304e8d0..6c9516cd08 100644
--- a/app/models/spree/stock/availability_validator.rb
+++ b/app/models/spree/stock/availability_validator.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   module Stock
     class AvailabilityValidator < ActiveModel::Validator

From b7255130b6130c4da8bf9126e77542ff7f83be3f Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:18:07 +0100
Subject: [PATCH 21/47] Bring Package from spree_core

---
 app/models/spree/stock/package.rb       | 116 ++++++++++++++++++++++++
 spec/models/spree/stock/package_spec.rb | 110 ++++++++++++++++++++++
 2 files changed, 226 insertions(+)
 create mode 100644 app/models/spree/stock/package.rb
 create mode 100644 spec/models/spree/stock/package_spec.rb

diff --git a/app/models/spree/stock/package.rb b/app/models/spree/stock/package.rb
new file mode 100644
index 0000000000..68ef8837ab
--- /dev/null
+++ b/app/models/spree/stock/package.rb
@@ -0,0 +1,116 @@
+module Spree
+  module Stock
+    class Package
+      ContentItem = Struct.new(:variant, :quantity, :state)
+
+      attr_reader :stock_location, :order, :contents
+      attr_accessor :shipping_rates
+
+      def initialize(stock_location, order, contents=[])
+        @stock_location = stock_location
+        @order = order
+        @contents = contents
+        @shipping_rates = Array.new
+      end
+
+      def add(variant, quantity, state=:on_hand)
+        contents << ContentItem.new(variant, quantity, state)
+      end
+
+      def weight
+        contents.sum { |item| item.variant.weight * item.quantity }
+      end
+
+      def on_hand
+        contents.select { |item| item.state == :on_hand }
+      end
+
+      def backordered
+        contents.select { |item| item.state == :backordered }
+      end
+
+      def find_item(variant, state=:on_hand)
+        contents.select do |item|
+          item.variant == variant &&
+          item.state == state
+        end.first
+      end
+
+      def quantity(state=nil)
+        case state
+        when :on_hand
+          on_hand.sum { |item| item.quantity }
+        when :backordered
+          backordered.sum { |item| item.quantity }
+        else
+          contents.sum { |item| item.quantity }
+        end
+      end
+
+      def empty?
+        quantity == 0
+      end
+
+      def flattened
+        flat = []
+        contents.each do |item|
+          item.quantity.times do
+            flat << ContentItem.new(item.variant, 1, item.state)
+          end
+        end
+        flat
+      end
+
+      def flattened=(flattened)
+        contents.clear
+        flattened.each do |item|
+          current_item = find_item(item.variant, item.state)
+          if current_item
+            current_item.quantity += 1
+          else
+            add(item.variant, item.quantity, item.state)
+          end
+        end
+      end
+
+      def currency
+        #TODO calculate from first variant?
+      end
+
+      def shipping_categories
+        contents.map { |item| item.variant.shipping_category }.compact.uniq
+      end
+
+      def shipping_methods
+        shipping_categories.map { |sc| sc.shipping_methods }.flatten.uniq
+      end
+
+      def inspect
+        out = "#{order} - "
+        out << contents.map do |content_item|
+          "#{content_item.variant.name} #{content_item.quantity} #{content_item.state}"
+        end.join('/')
+        out
+      end
+
+      def to_shipment
+        shipment = Spree::Shipment.new
+        shipment.order = order
+        shipment.stock_location = stock_location
+        shipment.shipping_rates = shipping_rates
+
+        contents.each do |item|
+          item.quantity.times do |n|
+            unit = shipment.inventory_units.build
+            unit.pending = true
+            unit.order = order
+            unit.variant = item.variant
+            unit.state = item.state.to_s
+          end
+        end
+
+        shipment
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/stock/package_spec.rb b/spec/models/spree/stock/package_spec.rb
new file mode 100644
index 0000000000..532ce23dae
--- /dev/null
+++ b/spec/models/spree/stock/package_spec.rb
@@ -0,0 +1,110 @@
+require 'spec_helper'
+
+module Spree
+  module Stock
+    describe Package do
+      let(:variant) { build(:variant, weight: 25.0) }
+      let(:stock_location) { build(:stock_location) }
+      let(:order) { build(:order) }
+
+      subject { Package.new(stock_location, order) }
+
+      it 'calculates the weight of all the contents' do
+        subject.add variant, 4
+        subject.weight.should == 100.0
+      end
+
+      it 'filters by on_hand and backordered' do
+        subject.add variant, 4, :on_hand
+        subject.add variant, 3, :backordered
+        subject.on_hand.count.should eq 1
+        subject.backordered.count.should eq 1
+      end
+
+      it 'calculates the quantity by state' do
+        subject.add variant, 4, :on_hand
+        subject.add variant, 3, :backordered
+
+        subject.quantity.should eq 7
+        subject.quantity(:on_hand).should eq 4
+        subject.quantity(:backordered).should eq 3
+      end
+
+      it 'returns nil for content item not found' do
+        item = subject.find_item(variant, :on_hand)
+        item.should be_nil
+      end
+
+      it 'finds content item for a variant' do
+        subject.add variant, 4, :on_hand
+        item = subject.find_item(variant, :on_hand)
+        item.quantity.should eq 4
+      end
+
+      it 'get flattened contents' do
+        subject.add variant, 4, :on_hand
+        subject.add variant, 2, :backordered
+        flattened = subject.flattened
+        flattened.select { |i| i.state == :on_hand }.size.should eq 4
+        flattened.select { |i| i.state == :backordered }.size.should eq 2
+      end
+
+      it 'set contents from flattened' do
+        flattened = [Package::ContentItem.new(variant, 1, :on_hand),
+                    Package::ContentItem.new(variant, 1, :on_hand),
+                    Package::ContentItem.new(variant, 1, :backordered),
+                    Package::ContentItem.new(variant, 1, :backordered)]
+
+        subject.flattened = flattened
+        subject.on_hand.size.should eq 1
+        subject.on_hand.first.quantity.should eq 2
+
+        subject.backordered.size.should eq 1
+      end
+
+      # Contains regression test for #2804
+      it 'builds a list of shipping methods from all categories' do
+        shipping_method1 = create(:shipping_method)
+        shipping_method2 = create(:shipping_method)
+        variant1 = mock_model(Variant, shipping_category: shipping_method1.shipping_categories.first)
+        variant2 = mock_model(Variant, shipping_category: shipping_method2.shipping_categories.first)
+        variant3 = mock_model(Variant, shipping_category: nil)
+        contents = [Package::ContentItem.new(variant1, 1),
+                    Package::ContentItem.new(variant1, 1),
+                    Package::ContentItem.new(variant2, 1),
+                    Package::ContentItem.new(variant3, 1)]
+
+        package = Package.new(stock_location, order, contents)
+        package.shipping_methods.size.should eq 2
+      end
+
+
+      it "can convert to a shipment" do
+        flattened = [Package::ContentItem.new(variant, 2, :on_hand),
+                    Package::ContentItem.new(variant, 1, :backordered)]
+        subject.flattened = flattened
+
+        shipping_method = build(:shipping_method)
+        subject.shipping_rates = [ Spree::ShippingRate.new(shipping_method: shipping_method, cost: 10.00, selected: true) ]
+
+        shipment = subject.to_shipment
+        shipment.order.should == subject.order
+        shipment.stock_location.should == subject.stock_location
+        shipment.inventory_units.size.should == 3
+
+        first_unit = shipment.inventory_units.first
+        first_unit.variant.should == variant
+        first_unit.state.should == 'on_hand'
+        first_unit.order.should == subject.order
+        first_unit.should be_pending
+
+        last_unit = shipment.inventory_units.last
+        last_unit.variant.should == variant
+        last_unit.state.should == 'backordered'
+        last_unit.order.should == subject.order
+
+        shipment.shipping_method.should eq shipping_method
+      end
+    end
+  end
+end

From bdb40d68e99ca4c8df7fe65b65cb422facef5bac Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:22:15 +0100
Subject: [PATCH 22/47] Fix easy rubocop issues

---
 app/models/spree/stock/package.rb       | 28 +++++++++---------
 spec/models/spree/stock/package_spec.rb | 39 ++++++++++++++-----------
 2 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/app/models/spree/stock/package.rb b/app/models/spree/stock/package.rb
index 68ef8837ab..e724b3efa1 100644
--- a/app/models/spree/stock/package.rb
+++ b/app/models/spree/stock/package.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   module Stock
     class Package
@@ -6,14 +8,14 @@ module Spree
       attr_reader :stock_location, :order, :contents
       attr_accessor :shipping_rates
 
-      def initialize(stock_location, order, contents=[])
+      def initialize(stock_location, order, contents = [])
         @stock_location = stock_location
         @order = order
         @contents = contents
-        @shipping_rates = Array.new
+        @shipping_rates = []
       end
 
-      def add(variant, quantity, state=:on_hand)
+      def add(variant, quantity, state = :on_hand)
         contents << ContentItem.new(variant, quantity, state)
       end
 
@@ -29,26 +31,26 @@ module Spree
         contents.select { |item| item.state == :backordered }
       end
 
-      def find_item(variant, state=:on_hand)
+      def find_item(variant, state = :on_hand)
         contents.select do |item|
           item.variant == variant &&
-          item.state == state
+            item.state == state
         end.first
       end
 
-      def quantity(state=nil)
+      def quantity(state = nil)
         case state
         when :on_hand
-          on_hand.sum { |item| item.quantity }
+          on_hand.sum(&:quantity)
         when :backordered
-          backordered.sum { |item| item.quantity }
+          backordered.sum(&:quantity)
         else
-          contents.sum { |item| item.quantity }
+          contents.sum(&:quantity)
         end
       end
 
       def empty?
-        quantity == 0
+        quantity.zero?
       end
 
       def flattened
@@ -74,7 +76,7 @@ module Spree
       end
 
       def currency
-        #TODO calculate from first variant?
+        # TODO calculate from first variant?
       end
 
       def shipping_categories
@@ -82,7 +84,7 @@ module Spree
       end
 
       def shipping_methods
-        shipping_categories.map { |sc| sc.shipping_methods }.flatten.uniq
+        shipping_categories.map(&:shipping_methods).flatten.uniq
       end
 
       def inspect
@@ -100,7 +102,7 @@ module Spree
         shipment.shipping_rates = shipping_rates
 
         contents.each do |item|
-          item.quantity.times do |n|
+          item.quantity.times do
             unit = shipment.inventory_units.build
             unit.pending = true
             unit.order = order
diff --git a/spec/models/spree/stock/package_spec.rb b/spec/models/spree/stock/package_spec.rb
index 532ce23dae..9059f10098 100644
--- a/spec/models/spree/stock/package_spec.rb
+++ b/spec/models/spree/stock/package_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 module Spree
@@ -51,9 +53,9 @@ module Spree
 
       it 'set contents from flattened' do
         flattened = [Package::ContentItem.new(variant, 1, :on_hand),
-                    Package::ContentItem.new(variant, 1, :on_hand),
-                    Package::ContentItem.new(variant, 1, :backordered),
-                    Package::ContentItem.new(variant, 1, :backordered)]
+                     Package::ContentItem.new(variant, 1, :on_hand),
+                     Package::ContentItem.new(variant, 1, :backordered),
+                     Package::ContentItem.new(variant, 1, :backordered)]
 
         subject.flattened = flattened
         subject.on_hand.size.should eq 1
@@ -66,8 +68,10 @@ module Spree
       it 'builds a list of shipping methods from all categories' do
         shipping_method1 = create(:shipping_method)
         shipping_method2 = create(:shipping_method)
-        variant1 = mock_model(Variant, shipping_category: shipping_method1.shipping_categories.first)
-        variant2 = mock_model(Variant, shipping_category: shipping_method2.shipping_categories.first)
+        variant1 = mock_model(Variant,
+                              shipping_category: shipping_method1.shipping_categories.first)
+        variant2 = mock_model(Variant,
+                              shipping_category: shipping_method2.shipping_categories.first)
         variant3 = mock_model(Variant, shipping_category: nil)
         contents = [Package::ContentItem.new(variant1, 1),
                     Package::ContentItem.new(variant1, 1),
@@ -78,30 +82,31 @@ module Spree
         package.shipping_methods.size.should eq 2
       end
 
-
       it "can convert to a shipment" do
         flattened = [Package::ContentItem.new(variant, 2, :on_hand),
-                    Package::ContentItem.new(variant, 1, :backordered)]
+                     Package::ContentItem.new(variant, 1, :backordered)]
         subject.flattened = flattened
 
         shipping_method = build(:shipping_method)
-        subject.shipping_rates = [ Spree::ShippingRate.new(shipping_method: shipping_method, cost: 10.00, selected: true) ]
+        subject.shipping_rates = [
+          Spree::ShippingRate.new(shipping_method: shipping_method, cost: 10.00, selected: true)
+        ]
 
         shipment = subject.to_shipment
-        shipment.order.should == subject.order
-        shipment.stock_location.should == subject.stock_location
-        shipment.inventory_units.size.should == 3
+        expect(shipment.order).to eq subject.order
+        expect(shipment.stock_location).to eq subject.stock_location
+        expect(shipment.inventory_units.size).to eq 3
 
         first_unit = shipment.inventory_units.first
-        first_unit.variant.should == variant
-        first_unit.state.should == 'on_hand'
-        first_unit.order.should == subject.order
+        expect(first_unit.variant).to eq variant
+        expect(first_unit.state).to eq 'on_hand'
+        expect(first_unit.order).to eq subject.order
         first_unit.should be_pending
 
         last_unit = shipment.inventory_units.last
-        last_unit.variant.should == variant
-        last_unit.state.should == 'backordered'
-        last_unit.order.should == subject.order
+        expect(last_unit.variant).to eq variant
+        expect(last_unit.state).to eq 'backordered'
+        expect(last_unit.order).to eq subject.order
 
         shipment.shipping_method.should eq shipping_method
       end

From 55a4021157ba7c4ae15614a268ed9e466e9305da Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:25:19 +0100
Subject: [PATCH 23/47] Convert to modern rspec syntax

---
 spec/models/spree/stock/package_spec.rb | 42 ++++++++++++-------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/spec/models/spree/stock/package_spec.rb b/spec/models/spree/stock/package_spec.rb
index 9059f10098..6b27b405ac 100644
--- a/spec/models/spree/stock/package_spec.rb
+++ b/spec/models/spree/stock/package_spec.rb
@@ -13,42 +13,42 @@ module Spree
 
       it 'calculates the weight of all the contents' do
         subject.add variant, 4
-        subject.weight.should == 100.0
+        expect(subject.weight).to eq 100.0
       end
 
       it 'filters by on_hand and backordered' do
         subject.add variant, 4, :on_hand
         subject.add variant, 3, :backordered
-        subject.on_hand.count.should eq 1
-        subject.backordered.count.should eq 1
+        expect(subject.on_hand.count).to eq 1
+        expect(subject.backordered.count).to eq 1
       end
 
       it 'calculates the quantity by state' do
         subject.add variant, 4, :on_hand
         subject.add variant, 3, :backordered
 
-        subject.quantity.should eq 7
-        subject.quantity(:on_hand).should eq 4
-        subject.quantity(:backordered).should eq 3
+        expect(subject.quantity).to eq 7
+        expect(subject.quantity(:on_hand)).to eq 4
+        expect(subject.quantity(:backordered)).to eq 3
       end
 
       it 'returns nil for content item not found' do
         item = subject.find_item(variant, :on_hand)
-        item.should be_nil
+        expect(item).to be_nil
       end
 
       it 'finds content item for a variant' do
         subject.add variant, 4, :on_hand
         item = subject.find_item(variant, :on_hand)
-        item.quantity.should eq 4
+        expect(item.quantity).to eq 4
       end
 
       it 'get flattened contents' do
         subject.add variant, 4, :on_hand
         subject.add variant, 2, :backordered
         flattened = subject.flattened
-        flattened.select { |i| i.state == :on_hand }.size.should eq 4
-        flattened.select { |i| i.state == :backordered }.size.should eq 2
+        expect(flattened.select { |i| i.state == :on_hand }.size).to eq 4
+        expect(flattened.select { |i| i.state == :backordered }.size).to eq 2
       end
 
       it 'set contents from flattened' do
@@ -58,28 +58,28 @@ module Spree
                      Package::ContentItem.new(variant, 1, :backordered)]
 
         subject.flattened = flattened
-        subject.on_hand.size.should eq 1
-        subject.on_hand.first.quantity.should eq 2
+        expect(subject.on_hand.size).to eq 1
+        expect(subject.on_hand.first.quantity).to eq 2
 
-        subject.backordered.size.should eq 1
+        expect(subject.backordered.size).to eq 1
       end
 
       # Contains regression test for #2804
       it 'builds a list of shipping methods from all categories' do
         shipping_method1 = create(:shipping_method)
         shipping_method2 = create(:shipping_method)
-        variant1 = mock_model(Variant,
-                              shipping_category: shipping_method1.shipping_categories.first)
-        variant2 = mock_model(Variant,
-                              shipping_category: shipping_method2.shipping_categories.first)
-        variant3 = mock_model(Variant, shipping_category: nil)
+        variant1 = create(:variant,
+                          shipping_category: shipping_method1.shipping_categories.first)
+        variant2 = create(:variant,
+                          shipping_category: shipping_method2.shipping_categories.first)
+        variant3 = create(:variant, shipping_category: nil)
         contents = [Package::ContentItem.new(variant1, 1),
                     Package::ContentItem.new(variant1, 1),
                     Package::ContentItem.new(variant2, 1),
                     Package::ContentItem.new(variant3, 1)]
 
         package = Package.new(stock_location, order, contents)
-        package.shipping_methods.size.should eq 2
+        expect(package.shipping_methods.size).to eq 2
       end
 
       it "can convert to a shipment" do
@@ -101,14 +101,14 @@ module Spree
         expect(first_unit.variant).to eq variant
         expect(first_unit.state).to eq 'on_hand'
         expect(first_unit.order).to eq subject.order
-        first_unit.should be_pending
+        expect(first_unit).to be_pending
 
         last_unit = shipment.inventory_units.last
         expect(last_unit.variant).to eq variant
         expect(last_unit.state).to eq 'backordered'
         expect(last_unit.order).to eq subject.order
 
-        shipment.shipping_method.should eq shipping_method
+        expect(shipment.shipping_method).to eq shipping_method
       end
     end
   end

From 4e5259f4911f3c49171e1cf6cc84ce8473695c10 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:29:18 +0100
Subject: [PATCH 24/47] Bring shipment from spree_core

---
 app/models/spree/shipment.rb       | 290 ++++++++++++++++++
 spec/models/spree/shipment_spec.rb | 476 +++++++++++++++++++++++++++--
 2 files changed, 745 insertions(+), 21 deletions(-)
 create mode 100644 app/models/spree/shipment.rb

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
new file mode 100644
index 0000000000..b49f8f5bdc
--- /dev/null
+++ b/app/models/spree/shipment.rb
@@ -0,0 +1,290 @@
+require 'ostruct'
+
+module Spree
+  class Shipment < ActiveRecord::Base
+    belongs_to :order, class_name: 'Spree::Order'
+    belongs_to :address, class_name: 'Spree::Address'
+    belongs_to :stock_location, class_name: 'Spree::StockLocation'
+
+    has_many :shipping_rates, dependent: :delete_all
+    has_many :shipping_methods, through: :shipping_rates
+    has_many :state_changes, as: :stateful
+    has_many :inventory_units, dependent: :delete_all
+    has_one :adjustment, as: :source, dependent: :destroy
+
+    before_create :generate_shipment_number
+    after_save :ensure_correct_adjustment, :update_order
+
+    attr_accessor :special_instructions
+
+    accepts_nested_attributes_for :address
+    accepts_nested_attributes_for :inventory_units
+
+    make_permalink field: :number
+
+    scope :shipped, -> { with_state('shipped') }
+    scope :ready,   -> { with_state('ready') }
+    scope :pending, -> { with_state('pending') }
+    scope :with_state, ->(*s) { where(state: s) }
+    scope :trackable, -> { where("tracking IS NOT NULL AND tracking != ''") }
+
+    # shipment state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
+    state_machine initial: :pending, use_transactions: false do
+      event :ready do
+        transition from: :pending, to: :ready, if: lambda { |shipment|
+          # Fix for #2040
+          shipment.determine_state(shipment.order) == 'ready'
+        }
+      end
+
+      event :pend do
+        transition from: :ready, to: :pending
+      end
+
+      event :ship do
+        transition from: :ready, to: :shipped
+      end
+      after_transition to: :shipped, do: :after_ship
+
+      event :cancel do
+        transition to: :canceled, from: [:pending, :ready]
+      end
+      after_transition to: :canceled, do: :after_cancel
+
+      event :resume do
+        transition from: :canceled, to: :ready, if: lambda { |shipment|
+          shipment.determine_state(shipment.order) == :ready
+        }
+        transition from: :canceled, to: :pending, if: lambda { |shipment|
+          shipment.determine_state(shipment.order) == :ready
+        }
+        transition from: :canceled, to: :pending
+      end
+      after_transition from: :canceled, to: [:pending, :ready], do: :after_resume
+    end
+
+    def to_param
+      number if number
+      generate_shipment_number unless number
+      number.to_s.to_url.upcase
+    end
+
+    def backordered?
+      inventory_units.any? { |inventory_unit| inventory_unit.backordered? }
+    end
+
+    def shipped=(value)
+      return unless value == '1' && shipped_at.nil?
+      self.shipped_at = Time.now
+    end
+
+    def shipping_method
+      selected_shipping_rate.try(:shipping_method) || shipping_rates.first.try(:shipping_method)
+    end
+
+    def add_shipping_method(shipping_method, selected = false)
+      shipping_rates.create(shipping_method: shipping_method, selected: selected)
+    end
+
+    def selected_shipping_rate
+      shipping_rates.where(selected: true).first
+    end
+
+    def selected_shipping_rate_id
+      selected_shipping_rate.try(:id)
+    end
+
+    def selected_shipping_rate_id=(id)
+      shipping_rates.update_all(selected: false)
+      shipping_rates.update(id, selected: true)
+      self.save!
+    end
+
+    def refresh_rates
+      return shipping_rates if shipped?
+
+      self.shipping_rates = Stock::Estimator.new(order).shipping_rates(to_package)
+
+      if shipping_method
+        selected_rate = shipping_rates.detect { |rate|
+          rate.shipping_method_id == shipping_method.id
+        }
+        self.selected_shipping_rate_id = selected_rate.id if selected_rate
+      end
+
+      shipping_rates
+    end
+
+    def currency
+      order ? order.currency : Spree::Config[:currency]
+    end
+
+    # The adjustment amount associated with this shipment (if any.)  Returns only the first adjustment to match
+    # the shipment but there should never really be more than one.
+    def cost
+      adjustment ? adjustment.amount : 0
+    end
+
+    alias_method :amount, :cost
+
+    def display_cost
+      Spree::Money.new(cost, { currency: currency })
+    end
+
+    alias_method :display_amount, :display_cost
+
+    def item_cost
+      line_items.map(&:amount).sum
+    end
+
+    def display_item_cost
+      Spree::Money.new(item_cost, { currency: currency })
+    end
+
+    def total_cost
+      cost + item_cost
+    end
+
+    def display_total_cost
+      Spree::Money.new(total_cost, { currency: currency })
+    end
+
+    def editable_by?(user)
+      !shipped?
+    end
+
+    def manifest
+      inventory_units.joins(:variant).includes(:variant).group_by(&:variant).map do |variant, units|
+        states = {}
+        units.group_by(&:state).each { |state, iu| states[state] = iu.count }
+        OpenStruct.new(variant: variant, quantity: units.length, states: states)
+      end
+    end
+
+    def line_items
+      if order.complete? and Spree::Config[:track_inventory_levels]
+        order.line_items.select { |li| inventory_units.pluck(:variant_id).include?(li.variant_id) }
+      else
+        order.line_items
+      end
+    end
+
+    def finalize!
+      InventoryUnit.finalize_units!(inventory_units)
+      manifest.each { |item| manifest_unstock(item) }
+    end
+
+    def after_cancel
+      manifest.each { |item| manifest_restock(item) }
+    end
+
+    def after_resume
+      manifest.each { |item| manifest_unstock(item) }
+    end
+
+    # Updates various aspects of the Shipment while bypassing any callbacks.  Note that this method takes an explicit reference to the
+    # Order object.  This is necessary because the association actually has a stale (and unsaved) copy of the Order and so it will not
+    # yield the correct results.
+    def update!(order)
+      old_state = state
+      new_state = determine_state(order)
+      update_column :state, new_state
+      after_ship if new_state == 'shipped' and old_state != 'shipped'
+    end
+
+    # Determines the appropriate +state+ according to the following logic:
+    #
+    # pending    unless order is complete and +order.payment_state+ is +paid+
+    # shipped    if already shipped (ie. does not change the state)
+    # ready      all other cases
+    def determine_state(order)
+      return 'canceled' if order.canceled?
+      return 'pending' unless order.can_ship?
+      return 'pending' if inventory_units.any? &:backordered?
+      return 'shipped' if state == 'shipped'
+      order.paid? ? 'ready' : 'pending'
+    end
+
+    def tracking_url
+      @tracking_url ||= shipping_method.build_tracking_url(tracking)
+    end
+
+    def include?(variant)
+      inventory_units_for(variant).present?
+    end
+
+    def inventory_units_for(variant)
+      inventory_units.group_by(&:variant_id)[variant.id] || []
+    end
+
+    def to_package
+      package = Spree::Config.package_factory.new(stock_location, order)
+      inventory_units.includes(:variant).each do |inventory_unit|
+        package.add inventory_unit.variant, 1, inventory_unit.state_name
+      end
+      package
+    end
+
+    def set_up_inventory(state, variant, order)
+      self.inventory_units.create(variant_id: variant.id, state: state, order_id: order.id)
+    end
+
+    private
+
+      def manifest_unstock(item)
+        stock_location.unstock item.variant, item.quantity, self
+      end
+
+      def manifest_restock(item)
+        stock_location.restock item.variant, item.quantity, self
+      end
+
+      def generate_shipment_number
+        return number unless number.blank?
+        record = true
+        while record
+          random = "H#{Array.new(11) { rand(9) }.join}"
+          record = self.class.where(number: random).first
+        end
+        self.number = random
+      end
+
+      def description_for_shipping_charge
+        "#{Spree.t(:shipping)} (#{shipping_method.name})"
+      end
+
+      def validate_shipping_method
+        unless shipping_method.nil?
+          errors.add :shipping_method, Spree.t(:is_not_available_to_shipment_address) unless shipping_method.include?(address)
+        end
+      end
+
+      def after_ship
+        inventory_units.each &:ship!
+        adjustment.finalize!
+        send_shipped_email
+        touch :shipped_at
+      end
+
+      def send_shipped_email
+        ShipmentMailer.shipped_email(self.id).deliver
+      end
+
+      def ensure_correct_adjustment
+        if adjustment
+          adjustment.originator = shipping_method
+          adjustment.label = shipping_method.adjustment_label
+          adjustment.amount = selected_shipping_rate.cost if adjustment.open?
+          adjustment.save!
+          adjustment.reload
+        elsif selected_shipping_rate_id
+          shipping_method.create_adjustment shipping_method.adjustment_label, order, self, true, "open"
+          reload #ensure adjustment is present on later saves
+        end
+      end
+
+      def update_order
+        order.update!
+      end
+  end
+end
diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index b37dd498ff..a9992c89c5 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -1,33 +1,467 @@
-require "spec_helper"
+require 'spec_helper'
+require 'benchmark'
 
 describe Spree::Shipment do
-  describe "manifest" do
-    let!(:product) { create(:product) }
-    let!(:order) { create(:order, distributor: product.supplier) }
-    let!(:deleted_variant) { create(:variant, product: product) }
-    let!(:other_variant) { create(:variant, product: product) }
-    let!(:line_item_for_deleted) { create(:line_item, order: order, variant: deleted_variant) }
-    let!(:line_item_for_other) { create(:line_item, order: order, variant: other_variant) }
-    let!(:shipment) { create(:shipment_with, :shipping_method, order: order) }
+  let(:order) { mock_model Spree::Order, backordered?: false,
+                                         canceled?: false,
+                                         can_ship?: true,
+                                         currency: 'USD' }
+  let(:shipping_method) { create(:shipping_method, name: "UPS") }
+  let(:shipment) do
+    shipment = Spree::Shipment.new order: order
+    shipment.stub(shipping_method: shipping_method)
+    shipment.state = 'pending'
+    shipment
+  end
 
-    context "when the variant is soft-deleted" do
-      before { deleted_variant.delete }
+  let(:charge) { create(:adjustment) }
+  let(:variant) { mock_model(Spree::Variant) }
 
-      it "can still access the variant" do
-        shipment.reload
-        variants = shipment.manifest.map(&:variant).uniq
-        expect(variants.sort_by(&:id)).to eq([deleted_variant, other_variant].sort_by(&:id))
+  it 'is backordered if one if its inventory_units is backordered' do
+    shipment.stub(inventory_units: [
+      mock_model(Spree::InventoryUnit, backordered?: false),
+      mock_model(Spree::InventoryUnit, backordered?: true)
+    ])
+    shipment.should be_backordered
+  end
+
+  context "#cost" do
+    it "should return the amount of any shipping charges that it originated" do
+      shipment.stub_chain :adjustment, amount: 10
+      shipment.cost.should == 10
+    end
+
+    it "should return 0 if there are no relevant shipping adjustments" do
+      shipment.cost.should == 0
+    end
+  end
+
+  context "display_cost" do
+    it "retuns a Spree::Money" do
+      shipment.stub(:cost) { 21.22 }
+      shipment.display_cost.should == Spree::Money.new(21.22)
+    end
+  end
+
+  context "display_item_cost" do
+    it "retuns a Spree::Money" do
+      shipment.stub(:item_cost) { 21.22 }
+      shipment.display_item_cost.should == Spree::Money.new(21.22)
+    end
+  end
+
+  context "display_total_cost" do
+    it "retuns a Spree::Money" do
+      shipment.stub(:total_cost) { 21.22 }
+      shipment.display_total_cost.should == Spree::Money.new(21.22)
+    end
+  end
+
+  it "#item_cost" do
+    shipment = create(:shipment, order: create(:order_with_totals))
+    shipment.item_cost.should eql(10.0)
+  end
+
+  context "manifest" do
+    let(:order) { Spree::Order.create }
+    let(:variant) { create(:variant) }
+    let!(:line_item) { order.contents.add variant }
+    let!(:shipment) { order.create_proposed_shipments.first }
+
+    it "returns variant expected" do
+      expect(shipment.manifest.first.variant).to eq variant
+    end
+
+    context "variant was removed" do
+      before { variant.product.destroy }
+
+      it "still returns variant expected" do
+        expect(shipment.manifest.first.variant).to eq variant
       end
     end
 
-    context "when the product is soft-deleted" do
-      before { deleted_variant.product.delete }
+    describe "with soft-deleted products or variants" do
+      let!(:product) { create(:product) }
+      let!(:order) { create(:order, distributor: product.supplier) }
+      let!(:deleted_variant) { create(:variant, product: product) }
+      let!(:other_variant) { create(:variant, product: product) }
+      let!(:line_item_for_deleted) { create(:line_item, order: order, variant: deleted_variant) }
+      let!(:line_item_for_other) { create(:line_item, order: order, variant: other_variant) }
+      let!(:shipment) { create(:shipment_with, :shipping_method, order: order) }
 
-      it "can still access the variant" do
-        shipment.reload
-        variants = shipment.manifest.map(&:variant)
-        expect(variants.sort_by(&:id)).to eq([deleted_variant, other_variant].sort_by(&:id))
+      context "when the variant is soft-deleted" do
+        before { deleted_variant.delete }
+
+        it "can still access the variant" do
+          shipment.reload
+          variants = shipment.manifest.map(&:variant).uniq
+          expect(variants.sort_by(&:id)).to eq([deleted_variant, other_variant].sort_by(&:id))
+        end
+      end
+
+      context "when the product is soft-deleted" do
+        before { deleted_variant.product.delete }
+
+        it "can still access the variant" do
+          shipment.reload
+          variants = shipment.manifest.map(&:variant)
+          expect(variants.sort_by(&:id)).to eq([deleted_variant, other_variant].sort_by(&:id))
+        end
       end
     end
   end
+
+  context 'shipping_rates' do
+    let(:shipment) { create(:shipment) }
+    let(:shipping_method1) { create(:shipping_method) }
+    let(:shipping_method2) { create(:shipping_method) }
+    let(:shipping_rates) { [
+      Spree::ShippingRate.new(shipping_method: shipping_method1, cost: 10.00, selected: true),
+      Spree::ShippingRate.new(shipping_method: shipping_method2, cost: 20.00)
+    ] }
+
+    it 'returns shipping_method from selected shipping_rate' do
+      shipment.shipping_rates.delete_all
+      shipment.shipping_rates.create shipping_method: shipping_method1, cost: 10.00, selected: true
+      shipment.shipping_method.should eq shipping_method1
+    end
+
+    context 'refresh_rates' do
+      let(:mock_estimator) { double('estimator', shipping_rates: shipping_rates) }
+
+      it 'should request new rates, and maintain shipping_method selection' do
+        Spree::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
+        shipment.stub(shipping_method: shipping_method2)
+
+        shipment.refresh_rates.should == shipping_rates
+        shipment.reload.selected_shipping_rate.shipping_method_id.should == shipping_method2.id
+      end
+
+      it 'should handle no shipping_method selection' do
+        Spree::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
+        shipment.stub(shipping_method: nil)
+        shipment.refresh_rates.should == shipping_rates
+        shipment.reload.selected_shipping_rate.should_not be_nil
+      end
+
+      it 'should not refresh if shipment is shipped' do
+        Spree::Stock::Estimator.should_not_receive(:new)
+        shipment.shipping_rates.delete_all
+        shipment.stub(shipped?: true)
+        shipment.refresh_rates.should == []
+      end
+
+      context 'to_package' do
+        it 'should use symbols for states when adding contents to package' do
+          shipment.stub_chain(:inventory_units, includes: [ build(:inventory_unit, variant: variant, state: 'on_hand'),
+                                                            build(:inventory_unit, variant: variant, state: 'backordered') ] )
+          package = shipment.to_package
+          package.on_hand.count.should eq 1
+          package.backordered.count.should eq 1
+        end
+      end
+    end
+  end
+
+  it '#total_cost' do
+    shipment.stub cost: 5.0
+    shipment.stub item_cost: 50.0
+    shipment.total_cost.should eql(55.0)
+  end
+
+  context "#update!" do
+    shared_examples_for "immutable once shipped" do
+      it "should remain in shipped state once shipped" do
+        shipment.state = 'shipped'
+        shipment.should_receive(:update_column).with(:state, 'shipped')
+        shipment.update!(order)
+      end
+    end
+
+    shared_examples_for "pending if backordered" do
+      it "should have a state of pending if backordered" do
+        shipment.stub(inventory_units: [mock_model(Spree::InventoryUnit, backordered?: true)])
+        shipment.should_receive(:update_column).with(:state, 'pending')
+        shipment.update!(order)
+      end
+    end
+
+    context "when order cannot ship" do
+      before { order.stub can_ship?: false }
+      it "should result in a 'pending' state" do
+        shipment.should_receive(:update_column).with(:state, 'pending')
+        shipment.update!(order)
+      end
+    end
+
+    context "when order is paid" do
+      before { order.stub paid?: true }
+      it "should result in a 'ready' state" do
+        shipment.should_receive(:update_column).with(:state, 'ready')
+        shipment.update!(order)
+      end
+      it_should_behave_like 'immutable once shipped'
+      it_should_behave_like 'pending if backordered'
+    end
+
+    context "when order has balance due" do
+      before { order.stub paid?: false }
+      it "should result in a 'pending' state" do
+        shipment.state = 'ready'
+        shipment.should_receive(:update_column).with(:state, 'pending')
+        shipment.update!(order)
+      end
+      it_should_behave_like 'immutable once shipped'
+      it_should_behave_like 'pending if backordered'
+    end
+
+    context "when order has a credit owed" do
+      before { order.stub payment_state: 'credit_owed', paid?: true }
+      it "should result in a 'ready' state" do
+        shipment.state = 'pending'
+        shipment.should_receive(:update_column).with(:state, 'ready')
+        shipment.update!(order)
+      end
+      it_should_behave_like 'immutable once shipped'
+      it_should_behave_like 'pending if backordered'
+    end
+
+    context "when shipment state changes to shipped" do
+      it "should call after_ship" do
+        shipment.state = 'pending'
+        shipment.should_receive :after_ship
+        shipment.stub determine_state: 'shipped'
+        shipment.should_receive(:update_column).with(:state, 'shipped')
+        shipment.update!(order)
+      end
+    end
+  end
+
+  context "when track_inventory is false" do
+    before { Spree::Config.set track_inventory_levels: false }
+    after { Spree::Config.set track_inventory_levels: true }
+
+    it "should not use the line items from order when track_inventory_levels is false" do
+      line_items = [mock_model(Spree::LineItem)]
+      order.stub complete?: true
+      order.stub line_items: line_items
+      shipment.line_items.should == line_items
+    end
+  end
+
+  context "when order is completed" do
+    after { Spree::Config.set track_inventory_levels: true }
+
+    before do
+      order.stub completed?: true
+      order.stub canceled?: false
+    end
+
+    context "with inventory tracking" do
+      before { Spree::Config.set track_inventory_levels: true }
+
+      it "should validate with inventory" do
+        shipment.inventory_units = [create(:inventory_unit)]
+        shipment.valid?.should be_true
+      end
+    end
+
+    context "without inventory tracking" do
+      before { Spree::Config.set track_inventory_levels: false }
+
+      it "should validate with no inventory" do
+        shipment.valid?.should be_true
+      end
+    end
+  end
+
+  context "#cancel" do
+    it 'cancels the shipment' do
+      shipment.stub(:ensure_correct_adjustment)
+      shipment.order.stub(:update!)
+
+      shipment.state = 'pending'
+      shipment.should_receive(:after_cancel)
+      shipment.cancel!
+      shipment.state.should eq 'canceled'
+    end
+
+    it 'restocks the items' do
+      shipment.stub_chain(:inventory_units, :joins, includes: [mock_model(Spree::InventoryUnit, variant: variant)])
+      shipment.stock_location = mock_model(Spree::StockLocation)
+      shipment.stock_location.should_receive(:restock).with(variant, 1, shipment)
+      shipment.after_cancel
+    end
+  end
+
+  context "#resume" do
+    it 'will determine new state based on order' do
+      shipment.stub(:ensure_correct_adjustment)
+      shipment.order.stub(:update!)
+
+      shipment.state = 'canceled'
+      shipment.should_receive(:determine_state).and_return(:ready)
+      shipment.should_receive(:after_resume)
+      shipment.resume!
+      shipment.state.should eq 'ready'
+    end
+
+    it 'unstocks them items' do
+      shipment.stub_chain(:inventory_units, :joins, includes: [mock_model(Spree::InventoryUnit, variant: variant)])
+      shipment.stock_location = mock_model(Spree::StockLocation)
+      shipment.stock_location.should_receive(:unstock).with(variant, 1, shipment)
+      shipment.after_resume
+    end
+
+    it 'will determine new state based on order' do
+      shipment.stub(:ensure_correct_adjustment)
+      shipment.order.stub(:update!)
+
+      shipment.state = 'canceled'
+      shipment.should_receive(:determine_state).twice.and_return('ready')
+      shipment.should_receive(:after_resume)
+      shipment.resume!
+      # Shipment is pending because order is already paid
+      shipment.state.should eq 'pending'
+    end
+  end
+
+  context "#ship" do
+    before do
+      order.stub(:update!)
+      shipment.stub(require_inventory: false, update_order: true, state: 'ready')
+      shipment.stub(adjustment: charge)
+      shipping_method.stub(:create_adjustment)
+      shipment.stub(:ensure_correct_adjustment)
+    end
+
+    it "should update shipped_at timestamp" do
+      shipment.stub(:send_shipped_email)
+      shipment.ship!
+      shipment.shipped_at.should_not be_nil
+      # Ensure value is persisted
+      shipment.reload
+      shipment.shipped_at.should_not be_nil
+    end
+
+    it "should send a shipment email" do
+      mail_message = double 'Mail::Message'
+      shipment_id = nil
+      Spree::ShipmentMailer.should_receive(:shipped_email) { |*args|
+        shipment_id = args[0]
+        mail_message
+      }
+      mail_message.should_receive :deliver
+      shipment.ship!
+      shipment_id.should == shipment.id
+    end
+
+    it "should finalize the shipment's adjustment" do
+      shipment.stub(:send_shipped_email)
+      shipment.ship!
+      shipment.adjustment.state.should == 'finalized'
+      shipment.adjustment.should be_immutable
+    end
+  end
+
+  context "#ready" do
+    # Regression test for #2040
+    it "cannot ready a shipment for an order if the order is unpaid" do
+      order.stub(paid?: false)
+      assert !shipment.can_ready?
+    end
+  end
+
+  context "ensure_correct_adjustment" do
+    before { shipment.stub(:reload) }
+
+    it "should create adjustment when not present" do
+      shipment.stub(:selected_shipping_rate_id => 1)
+      shipping_method.should_receive(:create_adjustment).with(shipping_method.adjustment_label, order, shipment, true, "open")
+      shipment.send(:ensure_correct_adjustment)
+    end
+
+    # Regression test for #3138
+    it "should use the shipping method's adjustment label" do
+      shipment.stub(:selected_shipping_rate_id => 1)
+      shipping_method.stub(:adjustment_label => "Foobar")
+      shipping_method.should_receive(:create_adjustment).with("Foobar", order, shipment, true, "open")
+      shipment.send(:ensure_correct_adjustment)
+    end
+
+    it "should update originator when adjustment is present" do
+      shipment.stub(selected_shipping_rate: mock_model(Spree::ShippingRate, cost: 10.00))
+      shipment.stub(adjustment: mock_model(Spree::Adjustment, open?: true))
+      shipment.adjustment.should_receive(:originator=).with(shipping_method)
+      shipment.adjustment.should_receive(:label=).with(shipping_method.adjustment_label)
+      shipment.adjustment.should_receive(:amount=).with(10.00)
+      shipment.adjustment.should_receive(:save!)
+      shipment.adjustment.should_receive(:reload)
+      shipment.send(:ensure_correct_adjustment)
+    end
+
+    it 'should not update amount if adjustment is not open?' do
+      shipment.stub(selected_shipping_rate: mock_model(Spree::ShippingRate, cost: 10.00))
+      shipment.stub(adjustment: mock_model(Spree::Adjustment, open?: false))
+      shipment.adjustment.should_receive(:originator=).with(shipping_method)
+      shipment.adjustment.should_receive(:label=).with(shipping_method.adjustment_label)
+      shipment.adjustment.should_not_receive(:amount=).with(10.00)
+      shipment.adjustment.should_receive(:save!)
+      shipment.adjustment.should_receive(:reload)
+      shipment.send(:ensure_correct_adjustment)
+    end
+  end
+
+  context "update_order" do
+    it "should update order" do
+      order.should_receive(:update!)
+      shipment.send(:update_order)
+    end
+  end
+
+  context "after_save" do
+    it "should run correct callbacks" do
+      shipment.should_receive(:ensure_correct_adjustment)
+      shipment.should_receive(:update_order)
+      shipment.run_callbacks(:save)
+    end
+  end
+
+  context "currency" do
+    it "returns the order currency" do
+      shipment.currency.should == order.currency
+    end
+  end
+
+  context "#tracking_url" do
+    it "uses shipping method to determine url" do
+      shipping_method.should_receive(:build_tracking_url).with('1Z12345').and_return(:some_url)
+      shipment.tracking = '1Z12345'
+
+      shipment.tracking_url.should == :some_url
+    end
+  end
+
+  context "set up new inventory units" do
+    let(:variant) { double("Variant", id: 9) }
+    let(:inventory_units) { double }
+    let(:params) do
+      { variant_id: variant.id, state: 'on_hand', order_id: order.id }
+    end
+
+    before { shipment.stub inventory_units: inventory_units }
+
+    it "associates variant and order" do
+      expect(inventory_units).to receive(:create).with(params)
+      unit = shipment.set_up_inventory('on_hand', variant, order)
+    end
+  end
+
+  # Regression test for #3349
+  context "#destroy" do
+    it "destroys linked shipping_rates" do
+      reflection = Spree::Shipment.reflect_on_association(:shipping_rates)
+      reflection.options[:dependent] = :destroy
+    end
+  end
 end

From 494251b7cf40f85a495378b74a7d67c5e21890df Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:43:28 +0100
Subject: [PATCH 25/47] Fix simple rubocop issues

---
 app/models/spree/shipment.rb       | 132 ++++++++++++++++-------------
 spec/models/spree/shipment_spec.rb |  90 +++++++++++---------
 2 files changed, 124 insertions(+), 98 deletions(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index b49f8f5bdc..f3b0b03f0c 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'ostruct'
 
 module Spree
@@ -28,7 +30,8 @@ module Spree
     scope :with_state, ->(*s) { where(state: s) }
     scope :trackable, -> { where("tracking IS NOT NULL AND tracking != ''") }
 
-    # shipment state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
+    # Shipment state machine
+    # See http://github.com/pluginaweek/state_machine/tree/master for details
     state_machine initial: :pending, use_transactions: false do
       event :ready do
         transition from: :pending, to: :ready, if: lambda { |shipment|
@@ -70,12 +73,13 @@ module Spree
     end
 
     def backordered?
-      inventory_units.any? { |inventory_unit| inventory_unit.backordered? }
+      inventory_units.any?(&:backordered?)
     end
 
     def shipped=(value)
       return unless value == '1' && shipped_at.nil?
-      self.shipped_at = Time.now
+
+      self.shipped_at = Time.zone.now
     end
 
     def shipping_method
@@ -97,7 +101,7 @@ module Spree
     def selected_shipping_rate_id=(id)
       shipping_rates.update_all(selected: false)
       shipping_rates.update(id, selected: true)
-      self.save!
+      save!
     end
 
     def refresh_rates
@@ -119,8 +123,9 @@ module Spree
       order ? order.currency : Spree::Config[:currency]
     end
 
-    # The adjustment amount associated with this shipment (if any.)  Returns only the first adjustment to match
-    # the shipment but there should never really be more than one.
+    # The adjustment amount associated with this shipment (if any)
+    #   Returns only the first adjustment to match the shipment
+    #   There should never really be more than one.
     def cost
       adjustment ? adjustment.amount : 0
     end
@@ -149,7 +154,7 @@ module Spree
       Spree::Money.new(total_cost, { currency: currency })
     end
 
-    def editable_by?(user)
+    def editable_by?(_user)
       !shipped?
     end
 
@@ -162,7 +167,7 @@ module Spree
     end
 
     def line_items
-      if order.complete? and Spree::Config[:track_inventory_levels]
+      if order.complete? && Spree::Config[:track_inventory_levels]
         order.line_items.select { |li| inventory_units.pluck(:variant_id).include?(li.variant_id) }
       else
         order.line_items
@@ -182,14 +187,15 @@ module Spree
       manifest.each { |item| manifest_unstock(item) }
     end
 
-    # Updates various aspects of the Shipment while bypassing any callbacks.  Note that this method takes an explicit reference to the
-    # Order object.  This is necessary because the association actually has a stale (and unsaved) copy of the Order and so it will not
-    # yield the correct results.
+    # Updates various aspects of the Shipment while bypassing any callbacks.
+    #   Note that this method takes an explicit reference to the Order object.
+    #   This is necessary because the association actually has a stale (and unsaved) copy of the
+    #     Order and so it will not yield the correct results.
     def update!(order)
       old_state = state
       new_state = determine_state(order)
       update_column :state, new_state
-      after_ship if new_state == 'shipped' and old_state != 'shipped'
+      after_ship if new_state == 'shipped' && old_state != 'shipped'
     end
 
     # Determines the appropriate +state+ according to the following logic:
@@ -200,8 +206,9 @@ module Spree
     def determine_state(order)
       return 'canceled' if order.canceled?
       return 'pending' unless order.can_ship?
-      return 'pending' if inventory_units.any? &:backordered?
+      return 'pending' if inventory_units.any?(&:backordered?)
       return 'shipped' if state == 'shipped'
+
       order.paid? ? 'ready' : 'pending'
     end
 
@@ -226,65 +233,72 @@ module Spree
     end
 
     def set_up_inventory(state, variant, order)
-      self.inventory_units.create(variant_id: variant.id, state: state, order_id: order.id)
+      inventory_units.create(variant_id: variant.id, state: state, order_id: order.id)
     end
 
     private
 
-      def manifest_unstock(item)
-        stock_location.unstock item.variant, item.quantity, self
-      end
+    def manifest_unstock(item)
+      stock_location.unstock item.variant, item.quantity, self
+    end
 
-      def manifest_restock(item)
-        stock_location.restock item.variant, item.quantity, self
-      end
+    def manifest_restock(item)
+      stock_location.restock item.variant, item.quantity, self
+    end
 
-      def generate_shipment_number
-        return number unless number.blank?
-        record = true
-        while record
-          random = "H#{Array.new(11) { rand(9) }.join}"
-          record = self.class.where(number: random).first
-        end
-        self.number = random
-      end
+    def generate_shipment_number
+      return number if number.present?
 
-      def description_for_shipping_charge
-        "#{Spree.t(:shipping)} (#{shipping_method.name})"
+      record = true
+      while record
+        random = "H#{Array.new(11) { rand(9) }.join}"
+        record = self.class.where(number: random).first
       end
+      self.number = random
+    end
 
-      def validate_shipping_method
-        unless shipping_method.nil?
-          errors.add :shipping_method, Spree.t(:is_not_available_to_shipment_address) unless shipping_method.include?(address)
-        end
-      end
+    def description_for_shipping_charge
+      "#{Spree.t(:shipping)} (#{shipping_method.name})"
+    end
 
-      def after_ship
-        inventory_units.each &:ship!
-        adjustment.finalize!
-        send_shipped_email
-        touch :shipped_at
-      end
+    def validate_shipping_method
+      return if shipping_method.nil?
 
-      def send_shipped_email
-        ShipmentMailer.shipped_email(self.id).deliver
-      end
+      return if shipping_method.include?(address)
 
-      def ensure_correct_adjustment
-        if adjustment
-          adjustment.originator = shipping_method
-          adjustment.label = shipping_method.adjustment_label
-          adjustment.amount = selected_shipping_rate.cost if adjustment.open?
-          adjustment.save!
-          adjustment.reload
-        elsif selected_shipping_rate_id
-          shipping_method.create_adjustment shipping_method.adjustment_label, order, self, true, "open"
-          reload #ensure adjustment is present on later saves
-        end
-      end
+      errors.add :shipping_method, Spree.t(:is_not_available_to_shipment_address)
+    end
 
-      def update_order
-        order.update!
+    def after_ship
+      inventory_units.each(&:ship!)
+      adjustment.finalize!
+      send_shipped_email
+      touch :shipped_at
+    end
+
+    def send_shipped_email
+      ShipmentMailer.shipped_email(id).deliver
+    end
+
+    def ensure_correct_adjustment
+      if adjustment
+        adjustment.originator = shipping_method
+        adjustment.label = shipping_method.adjustment_label
+        adjustment.amount = selected_shipping_rate.cost if adjustment.open?
+        adjustment.save!
+        adjustment.reload
+      elsif selected_shipping_rate_id
+        shipping_method.create_adjustment(shipping_method.adjustment_label,
+                                          order,
+                                          self,
+                                          true,
+                                          "open")
+        reload # ensure adjustment is present on later saves
       end
+    end
+
+    def update_order
+      order.update!
+    end
   end
 end
diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index a9992c89c5..0c71a3ea58 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -1,11 +1,12 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 require 'benchmark'
 
 describe Spree::Shipment do
-  let(:order) { mock_model Spree::Order, backordered?: false,
-                                         canceled?: false,
-                                         can_ship?: true,
-                                         currency: 'USD' }
+  let(:order) {
+    create(:order, backordered?: false, canceled?: false, can_ship?: true, currency: 'USD')
+  }
   let(:shipping_method) { create(:shipping_method, name: "UPS") }
   let(:shipment) do
     shipment = Spree::Shipment.new order: order
@@ -19,41 +20,41 @@ describe Spree::Shipment do
 
   it 'is backordered if one if its inventory_units is backordered' do
     shipment.stub(inventory_units: [
-      mock_model(Spree::InventoryUnit, backordered?: false),
-      mock_model(Spree::InventoryUnit, backordered?: true)
-    ])
+                    create(:inventory_unit, backordered?: false),
+                    create(:inventory_unit, backordered?: true)
+                  ])
     shipment.should be_backordered
   end
 
   context "#cost" do
     it "should return the amount of any shipping charges that it originated" do
       shipment.stub_chain :adjustment, amount: 10
-      shipment.cost.should == 10
+      expect(shipment.cost).to eq 10
     end
 
     it "should return 0 if there are no relevant shipping adjustments" do
-      shipment.cost.should == 0
+      expect(shipment.cost).to eq 0
     end
   end
 
   context "display_cost" do
     it "retuns a Spree::Money" do
       shipment.stub(:cost) { 21.22 }
-      shipment.display_cost.should == Spree::Money.new(21.22)
+      expect(shipment.display_cost).to eq Spree::Money.new(21.22)
     end
   end
 
   context "display_item_cost" do
     it "retuns a Spree::Money" do
       shipment.stub(:item_cost) { 21.22 }
-      shipment.display_item_cost.should == Spree::Money.new(21.22)
+      expect(shipment.display_item_cost).to eq Spree::Money.new(21.22)
     end
   end
 
   context "display_total_cost" do
     it "retuns a Spree::Money" do
       shipment.stub(:total_cost) { 21.22 }
-      shipment.display_total_cost.should == Spree::Money.new(21.22)
+      expect(shipment.display_total_cost).to eq Spree::Money.new(21.22)
     end
   end
 
@@ -115,10 +116,12 @@ describe Spree::Shipment do
     let(:shipment) { create(:shipment) }
     let(:shipping_method1) { create(:shipping_method) }
     let(:shipping_method2) { create(:shipping_method) }
-    let(:shipping_rates) { [
-      Spree::ShippingRate.new(shipping_method: shipping_method1, cost: 10.00, selected: true),
-      Spree::ShippingRate.new(shipping_method: shipping_method2, cost: 20.00)
-    ] }
+    let(:shipping_rates) {
+      [
+        Spree::ShippingRate.new(shipping_method: shipping_method1, cost: 10.00, selected: true),
+        Spree::ShippingRate.new(shipping_method: shipping_method2, cost: 20.00)
+      ]
+    }
 
     it 'returns shipping_method from selected shipping_rate' do
       shipment.shipping_rates.delete_all
@@ -133,14 +136,14 @@ describe Spree::Shipment do
         Spree::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
         shipment.stub(shipping_method: shipping_method2)
 
-        shipment.refresh_rates.should == shipping_rates
-        shipment.reload.selected_shipping_rate.shipping_method_id.should == shipping_method2.id
+        expect(shipment.refresh_rates).to eq shipping_rates
+        expect(shipment.reload.selected_shipping_rate.shipping_method_id).to eq shipping_method2.id
       end
 
       it 'should handle no shipping_method selection' do
         Spree::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
         shipment.stub(shipping_method: nil)
-        shipment.refresh_rates.should == shipping_rates
+        expect(shipment.refresh_rates).to eq shipping_rates
         shipment.reload.selected_shipping_rate.should_not be_nil
       end
 
@@ -148,13 +151,16 @@ describe Spree::Shipment do
         Spree::Stock::Estimator.should_not_receive(:new)
         shipment.shipping_rates.delete_all
         shipment.stub(shipped?: true)
-        shipment.refresh_rates.should == []
+        expect(shipment.refresh_rates).to eq []
       end
 
       context 'to_package' do
         it 'should use symbols for states when adding contents to package' do
-          shipment.stub_chain(:inventory_units, includes: [ build(:inventory_unit, variant: variant, state: 'on_hand'),
-                                                            build(:inventory_unit, variant: variant, state: 'backordered') ] )
+          shipment.stub_chain(:inventory_units,
+                              includes: [build(:inventory_unit, variant: variant,
+                                                                state: 'on_hand'),
+                                         build(:inventory_unit, variant: variant,
+                                                                state: 'backordered')] )
           package = shipment.to_package
           package.on_hand.count.should eq 1
           package.backordered.count.should eq 1
@@ -245,7 +251,7 @@ describe Spree::Shipment do
       line_items = [mock_model(Spree::LineItem)]
       order.stub complete?: true
       order.stub line_items: line_items
-      shipment.line_items.should == line_items
+      expect(shipment.line_items).to eq line_items
     end
   end
 
@@ -287,7 +293,9 @@ describe Spree::Shipment do
     end
 
     it 'restocks the items' do
-      shipment.stub_chain(:inventory_units, :joins, includes: [mock_model(Spree::InventoryUnit, variant: variant)])
+      shipment.stub_chain(:inventory_units,
+                          :joins,
+                          includes: [mock_model(Spree::InventoryUnit, variant: variant)])
       shipment.stock_location = mock_model(Spree::StockLocation)
       shipment.stock_location.should_receive(:restock).with(variant, 1, shipment)
       shipment.after_cancel
@@ -307,7 +315,9 @@ describe Spree::Shipment do
     end
 
     it 'unstocks them items' do
-      shipment.stub_chain(:inventory_units, :joins, includes: [mock_model(Spree::InventoryUnit, variant: variant)])
+      shipment.stub_chain(:inventory_units,
+                          :joins,
+                          includes: [mock_model(Spree::InventoryUnit, variant: variant)])
       shipment.stock_location = mock_model(Spree::StockLocation)
       shipment.stock_location.should_receive(:unstock).with(variant, 1, shipment)
       shipment.after_resume
@@ -353,13 +363,13 @@ describe Spree::Shipment do
       }
       mail_message.should_receive :deliver
       shipment.ship!
-      shipment_id.should == shipment.id
+      expect(shipment_id).to eq shipment.id
     end
 
     it "should finalize the shipment's adjustment" do
       shipment.stub(:send_shipped_email)
       shipment.ship!
-      shipment.adjustment.state.should == 'finalized'
+      expect(shipment.adjustment.state).to eq 'finalized'
       shipment.adjustment.should be_immutable
     end
   end
@@ -376,17 +386,19 @@ describe Spree::Shipment do
     before { shipment.stub(:reload) }
 
     it "should create adjustment when not present" do
-      shipment.stub(:selected_shipping_rate_id => 1)
-      shipping_method.should_receive(:create_adjustment).with(shipping_method.adjustment_label, order, shipment, true, "open")
-      shipment.send(:ensure_correct_adjustment)
+      shipment.stub(selected_shipping_rate_id: 1)
+      shipping_method.should_receive(:create_adjustment).with(shipping_method.adjustment_label,
+                                                              order, shipment, true, "open")
+      shipment.__send__(:ensure_correct_adjustment)
     end
 
     # Regression test for #3138
     it "should use the shipping method's adjustment label" do
-      shipment.stub(:selected_shipping_rate_id => 1)
-      shipping_method.stub(:adjustment_label => "Foobar")
-      shipping_method.should_receive(:create_adjustment).with("Foobar", order, shipment, true, "open")
-      shipment.send(:ensure_correct_adjustment)
+      shipment.stub(selected_shipping_rate_id: 1)
+      shipping_method.stub(adjustment_label: "Foobar")
+      shipping_method.should_receive(:create_adjustment).with("Foobar", order,
+                                                              shipment, true, "open")
+      shipment.__send__(:ensure_correct_adjustment)
     end
 
     it "should update originator when adjustment is present" do
@@ -397,7 +409,7 @@ describe Spree::Shipment do
       shipment.adjustment.should_receive(:amount=).with(10.00)
       shipment.adjustment.should_receive(:save!)
       shipment.adjustment.should_receive(:reload)
-      shipment.send(:ensure_correct_adjustment)
+      shipment.__send__(:ensure_correct_adjustment)
     end
 
     it 'should not update amount if adjustment is not open?' do
@@ -408,14 +420,14 @@ describe Spree::Shipment do
       shipment.adjustment.should_not_receive(:amount=).with(10.00)
       shipment.adjustment.should_receive(:save!)
       shipment.adjustment.should_receive(:reload)
-      shipment.send(:ensure_correct_adjustment)
+      shipment.__send__(:ensure_correct_adjustment)
     end
   end
 
   context "update_order" do
     it "should update order" do
       order.should_receive(:update!)
-      shipment.send(:update_order)
+      shipment.__send__(:update_order)
     end
   end
 
@@ -429,7 +441,7 @@ describe Spree::Shipment do
 
   context "currency" do
     it "returns the order currency" do
-      shipment.currency.should == order.currency
+      expect(shipment.currency).to eq order.currency
     end
   end
 
@@ -438,7 +450,7 @@ describe Spree::Shipment do
       shipping_method.should_receive(:build_tracking_url).with('1Z12345').and_return(:some_url)
       shipment.tracking = '1Z12345'
 
-      shipment.tracking_url.should == :some_url
+      expect(shipment.tracking_url).to eq :some_url
     end
   end
 

From 7a03f57da0fed481070ceb5db8896a89e589048a Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:48:08 +0100
Subject: [PATCH 26/47] Merge shipment decorator with class brought from
 spree_core

---
 app/controllers/spree/orders_controller.rb |  2 +-
 app/models/spree/shipment.rb               | 25 ++++++++++++-
 app/models/spree/shipment_decorator.rb     | 41 ----------------------
 3 files changed, 25 insertions(+), 43 deletions(-)
 delete mode 100644 app/models/spree/shipment_decorator.rb

diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb
index 85939fe1ad..545ae1ce60 100644
--- a/app/controllers/spree/orders_controller.rb
+++ b/app/controllers/spree/orders_controller.rb
@@ -166,7 +166,7 @@ module Spree
     # recalculates the shipment taxes
     def update_totals_and_taxes
       @order.updater.update_totals
-      @order.shipment&.ensure_correct_adjustment_with_included_tax
+      @order.shipment&.ensure_correct_adjustment
     end
 
     # Sets the adjustments to open to perform the block's action and restores
diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index f3b0b03f0c..c5fea38d2c 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -159,13 +159,18 @@ module Spree
     end
 
     def manifest
-      inventory_units.joins(:variant).includes(:variant).group_by(&:variant).map do |variant, units|
+      inventory_units.group_by(&:variant).map do |variant, units|
         states = {}
         units.group_by(&:state).each { |state, iu| states[state] = iu.count }
+        scoper.scope(variant)
         OpenStruct.new(variant: variant, quantity: units.length, states: states)
       end
     end
 
+    def scoper
+      @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor)
+    end
+
     def line_items
       if order.complete? && Spree::Config[:track_inventory_levels]
         order.line_items.select { |li| inventory_units.pluck(:variant_id).include?(li.variant_id) }
@@ -295,10 +300,28 @@ module Spree
                                           "open")
         reload # ensure adjustment is present on later saves
       end
+
+      update_adjustment_included_tax if adjustment
+    end
+
+    def update_adjustment_included_tax
+      if Config.shipment_inc_vat && (order.distributor.nil? || order.distributor.charges_sales_tax)
+        adjustment.set_included_tax! Config.shipping_tax_rate
+      else
+        adjustment.set_included_tax! 0
+      end
     end
 
     def update_order
       order.update!
     end
+
+    # NOTE: This is an override of spree's method, needed to allow orders
+    # without line items (ie. user invoices) to not have inventory units
+    def require_inventory
+      return false unless line_items.count > 0 # This line altered
+
+      order.completed? && !order.canceled?
+    end
   end
 end
diff --git a/app/models/spree/shipment_decorator.rb b/app/models/spree/shipment_decorator.rb
deleted file mode 100644
index b23bd7266f..0000000000
--- a/app/models/spree/shipment_decorator.rb
+++ /dev/null
@@ -1,41 +0,0 @@
-module Spree
-  Shipment.class_eval do
-    def ensure_correct_adjustment_with_included_tax
-      ensure_correct_adjustment_without_included_tax
-
-      update_adjustment_included_tax if adjustment
-    end
-    alias_method_chain :ensure_correct_adjustment, :included_tax
-
-    def update_adjustment_included_tax
-      if Config.shipment_inc_vat && (order.distributor.nil? || order.distributor.charges_sales_tax)
-        adjustment.set_included_tax! Config.shipping_tax_rate
-      else
-        adjustment.set_included_tax! 0
-      end
-    end
-
-    def manifest
-      inventory_units.group_by(&:variant).map do |variant, units|
-        states = {}
-        units.group_by(&:state).each { |state, iu| states[state] = iu.count }
-        scoper.scope(variant)
-        OpenStruct.new(variant: variant, quantity: units.length, states: states)
-      end
-    end
-
-    def scoper
-      @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor)
-    end
-
-    private
-
-    # NOTE: This is an override of spree's method, needed to allow orders
-    # without line items (ie. user invoices) to not have inventory units
-    def require_inventory
-      return false unless line_items.count > 0 # This line altered
-
-      order.completed? && !order.canceled?
-    end
-  end
-end

From 2e33e02d7f14cc866dbffe2ec6a09f9931c20638 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 17:48:43 +0100
Subject: [PATCH 27/47] Remove dead code, this method was removed in spree
 2.0.4

---
 app/models/spree/shipment.rb       | 8 --------
 spec/models/spree/shipment_spec.rb | 2 +-
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index c5fea38d2c..bff7421c70 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -315,13 +315,5 @@ module Spree
     def update_order
       order.update!
     end
-
-    # NOTE: This is an override of spree's method, needed to allow orders
-    # without line items (ie. user invoices) to not have inventory units
-    def require_inventory
-      return false unless line_items.count > 0 # This line altered
-
-      order.completed? && !order.canceled?
-    end
   end
 end
diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index 0c71a3ea58..66561b32a9 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -339,7 +339,7 @@ describe Spree::Shipment do
   context "#ship" do
     before do
       order.stub(:update!)
-      shipment.stub(require_inventory: false, update_order: true, state: 'ready')
+      shipment.stub(update_order: true, state: 'ready')
       shipment.stub(adjustment: charge)
       shipping_method.stub(:create_adjustment)
       shipment.stub(:ensure_correct_adjustment)

From 46cf106047246e58d2d9f9e3bbd2c5cd00625567 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 19:40:30 +0100
Subject: [PATCH 28/47] Fix shipment spec brought from spree

---
 spec/models/spree/shipment_spec.rb | 219 ++++++++++++++---------------
 1 file changed, 109 insertions(+), 110 deletions(-)

diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index 66561b32a9..e3b7ef9671 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -4,10 +4,8 @@ require 'spec_helper'
 require 'benchmark'
 
 describe Spree::Shipment do
-  let(:order) {
-    create(:order, backordered?: false, canceled?: false, can_ship?: true, currency: 'USD')
-  }
-  let(:shipping_method) { create(:shipping_method, name: "UPS") }
+  let(:order) { build(:order) }
+  let(:shipping_method) { build(:shipping_method, name: "UPS") }
   let(:shipment) do
     shipment = Spree::Shipment.new order: order
     shipment.stub(shipping_method: shipping_method)
@@ -15,15 +13,16 @@ describe Spree::Shipment do
     shipment
   end
 
-  let(:charge) { create(:adjustment) }
-  let(:variant) { mock_model(Spree::Variant) }
+  let(:charge) { build(:adjustment) }
+  let(:variant) { build(:variant) }
 
-  it 'is backordered if one if its inventory_units is backordered' do
-    shipment.stub(inventory_units: [
-                    create(:inventory_unit, backordered?: false),
-                    create(:inventory_unit, backordered?: true)
-                  ])
-    shipment.should be_backordered
+  it 'is backordered if one of its inventory_units is backordered' do
+    unit1 = create(:inventory_unit)
+    unit2 = create(:inventory_unit)
+    allow(unit1).to receive(:backordered?) { false }
+    allow(unit2).to receive(:backordered?) { true }
+    shipment.stub(inventory_units: [unit1, unit2])
+    expect(shipment).to be_backordered
   end
 
   context "#cost" do
@@ -60,7 +59,7 @@ describe Spree::Shipment do
 
   it "#item_cost" do
     shipment = create(:shipment, order: create(:order_with_totals))
-    shipment.item_cost.should eql(10.0)
+    expect(shipment.item_cost).to eql(10.0)
   end
 
   context "manifest" do
@@ -84,29 +83,22 @@ describe Spree::Shipment do
     describe "with soft-deleted products or variants" do
       let!(:product) { create(:product) }
       let!(:order) { create(:order, distributor: product.supplier) }
-      let!(:deleted_variant) { create(:variant, product: product) }
-      let!(:other_variant) { create(:variant, product: product) }
-      let!(:line_item_for_deleted) { create(:line_item, order: order, variant: deleted_variant) }
-      let!(:line_item_for_other) { create(:line_item, order: order, variant: other_variant) }
-      let!(:shipment) { create(:shipment_with, :shipping_method, order: order) }
 
       context "when the variant is soft-deleted" do
-        before { deleted_variant.delete }
-
         it "can still access the variant" do
-          shipment.reload
-          variants = shipment.manifest.map(&:variant).uniq
-          expect(variants.sort_by(&:id)).to eq([deleted_variant, other_variant].sort_by(&:id))
+          order.line_items.first.variant.delete
+
+          variants = shipment.reload.manifest.map(&:variant).uniq
+          expect(variants).to eq [order.line_items.first.variant]
         end
       end
 
       context "when the product is soft-deleted" do
-        before { deleted_variant.product.delete }
-
         it "can still access the variant" do
-          shipment.reload
-          variants = shipment.manifest.map(&:variant)
-          expect(variants.sort_by(&:id)).to eq([deleted_variant, other_variant].sort_by(&:id))
+          order.line_items.first.variant.delete
+
+          variants = shipment.reload.manifest.map(&:variant)
+          expect(variants).to eq [order.line_items.first.variant]
         end
       end
     end
@@ -126,7 +118,7 @@ describe Spree::Shipment do
     it 'returns shipping_method from selected shipping_rate' do
       shipment.shipping_rates.delete_all
       shipment.shipping_rates.create shipping_method: shipping_method1, cost: 10.00, selected: true
-      shipment.shipping_method.should eq shipping_method1
+      expect(shipment.shipping_method).to eq shipping_method1
     end
 
     context 'refresh_rates' do
@@ -144,7 +136,7 @@ describe Spree::Shipment do
         Spree::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
         shipment.stub(shipping_method: nil)
         expect(shipment.refresh_rates).to eq shipping_rates
-        shipment.reload.selected_shipping_rate.should_not be_nil
+        expect(shipment.reload.selected_shipping_rate).to_not be_nil
       end
 
       it 'should not refresh if shipment is shipped' do
@@ -162,8 +154,8 @@ describe Spree::Shipment do
                                          build(:inventory_unit, variant: variant,
                                                                 state: 'backordered')] )
           package = shipment.to_package
-          package.on_hand.count.should eq 1
-          package.backordered.count.should eq 1
+          expect(package.on_hand.count).to eq 1
+          expect(package.backordered.count).to eq 1
         end
       end
     end
@@ -172,7 +164,7 @@ describe Spree::Shipment do
   it '#total_cost' do
     shipment.stub cost: 5.0
     shipment.stub item_cost: 50.0
-    shipment.total_cost.should eql(55.0)
+    expect(shipment.total_cost).to eql(55.0)
   end
 
   context "#update!" do
@@ -186,50 +178,75 @@ describe Spree::Shipment do
 
     shared_examples_for "pending if backordered" do
       it "should have a state of pending if backordered" do
-        shipment.stub(inventory_units: [mock_model(Spree::InventoryUnit, backordered?: true)])
+        unit = create(:inventory_unit)
+        allow(unit).to receive(:backordered?) { true }
+        shipment.stub(inventory_units: [unit])
         shipment.should_receive(:update_column).with(:state, 'pending')
         shipment.update!(order)
       end
     end
 
+    context "when order is canceled" do
+      it "should result in a 'pending' state" do
+        allow(order).to receive(:canceled?) { true }
+
+        shipment.should_receive(:update_column).with(:state, 'canceled')
+        shipment.update!(order)
+      end
+    end
+
     context "when order cannot ship" do
-      before { order.stub can_ship?: false }
       it "should result in a 'pending' state" do
+        allow(order).to receive(:can_ship?) { false }
+
         shipment.should_receive(:update_column).with(:state, 'pending')
         shipment.update!(order)
       end
     end
 
-    context "when order is paid" do
-      before { order.stub paid?: true }
-      it "should result in a 'ready' state" do
-        shipment.should_receive(:update_column).with(:state, 'ready')
-        shipment.update!(order)
-      end
-      it_should_behave_like 'immutable once shipped'
-      it_should_behave_like 'pending if backordered'
-    end
+    context "when order can ship" do
+      before { allow(order).to receive(:can_ship?) { true } }
 
-    context "when order has balance due" do
-      before { order.stub paid?: false }
-      it "should result in a 'pending' state" do
-        shipment.state = 'ready'
-        shipment.should_receive(:update_column).with(:state, 'pending')
-        shipment.update!(order)
-      end
-      it_should_behave_like 'immutable once shipped'
-      it_should_behave_like 'pending if backordered'
-    end
+      context "when order is paid" do
+        before { allow(order).to receive(:paid?) { true } }
 
-    context "when order has a credit owed" do
-      before { order.stub payment_state: 'credit_owed', paid?: true }
-      it "should result in a 'ready' state" do
-        shipment.state = 'pending'
-        shipment.should_receive(:update_column).with(:state, 'ready')
-        shipment.update!(order)
+        it "should result in a 'ready' state" do
+          shipment.should_receive(:update_column).with(:state, 'ready')
+          shipment.update!(order)
+        end
+
+        it_should_behave_like 'immutable once shipped'
+
+        it_should_behave_like 'pending if backordered'
+
+        context "when order has a credit owed" do
+          before { allow(order).to receive(:payment_state) { 'credit_owed' } }
+
+          it "should result in a 'ready' state" do
+            shipment.state = 'pending'
+            shipment.should_receive(:update_column).with(:state, 'ready')
+            shipment.update!(order)
+          end
+
+          it_should_behave_like 'immutable once shipped'
+
+          it_should_behave_like 'pending if backordered'
+        end
+      end
+
+      context "when order has balance due" do
+        before { allow(order).to receive(:paid?) { false } }
+
+        it "should result in a 'pending' state" do
+          shipment.state = 'ready'
+          shipment.should_receive(:update_column).with(:state, 'pending')
+          shipment.update!(order)
+        end
+
+        it_should_behave_like 'immutable once shipped'
+
+        it_should_behave_like 'pending if backordered'
       end
-      it_should_behave_like 'immutable once shipped'
-      it_should_behave_like 'pending if backordered'
     end
 
     context "when shipment state changes to shipped" do
@@ -243,41 +260,15 @@ describe Spree::Shipment do
     end
   end
 
-  context "when track_inventory is false" do
-    before { Spree::Config.set track_inventory_levels: false }
-    after { Spree::Config.set track_inventory_levels: true }
-
-    it "should not use the line items from order when track_inventory_levels is false" do
-      line_items = [mock_model(Spree::LineItem)]
-      order.stub complete?: true
-      order.stub line_items: line_items
-      expect(shipment.line_items).to eq line_items
-    end
-  end
-
   context "when order is completed" do
-    after { Spree::Config.set track_inventory_levels: true }
-
     before do
       order.stub completed?: true
       order.stub canceled?: false
     end
 
-    context "with inventory tracking" do
-      before { Spree::Config.set track_inventory_levels: true }
-
-      it "should validate with inventory" do
-        shipment.inventory_units = [create(:inventory_unit)]
-        shipment.valid?.should be_true
-      end
-    end
-
-    context "without inventory tracking" do
-      before { Spree::Config.set track_inventory_levels: false }
-
-      it "should validate with no inventory" do
-        shipment.valid?.should be_true
-      end
+    it "should validate with inventory" do
+      shipment.inventory_units = [create(:inventory_unit)]
+      expect(shipment.valid?).to be_truthy
     end
   end
 
@@ -289,14 +280,16 @@ describe Spree::Shipment do
       shipment.state = 'pending'
       shipment.should_receive(:after_cancel)
       shipment.cancel!
-      shipment.state.should eq 'canceled'
+      expect(shipment.state).to eq 'canceled'
     end
 
     it 'restocks the items' do
+      unit = double(:inventory_unit, variant: variant)
+      allow(unit).to receive(:quantity) { 1 }
       shipment.stub_chain(:inventory_units,
-                          :joins,
-                          includes: [mock_model(Spree::InventoryUnit, variant: variant)])
-      shipment.stock_location = mock_model(Spree::StockLocation)
+                          :group_by,
+                          map: [unit])
+      shipment.stock_location = build(:stock_location)
       shipment.stock_location.should_receive(:restock).with(variant, 1, shipment)
       shipment.after_cancel
     end
@@ -311,14 +304,16 @@ describe Spree::Shipment do
       shipment.should_receive(:determine_state).and_return(:ready)
       shipment.should_receive(:after_resume)
       shipment.resume!
-      shipment.state.should eq 'ready'
+      expect(shipment.state).to eq 'ready'
     end
 
-    it 'unstocks them items' do
+    it 'unstocks the items' do
+      unit = create(:inventory_unit, variant: variant)
+      allow(unit).to receive(:quantity) { 1 }
       shipment.stub_chain(:inventory_units,
-                          :joins,
-                          includes: [mock_model(Spree::InventoryUnit, variant: variant)])
-      shipment.stock_location = mock_model(Spree::StockLocation)
+                          :group_by,
+                          map: [unit])
+      shipment.stock_location = create(:stock_location)
       shipment.stock_location.should_receive(:unstock).with(variant, 1, shipment)
       shipment.after_resume
     end
@@ -332,7 +327,7 @@ describe Spree::Shipment do
       shipment.should_receive(:after_resume)
       shipment.resume!
       # Shipment is pending because order is already paid
-      shipment.state.should eq 'pending'
+      expect(shipment.state).to eq 'pending'
     end
   end
 
@@ -348,10 +343,10 @@ describe Spree::Shipment do
     it "should update shipped_at timestamp" do
       shipment.stub(:send_shipped_email)
       shipment.ship!
-      shipment.shipped_at.should_not be_nil
+      expect(shipment.shipped_at).to_not be_nil
       # Ensure value is persisted
       shipment.reload
-      shipment.shipped_at.should_not be_nil
+      expect(shipment.shipped_at).to_not be_nil
     end
 
     it "should send a shipment email" do
@@ -370,7 +365,7 @@ describe Spree::Shipment do
       shipment.stub(:send_shipped_email)
       shipment.ship!
       expect(shipment.adjustment.state).to eq 'finalized'
-      shipment.adjustment.should be_immutable
+      expect(shipment.adjustment).to be_immutable
     end
   end
 
@@ -402,23 +397,27 @@ describe Spree::Shipment do
     end
 
     it "should update originator when adjustment is present" do
-      shipment.stub(selected_shipping_rate: mock_model(Spree::ShippingRate, cost: 10.00))
-      shipment.stub(adjustment: mock_model(Spree::Adjustment, open?: true))
+      shipment.stub(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
+      adjustment = build(:adjustment)
+      shipment.stub(adjustment: adjustment)
+      allow(adjustment).to receive(:open?) { true }
       shipment.adjustment.should_receive(:originator=).with(shipping_method)
       shipment.adjustment.should_receive(:label=).with(shipping_method.adjustment_label)
       shipment.adjustment.should_receive(:amount=).with(10.00)
-      shipment.adjustment.should_receive(:save!)
+      allow(shipment.adjustment).to receive(:save!)
       shipment.adjustment.should_receive(:reload)
       shipment.__send__(:ensure_correct_adjustment)
     end
 
     it 'should not update amount if adjustment is not open?' do
-      shipment.stub(selected_shipping_rate: mock_model(Spree::ShippingRate, cost: 10.00))
-      shipment.stub(adjustment: mock_model(Spree::Adjustment, open?: false))
+      shipment.stub(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
+      adjustment = build(:adjustment)
+      shipment.stub(adjustment: adjustment)
+      allow(adjustment).to receive(:open?) { false }
       shipment.adjustment.should_receive(:originator=).with(shipping_method)
       shipment.adjustment.should_receive(:label=).with(shipping_method.adjustment_label)
       shipment.adjustment.should_not_receive(:amount=).with(10.00)
-      shipment.adjustment.should_receive(:save!)
+      allow(shipment.adjustment).to receive(:save!)
       shipment.adjustment.should_receive(:reload)
       shipment.__send__(:ensure_correct_adjustment)
     end

From 8e116dd58aba5ba8000e896fd312f90df63ca1bf Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Wed, 1 Jul 2020 20:55:58 +0100
Subject: [PATCH 29/47] Make ensure_correct_adjustment a public method because
 we call it in OFN

---
 app/models/spree/shipment.rb | 38 ++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index bff7421c70..d35c84ca08 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -241,6 +241,25 @@ module Spree
       inventory_units.create(variant_id: variant.id, state: state, order_id: order.id)
     end
 
+    def ensure_correct_adjustment
+      if adjustment
+        adjustment.originator = shipping_method
+        adjustment.label = shipping_method.adjustment_label
+        adjustment.amount = selected_shipping_rate.cost if adjustment.open?
+        adjustment.save!
+        adjustment.reload
+      elsif selected_shipping_rate_id
+        shipping_method.create_adjustment(shipping_method.adjustment_label,
+                                          order,
+                                          self,
+                                          true,
+                                          "open")
+        reload # ensure adjustment is present on later saves
+      end
+
+      update_adjustment_included_tax if adjustment
+    end
+
     private
 
     def manifest_unstock(item)
@@ -285,25 +304,6 @@ module Spree
       ShipmentMailer.shipped_email(id).deliver
     end
 
-    def ensure_correct_adjustment
-      if adjustment
-        adjustment.originator = shipping_method
-        adjustment.label = shipping_method.adjustment_label
-        adjustment.amount = selected_shipping_rate.cost if adjustment.open?
-        adjustment.save!
-        adjustment.reload
-      elsif selected_shipping_rate_id
-        shipping_method.create_adjustment(shipping_method.adjustment_label,
-                                          order,
-                                          self,
-                                          true,
-                                          "open")
-        reload # ensure adjustment is present on later saves
-      end
-
-      update_adjustment_included_tax if adjustment
-    end
-
     def update_adjustment_included_tax
       if Config.shipment_inc_vat && (order.distributor.nil? || order.distributor.charges_sales_tax)
         adjustment.set_included_tax! Config.shipping_tax_rate

From 1b28592f58f096038baf48f9d438369a2d69e5f7 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 14:28:19 +0100
Subject: [PATCH 30/47] Now that the stock code is on out side we can clean up!

Remove everything related to splitters (including bringing environment.rb so we remove the splitters variable from it
---
 app/models/spree/stock/coordinator.rb         |  7 +---
 app/models/spree/stock/packer.rb              | 21 ++---------
 config/application.rb                         | 11 ------
 .../order_management/stock/basic_splitter.rb  | 29 ---------------
 .../stock/basic_splitter_spec.rb              | 37 -------------------
 lib/spree/core/environment.rb                 | 14 +++++++
 spec/config/application_spec.rb               |  9 -----
 spec/models/spree/order/checkout_spec.rb      |  3 --
 spec/models/spree/stock/packer_spec.rb        |  5 ---
 9 files changed, 18 insertions(+), 118 deletions(-)
 delete mode 100644 engines/order_management/app/services/order_management/stock/basic_splitter.rb
 delete mode 100644 engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
 create mode 100644 lib/spree/core/environment.rb
 delete mode 100644 spec/config/application_spec.rb

diff --git a/app/models/spree/stock/coordinator.rb b/app/models/spree/stock/coordinator.rb
index 4b42060bd0..bbbc176956 100644
--- a/app/models/spree/stock/coordinator.rb
+++ b/app/models/spree/stock/coordinator.rb
@@ -51,12 +51,7 @@ module Spree
       end
 
       def build_packer(stock_location, order)
-        Packer.new(stock_location, order, splitters(stock_location))
-      end
-
-      def splitters(_stock_location)
-        # extension point to return custom splitters for a location
-        Rails.application.config.spree.stock_splitters
+        Packer.new(stock_location, order)
       end
     end
   end
diff --git a/app/models/spree/stock/packer.rb b/app/models/spree/stock/packer.rb
index 242f46536e..6fb58531c3 100644
--- a/app/models/spree/stock/packer.rb
+++ b/app/models/spree/stock/packer.rb
@@ -3,21 +3,16 @@
 module Spree
   module Stock
     class Packer
-      attr_reader :stock_location, :order, :splitters, :package_factory
+      attr_reader :stock_location, :order, :package_factory
 
-      def initialize(stock_location, order, splitters = [OrderManagement::Stock::BasicSplitter])
+      def initialize(stock_location, order)
         @stock_location = stock_location
         @order = order
-        @splitters = splitters
         @package_factory = Spree::Config.package_factory
       end
 
       def packages
-        if splitters.empty?
-          [default_package]
-        else
-          build_splitter.split [default_package]
-        end
+        [default_package]
       end
 
       def default_package
@@ -35,16 +30,6 @@ module Spree
         end
         package
       end
-
-      private
-
-      def build_splitter
-        splitter = nil
-        splitters.reverse.each do |klass|
-          splitter = klass.new(self, splitter)
-        end
-        splitter
-      end
     end
   end
 end
diff --git a/config/application.rb b/config/application.rb
index 419638b236..547795ea5f 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -76,17 +76,6 @@ module Openfoodnetwork
       ]
     end
 
-    # Every splitter (except Base splitter) will split the order in multiple packages
-    #   Each package will generate a separate shipment in the order
-    #   Base splitter does not split the packages
-    #   So, because in OFN we have locked orders to have only one shipment,
-    #     we must use this splitter and no other
-    initializer "spree.register.stock_splitters" do |app|
-      app.config.spree.stock_splitters = [
-        OrderManagement::Stock::BasicSplitter
-      ]
-    end
-
     # Register Spree payment methods
     initializer "spree.gateway.payment_methods", :after => "spree.register.payment_methods" do |app|
       app.config.spree.payment_methods << Spree::Gateway::Migs
diff --git a/engines/order_management/app/services/order_management/stock/basic_splitter.rb b/engines/order_management/app/services/order_management/stock/basic_splitter.rb
deleted file mode 100644
index eacbf05704..0000000000
--- a/engines/order_management/app/services/order_management/stock/basic_splitter.rb
+++ /dev/null
@@ -1,29 +0,0 @@
-# frozen_string_literal: true
-
-module OrderManagement
-  module Stock
-    class BasicSplitter
-      attr_reader :packer, :next_splitter
-
-      def initialize(packer, next_splitter = nil)
-        @packer = packer
-        @next_splitter = next_splitter
-      end
-      delegate :stock_location, :order, to: :packer
-
-      def split(packages)
-        return_next(packages)
-      end
-
-      private
-
-      def return_next(packages)
-        next_splitter ? next_splitter.split(packages) : packages
-      end
-
-      def build_package(contents = [])
-        @packer.package_factory.new(stock_location, order, contents)
-      end
-    end
-  end
-end
diff --git a/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb b/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
deleted file mode 100644
index a8265a1d01..0000000000
--- a/engines/order_management/spec/services/order_management/stock/basic_splitter_spec.rb
+++ /dev/null
@@ -1,37 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-module OrderManagement
-  module Stock
-    describe BasicSplitter do
-      let(:packer) { build(:stock_packer) }
-
-      it 'continues to splitter chain' do
-        splitter1 = BasicSplitter.new(packer)
-        splitter2 = BasicSplitter.new(packer, splitter1)
-        packages = []
-
-        expect(splitter1).to receive(:split).with(packages)
-        splitter2.split(packages)
-      end
-
-      it 'builds package using package factory' do
-        # Basic extension of Base splitter used to test build_package method
-        class ::RealSplitter < BasicSplitter
-          def split(_packages)
-            build_package
-          end
-        end
-
-        # Custom package used to test setting package factory
-        class ::CustomPackage
-          def initialize(stock_location, order, splitters); end
-        end
-        allow(Spree::Config).to receive(:package_factory) { CustomPackage }
-
-        expect(::RealSplitter.new(packer).split(nil).class).to eq CustomPackage
-      end
-    end
-  end
-end
diff --git a/lib/spree/core/environment.rb b/lib/spree/core/environment.rb
new file mode 100644
index 0000000000..443b110c0a
--- /dev/null
+++ b/lib/spree/core/environment.rb
@@ -0,0 +1,14 @@
+module Spree
+  module Core
+    class Environment
+      include EnvironmentExtension
+
+      attr_accessor :calculators, :payment_methods, :preferences
+
+      def initialize
+        @calculators = Calculators.new
+        @preferences = Spree::AppConfiguration.new
+      end
+    end
+  end
+end
diff --git a/spec/config/application_spec.rb b/spec/config/application_spec.rb
deleted file mode 100644
index 40c8d1c302..0000000000
--- a/spec/config/application_spec.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-require 'spec_helper'
-
-describe Openfoodnetwork::Application, 'configuration' do
-  let(:config) { described_class.config }
-
-  it "sets OrderManagement::Stock::BasicSplitter as the only stock splitter" do
-    expect(config.spree.stock_splitters).to eq [OrderManagement::Stock::BasicSplitter]
-  end
-end
diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb
index e2a133ebbc..8eed383c6a 100644
--- a/spec/models/spree/order/checkout_spec.rb
+++ b/spec/models/spree/order/checkout_spec.rb
@@ -42,9 +42,6 @@ describe Spree::Order do
     it "can progress to delivery" do
       shipping_method.shipping_categories << other_shipping_category
 
-      # If the shipping category package splitter is enabled,
-      #   an order with products with two shipping categories will be split into two shipments
-      #   and the spec will fail with a unique constraint error on index_spree_shipments_on_order_id
       order.next
       order.next
       expect(order.state).to eq "delivery"
diff --git a/spec/models/spree/stock/packer_spec.rb b/spec/models/spree/stock/packer_spec.rb
index cd694d87e1..fb20808644 100644
--- a/spec/models/spree/stock/packer_spec.rb
+++ b/spec/models/spree/stock/packer_spec.rb
@@ -20,11 +20,6 @@ module Spree
           expect(packages.size).to eq 1
           expect(packages.first.contents.size).to eq 5
         end
-
-        it 'allows users to set splitters to an empty array' do
-          packages = Packer.new(stock_location, order, []).packages
-          expect(packages.size).to eq 1
-        end
       end
 
       context 'default_package' do

From 943cb7bf05d1dc9f90637152e6df466cdb6658c4 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 14:37:00 +0100
Subject: [PATCH 31/47] Move Stock::Package to OrderManagement::Stock::Package

---
 app/models/stock/package.rb                   | 45 --------------
 config/initializers/spree.rb                  |  2 +-
 .../order_management/stock/package.rb         | 49 +++++++++++++++
 .../order_management/stock/package_spec.rb    | 60 +++++++++++++++++++
 .../flat_percent_item_total_spec.rb           |  2 +-
 spec/models/stock/package_spec.rb             | 58 ------------------
 6 files changed, 111 insertions(+), 105 deletions(-)
 delete mode 100644 app/models/stock/package.rb
 create mode 100644 engines/order_management/app/services/order_management/stock/package.rb
 create mode 100644 engines/order_management/spec/services/order_management/stock/package_spec.rb
 delete mode 100644 spec/models/stock/package_spec.rb

diff --git a/app/models/stock/package.rb b/app/models/stock/package.rb
deleted file mode 100644
index eada3741c1..0000000000
--- a/app/models/stock/package.rb
+++ /dev/null
@@ -1,45 +0,0 @@
-# Extends Spree's Package implementation to skip shipping methods that are not
-# valid for OFN.
-#
-# It requires the following configuration in config/initializers/spree.rb:
-#
-#   Spree.config do |config|
-#     ...
-#     config.package_factory = Stock::Package
-#   end
-#
-module Stock
-  class Package < Spree::Stock::Package
-    # Returns all existing shipping categories.
-    #   It does not filter by the shipping categories of the products in the order.
-    #   It allows checkout of products with categories that are not the shipping methods categories
-    #   It disables the matching of product shipping category with shipping method's category
-    #
-    # @return [Array<Spree::ShippingCategory>]
-    def shipping_categories
-      Spree::ShippingCategory.all
-    end
-
-    # Skips the methods that are not used by the order's distributor
-    #
-    # @return [Array<Spree::ShippingMethod>]
-    def shipping_methods
-      available_shipping_methods = super.to_a
-
-      available_shipping_methods.keep_if do |shipping_method|
-        ships_with?(order.distributor.shipping_methods.to_a, shipping_method)
-      end
-    end
-
-    private
-
-    # Checks whether the given distributor provides the specified shipping method
-    #
-    # @param shipping_methods [Array<Spree::ShippingMethod>]
-    # @param shipping_method [Spree::ShippingMethod]
-    # @return [Boolean]
-    def ships_with?(shipping_methods, shipping_method)
-      shipping_methods.include?(shipping_method)
-    end
-  end
-end
diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb
index 5aca0161f8..d331a0567f 100644
--- a/config/initializers/spree.rb
+++ b/config/initializers/spree.rb
@@ -30,7 +30,7 @@ Spree.config do |config|
   config.auto_capture = true
   #config.override_actionmailer_config = false
 
-  config.package_factory = Stock::Package
+  config.package_factory = OrderManagement::Stock::Package
   config.order_updater_decorator = OrderUpdater
 
   # S3 settings
diff --git a/engines/order_management/app/services/order_management/stock/package.rb b/engines/order_management/app/services/order_management/stock/package.rb
new file mode 100644
index 0000000000..6397c6d90e
--- /dev/null
+++ b/engines/order_management/app/services/order_management/stock/package.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+# Extends Spree's Package implementation to skip shipping methods that are not
+# valid for OFN.
+#
+# It requires the following configuration in config/initializers/spree.rb:
+#
+#   Spree.config do |config|
+#     ...
+#     config.package_factory = OrderManagement::Stock::Package
+#   end
+#
+module OrderManagement
+  module Stock
+    class Package < Spree::Stock::Package
+      # Returns all existing shipping categories.
+      #   It does not filter by the shipping categories of the products in the order: it allows
+      #     checkout of products with categories that are not the shipping method's categories
+      #   It disables the matching of product shipping category with shipping method's category
+      #
+      # @return [Array<Spree::ShippingCategory>]
+      def shipping_categories
+        Spree::ShippingCategory.all
+      end
+
+      # Skips the methods that are not used by the order's distributor
+      #
+      # @return [Array<Spree::ShippingMethod>]
+      def shipping_methods
+        available_shipping_methods = super.to_a
+
+        available_shipping_methods.keep_if do |shipping_method|
+          ships_with?(order.distributor.shipping_methods.to_a, shipping_method)
+        end
+      end
+
+      private
+
+      # Checks whether the given distributor provides the specified shipping method
+      #
+      # @param shipping_methods [Array<Spree::ShippingMethod>]
+      # @param shipping_method [Spree::ShippingMethod]
+      # @return [Boolean]
+      def ships_with?(shipping_methods, shipping_method)
+        shipping_methods.include?(shipping_method)
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/stock/package_spec.rb b/engines/order_management/spec/services/order_management/stock/package_spec.rb
new file mode 100644
index 0000000000..dd06edb775
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/stock/package_spec.rb
@@ -0,0 +1,60 @@
+require 'spec_helper'
+
+module OrderManagement
+  module Stock
+    describe Package do
+      let(:stock_location) { double(:stock_location) }
+
+      subject(:package) { Package.new(stock_location, order, contents) }
+
+      let(:enterprise) { create(:enterprise) }
+      let(:other_enterprise) { create(:enterprise) }
+
+      let(:order) { build(:order, distributor: enterprise) }
+
+      let(:variant1) do
+        instance_double(
+          Spree::Variant,
+          shipping_category: shipping_method1.shipping_categories.first
+        )
+      end
+      let(:variant2) do
+        instance_double(
+          Spree::Variant,
+          shipping_category: shipping_method2.shipping_categories.first
+        )
+      end
+      let(:variant3) do
+        instance_double(Spree::Variant, shipping_category: nil)
+      end
+
+      let(:contents) do
+        [
+          Package::ContentItem.new(variant1, 1),
+          Package::ContentItem.new(variant1, 1),
+          Package::ContentItem.new(variant2, 1),
+          Package::ContentItem.new(variant3, 1)
+        ]
+      end
+
+      let(:shipping_method1) { create(:shipping_method, distributors: [enterprise]) }
+      let(:shipping_method2) { create(:shipping_method, distributors: [other_enterprise]) }
+
+      describe '#shipping_methods' do
+        it 'does not return shipping methods not used by the package\'s order distributor' do
+          expect(package.shipping_methods).to eq [shipping_method1]
+        end
+      end
+
+      describe '#shipping_categories' do
+        it "returns shipping categories that are not shipping categories of the order's products" do
+          package
+          other_shipping_category = Spree::ShippingCategory.create(name: "Custom")
+
+          expect(package.shipping_categories).to eq [shipping_method1.shipping_categories.first,
+                                                     other_shipping_category]
+        end
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/calculator/flat_percent_item_total_spec.rb b/spec/models/spree/calculator/flat_percent_item_total_spec.rb
index 529c2b11d9..ec4b54fc70 100644
--- a/spec/models/spree/calculator/flat_percent_item_total_spec.rb
+++ b/spec/models/spree/calculator/flat_percent_item_total_spec.rb
@@ -14,7 +14,7 @@ describe Spree::Calculator::FlatPercentItemTotal do
     it_behaves_like "a model using the LocalizedNumber module", [:preferred_flat_percent]
   end
 
-  it "computes amount correctly for a given Stock::Package" do
+  it "computes amount correctly for a given OrderManagement::Stock::Package" do
     order = double(:order, line_items: [line_item] )
     package = double(:package, order: order)
 
diff --git a/spec/models/stock/package_spec.rb b/spec/models/stock/package_spec.rb
deleted file mode 100644
index 6a3ac355cc..0000000000
--- a/spec/models/stock/package_spec.rb
+++ /dev/null
@@ -1,58 +0,0 @@
-require 'spec_helper'
-
-module Stock
-  describe Package do
-    let(:stock_location) { double(:stock_location) }
-
-    subject(:package) { Package.new(stock_location, order, contents) }
-
-    let(:enterprise) { create(:enterprise) }
-    let(:other_enterprise) { create(:enterprise) }
-
-    let(:order) { build(:order, distributor: enterprise) }
-
-    let(:variant1) do
-      instance_double(
-        Spree::Variant,
-        shipping_category: shipping_method1.shipping_categories.first
-      )
-    end
-    let(:variant2) do
-      instance_double(
-        Spree::Variant,
-        shipping_category: shipping_method2.shipping_categories.first
-      )
-    end
-    let(:variant3) do
-      instance_double(Spree::Variant, shipping_category: nil)
-    end
-
-    let(:contents) do
-      [
-        Package::ContentItem.new(variant1, 1),
-        Package::ContentItem.new(variant1, 1),
-        Package::ContentItem.new(variant2, 1),
-        Package::ContentItem.new(variant3, 1)
-      ]
-    end
-
-    let(:shipping_method1) { create(:shipping_method, distributors: [enterprise]) }
-    let(:shipping_method2) { create(:shipping_method, distributors: [other_enterprise]) }
-
-    describe '#shipping_methods' do
-      it 'does not return shipping methods not used by the package\'s order distributor' do
-        expect(package.shipping_methods).to eq [shipping_method1]
-      end
-    end
-
-    describe '#shipping_categories' do
-      it "returns shipping categories that are not shipping categories of the order's products" do
-        package
-        other_shipping_category = Spree::ShippingCategory.create(name: "Custom")
-
-        expect(package.shipping_categories).to eq [shipping_method1.shipping_categories.first,
-                                                   other_shipping_category]
-      end
-    end
-  end
-end

From f0b3ed0d79973279174d3ce61e8be8ded4842f08 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 14:47:01 +0100
Subject: [PATCH 32/47] Merge Spree::Stock::Package into
 OrderManagement::Stock::Package

---
 .../spree/app_configuration_decorator.rb      |   4 +
 app/models/spree/stock/package.rb             | 118 -----------
 config/initializers/spree.rb                  |   1 -
 .../order_management/stock/package.rb         | 120 +++++++++--
 .../order_management/stock/package_spec.rb    | 191 ++++++++++++++----
 spec/models/spree/stock/package_spec.rb       | 115 -----------
 spec/models/spree/stock/packer_spec.rb        |  13 --
 7 files changed, 262 insertions(+), 300 deletions(-)
 delete mode 100644 app/models/spree/stock/package.rb
 delete mode 100644 spec/models/spree/stock/package_spec.rb

diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb
index a039ddbd45..4a2be00f6e 100644
--- a/app/models/spree/app_configuration_decorator.rb
+++ b/app/models/spree/app_configuration_decorator.rb
@@ -41,4 +41,8 @@ Spree::AppConfiguration.class_eval do
 
   # Enable cache
   preference :enable_products_cache?, :boolean, default: (Rails.env.production? || Rails.env.staging?)
+
+  def package_factory
+    @package_factory ||= OrderManagement::Stock::Package
+  end
 end
diff --git a/app/models/spree/stock/package.rb b/app/models/spree/stock/package.rb
deleted file mode 100644
index e724b3efa1..0000000000
--- a/app/models/spree/stock/package.rb
+++ /dev/null
@@ -1,118 +0,0 @@
-# frozen_string_literal: true
-
-module Spree
-  module Stock
-    class Package
-      ContentItem = Struct.new(:variant, :quantity, :state)
-
-      attr_reader :stock_location, :order, :contents
-      attr_accessor :shipping_rates
-
-      def initialize(stock_location, order, contents = [])
-        @stock_location = stock_location
-        @order = order
-        @contents = contents
-        @shipping_rates = []
-      end
-
-      def add(variant, quantity, state = :on_hand)
-        contents << ContentItem.new(variant, quantity, state)
-      end
-
-      def weight
-        contents.sum { |item| item.variant.weight * item.quantity }
-      end
-
-      def on_hand
-        contents.select { |item| item.state == :on_hand }
-      end
-
-      def backordered
-        contents.select { |item| item.state == :backordered }
-      end
-
-      def find_item(variant, state = :on_hand)
-        contents.select do |item|
-          item.variant == variant &&
-            item.state == state
-        end.first
-      end
-
-      def quantity(state = nil)
-        case state
-        when :on_hand
-          on_hand.sum(&:quantity)
-        when :backordered
-          backordered.sum(&:quantity)
-        else
-          contents.sum(&:quantity)
-        end
-      end
-
-      def empty?
-        quantity.zero?
-      end
-
-      def flattened
-        flat = []
-        contents.each do |item|
-          item.quantity.times do
-            flat << ContentItem.new(item.variant, 1, item.state)
-          end
-        end
-        flat
-      end
-
-      def flattened=(flattened)
-        contents.clear
-        flattened.each do |item|
-          current_item = find_item(item.variant, item.state)
-          if current_item
-            current_item.quantity += 1
-          else
-            add(item.variant, item.quantity, item.state)
-          end
-        end
-      end
-
-      def currency
-        # TODO calculate from first variant?
-      end
-
-      def shipping_categories
-        contents.map { |item| item.variant.shipping_category }.compact.uniq
-      end
-
-      def shipping_methods
-        shipping_categories.map(&:shipping_methods).flatten.uniq
-      end
-
-      def inspect
-        out = "#{order} - "
-        out << contents.map do |content_item|
-          "#{content_item.variant.name} #{content_item.quantity} #{content_item.state}"
-        end.join('/')
-        out
-      end
-
-      def to_shipment
-        shipment = Spree::Shipment.new
-        shipment.order = order
-        shipment.stock_location = stock_location
-        shipment.shipping_rates = shipping_rates
-
-        contents.each do |item|
-          item.quantity.times do
-            unit = shipment.inventory_units.build
-            unit.pending = true
-            unit.order = order
-            unit.variant = item.variant
-            unit.state = item.state.to_s
-          end
-        end
-
-        shipment
-      end
-    end
-  end
-end
diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb
index d331a0567f..6ff230c5f5 100644
--- a/config/initializers/spree.rb
+++ b/config/initializers/spree.rb
@@ -30,7 +30,6 @@ Spree.config do |config|
   config.auto_capture = true
   #config.override_actionmailer_config = false
 
-  config.package_factory = OrderManagement::Stock::Package
   config.order_updater_decorator = OrderUpdater
 
   # S3 settings
diff --git a/engines/order_management/app/services/order_management/stock/package.rb b/engines/order_management/app/services/order_management/stock/package.rb
index 6397c6d90e..3009d0b19c 100644
--- a/engines/order_management/app/services/order_management/stock/package.rb
+++ b/engines/order_management/app/services/order_management/stock/package.rb
@@ -1,22 +1,87 @@
 # frozen_string_literal: true
 
-# Extends Spree's Package implementation to skip shipping methods that are not
-# valid for OFN.
-#
-# It requires the following configuration in config/initializers/spree.rb:
-#
-#   Spree.config do |config|
-#     ...
-#     config.package_factory = OrderManagement::Stock::Package
-#   end
-#
 module OrderManagement
   module Stock
-    class Package < Spree::Stock::Package
+    class Package
+      ContentItem = Struct.new(:variant, :quantity, :state)
+
+      attr_reader :stock_location, :order, :contents
+      attr_accessor :shipping_rates
+
+      def initialize(stock_location, order, contents = [])
+        @stock_location = stock_location
+        @order = order
+        @contents = contents
+        @shipping_rates = []
+      end
+
+      def add(variant, quantity, state = :on_hand)
+        contents << ContentItem.new(variant, quantity, state)
+      end
+
+      def weight
+        contents.sum { |item| item.variant.weight * item.quantity }
+      end
+
+      def on_hand
+        contents.select { |item| item.state == :on_hand }
+      end
+
+      def backordered
+        contents.select { |item| item.state == :backordered }
+      end
+
+      def find_item(variant, state = :on_hand)
+        contents.select do |item|
+          item.variant == variant &&
+            item.state == state
+        end.first
+      end
+
+      def quantity(state = nil)
+        case state
+        when :on_hand
+          on_hand.sum(&:quantity)
+        when :backordered
+          backordered.sum(&:quantity)
+        else
+          contents.sum(&:quantity)
+        end
+      end
+
+      def empty?
+        quantity.zero?
+      end
+
+      def flattened
+        flat = []
+        contents.each do |item|
+          item.quantity.times do
+            flat << ContentItem.new(item.variant, 1, item.state)
+          end
+        end
+        flat
+      end
+
+      def flattened=(flattened)
+        contents.clear
+        flattened.each do |item|
+          current_item = find_item(item.variant, item.state)
+          if current_item
+            current_item.quantity += 1
+          else
+            add(item.variant, item.quantity, item.state)
+          end
+        end
+      end
+
+      def currency
+        # TODO calculate from first variant?
+      end
+
       # Returns all existing shipping categories.
-      #   It does not filter by the shipping categories of the products in the order: it allows
-      #     checkout of products with categories that are not the shipping method's categories
       #   It disables the matching of product shipping category with shipping method's category
+      #   It allows checkout of products with categories that are not the ship method's categories
       #
       # @return [Array<Spree::ShippingCategory>]
       def shipping_categories
@@ -27,13 +92,40 @@ module OrderManagement
       #
       # @return [Array<Spree::ShippingMethod>]
       def shipping_methods
-        available_shipping_methods = super.to_a
+        available_shipping_methods = shipping_categories.map(&:shipping_methods).flatten.uniq.to_a
 
         available_shipping_methods.keep_if do |shipping_method|
           ships_with?(order.distributor.shipping_methods.to_a, shipping_method)
         end
       end
 
+      def inspect
+        out = "#{order} - "
+        out << contents.map do |content_item|
+          "#{content_item.variant.name} #{content_item.quantity} #{content_item.state}"
+        end.join('/')
+        out
+      end
+
+      def to_shipment
+        shipment = Spree::Shipment.new
+        shipment.order = order
+        shipment.stock_location = stock_location
+        shipment.shipping_rates = shipping_rates
+
+        contents.each do |item|
+          item.quantity.times do
+            unit = shipment.inventory_units.build
+            unit.pending = true
+            unit.order = order
+            unit.variant = item.variant
+            unit.state = item.state.to_s
+          end
+        end
+
+        shipment
+      end
+
       private
 
       # Checks whether the given distributor provides the specified shipping method
diff --git a/engines/order_management/spec/services/order_management/stock/package_spec.rb b/engines/order_management/spec/services/order_management/stock/package_spec.rb
index dd06edb775..205d50ae1a 100644
--- a/engines/order_management/spec/services/order_management/stock/package_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/package_spec.rb
@@ -1,58 +1,171 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 module OrderManagement
   module Stock
     describe Package do
-      let(:stock_location) { double(:stock_location) }
+      context "base tests" do
+        let(:variant) { build(:variant, weight: 25.0) }
+        let(:stock_location) { build(:stock_location) }
+        let(:distributor) { create(:enterprise) }
+        let(:order) { build(:order, distributor: distributor) }
 
-      subject(:package) { Package.new(stock_location, order, contents) }
+        subject { Package.new(stock_location, order) }
 
-      let(:enterprise) { create(:enterprise) }
-      let(:other_enterprise) { create(:enterprise) }
+        it 'calculates the weight of all the contents' do
+          subject.add variant, 4
+          expect(subject.weight).to eq 100.0
+        end
 
-      let(:order) { build(:order, distributor: enterprise) }
+        it 'filters by on_hand and backordered' do
+          subject.add variant, 4, :on_hand
+          subject.add variant, 3, :backordered
+          expect(subject.on_hand.count).to eq 1
+          expect(subject.backordered.count).to eq 1
+        end
 
-      let(:variant1) do
-        instance_double(
-          Spree::Variant,
-          shipping_category: shipping_method1.shipping_categories.first
-        )
-      end
-      let(:variant2) do
-        instance_double(
-          Spree::Variant,
-          shipping_category: shipping_method2.shipping_categories.first
-        )
-      end
-      let(:variant3) do
-        instance_double(Spree::Variant, shipping_category: nil)
-      end
+        it 'calculates the quantity by state' do
+          subject.add variant, 4, :on_hand
+          subject.add variant, 3, :backordered
 
-      let(:contents) do
-        [
-          Package::ContentItem.new(variant1, 1),
-          Package::ContentItem.new(variant1, 1),
-          Package::ContentItem.new(variant2, 1),
-          Package::ContentItem.new(variant3, 1)
-        ]
-      end
+          expect(subject.quantity).to eq 7
+          expect(subject.quantity(:on_hand)).to eq 4
+          expect(subject.quantity(:backordered)).to eq 3
+        end
 
-      let(:shipping_method1) { create(:shipping_method, distributors: [enterprise]) }
-      let(:shipping_method2) { create(:shipping_method, distributors: [other_enterprise]) }
+        it 'returns nil for content item not found' do
+          item = subject.find_item(variant, :on_hand)
+          expect(item).to be_nil
+        end
 
-      describe '#shipping_methods' do
-        it 'does not return shipping methods not used by the package\'s order distributor' do
-          expect(package.shipping_methods).to eq [shipping_method1]
+        it 'finds content item for a variant' do
+          subject.add variant, 4, :on_hand
+          item = subject.find_item(variant, :on_hand)
+          expect(item.quantity).to eq 4
+        end
+
+        it 'get flattened contents' do
+          subject.add variant, 4, :on_hand
+          subject.add variant, 2, :backordered
+          flattened = subject.flattened
+          expect(flattened.select { |i| i.state == :on_hand }.size).to eq 4
+          expect(flattened.select { |i| i.state == :backordered }.size).to eq 2
+        end
+
+        it 'set contents from flattened' do
+          flattened = [Package::ContentItem.new(variant, 1, :on_hand),
+                       Package::ContentItem.new(variant, 1, :on_hand),
+                       Package::ContentItem.new(variant, 1, :backordered),
+                       Package::ContentItem.new(variant, 1, :backordered)]
+
+          subject.flattened = flattened
+          expect(subject.on_hand.size).to eq 1
+          expect(subject.on_hand.first.quantity).to eq 2
+
+          expect(subject.backordered.size).to eq 1
+        end
+
+        # Contains regression test for #2804
+        it 'builds a list of shipping methods from all categories' do
+          shipping_method1 = create(:shipping_method, distributors: [distributor])
+          shipping_method2 = create(:shipping_method, distributors: [distributor])
+          variant1 = create(:variant,
+                            shipping_category: shipping_method1.shipping_categories.first)
+          variant2 = create(:variant,
+                            shipping_category: shipping_method2.shipping_categories.first)
+          variant3 = create(:variant, shipping_category: nil)
+          contents = [Package::ContentItem.new(variant1, 1),
+                      Package::ContentItem.new(variant1, 1),
+                      Package::ContentItem.new(variant2, 1),
+                      Package::ContentItem.new(variant3, 1)]
+
+          package = Package.new(stock_location, order, contents)
+          expect(package.shipping_methods.size).to eq 2
+        end
+
+        it "can convert to a shipment" do
+          flattened = [Package::ContentItem.new(variant, 2, :on_hand),
+                       Package::ContentItem.new(variant, 1, :backordered)]
+          subject.flattened = flattened
+
+          shipping_method = build(:shipping_method)
+          subject.shipping_rates = [
+            Spree::ShippingRate.new(shipping_method: shipping_method, cost: 10.00, selected: true)
+          ]
+
+          shipment = subject.to_shipment
+          expect(shipment.order).to eq subject.order
+          expect(shipment.stock_location).to eq subject.stock_location
+          expect(shipment.inventory_units.size).to eq 3
+
+          first_unit = shipment.inventory_units.first
+          expect(first_unit.variant).to eq variant
+          expect(first_unit.state).to eq 'on_hand'
+          expect(first_unit.order).to eq subject.order
+          expect(first_unit).to be_pending
+
+          last_unit = shipment.inventory_units.last
+          expect(last_unit.variant).to eq variant
+          expect(last_unit.state).to eq 'backordered'
+          expect(last_unit.order).to eq subject.order
+
+          expect(shipment.shipping_method).to eq shipping_method
         end
       end
 
-      describe '#shipping_categories' do
-        it "returns shipping categories that are not shipping categories of the order's products" do
-          package
-          other_shipping_category = Spree::ShippingCategory.create(name: "Custom")
+      context "#shipping_methods and #shipping_categories" do
+        let(:stock_location) { double(:stock_location) }
 
-          expect(package.shipping_categories).to eq [shipping_method1.shipping_categories.first,
-                                                     other_shipping_category]
+        subject(:package) { Package.new(stock_location, order, contents) }
+
+        let(:enterprise) { create(:enterprise) }
+        let(:other_enterprise) { create(:enterprise) }
+
+        let(:order) { build(:order, distributor: enterprise) }
+
+        let(:variant1) do
+          instance_double(
+            Spree::Variant,
+            shipping_category: shipping_method1.shipping_categories.first
+          )
+        end
+        let(:variant2) do
+          instance_double(
+            Spree::Variant,
+            shipping_category: shipping_method2.shipping_categories.first
+          )
+        end
+        let(:variant3) do
+          instance_double(Spree::Variant, shipping_category: nil)
+        end
+
+        let(:contents) do
+          [
+            Package::ContentItem.new(variant1, 1),
+            Package::ContentItem.new(variant1, 1),
+            Package::ContentItem.new(variant2, 1),
+            Package::ContentItem.new(variant3, 1)
+          ]
+        end
+
+        let(:shipping_method1) { create(:shipping_method, distributors: [enterprise]) }
+        let(:shipping_method2) { create(:shipping_method, distributors: [other_enterprise]) }
+
+        describe '#shipping_methods' do
+          it 'does not return shipping methods not used by the package\'s order distributor' do
+            expect(package.shipping_methods).to eq [shipping_method1]
+          end
+        end
+
+        describe '#shipping_categories' do
+          it "returns ship categories that are not the ship categories of the order's products" do
+            package
+            other_shipping_category = Spree::ShippingCategory.create(name: "Custom")
+
+            expect(package.shipping_categories).to eq [shipping_method1.shipping_categories.first,
+                                                       other_shipping_category]
+          end
         end
       end
     end
diff --git a/spec/models/spree/stock/package_spec.rb b/spec/models/spree/stock/package_spec.rb
deleted file mode 100644
index 6b27b405ac..0000000000
--- a/spec/models/spree/stock/package_spec.rb
+++ /dev/null
@@ -1,115 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-module Spree
-  module Stock
-    describe Package do
-      let(:variant) { build(:variant, weight: 25.0) }
-      let(:stock_location) { build(:stock_location) }
-      let(:order) { build(:order) }
-
-      subject { Package.new(stock_location, order) }
-
-      it 'calculates the weight of all the contents' do
-        subject.add variant, 4
-        expect(subject.weight).to eq 100.0
-      end
-
-      it 'filters by on_hand and backordered' do
-        subject.add variant, 4, :on_hand
-        subject.add variant, 3, :backordered
-        expect(subject.on_hand.count).to eq 1
-        expect(subject.backordered.count).to eq 1
-      end
-
-      it 'calculates the quantity by state' do
-        subject.add variant, 4, :on_hand
-        subject.add variant, 3, :backordered
-
-        expect(subject.quantity).to eq 7
-        expect(subject.quantity(:on_hand)).to eq 4
-        expect(subject.quantity(:backordered)).to eq 3
-      end
-
-      it 'returns nil for content item not found' do
-        item = subject.find_item(variant, :on_hand)
-        expect(item).to be_nil
-      end
-
-      it 'finds content item for a variant' do
-        subject.add variant, 4, :on_hand
-        item = subject.find_item(variant, :on_hand)
-        expect(item.quantity).to eq 4
-      end
-
-      it 'get flattened contents' do
-        subject.add variant, 4, :on_hand
-        subject.add variant, 2, :backordered
-        flattened = subject.flattened
-        expect(flattened.select { |i| i.state == :on_hand }.size).to eq 4
-        expect(flattened.select { |i| i.state == :backordered }.size).to eq 2
-      end
-
-      it 'set contents from flattened' do
-        flattened = [Package::ContentItem.new(variant, 1, :on_hand),
-                     Package::ContentItem.new(variant, 1, :on_hand),
-                     Package::ContentItem.new(variant, 1, :backordered),
-                     Package::ContentItem.new(variant, 1, :backordered)]
-
-        subject.flattened = flattened
-        expect(subject.on_hand.size).to eq 1
-        expect(subject.on_hand.first.quantity).to eq 2
-
-        expect(subject.backordered.size).to eq 1
-      end
-
-      # Contains regression test for #2804
-      it 'builds a list of shipping methods from all categories' do
-        shipping_method1 = create(:shipping_method)
-        shipping_method2 = create(:shipping_method)
-        variant1 = create(:variant,
-                          shipping_category: shipping_method1.shipping_categories.first)
-        variant2 = create(:variant,
-                          shipping_category: shipping_method2.shipping_categories.first)
-        variant3 = create(:variant, shipping_category: nil)
-        contents = [Package::ContentItem.new(variant1, 1),
-                    Package::ContentItem.new(variant1, 1),
-                    Package::ContentItem.new(variant2, 1),
-                    Package::ContentItem.new(variant3, 1)]
-
-        package = Package.new(stock_location, order, contents)
-        expect(package.shipping_methods.size).to eq 2
-      end
-
-      it "can convert to a shipment" do
-        flattened = [Package::ContentItem.new(variant, 2, :on_hand),
-                     Package::ContentItem.new(variant, 1, :backordered)]
-        subject.flattened = flattened
-
-        shipping_method = build(:shipping_method)
-        subject.shipping_rates = [
-          Spree::ShippingRate.new(shipping_method: shipping_method, cost: 10.00, selected: true)
-        ]
-
-        shipment = subject.to_shipment
-        expect(shipment.order).to eq subject.order
-        expect(shipment.stock_location).to eq subject.stock_location
-        expect(shipment.inventory_units.size).to eq 3
-
-        first_unit = shipment.inventory_units.first
-        expect(first_unit.variant).to eq variant
-        expect(first_unit.state).to eq 'on_hand'
-        expect(first_unit.order).to eq subject.order
-        expect(first_unit).to be_pending
-
-        last_unit = shipment.inventory_units.last
-        expect(last_unit.variant).to eq variant
-        expect(last_unit.state).to eq 'backordered'
-        expect(last_unit.order).to eq subject.order
-
-        expect(shipment.shipping_method).to eq shipping_method
-      end
-    end
-  end
-end
diff --git a/spec/models/spree/stock/packer_spec.rb b/spec/models/spree/stock/packer_spec.rb
index fb20808644..c5bdefd489 100644
--- a/spec/models/spree/stock/packer_spec.rb
+++ b/spec/models/spree/stock/packer_spec.rb
@@ -39,19 +39,6 @@ module Spree
           expect(package.backordered.size).to eq 5
         end
 
-        context 'when a packer factory is not specified' do
-          let(:package) { double(:package, add: true) }
-
-          it 'calls Spree::Stock::Package' do
-            expect(Package)
-              .to receive(:new)
-              .with(stock_location, order)
-              .and_return(package)
-
-            subject.default_package
-          end
-        end
-
         context 'when a packer factory is specified' do
           before do
             allow(Spree::Config).to receive(:package_factory) { TestPackageFactory }

From b487185a65a7b36276343f3f6ab0d23c6602dcb4 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 15:37:40 +0100
Subject: [PATCH 33/47] Remove package factory, it is no longer needed, we can
 just call the Package class in the two places where it is used

---
 .../spree/app_configuration_decorator.rb      |  4 ----
 app/models/spree/shipment.rb                  |  2 +-
 app/models/spree/stock/packer.rb              |  5 ++--
 spec/models/spree/stock/packer_spec.rb        | 23 -------------------
 4 files changed, 3 insertions(+), 31 deletions(-)

diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb
index 4a2be00f6e..a039ddbd45 100644
--- a/app/models/spree/app_configuration_decorator.rb
+++ b/app/models/spree/app_configuration_decorator.rb
@@ -41,8 +41,4 @@ Spree::AppConfiguration.class_eval do
 
   # Enable cache
   preference :enable_products_cache?, :boolean, default: (Rails.env.production? || Rails.env.staging?)
-
-  def package_factory
-    @package_factory ||= OrderManagement::Stock::Package
-  end
 end
diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index d35c84ca08..f271af3c8f 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -230,7 +230,7 @@ module Spree
     end
 
     def to_package
-      package = Spree::Config.package_factory.new(stock_location, order)
+      package = OrderManagement::Stock::Package.new(stock_location, order)
       inventory_units.includes(:variant).each do |inventory_unit|
         package.add inventory_unit.variant, 1, inventory_unit.state_name
       end
diff --git a/app/models/spree/stock/packer.rb b/app/models/spree/stock/packer.rb
index 6fb58531c3..a936d24c14 100644
--- a/app/models/spree/stock/packer.rb
+++ b/app/models/spree/stock/packer.rb
@@ -3,12 +3,11 @@
 module Spree
   module Stock
     class Packer
-      attr_reader :stock_location, :order, :package_factory
+      attr_reader :stock_location, :order
 
       def initialize(stock_location, order)
         @stock_location = stock_location
         @order = order
-        @package_factory = Spree::Config.package_factory
       end
 
       def packages
@@ -16,7 +15,7 @@ module Spree
       end
 
       def default_package
-        package = package_factory.new(stock_location, order)
+        package = OrderManagement::Stock::Package.new(stock_location, order)
         order.line_items.each do |line_item|
           if Config.track_inventory_levels
             next unless stock_location.stock_item(line_item.variant)
diff --git a/spec/models/spree/stock/packer_spec.rb b/spec/models/spree/stock/packer_spec.rb
index c5bdefd489..ab5da00dd4 100644
--- a/spec/models/spree/stock/packer_spec.rb
+++ b/spec/models/spree/stock/packer_spec.rb
@@ -10,10 +10,6 @@ module Spree
 
       subject { Packer.new(stock_location, order) }
 
-      before do
-        allow(Spree::Config).to receive(:package_factory) { Package }
-      end
-
       context 'packages' do
         it 'builds an array of packages' do
           packages = subject.packages
@@ -38,25 +34,6 @@ module Spree
           expect(package.on_hand.size).to eq 5
           expect(package.backordered.size).to eq 5
         end
-
-        context 'when a packer factory is specified' do
-          before do
-            allow(Spree::Config).to receive(:package_factory) { TestPackageFactory }
-          end
-
-          class TestPackageFactory; end
-
-          let(:package) { double(:package, add: true) }
-
-          it 'calls the specified factory' do
-            expect(TestPackageFactory)
-              .to receive(:new)
-              .with(stock_location, order)
-              .and_return(package)
-
-            subject.default_package
-          end
-        end
       end
     end
   end

From 01b1abbd5283d3a0e2456561f3870b1d01fce5ee Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 15:42:01 +0100
Subject: [PATCH 34/47] Bring method from Spree::Order so that we can move
 Coordiantor to the order management engine

---
 app/models/spree/order_decorator.rb | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb
index a36d8e1312..f45e3340a0 100644
--- a/app/models/spree/order_decorator.rb
+++ b/app/models/spree/order_decorator.rb
@@ -89,6 +89,18 @@ Spree::Order.class_eval do
     where("state != ?", state)
   }
 
+  def create_proposed_shipments
+    adjustments.shipping.delete_all
+    shipments.destroy_all
+
+    packages = Spree::Stock::Coordinator.new(self).packages
+    packages.each do |package|
+      shipments << package.to_shipment
+    end
+
+    shipments
+  end
+
   # -- Methods
   def products_available_from_new_distribution
     # Check that the line_items in the current order are available from a newly selected distribution

From 83974a832c47ffe93e04a845532d0c68769d071e Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 15:44:33 +0100
Subject: [PATCH 35/47] Move Coordinator from Spree::Stock to
 OrderManagement::Stock

---
 app/models/spree/order_decorator.rb                    |  2 +-
 .../services/order_management}/stock/coordinator.rb    | 10 +++++-----
 .../order_management}/stock/coordinator_spec.rb        |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)
 rename {app/models/spree => engines/order_management/app/services/order_management}/stock/coordinator.rb (84%)
 rename {spec/models/spree => engines/order_management/spec/services/order_management}/stock/coordinator_spec.rb (96%)

diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb
index f45e3340a0..71574359a7 100644
--- a/app/models/spree/order_decorator.rb
+++ b/app/models/spree/order_decorator.rb
@@ -93,7 +93,7 @@ Spree::Order.class_eval do
     adjustments.shipping.delete_all
     shipments.destroy_all
 
-    packages = Spree::Stock::Coordinator.new(self).packages
+    packages = OrderManagement::Stock::Coordinator.new(self).packages
     packages.each do |package|
       shipments << package.to_shipment
     end
diff --git a/app/models/spree/stock/coordinator.rb b/engines/order_management/app/services/order_management/stock/coordinator.rb
similarity index 84%
rename from app/models/spree/stock/coordinator.rb
rename to engines/order_management/app/services/order_management/stock/coordinator.rb
index bbbc176956..84cea7d9cf 100644
--- a/app/models/spree/stock/coordinator.rb
+++ b/engines/order_management/app/services/order_management/stock/coordinator.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Spree
+module OrderManagement
   module Stock
     class Coordinator
       attr_reader :order
@@ -25,7 +25,7 @@ module Spree
       #
       # Returns an array of Package instances
       def build_packages(packages = [])
-        StockLocation.active.each do |stock_location|
+        Spree::StockLocation.active.each do |stock_location|
           next unless stock_location.stock_items.
             where(variant_id: order.line_items.pluck(:variant_id)).exists?
 
@@ -38,12 +38,12 @@ module Spree
       private
 
       def prioritize_packages(packages)
-        prioritizer = Prioritizer.new(order, packages)
+        prioritizer = Spree::Stock::Prioritizer.new(order, packages)
         prioritizer.prioritized_packages
       end
 
       def estimate_packages(packages)
-        estimator = Estimator.new(order)
+        estimator = Spree::Stock::Estimator.new(order)
         packages.each do |package|
           package.shipping_rates = estimator.shipping_rates(package)
         end
@@ -51,7 +51,7 @@ module Spree
       end
 
       def build_packer(stock_location, order)
-        Packer.new(stock_location, order)
+        Spree::Stock::Packer.new(stock_location, order)
       end
     end
   end
diff --git a/spec/models/spree/stock/coordinator_spec.rb b/engines/order_management/spec/services/order_management/stock/coordinator_spec.rb
similarity index 96%
rename from spec/models/spree/stock/coordinator_spec.rb
rename to engines/order_management/spec/services/order_management/stock/coordinator_spec.rb
index 63f81e96ce..c2ab44e5f7 100644
--- a/spec/models/spree/stock/coordinator_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/coordinator_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-module Spree
+module OrderManagement
   module Stock
     describe Coordinator do
       let!(:order) { create(:order_with_line_items, distributor: create(:distributor_enterprise)) }

From ee66e37521f6f7fe2d72f9a983f6bcb55e762a7f Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 20:20:04 +0100
Subject: [PATCH 36/47] Move adjuster, estimator, packer and prioritizer to
 order management engine

---
 app/models/spree/shipment.rb                         |  2 +-
 .../app/services/order_management}/stock/adjuster.rb |  2 +-
 .../services/order_management/stock/coordinator.rb   |  6 +++---
 .../services/order_management}/stock/estimator.rb    |  2 +-
 .../app/services/order_management}/stock/packer.rb   |  4 ++--
 .../services/order_management}/stock/prioritizer.rb  |  4 ++--
 .../order_management}/stock/estimator_spec.rb        | 12 ++++++------
 .../services/order_management}/stock/packer_spec.rb  |  2 +-
 .../order_management}/stock/prioritizer_spec.rb      |  2 +-
 spec/models/spree/shipment_spec.rb                   |  6 +++---
 10 files changed, 21 insertions(+), 21 deletions(-)
 rename {app/models/spree => engines/order_management/app/services/order_management}/stock/adjuster.rb (96%)
 rename {app/models/spree => engines/order_management/app/services/order_management}/stock/estimator.rb (98%)
 rename {app/models/spree => engines/order_management/app/services/order_management}/stock/packer.rb (92%)
 rename {app/models/spree => engines/order_management/app/services/order_management}/stock/prioritizer.rb (90%)
 rename {spec/models/spree => engines/order_management/spec/services/order_management}/stock/estimator_spec.rb (89%)
 rename {spec/models/spree => engines/order_management/spec/services/order_management}/stock/packer_spec.rb (98%)
 rename {spec/models/spree => engines/order_management/spec/services/order_management}/stock/prioritizer_spec.rb (99%)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index f271af3c8f..68002d6ee2 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -107,7 +107,7 @@ module Spree
     def refresh_rates
       return shipping_rates if shipped?
 
-      self.shipping_rates = Stock::Estimator.new(order).shipping_rates(to_package)
+      self.shipping_rates = OrderManagement::Stock::Estimator.new(order).shipping_rates(to_package)
 
       if shipping_method
         selected_rate = shipping_rates.detect { |rate|
diff --git a/app/models/spree/stock/adjuster.rb b/engines/order_management/app/services/order_management/stock/adjuster.rb
similarity index 96%
rename from app/models/spree/stock/adjuster.rb
rename to engines/order_management/app/services/order_management/stock/adjuster.rb
index 4926e8a7e6..ed1337a16a 100644
--- a/app/models/spree/stock/adjuster.rb
+++ b/engines/order_management/app/services/order_management/stock/adjuster.rb
@@ -2,7 +2,7 @@
 
 # Used by Prioritizer to adjust item quantities
 # see prioritizer_spec for use cases
-module Spree
+module OrderManagement
   module Stock
     class Adjuster
       attr_accessor :variant, :need, :status
diff --git a/engines/order_management/app/services/order_management/stock/coordinator.rb b/engines/order_management/app/services/order_management/stock/coordinator.rb
index 84cea7d9cf..215a82ffe3 100644
--- a/engines/order_management/app/services/order_management/stock/coordinator.rb
+++ b/engines/order_management/app/services/order_management/stock/coordinator.rb
@@ -38,12 +38,12 @@ module OrderManagement
       private
 
       def prioritize_packages(packages)
-        prioritizer = Spree::Stock::Prioritizer.new(order, packages)
+        prioritizer = OrderManagement::Stock::Prioritizer.new(order, packages)
         prioritizer.prioritized_packages
       end
 
       def estimate_packages(packages)
-        estimator = Spree::Stock::Estimator.new(order)
+        estimator = OrderManagement::Stock::Estimator.new(order)
         packages.each do |package|
           package.shipping_rates = estimator.shipping_rates(package)
         end
@@ -51,7 +51,7 @@ module OrderManagement
       end
 
       def build_packer(stock_location, order)
-        Spree::Stock::Packer.new(stock_location, order)
+        OrderManagement::Stock::Packer.new(stock_location, order)
       end
     end
   end
diff --git a/app/models/spree/stock/estimator.rb b/engines/order_management/app/services/order_management/stock/estimator.rb
similarity index 98%
rename from app/models/spree/stock/estimator.rb
rename to engines/order_management/app/services/order_management/stock/estimator.rb
index 9f14033590..c97d9c3b99 100644
--- a/app/models/spree/stock/estimator.rb
+++ b/engines/order_management/app/services/order_management/stock/estimator.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Spree
+module OrderManagement
   module Stock
     class Estimator
       attr_reader :order, :currency
diff --git a/app/models/spree/stock/packer.rb b/engines/order_management/app/services/order_management/stock/packer.rb
similarity index 92%
rename from app/models/spree/stock/packer.rb
rename to engines/order_management/app/services/order_management/stock/packer.rb
index a936d24c14..867e8889dd 100644
--- a/app/models/spree/stock/packer.rb
+++ b/engines/order_management/app/services/order_management/stock/packer.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Spree
+module OrderManagement
   module Stock
     class Packer
       attr_reader :stock_location, :order
@@ -17,7 +17,7 @@ module Spree
       def default_package
         package = OrderManagement::Stock::Package.new(stock_location, order)
         order.line_items.each do |line_item|
-          if Config.track_inventory_levels
+          if Spree::Config.track_inventory_levels
             next unless stock_location.stock_item(line_item.variant)
 
             on_hand, backordered = stock_location.fill_status(line_item.variant, line_item.quantity)
diff --git a/app/models/spree/stock/prioritizer.rb b/engines/order_management/app/services/order_management/stock/prioritizer.rb
similarity index 90%
rename from app/models/spree/stock/prioritizer.rb
rename to engines/order_management/app/services/order_management/stock/prioritizer.rb
index 41ef339702..16b992333f 100644
--- a/app/models/spree/stock/prioritizer.rb
+++ b/engines/order_management/app/services/order_management/stock/prioritizer.rb
@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
-module Spree
+module OrderManagement
   module Stock
     class Prioritizer
       attr_reader :packages, :order
 
-      def initialize(order, packages, adjuster_class = Adjuster)
+      def initialize(order, packages, adjuster_class = OrderManagement::Stock::Adjuster)
         @order = order
         @packages = packages
         @adjuster_class = adjuster_class
diff --git a/spec/models/spree/stock/estimator_spec.rb b/engines/order_management/spec/services/order_management/stock/estimator_spec.rb
similarity index 89%
rename from spec/models/spree/stock/estimator_spec.rb
rename to engines/order_management/spec/services/order_management/stock/estimator_spec.rb
index 6cdf8106b2..74215b031c 100644
--- a/spec/models/spree/stock/estimator_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/estimator_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-module Spree
+module OrderManagement
   module Stock
     describe Estimator do
       let!(:shipping_method) { create(:shipping_method, zones: [Spree::Zone.global] ) }
@@ -13,11 +13,11 @@ module Spree
       context "#shipping rates" do
         before(:each) do
           shipping_method.zones.first.members.create(zoneable: order.ship_address.country)
-          allow_any_instance_of(ShippingMethod).to receive_message_chain(:calculator, :available?).and_return(true)
-          allow_any_instance_of(ShippingMethod).to receive_message_chain(:calculator, :compute).and_return(4.00)
-          allow_any_instance_of(ShippingMethod).
+          allow_any_instance_of(Spree::ShippingMethod).to receive_message_chain(:calculator, :available?).and_return(true)
+          allow_any_instance_of(Spree::ShippingMethod).to receive_message_chain(:calculator, :compute).and_return(4.00)
+          allow_any_instance_of(Spree::ShippingMethod).
             to receive_message_chain(:calculator, :preferences).and_return({ currency: order.currency })
-          allow_any_instance_of(ShippingMethod).to receive_message_chain(:calculator, :marked_for_destruction?)
+          allow_any_instance_of(Spree::ShippingMethod).to receive_message_chain(:calculator, :marked_for_destruction?)
 
           allow(package).to receive_messages(shipping_methods: [shipping_method])
         end
@@ -39,7 +39,7 @@ module Spree
 
         context "the calculator is not available for that order" do
           it "does not return shipping rates from a shipping method" do
-            allow_any_instance_of(ShippingMethod).to receive_message_chain(:calculator, :available?).and_return(false)
+            allow_any_instance_of(Spree::ShippingMethod).to receive_message_chain(:calculator, :available?).and_return(false)
             shipping_rates = subject.shipping_rates(package)
             expect(shipping_rates).to eq []
           end
diff --git a/spec/models/spree/stock/packer_spec.rb b/engines/order_management/spec/services/order_management/stock/packer_spec.rb
similarity index 98%
rename from spec/models/spree/stock/packer_spec.rb
rename to engines/order_management/spec/services/order_management/stock/packer_spec.rb
index ab5da00dd4..f1e2b432bf 100644
--- a/spec/models/spree/stock/packer_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/packer_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-module Spree
+module OrderManagement
   module Stock
     describe Packer do
       let(:order) { create(:order_with_line_items, line_items_count: 5) }
diff --git a/spec/models/spree/stock/prioritizer_spec.rb b/engines/order_management/spec/services/order_management/stock/prioritizer_spec.rb
similarity index 99%
rename from spec/models/spree/stock/prioritizer_spec.rb
rename to engines/order_management/spec/services/order_management/stock/prioritizer_spec.rb
index 4715da1ab8..b1783eb734 100644
--- a/spec/models/spree/stock/prioritizer_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/prioritizer_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-module Spree
+module OrderManagement
   module Stock
     describe Prioritizer do
       let(:order) { create(:order_with_line_items, line_items_count: 2) }
diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index e3b7ef9671..457df53a6d 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -125,7 +125,7 @@ describe Spree::Shipment do
       let(:mock_estimator) { double('estimator', shipping_rates: shipping_rates) }
 
       it 'should request new rates, and maintain shipping_method selection' do
-        Spree::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
+        OrderManagement::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
         shipment.stub(shipping_method: shipping_method2)
 
         expect(shipment.refresh_rates).to eq shipping_rates
@@ -133,14 +133,14 @@ describe Spree::Shipment do
       end
 
       it 'should handle no shipping_method selection' do
-        Spree::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
+        OrderManagement::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
         shipment.stub(shipping_method: nil)
         expect(shipment.refresh_rates).to eq shipping_rates
         expect(shipment.reload.selected_shipping_rate).to_not be_nil
       end
 
       it 'should not refresh if shipment is shipped' do
-        Spree::Stock::Estimator.should_not_receive(:new)
+        OrderManagement::Stock::Estimator.should_not_receive(:new)
         shipment.shipping_rates.delete_all
         shipment.stub(shipped?: true)
         expect(shipment.refresh_rates).to eq []

From ee937988e8a72ed0ba0b6889e4535ce35a4efc7d Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 20:24:00 +0100
Subject: [PATCH 37/47] Fix easy rubocop issues

---
 .../order_management/stock/estimator_spec.rb  | 39 ++++++++++++-------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/engines/order_management/spec/services/order_management/stock/estimator_spec.rb b/engines/order_management/spec/services/order_management/stock/estimator_spec.rb
index 74215b031c..892b7e1cf2 100644
--- a/engines/order_management/spec/services/order_management/stock/estimator_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/estimator_spec.rb
@@ -13,11 +13,15 @@ module OrderManagement
       context "#shipping rates" do
         before(:each) do
           shipping_method.zones.first.members.create(zoneable: order.ship_address.country)
-          allow_any_instance_of(Spree::ShippingMethod).to receive_message_chain(:calculator, :available?).and_return(true)
-          allow_any_instance_of(Spree::ShippingMethod).to receive_message_chain(:calculator, :compute).and_return(4.00)
           allow_any_instance_of(Spree::ShippingMethod).
-            to receive_message_chain(:calculator, :preferences).and_return({ currency: order.currency })
-          allow_any_instance_of(Spree::ShippingMethod).to receive_message_chain(:calculator, :marked_for_destruction?)
+            to receive_message_chain(:calculator, :available?).and_return(true)
+          allow_any_instance_of(Spree::ShippingMethod).
+            to receive_message_chain(:calculator, :compute).and_return(4.00)
+          allow_any_instance_of(Spree::ShippingMethod).
+            to receive_message_chain(:calculator, :preferences).
+            and_return({ currency: order.currency })
+          allow_any_instance_of(Spree::ShippingMethod).
+            to receive_message_chain(:calculator, :marked_for_destruction?)
 
           allow(package).to receive_messages(shipping_methods: [shipping_method])
         end
@@ -39,7 +43,8 @@ module OrderManagement
 
         context "the calculator is not available for that order" do
           it "does not return shipping rates from a shipping method" do
-            allow_any_instance_of(Spree::ShippingMethod).to receive_message_chain(:calculator, :available?).and_return(false)
+            allow_any_instance_of(Spree::ShippingMethod).
+              to receive_message_chain(:calculator, :available?).and_return(false)
             shipping_rates = subject.shipping_rates(package)
             expect(shipping_rates).to eq []
           end
@@ -62,9 +67,12 @@ module OrderManagement
 
         it "sorts shipping rates by cost" do
           shipping_methods = 3.times.map { create(:shipping_method) }
-          allow(shipping_methods[0]).to receive_message_chain(:calculator, :compute).and_return(5.00)
-          allow(shipping_methods[1]).to receive_message_chain(:calculator, :compute).and_return(3.00)
-          allow(shipping_methods[2]).to receive_message_chain(:calculator, :compute).and_return(4.00)
+          allow(shipping_methods[0]).
+            to receive_message_chain(:calculator, :compute).and_return(5.00)
+          allow(shipping_methods[1]).
+            to receive_message_chain(:calculator, :compute).and_return(3.00)
+          allow(shipping_methods[2]).
+            to receive_message_chain(:calculator, :compute).and_return(4.00)
 
           allow(subject).to receive(:shipping_methods).and_return(shipping_methods)
 
@@ -76,8 +84,10 @@ module OrderManagement
           let(:shipping_methods) { 2.times.map { create(:shipping_method) } }
 
           it "selects the most affordable shipping rate" do
-            allow(shipping_methods[0]).to receive_message_chain(:calculator, :compute).and_return(5.00)
-            allow(shipping_methods[1]).to receive_message_chain(:calculator, :compute).and_return(3.00)
+            allow(shipping_methods[0]).
+              to receive_message_chain(:calculator, :compute).and_return(5.00)
+            allow(shipping_methods[1]).
+              to receive_message_chain(:calculator, :compute).and_return(3.00)
 
             allow(subject).to receive(:shipping_methods).and_return(shipping_methods)
 
@@ -86,8 +96,10 @@ module OrderManagement
           end
 
           it "selects the cheapest shipping rate and doesn't raise exception over nil cost" do
-            allow(shipping_methods[0]).to receive_message_chain(:calculator, :compute).and_return(1.00)
-            allow(shipping_methods[1]).to receive_message_chain(:calculator, :compute).and_return(nil)
+            allow(shipping_methods[0]).
+              to receive_message_chain(:calculator, :compute).and_return(1.00)
+            allow(shipping_methods[1]).
+              to receive_message_chain(:calculator, :compute).and_return(nil)
 
             allow(subject).to receive(:shipping_methods).and_return(shipping_methods)
 
@@ -104,7 +116,8 @@ module OrderManagement
             allow(backend_method).to receive_message_chain(:calculator, :compute).and_return(0.00)
             allow(generic_method).to receive_message_chain(:calculator, :compute).and_return(5.00)
 
-            allow(subject).to receive(:shipping_methods).and_return([backend_method, generic_method])
+            allow(subject).
+              to receive(:shipping_methods).and_return([backend_method, generic_method])
 
             expect(subject.shipping_rates(package).map(&:selected)).to eq [false, true]
           end

From ff046f7a6cd7611e79bb76a7b67e0399577534fb Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 20:25:14 +0100
Subject: [PATCH 38/47] Remove conditionals related to
 Config.track_inventory_levels, this config is always true in OFN

---
 app/models/spree/shipment.rb                         |  2 +-
 .../app/services/order_management/stock/packer.rb    | 12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index 68002d6ee2..993fa0f62c 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -172,7 +172,7 @@ module Spree
     end
 
     def line_items
-      if order.complete? && Spree::Config[:track_inventory_levels]
+      if order.complete?
         order.line_items.select { |li| inventory_units.pluck(:variant_id).include?(li.variant_id) }
       else
         order.line_items
diff --git a/engines/order_management/app/services/order_management/stock/packer.rb b/engines/order_management/app/services/order_management/stock/packer.rb
index 867e8889dd..d66881a32b 100644
--- a/engines/order_management/app/services/order_management/stock/packer.rb
+++ b/engines/order_management/app/services/order_management/stock/packer.rb
@@ -17,15 +17,11 @@ module OrderManagement
       def default_package
         package = OrderManagement::Stock::Package.new(stock_location, order)
         order.line_items.each do |line_item|
-          if Spree::Config.track_inventory_levels
-            next unless stock_location.stock_item(line_item.variant)
+          next unless stock_location.stock_item(line_item.variant)
 
-            on_hand, backordered = stock_location.fill_status(line_item.variant, line_item.quantity)
-            package.add line_item.variant, on_hand, :on_hand if on_hand.positive?
-            package.add line_item.variant, backordered, :backordered if backordered.positive?
-          else
-            package.add line_item.variant, line_item.quantity, :on_hand
-          end
+          on_hand, backordered = stock_location.fill_status(line_item.variant, line_item.quantity)
+          package.add line_item.variant, on_hand, :on_hand if on_hand.positive?
+          package.add line_item.variant, backordered, :backordered if backordered.positive?
         end
         package
       end

From d323c5bdcb3bea36c0309c05ecf22daaafbb97b1 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 20:45:01 +0100
Subject: [PATCH 39/47] Simplify packer and coordinator baed on the fact that
 there's only one stock_location so there will only be one package per order

---
 .../order_management/stock/coordinator.rb     | 27 +++++----------
 .../services/order_management/stock/packer.rb |  6 +---
 .../order_management/stock/packer_spec.rb     | 33 +++++++------------
 3 files changed, 22 insertions(+), 44 deletions(-)

diff --git a/engines/order_management/app/services/order_management/stock/coordinator.rb b/engines/order_management/app/services/order_management/stock/coordinator.rb
index 215a82ffe3..64bbbc8cbc 100644
--- a/engines/order_management/app/services/order_management/stock/coordinator.rb
+++ b/engines/order_management/app/services/order_management/stock/coordinator.rb
@@ -15,24 +15,14 @@ module OrderManagement
         estimate_packages(packages)
       end
 
-      # Build packages as per stock location
+      # Build package with default stock location
+      # No need to check items are in the stock location,
+      #   there is only one stock location so the items will be on that stock location.
       #
-      # It needs to check whether each stock location holds at least one stock
-      # item for the order. In case none is found it wouldn't make any sense
-      # to build a package because it would be empty. Plus we avoid errors down
-      # the stack because it would assume the stock location has stock items
-      # for the given order
-      #
-      # Returns an array of Package instances
-      def build_packages(packages = [])
-        Spree::StockLocation.active.each do |stock_location|
-          next unless stock_location.stock_items.
-            where(variant_id: order.line_items.pluck(:variant_id)).exists?
-
-          packer = build_packer(stock_location, order)
-          packages += packer.packages
-        end
-        packages
+      # Returns an array with a single Package for the default stock location
+      def build_packages
+        packer = build_packer(order)
+        [packer.package]
       end
 
       private
@@ -50,7 +40,8 @@ module OrderManagement
         packages
       end
 
-      def build_packer(stock_location, order)
+      def build_packer(order)
+        stock_location = DefaultStockLocation.find_or_create
         OrderManagement::Stock::Packer.new(stock_location, order)
       end
     end
diff --git a/engines/order_management/app/services/order_management/stock/packer.rb b/engines/order_management/app/services/order_management/stock/packer.rb
index d66881a32b..49141d3fef 100644
--- a/engines/order_management/app/services/order_management/stock/packer.rb
+++ b/engines/order_management/app/services/order_management/stock/packer.rb
@@ -10,11 +10,7 @@ module OrderManagement
         @order = order
       end
 
-      def packages
-        [default_package]
-      end
-
-      def default_package
+      def package
         package = OrderManagement::Stock::Package.new(stock_location, order)
         order.line_items.each do |line_item|
           next unless stock_location.stock_item(line_item.variant)
diff --git a/engines/order_management/spec/services/order_management/stock/packer_spec.rb b/engines/order_management/spec/services/order_management/stock/packer_spec.rb
index f1e2b432bf..1100090af0 100644
--- a/engines/order_management/spec/services/order_management/stock/packer_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/packer_spec.rb
@@ -10,30 +10,21 @@ module OrderManagement
 
       subject { Packer.new(stock_location, order) }
 
-      context 'packages' do
-        it 'builds an array of packages' do
-          packages = subject.packages
-          expect(packages.size).to eq 1
-          expect(packages.first.contents.size).to eq 5
-        end
+      before { order.line_items.first.variant.update(weight: 1) }
+
+      it 'builds a package with all the items' do
+        package = subject.package
+
+        expect(package.contents.size).to eq 5
+        expect(package.weight).to be_positive
       end
 
-      context 'default_package' do
-        before { order.line_items.first.variant.update(weight: 1) }
+      it 'variants are added as backordered without enough on_hand' do
+        expect(stock_location).to receive(:fill_status).exactly(5).times.and_return([2, 3])
 
-        it 'contains all the items' do
-          package = subject.default_package
-          expect(package.contents.size).to eq 5
-          expect(package.weight).to be_positive
-        end
-
-        it 'variants are added as backordered without enough on_hand' do
-          expect(stock_location).to receive(:fill_status).exactly(5).times.and_return([2, 3])
-
-          package = subject.default_package
-          expect(package.on_hand.size).to eq 5
-          expect(package.backordered.size).to eq 5
-        end
+        package = subject.package
+        expect(package.on_hand.size).to eq 5
+        expect(package.backordered.size).to eq 5
       end
     end
   end

From 21ec6ccf0dd2b1f19786da488a7b7a6872b23b60 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 20:48:23 +0100
Subject: [PATCH 40/47] Remove unused sort packages from prioritizer

---
 .../app/services/order_management/stock/prioritizer.rb       | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/engines/order_management/app/services/order_management/stock/prioritizer.rb b/engines/order_management/app/services/order_management/stock/prioritizer.rb
index 16b992333f..9e8eea4289 100644
--- a/engines/order_management/app/services/order_management/stock/prioritizer.rb
+++ b/engines/order_management/app/services/order_management/stock/prioritizer.rb
@@ -12,7 +12,6 @@ module OrderManagement
       end
 
       def prioritized_packages
-        sort_packages
         adjust_packages
         prune_packages
         packages
@@ -38,10 +37,6 @@ module OrderManagement
         end
       end
 
-      def sort_packages
-        # order packages by preferred stock_locations
-      end
-
       def prune_packages
         packages.reject!(&:empty?)
       end

From 659de3d24d9a732d216ec7ca625aa787106add51 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 20:54:07 +0100
Subject: [PATCH 41/47] Replay spree commit
 a4622ee13a723f0dba2943967b445b9989f67fb2 to fix issue introduced in spree 2.1

---
 app/models/spree/shipment.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index 993fa0f62c..99d3a57fc0 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -107,11 +107,14 @@ module Spree
     def refresh_rates
       return shipping_rates if shipped?
 
+      # The call to Stock::Estimator below will replace the current shipping_method
+      original_shipping_method_id = shipping_method.try(:id)
+
       self.shipping_rates = OrderManagement::Stock::Estimator.new(order).shipping_rates(to_package)
 
-      if shipping_method
+      if original_shipping_method_id
         selected_rate = shipping_rates.detect { |rate|
-          rate.shipping_method_id == shipping_method.id
+          rate.shipping_method_id == original_shipping_method_id
         }
         self.selected_shipping_rate_id = selected_rate.id if selected_rate
       end

From 758bb17142cdb2a8291c534791f616b5924ce972 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Thu, 2 Jul 2020 21:23:32 +0100
Subject: [PATCH 42/47] Fix some easy rubocop issues and add some exceptions to
 to manual todo list

---
 .rubocop_manual_todo.yml                             | 12 ++++++++++++
 app/models/spree/shipment.rb                         |  7 +++----
 .../order_management/stock/estimator_spec.rb         |  2 +-
 lib/spree/core/environment.rb                        |  2 ++
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 94fdd0bd31..c5b16934f0 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -391,6 +391,7 @@ Metrics/AbcSize:
   - app/models/spree/order_decorator.rb
   - app/models/spree/payment_decorator.rb
   - app/models/spree/product_decorator.rb
+  - app/models/spree/shipment.rb
   - app/models/spree/taxon_decorator.rb
   - app/models/spree/tax_rate_decorator.rb
   - app/serializers/api/admin/enterprise_serializer.rb
@@ -400,6 +401,9 @@ Metrics/AbcSize:
   - app/services/create_order_cycle.rb
   - app/services/order_cycle_form.rb
   - app/services/order_syncer.rb
+  - engines/order_management/app/services/order_management/stock/estimator.rb
+  - engines/order_management/app/services/order_management/stock/package.rb
+  - engines/order_management/app/services/order_management/stock/packer.rb
   - engines/order_management/app/services/order_management/subscriptions/validator.rb
   - lib/active_merchant/billing/gateways/stripe_decorator.rb
   - lib/active_merchant/billing/gateways/stripe_payment_intents.rb
@@ -457,6 +461,7 @@ Metrics/BlockLength:
     "scenario"
   ]
   Exclude:
+  - app/models/spree/shipment.rb
   - lib/tasks/data.rake
   - spec/controllers/spree/admin/invoices_controller_spec.rb
   - spec/factories/enterprise_factory.rb
@@ -496,6 +501,7 @@ Metrics/CyclomaticComplexity:
   - app/models/spree/product_decorator.rb
   - app/models/variant_override_set.rb
   - app/services/cart_service.rb
+  - engines/order_management/app/services/order_management/stock/estimator.rb
   - lib/active_merchant/billing/gateways/stripe_payment_intents.rb
   - lib/discourse/single_sign_on.rb
   - lib/open_food_network/bulk_coop_report.rb
@@ -520,6 +526,7 @@ Metrics/PerceivedComplexity:
   - app/models/spree/ability_decorator.rb
   - app/models/spree/order_decorator.rb
   - app/models/spree/product_decorator.rb
+  - engines/order_management/app/services/order_management/stock/estimator.rb
   - lib/active_merchant/billing/gateways/stripe_payment_intents.rb
   - lib/discourse/single_sign_on.rb
   - lib/open_food_network/bulk_coop_report.rb
@@ -585,11 +592,14 @@ Metrics/MethodLength:
   - app/models/spree/payment_decorator.rb
   - app/models/spree/payment_method_decorator.rb
   - app/models/spree/product_decorator.rb
+  - app/models/spree/shipment.rb
   - app/serializers/api/admin/order_cycle_serializer.rb
   - app/serializers/api/cached_enterprise_serializer.rb
   - app/services/order_cycle_form.rb
   - app/services/permitted_attributes/checkout.rb
   - engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb
+  - engines/order_management/app/services/order_management/stock/estimator.rb
+  - engines/order_management/app/services/order_management/stock/package.rb
   - lib/active_merchant/billing/gateways/stripe_payment_intents.rb
   - lib/discourse/single_sign_on.rb
   - lib/open_food_network/bulk_coop_report.rb
@@ -652,6 +662,7 @@ Metrics/ClassLength:
   - app/models/product_import/entry_validator.rb
   - app/models/product_import/product_importer.rb
   - app/models/spree/ability_decorator.rb
+  - app/models/spree/shipment.rb
   - app/models/spree/user.rb
   - app/serializers/api/cached_enterprise_serializer.rb
   - app/serializers/api/enterprise_shopfront_serializer.rb
@@ -676,6 +687,7 @@ Metrics/ModuleLength:
   - app/helpers/injection_helper.rb
   - app/helpers/spree/admin/base_helper.rb
   - app/helpers/spree/admin/navigation_helper.rb
+  - engines/order_management/spec/services/order_management/stock/package_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index 99d3a57fc0..43b0b586c0 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -67,7 +67,6 @@ module Spree
     end
 
     def to_param
-      number if number
       generate_shipment_number unless number
       number.to_s.to_url.upcase
     end
@@ -136,7 +135,7 @@ module Spree
     alias_method :amount, :cost
 
     def display_cost
-      Spree::Money.new(cost, { currency: currency })
+      Spree::Money.new(cost, currency: currency)
     end
 
     alias_method :display_amount, :display_cost
@@ -146,7 +145,7 @@ module Spree
     end
 
     def display_item_cost
-      Spree::Money.new(item_cost, { currency: currency })
+      Spree::Money.new(item_cost, currency: currency)
     end
 
     def total_cost
@@ -154,7 +153,7 @@ module Spree
     end
 
     def display_total_cost
-      Spree::Money.new(total_cost, { currency: currency })
+      Spree::Money.new(total_cost, currency: currency)
     end
 
     def editable_by?(_user)
diff --git a/engines/order_management/spec/services/order_management/stock/estimator_spec.rb b/engines/order_management/spec/services/order_management/stock/estimator_spec.rb
index 892b7e1cf2..6d14903cbd 100644
--- a/engines/order_management/spec/services/order_management/stock/estimator_spec.rb
+++ b/engines/order_management/spec/services/order_management/stock/estimator_spec.rb
@@ -19,7 +19,7 @@ module OrderManagement
             to receive_message_chain(:calculator, :compute).and_return(4.00)
           allow_any_instance_of(Spree::ShippingMethod).
             to receive_message_chain(:calculator, :preferences).
-            and_return({ currency: order.currency })
+            and_return(currency: order.currency)
           allow_any_instance_of(Spree::ShippingMethod).
             to receive_message_chain(:calculator, :marked_for_destruction?)
 
diff --git a/lib/spree/core/environment.rb b/lib/spree/core/environment.rb
index 443b110c0a..4cd6fb5133 100644
--- a/lib/spree/core/environment.rb
+++ b/lib/spree/core/environment.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Spree
   module Core
     class Environment

From cd60ee2116e7d7fe18d2e642e6dc3ee035d416bc Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 3 Jul 2020 10:19:53 +0100
Subject: [PATCH 43/47] Use flat_map to make ship methods selection faster

---
 .../app/services/order_management/stock/package.rb              | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/engines/order_management/app/services/order_management/stock/package.rb b/engines/order_management/app/services/order_management/stock/package.rb
index 3009d0b19c..f3bfaf38f0 100644
--- a/engines/order_management/app/services/order_management/stock/package.rb
+++ b/engines/order_management/app/services/order_management/stock/package.rb
@@ -92,7 +92,7 @@ module OrderManagement
       #
       # @return [Array<Spree::ShippingMethod>]
       def shipping_methods
-        available_shipping_methods = shipping_categories.map(&:shipping_methods).flatten.uniq.to_a
+        available_shipping_methods = shipping_categories.flat_map(&:shipping_methods).uniq.to_a
 
         available_shipping_methods.keep_if do |shipping_method|
           ships_with?(order.distributor.shipping_methods.to_a, shipping_method)

From 07a44cfaf380cdab68c0c31edf008bed4d630cec Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 3 Jul 2020 10:18:56 +0100
Subject: [PATCH 44/47] Update selected shipping rate if there is an original
 shipping method to keep and it is different from the one selected through the
 Estimator process

Make sure the shipment is saved (callbacks!) whenever the ship method has changed in the refresh_rates process
---
 app/models/spree/shipment.rb       | 30 +++++++++++++++++++++++-------
 spec/models/spree/shipment_spec.rb |  3 ++-
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb
index 43b0b586c0..98ea4ac982 100644
--- a/app/models/spree/shipment.rb
+++ b/app/models/spree/shipment.rb
@@ -108,19 +108,35 @@ module Spree
 
       # The call to Stock::Estimator below will replace the current shipping_method
       original_shipping_method_id = shipping_method.try(:id)
-
       self.shipping_rates = OrderManagement::Stock::Estimator.new(order).shipping_rates(to_package)
 
-      if original_shipping_method_id
-        selected_rate = shipping_rates.detect { |rate|
-          rate.shipping_method_id == original_shipping_method_id
-        }
-        self.selected_shipping_rate_id = selected_rate.id if selected_rate
-      end
+      keep_original_shipping_method_selection(original_shipping_method_id)
 
       shipping_rates
     end
 
+    def keep_original_shipping_method_selection(original_shipping_method_id)
+      return if shipping_method&.id == original_shipping_method_id
+
+      rate_for_original_shipping_method = find_shipping_rate_for(original_shipping_method_id)
+      if rate_for_original_shipping_method.present?
+        self.selected_shipping_rate_id = rate_for_original_shipping_method.id
+      else
+        # If there's no original ship method to keep, or if it cannot be found on the ship rates
+        #   But there's a new ship method selected (first if clause in this method)
+        #   We need to save the shipment so that callbacks are triggered
+        save!
+      end
+    end
+
+    def find_shipping_rate_for(shipping_method_id)
+      return unless shipping_method_id
+
+      shipping_rates.detect { |rate|
+        rate.shipping_method_id == shipping_method_id
+      }
+    end
+
     def currency
       order ? order.currency : Spree::Config[:currency]
     end
diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index 457df53a6d..30f088133c 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -126,7 +126,8 @@ describe Spree::Shipment do
 
       it 'should request new rates, and maintain shipping_method selection' do
         OrderManagement::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
-        shipment.stub(shipping_method: shipping_method2)
+        # The first call is for the original shippping method, the second call is after the Estimator was executed
+        allow(shipment).to receive(:shipping_method).and_return(shipping_method2, shipping_method1)
 
         expect(shipment.refresh_rates).to eq shipping_rates
         expect(shipment.reload.selected_shipping_rate.shipping_method_id).to eq shipping_method2.id

From 7b89b52ab897c666ca37b8ecfe66defb51d1b996 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 3 Jul 2020 13:44:25 +0100
Subject: [PATCH 45/47] Transpec shipment_spec brough from spree_core

---
 spec/models/spree/shipment_spec.rb | 150 ++++++++++++++---------------
 1 file changed, 75 insertions(+), 75 deletions(-)

diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index 30f088133c..27fac8e151 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -8,7 +8,7 @@ describe Spree::Shipment do
   let(:shipping_method) { build(:shipping_method, name: "UPS") }
   let(:shipment) do
     shipment = Spree::Shipment.new order: order
-    shipment.stub(shipping_method: shipping_method)
+    allow(shipment).to receive_messages(shipping_method: shipping_method)
     shipment.state = 'pending'
     shipment
   end
@@ -21,13 +21,13 @@ describe Spree::Shipment do
     unit2 = create(:inventory_unit)
     allow(unit1).to receive(:backordered?) { false }
     allow(unit2).to receive(:backordered?) { true }
-    shipment.stub(inventory_units: [unit1, unit2])
+    allow(shipment).to receive_messages(inventory_units: [unit1, unit2])
     expect(shipment).to be_backordered
   end
 
   context "#cost" do
     it "should return the amount of any shipping charges that it originated" do
-      shipment.stub_chain :adjustment, amount: 10
+      allow(shipment).to receive_message_chain :adjustment, amount: 10
       expect(shipment.cost).to eq 10
     end
 
@@ -38,21 +38,21 @@ describe Spree::Shipment do
 
   context "display_cost" do
     it "retuns a Spree::Money" do
-      shipment.stub(:cost) { 21.22 }
+      allow(shipment).to receive(:cost) { 21.22 }
       expect(shipment.display_cost).to eq Spree::Money.new(21.22)
     end
   end
 
   context "display_item_cost" do
     it "retuns a Spree::Money" do
-      shipment.stub(:item_cost) { 21.22 }
+      allow(shipment).to receive(:item_cost) { 21.22 }
       expect(shipment.display_item_cost).to eq Spree::Money.new(21.22)
     end
   end
 
   context "display_total_cost" do
     it "retuns a Spree::Money" do
-      shipment.stub(:total_cost) { 21.22 }
+      allow(shipment).to receive(:total_cost) { 21.22 }
       expect(shipment.display_total_cost).to eq Spree::Money.new(21.22)
     end
   end
@@ -125,7 +125,7 @@ describe Spree::Shipment do
       let(:mock_estimator) { double('estimator', shipping_rates: shipping_rates) }
 
       it 'should request new rates, and maintain shipping_method selection' do
-        OrderManagement::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
+        expect(OrderManagement::Stock::Estimator).to receive(:new).with(shipment.order).and_return(mock_estimator)
         # The first call is for the original shippping method, the second call is after the Estimator was executed
         allow(shipment).to receive(:shipping_method).and_return(shipping_method2, shipping_method1)
 
@@ -134,22 +134,22 @@ describe Spree::Shipment do
       end
 
       it 'should handle no shipping_method selection' do
-        OrderManagement::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator)
-        shipment.stub(shipping_method: nil)
+        expect(OrderManagement::Stock::Estimator).to receive(:new).with(shipment.order).and_return(mock_estimator)
+        allow(shipment).to receive_messages(shipping_method: nil)
         expect(shipment.refresh_rates).to eq shipping_rates
         expect(shipment.reload.selected_shipping_rate).to_not be_nil
       end
 
       it 'should not refresh if shipment is shipped' do
-        OrderManagement::Stock::Estimator.should_not_receive(:new)
+        expect(OrderManagement::Stock::Estimator).not_to receive(:new)
         shipment.shipping_rates.delete_all
-        shipment.stub(shipped?: true)
+        allow(shipment).to receive_messages(shipped?: true)
         expect(shipment.refresh_rates).to eq []
       end
 
       context 'to_package' do
         it 'should use symbols for states when adding contents to package' do
-          shipment.stub_chain(:inventory_units,
+          allow(shipment).to receive_message_chain(:inventory_units,
                               includes: [build(:inventory_unit, variant: variant,
                                                                 state: 'on_hand'),
                                          build(:inventory_unit, variant: variant,
@@ -163,8 +163,8 @@ describe Spree::Shipment do
   end
 
   it '#total_cost' do
-    shipment.stub cost: 5.0
-    shipment.stub item_cost: 50.0
+    allow(shipment).to receive_messages cost: 5.0
+    allow(shipment).to receive_messages item_cost: 50.0
     expect(shipment.total_cost).to eql(55.0)
   end
 
@@ -172,7 +172,7 @@ describe Spree::Shipment do
     shared_examples_for "immutable once shipped" do
       it "should remain in shipped state once shipped" do
         shipment.state = 'shipped'
-        shipment.should_receive(:update_column).with(:state, 'shipped')
+        expect(shipment).to receive(:update_column).with(:state, 'shipped')
         shipment.update!(order)
       end
     end
@@ -181,8 +181,8 @@ describe Spree::Shipment do
       it "should have a state of pending if backordered" do
         unit = create(:inventory_unit)
         allow(unit).to receive(:backordered?) { true }
-        shipment.stub(inventory_units: [unit])
-        shipment.should_receive(:update_column).with(:state, 'pending')
+        allow(shipment).to receive_messages(inventory_units: [unit])
+        expect(shipment).to receive(:update_column).with(:state, 'pending')
         shipment.update!(order)
       end
     end
@@ -191,7 +191,7 @@ describe Spree::Shipment do
       it "should result in a 'pending' state" do
         allow(order).to receive(:canceled?) { true }
 
-        shipment.should_receive(:update_column).with(:state, 'canceled')
+        expect(shipment).to receive(:update_column).with(:state, 'canceled')
         shipment.update!(order)
       end
     end
@@ -200,7 +200,7 @@ describe Spree::Shipment do
       it "should result in a 'pending' state" do
         allow(order).to receive(:can_ship?) { false }
 
-        shipment.should_receive(:update_column).with(:state, 'pending')
+        expect(shipment).to receive(:update_column).with(:state, 'pending')
         shipment.update!(order)
       end
     end
@@ -212,7 +212,7 @@ describe Spree::Shipment do
         before { allow(order).to receive(:paid?) { true } }
 
         it "should result in a 'ready' state" do
-          shipment.should_receive(:update_column).with(:state, 'ready')
+          expect(shipment).to receive(:update_column).with(:state, 'ready')
           shipment.update!(order)
         end
 
@@ -225,7 +225,7 @@ describe Spree::Shipment do
 
           it "should result in a 'ready' state" do
             shipment.state = 'pending'
-            shipment.should_receive(:update_column).with(:state, 'ready')
+            expect(shipment).to receive(:update_column).with(:state, 'ready')
             shipment.update!(order)
           end
 
@@ -240,7 +240,7 @@ describe Spree::Shipment do
 
         it "should result in a 'pending' state" do
           shipment.state = 'ready'
-          shipment.should_receive(:update_column).with(:state, 'pending')
+          expect(shipment).to receive(:update_column).with(:state, 'pending')
           shipment.update!(order)
         end
 
@@ -253,9 +253,9 @@ describe Spree::Shipment do
     context "when shipment state changes to shipped" do
       it "should call after_ship" do
         shipment.state = 'pending'
-        shipment.should_receive :after_ship
-        shipment.stub determine_state: 'shipped'
-        shipment.should_receive(:update_column).with(:state, 'shipped')
+        expect(shipment).to receive :after_ship
+        allow(shipment).to receive_messages determine_state: 'shipped'
+        expect(shipment).to receive(:update_column).with(:state, 'shipped')
         shipment.update!(order)
       end
     end
@@ -263,8 +263,8 @@ describe Spree::Shipment do
 
   context "when order is completed" do
     before do
-      order.stub completed?: true
-      order.stub canceled?: false
+      allow(order).to receive_messages completed?: true
+      allow(order).to receive_messages canceled?: false
     end
 
     it "should validate with inventory" do
@@ -275,11 +275,11 @@ describe Spree::Shipment do
 
   context "#cancel" do
     it 'cancels the shipment' do
-      shipment.stub(:ensure_correct_adjustment)
-      shipment.order.stub(:update!)
+      allow(shipment).to receive(:ensure_correct_adjustment)
+      allow(shipment.order).to receive(:update!)
 
       shipment.state = 'pending'
-      shipment.should_receive(:after_cancel)
+      expect(shipment).to receive(:after_cancel)
       shipment.cancel!
       expect(shipment.state).to eq 'canceled'
     end
@@ -287,23 +287,23 @@ describe Spree::Shipment do
     it 'restocks the items' do
       unit = double(:inventory_unit, variant: variant)
       allow(unit).to receive(:quantity) { 1 }
-      shipment.stub_chain(:inventory_units,
+      allow(shipment).to receive_message_chain(:inventory_units,
                           :group_by,
                           map: [unit])
       shipment.stock_location = build(:stock_location)
-      shipment.stock_location.should_receive(:restock).with(variant, 1, shipment)
+      expect(shipment.stock_location).to receive(:restock).with(variant, 1, shipment)
       shipment.after_cancel
     end
   end
 
   context "#resume" do
     it 'will determine new state based on order' do
-      shipment.stub(:ensure_correct_adjustment)
-      shipment.order.stub(:update!)
+      allow(shipment).to receive(:ensure_correct_adjustment)
+      allow(shipment.order).to receive(:update!)
 
       shipment.state = 'canceled'
-      shipment.should_receive(:determine_state).and_return(:ready)
-      shipment.should_receive(:after_resume)
+      expect(shipment).to receive(:determine_state).and_return(:ready)
+      expect(shipment).to receive(:after_resume)
       shipment.resume!
       expect(shipment.state).to eq 'ready'
     end
@@ -311,21 +311,21 @@ describe Spree::Shipment do
     it 'unstocks the items' do
       unit = create(:inventory_unit, variant: variant)
       allow(unit).to receive(:quantity) { 1 }
-      shipment.stub_chain(:inventory_units,
+      allow(shipment).to receive_message_chain(:inventory_units,
                           :group_by,
                           map: [unit])
       shipment.stock_location = create(:stock_location)
-      shipment.stock_location.should_receive(:unstock).with(variant, 1, shipment)
+      expect(shipment.stock_location).to receive(:unstock).with(variant, 1, shipment)
       shipment.after_resume
     end
 
     it 'will determine new state based on order' do
-      shipment.stub(:ensure_correct_adjustment)
-      shipment.order.stub(:update!)
+      allow(shipment).to receive(:ensure_correct_adjustment)
+      allow(shipment.order).to receive(:update!)
 
       shipment.state = 'canceled'
-      shipment.should_receive(:determine_state).twice.and_return('ready')
-      shipment.should_receive(:after_resume)
+      expect(shipment).to receive(:determine_state).twice.and_return('ready')
+      expect(shipment).to receive(:after_resume)
       shipment.resume!
       # Shipment is pending because order is already paid
       expect(shipment.state).to eq 'pending'
@@ -334,15 +334,15 @@ describe Spree::Shipment do
 
   context "#ship" do
     before do
-      order.stub(:update!)
-      shipment.stub(update_order: true, state: 'ready')
-      shipment.stub(adjustment: charge)
-      shipping_method.stub(:create_adjustment)
-      shipment.stub(:ensure_correct_adjustment)
+      allow(order).to receive(:update!)
+      allow(shipment).to receive_messages(update_order: true, state: 'ready')
+      allow(shipment).to receive_messages(adjustment: charge)
+      allow(shipping_method).to receive(:create_adjustment)
+      allow(shipment).to receive(:ensure_correct_adjustment)
     end
 
     it "should update shipped_at timestamp" do
-      shipment.stub(:send_shipped_email)
+      allow(shipment).to receive(:send_shipped_email)
       shipment.ship!
       expect(shipment.shipped_at).to_not be_nil
       # Ensure value is persisted
@@ -353,17 +353,17 @@ describe Spree::Shipment do
     it "should send a shipment email" do
       mail_message = double 'Mail::Message'
       shipment_id = nil
-      Spree::ShipmentMailer.should_receive(:shipped_email) { |*args|
+      expect(Spree::ShipmentMailer).to receive(:shipped_email) { |*args|
         shipment_id = args[0]
         mail_message
       }
-      mail_message.should_receive :deliver
+      expect(mail_message).to receive :deliver
       shipment.ship!
       expect(shipment_id).to eq shipment.id
     end
 
     it "should finalize the shipment's adjustment" do
-      shipment.stub(:send_shipped_email)
+      allow(shipment).to receive(:send_shipped_email)
       shipment.ship!
       expect(shipment.adjustment.state).to eq 'finalized'
       expect(shipment.adjustment).to be_immutable
@@ -373,68 +373,68 @@ describe Spree::Shipment do
   context "#ready" do
     # Regression test for #2040
     it "cannot ready a shipment for an order if the order is unpaid" do
-      order.stub(paid?: false)
+      allow(order).to receive_messages(paid?: false)
       assert !shipment.can_ready?
     end
   end
 
   context "ensure_correct_adjustment" do
-    before { shipment.stub(:reload) }
+    before { allow(shipment).to receive(:reload) }
 
     it "should create adjustment when not present" do
-      shipment.stub(selected_shipping_rate_id: 1)
-      shipping_method.should_receive(:create_adjustment).with(shipping_method.adjustment_label,
+      allow(shipment).to receive_messages(selected_shipping_rate_id: 1)
+      expect(shipping_method).to receive(:create_adjustment).with(shipping_method.adjustment_label,
                                                               order, shipment, true, "open")
       shipment.__send__(:ensure_correct_adjustment)
     end
 
     # Regression test for #3138
     it "should use the shipping method's adjustment label" do
-      shipment.stub(selected_shipping_rate_id: 1)
-      shipping_method.stub(adjustment_label: "Foobar")
-      shipping_method.should_receive(:create_adjustment).with("Foobar", order,
+      allow(shipment).to receive_messages(selected_shipping_rate_id: 1)
+      allow(shipping_method).to receive_messages(adjustment_label: "Foobar")
+      expect(shipping_method).to receive(:create_adjustment).with("Foobar", order,
                                                               shipment, true, "open")
       shipment.__send__(:ensure_correct_adjustment)
     end
 
     it "should update originator when adjustment is present" do
-      shipment.stub(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
+      allow(shipment).to receive_messages(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
       adjustment = build(:adjustment)
-      shipment.stub(adjustment: adjustment)
+      allow(shipment).to receive_messages(adjustment: adjustment)
       allow(adjustment).to receive(:open?) { true }
-      shipment.adjustment.should_receive(:originator=).with(shipping_method)
-      shipment.adjustment.should_receive(:label=).with(shipping_method.adjustment_label)
-      shipment.adjustment.should_receive(:amount=).with(10.00)
+      expect(shipment.adjustment).to receive(:originator=).with(shipping_method)
+      expect(shipment.adjustment).to receive(:label=).with(shipping_method.adjustment_label)
+      expect(shipment.adjustment).to receive(:amount=).with(10.00)
       allow(shipment.adjustment).to receive(:save!)
-      shipment.adjustment.should_receive(:reload)
+      expect(shipment.adjustment).to receive(:reload)
       shipment.__send__(:ensure_correct_adjustment)
     end
 
     it 'should not update amount if adjustment is not open?' do
-      shipment.stub(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
+      allow(shipment).to receive_messages(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
       adjustment = build(:adjustment)
-      shipment.stub(adjustment: adjustment)
+      allow(shipment).to receive_messages(adjustment: adjustment)
       allow(adjustment).to receive(:open?) { false }
-      shipment.adjustment.should_receive(:originator=).with(shipping_method)
-      shipment.adjustment.should_receive(:label=).with(shipping_method.adjustment_label)
-      shipment.adjustment.should_not_receive(:amount=).with(10.00)
+      expect(shipment.adjustment).to receive(:originator=).with(shipping_method)
+      expect(shipment.adjustment).to receive(:label=).with(shipping_method.adjustment_label)
+      expect(shipment.adjustment).not_to receive(:amount=).with(10.00)
       allow(shipment.adjustment).to receive(:save!)
-      shipment.adjustment.should_receive(:reload)
+      expect(shipment.adjustment).to receive(:reload)
       shipment.__send__(:ensure_correct_adjustment)
     end
   end
 
   context "update_order" do
     it "should update order" do
-      order.should_receive(:update!)
+      expect(order).to receive(:update!)
       shipment.__send__(:update_order)
     end
   end
 
   context "after_save" do
     it "should run correct callbacks" do
-      shipment.should_receive(:ensure_correct_adjustment)
-      shipment.should_receive(:update_order)
+      expect(shipment).to receive(:ensure_correct_adjustment)
+      expect(shipment).to receive(:update_order)
       shipment.run_callbacks(:save)
     end
   end
@@ -447,7 +447,7 @@ describe Spree::Shipment do
 
   context "#tracking_url" do
     it "uses shipping method to determine url" do
-      shipping_method.should_receive(:build_tracking_url).with('1Z12345').and_return(:some_url)
+      expect(shipping_method).to receive(:build_tracking_url).with('1Z12345').and_return(:some_url)
       shipment.tracking = '1Z12345'
 
       expect(shipment.tracking_url).to eq :some_url
@@ -461,7 +461,7 @@ describe Spree::Shipment do
       { variant_id: variant.id, state: 'on_hand', order_id: order.id }
     end
 
-    before { shipment.stub inventory_units: inventory_units }
+    before { allow(shipment).to receive_messages inventory_units: inventory_units }
 
     it "associates variant and order" do
       expect(inventory_units).to receive(:create).with(params)

From b883a0eb75321f7343bdaee702369c126e358574 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 3 Jul 2020 13:47:46 +0100
Subject: [PATCH 46/47] Fix easy rubocop issues in shipment_spec

---
 spec/models/spree/shipment_spec.rb | 38 +++++++++++++++++-------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index 27fac8e151..e24cebb801 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -125,8 +125,10 @@ describe Spree::Shipment do
       let(:mock_estimator) { double('estimator', shipping_rates: shipping_rates) }
 
       it 'should request new rates, and maintain shipping_method selection' do
-        expect(OrderManagement::Stock::Estimator).to receive(:new).with(shipment.order).and_return(mock_estimator)
-        # The first call is for the original shippping method, the second call is after the Estimator was executed
+        expect(OrderManagement::Stock::Estimator).
+          to receive(:new).with(shipment.order).and_return(mock_estimator)
+        # The first call is for the original shippping method,
+        #   the second call is for the shippping method after the Estimator was executed
         allow(shipment).to receive(:shipping_method).and_return(shipping_method2, shipping_method1)
 
         expect(shipment.refresh_rates).to eq shipping_rates
@@ -134,7 +136,8 @@ describe Spree::Shipment do
       end
 
       it 'should handle no shipping_method selection' do
-        expect(OrderManagement::Stock::Estimator).to receive(:new).with(shipment.order).and_return(mock_estimator)
+        expect(OrderManagement::Stock::Estimator).
+          to receive(:new).with(shipment.order).and_return(mock_estimator)
         allow(shipment).to receive_messages(shipping_method: nil)
         expect(shipment.refresh_rates).to eq shipping_rates
         expect(shipment.reload.selected_shipping_rate).to_not be_nil
@@ -149,11 +152,12 @@ describe Spree::Shipment do
 
       context 'to_package' do
         it 'should use symbols for states when adding contents to package' do
-          allow(shipment).to receive_message_chain(:inventory_units,
-                              includes: [build(:inventory_unit, variant: variant,
-                                                                state: 'on_hand'),
-                                         build(:inventory_unit, variant: variant,
-                                                                state: 'backordered')] )
+          allow(shipment).
+            to receive_message_chain(:inventory_units,
+                                     includes: [build(:inventory_unit, variant: variant,
+                                                                       state: 'on_hand'),
+                                                build(:inventory_unit, variant: variant,
+                                                                       state: 'backordered')] )
           package = shipment.to_package
           expect(package.on_hand.count).to eq 1
           expect(package.backordered.count).to eq 1
@@ -288,8 +292,8 @@ describe Spree::Shipment do
       unit = double(:inventory_unit, variant: variant)
       allow(unit).to receive(:quantity) { 1 }
       allow(shipment).to receive_message_chain(:inventory_units,
-                          :group_by,
-                          map: [unit])
+                                               :group_by,
+                                               map: [unit])
       shipment.stock_location = build(:stock_location)
       expect(shipment.stock_location).to receive(:restock).with(variant, 1, shipment)
       shipment.after_cancel
@@ -312,8 +316,8 @@ describe Spree::Shipment do
       unit = create(:inventory_unit, variant: variant)
       allow(unit).to receive(:quantity) { 1 }
       allow(shipment).to receive_message_chain(:inventory_units,
-                          :group_by,
-                          map: [unit])
+                                               :group_by,
+                                               map: [unit])
       shipment.stock_location = create(:stock_location)
       expect(shipment.stock_location).to receive(:unstock).with(variant, 1, shipment)
       shipment.after_resume
@@ -384,7 +388,7 @@ describe Spree::Shipment do
     it "should create adjustment when not present" do
       allow(shipment).to receive_messages(selected_shipping_rate_id: 1)
       expect(shipping_method).to receive(:create_adjustment).with(shipping_method.adjustment_label,
-                                                              order, shipment, true, "open")
+                                                                  order, shipment, true, "open")
       shipment.__send__(:ensure_correct_adjustment)
     end
 
@@ -393,12 +397,13 @@ describe Spree::Shipment do
       allow(shipment).to receive_messages(selected_shipping_rate_id: 1)
       allow(shipping_method).to receive_messages(adjustment_label: "Foobar")
       expect(shipping_method).to receive(:create_adjustment).with("Foobar", order,
-                                                              shipment, true, "open")
+                                                                  shipment, true, "open")
       shipment.__send__(:ensure_correct_adjustment)
     end
 
     it "should update originator when adjustment is present" do
-      allow(shipment).to receive_messages(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
+      allow(shipment).
+        to receive_messages(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
       adjustment = build(:adjustment)
       allow(shipment).to receive_messages(adjustment: adjustment)
       allow(adjustment).to receive(:open?) { true }
@@ -411,7 +416,8 @@ describe Spree::Shipment do
     end
 
     it 'should not update amount if adjustment is not open?' do
-      allow(shipment).to receive_messages(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
+      allow(shipment).
+        to receive_messages(selected_shipping_rate: Spree::ShippingRate.new(cost: 10.00))
       adjustment = build(:adjustment)
       allow(shipment).to receive_messages(adjustment: adjustment)
       allow(adjustment).to receive(:open?) { false }

From 804450bcc57447d7c58c14040232973d0e6b3267 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Fri, 3 Jul 2020 15:35:13 +0100
Subject: [PATCH 47/47] Fix buggy spec

The different shipping method was in the page but only as an option in the dropdown, not as the final selected shipping method! That was the cause of bug #5694. We now check for the label Shipping which preceeds the final shipping method selection in the order page
---
 spec/features/admin/order_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb
index 0017d6b72a..009092686b 100644
--- a/spec/features/admin/order_spec.rb
+++ b/spec/features/admin/order_spec.rb
@@ -285,7 +285,7 @@ feature '
                          from: 'selected_shipping_rate_id'
           find('.save-method').click
 
-          expect(page).to have_content different_shipping_method_for_distributor1.name
+          expect(page).to have_content "Shipping: #{different_shipping_method_for_distributor1.name}"
         end
       end