Only destroy tax adjustment for open order when the associated tax ra…

…te is deleted. For closed order just nullify the source_id.

Update line_item & order when adjustments are deleted.

Add rspec tests to cover cases where adjustments get (or not get) destroyed when taxrate/promotion is deleted.

Move deals_with_adjustments to a separate mixin.

Add new mixin to core

Revert previous changes to remove unnecessary tests.

Remove order.update! and simplify rspec.

Fixes #4828
Fixes #4771

Conflicts:
	core/spec/models/spree/tax_rate_spec.rb

core/app/models/spree/promotion/actions/create_adjustment.rb

@@ -3,13 +3,14 @@ class Promotion
     module Actions
       class CreateAdjustment < PromotionAction
         include Spree::Core::CalculatedAdjustments
+        include Spree::Core::AdjustmentSource
 
         has_many :adjustments, as: :source
 
         delegate :eligible?, to: :promotion
 
         before_validation :ensure_action_has_calculator
-        before_destroy :deals_with_adjustments
+        before_destroy :deals_with_adjustments_for_deleted_source
 
         # Creates the adjustment related to a promotion for the order passed
         # through options hash
@@ -55,16 +56,6 @@ def ensure_action_has_calculator
             self.calculator = Calculator::FlatPercentItemTotal.new
           end
 
-          def deals_with_adjustments
-            adjustment_scope = self.adjustments.joins("LEFT OUTER JOIN spree_orders ON spree_orders.id = spree_adjustments.adjustable_id")
-            # For incomplete orders, remove the adjustment completely.
-            adjustment_scope.where("spree_orders.completed_at IS NULL").readonly(false).destroy_all
-
-            # For complete orders, the source will be invalid.
-            # Therefore we nullify the source_id, leaving the adjustment in place.
-            # This would mean that the order's total is not altered at all.
-            adjustment_scope.where("spree_orders.completed_at IS NOT NULL").update_all("source_id = NULL")
-          end
       end
     end
   end

core/app/models/spree/promotion/actions/create_item_adjustments.rb

@@ -3,13 +3,14 @@ class Promotion
     module Actions
       class CreateItemAdjustments < PromotionAction
         include Spree::Core::CalculatedAdjustments
+        include Spree::Core::AdjustmentSource
 
         has_many :adjustments, as: :source
 
         delegate :eligible?, to: :promotion
 
         before_validation :ensure_action_has_calculator
-        before_destroy :deals_with_adjustments
+        before_destroy :deals_with_adjustments_for_deleted_source
 
         def perform(payload = {})
           order = payload[:order]
@@ -62,24 +63,6 @@ def ensure_action_has_calculator
             self.calculator = Calculator::PercentOnLineItem.new
           end
 
-          def deals_with_adjustments
-            adjustment_scope = self.adjustments.includes(:order).references(:spree_orders)
-
-            # For incomplete orders, remove the adjustment completely.
-            adjustment_scope.where("spree_orders.completed_at IS NULL").each do |adjustment|
-              adjustment.destroy
-            end
-
-            # For complete orders, the source will be invalid.
-            # Therefore we nullify the source_id, leaving the adjustment in place.
-            # This would mean that the order's total is not altered at all.
-            adjustment_scope.where("spree_orders.completed_at IS NOT NULL").each do |adjustment|
-              adjustment.update_columns(
-                source_id: nil,
-                updated_at: Time.now,
-              )
-            end
-          end
       end
     end
   end

core/app/models/spree/tax_rate.rb

@@ -12,15 +12,18 @@ module Spree
   class TaxRate < Spree::Base
     acts_as_paranoid
     include Spree::Core::CalculatedAdjustments
+    include Spree::Core::AdjustmentSource
     belongs_to :zone, class_name: "Spree::Zone"
     belongs_to :tax_category, class_name: "Spree::TaxCategory"
 
-    has_many :adjustments, as: :source, dependent: :destroy
+    has_many :adjustments, as: :source
 
     validates :amount, presence: true, numericality: true
     validates :tax_category_id, presence: true
     validates_with DefaultTaxZoneValidator
 
+    before_destroy :deals_with_adjustments_for_deleted_source
+
     scope :by_zone, ->(zone) { where(zone_id: zone) }
 
     # Gets the array of TaxRates appropriate for the specified order
@@ -177,5 +180,6 @@ def create_label
         label << (name.present? ? name : tax_category.name) + " "
         label << (show_rate_in_label? ? "#{amount * 100}%" : "")
       end
+
   end
 end

core/lib/spree/core.rb

@@ -65,6 +65,7 @@ class DestroyWithOrdersError < StandardError; end
 require 'spree/core/delegate_belongs_to'
 require 'spree/core/permalinks'
 require 'spree/core/calculated_adjustments'
+require 'spree/core/adjustment_source'
 require 'spree/core/product_duplicator'
 require 'spree/core/controller_helpers'
 require 'spree/core/controller_helpers/search'

core/lib/spree/core/adjustment_source.rb

@@ -0,0 +1,26 @@
+module Spree
+  module Core
+    module AdjustmentSource
+      def self.included(klass)
+        klass.class_eval do
+          def deals_with_adjustments_for_deleted_source
+            adjustment_scope = self.adjustments.includes(:order).references(:spree_orders)
+
+            # For incomplete orders, remove the adjustment completely.
+            adjustment_scope.where("spree_orders.completed_at IS NULL").destroy_all
+
+            # For complete orders, the source will be invalid.
+            # Therefore we nullify the source_id, leaving the adjustment in place.
+            # This would mean that the order's total is not altered at all.
+            adjustment_scope.where("spree_orders.completed_at IS NOT NULL").each do |adjustment|
+              adjustment.update_columns(
+                source_id: nil,
+                updated_at: Time.now,
+              )
+            end
+          end
+        end
+      end
+    end
+  end
+end

core/spec/models/spree/tax_rate_spec.rb

@@ -302,21 +302,32 @@
         context "when price does not include tax" do
           before do
             @order.stub :tax_zone => @zone
-
             [@rate1, @rate2].each do |rate|
               rate.included_in_price = false
               rate.zone = @zone
               rate.save
             end
+            Spree::TaxRate.adjust(@order.tax_zone, @order.line_items)
+          end
+
+          it "should delete adjustments for open order when taxrate is deleted" do
+            @rate1.destroy!
+            @rate2.destroy!
+            line_item.adjustments.count.should == 0
+          end
+
+          it "should not delete adjustments for complete order when taxrate is deleted" do
+            @order.update_column :completed_at, Time.now
+            @rate1.destroy!
+            @rate2.destroy!
+            line_item.adjustments.count.should == 2
           end
 
           it "should create an adjustment" do
-            Spree::TaxRate.adjust(@order.tax_zone, @order.line_items)
             line_item.adjustments.count.should == 2
           end
 
           it "should not create a tax refund" do
-            Spree::TaxRate.adjust(@order.tax_zone, @order.line_items)
             line_item.adjustments.credit.count.should == 0
           end
         end