Extract tax rate transition logic to service

app/controllers/spree/admin/tax_rates_controller.rb

@@ -3,26 +3,12 @@ module Spree
     class TaxRatesController < ::Admin::ResourceController
       before_action :load_data
 
+      delegate :transition_rate!, :updated_rate, to: :updater
+
       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
+        transition_tax_rate
       end
 
       private
@@ -35,10 +21,18 @@ module Spree
         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
+      def transition_tax_rate
+        if transition_rate!
+          redirect_to location_after_save,
+                      flash: { success: flash_message_for(updated_rate, :successfully_updated) }
+        else
+          redirect_to spree.edit_admin_tax_rate_path(@tax_rate),
+                      flash: { error: updated_rate.errors.full_messages.to_sentence }
+        end
+      end
+
+      def updater
+        @updater ||= TaxRateUpdater.new(@tax_rate, permitted_resource_params)
       end
 
       def load_data

app/services/tax_rate_updater.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+# 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.
+
+class TaxRateUpdater
+  def initialize(current_rate, permitted_params)
+    @current_rate = current_rate
+    @permitted_params = permitted_params
+  end
+
+  def updated_rate
+    @updated_rate ||= begin
+      clone = clone_tax_rate!
+      clone.assign_attributes(permitted_params)
+      clone
+    end
+  end
+
+  def transition_rate!
+    updated_rate.save && current_rate.destroy
+  end
+
+  private
+
+  attr_reader :current_rate, :permitted_params
+
+  def clone_tax_rate!
+    cloned_rate = current_rate.deep_dup
+    cloned_rate.calculator = current_rate.calculator.deep_dup
+    cloned_rate
+  end
+end

spec/services/tax_rate_updater_spec.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe TaxRateUpdater do
+  let!(:old_tax_rate) {
+    create(:tax_rate, name: "Test Rate", amount: 0.2, calculator: Calculator::DefaultTax.new)
+  }
+  let(:params) { { amount: 0.5 } }
+  let(:service) { TaxRateUpdater.new(old_tax_rate, params) }
+  let(:new_tax_rate) { service.updated_rate }
+
+  describe "#updated_rate" do
+    it "returns a cloned (unsaved) tax rate with the new attributes assigned" do
+      expect(new_tax_rate).to_not be old_tax_rate
+      expect(new_tax_rate.amount).to eq params[:amount]
+      expect(new_tax_rate.id).to be_nil
+      expect(new_tax_rate.calculator.class).to eq old_tax_rate.calculator.class
+      expect(new_tax_rate).to be_valid
+    end
+  end
+
+  describe "#transition_rate!" do
+    it "saves the new tax_rate and deletes the old tax_rate" do
+      expect(new_tax_rate).to receive(:save).and_call_original
+      expect(old_tax_rate).to receive(:destroy).and_call_original
+
+      expect(service.transition_rate!).to be_truthy
+
+      expect(new_tax_rate.reload.persisted?).to be true
+      expect(old_tax_rate.reload.deleted?).to be true
+    end
+
+    context "when saving the new tax_rate fails" do
+      it "does not delete the old tax_rate and returns a falsey value" do
+        expect(new_tax_rate).to receive(:save) { false }
+        expect(old_tax_rate).to_not receive(:destroy)
+
+        expect(service.transition_rate!).to_not be_truthy
+      end
+    end
+  end
+end