From 2cc9807be051d652d029620a5fb7379d60bafd4d Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Mon, 22 May 2017 02:01:25 -0600 Subject: [PATCH 1/4] Spacing fix --- core/app/models/spree/tax/item_adjuster.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/app/models/spree/tax/item_adjuster.rb b/core/app/models/spree/tax/item_adjuster.rb index 193fb372947..6581dc306d3 100644 --- a/core/app/models/spree/tax/item_adjuster.rb +++ b/core/app/models/spree/tax/item_adjuster.rb @@ -31,7 +31,7 @@ def adjust! # Find an existing adjustment from the same source. # All tax adjustments already have source_type == 'Spree::TaxRate' so # we need only check source_id. - adjustment = tax_adjustments.detect{|a| a.source_id == rate.id } + adjustment = tax_adjustments.detect { |a| a.source_id == rate.id } if adjustment adjustment.update! adjustment From 64fc9959f3fb75ba08dd32c85c3d70227f863b39 Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Mon, 22 May 2017 03:24:55 -0600 Subject: [PATCH 2/4] Use in-memory objects in specs --- core/spec/models/spree/tax/item_adjuster_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/spec/models/spree/tax/item_adjuster_spec.rb b/core/spec/models/spree/tax/item_adjuster_spec.rb index d1282eae97c..596938c5ca7 100644 --- a/core/spec/models/spree/tax/item_adjuster_spec.rb +++ b/core/spec/models/spree/tax/item_adjuster_spec.rb @@ -6,7 +6,7 @@ let(:item) { create(:line_item, order: order) } def tax_adjustments - item.adjustments.tax.to_a + item.adjustments.select(&:tax?).to_a end describe 'initialization' do From 6506550315d7d9d532b910b3e884eb0a6de92aca Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Mon, 22 May 2017 09:28:12 -0600 Subject: [PATCH 3/4] Ignore adjustment.finalized on Tax adjustments Otherwise taxes don't get updated after an order completes (because all adjustments are finalized when an order completes). --- core/app/models/spree/adjustment.rb | 4 +- core/spec/models/spree/adjustment_spec.rb | 98 ++++++++++++++----- .../models/spree/tax/item_adjuster_spec.rb | 21 +++- 3 files changed, 94 insertions(+), 29 deletions(-) diff --git a/core/app/models/spree/adjustment.rb b/core/app/models/spree/adjustment.rb index 2cbd5ffe49b..bf84b4bfcca 100644 --- a/core/app/models/spree/adjustment.rb +++ b/core/app/models/spree/adjustment.rb @@ -97,7 +97,9 @@ def cancellation? # # @return [BigDecimal] New amount of this adjustment def update! - return amount if finalized? + if finalized? && !tax? + return amount + end # If the adjustment has no source, do not attempt to re-calculate the amount. # Chances are likely that this was a manually created adjustment in the admin backend. diff --git a/core/spec/models/spree/adjustment_spec.rb b/core/spec/models/spree/adjustment_spec.rb index b5489d70e0d..92f97008dd6 100644 --- a/core/spec/models/spree/adjustment_spec.rb +++ b/core/spec/models/spree/adjustment_spec.rb @@ -79,54 +79,98 @@ end context '#update!' do - let(:adjustment) { Spree::Adjustment.create!(label: 'Adjustment', order: order, adjustable: order, amount: 5, finalized: finalized, source: source) } - let(:source) { mock_model(Spree::TaxRate, compute_amount: 10) } - subject { adjustment.update! } + let(:adjustment) do + line_item.adjustments.create!( + label: 'Adjustment', + order: order, + adjustable: order, + amount: 5, + finalized: finalized, + source: source, + ) + end + let(:order) { create(:order_with_line_items, line_items_price: 100) } + let(:line_item) { order.line_items.to_a.first } - context "when adjustment is closed" do + context "when adjustment is finalized" do let(:finalized) { true } - it "does not update the adjustment" do - expect(adjustment).to_not receive(:update_column) - subject + context 'with a promotion adjustment' do + let(:source) { promotion.actions.first! } + let(:promotion) { create(:promotion, :with_line_item_adjustment, adjustment_rate: 7) } + + it 'does not update the adjustment' do + expect { subject }.not_to change { adjustment.amount } + end end - end - context "when adjustment isn't finalized" do - let(:finalized) { false } + context 'with a tax adjustment' do + let(:source) { mock_model(Spree::TaxRate, compute_amount: 10) } - it "updates the amount" do - expect { subject }.to change { adjustment.amount }.to(10) + it 'updates the adjustment' do + expect { subject }.to change { adjustment.amount }.from(5).to(10) + end end - context "it is a promotion adjustment" do - let(:promotion) { create(:promotion, :with_order_adjustment, starts_at: promo_start_date) } - let(:promo_start_date) { nil } - let(:promotion_code) { promotion.codes.first } - let(:order) { create(:order_with_line_items, line_items_count: 1) } + context 'with a sourceless adjustment' do + let(:source) { nil } - let!(:adjustment) do - promotion.activate(order: order, promotion_code: promotion_code) - order.adjustments.first + it 'does nothing' do + expect { subject }.not_to change { adjustment.amount } end + end + end + + context "when adjustment isn't finalized" do + let(:finalized) { false } + + context 'with a promotion adjustment' do + let(:source) { promotion.actions.first! } + let(:promotion) { create(:promotion, :with_line_item_adjustment, adjustment_rate: 7) } - context "the promotion is eligible" do - it "sets the adjustment elgiible to true" do + context 'when the promotion is eligible' do + it 'updates the adjustment' do + expect { subject }.to change { adjustment.amount }.from(5).to(-7) + end + + it 'sets the adjustment elgiible to true' do subject - expect(adjustment.eligible).to eq true + expect(adjustment.eligible).to eq(true) end end - context "the promotion is not eligible" do - let(:promo_start_date) { Date.tomorrow } + context 'when the promotion is not eligible' do + before do + promotion.update!(starts_at: 1.day.from_now) + end - it "sets the adjustment elgiible to false" do + it 'zeros out the adjustment' do + expect { subject }.to change { adjustment.amount }.from(5).to(0) + end + + it 'sets the adjustment elgiible to false' do subject - expect(adjustment.eligible).to eq false + expect(adjustment.eligible).to eq(false) end end end + + context 'with a tax adjustment' do + let(:source) { mock_model(Spree::TaxRate, compute_amount: 10) } + + it 'updates the adjustment' do + expect { subject }.to change { adjustment.amount }.from(5).to(10) + end + end + + context 'with a sourceless adjustment' do + let(:source) { nil } + + it 'does nothing' do + expect { subject }.to_not change { adjustment.amount } + end + end end end diff --git a/core/spec/models/spree/tax/item_adjuster_spec.rb b/core/spec/models/spree/tax/item_adjuster_spec.rb index 596938c5ca7..6429d5c5086 100644 --- a/core/spec/models/spree/tax/item_adjuster_spec.rb +++ b/core/spec/models/spree/tax/item_adjuster_spec.rb @@ -67,7 +67,7 @@ def tax_adjustments context 'and all rates have the same tax category as the item' do let(:item) { create :line_item, order: order, tax_category: item_tax_category } let(:item_tax_category) { create(:tax_category) } - let(:rate_1) { create :tax_rate, tax_category: item_tax_category } + let(:rate_1) { create :tax_rate, tax_category: item_tax_category, amount: 0.1 } let(:rate_2) { create :tax_rate } let(:rates_for_order_zone) { [rate_1, rate_2] } @@ -75,6 +75,25 @@ def tax_adjustments adjuster.adjust! expect(tax_adjustments.length).to eq(1) end + + context 'when the adjustment exists' do + before do + adjuster.adjust! + end + + context 'when the existing adjustment is finalized' do + before do + tax_adjustments.first.finalize! + end + + it 'updates the adjustment' do + item.update_columns(price: item.price * 2) + adjuster.adjust! + expect(tax_adjustments.length).to eq(1) + expect(tax_adjustments.first.amount).to eq(0.1 * item.price) + end + end + end end end end From 22c96e1618425060991254f06a2d9452e5ae72b0 Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Mon, 22 May 2017 09:33:15 -0600 Subject: [PATCH 4/4] Changelog entry for ignoring adjustment.finalized on tax adjustments --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 236a3be3238..0d270a06a81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Solidus 2.3.0 (master, unreleased) +- Ignore `adjustment.finalized` on tax adjustments. [\#1936](https://github.com/solidusio/solidus/pull/1936) ([jordan-brough](https://github.com/jordan-brough)) - Deprecate `#simple_current_order` [\#1915](https://github.com/solidusio/solidus/pull/1915) ([ericsaupe](https://github.com/ericsaupe))