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] 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