From 9789911523b607183cc445db8ca05fc57063e78a Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 28 Mar 2023 11:41:26 +1100 Subject: [PATCH] Rework how voucher are applied to an order and handle tax calculation At the time when we can apply a voucher to an order (payment step) we don't have any data on possible taxes and payment fees. These are calculated when we move to the summary step. So voucher are applied in two step: - create an "open" voucher adjustment on the order when a customer enters a voucher code. - when we move to the summary step, recalulate the amount and possible tax for the voucher adjustment, once taxes and payment fees have been included in the order. And then close the original and update order to reflect the changes --- app/controllers/split_checkout_controller.rb | 9 ++ app/models/voucher.rb | 90 ++++++++++- spec/models/voucher_spec.rb | 144 ++++++++++++++++++ .../consumer/split_checkout_tax_incl_spec.rb | 38 +++++ .../split_checkout_tax_not_incl_spec.rb | 41 +++++ 5 files changed, 321 insertions(+), 1 deletion(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 5202673270..a125ca6e20 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -290,5 +290,14 @@ class SplitCheckoutController < ::BaseController def recalculate_tax @order.create_tax_charge! @order.update_order! + + apply_voucher if @order.vouchers.present? + end + + def apply_voucher + Voucher.adjust!(@order) + + # update order to take into account the voucher we applied + @order.update_order! end end diff --git a/app/models/voucher.rb b/app/models/voucher.rb index b87e59a94b..99707fd5e0 100644 --- a/app/models/voucher.rb +++ b/app/models/voucher.rb @@ -13,6 +13,73 @@ class Voucher < ApplicationRecord before_validation :add_calculator + def self.adjust!(order) + return if order.nil? + + # Find open Voucher Adjustment + return if order.vouchers.empty? + + # We only support one voucher per order right now, we could just loop on vouchers + adjustment = order.vouchers.first + + # Recalculate value + amount = adjustment.originator.compute_amount(order) + + if order.additional_tax_total.positive? + handle_tax_excluded_from_price(order, amount) + else + handle_tax_included_in_price(order, amount) + end + + # Move to closed state + adjustment.close + end + + def self.handle_tax_excluded_from_price(order, amount) + voucher_rate = amount / order.total + + # TODO: might need to use VoucherTax has originator (sub class of Voucher) + # Adding the voucher tax part + tax_amount = voucher_rate * order.additional_tax_total + + adjustment = order.vouchers.first + adjustment_attributes = { + amount: tax_amount, + originator: adjustment.originator, + order: order, + label: "Tax #{adjustment.label}", + mandatory: false, + state: 'closed', + tax_category: nil, + included_tax: 0 + } + order.adjustments.create(adjustment_attributes) + + # Update the adjustment amount + amount = voucher_rate * (order.total - order.additional_tax_total) + + adjustment.update_columns( + amount: amount, + updated_at: Time.zone.now + ) + end + + def self.handle_tax_included_in_price(order, amount) + voucher_rate = amount / order.total + included_tax = voucher_rate * order.included_tax_total + + # Update Adjustment + adjustment = order.vouchers.first + + return unless amount != adjustment.amount || included_tax != 0 + + adjustment.update_columns( + amount: amount, + included_tax: included_tax, + updated_at: Time.zone.now + ) + end + def value 10 end @@ -21,9 +88,30 @@ class Voucher < ApplicationRecord Spree::Money.new(value) end + # override the one from CalculatedAdjustments + # Create an "open" adjustment which will be updated later once tax and other fees have + # been applied to the order + def create_adjustment(label, order, mandatory = false, _state = "open", tax_category = nil) + amount = compute_amount(order) + + return if amount.zero? && !mandatory + + adjustment_attributes = { + amount: amount, + originator: self, + order: order, + label: label, + mandatory: mandatory, + state: "open", + tax_category: tax_category + } + + order.adjustments.create(adjustment_attributes) + end + # override the one from CalculatedAdjustments so we limit adjustment to the maximum amount # needed to cover the order, ie if the voucher covers more than the order.total we only need - # to create a adjustment covering the order.total + # to create an adjustment covering the order.total # Doesn't work with taxes for now def compute_amount(order) amount = calculator.compute(order) diff --git a/spec/models/voucher_spec.rb b/spec/models/voucher_spec.rb index 97675b0d0a..fed7b91cd1 100644 --- a/spec/models/voucher_spec.rb +++ b/spec/models/voucher_spec.rb @@ -29,6 +29,131 @@ describe Voucher do end end + describe '.adjust!' do + let(:voucher) { Voucher.create(code: 'new_code', enterprise: enterprise) } + + context 'when voucher covers the order total' do + subject { order.vouchers.first } + + let(:order) { create(:order_with_totals) } + + it 'updates the adjustment amount to -order.total' do + voucher.create_adjustment(voucher.code, order) + + order.total = 6 + order.save! + + Voucher.adjust!(order) + + expect(subject.amount.to_f).to eq(-6.0) + end + end + + context 'with price included in order price' do + subject { order.vouchers.first } + + let(:order) do + create( + :order_with_taxes, + distributor: enterprise, + ship_address: create(:address), + product_price: 110, + tax_rate_amount: 0.10, + included_in_price: true, + tax_rate_name: "Tax 1" + ) + end + + before do + # create adjustment before tax are set + voucher.create_adjustment(voucher.code, order) + + # Update taxes + order.create_tax_charge! + order.update_shipping_fees! + order.update_order! + + Voucher.adjust!(order) + end + + it 'updates the adjustment included_tax' do + # voucher_rate = amount / order.total + # -10 / 150 = -0.066666667 + # included_tax = voucher_rate * order.included_tax_total + # -0.66666666 * 10 = -0.67 + expect(subject.included_tax.to_f).to eq(-0.67) + end + + it 'moves the adjustment state to closed' do + expect(subject.state).to eq('closed') + end + end + + context 'with price not included in order price' do + let(:order) do + create( + :order_with_taxes, + distributor: enterprise, + ship_address: create(:address), + product_price: 110, + tax_rate_amount: 0.10, + included_in_price: false, + tax_rate_name: "Tax 1" + ) + end + + before do + # create adjustment before tax are set + voucher.create_adjustment(voucher.code, order) + + # Update taxes + order.create_tax_charge! + order.update_shipping_fees! + order.update_order! + + Voucher.adjust!(order) + end + + it 'includes amount withou tax' do + adjustment = order.vouchers.first + # voucher_rate = amount / order.total + # -10 / 161 = -0.062111801 + # amount = voucher_rate * (order.total - order.additional_tax_total) + # -0.062111801 * (161 -11) = -9.32 + expect(adjustment.amount.to_f).to eq(-9.32) + end + + it 'creates a tax adjustment' do + # voucher_rate = amount / order.total + # -10 / 161 = -0.062111801 + # amount = voucher_rate * order.additional_tax_total + # -0.0585 * 11 = -0.68 + tax_adjustment = order.vouchers.second + expect(tax_adjustment.amount.to_f).to eq(-0.68) + expect(tax_adjustment.label).to match("Tax") + end + + it 'moves the adjustment state to closed' do + adjustment = order.vouchers.first + expect(adjustment.state).to eq('closed') + end + end + + context 'when no order given' do + it "doesn't blow up" do + expect { Voucher.adjust!(nil) }.to_not raise_error + end + end + + context 'when no voucher used on the given order' do + let(:order) { create(:order_with_line_items, line_items_count: 1, distributor: enterprise) } + + it "doesn't blow up" do + expect { Voucher.adjust!(order) }.to_not raise_error + end + end + end + describe '#compute_amount' do subject { Voucher.create(code: 'new_code', enterprise: enterprise) } @@ -47,4 +172,23 @@ describe Voucher do end end end + + describe '#create_adjustment' do + subject(:adjustment) { voucher.create_adjustment(voucher.code, order) } + + let(:voucher) { Voucher.create(code: 'new_code', enterprise: enterprise) } + let(:order) { create(:order_with_line_items, line_items_count: 1, distributor: enterprise) } + + it 'includes the full voucher amount' do + expect(adjustment.amount.to_f).to eq(-10.0) + end + + it 'has no included_tax' do + expect(adjustment.included_tax.to_f).to eq(0.0) + end + + it 'sets the adjustment as open' do + expect(adjustment.state).to eq("open") + end + end end diff --git a/spec/system/consumer/split_checkout_tax_incl_spec.rb b/spec/system/consumer/split_checkout_tax_incl_spec.rb index 146104e824..916e1db80f 100644 --- a/spec/system/consumer/split_checkout_tax_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_incl_spec.rb @@ -134,6 +134,44 @@ describe "As a consumer, I want to see adjustment breakdown" do # DB checks assert_db_tax_incl end + + context "when using a voucher" do + let!(:voucher) { Voucher.create(code: 'some_code', enterprise: distributor) } + + it "will include a tax included amount on the voucher adjustment" do + visit checkout_step_path(:details) + + choose "Delivery" + + click_button "Next - Payment method" + + # add Voucher + fill_in "Enter voucher code", with: voucher.code + click_button("Apply") + + # Choose payment ?? + click_on "Next - Order summary" + click_on "Complete order" + + # UI checks + expect(page).to have_content("Confirmed") + expect(page).to have_selector('#order_total', text: with_currency(0.00)) + expect(page).to have_selector('#tax-row', text: with_currency(1.15)) + + # Voucher + within "#line-items" do + expect(page).to have_content(voucher.code) + expect(page).to have_content(with_currency(-10.00)) + end + + # DB check + order_within_zone.reload + voucher_adjustment = order_within_zone.vouchers.first + + expect(voucher_adjustment.amount.to_f).to eq(-10) + expect(voucher_adjustment.included_tax.to_f).to eq(-1.15) + end + end end end diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb index 435d02d658..6cb7f45a9e 100644 --- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb @@ -142,6 +142,47 @@ describe "As a consumer, I want to see adjustment breakdown" do expect(page).to have_selector('#order_total', text: with_currency(11.30)) expect(page).to have_selector('#tax-row', text: with_currency(1.30)) end + + context "when using a voucher" do + let!(:voucher) { Voucher.create(code: 'some_code', enterprise: distributor) } + + it "will include a tax included amount on the voucher adjustment" do + visit checkout_step_path(:details) + + choose "Delivery" + + click_button "Next - Payment method" + # add Voucher + fill_in "Enter voucher code", with: voucher.code + click_button("Apply") + + # Choose payment ?? + click_on "Next - Order summary" + click_on "Complete order" + + # UI checks + expect(page).to have_content("Confirmed") + expect(page).to have_selector('#order_total', text: with_currency(1.30)) + expect(page).to have_selector('#tax-row', text: with_currency(1.30)) + + # Voucher + within "#line-items" do + expect(page).to have_content(voucher.code) + expect(page).to have_content(with_currency(-8.85)) + + expect(page).to have_content("Tax #{voucher.code}") + expect(page).to have_content(with_currency(-1.15)) + end + + # DB check + order_within_zone.reload + voucher_adjustment = order_within_zone.vouchers.first + voucher_tax_adjustment = order_within_zone.vouchers.second + + expect(voucher_adjustment.amount.to_f).to eq(-8.85) + expect(voucher_tax_adjustment.amount.to_f).to eq(-1.15) + end + end end end