From f743b5f02f6d63dfd829ed91ac2867fd9b08fc17 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 13 Sep 2018 14:45:10 +0200
Subject: [PATCH 01/28] Extract Settings from Product Import processor

This encapsulates the data structure used by the entry processor to
check various settings. It still requires a lot of work to move more
logic to this new class.
---
 app/models/product_import/entry_processor.rb |  6 ++-
 app/models/product_import/settings.rb        | 13 +++++
 spec/models/product_import/settings_spec.rb  | 51 ++++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 app/models/product_import/settings.rb
 create mode 100644 spec/models/product_import/settings_spec.rb

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 315721ffbb..028b973454 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -198,12 +198,14 @@ module ProductImport
     end
 
     def assign_defaults(object, entry)
+      settings = Settings.new(@import_settings)
+
       # Assigns a default value for a specified field e.g. category='Vegetables', setting this value
       # either for all entries (overwrite_all), or only for those entries where the field was blank
       # in the spreadsheet (overwrite_empty), depending on selected import settings
-      return unless @import_settings.key?(:settings) && @import_settings[:settings][entry.supplier_id.to_s] && @import_settings[:settings][entry.supplier_id.to_s]['defaults']
+      return unless settings.defaults(entry)
 
-      @import_settings[:settings][entry.supplier_id.to_s]['defaults'].each do |attribute, setting|
+      settings.defaults(entry).each do |attribute, setting|
         next unless setting['active']
 
         case setting['mode']
diff --git a/app/models/product_import/settings.rb b/app/models/product_import/settings.rb
new file mode 100644
index 0000000000..a97ee544ff
--- /dev/null
+++ b/app/models/product_import/settings.rb
@@ -0,0 +1,13 @@
+module ProductImport
+  class Settings
+    def initialize(import_settings)
+      @import_settings = import_settings
+    end
+
+    def defaults(entry)
+      @import_settings.key?(:settings) &&
+        @import_settings[:settings][entry.supplier_id.to_s] &&
+        @import_settings[:settings][entry.supplier_id.to_s]['defaults']
+    end
+  end
+end
diff --git a/spec/models/product_import/settings_spec.rb b/spec/models/product_import/settings_spec.rb
new file mode 100644
index 0000000000..768bc315ad
--- /dev/null
+++ b/spec/models/product_import/settings_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe ProductImport::Settings do
+  let(:settings) { described_class.new(import_settings) }
+
+  describe '#defaults' do
+    let(:entry) { instance_double(ProductImport::SpreadsheetEntry) }
+    let(:import_settings) { {} }
+
+    context 'when there are no settings' do
+      it 'returns false' do
+        expect(settings.defaults(entry)).to be_falsey
+      end
+    end
+
+    context 'when there are settings' do
+      let(:entry) do
+        instance_double(ProductImport::SpreadsheetEntry, supplier_id: 1)
+      end
+      let(:import_settings) { { settings: {} } }
+
+      context 'and there is no data for the specified entry' do
+        it 'returns a falsey' do
+          expect(settings.defaults(entry)).to be_falsey
+        end
+      end
+
+      context 'and there is data for the specified entry' do
+        context 'and it has no defaults' do
+          let(:import_settings) do
+            { settings: { '1' => { 'foo' => 'bar' } } }
+          end
+
+          it 'returns a falsey' do
+            expect(settings.defaults(entry)).to be_falsey
+          end
+        end
+
+        context 'and it has defaults' do
+          let(:import_settings) do
+            { settings: { '1' => { 'defaults' => 'default value' } } }
+          end
+
+          it 'returns a truthy' do
+            expect(settings.defaults(entry)).to eq('default value')
+          end
+        end
+      end
+    end
+  end
+end

From 3150741849e375ef3a78dfb611e1df47f4698c21 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 21 Sep 2018 18:47:43 +0200
Subject: [PATCH 02/28] Extract ResetAbsent class from EntryProcessor

---
 app/models/product_import/entry_processor.rb  | 55 +++---------
 app/models/product_import/reset_absent.rb     | 53 ++++++++++++
 .../product_import/entry_processor_spec.rb    | 40 +++++++++
 .../product_import/reset_absent_spec.rb       | 86 +++++++++++++++++++
 4 files changed, 191 insertions(+), 43 deletions(-)
 create mode 100644 app/models/product_import/reset_absent.rb
 create mode 100644 spec/models/product_import/entry_processor_spec.rb
 create mode 100644 spec/models/product_import/reset_absent_spec.rb

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 028b973454..323b4859b8 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -4,7 +4,7 @@
 
 module ProductImport
   class EntryProcessor
-    attr_reader :inventory_created, :inventory_updated, :products_created, :variants_created, :variants_updated, :products_reset_count, :supplier_products, :total_supplier_products
+    attr_reader :inventory_created, :inventory_updated, :products_created, :variants_created, :variants_updated, :products_reset_count, :supplier_products, :total_supplier_products, :import_settings
 
     def initialize(importer, validator, import_settings, spreadsheet_data, editable_enterprises, import_time, updated_ids)
       @importer = importer
@@ -60,46 +60,23 @@ module ProductImport
     end
 
     def reset_absent_items
-      # For selected enterprises; set stock to zero for all products/inventory
-      # that were not listed in the newly uploaded spreadsheet
-      return unless data_for_stock_reset?
-      suppliers_to_reset_products = []
-      suppliers_to_reset_inventories = []
-
-      settings = @import_settings[:settings]
-
-      @import_settings[:enterprises_to_reset].each do |enterprise_id|
-        suppliers_to_reset_products.push Integer(enterprise_id) if settings['reset_all_absent'] && permission_by_id?(enterprise_id) && !importing_into_inventory?
-        suppliers_to_reset_inventories.push Integer(enterprise_id) if settings['reset_all_absent'] && permission_by_id?(enterprise_id) && importing_into_inventory?
-      end
-
-      unless suppliers_to_reset_inventories.empty?
-        @products_reset_count += VariantOverride.
-          where('variant_overrides.hub_id IN (?)
-          AND variant_overrides.id NOT IN (?)', suppliers_to_reset_inventories, @import_settings[:updated_ids]).
-          update_all(count_on_hand: 0)
-      end
-
-      return if suppliers_to_reset_products.empty?
-
-      @products_reset_count += Spree::Variant.joins(:product).
-        where('spree_products.supplier_id IN (?)
-        AND spree_variants.id NOT IN (?)
-        AND spree_variants.is_master = false
-        AND spree_variants.deleted_at IS NULL', suppliers_to_reset_products, @import_settings[:updated_ids]).
-        update_all(count_on_hand: 0)
+      ResetAbsent.new(self).call
     end
 
     def total_saved_count
       @products_created + @variants_created + @variants_updated + @inventory_created + @inventory_updated
     end
 
-    private
-
-    def data_for_stock_reset?
-      @import_settings[:settings] && @import_settings[:updated_ids] && @import_settings[:enterprises_to_reset]
+    def permission_by_id?(supplier_id)
+      @editable_enterprises.value?(Integer(supplier_id))
     end
 
+    def importing_into_inventory?
+      import_settings[:settings] && import_settings[:settings]['import_into'] == 'inventories'
+    end
+
+    private
+
     def save_to_inventory(entry)
       save_new_inventory_item entry if entry.validates_as? 'new_inventory_item'
       save_existing_inventory_item entry if entry.validates_as? 'existing_inventory_item'
@@ -126,7 +103,7 @@ module ProductImport
     end
 
     def import_into_inventory?(entry)
-      entry.supplier_id && @import_settings[:settings]['import_into'] == 'inventories'
+      entry.supplier_id && import_settings[:settings]['import_into'] == 'inventories'
     end
 
     def save_new_inventory_item(entry)
@@ -198,7 +175,7 @@ module ProductImport
     end
 
     def assign_defaults(object, entry)
-      settings = Settings.new(@import_settings)
+      settings = Settings.new(import_settings)
 
       # Assigns a default value for a specified field e.g. category='Vegetables', setting this value
       # either for all entries (overwrite_all), or only for those entries where the field was blank
@@ -241,13 +218,5 @@ module ProductImport
       variant.import_date = @import_time
       variant.save
     end
-
-    def permission_by_id?(supplier_id)
-      @editable_enterprises.value?(Integer(supplier_id))
-    end
-
-    def importing_into_inventory?
-      @import_settings[:settings] && @import_settings[:settings]['import_into'] == 'inventories'
-    end
   end
 end
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
new file mode 100644
index 0000000000..aad5ed4de9
--- /dev/null
+++ b/app/models/product_import/reset_absent.rb
@@ -0,0 +1,53 @@
+require 'delegate'
+
+module ProductImport
+  class ResetAbsent < SimpleDelegator
+    def call
+      # For selected enterprises; set stock to zero for all products/inventory
+      # that were not listed in the newly uploaded spreadsheet
+      return unless data_for_stock_reset?
+      suppliers_to_reset_products = []
+      suppliers_to_reset_inventories = []
+
+      settings = import_settings[:settings]
+
+      import_settings[:enterprises_to_reset].each do |enterprise_id|
+        if settings['reset_all_absent'] &&
+           permission_by_id?(enterprise_id) &&
+           !importing_into_inventory?
+          suppliers_to_reset_products.push(Integer(enterprise_id))
+        end
+
+        if settings['reset_all_absent'] &&
+           permission_by_id?(enterprise_id) &&
+           importing_into_inventory?
+          suppliers_to_reset_inventories.push(Integer(enterprise_id))
+        end
+      end
+
+      unless suppliers_to_reset_inventories.empty?
+        @products_reset_count += VariantOverride.
+          where('variant_overrides.hub_id IN (?)
+          AND variant_overrides.id NOT IN (?)', suppliers_to_reset_inventories, import_settings[:updated_ids]).
+        update_all(count_on_hand: 0)
+      end
+
+      return if suppliers_to_reset_products.empty?
+
+      @products_reset_count += Spree::Variant.joins(:product).
+        where('spree_products.supplier_id IN (?)
+        AND spree_variants.id NOT IN (?)
+        AND spree_variants.is_master = false
+        AND spree_variants.deleted_at IS NULL', suppliers_to_reset_products, import_settings[:updated_ids]).
+      update_all(count_on_hand: 0)
+    end
+
+    private
+
+    def data_for_stock_reset?
+      import_settings[:settings] &&
+        import_settings[:updated_ids] &&
+        import_settings[:enterprises_to_reset]
+    end
+  end
+end
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
new file mode 100644
index 0000000000..866bf742b8
--- /dev/null
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+
+describe ProductImport::EntryProcessor do
+  let(:importer) { double(:importer) }
+  let(:validator) { double(:validator) }
+  let(:import_settings) { double(:import_settings) }
+  let(:spreadsheet_data) { double(:spreadsheet_data) }
+  let(:editable_enterprises) { double(:editable_enterprises) }
+  let(:import_time) { double(:import_time) }
+  let(:updated_ids) { double(:updated_ids) }
+
+  let(:entry_processor) do
+    described_class.new(
+      importer,
+      validator,
+      import_settings,
+      spreadsheet_data,
+      editable_enterprises,
+      import_time,
+      updated_ids
+    )
+  end
+
+  describe '#reset_absent_items' do
+    let(:reset_absent) { double(ProductImport::ResetAbsent, call: true) }
+
+    before do
+      allow(ProductImport::ResetAbsent)
+        .to receive(:new)
+        .and_return(reset_absent)
+    end
+
+    it 'delegates to ResetAbsent' do
+      entry_processor.reset_absent_items
+
+      expect(ProductImport::ResetAbsent)
+        .to have_received(:new).with(entry_processor)
+    end
+  end
+end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
new file mode 100644
index 0000000000..2be1df43c7
--- /dev/null
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -0,0 +1,86 @@
+require 'spec_helper'
+
+describe ProductImport::ResetAbsent do
+  let(:importer) { double(:importer) }
+  let(:validator) { double(:validator) }
+  let(:spreadsheet_data) { double(:spreadsheet_data) }
+  let(:editable_enterprises) { double(:editable_enterprises) }
+  let(:import_time) { double(:import_time) }
+  let(:updated_ids) { double(:updated_ids) }
+
+  let(:entry_processor) do
+    ProductImport::EntryProcessor.new(
+      importer,
+      validator,
+      import_settings,
+      spreadsheet_data,
+      editable_enterprises,
+      import_time,
+      updated_ids
+    )
+  end
+
+  let(:reset_absent) { described_class.new(entry_processor) }
+
+  describe '#call' do
+    context 'when there are no settings' do
+      let(:import_settings) { { updated_ids: [], enterprises_to_reset: [] } }
+
+      it 'returns nil' do
+        expect(reset_absent.call).to be_nil
+      end
+    end
+
+    context 'when there are no updated_ids' do
+      let(:import_settings) { { settings: [], enterprises_to_reset: [] } }
+
+      it 'returns nil' do
+        expect(reset_absent.call).to be_nil
+      end
+    end
+
+    context 'when there are no enterprises_to_reset' do
+      let(:import_settings) { { settings: [], updated_ids: [] } }
+
+      it 'returns nil' do
+        expect(reset_absent.call).to be_nil
+      end
+    end
+
+    context 'when there are settings, updated_ids and enterprises_to_reset' do
+      let(:import_settings) do
+        {
+          settings: { 'reset_all_absent' => true },
+          updated_ids: [1],
+          enterprises_to_reset: [2]
+        }
+      end
+
+      before do
+        allow(entry_processor).to receive(:permission_by_id?).with(2) { true }
+      end
+
+      context 'and not importing into inventory' do
+        before do
+          allow(entry_processor)
+            .to receive(:importing_into_inventory?) { false }
+        end
+
+        it 'returns true' do
+          expect(reset_absent.call).to eq(true)
+        end
+      end
+
+      context 'and importing into inventory' do
+        before do
+          allow(entry_processor)
+            .to receive(:importing_into_inventory?) { true }
+        end
+
+        it 'returns true' do
+          expect(reset_absent.call).to eq(true)
+        end
+      end
+    end
+  end
+end

From 663db47433a9261aa97fcfeae61a7b7ed78b05cd Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 24 Sep 2018 13:49:27 +0200
Subject: [PATCH 03/28] Move products_reset_count to ResetAbsent

---
 app/models/product_import/entry_processor.rb  | 11 ++--
 app/models/product_import/reset_absent.rb     | 36 +++++++++----
 .../product_import/entry_processor_spec.rb    | 19 ++++++-
 .../product_import/reset_absent_spec.rb       | 54 ++++++++++++++++---
 4 files changed, 99 insertions(+), 21 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 323b4859b8..a6ace4d659 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -4,7 +4,9 @@
 
 module ProductImport
   class EntryProcessor
-    attr_reader :inventory_created, :inventory_updated, :products_created, :variants_created, :variants_updated, :products_reset_count, :supplier_products, :total_supplier_products, :import_settings
+    delegate :products_reset_count, to: :reset_absent
+
+    attr_reader :inventory_created, :inventory_updated, :products_created, :variants_created, :variants_updated, :supplier_products, :total_supplier_products, :import_settings
 
     def initialize(importer, validator, import_settings, spreadsheet_data, editable_enterprises, import_time, updated_ids)
       @importer = importer
@@ -20,7 +22,6 @@ module ProductImport
       @products_created = 0
       @variants_created = 0
       @variants_updated = 0
-      @products_reset_count = 0
       @supplier_products = {}
       @total_supplier_products = 0
     end
@@ -60,7 +61,11 @@ module ProductImport
     end
 
     def reset_absent_items
-      ResetAbsent.new(self).call
+      reset_absent.call
+    end
+
+    def reset_absent
+      @reset_absent ||= ResetAbsent.new(self)
     end
 
     def total_saved_count
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index aad5ed4de9..439945bb06 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -2,6 +2,13 @@ require 'delegate'
 
 module ProductImport
   class ResetAbsent < SimpleDelegator
+    attr_reader :products_reset_count
+
+    def initialize(decorated)
+      super
+      @products_reset_count = 0
+    end
+
     def call
       # For selected enterprises; set stock to zero for all products/inventory
       # that were not listed in the newly uploaded spreadsheet
@@ -26,20 +33,29 @@ module ProductImport
       end
 
       unless suppliers_to_reset_inventories.empty?
-        @products_reset_count += VariantOverride.
-          where('variant_overrides.hub_id IN (?)
-          AND variant_overrides.id NOT IN (?)', suppliers_to_reset_inventories, import_settings[:updated_ids]).
-        update_all(count_on_hand: 0)
+        relation = VariantOverride
+          .where(
+            'variant_overrides.hub_id IN (?) ' \
+            'AND variant_overrides.id NOT IN (?)',
+            suppliers_to_reset_inventories,
+            import_settings[:updated_ids]
+          )
+        @products_reset_count += relation.update_all(count_on_hand: 0)
       end
 
       return if suppliers_to_reset_products.empty?
 
-      @products_reset_count += Spree::Variant.joins(:product).
-        where('spree_products.supplier_id IN (?)
-        AND spree_variants.id NOT IN (?)
-        AND spree_variants.is_master = false
-        AND spree_variants.deleted_at IS NULL', suppliers_to_reset_products, import_settings[:updated_ids]).
-      update_all(count_on_hand: 0)
+      relation = Spree::Variant
+        .joins(:product)
+        .where(
+          'spree_products.supplier_id IN (?) ' \
+          'AND spree_variants.id NOT IN (?) ' \
+          'AND spree_variants.is_master = false ' \
+          'AND spree_variants.deleted_at IS NULL',
+          suppliers_to_reset_products,
+          import_settings[:updated_ids]
+        )
+      @products_reset_count += relation.update_all(count_on_hand: 0)
     end
 
     private
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index 866bf742b8..88205a85d3 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -22,7 +22,7 @@ describe ProductImport::EntryProcessor do
   end
 
   describe '#reset_absent_items' do
-    let(:reset_absent) { double(ProductImport::ResetAbsent, call: true) }
+    let(:reset_absent) { instance_double(ProductImport::ResetAbsent, call: true) }
 
     before do
       allow(ProductImport::ResetAbsent)
@@ -37,4 +37,21 @@ describe ProductImport::EntryProcessor do
         .to have_received(:new).with(entry_processor)
     end
   end
+
+  describe '#products_reset_count' do
+    let(:reset_absent) { instance_double(ProductImport::ResetAbsent) }
+
+    before do
+      allow(ProductImport::ResetAbsent)
+        .to receive(:new)
+        .and_return(reset_absent)
+
+      allow(reset_absent).to receive(:products_reset_count)
+    end
+
+    it 'delegates to ResetAbsent' do
+      entry_processor.products_reset_count
+      expect(reset_absent).to have_received(:products_reset_count)
+    end
+  end
 end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 2be1df43c7..f2addc9dc3 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -51,36 +51,76 @@ describe ProductImport::ResetAbsent do
       let(:import_settings) do
         {
           settings: { 'reset_all_absent' => true },
-          updated_ids: [1],
-          enterprises_to_reset: [2]
+          updated_ids: [0],
+          enterprises_to_reset: [enterprise.id]
         }
       end
 
       before do
-        allow(entry_processor).to receive(:permission_by_id?).with(2) { true }
+        allow(entry_processor)
+          .to receive(:permission_by_id?).with(enterprise.id) { true }
       end
 
       context 'and not importing into inventory' do
+        let(:variant) { create(:variant) }
+        let(:enterprise) { variant.product.supplier }
+
         before do
           allow(entry_processor)
             .to receive(:importing_into_inventory?) { false }
         end
 
-        it 'returns true' do
-          expect(reset_absent.call).to eq(true)
+        it 'returns the number of products reset' do
+          expect(reset_absent.call).to eq(2)
         end
       end
 
       context 'and importing into inventory' do
+        let(:variant) { create(:variant) }
+        let(:enterprise) { variant.product.supplier }
+        let(:variant_override) do
+          create(:variant_override, variant: variant, hub: enterprise)
+        end
+
+        before do
+          variant_override
+
+          allow(entry_processor)
+            .to receive(:permission_by_id?).with(enterprise.id) { true }
+        end
+
         before do
           allow(entry_processor)
             .to receive(:importing_into_inventory?) { true }
         end
 
-        it 'returns true' do
-          expect(reset_absent.call).to eq(true)
+        it 'returns nil' do
+          expect(reset_absent.call).to be_nil
         end
       end
     end
   end
+
+  describe '#products_reset_count' do
+    let(:variant) { create(:variant) }
+    let(:enterprise_id) { variant.product.supplier_id }
+
+    before do
+      allow(entry_processor)
+        .to receive(:permission_by_id?).with(enterprise_id) { true }
+    end
+
+    let(:import_settings) do
+      {
+        settings: { 'reset_all_absent' => true },
+        updated_ids: [0],
+        enterprises_to_reset: [enterprise_id]
+      }
+    end
+
+    it 'returns the number of reset products or variants' do
+      reset_absent.call
+      expect(reset_absent.products_reset_count).to eq(2)
+    end
+  end
 end

From ed073e9750ddf60ffae728b97a947889fca71090 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 24 Sep 2018 14:15:57 +0200
Subject: [PATCH 04/28] Rely on Settings and don't access internal struct.

---
 app/models/product_import/reset_absent.rb   | 28 +++++++----
 app/models/product_import/settings.rb       | 16 +++++-
 spec/models/product_import/settings_spec.rb | 54 +++++++++++++++++++++
 3 files changed, 86 insertions(+), 12 deletions(-)

diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 439945bb06..54d3340c93 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -7,25 +7,29 @@ module ProductImport
     def initialize(decorated)
       super
       @products_reset_count = 0
+
+      settings = ProductImport::Settings.new(import_settings)
+      @settings = settings.settings
+      @updated_ids = settings.updated_ids
+      @enterprises_to_reset = settings.enterprises_to_reset
     end
 
     def call
       # For selected enterprises; set stock to zero for all products/inventory
       # that were not listed in the newly uploaded spreadsheet
       return unless data_for_stock_reset?
+
       suppliers_to_reset_products = []
       suppliers_to_reset_inventories = []
 
-      settings = import_settings[:settings]
-
-      import_settings[:enterprises_to_reset].each do |enterprise_id|
-        if settings['reset_all_absent'] &&
+      enterprises_to_reset.each do |enterprise_id|
+        if reset_all_absent? &&
            permission_by_id?(enterprise_id) &&
            !importing_into_inventory?
           suppliers_to_reset_products.push(Integer(enterprise_id))
         end
 
-        if settings['reset_all_absent'] &&
+        if reset_all_absent? &&
            permission_by_id?(enterprise_id) &&
            importing_into_inventory?
           suppliers_to_reset_inventories.push(Integer(enterprise_id))
@@ -38,7 +42,7 @@ module ProductImport
             'variant_overrides.hub_id IN (?) ' \
             'AND variant_overrides.id NOT IN (?)',
             suppliers_to_reset_inventories,
-            import_settings[:updated_ids]
+            updated_ids
           )
         @products_reset_count += relation.update_all(count_on_hand: 0)
       end
@@ -53,17 +57,21 @@ module ProductImport
           'AND spree_variants.is_master = false ' \
           'AND spree_variants.deleted_at IS NULL',
           suppliers_to_reset_products,
-          import_settings[:updated_ids]
+          updated_ids
         )
       @products_reset_count += relation.update_all(count_on_hand: 0)
     end
 
     private
 
+    attr_reader :settings, :updated_ids, :enterprises_to_reset
+
     def data_for_stock_reset?
-      import_settings[:settings] &&
-        import_settings[:updated_ids] &&
-        import_settings[:enterprises_to_reset]
+      settings && updated_ids && enterprises_to_reset
+    end
+
+    def reset_all_absent?
+      settings['reset_all_absent']
     end
   end
 end
diff --git a/app/models/product_import/settings.rb b/app/models/product_import/settings.rb
index a97ee544ff..c0ef038ac1 100644
--- a/app/models/product_import/settings.rb
+++ b/app/models/product_import/settings.rb
@@ -6,8 +6,20 @@ module ProductImport
 
     def defaults(entry)
       @import_settings.key?(:settings) &&
-        @import_settings[:settings][entry.supplier_id.to_s] &&
-        @import_settings[:settings][entry.supplier_id.to_s]['defaults']
+        settings[entry.supplier_id.to_s] &&
+        settings[entry.supplier_id.to_s]['defaults']
+    end
+
+    def settings
+      @import_settings[:settings]
+    end
+
+    def updated_ids
+      @import_settings[:updated_ids]
+    end
+
+    def enterprises_to_reset
+      @import_settings[:enterprises_to_reset]
     end
   end
 end
diff --git a/spec/models/product_import/settings_spec.rb b/spec/models/product_import/settings_spec.rb
index 768bc315ad..7673efe65b 100644
--- a/spec/models/product_import/settings_spec.rb
+++ b/spec/models/product_import/settings_spec.rb
@@ -48,4 +48,58 @@ describe ProductImport::Settings do
       end
     end
   end
+
+  describe '#settings' do
+    context 'when settings are specified' do
+      let(:import_settings) { { settings: { foo: 'bar' } } }
+
+      it 'returns them' do
+        expect(settings.settings).to eq(foo: 'bar')
+      end
+    end
+
+    context 'when settings are not specified' do
+      let(:import_settings) { {} }
+
+      it 'returns nil' do
+        expect(settings.settings).to be_nil
+      end
+    end
+  end
+
+  describe '#updated_ids' do
+    context 'when updated_ids are specified' do
+      let(:import_settings) { { updated_ids: [2] } }
+
+      it 'returns them' do
+        expect(settings.updated_ids).to eq([2])
+      end
+    end
+
+    context 'when updated_ids are not specified' do
+      let(:import_settings) { {} }
+
+      it 'returns nil' do
+        expect(settings.updated_ids).to be_nil
+      end
+    end
+  end
+
+  describe '#enterprises_to_reset' do
+    context 'when enterprises_to_reset are specified' do
+      let(:import_settings) { { enterprises_to_reset: [2] } }
+
+      it 'returns them' do
+        expect(settings.enterprises_to_reset).to eq([2])
+      end
+    end
+
+    context 'when enterprises_to_reset are not specified' do
+      let(:import_settings) { {} }
+
+      it 'returns nil' do
+        expect(settings.enterprises_to_reset).to be_nil
+      end
+    end
+  end
 end

From b5766a2dd96230b81036289eba39a17df681b7f0 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 24 Sep 2018 14:39:22 +0200
Subject: [PATCH 05/28] Extract common conditional clauses

This also turns local vars into ivars so that the behaviour can be
thoroughly tested. These ivars are meant to be removed once this class
is refactored further. Now there's no other way to ensure its state.
---
 app/models/product_import/reset_absent.rb     | 26 +++++-----
 .../product_import/reset_absent_spec.rb       | 49 +++++++++++++++++++
 2 files changed, 61 insertions(+), 14 deletions(-)

diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 54d3340c93..74ba1d41d9 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -19,35 +19,33 @@ module ProductImport
       # that were not listed in the newly uploaded spreadsheet
       return unless data_for_stock_reset?
 
-      suppliers_to_reset_products = []
-      suppliers_to_reset_inventories = []
+      @suppliers_to_reset_products = []
+      @suppliers_to_reset_inventories = []
 
       enterprises_to_reset.each do |enterprise_id|
-        if reset_all_absent? &&
-           permission_by_id?(enterprise_id) &&
-           !importing_into_inventory?
-          suppliers_to_reset_products.push(Integer(enterprise_id))
+        next unless reset_all_absent? && permission_by_id?(enterprise_id)
+
+        if !importing_into_inventory?
+          @suppliers_to_reset_products.push(Integer(enterprise_id))
         end
 
-        if reset_all_absent? &&
-           permission_by_id?(enterprise_id) &&
-           importing_into_inventory?
-          suppliers_to_reset_inventories.push(Integer(enterprise_id))
+        if importing_into_inventory?
+          @suppliers_to_reset_inventories.push(Integer(enterprise_id))
         end
       end
 
-      unless suppliers_to_reset_inventories.empty?
+      unless @suppliers_to_reset_inventories.empty?
         relation = VariantOverride
           .where(
             'variant_overrides.hub_id IN (?) ' \
             'AND variant_overrides.id NOT IN (?)',
-            suppliers_to_reset_inventories,
+            @suppliers_to_reset_inventories,
             updated_ids
           )
         @products_reset_count += relation.update_all(count_on_hand: 0)
       end
 
-      return if suppliers_to_reset_products.empty?
+      return if @suppliers_to_reset_products.empty?
 
       relation = Spree::Variant
         .joins(:product)
@@ -56,7 +54,7 @@ module ProductImport
           'AND spree_variants.id NOT IN (?) ' \
           'AND spree_variants.is_master = false ' \
           'AND spree_variants.deleted_at IS NULL',
-          suppliers_to_reset_products,
+          @suppliers_to_reset_products,
           updated_ids
         )
       @products_reset_count += relation.update_all(count_on_hand: 0)
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index f2addc9dc3..c6fa531af9 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -99,6 +99,55 @@ describe ProductImport::ResetAbsent do
         end
       end
     end
+
+    context 'when reset_all_absent is not set' do
+      let(:import_settings) do
+        {
+          settings: { 'reset_all_absent' => false },
+          updated_ids: [0],
+          enterprises_to_reset: [1]
+        }
+      end
+
+      it 'does not reset anything' do
+        reset_absent.call
+
+        suppliers_to_reset_products = reset_absent
+          .instance_variable_get('@suppliers_to_reset_products')
+        suppliers_to_reset_inventories = reset_absent
+          .instance_variable_get('@suppliers_to_reset_inventories')
+
+        expect(suppliers_to_reset_products).to eq([])
+        expect(suppliers_to_reset_inventories).to eq([])
+      end
+    end
+
+    context 'the enterprise has no permission' do
+      let(:import_settings) do
+        {
+          settings: { 'reset_all_absent' => true },
+          updated_ids: [0],
+          enterprises_to_reset: [1]
+        }
+      end
+
+      before do
+        allow(entry_processor)
+          .to receive(:permission_by_id?).with(1) { false }
+      end
+
+      it 'does not reset anything' do
+        reset_absent.call
+
+        suppliers_to_reset_products = reset_absent
+          .instance_variable_get('@suppliers_to_reset_products')
+        suppliers_to_reset_inventories = reset_absent
+          .instance_variable_get('@suppliers_to_reset_inventories')
+
+        expect(suppliers_to_reset_products).to eq([])
+        expect(suppliers_to_reset_inventories).to eq([])
+      end
+    end
   end
 
   describe '#products_reset_count' do

From 5ac3598550e27f19e3a74013990f6cad3ca8d317 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 24 Sep 2018 17:01:48 +0200
Subject: [PATCH 06/28] Inject Settings object in ResetAbsent

---
 app/models/product_import/entry_processor.rb  |  5 +-
 app/models/product_import/reset_absent.rb     |  5 +-
 .../product_import/entry_processor_spec.rb    | 12 ++--
 .../product_import/reset_absent_spec.rb       | 61 ++++++++++++++-----
 4 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index a6ace4d659..d33d417608 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -65,7 +65,10 @@ module ProductImport
     end
 
     def reset_absent
-      @reset_absent ||= ResetAbsent.new(self)
+      @reset_absent ||= ResetAbsent.new(
+        self,
+        Settings.new(import_settings)
+      )
     end
 
     def total_saved_count
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 74ba1d41d9..d0b11ffe02 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -4,11 +4,10 @@ module ProductImport
   class ResetAbsent < SimpleDelegator
     attr_reader :products_reset_count
 
-    def initialize(decorated)
-      super
+    def initialize(decorated, settings)
+      super(decorated)
       @products_reset_count = 0
 
-      settings = ProductImport::Settings.new(import_settings)
       @settings = settings.settings
       @updated_ids = settings.updated_ids
       @enterprises_to_reset = settings.enterprises_to_reset
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index 88205a85d3..343a13f6d4 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -22,19 +22,21 @@ describe ProductImport::EntryProcessor do
   end
 
   describe '#reset_absent_items' do
-    let(:reset_absent) { instance_double(ProductImport::ResetAbsent, call: true) }
+    let(:reset_absent) do
+      instance_double(ProductImport::ResetAbsent, call: true)
+    end
+    let(:settings) { instance_double(ProductImport::Settings) }
 
     before do
-      allow(ProductImport::ResetAbsent)
-        .to receive(:new)
-        .and_return(reset_absent)
+      allow(ProductImport::ResetAbsent).to receive(:new) { reset_absent }
+      allow(ProductImport::Settings).to receive(:new) { settings }
     end
 
     it 'delegates to ResetAbsent' do
       entry_processor.reset_absent_items
 
       expect(ProductImport::ResetAbsent)
-        .to have_received(:new).with(entry_processor)
+        .to have_received(:new).with(entry_processor, settings)
     end
   end
 
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index c6fa531af9..046accf848 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -7,6 +7,7 @@ describe ProductImport::ResetAbsent do
   let(:editable_enterprises) { double(:editable_enterprises) }
   let(:import_time) { double(:import_time) }
   let(:updated_ids) { double(:updated_ids) }
+  let(:import_settings) { double(:import_settings) }
 
   let(:entry_processor) do
     ProductImport::EntryProcessor.new(
@@ -20,11 +21,18 @@ describe ProductImport::ResetAbsent do
     )
   end
 
-  let(:reset_absent) { described_class.new(entry_processor) }
+  let(:reset_absent) { described_class.new(entry_processor, settings) }
 
   describe '#call' do
     context 'when there are no settings' do
-      let(:import_settings) { { updated_ids: [], enterprises_to_reset: [] } }
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
+          settings: nil,
+          updated_ids: [],
+          enterprises_to_reset: []
+        )
+      end
 
       it 'returns nil' do
         expect(reset_absent.call).to be_nil
@@ -32,7 +40,14 @@ describe ProductImport::ResetAbsent do
     end
 
     context 'when there are no updated_ids' do
-      let(:import_settings) { { settings: [], enterprises_to_reset: [] } }
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
+          settings: [],
+          updated_ids: nil,
+          enterprises_to_reset: []
+        )
+      end
 
       it 'returns nil' do
         expect(reset_absent.call).to be_nil
@@ -40,7 +55,14 @@ describe ProductImport::ResetAbsent do
     end
 
     context 'when there are no enterprises_to_reset' do
-      let(:import_settings) { { settings: [], updated_ids: [] } }
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
+          settings: [],
+          updated_ids: [],
+          enterprises_to_reset: nil
+        )
+      end
 
       it 'returns nil' do
         expect(reset_absent.call).to be_nil
@@ -48,12 +70,13 @@ describe ProductImport::ResetAbsent do
     end
 
     context 'when there are settings, updated_ids and enterprises_to_reset' do
-      let(:import_settings) do
-        {
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
           settings: { 'reset_all_absent' => true },
           updated_ids: [0],
           enterprises_to_reset: [enterprise.id]
-        }
+        )
       end
 
       before do
@@ -101,12 +124,13 @@ describe ProductImport::ResetAbsent do
     end
 
     context 'when reset_all_absent is not set' do
-      let(:import_settings) do
-        {
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
           settings: { 'reset_all_absent' => false },
           updated_ids: [0],
           enterprises_to_reset: [1]
-        }
+        )
       end
 
       it 'does not reset anything' do
@@ -123,12 +147,13 @@ describe ProductImport::ResetAbsent do
     end
 
     context 'the enterprise has no permission' do
-      let(:import_settings) do
-        {
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
           settings: { 'reset_all_absent' => true },
           updated_ids: [0],
           enterprises_to_reset: [1]
-        }
+        )
       end
 
       before do
@@ -157,14 +182,18 @@ describe ProductImport::ResetAbsent do
     before do
       allow(entry_processor)
         .to receive(:permission_by_id?).with(enterprise_id) { true }
+
+      allow(entry_processor)
+        .to receive(:importing_into_inventory?) { false }
     end
 
-    let(:import_settings) do
-      {
+    let(:settings) do
+      instance_double(
+        ProductImport::Settings,
         settings: { 'reset_all_absent' => true },
         updated_ids: [0],
         enterprises_to_reset: [enterprise_id]
-      }
+      )
     end
 
     it 'returns the number of reset products or variants' do

From e04162415a54801adf7929e5589dd863e7a08f1b Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 24 Sep 2018 17:04:15 +0200
Subject: [PATCH 07/28] Move code comment to be the method's doc

---
 app/models/product_import/reset_absent.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index d0b11ffe02..e4df27cedc 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -13,9 +13,9 @@ module ProductImport
       @enterprises_to_reset = settings.enterprises_to_reset
     end
 
+    # For selected enterprises; set stock to zero for all products/inventory
+    # that were not listed in the newly uploaded spreadsheet
     def call
-      # For selected enterprises; set stock to zero for all products/inventory
-      # that were not listed in the newly uploaded spreadsheet
       return unless data_for_stock_reset?
 
       @suppliers_to_reset_products = []

From c955e151b7c9e44fff0fa69d373201118c91376b Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 24 Sep 2018 17:53:51 +0200
Subject: [PATCH 08/28] Pass enterprise ids as strs as current code does

---
 app/models/product_import/reset_absent.rb       |  4 ++--
 spec/models/product_import/reset_absent_spec.rb | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index e4df27cedc..01e9b7f1d9 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -25,11 +25,11 @@ module ProductImport
         next unless reset_all_absent? && permission_by_id?(enterprise_id)
 
         if !importing_into_inventory?
-          @suppliers_to_reset_products.push(Integer(enterprise_id))
+          @suppliers_to_reset_products << enterprise_id.to_i
         end
 
         if importing_into_inventory?
-          @suppliers_to_reset_inventories.push(Integer(enterprise_id))
+          @suppliers_to_reset_inventories << enterprise_id.to_i
         end
       end
 
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 046accf848..b34e1ae4fa 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -75,13 +75,13 @@ describe ProductImport::ResetAbsent do
           ProductImport::Settings,
           settings: { 'reset_all_absent' => true },
           updated_ids: [0],
-          enterprises_to_reset: [enterprise.id]
+          enterprises_to_reset: [enterprise.id.to_s]
         )
       end
 
       before do
         allow(entry_processor)
-          .to receive(:permission_by_id?).with(enterprise.id) { true }
+          .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
       end
 
       context 'and not importing into inventory' do
@@ -109,7 +109,7 @@ describe ProductImport::ResetAbsent do
           variant_override
 
           allow(entry_processor)
-            .to receive(:permission_by_id?).with(enterprise.id) { true }
+            .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
         end
 
         before do
@@ -129,7 +129,7 @@ describe ProductImport::ResetAbsent do
           ProductImport::Settings,
           settings: { 'reset_all_absent' => false },
           updated_ids: [0],
-          enterprises_to_reset: [1]
+          enterprises_to_reset: ['1']
         )
       end
 
@@ -152,13 +152,13 @@ describe ProductImport::ResetAbsent do
           ProductImport::Settings,
           settings: { 'reset_all_absent' => true },
           updated_ids: [0],
-          enterprises_to_reset: [1]
+          enterprises_to_reset: ['1']
         )
       end
 
       before do
         allow(entry_processor)
-          .to receive(:permission_by_id?).with(1) { false }
+          .to receive(:permission_by_id?).with('1') { false }
       end
 
       it 'does not reset anything' do
@@ -181,7 +181,7 @@ describe ProductImport::ResetAbsent do
 
     before do
       allow(entry_processor)
-        .to receive(:permission_by_id?).with(enterprise_id) { true }
+        .to receive(:permission_by_id?).with(enterprise_id.to_s) { true }
 
       allow(entry_processor)
         .to receive(:importing_into_inventory?) { false }
@@ -192,7 +192,7 @@ describe ProductImport::ResetAbsent do
         ProductImport::Settings,
         settings: { 'reset_all_absent' => true },
         updated_ids: [0],
-        enterprises_to_reset: [enterprise_id]
+        enterprises_to_reset: [enterprise_id.to_s]
       )
     end
 

From b940f06238972b0dfaea17598cd531d887ac3684 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 24 Sep 2018 18:06:20 +0200
Subject: [PATCH 09/28] Return early if reset_all_absent is not set

No need to go through half of the algorithm if the setting is not
enabled. It does not change per enterprise.

This also assumes "Previously we were updating both (products and inventory)
at the same time during one import, but now it's one or the other" in
Matt's words.
---
 app/models/product_import/reset_absent.rb       | 16 +++++++---------
 spec/models/product_import/reset_absent_spec.rb |  7 ++++++-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 01e9b7f1d9..250369ba64 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -11,25 +11,23 @@ module ProductImport
       @settings = settings.settings
       @updated_ids = settings.updated_ids
       @enterprises_to_reset = settings.enterprises_to_reset
+
+      @suppliers_to_reset_products = []
+      @suppliers_to_reset_inventories = []
     end
 
     # For selected enterprises; set stock to zero for all products/inventory
     # that were not listed in the newly uploaded spreadsheet
     def call
-      return unless data_for_stock_reset?
-
-      @suppliers_to_reset_products = []
-      @suppliers_to_reset_inventories = []
+      return unless data_for_stock_reset? && reset_all_absent?
 
       enterprises_to_reset.each do |enterprise_id|
-        next unless reset_all_absent? && permission_by_id?(enterprise_id)
-
-        if !importing_into_inventory?
-          @suppliers_to_reset_products << enterprise_id.to_i
-        end
+        next unless permission_by_id?(enterprise_id)
 
         if importing_into_inventory?
           @suppliers_to_reset_inventories << enterprise_id.to_i
+        else
+          @suppliers_to_reset_products << enterprise_id.to_i
         end
       end
 
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index b34e1ae4fa..47dc713d43 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -133,6 +133,11 @@ describe ProductImport::ResetAbsent do
         )
       end
 
+      before do
+        allow(entry_processor)
+          .to receive(:permission_by_id?).with('1') { true }
+      end
+
       it 'does not reset anything' do
         reset_absent.call
 
@@ -146,7 +151,7 @@ describe ProductImport::ResetAbsent do
       end
     end
 
-    context 'the enterprise has no permission' do
+    context 'when the enterprise has no permission' do
       let(:settings) do
         instance_double(
           ProductImport::Settings,

From fd84bea463ceda48fdd7cae62cc9d040e3546248 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 12:20:52 +0200
Subject: [PATCH 10/28] Test that variants or overrides are actually reset

---
 spec/models/product_import/reset_absent_spec.rb | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 47dc713d43..b6ac542ba2 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -96,6 +96,14 @@ describe ProductImport::ResetAbsent do
         it 'returns the number of products reset' do
           expect(reset_absent.call).to eq(2)
         end
+
+        it 'resets the products of the specified suppliers' do
+          suppliers_to_reset_products = reset_absent
+            .instance_variable_get('@suppliers_to_reset_products')
+
+          reset_absent.call
+          expect(suppliers_to_reset_products).to eq([enterprise.id])
+        end
       end
 
       context 'and importing into inventory' do
@@ -120,6 +128,14 @@ describe ProductImport::ResetAbsent do
         it 'returns nil' do
           expect(reset_absent.call).to be_nil
         end
+
+        it 'resets the inventories of the specified suppliers' do
+          suppliers_to_reset_inventories = reset_absent
+            .instance_variable_get('@suppliers_to_reset_inventories')
+
+          reset_absent.call
+          expect(suppliers_to_reset_inventories).to eq([enterprise.id])
+        end
       end
     end
 

From a409353d372f73c4040a6078c4e95c51760f7c3a Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 12:41:38 +0200
Subject: [PATCH 11/28] Move import_settings query methods to Settings

---
 app/models/product_import/entry_processor.rb  |  11 +-
 app/models/product_import/reset_absent.rb     |  22 +--
 app/models/product_import/settings.rb         |  12 ++
 .../product_import/entry_processor_spec.rb    |  15 +++
 .../product_import/reset_absent_spec.rb       |  42 ++----
 spec/models/product_import/settings_spec.rb   | 125 ++++++++++++++++++
 6 files changed, 175 insertions(+), 52 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index d33d417608..9afb1e3087 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -5,6 +5,7 @@
 module ProductImport
   class EntryProcessor
     delegate :products_reset_count, to: :reset_absent
+    delegate :importing_into_inventory?, to: :settings
 
     attr_reader :inventory_created, :inventory_updated, :products_created, :variants_created, :variants_updated, :supplier_products, :total_supplier_products, :import_settings
 
@@ -24,6 +25,8 @@ module ProductImport
       @variants_updated = 0
       @supplier_products = {}
       @total_supplier_products = 0
+
+      @settings = Settings.new(import_settings)
     end
 
     def save_all(entries)
@@ -79,12 +82,10 @@ module ProductImport
       @editable_enterprises.value?(Integer(supplier_id))
     end
 
-    def importing_into_inventory?
-      import_settings[:settings] && import_settings[:settings]['import_into'] == 'inventories'
-    end
-
     private
 
+    attr_reader :settings
+
     def save_to_inventory(entry)
       save_new_inventory_item entry if entry.validates_as? 'new_inventory_item'
       save_existing_inventory_item entry if entry.validates_as? 'existing_inventory_item'
@@ -183,8 +184,6 @@ module ProductImport
     end
 
     def assign_defaults(object, entry)
-      settings = Settings.new(import_settings)
-
       # Assigns a default value for a specified field e.g. category='Vegetables', setting this value
       # either for all entries (overwrite_all), or only for those entries where the field was blank
       # in the spreadsheet (overwrite_empty), depending on selected import settings
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 250369ba64..022e456ead 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -8,9 +8,7 @@ module ProductImport
       super(decorated)
       @products_reset_count = 0
 
-      @settings = settings.settings
-      @updated_ids = settings.updated_ids
-      @enterprises_to_reset = settings.enterprises_to_reset
+      @settings = settings
 
       @suppliers_to_reset_products = []
       @suppliers_to_reset_inventories = []
@@ -19,9 +17,9 @@ module ProductImport
     # For selected enterprises; set stock to zero for all products/inventory
     # that were not listed in the newly uploaded spreadsheet
     def call
-      return unless data_for_stock_reset? && reset_all_absent?
+      return unless settings.data_for_stock_reset? && settings.reset_all_absent?
 
-      enterprises_to_reset.each do |enterprise_id|
+      settings.enterprises_to_reset.each do |enterprise_id|
         next unless permission_by_id?(enterprise_id)
 
         if importing_into_inventory?
@@ -37,7 +35,7 @@ module ProductImport
             'variant_overrides.hub_id IN (?) ' \
             'AND variant_overrides.id NOT IN (?)',
             @suppliers_to_reset_inventories,
-            updated_ids
+            settings.updated_ids
           )
         @products_reset_count += relation.update_all(count_on_hand: 0)
       end
@@ -52,21 +50,13 @@ module ProductImport
           'AND spree_variants.is_master = false ' \
           'AND spree_variants.deleted_at IS NULL',
           @suppliers_to_reset_products,
-          updated_ids
+          settings.updated_ids
         )
       @products_reset_count += relation.update_all(count_on_hand: 0)
     end
 
     private
 
-    attr_reader :settings, :updated_ids, :enterprises_to_reset
-
-    def data_for_stock_reset?
-      settings && updated_ids && enterprises_to_reset
-    end
-
-    def reset_all_absent?
-      settings['reset_all_absent']
-    end
+    attr_reader :settings
   end
 end
diff --git a/app/models/product_import/settings.rb b/app/models/product_import/settings.rb
index c0ef038ac1..4a0df965cb 100644
--- a/app/models/product_import/settings.rb
+++ b/app/models/product_import/settings.rb
@@ -21,5 +21,17 @@ module ProductImport
     def enterprises_to_reset
       @import_settings[:enterprises_to_reset]
     end
+
+    def importing_into_inventory?
+      settings && settings['import_into'] == 'inventories'
+    end
+
+    def reset_all_absent?
+      settings['reset_all_absent']
+    end
+
+    def data_for_stock_reset?
+      !!(settings && updated_ids && enterprises_to_reset)
+    end
   end
 end
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index 343a13f6d4..152012cf58 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -56,4 +56,19 @@ describe ProductImport::EntryProcessor do
       expect(reset_absent).to have_received(:products_reset_count)
     end
   end
+
+  describe '#importing_into_inventory?' do
+    let(:settings) do
+      instance_double(ProductImport::Settings, importing_into_inventory?: true)
+    end
+
+    before do
+      allow(ProductImport::Settings).to receive(:new) { settings }
+    end
+
+    it 'delegates to Settings' do
+      entry_processor.importing_into_inventory?
+      expect(settings).to have_received(:importing_into_inventory?)
+    end
+  end
 end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index b6ac542ba2..4551c217d3 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -24,29 +24,9 @@ describe ProductImport::ResetAbsent do
   let(:reset_absent) { described_class.new(entry_processor, settings) }
 
   describe '#call' do
-    context 'when there are no settings' do
+    context 'when there is no data' do
       let(:settings) do
-        instance_double(
-          ProductImport::Settings,
-          settings: nil,
-          updated_ids: [],
-          enterprises_to_reset: []
-        )
-      end
-
-      it 'returns nil' do
-        expect(reset_absent.call).to be_nil
-      end
-    end
-
-    context 'when there are no updated_ids' do
-      let(:settings) do
-        instance_double(
-          ProductImport::Settings,
-          settings: [],
-          updated_ids: nil,
-          enterprises_to_reset: []
-        )
+        instance_double(ProductImport::Settings, data_for_stock_reset?: false)
       end
 
       it 'returns nil' do
@@ -58,9 +38,7 @@ describe ProductImport::ResetAbsent do
       let(:settings) do
         instance_double(
           ProductImport::Settings,
-          settings: [],
-          updated_ids: [],
-          enterprises_to_reset: nil
+          enterprises_to_reset: []
         )
       end
 
@@ -69,11 +47,12 @@ describe ProductImport::ResetAbsent do
       end
     end
 
-    context 'when there are settings, updated_ids and enterprises_to_reset' do
+    context 'when there are updated_ids and enterprises_to_reset' do
       let(:settings) do
         instance_double(
           ProductImport::Settings,
-          settings: { 'reset_all_absent' => true },
+          reset_all_absent?: true,
+          data_for_stock_reset?: true,
           updated_ids: [0],
           enterprises_to_reset: [enterprise.id.to_s]
         )
@@ -143,7 +122,8 @@ describe ProductImport::ResetAbsent do
       let(:settings) do
         instance_double(
           ProductImport::Settings,
-          settings: { 'reset_all_absent' => false },
+          reset_all_absent?: false,
+          data_for_stock_reset?: true,
           updated_ids: [0],
           enterprises_to_reset: ['1']
         )
@@ -171,7 +151,8 @@ describe ProductImport::ResetAbsent do
       let(:settings) do
         instance_double(
           ProductImport::Settings,
-          settings: { 'reset_all_absent' => true },
+          reset_all_absent?: true,
+          data_for_stock_reset?: true,
           updated_ids: [0],
           enterprises_to_reset: ['1']
         )
@@ -211,7 +192,8 @@ describe ProductImport::ResetAbsent do
     let(:settings) do
       instance_double(
         ProductImport::Settings,
-        settings: { 'reset_all_absent' => true },
+        reset_all_absent?: true,
+        data_for_stock_reset?: true,
         updated_ids: [0],
         enterprises_to_reset: [enterprise_id.to_s]
       )
diff --git a/spec/models/product_import/settings_spec.rb b/spec/models/product_import/settings_spec.rb
index 7673efe65b..a7f823b2d8 100644
--- a/spec/models/product_import/settings_spec.rb
+++ b/spec/models/product_import/settings_spec.rb
@@ -102,4 +102,129 @@ describe ProductImport::Settings do
       end
     end
   end
+
+  describe '#importing_into_inventory?' do
+    context 'when :settings is specified' do
+      context 'and import_into is not specified' do
+        let(:import_settings) { { settings: {} } }
+
+        it 'returns false' do
+          expect(settings.importing_into_inventory?).to eq(false)
+        end
+      end
+
+      context 'and import_into is equal to inventories' do
+        let(:import_settings) do
+          { settings: { 'import_into' => 'inventories' } }
+        end
+
+        it 'returns true' do
+          expect(settings.importing_into_inventory?).to eq(true)
+        end
+      end
+
+      context 'and import_into is not equal to inventories' do
+        let(:import_settings) do
+          { settings: { 'import_into' => 'other' } }
+        end
+
+        it 'returns false' do
+          expect(settings.importing_into_inventory?).to eq(false)
+        end
+      end
+    end
+
+    context 'when :settings is not specified' do
+      let(:import_settings) { {} }
+
+      it 'returns falsy' do
+        expect(settings.importing_into_inventory?).to be_falsy
+      end
+    end
+  end
+
+  describe '#reset_all_absent?' do
+    context 'when :settings is not specified' do
+      let(:import_settings) { {} }
+
+      it 'raises' do
+        expect { settings.reset_all_absent? }.to raise_error(NoMethodError)
+      end
+    end
+
+    context 'when reset_all_absent is not set' do
+      let(:import_settings) do
+        { settings: {} }
+      end
+
+      it 'returns nil' do
+        expect(settings.reset_all_absent?).to be_nil
+      end
+    end
+
+    context 'when reset_all_absent is set' do
+      let(:import_settings) do
+        { settings: { 'reset_all_absent' => true } }
+      end
+
+      it 'returns true' do
+        expect(settings.reset_all_absent?).to eq(true)
+      end
+    end
+  end
+
+  describe '#data_for_stock_reset?' do
+    context 'when there are no settings' do
+      let(:import_settings) do
+        {
+          updated_ids: [],
+          enterprises_to_reset: []
+        }
+      end
+
+      it 'returns false' do
+        expect(settings.data_for_stock_reset?).to eq(false)
+      end
+    end
+
+    context 'when there are no updated_ids' do
+      let(:import_settings) do
+        {
+          settings: [],
+          enterprises_to_reset: []
+        }
+      end
+
+      it 'returns false' do
+        expect(settings.data_for_stock_reset?).to eq(false)
+      end
+    end
+
+    context 'when there are no enterprises_to_reset' do
+      let(:import_settings) do
+        {
+          settings: [],
+          updated_ids: []
+        }
+      end
+
+      it 'returns false' do
+        expect(settings.data_for_stock_reset?).to eq(false)
+      end
+    end
+
+    context 'when there are settings, updated_ids and enterprises_to_reset' do
+      let(:import_settings) do
+        {
+          settings: { 'something' => true },
+          updated_ids: [0],
+          enterprises_to_reset: [0]
+        }
+      end
+
+      it 'returns true' do
+        expect(settings.data_for_stock_reset?).to eq(true)
+      end
+    end
+  end
 end

From 23471346b63cbb8e068a9c0de56e5d0356b75780 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 13:45:53 +0200
Subject: [PATCH 12/28] Do not call ResetAbsent when preconditions not met

---
 app/models/product_import/entry_processor.rb  |  2 +
 app/models/product_import/reset_absent.rb     |  2 -
 .../product_import/entry_processor_spec.rb    | 49 +++++++++++++++++--
 .../product_import/reset_absent_spec.rb       | 39 ---------------
 4 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 9afb1e3087..5387da4be1 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -64,6 +64,8 @@ module ProductImport
     end
 
     def reset_absent_items
+      return unless settings.data_for_stock_reset? && settings.reset_all_absent?
+
       reset_absent.call
     end
 
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 022e456ead..d1c0197d9c 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -17,8 +17,6 @@ module ProductImport
     # For selected enterprises; set stock to zero for all products/inventory
     # that were not listed in the newly uploaded spreadsheet
     def call
-      return unless settings.data_for_stock_reset? && settings.reset_all_absent?
-
       settings.enterprises_to_reset.each do |enterprise_id|
         next unless permission_by_id?(enterprise_id)
 
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index 152012cf58..96c1b79b87 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -25,18 +25,57 @@ describe ProductImport::EntryProcessor do
     let(:reset_absent) do
       instance_double(ProductImport::ResetAbsent, call: true)
     end
-    let(:settings) { instance_double(ProductImport::Settings) }
 
     before do
       allow(ProductImport::ResetAbsent).to receive(:new) { reset_absent }
       allow(ProductImport::Settings).to receive(:new) { settings }
     end
 
-    it 'delegates to ResetAbsent' do
-      entry_processor.reset_absent_items
+    context 'when there is no data' do
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
+          data_for_stock_reset?: false,
+          reset_all_absent?: true
+        )
+      end
 
-      expect(ProductImport::ResetAbsent)
-        .to have_received(:new).with(entry_processor, settings)
+      it 'does not call ResetAbsent' do
+        entry_processor.reset_absent_items
+        expect(reset_absent).not_to have_received(:call)
+      end
+    end
+
+    context 'when reset_all_absent is not set' do
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
+          data_for_stock_reset?: true,
+          reset_all_absent?: false
+        )
+      end
+
+      it 'does not call ResetAbsent' do
+        entry_processor.reset_absent_items
+        expect(reset_absent).not_to have_received(:call)
+      end
+    end
+
+    context 'when there is data and reset_all_absent is set' do
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
+          data_for_stock_reset?: true,
+          reset_all_absent?: true
+        )
+      end
+
+      it 'delegates to ResetAbsent' do
+        entry_processor.reset_absent_items
+
+        expect(ProductImport::ResetAbsent)
+          .to have_received(:new).with(entry_processor, settings)
+      end
     end
   end
 
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 4551c217d3..1d50e65a7d 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -24,16 +24,6 @@ describe ProductImport::ResetAbsent do
   let(:reset_absent) { described_class.new(entry_processor, settings) }
 
   describe '#call' do
-    context 'when there is no data' do
-      let(:settings) do
-        instance_double(ProductImport::Settings, data_for_stock_reset?: false)
-      end
-
-      it 'returns nil' do
-        expect(reset_absent.call).to be_nil
-      end
-    end
-
     context 'when there are no enterprises_to_reset' do
       let(:settings) do
         instance_double(
@@ -118,35 +108,6 @@ describe ProductImport::ResetAbsent do
       end
     end
 
-    context 'when reset_all_absent is not set' do
-      let(:settings) do
-        instance_double(
-          ProductImport::Settings,
-          reset_all_absent?: false,
-          data_for_stock_reset?: true,
-          updated_ids: [0],
-          enterprises_to_reset: ['1']
-        )
-      end
-
-      before do
-        allow(entry_processor)
-          .to receive(:permission_by_id?).with('1') { true }
-      end
-
-      it 'does not reset anything' do
-        reset_absent.call
-
-        suppliers_to_reset_products = reset_absent
-          .instance_variable_get('@suppliers_to_reset_products')
-        suppliers_to_reset_inventories = reset_absent
-          .instance_variable_get('@suppliers_to_reset_inventories')
-
-        expect(suppliers_to_reset_products).to eq([])
-        expect(suppliers_to_reset_inventories).to eq([])
-      end
-    end
-
     context 'when the enterprise has no permission' do
       let(:settings) do
         instance_double(

From 8d7a11b0ac18067eaec2ad757fac6a6352069798 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 13:58:41 +0200
Subject: [PATCH 13/28] Make all steps of the algorithm have 2 branches

---
 app/models/product_import/reset_absent.rb | 29 +++++++++++------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index d1c0197d9c..f3d08456d4 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -27,7 +27,7 @@ module ProductImport
         end
       end
 
-      unless @suppliers_to_reset_inventories.empty?
+      if @suppliers_to_reset_inventories.present?
         relation = VariantOverride
           .where(
             'variant_overrides.hub_id IN (?) ' \
@@ -36,21 +36,20 @@ module ProductImport
             settings.updated_ids
           )
         @products_reset_count += relation.update_all(count_on_hand: 0)
+        nil
+      elsif @suppliers_to_reset_products.present?
+        relation = Spree::Variant
+          .joins(:product)
+          .where(
+            'spree_products.supplier_id IN (?) ' \
+            'AND spree_variants.id NOT IN (?) ' \
+            'AND spree_variants.is_master = false ' \
+            'AND spree_variants.deleted_at IS NULL',
+            @suppliers_to_reset_products,
+            settings.updated_ids
+          )
+        @products_reset_count += relation.update_all(count_on_hand: 0)
       end
-
-      return if @suppliers_to_reset_products.empty?
-
-      relation = Spree::Variant
-        .joins(:product)
-        .where(
-          'spree_products.supplier_id IN (?) ' \
-          'AND spree_variants.id NOT IN (?) ' \
-          'AND spree_variants.is_master = false ' \
-          'AND spree_variants.deleted_at IS NULL',
-          @suppliers_to_reset_products,
-          settings.updated_ids
-        )
-      @products_reset_count += relation.update_all(count_on_hand: 0)
     end
 
     private

From a9444b8909b7665545c689876989bf60825ced8d Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 16:47:04 +0200
Subject: [PATCH 14/28] Extract InventoryReset and Products strategies

Extract this strategy classes from ResetAbsent and move #reset there.
---
 app/models/product_import/inventory_reset.rb  | 25 +++++++
 app/models/product_import/products_reset.rb   | 29 +++++++
 app/models/product_import/reset_absent.rb     | 53 +++++++------
 .../product_import/reset_absent_spec.rb       | 75 +++++++++++++------
 4 files changed, 138 insertions(+), 44 deletions(-)
 create mode 100644 app/models/product_import/inventory_reset.rb
 create mode 100644 app/models/product_import/products_reset.rb

diff --git a/app/models/product_import/inventory_reset.rb b/app/models/product_import/inventory_reset.rb
new file mode 100644
index 0000000000..136a5856ee
--- /dev/null
+++ b/app/models/product_import/inventory_reset.rb
@@ -0,0 +1,25 @@
+module ProductImport
+  class InventoryReset
+    def initialize(updated_ids, supplier_ids)
+      @updated_ids = updated_ids
+      @supplier_ids = supplier_ids
+    end
+
+    def reset
+      relation.update_all(count_on_hand: 0)
+    end
+
+    private
+
+    attr_reader :updated_ids, :supplier_ids
+
+    def relation
+      VariantOverride.where(
+        'variant_overrides.hub_id IN (?) ' \
+        'AND variant_overrides.id NOT IN (?)',
+        supplier_ids,
+        updated_ids
+      )
+    end
+  end
+end
diff --git a/app/models/product_import/products_reset.rb b/app/models/product_import/products_reset.rb
new file mode 100644
index 0000000000..fd93c3b684
--- /dev/null
+++ b/app/models/product_import/products_reset.rb
@@ -0,0 +1,29 @@
+module ProductImport
+  class ProductsReset
+    def initialize(updated_ids, supplier_ids)
+      @updated_ids = updated_ids
+      @supplier_ids = supplier_ids
+    end
+
+    def reset
+      relation.update_all(count_on_hand: 0)
+    end
+
+    private
+
+    attr_reader :updated_ids, :supplier_ids
+
+    def relation
+      Spree::Variant
+        .joins(:product)
+        .where(
+          'spree_products.supplier_id IN (?) ' \
+          'AND spree_variants.id NOT IN (?) ' \
+          'AND spree_variants.is_master = false ' \
+          'AND spree_variants.deleted_at IS NULL',
+          supplier_ids,
+          updated_ids
+        )
+    end
+  end
+end
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index f3d08456d4..8925c2386c 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -27,33 +27,40 @@ module ProductImport
         end
       end
 
-      if @suppliers_to_reset_inventories.present?
-        relation = VariantOverride
-          .where(
-            'variant_overrides.hub_id IN (?) ' \
-            'AND variant_overrides.id NOT IN (?)',
-            @suppliers_to_reset_inventories,
-            settings.updated_ids
-          )
-        @products_reset_count += relation.update_all(count_on_hand: 0)
-        nil
-      elsif @suppliers_to_reset_products.present?
-        relation = Spree::Variant
-          .joins(:product)
-          .where(
-            'spree_products.supplier_id IN (?) ' \
-            'AND spree_variants.id NOT IN (?) ' \
-            'AND spree_variants.is_master = false ' \
-            'AND spree_variants.deleted_at IS NULL',
-            @suppliers_to_reset_products,
-            settings.updated_ids
-          )
-        @products_reset_count += relation.update_all(count_on_hand: 0)
-      end
+      reset_stock if suppliers_to_reset?
     end
 
     private
 
     attr_reader :settings
+
+    def reset_stock
+      @products_reset_count += strategy.reset
+    end
+
+    def strategy
+      strategy_factory.new(settings.updated_ids, supplier_ids)
+    end
+
+    def strategy_factory
+      if @suppliers_to_reset_inventories.present?
+        ProductImport::InventoryReset
+      elsif @suppliers_to_reset_products.present?
+        ProductImport::ProductsReset
+      end
+    end
+
+    def supplier_ids
+      if @suppliers_to_reset_inventories.present?
+        @suppliers_to_reset_inventories
+      elsif @suppliers_to_reset_products.present?
+        @suppliers_to_reset_products
+      end
+    end
+
+    def suppliers_to_reset?
+      @suppliers_to_reset_inventories.present? ||
+        @suppliers_to_reset_products.present?
+    end
   end
 end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 1d50e65a7d..6ebe7db206 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -94,8 +94,8 @@ describe ProductImport::ResetAbsent do
             .to receive(:importing_into_inventory?) { true }
         end
 
-        it 'returns nil' do
-          expect(reset_absent.call).to be_nil
+        it 'returns the number of products reset' do
+          expect(reset_absent.call).to eq(1)
         end
 
         it 'resets the inventories of the specified suppliers' do
@@ -139,30 +139,63 @@ describe ProductImport::ResetAbsent do
   end
 
   describe '#products_reset_count' do
-    let(:variant) { create(:variant) }
-    let(:enterprise_id) { variant.product.supplier_id }
+    context 'and importing into inventory' do
+      let(:variant) { create(:variant) }
+      let(:enterprise) { variant.product.supplier }
+      let(:variant_override) do
+        create(:variant_override, variant: variant, hub: enterprise)
+      end
 
-    before do
-      allow(entry_processor)
-        .to receive(:permission_by_id?).with(enterprise_id.to_s) { true }
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
+          updated_ids: [0],
+          enterprises_to_reset: [enterprise.id.to_s]
+        )
+      end
 
-      allow(entry_processor)
-        .to receive(:importing_into_inventory?) { false }
+      before do
+        variant_override
+
+        allow(entry_processor)
+          .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
+      end
+
+      before do
+        allow(entry_processor)
+          .to receive(:importing_into_inventory?) { true }
+      end
+
+      it 'returns the number of reset variant overrides' do
+        reset_absent.call
+        expect(reset_absent.products_reset_count).to eq(1)
+      end
     end
 
-    let(:settings) do
-      instance_double(
-        ProductImport::Settings,
-        reset_all_absent?: true,
-        data_for_stock_reset?: true,
-        updated_ids: [0],
-        enterprises_to_reset: [enterprise_id.to_s]
-      )
-    end
+    context 'and not importing into inventory' do
+      let(:variant) { create(:variant) }
+      let(:enterprise_id) { variant.product.supplier_id }
 
-    it 'returns the number of reset products or variants' do
-      reset_absent.call
-      expect(reset_absent.products_reset_count).to eq(2)
+      before do
+        allow(entry_processor)
+          .to receive(:permission_by_id?).with(enterprise_id.to_s) { true }
+
+        allow(entry_processor)
+          .to receive(:importing_into_inventory?) { false }
+      end
+
+      let(:settings) do
+        instance_double(
+          ProductImport::Settings,
+          updated_ids: [0],
+          enterprises_to_reset: [enterprise_id.to_s]
+        )
+      end
+
+      it 'returns the number of reset products or variants' do
+        reset_absent.call
+        expect(reset_absent.products_reset_count).to eq(2)
+      end
     end
   end
 end

From a10e58e20acb59a0e3ac5ebdef464ab12be4bd7d Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 17:53:21 +0200
Subject: [PATCH 15/28] Inject strategy_factory into ResetAbsent

---
 app/models/product_import/entry_processor.rb     | 11 ++++++++++-
 app/models/product_import/reset_absent.rb        | 13 +++----------
 .../product_import/entry_processor_spec.rb       |  8 ++++++--
 spec/models/product_import/reset_absent_spec.rb  | 16 +++++++++++++++-
 4 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 5387da4be1..085df93ad1 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -72,10 +72,19 @@ module ProductImport
     def reset_absent
       @reset_absent ||= ResetAbsent.new(
         self,
-        Settings.new(import_settings)
+        Settings.new(import_settings),
+        strategy_factory
       )
     end
 
+    def strategy_factory
+      if importing_into_inventory?
+        InventoryReset
+      else
+        ProductsReset
+      end
+    end
+
     def total_saved_count
       @products_created + @variants_created + @variants_updated + @inventory_created + @inventory_updated
     end
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 8925c2386c..94bdc6d997 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -4,11 +4,12 @@ module ProductImport
   class ResetAbsent < SimpleDelegator
     attr_reader :products_reset_count
 
-    def initialize(decorated, settings)
+    def initialize(decorated, settings, strategy_factory)
       super(decorated)
       @products_reset_count = 0
 
       @settings = settings
+      @strategy_factory = strategy_factory
 
       @suppliers_to_reset_products = []
       @suppliers_to_reset_inventories = []
@@ -32,7 +33,7 @@ module ProductImport
 
     private
 
-    attr_reader :settings
+    attr_reader :settings, :strategy_factory
 
     def reset_stock
       @products_reset_count += strategy.reset
@@ -42,14 +43,6 @@ module ProductImport
       strategy_factory.new(settings.updated_ids, supplier_ids)
     end
 
-    def strategy_factory
-      if @suppliers_to_reset_inventories.present?
-        ProductImport::InventoryReset
-      elsif @suppliers_to_reset_products.present?
-        ProductImport::ProductsReset
-      end
-    end
-
     def supplier_ids
       if @suppliers_to_reset_inventories.present?
         @suppliers_to_reset_inventories
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index 96c1b79b87..af6dbd4b17 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -66,7 +66,8 @@ describe ProductImport::EntryProcessor do
         instance_double(
           ProductImport::Settings,
           data_for_stock_reset?: true,
-          reset_all_absent?: true
+          reset_all_absent?: true,
+          importing_into_inventory?: true
         )
       end
 
@@ -74,7 +75,8 @@ describe ProductImport::EntryProcessor do
         entry_processor.reset_absent_items
 
         expect(ProductImport::ResetAbsent)
-          .to have_received(:new).with(entry_processor, settings)
+          .to have_received(:new)
+          .with(entry_processor, settings, ProductImport::InventoryReset)
       end
     end
   end
@@ -88,6 +90,8 @@ describe ProductImport::EntryProcessor do
         .and_return(reset_absent)
 
       allow(reset_absent).to receive(:products_reset_count)
+
+      allow(import_settings).to receive(:[]).with(:settings)
     end
 
     it 'delegates to ResetAbsent' do
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 6ebe7db206..dd965a9907 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -21,7 +21,9 @@ describe ProductImport::ResetAbsent do
     )
   end
 
-  let(:reset_absent) { described_class.new(entry_processor, settings) }
+  let(:reset_absent) do
+    described_class.new(entry_processor, settings, strategy_factory)
+  end
 
   describe '#call' do
     context 'when there are no enterprises_to_reset' do
@@ -32,6 +34,8 @@ describe ProductImport::ResetAbsent do
         )
       end
 
+      let(:strategy_factory) { double(:strategy_factory) }
+
       it 'returns nil' do
         expect(reset_absent.call).to be_nil
       end
@@ -57,6 +61,8 @@ describe ProductImport::ResetAbsent do
         let(:variant) { create(:variant) }
         let(:enterprise) { variant.product.supplier }
 
+        let(:strategy_factory) { ProductImport::ProductsReset }
+
         before do
           allow(entry_processor)
             .to receive(:importing_into_inventory?) { false }
@@ -82,6 +88,8 @@ describe ProductImport::ResetAbsent do
           create(:variant_override, variant: variant, hub: enterprise)
         end
 
+        let(:strategy_factory) { ProductImport::InventoryReset }
+
         before do
           variant_override
 
@@ -119,6 +127,8 @@ describe ProductImport::ResetAbsent do
         )
       end
 
+      let(:strategy_factory) { double(:strategy_factory) }
+
       before do
         allow(entry_processor)
           .to receive(:permission_by_id?).with('1') { false }
@@ -154,6 +164,8 @@ describe ProductImport::ResetAbsent do
         )
       end
 
+      let(:strategy_factory) { ProductImport::InventoryReset }
+
       before do
         variant_override
 
@@ -192,6 +204,8 @@ describe ProductImport::ResetAbsent do
         )
       end
 
+      let(:strategy_factory) { ProductImport::ProductsReset }
+
       it 'returns the number of reset products or variants' do
         reset_absent.call
         expect(reset_absent.products_reset_count).to eq(2)

From 54d6bc5443a708204449affb6655ddeb6d6cbd53 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 18:31:25 +0200
Subject: [PATCH 16/28] Don't use importing_into_inventory? in ResetAbsent

This completely decouples ResetAbsent from the particular strategy used.
---
 app/models/product_import/entry_processor.rb  |  4 +++
 app/models/product_import/inventory_reset.rb  | 16 ++++++---
 app/models/product_import/products_reset.rb   | 16 ++++++---
 app/models/product_import/reset_absent.rb     | 33 ++++---------------
 .../product_import/reset_absent_spec.rb       | 32 +++++++-----------
 5 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 085df93ad1..5bdb0f6b4d 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -85,6 +85,10 @@ module ProductImport
       end
     end
 
+    def strategy
+      @strategy ||= strategy_factory.new
+    end
+
     def total_saved_count
       @products_created + @variants_created + @variants_updated + @inventory_created + @inventory_updated
     end
diff --git a/app/models/product_import/inventory_reset.rb b/app/models/product_import/inventory_reset.rb
index 136a5856ee..7bc9b45dd5 100644
--- a/app/models/product_import/inventory_reset.rb
+++ b/app/models/product_import/inventory_reset.rb
@@ -1,17 +1,23 @@
 module ProductImport
   class InventoryReset
-    def initialize(updated_ids, supplier_ids)
-      @updated_ids = updated_ids
-      @supplier_ids = supplier_ids
+    attr_reader :supplier_ids
+
+    def initialize
+      @supplier_ids = []
     end
 
-    def reset
+    def <<(values)
+      @supplier_ids << values
+    end
+
+    def reset(updated_ids, _supplier_ids)
+      @updated_ids = updated_ids
       relation.update_all(count_on_hand: 0)
     end
 
     private
 
-    attr_reader :updated_ids, :supplier_ids
+    attr_reader :updated_ids
 
     def relation
       VariantOverride.where(
diff --git a/app/models/product_import/products_reset.rb b/app/models/product_import/products_reset.rb
index fd93c3b684..d3580bf2a7 100644
--- a/app/models/product_import/products_reset.rb
+++ b/app/models/product_import/products_reset.rb
@@ -1,17 +1,23 @@
 module ProductImport
   class ProductsReset
-    def initialize(updated_ids, supplier_ids)
-      @updated_ids = updated_ids
-      @supplier_ids = supplier_ids
+    attr_reader :supplier_ids
+
+    def initialize
+      @supplier_ids = []
     end
 
-    def reset
+    def <<(values)
+      @supplier_ids << values
+    end
+
+    def reset(updated_ids, _supplier_ids)
+      @updated_ids = updated_ids
       relation.update_all(count_on_hand: 0)
     end
 
     private
 
-    attr_reader :updated_ids, :supplier_ids
+    attr_reader :updated_ids
 
     def relation
       Spree::Variant
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 94bdc6d997..0c7526f7b0 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -10,9 +10,6 @@ module ProductImport
 
       @settings = settings
       @strategy_factory = strategy_factory
-
-      @suppliers_to_reset_products = []
-      @suppliers_to_reset_inventories = []
     end
 
     # For selected enterprises; set stock to zero for all products/inventory
@@ -21,14 +18,10 @@ module ProductImport
       settings.enterprises_to_reset.each do |enterprise_id|
         next unless permission_by_id?(enterprise_id)
 
-        if importing_into_inventory?
-          @suppliers_to_reset_inventories << enterprise_id.to_i
-        else
-          @suppliers_to_reset_products << enterprise_id.to_i
-        end
+        strategy << enterprise_id.to_i
       end
 
-      reset_stock if suppliers_to_reset?
+      reset_stock if strategy.supplier_ids
     end
 
     private
@@ -36,24 +29,10 @@ module ProductImport
     attr_reader :settings, :strategy_factory
 
     def reset_stock
-      @products_reset_count += strategy.reset
-    end
-
-    def strategy
-      strategy_factory.new(settings.updated_ids, supplier_ids)
-    end
-
-    def supplier_ids
-      if @suppliers_to_reset_inventories.present?
-        @suppliers_to_reset_inventories
-      elsif @suppliers_to_reset_products.present?
-        @suppliers_to_reset_products
-      end
-    end
-
-    def suppliers_to_reset?
-      @suppliers_to_reset_inventories.present? ||
-        @suppliers_to_reset_products.present?
+      @products_reset_count += strategy.reset(
+        settings.updated_ids,
+        strategy.supplier_ids
+      )
     end
   end
 end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index dd965a9907..cd85a7dc4a 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -30,12 +30,17 @@ describe ProductImport::ResetAbsent do
       let(:settings) do
         instance_double(
           ProductImport::Settings,
-          enterprises_to_reset: []
+          enterprises_to_reset: [],
+          updated_ids: []
         )
       end
 
       let(:strategy_factory) { double(:strategy_factory) }
 
+      before do
+        allow(import_settings).to receive(:[]).with(:settings)
+      end
+
       it 'returns nil' do
         expect(reset_absent.call).to be_nil
       end
@@ -72,12 +77,9 @@ describe ProductImport::ResetAbsent do
           expect(reset_absent.call).to eq(2)
         end
 
-        it 'resets the products of the specified suppliers' do
-          suppliers_to_reset_products = reset_absent
-            .instance_variable_get('@suppliers_to_reset_products')
-
+        xit 'resets the products of the specified suppliers' do
           reset_absent.call
-          expect(suppliers_to_reset_products).to eq([enterprise.id])
+          expect(strategy.supplier_ids).to eq([enterprise.id])
         end
       end
 
@@ -106,12 +108,9 @@ describe ProductImport::ResetAbsent do
           expect(reset_absent.call).to eq(1)
         end
 
-        it 'resets the inventories of the specified suppliers' do
-          suppliers_to_reset_inventories = reset_absent
-            .instance_variable_get('@suppliers_to_reset_inventories')
-
+        xit 'resets the inventories of the specified suppliers' do
           reset_absent.call
-          expect(suppliers_to_reset_inventories).to eq([enterprise.id])
+          expect(strategy.supplier_ids).to eq([enterprise.id])
         end
       end
     end
@@ -134,16 +133,9 @@ describe ProductImport::ResetAbsent do
           .to receive(:permission_by_id?).with('1') { false }
       end
 
-      it 'does not reset anything' do
+      xit 'does not reset anything' do
         reset_absent.call
-
-        suppliers_to_reset_products = reset_absent
-          .instance_variable_get('@suppliers_to_reset_products')
-        suppliers_to_reset_inventories = reset_absent
-          .instance_variable_get('@suppliers_to_reset_inventories')
-
-        expect(suppliers_to_reset_products).to eq([])
-        expect(suppliers_to_reset_inventories).to eq([])
+        expect(strategy.supplier_ids).to eq([])
       end
     end
   end

From 95ae18a1ba17f5966c968d000e75d23ab3cc9bf2 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 20:57:36 +0200
Subject: [PATCH 17/28] Remove method delegation

---
 app/models/product_import/entry_processor.rb      |  5 ++---
 .../models/product_import/entry_processor_spec.rb | 15 ---------------
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 5bdb0f6b4d..e770e16931 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -5,7 +5,6 @@
 module ProductImport
   class EntryProcessor
     delegate :products_reset_count, to: :reset_absent
-    delegate :importing_into_inventory?, to: :settings
 
     attr_reader :inventory_created, :inventory_updated, :products_created, :variants_created, :variants_updated, :supplier_products, :total_supplier_products, :import_settings
 
@@ -47,7 +46,7 @@ module ProductImport
         next unless supplier_id && permission_by_id?(supplier_id)
 
         products_count =
-          if importing_into_inventory?
+          if settings.importing_into_inventory?
             VariantOverride.where('variant_overrides.hub_id IN (?)', supplier_id).count
           else
             Spree::Variant.
@@ -78,7 +77,7 @@ module ProductImport
     end
 
     def strategy_factory
-      if importing_into_inventory?
+      if settings.importing_into_inventory?
         InventoryReset
       else
         ProductsReset
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index af6dbd4b17..b062b6e90a 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -99,19 +99,4 @@ describe ProductImport::EntryProcessor do
       expect(reset_absent).to have_received(:products_reset_count)
     end
   end
-
-  describe '#importing_into_inventory?' do
-    let(:settings) do
-      instance_double(ProductImport::Settings, importing_into_inventory?: true)
-    end
-
-    before do
-      allow(ProductImport::Settings).to receive(:new) { settings }
-    end
-
-    it 'delegates to Settings' do
-      entry_processor.importing_into_inventory?
-      expect(settings).to have_received(:importing_into_inventory?)
-    end
-  end
 end

From f04fa4ed63a32f207a7e657d4bc469262e236db3 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 21:03:01 +0200
Subject: [PATCH 18/28] Do not treat ResetAbsent as a decorator anymore

This fully encapsulates it's logic and reduces its tight coupling with
EntryProcessor.
---
 app/models/product_import/entry_processor.rb  |   6 +-
 app/models/product_import/reset_absent.rb     |  20 +-
 .../product_import/entry_processor_spec.rb    |  54 +++++-
 .../product_import/reset_absent_spec.rb       | 180 +++++-------------
 4 files changed, 98 insertions(+), 162 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index e770e16931..05ca71c8d4 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -69,11 +69,7 @@ module ProductImport
     end
 
     def reset_absent
-      @reset_absent ||= ResetAbsent.new(
-        self,
-        Settings.new(import_settings),
-        strategy_factory
-      )
+      @reset_absent ||= ResetAbsent.new(self, settings, strategy)
     end
 
     def strategy_factory
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 0c7526f7b0..593b74f1b2 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -1,32 +1,30 @@
-require 'delegate'
-
 module ProductImport
-  class ResetAbsent < SimpleDelegator
+  class ResetAbsent
     attr_reader :products_reset_count
 
-    def initialize(decorated, settings, strategy_factory)
-      super(decorated)
-      @products_reset_count = 0
-
+    def initialize(entry_processor, settings, strategy)
+      @entry_processor = entry_processor
       @settings = settings
-      @strategy_factory = strategy_factory
+      @strategy = strategy
+
+      @products_reset_count = 0
     end
 
     # For selected enterprises; set stock to zero for all products/inventory
     # that were not listed in the newly uploaded spreadsheet
     def call
       settings.enterprises_to_reset.each do |enterprise_id|
-        next unless permission_by_id?(enterprise_id)
+        next unless entry_processor.permission_by_id?(enterprise_id)
 
         strategy << enterprise_id.to_i
       end
 
-      reset_stock if strategy.supplier_ids
+      reset_stock if strategy.supplier_ids.present?
     end
 
     private
 
-    attr_reader :settings, :strategy_factory
+    attr_reader :settings, :strategy, :entry_processor
 
     def reset_stock
       @products_reset_count += strategy.reset(
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index b062b6e90a..1f2e7eda62 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -67,31 +67,65 @@ describe ProductImport::EntryProcessor do
           ProductImport::Settings,
           data_for_stock_reset?: true,
           reset_all_absent?: true,
-          importing_into_inventory?: true
+          updated_ids: [1]
         )
       end
+      context 'when importing into inventory' do
+        let(:strategy) { instance_double(ProductImport::InventoryReset) }
 
-      it 'delegates to ResetAbsent' do
-        entry_processor.reset_absent_items
+        before do
+          allow(settings).to receive(:importing_into_inventory?) { true }
 
-        expect(ProductImport::ResetAbsent)
-          .to have_received(:new)
-          .with(entry_processor, settings, ProductImport::InventoryReset)
+          allow(ProductImport::InventoryReset)
+            .to receive(:new).with([1]) { strategy }
+        end
+
+        it 'delegates to ResetAbsent passing the appropriate strategy' do
+          entry_processor.reset_absent_items
+
+          expect(ProductImport::ResetAbsent)
+            .to have_received(:new)
+            .with(entry_processor, settings, strategy)
+        end
+      end
+
+      context 'when not importing into inventory' do
+        let(:strategy) { instance_double(ProductImport::ProductsReset) }
+
+        before do
+          allow(settings).to receive(:importing_into_inventory?) { false }
+
+          allow(ProductImport::ProductsReset)
+            .to receive(:new).with([1]) { strategy }
+        end
+
+        it 'delegates to ResetAbsent passing the appropriate strategy' do
+          entry_processor.reset_absent_items
+
+          expect(ProductImport::ResetAbsent)
+            .to have_received(:new)
+            .with(entry_processor, settings, strategy)
+        end
       end
     end
   end
 
   describe '#products_reset_count' do
     let(:reset_absent) { instance_double(ProductImport::ResetAbsent) }
+    let(:settings) do
+      instance_double(
+        ProductImport::Settings,
+        importing_into_inventory?: false,
+        updated_ids: [1]
+      )
+    end
 
     before do
+      allow(ProductImport::Settings).to receive(:new) { settings }
       allow(ProductImport::ResetAbsent)
-        .to receive(:new)
-        .and_return(reset_absent)
+        .to receive(:new).and_return(reset_absent)
 
       allow(reset_absent).to receive(:products_reset_count)
-
-      allow(import_settings).to receive(:[]).with(:settings)
     end
 
     it 'delegates to ResetAbsent' do
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index cd85a7dc4a..3e0ca117d5 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -1,28 +1,10 @@
 require 'spec_helper'
 
 describe ProductImport::ResetAbsent do
-  let(:importer) { double(:importer) }
-  let(:validator) { double(:validator) }
-  let(:spreadsheet_data) { double(:spreadsheet_data) }
-  let(:editable_enterprises) { double(:editable_enterprises) }
-  let(:import_time) { double(:import_time) }
-  let(:updated_ids) { double(:updated_ids) }
-  let(:import_settings) { double(:import_settings) }
-
-  let(:entry_processor) do
-    ProductImport::EntryProcessor.new(
-      importer,
-      validator,
-      import_settings,
-      spreadsheet_data,
-      editable_enterprises,
-      import_time,
-      updated_ids
-    )
-  end
+  let(:entry_processor) { instance_double(ProductImport::EntryProcessor) }
 
   let(:reset_absent) do
-    described_class.new(entry_processor, settings, strategy_factory)
+    described_class.new(entry_processor, settings, strategy)
   end
 
   describe '#call' do
@@ -35,10 +17,8 @@ describe ProductImport::ResetAbsent do
         )
       end
 
-      let(:strategy_factory) { double(:strategy_factory) }
-
-      before do
-        allow(import_settings).to receive(:[]).with(:settings)
+      let(:strategy) do
+        instance_double(ProductImport::InventoryReset, supplier_ids: [])
       end
 
       it 'returns nil' do
@@ -47,161 +27,89 @@ describe ProductImport::ResetAbsent do
     end
 
     context 'when there are updated_ids and enterprises_to_reset' do
+      let(:enterprise) { instance_double(Enterprise, id: 1) }
+
       let(:settings) do
         instance_double(
           ProductImport::Settings,
-          reset_all_absent?: true,
-          data_for_stock_reset?: true,
           updated_ids: [0],
           enterprises_to_reset: [enterprise.id.to_s]
         )
       end
 
+      let(:strategy) { instance_double(ProductImport::ProductsReset) }
+
       before do
         allow(entry_processor)
           .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
+
+        allow(strategy).to receive(:<<).with(enterprise.id)
+        allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
+        allow(strategy).to receive(:reset).with([0], [enterprise.id]) { 2 }
       end
 
-      context 'and not importing into inventory' do
-        let(:variant) { create(:variant) }
-        let(:enterprise) { variant.product.supplier }
-
-        let(:strategy_factory) { ProductImport::ProductsReset }
-
-        before do
-          allow(entry_processor)
-            .to receive(:importing_into_inventory?) { false }
-        end
-
-        it 'returns the number of products reset' do
-          expect(reset_absent.call).to eq(2)
-        end
-
-        xit 'resets the products of the specified suppliers' do
-          reset_absent.call
-          expect(strategy.supplier_ids).to eq([enterprise.id])
-        end
+      it 'returns the number of products reset' do
+        expect(reset_absent.call).to eq(2)
       end
 
-      context 'and importing into inventory' do
-        let(:variant) { create(:variant) }
-        let(:enterprise) { variant.product.supplier }
-        let(:variant_override) do
-          create(:variant_override, variant: variant, hub: enterprise)
-        end
-
-        let(:strategy_factory) { ProductImport::InventoryReset }
-
-        before do
-          variant_override
-
-          allow(entry_processor)
-            .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
-        end
-
-        before do
-          allow(entry_processor)
-            .to receive(:importing_into_inventory?) { true }
-        end
-
-        it 'returns the number of products reset' do
-          expect(reset_absent.call).to eq(1)
-        end
-
-        xit 'resets the inventories of the specified suppliers' do
-          reset_absent.call
-          expect(strategy.supplier_ids).to eq([enterprise.id])
-        end
+      it 'resets the products of the specified suppliers' do
+        expect(strategy).to receive(:reset).with([0], [enterprise.id]) { 2 }
+        reset_absent.call
       end
     end
 
     context 'when the enterprise has no permission' do
+      let(:enterprise) { instance_double(Enterprise, id: 1) }
+
       let(:settings) do
         instance_double(
           ProductImport::Settings,
-          reset_all_absent?: true,
-          data_for_stock_reset?: true,
           updated_ids: [0],
-          enterprises_to_reset: ['1']
+          enterprises_to_reset: [enterprise.id.to_s]
         )
       end
 
-      let(:strategy_factory) { double(:strategy_factory) }
+      let(:strategy) { instance_double(ProductImport::InventoryReset) }
 
       before do
         allow(entry_processor)
-          .to receive(:permission_by_id?).with('1') { false }
+          .to receive(:permission_by_id?).with(enterprise.id.to_s) { false }
+
+        allow(strategy).to receive(:supplier_ids) { [] }
       end
 
-      xit 'does not reset anything' do
+      it 'does not reset stock' do
+        expect(strategy).not_to receive(:reset)
         reset_absent.call
-        expect(strategy.supplier_ids).to eq([])
       end
     end
   end
 
   describe '#products_reset_count' do
-    context 'and importing into inventory' do
-      let(:variant) { create(:variant) }
-      let(:enterprise) { variant.product.supplier }
-      let(:variant_override) do
-        create(:variant_override, variant: variant, hub: enterprise)
-      end
+    let(:enterprise) { instance_double(Enterprise, id: 1) }
 
-      let(:settings) do
-        instance_double(
-          ProductImport::Settings,
-          updated_ids: [0],
-          enterprises_to_reset: [enterprise.id.to_s]
-        )
-      end
-
-      let(:strategy_factory) { ProductImport::InventoryReset }
-
-      before do
-        variant_override
-
-        allow(entry_processor)
-          .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
-      end
-
-      before do
-        allow(entry_processor)
-          .to receive(:importing_into_inventory?) { true }
-      end
-
-      it 'returns the number of reset variant overrides' do
-        reset_absent.call
-        expect(reset_absent.products_reset_count).to eq(1)
-      end
+    let(:settings) do
+      instance_double(
+        ProductImport::Settings,
+        updated_ids: [0],
+        enterprises_to_reset: [enterprise.id.to_s]
+      )
     end
 
-    context 'and not importing into inventory' do
-      let(:variant) { create(:variant) }
-      let(:enterprise_id) { variant.product.supplier_id }
+    let(:strategy) { instance_double(ProductImport::InventoryReset) }
 
-      before do
-        allow(entry_processor)
-          .to receive(:permission_by_id?).with(enterprise_id.to_s) { true }
+    before do
+      allow(entry_processor)
+        .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
 
-        allow(entry_processor)
-          .to receive(:importing_into_inventory?) { false }
-      end
+      allow(strategy).to receive(:<<).with(enterprise.id)
+      allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
+      allow(strategy).to receive(:reset).with([0], [enterprise.id]) { 1 }
+    end
 
-      let(:settings) do
-        instance_double(
-          ProductImport::Settings,
-          updated_ids: [0],
-          enterprises_to_reset: [enterprise_id.to_s]
-        )
-      end
-
-      let(:strategy_factory) { ProductImport::ProductsReset }
-
-      it 'returns the number of reset products or variants' do
-        reset_absent.call
-        expect(reset_absent.products_reset_count).to eq(2)
-      end
+    it 'returns the number of reset variant overrides' do
+      reset_absent.call
+      expect(reset_absent.products_reset_count).to eq(1)
     end
   end
 end

From 718f529ede96511a78932ca0a2ab5be3c0c356e9 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 21:31:34 +0200
Subject: [PATCH 19/28] Use nested module in test to improve readability

---
 .../product_import/reset_absent_spec.rb       | 164 +++++++++---------
 1 file changed, 83 insertions(+), 81 deletions(-)

diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 3e0ca117d5..76826a1fc2 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -1,43 +1,103 @@
 require 'spec_helper'
 
-describe ProductImport::ResetAbsent do
-  let(:entry_processor) { instance_double(ProductImport::EntryProcessor) }
+module ProductImport
+  describe ResetAbsent do
+    let(:entry_processor) { instance_double(EntryProcessor) }
 
-  let(:reset_absent) do
-    described_class.new(entry_processor, settings, strategy)
-  end
+    let(:reset_absent) do
+      described_class.new(entry_processor, settings, strategy)
+    end
 
-  describe '#call' do
-    context 'when there are no enterprises_to_reset' do
-      let(:settings) do
-        instance_double(
-          ProductImport::Settings,
-          enterprises_to_reset: [],
-          updated_ids: []
-        )
+    describe '#call' do
+      context 'when there are no enterprises_to_reset' do
+        let(:settings) do
+          instance_double(
+            Settings,
+            enterprises_to_reset: [],
+            updated_ids: []
+          )
+        end
+
+        let(:strategy) do
+          instance_double(InventoryReset, supplier_ids: [])
+        end
+
+        it 'returns nil' do
+          expect(reset_absent.call).to be_nil
+        end
       end
 
-      let(:strategy) do
-        instance_double(ProductImport::InventoryReset, supplier_ids: [])
+      context 'when there are updated_ids and enterprises_to_reset' do
+        let(:enterprise) { instance_double(Enterprise, id: 1) }
+
+        let(:settings) do
+          instance_double(
+            Settings,
+            updated_ids: [0],
+            enterprises_to_reset: [enterprise.id.to_s]
+          )
+        end
+
+        let(:strategy) { instance_double(ProductsReset) }
+
+        before do
+          allow(entry_processor)
+            .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
+
+          allow(strategy).to receive(:<<).with(enterprise.id)
+          allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
+          allow(strategy).to receive(:reset).with([0], [enterprise.id]) { 2 }
+        end
+
+        it 'returns the number of products reset' do
+          expect(reset_absent.call).to eq(2)
+        end
+
+        it 'resets the products of the specified suppliers' do
+          expect(strategy).to receive(:reset).with([0], [enterprise.id]) { 2 }
+          reset_absent.call
+        end
       end
 
-      it 'returns nil' do
-        expect(reset_absent.call).to be_nil
+      context 'when the enterprise has no permission' do
+        let(:enterprise) { instance_double(Enterprise, id: 1) }
+
+        let(:settings) do
+          instance_double(
+            Settings,
+            updated_ids: [0],
+            enterprises_to_reset: [enterprise.id.to_s]
+          )
+        end
+
+        let(:strategy) { instance_double(InventoryReset) }
+
+        before do
+          allow(entry_processor)
+            .to receive(:permission_by_id?).with(enterprise.id.to_s) { false }
+
+          allow(strategy).to receive(:supplier_ids) { [] }
+        end
+
+        it 'does not reset stock' do
+          expect(strategy).not_to receive(:reset)
+          reset_absent.call
+        end
       end
     end
 
-    context 'when there are updated_ids and enterprises_to_reset' do
+    describe '#products_reset_count' do
       let(:enterprise) { instance_double(Enterprise, id: 1) }
 
       let(:settings) do
         instance_double(
-          ProductImport::Settings,
+          Settings,
           updated_ids: [0],
           enterprises_to_reset: [enterprise.id.to_s]
         )
       end
 
-      let(:strategy) { instance_double(ProductImport::ProductsReset) }
+      let(:strategy) { instance_double(InventoryReset) }
 
       before do
         allow(entry_processor)
@@ -45,71 +105,13 @@ describe ProductImport::ResetAbsent do
 
         allow(strategy).to receive(:<<).with(enterprise.id)
         allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
-        allow(strategy).to receive(:reset).with([0], [enterprise.id]) { 2 }
+        allow(strategy).to receive(:reset).with([0], [enterprise.id]) { 1 }
       end
 
-      it 'returns the number of products reset' do
-        expect(reset_absent.call).to eq(2)
-      end
-
-      it 'resets the products of the specified suppliers' do
-        expect(strategy).to receive(:reset).with([0], [enterprise.id]) { 2 }
+      it 'returns the number of reset variant overrides' do
         reset_absent.call
+        expect(reset_absent.products_reset_count).to eq(1)
       end
     end
-
-    context 'when the enterprise has no permission' do
-      let(:enterprise) { instance_double(Enterprise, id: 1) }
-
-      let(:settings) do
-        instance_double(
-          ProductImport::Settings,
-          updated_ids: [0],
-          enterprises_to_reset: [enterprise.id.to_s]
-        )
-      end
-
-      let(:strategy) { instance_double(ProductImport::InventoryReset) }
-
-      before do
-        allow(entry_processor)
-          .to receive(:permission_by_id?).with(enterprise.id.to_s) { false }
-
-        allow(strategy).to receive(:supplier_ids) { [] }
-      end
-
-      it 'does not reset stock' do
-        expect(strategy).not_to receive(:reset)
-        reset_absent.call
-      end
-    end
-  end
-
-  describe '#products_reset_count' do
-    let(:enterprise) { instance_double(Enterprise, id: 1) }
-
-    let(:settings) do
-      instance_double(
-        ProductImport::Settings,
-        updated_ids: [0],
-        enterprises_to_reset: [enterprise.id.to_s]
-      )
-    end
-
-    let(:strategy) { instance_double(ProductImport::InventoryReset) }
-
-    before do
-      allow(entry_processor)
-        .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
-
-      allow(strategy).to receive(:<<).with(enterprise.id)
-      allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
-      allow(strategy).to receive(:reset).with([0], [enterprise.id]) { 1 }
-    end
-
-    it 'returns the number of reset variant overrides' do
-      reset_absent.call
-      expect(reset_absent.products_reset_count).to eq(1)
-    end
   end
 end

From 186801a1e2064b5e8ba55267116c2266927a97b4 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 21:38:11 +0200
Subject: [PATCH 20/28] Remove unused supplier_ids argument

---
 app/models/product_import/inventory_reset.rb    | 2 +-
 app/models/product_import/products_reset.rb     | 2 +-
 app/models/product_import/reset_absent.rb       | 5 +----
 spec/models/product_import/reset_absent_spec.rb | 6 +++---
 4 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/app/models/product_import/inventory_reset.rb b/app/models/product_import/inventory_reset.rb
index 7bc9b45dd5..cd064777c2 100644
--- a/app/models/product_import/inventory_reset.rb
+++ b/app/models/product_import/inventory_reset.rb
@@ -10,7 +10,7 @@ module ProductImport
       @supplier_ids << values
     end
 
-    def reset(updated_ids, _supplier_ids)
+    def reset(updated_ids)
       @updated_ids = updated_ids
       relation.update_all(count_on_hand: 0)
     end
diff --git a/app/models/product_import/products_reset.rb b/app/models/product_import/products_reset.rb
index d3580bf2a7..7462a56523 100644
--- a/app/models/product_import/products_reset.rb
+++ b/app/models/product_import/products_reset.rb
@@ -10,7 +10,7 @@ module ProductImport
       @supplier_ids << values
     end
 
-    def reset(updated_ids, _supplier_ids)
+    def reset(updated_ids)
       @updated_ids = updated_ids
       relation.update_all(count_on_hand: 0)
     end
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 593b74f1b2..06b2eb4c43 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -27,10 +27,7 @@ module ProductImport
     attr_reader :settings, :strategy, :entry_processor
 
     def reset_stock
-      @products_reset_count += strategy.reset(
-        settings.updated_ids,
-        strategy.supplier_ids
-      )
+      @products_reset_count += strategy.reset(settings.updated_ids)
     end
   end
 end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 76826a1fc2..5ef90d8f55 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -46,7 +46,7 @@ module ProductImport
 
           allow(strategy).to receive(:<<).with(enterprise.id)
           allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
-          allow(strategy).to receive(:reset).with([0], [enterprise.id]) { 2 }
+          allow(strategy).to receive(:reset).with([0]) { 2 }
         end
 
         it 'returns the number of products reset' do
@@ -54,7 +54,7 @@ module ProductImport
         end
 
         it 'resets the products of the specified suppliers' do
-          expect(strategy).to receive(:reset).with([0], [enterprise.id]) { 2 }
+          expect(strategy).to receive(:reset).with([0]) { 2 }
           reset_absent.call
         end
       end
@@ -105,7 +105,7 @@ module ProductImport
 
         allow(strategy).to receive(:<<).with(enterprise.id)
         allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
-        allow(strategy).to receive(:reset).with([0], [enterprise.id]) { 1 }
+        allow(strategy).to receive(:reset).with([0]) { 1 }
       end
 
       it 'returns the number of reset variant overrides' do

From 5eb10edbfd4aeb2c971af4825f9a38fe7528a248 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 25 Sep 2018 21:43:08 +0200
Subject: [PATCH 21/28] Inject #updated_ids to strategy

Their values are known beforehand.
---
 app/models/product_import/entry_processor.rb    |  2 +-
 app/models/product_import/inventory_reset.rb    |  6 +++---
 app/models/product_import/products_reset.rb     |  6 +++---
 app/models/product_import/reset_absent.rb       |  2 +-
 spec/models/product_import/reset_absent_spec.rb | 14 +++++---------
 5 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 05ca71c8d4..8b45a2bf4f 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -81,7 +81,7 @@ module ProductImport
     end
 
     def strategy
-      @strategy ||= strategy_factory.new
+      @strategy ||= strategy_factory.new(settings.updated_ids)
     end
 
     def total_saved_count
diff --git a/app/models/product_import/inventory_reset.rb b/app/models/product_import/inventory_reset.rb
index cd064777c2..a03a008592 100644
--- a/app/models/product_import/inventory_reset.rb
+++ b/app/models/product_import/inventory_reset.rb
@@ -2,16 +2,16 @@ module ProductImport
   class InventoryReset
     attr_reader :supplier_ids
 
-    def initialize
+    def initialize(updated_ids)
       @supplier_ids = []
+      @updated_ids = updated_ids
     end
 
     def <<(values)
       @supplier_ids << values
     end
 
-    def reset(updated_ids)
-      @updated_ids = updated_ids
+    def reset
       relation.update_all(count_on_hand: 0)
     end
 
diff --git a/app/models/product_import/products_reset.rb b/app/models/product_import/products_reset.rb
index 7462a56523..219b656f12 100644
--- a/app/models/product_import/products_reset.rb
+++ b/app/models/product_import/products_reset.rb
@@ -2,16 +2,16 @@ module ProductImport
   class ProductsReset
     attr_reader :supplier_ids
 
-    def initialize
+    def initialize(updated_ids)
       @supplier_ids = []
+      @updated_ids = updated_ids
     end
 
     def <<(values)
       @supplier_ids << values
     end
 
-    def reset(updated_ids)
-      @updated_ids = updated_ids
+    def reset
       relation.update_all(count_on_hand: 0)
     end
 
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 06b2eb4c43..385b27d4ce 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -27,7 +27,7 @@ module ProductImport
     attr_reader :settings, :strategy, :entry_processor
 
     def reset_stock
-      @products_reset_count += strategy.reset(settings.updated_ids)
+      @products_reset_count += strategy.reset
     end
   end
 end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 5ef90d8f55..e10a3a025b 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -13,8 +13,7 @@ module ProductImport
         let(:settings) do
           instance_double(
             Settings,
-            enterprises_to_reset: [],
-            updated_ids: []
+            enterprises_to_reset: []
           )
         end
 
@@ -27,13 +26,12 @@ module ProductImport
         end
       end
 
-      context 'when there are updated_ids and enterprises_to_reset' do
+      context 'when there are enterprises_to_reset' do
         let(:enterprise) { instance_double(Enterprise, id: 1) }
 
         let(:settings) do
           instance_double(
             Settings,
-            updated_ids: [0],
             enterprises_to_reset: [enterprise.id.to_s]
           )
         end
@@ -46,7 +44,7 @@ module ProductImport
 
           allow(strategy).to receive(:<<).with(enterprise.id)
           allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
-          allow(strategy).to receive(:reset).with([0]) { 2 }
+          allow(strategy).to receive(:reset) { 2 }
         end
 
         it 'returns the number of products reset' do
@@ -54,7 +52,7 @@ module ProductImport
         end
 
         it 'resets the products of the specified suppliers' do
-          expect(strategy).to receive(:reset).with([0]) { 2 }
+          expect(strategy).to receive(:reset) { 2 }
           reset_absent.call
         end
       end
@@ -65,7 +63,6 @@ module ProductImport
         let(:settings) do
           instance_double(
             Settings,
-            updated_ids: [0],
             enterprises_to_reset: [enterprise.id.to_s]
           )
         end
@@ -92,7 +89,6 @@ module ProductImport
       let(:settings) do
         instance_double(
           Settings,
-          updated_ids: [0],
           enterprises_to_reset: [enterprise.id.to_s]
         )
       end
@@ -105,7 +101,7 @@ module ProductImport
 
         allow(strategy).to receive(:<<).with(enterprise.id)
         allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
-        allow(strategy).to receive(:reset).with([0]) { 1 }
+        allow(strategy).to receive(:reset) { 1 }
       end
 
       it 'returns the number of reset variant overrides' do

From d527f6265a883cb5ac3fbdc4e156c1a3cb674153 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 26 Sep 2018 11:37:55 +0200
Subject: [PATCH 22/28] Remove pointless sum

---
 app/models/product_import/reset_absent.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 385b27d4ce..854b19cc08 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -27,7 +27,7 @@ module ProductImport
     attr_reader :settings, :strategy, :entry_processor
 
     def reset_stock
-      @products_reset_count += strategy.reset
+      @products_reset_count = strategy.reset
     end
   end
 end

From fd69c7672daf5e49554d039f0bc2e040bae69911 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 26 Sep 2018 11:38:22 +0200
Subject: [PATCH 23/28] Add specs for ResetAbsent strategies

This also fixes the case where there are no overrides to exclude.
---
 app/models/product_import/inventory_reset.rb  | 16 ++---
 app/models/product_import/products_reset.rb   | 20 +++---
 .../product_import/inventory_reset_spec.rb    | 70 +++++++++++++++++++
 .../product_import/products_reset_spec.rb     | 60 ++++++++++++++++
 4 files changed, 147 insertions(+), 19 deletions(-)
 create mode 100644 spec/models/product_import/inventory_reset_spec.rb
 create mode 100644 spec/models/product_import/products_reset_spec.rb

diff --git a/app/models/product_import/inventory_reset.rb b/app/models/product_import/inventory_reset.rb
index a03a008592..9aec385da4 100644
--- a/app/models/product_import/inventory_reset.rb
+++ b/app/models/product_import/inventory_reset.rb
@@ -2,9 +2,9 @@ module ProductImport
   class InventoryReset
     attr_reader :supplier_ids
 
-    def initialize(updated_ids)
+    def initialize(excluded_items_ids)
       @supplier_ids = []
-      @updated_ids = updated_ids
+      @excluded_items_ids = excluded_items_ids
     end
 
     def <<(values)
@@ -17,15 +17,13 @@ module ProductImport
 
     private
 
-    attr_reader :updated_ids
+    attr_reader :excluded_items_ids
 
     def relation
-      VariantOverride.where(
-        'variant_overrides.hub_id IN (?) ' \
-        'AND variant_overrides.id NOT IN (?)',
-        supplier_ids,
-        updated_ids
-      )
+      relation = VariantOverride.where(hub_id: supplier_ids)
+      return relation if excluded_items_ids.blank?
+
+      relation.where('id NOT IN (?)', excluded_items_ids)
     end
   end
 end
diff --git a/app/models/product_import/products_reset.rb b/app/models/product_import/products_reset.rb
index 219b656f12..ad15283283 100644
--- a/app/models/product_import/products_reset.rb
+++ b/app/models/product_import/products_reset.rb
@@ -2,9 +2,9 @@ module ProductImport
   class ProductsReset
     attr_reader :supplier_ids
 
-    def initialize(updated_ids)
+    def initialize(excluded_items_ids)
       @supplier_ids = []
-      @updated_ids = updated_ids
+      @excluded_items_ids = excluded_items_ids
     end
 
     def <<(values)
@@ -17,19 +17,19 @@ module ProductImport
 
     private
 
-    attr_reader :updated_ids
+    attr_reader :excluded_items_ids
 
     def relation
-      Spree::Variant
+      relation = Spree::Variant
         .joins(:product)
         .where(
-          'spree_products.supplier_id IN (?) ' \
-          'AND spree_variants.id NOT IN (?) ' \
-          'AND spree_variants.is_master = false ' \
-          'AND spree_variants.deleted_at IS NULL',
-          supplier_ids,
-          updated_ids
+          spree_products: { supplier_id: supplier_ids },
+          spree_variants: { is_master: false, deleted_at: nil }
         )
+
+      return relation if excluded_items_ids.blank?
+
+      relation.where('spree_variants.id NOT IN (?)', excluded_items_ids)
     end
   end
 end
diff --git a/spec/models/product_import/inventory_reset_spec.rb b/spec/models/product_import/inventory_reset_spec.rb
new file mode 100644
index 0000000000..412739ae10
--- /dev/null
+++ b/spec/models/product_import/inventory_reset_spec.rb
@@ -0,0 +1,70 @@
+require 'spec_helper'
+
+describe ProductImport::InventoryReset do
+  let(:inventory_reset) { described_class.new(excluded_items_ids) }
+
+  describe '#<<' do
+    let(:excluded_items_ids) { [] }
+    let(:supplier_ids) { 1 }
+
+    it 'stores the specified supplier_ids' do
+      inventory_reset << supplier_ids
+      expect(inventory_reset.supplier_ids).to eq([supplier_ids])
+    end
+  end
+
+  describe '#reset' do
+    let(:supplier_ids) { enterprise.id }
+    let(:enterprise) { variant.product.supplier }
+    let(:variant) { create(:variant) }
+
+    let!(:variant_override) do
+      create(
+        :variant_override,
+        count_on_hand: 10,
+        hub: enterprise,
+        variant: variant
+      )
+    end
+
+    before { inventory_reset << supplier_ids }
+
+    context 'when there are excluded_items_ids' do
+      let(:excluded_items_ids) { [variant_override.id] }
+
+      it 'does not update the count_on_hand of the excluded items' do
+        inventory_reset.reset
+        expect(variant_override.reload.count_on_hand).to eq(10)
+      end
+
+      it 'updates the count_on_hand of the non-excluded items' do
+        non_excluded_variant_override = create(
+          :variant_override,
+          count_on_hand: 3,
+          hub: enterprise,
+          variant: variant
+        )
+        inventory_reset.reset
+        expect(non_excluded_variant_override.reload.count_on_hand).to eq(0)
+      end
+    end
+
+    context 'when there are no excluded_items_ids' do
+      let(:excluded_items_ids) { [] }
+
+      it 'sets all count_on_hand to 0' do
+        inventory_reset.reset
+        expect(variant_override.reload.count_on_hand).to eq(0)
+      end
+    end
+
+    context 'when excluded_items_ids is nil' do
+      let(:excluded_items_ids) { nil }
+
+      it 'sets all count_on_hand to 0' do
+        inventory_reset.reset
+        expect(variant_override.reload.count_on_hand).to eq(0)
+      end
+    end
+  end
+end
diff --git a/spec/models/product_import/products_reset_spec.rb b/spec/models/product_import/products_reset_spec.rb
new file mode 100644
index 0000000000..f84820ad81
--- /dev/null
+++ b/spec/models/product_import/products_reset_spec.rb
@@ -0,0 +1,60 @@
+require 'spec_helper'
+
+describe ProductImport::ProductsReset do
+  let(:products_reset) { described_class.new(excluded_items_ids) }
+
+  describe '#<<' do
+    let(:excluded_items_ids) { [] }
+    let(:supplier_ids) { 1 }
+
+    it 'stores the specified supplier_ids' do
+      products_reset << supplier_ids
+      expect(products_reset.supplier_ids).to eq([supplier_ids])
+    end
+  end
+
+  describe '#reset' do
+    let(:supplier_ids) { enterprise.id }
+    let(:enterprise) { variant.product.supplier }
+    let(:variant) { create(:variant, count_on_hand: 2) }
+
+    before { products_reset << supplier_ids }
+
+    context 'when there are excluded_items_ids' do
+      let(:excluded_items_ids) { [variant.id] }
+
+      it 'does not update the count_on_hand of the excluded items' do
+        products_reset.reset
+        expect(variant.reload.count_on_hand).to eq(2)
+      end
+
+      it 'updates the count_on_hand of the non-excluded items' do
+        non_excluded_variant = create(
+          :variant,
+          count_on_hand: 3,
+          product: variant.product
+        )
+        products_reset.reset
+        expect(non_excluded_variant.reload.count_on_hand).to eq(0)
+      end
+    end
+
+    context 'when there are no excluded_items_ids' do
+      let(:excluded_items_ids) { [] }
+
+      it 'sets all count_on_hand to 0' do
+        products_reset.reset
+        expect(variant.reload.count_on_hand).to eq(0)
+      end
+    end
+
+    context 'when excluded_items_ids is nil' do
+      let(:excluded_items_ids) { nil }
+
+      it 'sets all count_on_hand to 0' do
+        products_reset.reset
+        expect(variant.reload.count_on_hand).to eq(0)
+      end
+    end
+  end
+end

From 8715fce29583403d0983d080dc84d61e54ae8a58 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 27 Sep 2018 15:34:46 +0200
Subject: [PATCH 24/28] Remove @import_settings in favor of Settings

---
 app/models/product_import/entry_processor.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 8b45a2bf4f..38cd6a448f 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -6,12 +6,14 @@ module ProductImport
   class EntryProcessor
     delegate :products_reset_count, to: :reset_absent
 
-    attr_reader :inventory_created, :inventory_updated, :products_created, :variants_created, :variants_updated, :supplier_products, :total_supplier_products, :import_settings
+    attr_reader :inventory_created, :inventory_updated, :products_created,
+                :variants_created, :variants_updated, :supplier_products,
+                :total_supplier_products
 
     def initialize(importer, validator, import_settings, spreadsheet_data, editable_enterprises, import_time, updated_ids)
       @importer = importer
       @validator = validator
-      @import_settings = import_settings
+      @settings = Settings.new(import_settings)
       @spreadsheet_data = spreadsheet_data
       @editable_enterprises = editable_enterprises
       @import_time = import_time
@@ -24,8 +26,6 @@ module ProductImport
       @variants_updated = 0
       @supplier_products = {}
       @total_supplier_products = 0
-
-      @settings = Settings.new(import_settings)
     end
 
     def save_all(entries)
@@ -122,7 +122,7 @@ module ProductImport
     end
 
     def import_into_inventory?(entry)
-      entry.supplier_id && import_settings[:settings]['import_into'] == 'inventories'
+      entry.supplier_id && settings.importing_into_inventory?
     end
 
     def save_new_inventory_item(entry)

From af93af1a646ec05cb12814a3126820ec5b1a26a2 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 1 Oct 2018 15:53:26 +0200
Subject: [PATCH 25/28] Replace strategy with a more explicit name

---
 app/models/product_import/entry_processor.rb  |  9 +++---
 app/models/product_import/reset_absent.rb     | 12 ++++----
 .../product_import/entry_processor_spec.rb    | 21 ++++++++-----
 .../product_import/reset_absent_spec.rb       | 30 ++++++++++---------
 4 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index 38cd6a448f..ae8e3de980 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -69,10 +69,10 @@ module ProductImport
     end
 
     def reset_absent
-      @reset_absent ||= ResetAbsent.new(self, settings, strategy)
+      @reset_absent ||= ResetAbsent.new(self, settings, reset_stock_strategy)
     end
 
-    def strategy_factory
+    def reset_stock_strategy_factory
       if settings.importing_into_inventory?
         InventoryReset
       else
@@ -80,8 +80,9 @@ module ProductImport
       end
     end
 
-    def strategy
-      @strategy ||= strategy_factory.new(settings.updated_ids)
+    def reset_stock_strategy
+      @reset_stock_strategy ||= reset_stock_strategy_factory
+        .new(settings.updated_ids)
     end
 
     def total_saved_count
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 854b19cc08..69bdc37877 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -2,10 +2,10 @@ module ProductImport
   class ResetAbsent
     attr_reader :products_reset_count
 
-    def initialize(entry_processor, settings, strategy)
+    def initialize(entry_processor, settings, reset_stock_strategy)
       @entry_processor = entry_processor
       @settings = settings
-      @strategy = strategy
+      @reset_stock_strategy = reset_stock_strategy
 
       @products_reset_count = 0
     end
@@ -16,18 +16,18 @@ module ProductImport
       settings.enterprises_to_reset.each do |enterprise_id|
         next unless entry_processor.permission_by_id?(enterprise_id)
 
-        strategy << enterprise_id.to_i
+        reset_stock_strategy << enterprise_id.to_i
       end
 
-      reset_stock if strategy.supplier_ids.present?
+      reset_stock if reset_stock_strategy.supplier_ids.present?
     end
 
     private
 
-    attr_reader :settings, :strategy, :entry_processor
+    attr_reader :settings, :reset_stock_strategy, :entry_processor
 
     def reset_stock
-      @products_reset_count = strategy.reset
+      @products_reset_count = reset_stock_strategy.reset
     end
   end
 end
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index 1f2e7eda62..1fbf9198d4 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -70,41 +70,46 @@ describe ProductImport::EntryProcessor do
           updated_ids: [1]
         )
       end
+
       context 'when importing into inventory' do
-        let(:strategy) { instance_double(ProductImport::InventoryReset) }
+        let(:reset_stock_strategy) do
+          instance_double(ProductImport::InventoryReset)
+        end
 
         before do
           allow(settings).to receive(:importing_into_inventory?) { true }
 
           allow(ProductImport::InventoryReset)
-            .to receive(:new).with([1]) { strategy }
+            .to receive(:new).with([1]) { reset_stock_strategy }
         end
 
-        it 'delegates to ResetAbsent passing the appropriate strategy' do
+        it 'delegates to ResetAbsent passing the appropriate reset_stock_strategy' do
           entry_processor.reset_absent_items
 
           expect(ProductImport::ResetAbsent)
             .to have_received(:new)
-            .with(entry_processor, settings, strategy)
+            .with(entry_processor, settings, reset_stock_strategy)
         end
       end
 
       context 'when not importing into inventory' do
-        let(:strategy) { instance_double(ProductImport::ProductsReset) }
+        let(:reset_stock_strategy) do
+          instance_double(ProductImport::ProductsReset)
+        end
 
         before do
           allow(settings).to receive(:importing_into_inventory?) { false }
 
           allow(ProductImport::ProductsReset)
-            .to receive(:new).with([1]) { strategy }
+            .to receive(:new).with([1]) { reset_stock_strategy }
         end
 
-        it 'delegates to ResetAbsent passing the appropriate strategy' do
+        it 'delegates to ResetAbsent passing the appropriate reset_stock_strategy' do
           entry_processor.reset_absent_items
 
           expect(ProductImport::ResetAbsent)
             .to have_received(:new)
-            .with(entry_processor, settings, strategy)
+            .with(entry_processor, settings, reset_stock_strategy)
         end
       end
     end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index e10a3a025b..4b04a4e052 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -5,7 +5,7 @@ module ProductImport
     let(:entry_processor) { instance_double(EntryProcessor) }
 
     let(:reset_absent) do
-      described_class.new(entry_processor, settings, strategy)
+      described_class.new(entry_processor, settings, reset_stock_strategy)
     end
 
     describe '#call' do
@@ -17,7 +17,7 @@ module ProductImport
           )
         end
 
-        let(:strategy) do
+        let(:reset_stock_strategy) do
           instance_double(InventoryReset, supplier_ids: [])
         end
 
@@ -36,15 +36,16 @@ module ProductImport
           )
         end
 
-        let(:strategy) { instance_double(ProductsReset) }
+        let(:reset_stock_strategy) { instance_double(ProductsReset) }
 
         before do
           allow(entry_processor)
             .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
 
-          allow(strategy).to receive(:<<).with(enterprise.id)
-          allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
-          allow(strategy).to receive(:reset) { 2 }
+          allow(reset_stock_strategy).to receive(:<<).with(enterprise.id)
+          allow(reset_stock_strategy)
+            .to receive(:supplier_ids) { [enterprise.id] }
+          allow(reset_stock_strategy).to receive(:reset) { 2 }
         end
 
         it 'returns the number of products reset' do
@@ -52,7 +53,7 @@ module ProductImport
         end
 
         it 'resets the products of the specified suppliers' do
-          expect(strategy).to receive(:reset) { 2 }
+          expect(reset_stock_strategy).to receive(:reset) { 2 }
           reset_absent.call
         end
       end
@@ -67,17 +68,17 @@ module ProductImport
           )
         end
 
-        let(:strategy) { instance_double(InventoryReset) }
+        let(:reset_stock_strategy) { instance_double(InventoryReset) }
 
         before do
           allow(entry_processor)
             .to receive(:permission_by_id?).with(enterprise.id.to_s) { false }
 
-          allow(strategy).to receive(:supplier_ids) { [] }
+          allow(reset_stock_strategy).to receive(:supplier_ids) { [] }
         end
 
         it 'does not reset stock' do
-          expect(strategy).not_to receive(:reset)
+          expect(reset_stock_strategy).not_to receive(:reset)
           reset_absent.call
         end
       end
@@ -93,15 +94,16 @@ module ProductImport
         )
       end
 
-      let(:strategy) { instance_double(InventoryReset) }
+      let(:reset_stock_strategy) { instance_double(InventoryReset) }
 
       before do
         allow(entry_processor)
           .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
 
-        allow(strategy).to receive(:<<).with(enterprise.id)
-        allow(strategy).to receive(:supplier_ids) { [enterprise.id] }
-        allow(strategy).to receive(:reset) { 1 }
+        allow(reset_stock_strategy).to receive(:<<).with(enterprise.id)
+        allow(reset_stock_strategy)
+          .to receive(:supplier_ids) { [enterprise.id] }
+        allow(reset_stock_strategy).to receive(:reset) { 1 }
       end
 
       it 'returns the number of reset variant overrides' do

From 148321f7b73ccc62efce1f23d9653725b50e1ef2 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 1 Oct 2018 16:00:42 +0200
Subject: [PATCH 26/28] Make strategies class names more explicit

---
 app/models/product_import/entry_processor.rb              | 4 ++--
 .../{inventory_reset.rb => inventory_reset_strategy.rb}   | 2 +-
 .../{products_reset.rb => products_reset_strategy.rb}     | 2 +-
 spec/models/product_import/entry_processor_spec.rb        | 8 ++++----
 ...ory_reset_spec.rb => inventory_reset_strategy_spec.rb} | 2 +-
 ...ucts_reset_spec.rb => products_reset_strategy_spec.rb} | 2 +-
 spec/models/product_import/reset_absent_spec.rb           | 8 ++++----
 7 files changed, 14 insertions(+), 14 deletions(-)
 rename app/models/product_import/{inventory_reset.rb => inventory_reset_strategy.rb} (94%)
 rename app/models/product_import/{products_reset.rb => products_reset_strategy.rb} (95%)
 rename spec/models/product_import/{inventory_reset_spec.rb => inventory_reset_strategy_spec.rb} (97%)
 rename spec/models/product_import/{products_reset_spec.rb => products_reset_strategy_spec.rb} (97%)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index ae8e3de980..e8ed57339b 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -74,9 +74,9 @@ module ProductImport
 
     def reset_stock_strategy_factory
       if settings.importing_into_inventory?
-        InventoryReset
+        InventoryResetStrategy
       else
-        ProductsReset
+        ProductsResetStrategy
       end
     end
 
diff --git a/app/models/product_import/inventory_reset.rb b/app/models/product_import/inventory_reset_strategy.rb
similarity index 94%
rename from app/models/product_import/inventory_reset.rb
rename to app/models/product_import/inventory_reset_strategy.rb
index 9aec385da4..f730b22376 100644
--- a/app/models/product_import/inventory_reset.rb
+++ b/app/models/product_import/inventory_reset_strategy.rb
@@ -1,5 +1,5 @@
 module ProductImport
-  class InventoryReset
+  class InventoryResetStrategy
     attr_reader :supplier_ids
 
     def initialize(excluded_items_ids)
diff --git a/app/models/product_import/products_reset.rb b/app/models/product_import/products_reset_strategy.rb
similarity index 95%
rename from app/models/product_import/products_reset.rb
rename to app/models/product_import/products_reset_strategy.rb
index ad15283283..369ff07ac0 100644
--- a/app/models/product_import/products_reset.rb
+++ b/app/models/product_import/products_reset_strategy.rb
@@ -1,5 +1,5 @@
 module ProductImport
-  class ProductsReset
+  class ProductsResetStrategy
     attr_reader :supplier_ids
 
     def initialize(excluded_items_ids)
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index 1fbf9198d4..48c8a8054d 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -73,13 +73,13 @@ describe ProductImport::EntryProcessor do
 
       context 'when importing into inventory' do
         let(:reset_stock_strategy) do
-          instance_double(ProductImport::InventoryReset)
+          instance_double(ProductImport::InventoryResetStrategy)
         end
 
         before do
           allow(settings).to receive(:importing_into_inventory?) { true }
 
-          allow(ProductImport::InventoryReset)
+          allow(ProductImport::InventoryResetStrategy)
             .to receive(:new).with([1]) { reset_stock_strategy }
         end
 
@@ -94,13 +94,13 @@ describe ProductImport::EntryProcessor do
 
       context 'when not importing into inventory' do
         let(:reset_stock_strategy) do
-          instance_double(ProductImport::ProductsReset)
+          instance_double(ProductImport::ProductsResetStrategy)
         end
 
         before do
           allow(settings).to receive(:importing_into_inventory?) { false }
 
-          allow(ProductImport::ProductsReset)
+          allow(ProductImport::ProductsResetStrategy)
             .to receive(:new).with([1]) { reset_stock_strategy }
         end
 
diff --git a/spec/models/product_import/inventory_reset_spec.rb b/spec/models/product_import/inventory_reset_strategy_spec.rb
similarity index 97%
rename from spec/models/product_import/inventory_reset_spec.rb
rename to spec/models/product_import/inventory_reset_strategy_spec.rb
index 412739ae10..7a86390871 100644
--- a/spec/models/product_import/inventory_reset_spec.rb
+++ b/spec/models/product_import/inventory_reset_strategy_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe ProductImport::InventoryReset do
+describe ProductImport::InventoryResetStrategy do
   let(:inventory_reset) { described_class.new(excluded_items_ids) }
 
   describe '#<<' do
diff --git a/spec/models/product_import/products_reset_spec.rb b/spec/models/product_import/products_reset_strategy_spec.rb
similarity index 97%
rename from spec/models/product_import/products_reset_spec.rb
rename to spec/models/product_import/products_reset_strategy_spec.rb
index f84820ad81..47c8fb393d 100644
--- a/spec/models/product_import/products_reset_spec.rb
+++ b/spec/models/product_import/products_reset_strategy_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe ProductImport::ProductsReset do
+describe ProductImport::ProductsResetStrategy do
   let(:products_reset) { described_class.new(excluded_items_ids) }
 
   describe '#<<' do
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 4b04a4e052..14fab6de2c 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -18,7 +18,7 @@ module ProductImport
         end
 
         let(:reset_stock_strategy) do
-          instance_double(InventoryReset, supplier_ids: [])
+          instance_double(InventoryResetStrategy, supplier_ids: [])
         end
 
         it 'returns nil' do
@@ -36,7 +36,7 @@ module ProductImport
           )
         end
 
-        let(:reset_stock_strategy) { instance_double(ProductsReset) }
+        let(:reset_stock_strategy) { instance_double(ProductsResetStrategy) }
 
         before do
           allow(entry_processor)
@@ -68,7 +68,7 @@ module ProductImport
           )
         end
 
-        let(:reset_stock_strategy) { instance_double(InventoryReset) }
+        let(:reset_stock_strategy) { instance_double(InventoryResetStrategy) }
 
         before do
           allow(entry_processor)
@@ -94,7 +94,7 @@ module ProductImport
         )
       end
 
-      let(:reset_stock_strategy) { instance_double(InventoryReset) }
+      let(:reset_stock_strategy) { instance_double(InventoryResetStrategy) }
 
       before do
         allow(entry_processor)

From 82654cd1ee5d151e6d0dcf5ca845b28bd38adde6 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 1 Oct 2018 16:33:46 +0200
Subject: [PATCH 27/28] Turn delegation to a reader to make it simpler

This makes the solution less smart and as a result ResetAbsent improves
it's consistency and returns always an integer.
---
 app/models/product_import/entry_processor.rb  |  7 ++--
 .../inventory_reset_strategy.rb               |  6 +++-
 .../product_import/products_reset_strategy.rb |  6 +++-
 app/models/product_import/reset_absent.rb     | 14 ++++----
 .../product_import/entry_processor_spec.rb    | 28 ++++++++++-----
 .../product_import/reset_absent_spec.rb       | 34 +++----------------
 6 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb
index e8ed57339b..22bae53907 100644
--- a/app/models/product_import/entry_processor.rb
+++ b/app/models/product_import/entry_processor.rb
@@ -4,11 +4,9 @@
 
 module ProductImport
   class EntryProcessor
-    delegate :products_reset_count, to: :reset_absent
-
     attr_reader :inventory_created, :inventory_updated, :products_created,
                 :variants_created, :variants_updated, :supplier_products,
-                :total_supplier_products
+                :total_supplier_products, :products_reset_count
 
     def initialize(importer, validator, import_settings, spreadsheet_data, editable_enterprises, import_time, updated_ids)
       @importer = importer
@@ -24,6 +22,7 @@ module ProductImport
       @products_created = 0
       @variants_created = 0
       @variants_updated = 0
+      @products_reset_count = 0
       @supplier_products = {}
       @total_supplier_products = 0
     end
@@ -65,7 +64,7 @@ module ProductImport
     def reset_absent_items
       return unless settings.data_for_stock_reset? && settings.reset_all_absent?
 
-      reset_absent.call
+      @products_reset_count = reset_absent.call
     end
 
     def reset_absent
diff --git a/app/models/product_import/inventory_reset_strategy.rb b/app/models/product_import/inventory_reset_strategy.rb
index f730b22376..d89891e19f 100644
--- a/app/models/product_import/inventory_reset_strategy.rb
+++ b/app/models/product_import/inventory_reset_strategy.rb
@@ -12,7 +12,11 @@ module ProductImport
     end
 
     def reset
-      relation.update_all(count_on_hand: 0)
+      if supplier_ids.present?
+        relation.update_all(count_on_hand: 0)
+      else
+        0
+      end
     end
 
     private
diff --git a/app/models/product_import/products_reset_strategy.rb b/app/models/product_import/products_reset_strategy.rb
index 369ff07ac0..bdb6fd7d5e 100644
--- a/app/models/product_import/products_reset_strategy.rb
+++ b/app/models/product_import/products_reset_strategy.rb
@@ -12,7 +12,11 @@ module ProductImport
     end
 
     def reset
-      relation.update_all(count_on_hand: 0)
+      if supplier_ids.present?
+        relation.update_all(count_on_hand: 0)
+      else
+        0
+      end
     end
 
     private
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 69bdc37877..5e8d7d6bb3 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -1,17 +1,15 @@
 module ProductImport
   class ResetAbsent
-    attr_reader :products_reset_count
-
     def initialize(entry_processor, settings, reset_stock_strategy)
       @entry_processor = entry_processor
       @settings = settings
       @reset_stock_strategy = reset_stock_strategy
-
-      @products_reset_count = 0
     end
 
     # For selected enterprises; set stock to zero for all products/inventory
     # that were not listed in the newly uploaded spreadsheet
+    #
+    # @return [Integer] number of items affected by the reset
     def call
       settings.enterprises_to_reset.each do |enterprise_id|
         next unless entry_processor.permission_by_id?(enterprise_id)
@@ -19,7 +17,7 @@ module ProductImport
         reset_stock_strategy << enterprise_id.to_i
       end
 
-      reset_stock if reset_stock_strategy.supplier_ids.present?
+      reset_stock
     end
 
     private
@@ -27,7 +25,11 @@ module ProductImport
     attr_reader :settings, :reset_stock_strategy, :entry_processor
 
     def reset_stock
-      @products_reset_count = reset_stock_strategy.reset
+      if reset_stock_strategy.supplier_ids.present?
+        reset_stock_strategy.reset
+      else
+        0
+      end
     end
   end
 end
diff --git a/spec/models/product_import/entry_processor_spec.rb b/spec/models/product_import/entry_processor_spec.rb
index 48c8a8054d..ab854c5ec4 100644
--- a/spec/models/product_import/entry_processor_spec.rb
+++ b/spec/models/product_import/entry_processor_spec.rb
@@ -116,26 +116,36 @@ describe ProductImport::EntryProcessor do
   end
 
   describe '#products_reset_count' do
-    let(:reset_absent) { instance_double(ProductImport::ResetAbsent) }
     let(:settings) do
       instance_double(
         ProductImport::Settings,
+        data_for_stock_reset?: true,
+        reset_all_absent?: true,
         importing_into_inventory?: false,
         updated_ids: [1]
       )
     end
 
-    before do
-      allow(ProductImport::Settings).to receive(:new) { settings }
-      allow(ProductImport::ResetAbsent)
-        .to receive(:new).and_return(reset_absent)
+    context 'when reset_absent_items was executed' do
+      let(:reset_absent) do
+        instance_double(ProductImport::ResetAbsent, call: 2)
+      end
 
-      allow(reset_absent).to receive(:products_reset_count)
+      before do
+        allow(ProductImport::Settings).to receive(:new) { settings }
+        allow(ProductImport::ResetAbsent).to receive(:new) { reset_absent }
+      end
+
+      it 'returns the number of items affected by the last reset' do
+        entry_processor.reset_absent_items
+        expect(entry_processor.products_reset_count).to eq(2)
+      end
     end
 
-    it 'delegates to ResetAbsent' do
-      entry_processor.products_reset_count
-      expect(reset_absent).to have_received(:products_reset_count)
+    context 'when ResetAbsent was not called' do
+      it 'returns 0' do
+        expect(entry_processor.products_reset_count).to eq(0)
+      end
     end
   end
 end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 14fab6de2c..07ef02ae45 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -21,8 +21,8 @@ module ProductImport
           instance_double(InventoryResetStrategy, supplier_ids: [])
         end
 
-        it 'returns nil' do
-          expect(reset_absent.call).to be_nil
+        it 'returns 0' do
+          expect(reset_absent.call).to eq(0)
         end
       end
 
@@ -81,34 +81,10 @@ module ProductImport
           expect(reset_stock_strategy).not_to receive(:reset)
           reset_absent.call
         end
-      end
-    end
 
-    describe '#products_reset_count' do
-      let(:enterprise) { instance_double(Enterprise, id: 1) }
-
-      let(:settings) do
-        instance_double(
-          Settings,
-          enterprises_to_reset: [enterprise.id.to_s]
-        )
-      end
-
-      let(:reset_stock_strategy) { instance_double(InventoryResetStrategy) }
-
-      before do
-        allow(entry_processor)
-          .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
-
-        allow(reset_stock_strategy).to receive(:<<).with(enterprise.id)
-        allow(reset_stock_strategy)
-          .to receive(:supplier_ids) { [enterprise.id] }
-        allow(reset_stock_strategy).to receive(:reset) { 1 }
-      end
-
-      it 'returns the number of reset variant overrides' do
-        reset_absent.call
-        expect(reset_absent.products_reset_count).to eq(1)
+        it 'returns 0' do
+          expect(reset_absent.call).to eq(0)
+        end
       end
     end
   end

From 6a7359a3c5c1ff2f25d306d3d082f6c4f1351c89 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 1 Oct 2018 17:37:49 +0200
Subject: [PATCH 28/28] Pass supplier_ids in strategy #reset instead

This removes the need to expose supplier_ids through #<< and makes both
ResetAbsent and its strategies simpler.

This could be made even simpler if the strategies just implemented
`#relation` as public method and ResetAbsent called `#update_all` on
them. The data to be fetched is the only thing that changes but the
update is the same.
---
 .../inventory_reset_strategy.rb               |  11 +-
 .../product_import/products_reset_strategy.rb |  11 +-
 app/models/product_import/reset_absent.rb     |  20 +--
 .../inventory_reset_strategy_spec.rb          | 138 ++++++++++++++----
 .../products_reset_strategy_spec.rb           | 136 +++++++++++++----
 .../product_import/reset_absent_spec.rb       |  29 ++--
 6 files changed, 243 insertions(+), 102 deletions(-)

diff --git a/app/models/product_import/inventory_reset_strategy.rb b/app/models/product_import/inventory_reset_strategy.rb
index d89891e19f..c22cb6218a 100644
--- a/app/models/product_import/inventory_reset_strategy.rb
+++ b/app/models/product_import/inventory_reset_strategy.rb
@@ -1,17 +1,12 @@
 module ProductImport
   class InventoryResetStrategy
-    attr_reader :supplier_ids
-
     def initialize(excluded_items_ids)
-      @supplier_ids = []
       @excluded_items_ids = excluded_items_ids
     end
 
-    def <<(values)
-      @supplier_ids << values
-    end
+    def reset(supplier_ids)
+      @supplier_ids = supplier_ids
 
-    def reset
       if supplier_ids.present?
         relation.update_all(count_on_hand: 0)
       else
@@ -21,7 +16,7 @@ module ProductImport
 
     private
 
-    attr_reader :excluded_items_ids
+    attr_reader :excluded_items_ids, :supplier_ids
 
     def relation
       relation = VariantOverride.where(hub_id: supplier_ids)
diff --git a/app/models/product_import/products_reset_strategy.rb b/app/models/product_import/products_reset_strategy.rb
index bdb6fd7d5e..80dd6a448c 100644
--- a/app/models/product_import/products_reset_strategy.rb
+++ b/app/models/product_import/products_reset_strategy.rb
@@ -1,17 +1,12 @@
 module ProductImport
   class ProductsResetStrategy
-    attr_reader :supplier_ids
-
     def initialize(excluded_items_ids)
-      @supplier_ids = []
       @excluded_items_ids = excluded_items_ids
     end
 
-    def <<(values)
-      @supplier_ids << values
-    end
+    def reset(supplier_ids)
+      @supplier_ids = supplier_ids
 
-    def reset
       if supplier_ids.present?
         relation.update_all(count_on_hand: 0)
       else
@@ -21,7 +16,7 @@ module ProductImport
 
     private
 
-    attr_reader :excluded_items_ids
+    attr_reader :excluded_items_ids, :supplier_ids
 
     def relation
       relation = Spree::Variant
diff --git a/app/models/product_import/reset_absent.rb b/app/models/product_import/reset_absent.rb
index 5e8d7d6bb3..549ea1e7de 100644
--- a/app/models/product_import/reset_absent.rb
+++ b/app/models/product_import/reset_absent.rb
@@ -11,24 +11,20 @@ module ProductImport
     #
     # @return [Integer] number of items affected by the reset
     def call
-      settings.enterprises_to_reset.each do |enterprise_id|
-        next unless entry_processor.permission_by_id?(enterprise_id)
-
-        reset_stock_strategy << enterprise_id.to_i
-      end
-
-      reset_stock
+      reset_stock_strategy.reset(authorized_enterprises)
     end
 
     private
 
     attr_reader :settings, :reset_stock_strategy, :entry_processor
 
-    def reset_stock
-      if reset_stock_strategy.supplier_ids.present?
-        reset_stock_strategy.reset
-      else
-        0
+    # Returns the enterprises that have permissions to be reset
+    #
+    # @return [Array<Integer>] array of Enterprise ids
+    def authorized_enterprises
+      settings.enterprises_to_reset.map do |enterprise_id|
+        next unless entry_processor.permission_by_id?(enterprise_id)
+        enterprise_id.to_i
       end
     end
   end
diff --git a/spec/models/product_import/inventory_reset_strategy_spec.rb b/spec/models/product_import/inventory_reset_strategy_spec.rb
index 7a86390871..011fe9f304 100644
--- a/spec/models/product_import/inventory_reset_strategy_spec.rb
+++ b/spec/models/product_import/inventory_reset_strategy_spec.rb
@@ -3,16 +3,6 @@ require 'spec_helper'
 describe ProductImport::InventoryResetStrategy do
   let(:inventory_reset) { described_class.new(excluded_items_ids) }
 
-  describe '#<<' do
-    let(:excluded_items_ids) { [] }
-    let(:supplier_ids) { 1 }
-
-    it 'stores the specified supplier_ids' do
-      inventory_reset << supplier_ids
-      expect(inventory_reset.supplier_ids).to eq([supplier_ids])
-    end
-  end
-
   describe '#reset' do
     let(:supplier_ids) { enterprise.id }
     let(:enterprise) { variant.product.supplier }
@@ -27,43 +17,131 @@ describe ProductImport::InventoryResetStrategy do
       )
     end
 
-    before { inventory_reset << supplier_ids }
-
     context 'when there are excluded_items_ids' do
       let(:excluded_items_ids) { [variant_override.id] }
 
-      it 'does not update the count_on_hand of the excluded items' do
-        inventory_reset.reset
-        expect(variant_override.reload.count_on_hand).to eq(10)
+      context 'and supplier_ids is []' do
+        let(:supplier_ids) { [] }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          inventory_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
       end
 
-      it 'updates the count_on_hand of the non-excluded items' do
-        non_excluded_variant_override = create(
-          :variant_override,
-          count_on_hand: 3,
-          hub: enterprise,
-          variant: variant
-        )
-        inventory_reset.reset
-        expect(non_excluded_variant_override.reload.count_on_hand).to eq(0)
+      context 'and supplier_ids is nil' do
+        let(:supplier_ids) { nil }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          inventory_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is set' do
+        it 'does not update the count_on_hand of the excluded items' do
+          inventory_reset.reset(supplier_ids)
+          expect(variant_override.reload.count_on_hand).to eq(10)
+        end
+
+        it 'updates the count_on_hand of the non-excluded items' do
+          non_excluded_variant_override = create(
+            :variant_override,
+            count_on_hand: 3,
+            hub: enterprise,
+            variant: variant
+          )
+          inventory_reset.reset(supplier_ids)
+          expect(non_excluded_variant_override.reload.count_on_hand).to eq(0)
+        end
       end
     end
 
     context 'when there are no excluded_items_ids' do
       let(:excluded_items_ids) { [] }
 
-      it 'sets all count_on_hand to 0' do
-        inventory_reset.reset
-        expect(variant_override.reload.count_on_hand).to eq(0)
+      context 'and supplier_ids is []' do
+        let(:supplier_ids) { [] }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          inventory_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is nil' do
+        let(:supplier_ids) { nil }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          inventory_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is set' do
+        it 'sets all count_on_hand to 0' do
+          inventory_reset.reset(supplier_ids)
+          expect(variant_override.reload.count_on_hand).to eq(0)
+        end
       end
     end
 
     context 'when excluded_items_ids is nil' do
       let(:excluded_items_ids) { nil }
 
-      it 'sets all count_on_hand to 0' do
-        inventory_reset.reset
-        expect(variant_override.reload.count_on_hand).to eq(0)
+      context 'and supplier_ids is []' do
+        let(:supplier_ids) { [] }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          inventory_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is nil' do
+        let(:supplier_ids) { nil }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          inventory_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is set' do
+        it 'sets all count_on_hand to 0' do
+          inventory_reset.reset(supplier_ids)
+          expect(variant_override.reload.count_on_hand).to eq(0)
+        end
       end
     end
   end
diff --git a/spec/models/product_import/products_reset_strategy_spec.rb b/spec/models/product_import/products_reset_strategy_spec.rb
index 47c8fb393d..ba5a33c8d1 100644
--- a/spec/models/product_import/products_reset_strategy_spec.rb
+++ b/spec/models/product_import/products_reset_strategy_spec.rb
@@ -3,57 +3,135 @@ require 'spec_helper'
 describe ProductImport::ProductsResetStrategy do
   let(:products_reset) { described_class.new(excluded_items_ids) }
 
-  describe '#<<' do
-    let(:excluded_items_ids) { [] }
-    let(:supplier_ids) { 1 }
-
-    it 'stores the specified supplier_ids' do
-      products_reset << supplier_ids
-      expect(products_reset.supplier_ids).to eq([supplier_ids])
-    end
-  end
-
   describe '#reset' do
     let(:supplier_ids) { enterprise.id }
     let(:enterprise) { variant.product.supplier }
     let(:variant) { create(:variant, count_on_hand: 2) }
 
-    before { products_reset << supplier_ids }
-
     context 'when there are excluded_items_ids' do
       let(:excluded_items_ids) { [variant.id] }
 
-      it 'does not update the count_on_hand of the excluded items' do
-        products_reset.reset
-        expect(variant.reload.count_on_hand).to eq(2)
+      context 'and supplier_ids is []' do
+        let(:supplier_ids) { [] }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          products_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
       end
 
-      it 'updates the count_on_hand of the non-excluded items' do
-        non_excluded_variant = create(
-          :variant,
-          count_on_hand: 3,
-          product: variant.product
-        )
-        products_reset.reset
-        expect(non_excluded_variant.reload.count_on_hand).to eq(0)
+      context 'and supplier_ids is nil' do
+        let(:supplier_ids) { nil }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          products_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is set' do
+        it 'does not update the count_on_hand of the excluded items' do
+          products_reset.reset(supplier_ids)
+          expect(variant.reload.count_on_hand).to eq(2)
+        end
+
+        it 'updates the count_on_hand of the non-excluded items' do
+          non_excluded_variant = create(
+            :variant,
+            count_on_hand: 3,
+            product: variant.product
+          )
+          products_reset.reset(supplier_ids)
+          expect(non_excluded_variant.reload.count_on_hand).to eq(0)
+        end
       end
     end
 
     context 'when there are no excluded_items_ids' do
       let(:excluded_items_ids) { [] }
 
-      it 'sets all count_on_hand to 0' do
-        products_reset.reset
-        expect(variant.reload.count_on_hand).to eq(0)
+      context 'and supplier_ids is []' do
+        let(:supplier_ids) { [] }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          products_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is nil' do
+        let(:supplier_ids) { nil }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          products_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is nil' do
+        it 'sets all count_on_hand to 0' do
+          products_reset.reset(supplier_ids)
+          expect(variant.reload.count_on_hand).to eq(0)
+        end
       end
     end
 
     context 'when excluded_items_ids is nil' do
       let(:excluded_items_ids) { nil }
 
-      it 'sets all count_on_hand to 0' do
-        products_reset.reset
-        expect(variant.reload.count_on_hand).to eq(0)
+      context 'and supplier_ids is []' do
+        let(:supplier_ids) { [] }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          products_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is nil' do
+        let(:supplier_ids) { nil }
+        let(:relation) do
+          instance_double(ActiveRecord::Relation, update_all: true)
+        end
+
+        before { allow(VariantOverride).to receive(:where) { relation } }
+
+        it 'does not update any DB record' do
+          products_reset.reset(supplier_ids)
+          expect(relation).not_to have_received(:update_all)
+        end
+      end
+
+      context 'and supplier_ids is nil' do
+        it 'sets all count_on_hand to 0' do
+          products_reset.reset(supplier_ids)
+          expect(variant.reload.count_on_hand).to eq(0)
+        end
       end
     end
   end
diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb
index 07ef02ae45..d642c67338 100644
--- a/spec/models/product_import/reset_absent_spec.rb
+++ b/spec/models/product_import/reset_absent_spec.rb
@@ -10,20 +10,21 @@ module ProductImport
 
     describe '#call' do
       context 'when there are no enterprises_to_reset' do
-        let(:settings) do
-          instance_double(
-            Settings,
-            enterprises_to_reset: []
-          )
-        end
+        let(:settings) { instance_double(Settings, enterprises_to_reset: []) }
+        let(:reset_stock_strategy) { instance_double(InventoryResetStrategy) }
 
-        let(:reset_stock_strategy) do
-          instance_double(InventoryResetStrategy, supplier_ids: [])
+        before do
+          allow(reset_stock_strategy).to receive(:reset).with([]) { 0 }
         end
 
         it 'returns 0' do
           expect(reset_absent.call).to eq(0)
         end
+
+        it 'calls the strategy' do
+          reset_absent.call
+          expect(reset_stock_strategy).to have_received(:reset)
+        end
       end
 
       context 'when there are enterprises_to_reset' do
@@ -42,10 +43,8 @@ module ProductImport
           allow(entry_processor)
             .to receive(:permission_by_id?).with(enterprise.id.to_s) { true }
 
-          allow(reset_stock_strategy).to receive(:<<).with(enterprise.id)
           allow(reset_stock_strategy)
-            .to receive(:supplier_ids) { [enterprise.id] }
-          allow(reset_stock_strategy).to receive(:reset) { 2 }
+            .to receive(:reset).with([enterprise.id]) { 2 }
         end
 
         it 'returns the number of products reset' do
@@ -53,8 +52,8 @@ module ProductImport
         end
 
         it 'resets the products of the specified suppliers' do
-          expect(reset_stock_strategy).to receive(:reset) { 2 }
           reset_absent.call
+          expect(reset_stock_strategy).to have_received(:reset)
         end
       end
 
@@ -74,12 +73,12 @@ module ProductImport
           allow(entry_processor)
             .to receive(:permission_by_id?).with(enterprise.id.to_s) { false }
 
-          allow(reset_stock_strategy).to receive(:supplier_ids) { [] }
+          allow(reset_stock_strategy).to receive(:reset).with([nil]) { 0 }
         end
 
-        it 'does not reset stock' do
-          expect(reset_stock_strategy).not_to receive(:reset)
+        it 'calls the strategy' do
           reset_absent.call
+          expect(reset_stock_strategy).to have_received(:reset)
         end
 
         it 'returns 0' do