mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-26 01:33:22 +00:00
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.
This commit is contained in:
@@ -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)
|
||||
|
||||
54
spec/controllers/spree/admin/tax_rates_controller_spec.rb
Normal file
54
spec/controllers/spree/admin/tax_rates_controller_spec.rb
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user