From 8cc4c6a63f41a821955eb17dbc2f2f50c9c67e49 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 15 Jan 2021 12:00:01 +0000
Subject: [PATCH] Introduce soft-deprecation strategy when modifying tax rates

If an admin changes the amount on a tax rate, and that rate has been used by adjustments in the past, we need to soft-delete and clone it to preserve the data integrity of previous adjustments that were created using that rate.
---
 .../spree/admin/tax_rates_controller.rb       | 36 +++++++++++++
 .../spree/admin/tax_rates_controller_spec.rb  | 54 +++++++++++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 spec/controllers/spree/admin/tax_rates_controller_spec.rb

diff --git a/app/controllers/spree/admin/tax_rates_controller.rb b/app/controllers/spree/admin/tax_rates_controller.rb
index 2df303bc75..e3389b3e0c 100644
--- a/app/controllers/spree/admin/tax_rates_controller.rb
+++ b/app/controllers/spree/admin/tax_rates_controller.rb
@@ -3,8 +3,44 @@ module Spree
     class TaxRatesController < ::Admin::ResourceController
       before_action :load_data
 
+      def update
+        return super unless amount_changed? && associated_adjustments?
+
+        # If a TaxRate is modified in production and the amount is changed, we need to clone
+        # and soft-delete it to preserve associated data on previous orders. For example; previous
+        # orders will have adjustments created with that rate. Those old orders will keep the
+        # rate they had when they were created, and new orders will have the new rate applied.
+
+        cloned_rate = clone_tax_rate(@tax_rate)
+        cloned_rate.assign_attributes(permitted_resource_params)
+
+        if cloned_rate.save
+          @tax_rate.destroy
+
+          redirect_to location_after_save,
+                      flash: { success: flash_message_for(cloned_rate, :successfully_updated) }
+        else
+          redirect_to spree.edit_admin_tax_rate_path(@tax_rate),
+                      flash: { error: cloned_rate.errors.full_messages.to_sentence }
+        end
+      end
+
       private
 
+      def amount_changed?
+        BigDecimal(permitted_resource_params[:amount]) != @tax_rate.amount
+      end
+
+      def associated_adjustments?
+        Spree::Adjustment.where(originator: @tax_rate).exists?
+      end
+
+      def clone_tax_rate(tax_rate)
+        cloned_rate = tax_rate.deep_dup
+        cloned_rate.calculator = tax_rate.calculator.deep_dup
+        cloned_rate
+      end
+
       def load_data
         @available_zones = Zone.order(:name)
         @available_categories = TaxCategory.order(:name)
diff --git a/spec/controllers/spree/admin/tax_rates_controller_spec.rb b/spec/controllers/spree/admin/tax_rates_controller_spec.rb
new file mode 100644
index 0000000000..266edc9430
--- /dev/null
+++ b/spec/controllers/spree/admin/tax_rates_controller_spec.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+module Spree
+  module Admin
+    describe TaxRatesController, type: :controller do
+      include AuthenticationHelper
+
+      let!(:tax_rate) {
+        create(:tax_rate, name: "Original Rate", amount: 0.1, calculator: build(:calculator))
+      }
+
+      describe "#update" do
+        before { controller_login_as_admin }
+
+        context "when the tax rate has associated adjustments" do
+          let!(:adjustment) { create(:adjustment, originator: tax_rate) }
+
+          context "when the amount is not changed" do
+            it "updates the record" do
+              expect {
+                spree_put :update, id: tax_rate.id, tax_rate: { name: "Updated Rate", amount: 0.1 }
+              }.to_not change{ Spree::TaxRate.with_deleted.count }
+
+              expect(response).to redirect_to spree.admin_tax_rates_url
+              expect(tax_rate.reload.name).to eq "Updated Rate"
+              expect(tax_rate.amount).to eq 0.1
+            end
+          end
+
+          context "when the amount is changed" do
+            it "duplicates the record and soft-deletes the duplicate" do
+              expect {
+                spree_put :update, id: tax_rate.id, tax_rate: { name: "Changed Rate", amount: 0.5 }
+              }.to change{ Spree::TaxRate.with_deleted.count }.by(1)
+
+              expect(response).to redirect_to spree.admin_tax_rates_url
+
+              deprecated_rate = tax_rate.reload
+              expect(deprecated_rate.name).to eq "Original Rate"
+              expect(deprecated_rate.amount).to eq 0.1
+              expect(deprecated_rate.deleted?).to be true
+
+              updated_rate = Spree::TaxRate.last
+              expect(updated_rate.name).to eq "Changed Rate"
+              expect(updated_rate.amount).to eq 0.5
+            end
+          end
+        end
+      end
+    end
+  end
+end