From 04248e70e9323fbcffff00b477dcba6981b40fc9 Mon Sep 17 00:00:00 2001 From: sawan gupta Date: Sun, 28 Feb 2016 01:08:54 +0530 Subject: [PATCH 1/4] complete specs for stock movement model --- core/app/models/spree/stock_movement.rb | 10 +- core/spec/models/spree/stock_movement_spec.rb | 139 +++++++++++++----- 2 files changed, 110 insertions(+), 39 deletions(-) diff --git a/core/app/models/spree/stock_movement.rb b/core/app/models/spree/stock_movement.rb index baac5229050..553735f6a74 100644 --- a/core/app/models/spree/stock_movement.rb +++ b/core/app/models/spree/stock_movement.rb @@ -1,5 +1,9 @@ module Spree class StockMovement < Spree::Base + + MAX_QUANTITY = 2147483647 + MIN_QUANTITY = -2147483648 + belongs_to :stock_item, class_name: 'Spree::StockItem', inverse_of: :stock_movements belongs_to :originator, polymorphic: true @@ -8,8 +12,8 @@ class StockMovement < Spree::Base with_options presence: true do validates :stock_item validates :quantity, numericality: { - greater_than_or_equal_to: -2**31, - less_than_or_equal_to: 2**31 - 1, + greater_than_or_equal_to: MIN_QUANTITY, + less_than_or_equal_to: MAX_QUANTITY, only_integer: true, allow_nil: true } @@ -20,7 +24,7 @@ class StockMovement < Spree::Base self.whitelisted_ransackable_attributes = ['quantity'] def readonly? - !new_record? + persisted? end private diff --git a/core/spec/models/spree/stock_movement_spec.rb b/core/spec/models/spree/stock_movement_spec.rb index 33a178651b3..238eeb8c6d8 100644 --- a/core/spec/models/spree/stock_movement_spec.rb +++ b/core/spec/models/spree/stock_movement_spec.rb @@ -1,55 +1,122 @@ require 'spec_helper' describe Spree::StockMovement, :type => :model do - let(:stock_location) { create(:stock_location_with_items) } - let(:stock_item) { stock_location.stock_items.order(:id).first } - subject { build(:stock_movement, stock_item: stock_item) } - it 'should belong to a stock item' do - expect(subject).to respond_to(:stock_item) + describe 'Constants' do + + describe 'MAX_QUANTITY' do + it 'return 2147483647' do + expect(Spree::StockMovement::MAX_QUANTITY).to eq(2147483647) + end + end + + describe 'MIN_QUANTITY' do + it 'return -2147483648' do + expect(Spree::StockMovement::MIN_QUANTITY).to eq(-2147483648) + end + end + end - it 'is readonly unless new' do - subject.save - expect { - subject.save - }.to raise_error(ActiveRecord::ReadOnlyRecord) + describe 'Associations' do + + it { should belong_to(:stock_item).class_name('Spree::StockItem').inverse_of(:stock_movements) } + it { should belong_to(:originator) } + end - it 'does not update count on hand when track inventory levels is false' do - Spree::Config[:track_inventory_levels] = false - subject.quantity = 1 - subject.save - stock_item.reload - expect(stock_item.count_on_hand).to eq(10) + describe 'Validations' do + + it { should validate_presence_of(:stock_item) } + it { should validate_presence_of(:quantity) } + it do should validate_numericality_of(:quantity). + is_greater_than_or_equal_to(Spree::StockMovement::MIN_QUANTITY). + is_less_than_or_equal_to(Spree::StockMovement::MAX_QUANTITY). + only_integer.allow_nil + end end - it 'does not update count on hand when variant inventory tracking is off' do - stock_item.variant.track_inventory = false - subject.quantity = 1 - subject.save - stock_item.reload - expect(stock_item.count_on_hand).to eq(10) + describe 'Callbacks' do + it { is_expected.to callback(:update_stock_item_quantity).after(:create) } end - context "when quantity is negative" do - context "after save" do - it "should decrement the stock item count on hand" do - subject.quantity = -1 - subject.save - stock_item.reload - expect(stock_item.count_on_hand).to eq(9) + describe 'Scope' do + describe '.recent' do + it 'should order chronologically by created at' do + expect(Spree::StockMovement.recent.to_sql). + to eq Spree::StockMovement.unscoped.order(created_at: :desc).to_sql end end end - context "when quantity is positive" do - context "after save" do - it "should increment the stock item count on hand" do - subject.quantity = 1 - subject.save - stock_item.reload - expect(stock_item.count_on_hand).to eq(11) + describe 'whitelisted ransackable attributes' do + it 'returns amount attribute' do + expect(Spree::StockMovement.whitelisted_ransackable_attributes).to eq(['quantity']) + end + end + + describe 'Insatance Methods' do + + let(:stock_location) { create(:stock_location_with_items) } + let(:stock_item) { stock_location.stock_items.order(:id).first } + + describe '#readonly?' do + let(:stock_movement) { create(:stock_movement, stock_item: stock_item) } + it 'should not update a persisted records' do + expect { + stock_movement.save + }.to raise_error(ActiveRecord::ReadOnlyRecord) + end + end + + describe '#update_stock_item_quantity' do + + let(:stock_movement) { build(:stock_movement, stock_item: stock_item) } + + context 'when track inventory levels is false' do + before do + Spree::Config[:track_inventory_levels] = false + stock_movement.quantity = 1 + stock_movement.save + stock_item.reload + end + it 'does not update count on hand' do + expect(stock_item.count_on_hand).to eq(10) + end + end + + context 'when track inventory tracking is off' do + before do + stock_item.variant.track_inventory = false + stock_movement.quantity = 1 + stock_movement.save + stock_item.reload + end + it 'does not update count on hand' do + expect(stock_item.count_on_hand).to eq(10) + end + end + + context 'when quantity is negative' do + before do + stock_movement.quantity = -1 + stock_movement.save + stock_item.reload + end + it 'should decrement the stock item count on hand' do + expect(stock_item.count_on_hand).to eq(9) + end + end + + context "when quantity is positive" do + before do + stock_movement.quantity = 1 + stock_movement.save + stock_item.reload + end + it "should increment the stock item count on hand" do + expect(stock_item.count_on_hand).to eq(11) + end end end end From 4bde471c4e24a7f8dc892aa4e73a6629ea38f2ba Mon Sep 17 00:00:00 2001 From: sawan gupta Date: Mon, 29 Feb 2016 12:54:03 +0530 Subject: [PATCH 2/4] update sytax 1) use is_expected syntax for should a matcher 2) change quantity contstants to a single hash --- core/app/models/spree/stock_movement.rb | 10 +++++---- core/spec/models/spree/stock_movement_spec.rb | 21 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/core/app/models/spree/stock_movement.rb b/core/app/models/spree/stock_movement.rb index 553735f6a74..207f4263a9d 100644 --- a/core/app/models/spree/stock_movement.rb +++ b/core/app/models/spree/stock_movement.rb @@ -1,8 +1,10 @@ module Spree class StockMovement < Spree::Base - MAX_QUANTITY = 2147483647 - MIN_QUANTITY = -2147483648 + QUANTITY = { + max: 2147483647, + min: -2147483648 + } belongs_to :stock_item, class_name: 'Spree::StockItem', inverse_of: :stock_movements belongs_to :originator, polymorphic: true @@ -12,8 +14,8 @@ class StockMovement < Spree::Base with_options presence: true do validates :stock_item validates :quantity, numericality: { - greater_than_or_equal_to: MIN_QUANTITY, - less_than_or_equal_to: MAX_QUANTITY, + greater_than_or_equal_to: QUANTITY[:min], + less_than_or_equal_to: QUANTITY[:max], only_integer: true, allow_nil: true } diff --git a/core/spec/models/spree/stock_movement_spec.rb b/core/spec/models/spree/stock_movement_spec.rb index 238eeb8c6d8..2de72b52cde 100644 --- a/core/spec/models/spree/stock_movement_spec.rb +++ b/core/spec/models/spree/stock_movement_spec.rb @@ -6,13 +6,13 @@ describe 'MAX_QUANTITY' do it 'return 2147483647' do - expect(Spree::StockMovement::MAX_QUANTITY).to eq(2147483647) + expect(Spree::StockMovement::QUANTITY[:max]).to eq(2147483647) end end describe 'MIN_QUANTITY' do it 'return -2147483648' do - expect(Spree::StockMovement::MIN_QUANTITY).to eq(-2147483648) + expect(Spree::StockMovement::QUANTITY[:min]).to eq(-2147483648) end end @@ -20,18 +20,19 @@ describe 'Associations' do - it { should belong_to(:stock_item).class_name('Spree::StockItem').inverse_of(:stock_movements) } - it { should belong_to(:originator) } + it { is_expected.to belong_to(:stock_item).class_name('Spree::StockItem').inverse_of(:stock_movements) } + it { is_expected.to belong_to(:originator) } end describe 'Validations' do - it { should validate_presence_of(:stock_item) } - it { should validate_presence_of(:quantity) } - it do should validate_numericality_of(:quantity). - is_greater_than_or_equal_to(Spree::StockMovement::MIN_QUANTITY). - is_less_than_or_equal_to(Spree::StockMovement::MAX_QUANTITY). + it { is_expected.to validate_presence_of(:stock_item) } + it { is_expected.to validate_presence_of(:quantity) } + it do + is_expected.to validate_numericality_of(:quantity). + is_greater_than_or_equal_to(Spree::StockMovement::QUANTITY[:min]). + is_less_than_or_equal_to(Spree::StockMovement::QUANTITY[:max]). only_integer.allow_nil end end @@ -62,7 +63,7 @@ describe '#readonly?' do let(:stock_movement) { create(:stock_movement, stock_item: stock_item) } - it 'should not update a persisted records' do + it 'should not update a persisted record' do expect { stock_movement.save }.to raise_error(ActiveRecord::ReadOnlyRecord) From 86a2a0fefd72a5b4a97730d41dde0f00414141fb Mon Sep 17 00:00:00 2001 From: sawan gupta Date: Thu, 3 Mar 2016 13:53:16 +0530 Subject: [PATCH 3/4] rename QUANTITY to QUANTITY_LIMITS and use 2**31 notation for constant value --- core/app/models/spree/stock_movement.rb | 10 +++---- core/spec/models/spree/stock_movement_spec.rb | 26 ++++++++++++------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/core/app/models/spree/stock_movement.rb b/core/app/models/spree/stock_movement.rb index 207f4263a9d..477a69f5f9e 100644 --- a/core/app/models/spree/stock_movement.rb +++ b/core/app/models/spree/stock_movement.rb @@ -1,9 +1,9 @@ module Spree class StockMovement < Spree::Base - QUANTITY = { - max: 2147483647, - min: -2147483648 + QUANTITY_LIMITS = { + max: 2**31 - 1, + min: -2**31 } belongs_to :stock_item, class_name: 'Spree::StockItem', inverse_of: :stock_movements @@ -14,8 +14,8 @@ class StockMovement < Spree::Base with_options presence: true do validates :stock_item validates :quantity, numericality: { - greater_than_or_equal_to: QUANTITY[:min], - less_than_or_equal_to: QUANTITY[:max], + greater_than_or_equal_to: QUANTITY_LIMITS[:min], + less_than_or_equal_to: QUANTITY_LIMITS[:max], only_integer: true, allow_nil: true } diff --git a/core/spec/models/spree/stock_movement_spec.rb b/core/spec/models/spree/stock_movement_spec.rb index 2de72b52cde..1f37e1b7bc7 100644 --- a/core/spec/models/spree/stock_movement_spec.rb +++ b/core/spec/models/spree/stock_movement_spec.rb @@ -4,15 +4,15 @@ describe 'Constants' do - describe 'MAX_QUANTITY' do - it 'return 2147483647' do - expect(Spree::StockMovement::QUANTITY[:max]).to eq(2147483647) + describe 'QUANTITY_LIMITS[:max]' do + it 'return 2**31 - 1' do + expect(Spree::StockMovement::QUANTITY_LIMITS[:max]).to eq(2**31 - 1) end end - describe 'MIN_QUANTITY' do - it 'return -2147483648' do - expect(Spree::StockMovement::QUANTITY[:min]).to eq(-2147483648) + describe 'QUANTITY_LIMITS[:min]' do + it 'return -2**31' do + expect(Spree::StockMovement::QUANTITY_LIMITS[:min]).to eq(-2**31) end end @@ -27,12 +27,18 @@ describe 'Validations' do - it { is_expected.to validate_presence_of(:stock_item) } - it { is_expected.to validate_presence_of(:quantity) } + it do + is_expected.to validate_presence_of(:stock_item) + end + + it do + is_expected.to validate_presence_of(:quantity) + end + it do is_expected.to validate_numericality_of(:quantity). - is_greater_than_or_equal_to(Spree::StockMovement::QUANTITY[:min]). - is_less_than_or_equal_to(Spree::StockMovement::QUANTITY[:max]). + is_greater_than_or_equal_to(Spree::StockMovement::QUANTITY_LIMITS[:min]). + is_less_than_or_equal_to(Spree::StockMovement::QUANTITY_LIMITS[:max]). only_integer.allow_nil end end From 45652ba5d34a4972217fcbba99f21d4a2e91f6b0 Mon Sep 17 00:00:00 2001 From: sawan gupta Date: Mon, 21 Mar 2016 12:36:48 +0530 Subject: [PATCH 4/4] fix hound ci issues --- core/app/models/spree/stock_movement.rb | 13 ++++++------- core/spec/models/spree/stock_movement_spec.rb | 16 +++------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/core/app/models/spree/stock_movement.rb b/core/app/models/spree/stock_movement.rb index 477a69f5f9e..b2fcb79a581 100644 --- a/core/app/models/spree/stock_movement.rb +++ b/core/app/models/spree/stock_movement.rb @@ -1,10 +1,9 @@ module Spree class StockMovement < Spree::Base - QUANTITY_LIMITS = { max: 2**31 - 1, min: -2**31 - } + }.freeze belongs_to :stock_item, class_name: 'Spree::StockItem', inverse_of: :stock_movements belongs_to :originator, polymorphic: true @@ -14,11 +13,11 @@ class StockMovement < Spree::Base with_options presence: true do validates :stock_item validates :quantity, numericality: { - greater_than_or_equal_to: QUANTITY_LIMITS[:min], - less_than_or_equal_to: QUANTITY_LIMITS[:max], - only_integer: true, - allow_nil: true - } + greater_than_or_equal_to: QUANTITY_LIMITS[:min], + less_than_or_equal_to: QUANTITY_LIMITS[:max], + only_integer: true, + allow_nil: true + } end scope :recent, -> { order(created_at: :desc) } diff --git a/core/spec/models/spree/stock_movement_spec.rb b/core/spec/models/spree/stock_movement_spec.rb index 1f37e1b7bc7..5abc7242f6c 100644 --- a/core/spec/models/spree/stock_movement_spec.rb +++ b/core/spec/models/spree/stock_movement_spec.rb @@ -3,7 +3,6 @@ describe Spree::StockMovement, :type => :model do describe 'Constants' do - describe 'QUANTITY_LIMITS[:max]' do it 'return 2**31 - 1' do expect(Spree::StockMovement::QUANTITY_LIMITS[:max]).to eq(2**31 - 1) @@ -15,18 +14,14 @@ expect(Spree::StockMovement::QUANTITY_LIMITS[:min]).to eq(-2**31) end end - end describe 'Associations' do - it { is_expected.to belong_to(:stock_item).class_name('Spree::StockItem').inverse_of(:stock_movements) } it { is_expected.to belong_to(:originator) } - end describe 'Validations' do - it do is_expected.to validate_presence_of(:stock_item) end @@ -37,9 +32,8 @@ it do is_expected.to validate_numericality_of(:quantity). - is_greater_than_or_equal_to(Spree::StockMovement::QUANTITY_LIMITS[:min]). - is_less_than_or_equal_to(Spree::StockMovement::QUANTITY_LIMITS[:max]). - only_integer.allow_nil + is_greater_than_or_equal_to(Spree::StockMovement::QUANTITY_LIMITS[:min]). + is_less_than_or_equal_to(Spree::StockMovement::QUANTITY_LIMITS[:max]).only_integer.allow_nil end end @@ -63,21 +57,17 @@ end describe 'Insatance Methods' do - let(:stock_location) { create(:stock_location_with_items) } let(:stock_item) { stock_location.stock_items.order(:id).first } describe '#readonly?' do let(:stock_movement) { create(:stock_movement, stock_item: stock_item) } it 'should not update a persisted record' do - expect { - stock_movement.save - }.to raise_error(ActiveRecord::ReadOnlyRecord) + expect { stock_movement.save }.to raise_error(ActiveRecord::ReadOnlyRecord) end end describe '#update_stock_item_quantity' do - let(:stock_movement) { build(:stock_movement, stock_item: stock_item) } context 'when track inventory levels is false' do