From 82654cd1ee5d151e6d0dcf5ca845b28bd38adde6 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 1 Oct 2018 16:33:46 +0200 Subject: [PATCH] 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