From 8254bd962595d2e926b141fed4513cefc55c7a6d Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 26 Sep 2023 15:24:29 +0200 Subject: [PATCH 1/8] Take into account voucher tax part when displaying tax included in price --- app/helpers/checkout_helper.rb | 4 +++- spec/helpers/checkout_helper_spec.rb | 25 +++++++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index b9b5ce8d40..dabbf3d4a6 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -53,7 +53,9 @@ module CheckoutHelper end def display_checkout_tax_total(order) - Spree::Money.new order.total_tax, currency: order.currency + total_tax = order.total_tax + VoucherAdjustmentsService.new(order).voucher_included_tax + + Spree::Money.new(total_tax, currency: order.currency) end def display_checkout_taxes_hash(order) diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb index 31ad004106..9cedc34d44 100644 --- a/spec/helpers/checkout_helper_spec.rb +++ b/spec/helpers/checkout_helper_spec.rb @@ -15,12 +15,29 @@ describe CheckoutHelper, type: :helper do helper.validated_input("test", "foo", type: :email) end - describe "displaying the tax total for an order" do - let(:order) { double(:order, total_tax: 123.45, currency: 'AUD') } + describe "#display_checkout_tax_total" do + subject(:display_checkout_tax_total) { helper.display_checkout_tax_total(order) } + + let(:order) { instance_double(Spree::Order, total_tax: 123.45, currency: 'AUD') } + let(:service) do + instance_double(VoucherAdjustmentsService, voucher_included_tax: voucher_included_tax) + end + let(:voucher_included_tax) { 0.0 } + + before do + allow(VoucherAdjustmentsService).to receive(:new).and_return(service) + end it "retrieves the total tax on the order" do - expect(helper.display_checkout_tax_total(order)).to eq(Spree::Money.new(123.45, - currency: 'AUD')) + expect(display_checkout_tax_total).to eq(Spree::Money.new(123.45, currency: 'AUD')) + end + + context "with a voucher" do + let(:voucher_included_tax) { -0.45 } + + it "displays the discounted total tax" do + expect(display_checkout_tax_total).to eq(Spree::Money.new(123.00, currency: 'AUD')) + end end end From 8d639c14cb47d510630764f0e47aaee9250090e2 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 26 Sep 2023 16:07:07 +0200 Subject: [PATCH 2/8] Add a fake adjustment voucher tax on admin order page When tax are included in price, voucher tax is stored in the voucher adjustment and not as its own adjustment. So we add a "fake adjustment" for display purposes. --- app/helpers/admin/orders_helper.rb | 12 +++++++++++- config/locales/en.yml | 1 + spec/helpers/admin/orders_helper_spec.rb | 25 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/app/helpers/admin/orders_helper.rb b/app/helpers/admin/orders_helper.rb index 72a414bfd6..7f8bf16b5c 100644 --- a/app/helpers/admin/orders_helper.rb +++ b/app/helpers/admin/orders_helper.rb @@ -7,7 +7,17 @@ module Admin # We exclude shipping method adjustments because they are displayed in a # separate table together with the order line items. def order_adjustments_for_display(order) - order.adjustments + order.all_adjustments.payment_fee.eligible + adjustments_for_display = order.adjustments + order.all_adjustments.payment_fee.eligible + + if VoucherAdjustmentsService.new(order).voucher_included_tax.negative? + adjustment = order.voucher_adjustments.first + adjustments_for_display << Spree::Adjustment.new( + label: I18n.t("admin.orders.edit.voucher_tax_included_in_price", label: adjustment.label), + amount: adjustment.included_tax + ) + end + + adjustments_for_display end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index b279f8f91b..e6a82c3c62 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -974,6 +974,7 @@ en: orders: edit: order_sure_want_to: Are you sure you want to %{event} this order? + voucher_tax_included_in_price: "%{label} (tax included in price)" invoice_email_sent: 'Invoice email has been sent' order_email_resent: 'Order email has been resent' bulk_management: diff --git a/spec/helpers/admin/orders_helper_spec.rb b/spec/helpers/admin/orders_helper_spec.rb index 07d3ca5035..617ea83007 100644 --- a/spec/helpers/admin/orders_helper_spec.rb +++ b/spec/helpers/admin/orders_helper_spec.rb @@ -5,6 +5,14 @@ require "spec_helper" describe Admin::OrdersHelper, type: :helper do describe "#order_adjustments_for_display" do let(:order) { create(:order) } + let(:service) do + instance_double(VoucherAdjustmentsService, voucher_included_tax: voucher_included_tax) + end + let(:voucher_included_tax) { 0.0 } + + before do + allow(VoucherAdjustmentsService).to receive(:new).and_return(service) + end it "selects eligible adjustments" do adjustment = create(:adjustment, order:, adjustable: order, amount: 1) @@ -32,5 +40,22 @@ describe Admin::OrdersHelper, type: :helper do expect(helper.order_adjustments_for_display(order)).to eq [] end + + context "with a voucher with tax included in price" do + let(:enterprise) { build(:enterprise) } + let(:voucher) do + create(:voucher_flat_rate, code: 'new_code', enterprise: enterprise, amount: 10) + end + let(:voucher_included_tax) { -0.5 } + + it "includes a fake tax voucher adjustment" do + voucher_adjustment = voucher.create_adjustment(voucher.code, order) + voucher_adjustment.update(included_tax: voucher_included_tax) + + fake_adjustment = helper.order_adjustments_for_display(order).last + expect(fake_adjustment.label).to eq("new_code (tax included in price)") + expect(fake_adjustment.amount).to eq(-0.5) + end + end end end From 1a66f3d94fa1e84a41d73ea1553adef7feb07656 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Thu, 28 Sep 2023 10:25:38 +0200 Subject: [PATCH 3/8] Fix system spec to take into account discounted tax --- spec/system/consumer/checkout/tax_incl_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/consumer/checkout/tax_incl_spec.rb b/spec/system/consumer/checkout/tax_incl_spec.rb index c1657070e1..2c74c738aa 100644 --- a/spec/system/consumer/checkout/tax_incl_spec.rb +++ b/spec/system/consumer/checkout/tax_incl_spec.rb @@ -131,7 +131,7 @@ describe "As a consumer, I want to see adjustment breakdown" do # 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)) + expect(page).to have_selector('#tax-row', text: with_currency(0.00)) # Voucher within "#line-items" do From 7f2c1feaf8065c16f6fa1de80b33c4faaef4c047 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Thu, 28 Sep 2023 10:33:55 +0200 Subject: [PATCH 4/8] Fix rubocop warning --- spec/helpers/admin/orders_helper_spec.rb | 6 ++---- spec/helpers/checkout_helper_spec.rb | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/spec/helpers/admin/orders_helper_spec.rb b/spec/helpers/admin/orders_helper_spec.rb index 617ea83007..54afea4d89 100644 --- a/spec/helpers/admin/orders_helper_spec.rb +++ b/spec/helpers/admin/orders_helper_spec.rb @@ -5,9 +5,7 @@ require "spec_helper" describe Admin::OrdersHelper, type: :helper do describe "#order_adjustments_for_display" do let(:order) { create(:order) } - let(:service) do - instance_double(VoucherAdjustmentsService, voucher_included_tax: voucher_included_tax) - end + let(:service) { instance_double(VoucherAdjustmentsService, voucher_included_tax:) } let(:voucher_included_tax) { 0.0 } before do @@ -44,7 +42,7 @@ describe Admin::OrdersHelper, type: :helper do context "with a voucher with tax included in price" do let(:enterprise) { build(:enterprise) } let(:voucher) do - create(:voucher_flat_rate, code: 'new_code', enterprise: enterprise, amount: 10) + create(:voucher_flat_rate, code: 'new_code', enterprise:, amount: 10) end let(:voucher_included_tax) { -0.5 } diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb index 9cedc34d44..f52b611d9b 100644 --- a/spec/helpers/checkout_helper_spec.rb +++ b/spec/helpers/checkout_helper_spec.rb @@ -19,9 +19,7 @@ describe CheckoutHelper, type: :helper do subject(:display_checkout_tax_total) { helper.display_checkout_tax_total(order) } let(:order) { instance_double(Spree::Order, total_tax: 123.45, currency: 'AUD') } - let(:service) do - instance_double(VoucherAdjustmentsService, voucher_included_tax: voucher_included_tax) - end + let(:service) { instance_double(VoucherAdjustmentsService, voucher_included_tax: ) } let(:voucher_included_tax) { 0.0 } before do From 74c8f06e800978eeb069358a38de2ec66aee9753 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 12 Oct 2023 14:17:41 +1100 Subject: [PATCH 5/8] Simplify helper with extracted method --- app/helpers/admin/orders_helper.rb | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/app/helpers/admin/orders_helper.rb b/app/helpers/admin/orders_helper.rb index 7f8bf16b5c..b3f6d7029c 100644 --- a/app/helpers/admin/orders_helper.rb +++ b/app/helpers/admin/orders_helper.rb @@ -7,17 +7,23 @@ module Admin # We exclude shipping method adjustments because they are displayed in a # separate table together with the order line items. def order_adjustments_for_display(order) - adjustments_for_display = order.adjustments + order.all_adjustments.payment_fee.eligible + order.adjustments + + voucher_included_tax_representations(order) + + order.all_adjustments.payment_fee.eligible + end - if VoucherAdjustmentsService.new(order).voucher_included_tax.negative? - adjustment = order.voucher_adjustments.first - adjustments_for_display << Spree::Adjustment.new( - label: I18n.t("admin.orders.edit.voucher_tax_included_in_price", label: adjustment.label), + def voucher_included_tax_representations(order) + return [] unless VoucherAdjustmentsService.new(order).voucher_included_tax.negative? + + adjustment = order.voucher_adjustments.first + + [ + Spree::Adjustment.new( + label: I18n.t("admin.orders.edit.voucher_tax_included_in_price", + label: adjustment.label), amount: adjustment.included_tax ) - end - - adjustments_for_display + ] end end end From f83e78a5b88c38b19650ee807716947d1570c305 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 12 Oct 2023 15:16:38 +1100 Subject: [PATCH 6/8] Clarify adjustment data only for display We are not creating a new adjustment here. --- app/helpers/admin/orders_helper.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/helpers/admin/orders_helper.rb b/app/helpers/admin/orders_helper.rb index b3f6d7029c..4e82a41739 100644 --- a/app/helpers/admin/orders_helper.rb +++ b/app/helpers/admin/orders_helper.rb @@ -2,6 +2,8 @@ module Admin module OrdersHelper + AdjustmentData = Struct.new(:label, :amount) + # Adjustments to display under "Order adjustments". # # We exclude shipping method adjustments because they are displayed in a @@ -18,10 +20,10 @@ module Admin adjustment = order.voucher_adjustments.first [ - Spree::Adjustment.new( - label: I18n.t("admin.orders.edit.voucher_tax_included_in_price", - label: adjustment.label), - amount: adjustment.included_tax + AdjustmentData.new( + I18n.t("admin.orders.edit.voucher_tax_included_in_price", + label: adjustment.label), + adjustment.included_tax ) ] end From f4fde0a42cbfc0a61a340c016287d97d3b4257ca Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 12 Oct 2023 15:20:17 +1100 Subject: [PATCH 7/8] Trying to clarify voucher tax amount The whole concept is confusing. Maybe translators will find better versions. --- config/locales/en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index e6a82c3c62..fa2a5c9a73 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -974,7 +974,7 @@ en: orders: edit: order_sure_want_to: Are you sure you want to %{event} this order? - voucher_tax_included_in_price: "%{label} (tax included in price)" + voucher_tax_included_in_price: "%{label} (tax included in voucher)" invoice_email_sent: 'Invoice email has been sent' order_email_resent: 'Order email has been resent' bulk_management: From 477ca39e58f07df3438592fcba7863e4bb80dfc4 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 12 Oct 2023 15:40:06 +1100 Subject: [PATCH 8/8] Update spec --- spec/helpers/admin/orders_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/admin/orders_helper_spec.rb b/spec/helpers/admin/orders_helper_spec.rb index 54afea4d89..ff00b73dc7 100644 --- a/spec/helpers/admin/orders_helper_spec.rb +++ b/spec/helpers/admin/orders_helper_spec.rb @@ -51,7 +51,7 @@ describe Admin::OrdersHelper, type: :helper do voucher_adjustment.update(included_tax: voucher_included_tax) fake_adjustment = helper.order_adjustments_for_display(order).last - expect(fake_adjustment.label).to eq("new_code (tax included in price)") + expect(fake_adjustment.label).to eq("new_code (tax included in voucher)") expect(fake_adjustment.amount).to eq(-0.5) end end