From 00e57c8a55fa5f6aabea266162d70aed4a728704 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 12 Feb 2020 11:56:17 +0000 Subject: [PATCH] Add module definition in the recently moved service and adapt all it's usages to refer to the new namespace --- .rubocop_todo.yml | 2 - app/models/product_import/entry_processor.rb | 2 +- .../product_import/products_reset_strategy.rb | 82 +++---- .../products_reset_strategy_spec.rb | 204 +++++++++--------- .../product_import/entry_processor_spec.rb | 4 +- .../product_import/reset_absent_spec.rb | 2 +- 6 files changed, 152 insertions(+), 144 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 33970eb805..894146e628 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -681,7 +681,6 @@ Style/FrozenStringLiteralComment: - 'app/models/product_import/entry_validator.rb' - 'app/models/product_import/inventory_reset_strategy.rb' - 'app/models/product_import/product_importer.rb' - - 'app/models/product_import/products_reset_strategy.rb' - 'app/models/product_import/reset_absent.rb' - 'app/models/product_import/settings.rb' - 'app/models/product_import/spreadsheet_data.rb' @@ -1257,7 +1256,6 @@ Style/FrozenStringLiteralComment: - 'spec/models/producer_property_spec.rb' - 'spec/models/product_import/entry_processor_spec.rb' - 'spec/models/product_import/inventory_reset_strategy_spec.rb' - - 'spec/models/product_import/products_reset_strategy_spec.rb' - 'spec/models/product_import/reset_absent_spec.rb' - 'spec/models/product_import/settings_spec.rb' - 'spec/models/product_importer_spec.rb' diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index eab1ed4cc3..9bc6dc47f4 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -74,7 +74,7 @@ module ProductImport if settings.importing_into_inventory? InventoryResetStrategy else - ProductsResetStrategy + Catalog::ProductImport::ProductsResetStrategy end end diff --git a/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb b/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb index 989f5a275c..c4cf28cdf1 100644 --- a/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb +++ b/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb @@ -1,45 +1,49 @@ -module ProductImport - class ProductsResetStrategy - def initialize(excluded_items_ids) - @excluded_items_ids = excluded_items_ids - end +# frozen_string_literal: true - def reset(enterprise_ids) - @enterprise_ids = enterprise_ids - - return 0 if enterprise_ids.blank? - - reset_variants_on_hand(enterprise_variants_relation) - end - - private - - attr_reader :excluded_items_ids, :enterprise_ids - - def enterprise_variants_relation - relation = Spree::Variant - .joins(:product) - .where( - spree_products: { supplier_id: enterprise_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 - - def reset_variants_on_hand(variants) - updated_records_count = 0 - variants.each do |variant| - updated_records_count += 1 if reset_variant_on_hand(variant) +module Catalog + module ProductImport + class ProductsResetStrategy + def initialize(excluded_items_ids) + @excluded_items_ids = excluded_items_ids end - updated_records_count - end - def reset_variant_on_hand(variant) - variant.on_hand = 0 - variant.on_hand.zero? + def reset(enterprise_ids) + @enterprise_ids = enterprise_ids + + return 0 if enterprise_ids.blank? + + reset_variants_on_hand(enterprise_variants_relation) + end + + private + + attr_reader :excluded_items_ids, :enterprise_ids + + def enterprise_variants_relation + relation = Spree::Variant + .joins(:product) + .where( + spree_products: { supplier_id: enterprise_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 + + def reset_variants_on_hand(variants) + updated_records_count = 0 + variants.each do |variant| + updated_records_count += 1 if reset_variant_on_hand(variant) + end + updated_records_count + end + + def reset_variant_on_hand(variant) + variant.on_hand = 0 + variant.on_hand.zero? + end end end end diff --git a/engines/catalog/spec/services/catalog/product_import/products_reset_strategy_spec.rb b/engines/catalog/spec/services/catalog/product_import/products_reset_strategy_spec.rb index e44249e8a3..a2fd01d01b 100644 --- a/engines/catalog/spec/services/catalog/product_import/products_reset_strategy_spec.rb +++ b/engines/catalog/spec/services/catalog/product_import/products_reset_strategy_spec.rb @@ -1,119 +1,125 @@ +# frozen_string_literal: true + require 'spec_helper' -describe ProductImport::ProductsResetStrategy do - let(:products_reset) { described_class.new(excluded_items_ids) } +module Catalog + module ProductImport + describe ProductsResetStrategy do + let(:products_reset) { described_class.new(excluded_items_ids) } - describe '#reset' do - let(:supplier_ids) { enterprise.id } - let(:product) { create(:product) } - let(:enterprise) { product.supplier } - let(:variant) { product.variants.first } + describe '#reset' do + let(:supplier_ids) { enterprise.id } + let(:product) { create(:product) } + let(:enterprise) { product.supplier } + let(:variant) { product.variants.first } - before { variant.on_hand = 2 } + before { variant.on_hand = 2 } - context 'when there are excluded_items_ids' do - let(:excluded_items_ids) { [variant.id] } + context 'when there are excluded_items_ids' do + let(:excluded_items_ids) { [variant.id] } - context 'and supplier_ids is []' do - let(:supplier_ids) { [] } + context 'and supplier_ids is []' do + let(:supplier_ids) { [] } - it 'does not reset the variant.on_hand' do - products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(2) - end - end - - context 'and supplier_ids is nil' do - let(:supplier_ids) { nil } - - it 'does not reset the variant.on_hand' do - products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(2) - end - end - - context 'and supplier_ids is set' do - it 'does not update the on_hand of the excluded items' do - products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(2) - end - - it 'updates the on_hand of the non-excluded items' do - non_excluded_variant = create( - :variant, - product: variant.product - ) - non_excluded_variant.on_hand = 3 - products_reset.reset(supplier_ids) - expect(non_excluded_variant.reload.on_hand).to eq(0) - end - end - end - - context 'when there are no excluded_items_ids' do - let(:excluded_items_ids) { [] } - - context 'and supplier_ids is []' do - let(:supplier_ids) { [] } - - it 'does not reset the variant.on_hand' do - products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(2) - end - end - - context 'and supplier_ids is nil' do - let(:supplier_ids) { nil } - - it 'does not reset the variant.on_hand' do - products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(2) - end - end - - context 'and supplier_ids is not nil' do - it 'sets all on_hand to 0' do - updated_records_count = products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(0) - expect(updated_records_count).to eq(1) - end - - context 'and there is an unresetable variant' do - before do - variant.stock_items = [] # this makes variant.on_hand raise an error + it 'does not reset the variant.on_hand' do + products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(2) + end end - it 'returns correct number of resetted variants' do - expect { products_reset.reset(supplier_ids) }.to raise_error RuntimeError + context 'and supplier_ids is nil' do + let(:supplier_ids) { nil } + + it 'does not reset the variant.on_hand' do + products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(2) + end + end + + context 'and supplier_ids is set' do + it 'does not update the on_hand of the excluded items' do + products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(2) + end + + it 'updates the on_hand of the non-excluded items' do + non_excluded_variant = create( + :variant, + product: variant.product + ) + non_excluded_variant.on_hand = 3 + products_reset.reset(supplier_ids) + expect(non_excluded_variant.reload.on_hand).to eq(0) + end end end - end - end - context 'when excluded_items_ids is nil' do - let(:excluded_items_ids) { nil } + context 'when there are no excluded_items_ids' do + let(:excluded_items_ids) { [] } - context 'and supplier_ids is []' do - let(:supplier_ids) { [] } + context 'and supplier_ids is []' do + let(:supplier_ids) { [] } - it 'does not reset the variant.on_hand' do - products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(2) + it 'does not reset the variant.on_hand' do + products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(2) + end + end + + context 'and supplier_ids is nil' do + let(:supplier_ids) { nil } + + it 'does not reset the variant.on_hand' do + products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(2) + end + end + + context 'and supplier_ids is not nil' do + it 'sets all on_hand to 0' do + updated_records_count = products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(0) + expect(updated_records_count).to eq(1) + end + + context 'and there is an unresetable variant' do + before do + variant.stock_items = [] # this makes variant.on_hand raise an error + end + + it 'returns correct number of resetted variants' do + expect { products_reset.reset(supplier_ids) }.to raise_error RuntimeError + end + end + end end - end - context 'and supplier_ids is nil' do - let(:supplier_ids) { nil } - it 'does not reset the variant.on_hand' do - products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(2) - end - end + context 'when excluded_items_ids is nil' do + let(:excluded_items_ids) { nil } - context 'and supplier_ids is nil' do - it 'sets all on_hand to 0' do - products_reset.reset(supplier_ids) - expect(variant.reload.on_hand).to eq(0) + context 'and supplier_ids is []' do + let(:supplier_ids) { [] } + + it 'does not reset the variant.on_hand' do + products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(2) + end + end + + context 'and supplier_ids is nil' do + let(:supplier_ids) { nil } + it 'does not reset the variant.on_hand' do + products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(2) + end + end + + context 'and supplier_ids is nil' do + it 'sets all on_hand to 0' do + products_reset.reset(supplier_ids) + expect(variant.reload.on_hand).to eq(0) + end + 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 ab854c5ec4..3f8db18fed 100644 --- a/spec/models/product_import/entry_processor_spec.rb +++ b/spec/models/product_import/entry_processor_spec.rb @@ -94,13 +94,13 @@ describe ProductImport::EntryProcessor do context 'when not importing into inventory' do let(:reset_stock_strategy) do - instance_double(ProductImport::ProductsResetStrategy) + instance_double(Catalog::ProductImport::ProductsResetStrategy) end before do allow(settings).to receive(:importing_into_inventory?) { false } - allow(ProductImport::ProductsResetStrategy) + allow(Catalog::ProductImport::ProductsResetStrategy) .to receive(:new).with([1]) { reset_stock_strategy } end diff --git a/spec/models/product_import/reset_absent_spec.rb b/spec/models/product_import/reset_absent_spec.rb index d642c67338..990f26ff15 100644 --- a/spec/models/product_import/reset_absent_spec.rb +++ b/spec/models/product_import/reset_absent_spec.rb @@ -37,7 +37,7 @@ module ProductImport ) end - let(:reset_stock_strategy) { instance_double(ProductsResetStrategy) } + let(:reset_stock_strategy) { instance_double(Catalog::ProductImport::ProductsResetStrategy) } before do allow(entry_processor)