diff --git a/app/controllers/spree/admin/tax_rates_controller.rb b/app/controllers/spree/admin/tax_rates_controller.rb index 2df303bc75..b1846bff65 100644 --- a/app/controllers/spree/admin/tax_rates_controller.rb +++ b/app/controllers/spree/admin/tax_rates_controller.rb @@ -3,8 +3,38 @@ 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? + + transition_tax_rate + 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 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 @available_zones = Zone.order(:name) @available_categories = TaxCategory.order(:name) diff --git a/app/services/tax_rate_updater.rb b/app/services/tax_rate_updater.rb new file mode 100644 index 0000000000..d02754640f --- /dev/null +++ b/app/services/tax_rate_updater.rb @@ -0,0 +1,37 @@ +# 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! + ActiveRecord::Base.transaction do + updated_rate.save && current_rate.destroy + end + 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 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 diff --git a/spec/services/tax_rate_updater_spec.rb b/spec/services/tax_rate_updater_spec.rb new file mode 100644 index 0000000000..6f030f9108 --- /dev/null +++ b/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