Merge pull request #1936 from jordan-brough/tax-fix-for-finalized-adj…

…ustments-v2

Tax fix for finalized adjustments v2

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

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.

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

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
 

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
@@ -67,14 +67,33 @@ 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] }
 
           it 'creates an adjustment for every matching rate' do
             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