From 29a4bf88d7520b2e26615fa7e44a9a6a27c65ef7 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 12 Sep 2023 14:13:04 +0200
Subject: [PATCH 01/19] DRY up specs

---
 .../sales_tax_totals_by_order_spec.rb         | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
index f9ce7e273e..571d3862ad 100644
--- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
+++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
@@ -104,9 +104,7 @@ describe "Sales Tax Totals By order" do
     end
 
     it "generates the report" do
-      login_as admin
-      visit admin_reports_path
-      click_on "Sales Tax Totals By Order"
+      visit_sales_tax_totals_by_order
 
       expect(page).to have_button("Go")
       click_on "Go"
@@ -165,10 +163,9 @@ describe "Sales Tax Totals By order" do
       order.recreate_all_fees!
       OrderWorkflow.new(order).complete!
     end
+
     it "generates the report" do
-      login_as admin
-      visit admin_reports_path
-      click_on "Sales Tax Totals By Order"
+      visit_sales_tax_totals_by_order
 
       expect(page).to have_button("Go")
       click_on "Go"
@@ -334,9 +331,7 @@ describe "Sales Tax Totals By order" do
       order2.recreate_all_fees!
       OrderWorkflow.new(order2).complete!
 
-      login_as admin
-      visit admin_reports_path
-      click_on "Sales Tax Totals By Order"
+      visit_sales_tax_totals_by_order
     end
 
     it "should load all the orders" do
@@ -467,6 +462,12 @@ describe "Sales Tax Totals By order" do
     end
   end
 
+  def visit_sales_tax_totals_by_order
+    login_as admin
+    visit admin_reports_path
+    click_on "Sales Tax Totals By Order"
+  end
+
   def generate_report
     click_on "Go"
     wait_for_download

From a7bceb0b28a7ac863126ce59db15039bce4894a0 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 12 Sep 2023 14:14:01 +0200
Subject: [PATCH 02/19] Add spec scenario where a voucher is added to the order

Voucher tax portion needs to be removed from the tax amount so we
can display the correct tax in the report.
---
 .../sales_tax_totals_by_order_spec.rb         | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
index 571d3862ad..1f00216e52 100644
--- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
+++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
@@ -153,6 +153,82 @@ describe "Sales Tax Totals By order" do
         "order1@example.com"
       ].join(" "))
     end
+
+    context "with a voucher" do
+      let(:voucher) do
+        create(:voucher_flat_rate, code: 'some_code', enterprise: distributor, amount: 10)
+      end
+
+      before do
+        Flipper.enable :vouchers
+
+        # Add voucher to the order
+        voucher.create_adjustment(voucher.code, order)
+        VoucherAdjustmentsService.new(order).update
+
+        # the enterprise fees can be known only when the user selects the variants
+        # we'll need to create them by calling recreate_all_fees!
+        order.recreate_all_fees!
+
+        order.update_totals_and_states
+        OrderWorkflow.new(order).complete!
+      end
+
+      pending "generates the report with the correct tax amount" do
+        visit_sales_tax_totals_by_order
+
+        expect(page).to have_button("Go")
+        click_on "Go"
+        expect(page.find("table.report__table thead tr").text).to have_content(table_header)
+
+        # Calculation details :
+        # Price excl tax = order.total - total_tax + voucher tax (negative number)
+        # Tax amount = Non adjusted tax - voucher tax
+        # Price incl tac = Price excl tax + non adjusted tax
+        expect(page.find("table.report__table tbody").text).to have_content([
+          "DistributorEnterpriseWithTax",
+          "oc1",
+          "ORDER_NUMBER_1",
+          "GST Food",
+          "State",
+          "1.5 %",
+          "105.39", # 109.61 - 4.61 + 0.39
+          "1.34", # 1.73 (State tax) - 0.39 (Voucher tax)
+          "106.73", # 109.61 - 4.61 + 1.73
+          "cfname",
+          "clname",
+          "ABC123",
+          "order1@example.com"
+        ].join(" "))
+
+        expect(page.find("table.report__table tbody").text).to have_content([
+          "DistributorEnterpriseWithTax",
+          "oc1",
+          "ORDER_NUMBER_1",
+          "GST Food",
+          "Country",
+          "2.5 %",
+          "105.39", # 109.61 - 4.61 + 0.39
+          "2.49", # 2.88 (country tax) - 0.39 (voucher tax)
+          "107.88",  # 109.61 - 4.61 + 2.88
+          "cfname",
+          "clname",
+          "ABC123",
+          "order1@example.com"
+        ].join(" "))
+
+        expect(page.find("table.report__table tbody").text).to have_content([
+          "TOTAL",
+          "105.39", # 109.61 - 4.61 + 0.39
+          "4.22", # 1.73 + 2.88 (state + country tax) - 0.39  (Voucher tax)
+          "109.61",
+          "cfname",
+          "clname",
+          "ABC123",
+          "order1@example.com"
+        ].join(" "))
+      end
+    end
   end
 
   context 'included tax' do

From 99253992923bdccecdb9bf55680ed3635e37ff4d Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 12 Sep 2023 14:36:28 +0200
Subject: [PATCH 03/19] Sales Tax Totals by order, fix tax amount when voucher
 applied

Apply the tax discount to the tax collumn when a voucher is added
to the order, and update total excluding tax accordingly.
---
 app/models/spree/adjustment.rb                |  1 +
 .../sales_tax/sales_tax_totals_by_order.rb    | 30 ++++++++++++++-----
 .../sales_tax_totals_by_order_spec.rb         |  2 +-
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index 8c8bfb848e..e53f08db77 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -70,6 +70,7 @@ module Spree
     scope :credit, -> { where('amount < 0') }
     scope :return_authorization, -> { where(originator_type: "Spree::ReturnAuthorization") }
     scope :voucher, -> { where(originator_type: "Voucher") }
+    scope :voucher_tax, -> { where(originator_type: "Voucher").where("label LIKE 'Tax%'") }
     scope :non_voucher, -> { where.not(originator_type: "Voucher") }
     scope :inclusive, -> { where(included: true) }
     scope :additional, -> { where(included: false) }
diff --git a/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb b/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
index 7437e11f87..0566967962 100644
--- a/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
+++ b/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
@@ -92,8 +92,8 @@ module Reporting
               summary_row: proc do |_key, items, _rows|
                 order = items.first.second
                 {
-                  total_excl_tax: order.total - order.total_tax,
-                  tax: order.total_tax,
+                  total_excl_tax: order.total - (order.total_tax + voucher_tax_adjustment(order)),
+                  tax: order.total_tax + voucher_tax_adjustment(order),
                   total_incl_tax: order.total,
                   first_name: order.customer&.first_name,
                   last_name: order.customer&.last_name,
@@ -130,20 +130,36 @@ module Reporting
         end
 
         def total_excl_tax(query_result_row)
-          order(query_result_row).total - order(query_result_row).total_tax
+          order = order(query_result_row)
+          total_excl_tax = order.total - order.total_tax
+
+          # Tax adjusment is a negative value, so we need to substract to add it to the total
+          total_excl_tax - voucher_tax_adjustment(order)
         end
 
         def tax_rate_total(query_result_row)
+          total_tax = non_adjusted_tax_rate_total(query_result_row)
+
+          # Tax adjustment is already a negative value
+          order = order(query_result_row)
+          total_tax + voucher_tax_adjustment(order)
+        end
+
+        def total_incl_tax(query_result_row)
+          order(query_result_row).total -
+            order(query_result_row).total_tax +
+            non_adjusted_tax_rate_total(query_result_row)
+        end
+
+        def non_adjusted_tax_rate_total(query_result_row)
           order(query_result_row).all_adjustments
             .tax
             .where(originator_id: tax_rate_id(query_result_row))
             .pick('sum(amount)') || 0
         end
 
-        def total_incl_tax(query_result_row)
-          order(query_result_row).total -
-            order(query_result_row).total_tax +
-            tax_rate_total(query_result_row)
+        def voucher_tax_adjustment(order)
+          order.all_adjustments.voucher_tax.first&.amount || 0
         end
 
         def first_name(query_result_row)
diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
index 1f00216e52..2eab301a35 100644
--- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
+++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
@@ -174,7 +174,7 @@ describe "Sales Tax Totals By order" do
         OrderWorkflow.new(order).complete!
       end
 
-      pending "generates the report with the correct tax amount" do
+      it "generates the report with the correct tax amount" do
         visit_sales_tax_totals_by_order
 
         expect(page).to have_button("Go")

From 0c005ad734d11fb3dec89c4fe153eb55978345af Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Thu, 14 Sep 2023 10:21:50 +0200
Subject: [PATCH 04/19] Sales Tax Totals by order included tax, fix tax amount
 when voucher applied

Plus specs
---
 .../sales_tax/sales_tax_totals_by_order.rb    |  5 +-
 .../sales_tax_totals_by_order_spec.rb         | 84 ++++++++++++++++++-
 2 files changed, 84 insertions(+), 5 deletions(-)

diff --git a/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb b/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
index 0566967962..61a66b7360 100644
--- a/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
+++ b/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
@@ -159,7 +159,10 @@ module Reporting
         end
 
         def voucher_tax_adjustment(order)
-          order.all_adjustments.voucher_tax.first&.amount || 0
+          included_tax_voucher_adjustment = order.voucher_adjustments.first&.included_tax || 0
+          excluded_tax_voucher_adjustment = order.all_adjustments.voucher_tax.first&.amount || 0
+
+          included_tax_voucher_adjustment + excluded_tax_voucher_adjustment
         end
 
         def first_name(query_result_row)
diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
index 2eab301a35..08ec770633 100644
--- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
+++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
@@ -210,7 +210,7 @@ describe "Sales Tax Totals By order" do
           "2.5 %",
           "105.39", # 109.61 - 4.61 + 0.39
           "2.49", # 2.88 (country tax) - 0.39 (voucher tax)
-          "107.88",  # 109.61 - 4.61 + 2.88
+          "107.88", # 109.61 - 4.61 + 2.88
           "cfname",
           "clname",
           "ABC123",
@@ -235,12 +235,12 @@ describe "Sales Tax Totals By order" do
     before do
       state_tax_rate.update!({ included_in_price: true })
       country_tax_rate.update!({ included_in_price: true })
-
-      order.recreate_all_fees!
-      OrderWorkflow.new(order).complete!
     end
 
     it "generates the report" do
+      order.recreate_all_fees!
+      OrderWorkflow.new(order).complete!
+
       visit_sales_tax_totals_by_order
 
       expect(page).to have_button("Go")
@@ -290,6 +290,82 @@ describe "Sales Tax Totals By order" do
         "order1@example.com"
       ].join(" "))
     end
+
+    context "with a voucher" do
+      let(:voucher) do
+        create(:voucher_flat_rate, code: 'some_code', enterprise: distributor, amount: 10)
+      end
+
+      before do
+        Flipper.enable :vouchers
+
+        # Add voucher to the order
+        voucher.create_adjustment(voucher.code, order)
+        VoucherAdjustmentsService.new(order).update
+
+        # the enterprise fees can be known only when the user selects the variants
+        # we'll need to create them by calling recreate_all_fees!
+        order.recreate_all_fees!
+
+        order.update_totals_and_states
+        OrderWorkflow.new(order).complete!
+      end
+
+      it "generates the report with the correct tax amount" do
+        visit_sales_tax_totals_by_order
+
+        expect(page).to have_button("Go")
+        click_on "Go"
+        expect(page.find("table.report__table thead tr").text).to have_content(table_header)
+
+        # Calculation details :
+        # Price excl tax = order.total - total_tax + voucher included tax (negative number)
+        # Tax amount = Non adjusted tax - voucher included tax
+        # Price incl tac = Price excl tax + non adjusted tax
+        expect(page.find("table.report__table tbody").text).to have_content([
+          "DistributorEnterpriseWithTax",
+          "oc1",
+          "ORDER_NUMBER_1",
+          "GST Food",
+          "State",
+          "1.5 %",
+          "100.89", # 105 - 4.5 + 0.39
+          "1.31", # 1.7 (State tax) - 0.39 (Voucher tax)
+          "102.2", # 105 - 4.5 + 1.7 TODO ?
+          "cfname",
+          "clname",
+          "ABC123",
+          "order1@example.com"
+        ].join(" "))
+
+        expect(page.find("table.report__table tbody").text).to have_content([
+          "DistributorEnterpriseWithTax",
+          "oc1",
+          "ORDER_NUMBER_1",
+          "GST Food",
+          "Country",
+          "2.5 %",
+          "100.89", # 105 - 4.5 + 0.39
+          "2.41", # 2.8 (country tax) - 0.39 (voucher tax)
+          "103.3",  # 105 - 4.5 + 2.8
+          "cfname",
+          "clname",
+          "ABC123",
+          "order1@example.com"
+        ].join(" "))
+
+        expect(page.find("table.report__table tbody").text).to have_content([
+          "TOTAL",
+          "100.89", # 105 - 4.5 + 0.39
+          "4.11", # 1.7 + 2.8 (state + country tax) - 0.39  (Voucher tax)
+          "105.0",
+          "cfname",
+          "clname",
+          "ABC123",
+          "order1@example.com"
+        ].join(" "))
+      end
+    end
   end
 
   context 'should filter by customer' do

From 776f4780387b78c1954d0872a99c43f669684740 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Fri, 15 Sep 2023 11:42:04 +0200
Subject: [PATCH 05/19] Make update the only public method

I think this got lost when refactoring, but only `update` is meant
to be a public method, other than `new`.
---
 app/services/voucher_adjustments_service.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/services/voucher_adjustments_service.rb b/app/services/voucher_adjustments_service.rb
index 98a4c63f5e..90a5b4e3bc 100644
--- a/app/services/voucher_adjustments_service.rb
+++ b/app/services/voucher_adjustments_service.rb
@@ -32,6 +32,8 @@ class VoucherAdjustmentsService
     end
   end
 
+  private
+
   def handle_tax_excluded_from_price(voucher)
     voucher_rate = voucher.rate(@order)
     adjustment = @order.voucher_adjustments.first

From 7c34145ed772266f1ab5f8378ec5464c8f242f46 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 18 Sep 2023 14:43:44 +0200
Subject: [PATCH 06/19] Add voucher_included_tax and voucher_excluded_tax

It retrieves the tax part of the voucher adjustment for tax included
in price and tax excluded from price respectively.
---
 app/services/voucher_adjustments_service.rb   | 19 ++++++
 .../voucher_adjustments_service_spec.rb       | 66 +++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/app/services/voucher_adjustments_service.rb b/app/services/voucher_adjustments_service.rb
index 90a5b4e3bc..5462985d18 100644
--- a/app/services/voucher_adjustments_service.rb
+++ b/app/services/voucher_adjustments_service.rb
@@ -5,6 +5,10 @@ class VoucherAdjustmentsService
     @order = order
   end
 
+  # The tax part of the voucher is stored as explained below:
+  # * tax included in price: included_tax field of the voucher adjustment
+  # * tax exckuded from price: as an extra voucher adjustment, with label starting by "Tax "
+  #
   def update
     return if @order.nil?
 
@@ -32,6 +36,21 @@ class VoucherAdjustmentsService
     end
   end
 
+  def voucher_included_tax
+    return 0.0 if @order.voucher_adjustments.empty?
+
+    # We only support one voucher per order for now
+    @order.voucher_adjustments.first.included_tax
+  end
+
+  def voucher_excluded_tax
+    return 0.0 if @order.voucher_adjustments.empty?
+
+    return 0.0 if @order.voucher_adjustments.where("label LIKE 'Tax%'").empty?
+
+    @order.voucher_adjustments.where("label LIKE 'Tax%'").first.amount
+  end
+
   private
 
   def handle_tax_excluded_from_price(voucher)
diff --git a/spec/services/voucher_adjustments_service_spec.rb b/spec/services/voucher_adjustments_service_spec.rb
index 4d710943c1..68ddbaf0fb 100644
--- a/spec/services/voucher_adjustments_service_spec.rb
+++ b/spec/services/voucher_adjustments_service_spec.rb
@@ -294,4 +294,70 @@ describe VoucherAdjustmentsService do
       end
     end
   end
+
+  describe "#voucher_included_tax" do
+    subject(:voucher_included_tax) { VoucherAdjustmentsService.new(order).voucher_included_tax }
+
+    let(:order) { create(:order_with_totals) }
+    let(:enterprise) { build(:enterprise) }
+    let(:voucher) do
+      create(:voucher_flat_rate, code: 'new_code', enterprise: enterprise, amount: 10)
+    end
+
+    it "returns included tax from voucher adjusment" do
+      voucher_adjustment = voucher.create_adjustment(voucher.code, order)
+      # Manually update included tax, so we don't have to do a big data setup to be able to use
+      # VoucherAdjustmentsService.update
+      voucher_adjustment.update(included_tax: 0.5)
+
+      expect(voucher_included_tax).to eq(0.5)
+    end
+
+    context "When no voucher adjustment linked to the order" do
+      it "returns 0.0" do
+        expect(voucher_included_tax).to eq(0.0)
+      end
+    end
+  end
+
+  describe "#voucher_excluded_tax" do
+    subject(:voucher_excluded_tax) { VoucherAdjustmentsService.new(order).voucher_excluded_tax }
+    let(:order) { create(:order_with_totals) }
+    let(:enterprise) { build(:enterprise) }
+    let(:voucher) do
+      create(:voucher_flat_rate, code: 'new_code', enterprise: enterprise, amount: 10)
+    end
+
+    it "returns the amount from the tax voucher adjustment" do
+      voucher_adjustment = voucher.create_adjustment(voucher.code, order)
+      # Manually add voucher tax adjustment, so we don't have to do a big data setup to be able to
+      # use VoucherAdjustmentsService.update
+      order.adjustments.create!(
+        originator: voucher_adjustment.originator,
+        order: order,
+        label: "Tax #{voucher_adjustment.label}",
+        mandatory: false,
+        state: 'open',
+        tax_category: nil,
+        included_tax: 0,
+        amount: 0.5
+      )
+
+      expect(voucher_excluded_tax).to eq(0.5)
+    end
+
+    context "when no voucher adjustment tax" do
+      it "returns 0" do
+        voucher_adjustment = voucher.create_adjustment(voucher.code, order)
+
+        expect(voucher_excluded_tax).to eq(0.0)
+      end
+    end
+
+    context "when no voucher adjustment linked to the order" do
+      it "returns 0.0" do
+        expect(voucher_excluded_tax).to eq(0.0)
+      end
+    end
+  end
 end

From bec5ad55dd49fb2f42bedf342130cecd08fca255 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 18 Sep 2023 14:45:13 +0200
Subject: [PATCH 07/19] Use VoucherAdjustmentsService to retrieve tax part of
 the voucher

---
 app/models/spree/adjustment.rb                              | 1 -
 .../reports/sales_tax/sales_tax_totals_by_order.rb          | 6 ++----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index e53f08db77..8c8bfb848e 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -70,7 +70,6 @@ module Spree
     scope :credit, -> { where('amount < 0') }
     scope :return_authorization, -> { where(originator_type: "Spree::ReturnAuthorization") }
     scope :voucher, -> { where(originator_type: "Voucher") }
-    scope :voucher_tax, -> { where(originator_type: "Voucher").where("label LIKE 'Tax%'") }
     scope :non_voucher, -> { where.not(originator_type: "Voucher") }
     scope :inclusive, -> { where(included: true) }
     scope :additional, -> { where(included: false) }
diff --git a/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb b/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
index 61a66b7360..7e3d7cc01d 100644
--- a/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
+++ b/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
@@ -159,10 +159,8 @@ module Reporting
         end
 
         def voucher_tax_adjustment(order)
-          included_tax_voucher_adjustment = order.voucher_adjustments.first&.included_tax || 0
-          excluded_tax_voucher_adjustment = order.all_adjustments.voucher_tax.first&.amount || 0
-
-          included_tax_voucher_adjustment + excluded_tax_voucher_adjustment
+          VoucherAdjustmentsService.new(order).voucher_included_tax +
+            VoucherAdjustmentsService.new(order).voucher_excluded_tax
         end
 
         def first_name(query_result_row)

From 3197480121a3ab040c129d96765e638e881e91cd Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 18 Sep 2023 15:17:12 +0200
Subject: [PATCH 08/19] Prevent voucher from starting with "Tax"

Plus spec
---
 app/models/voucher.rb       | 3 +++
 config/locales/en.yml       | 4 ++++
 spec/models/voucher_spec.rb | 5 +++++
 3 files changed, 12 insertions(+)

diff --git a/app/models/voucher.rb b/app/models/voucher.rb
index 9ba6a95c3b..bb841e9555 100644
--- a/app/models/voucher.rb
+++ b/app/models/voucher.rb
@@ -13,6 +13,9 @@ class Voucher < ApplicationRecord
            dependent: :nullify
 
   validates :code, presence: true, uniqueness: { scope: :enterprise_id }
+  # We store the tax portion of a voucher in a separate adjustment with a label starting by "Tax"
+  # when tax are exluded from price. To avoid any issue we a code stating by Tax in not allowed
+  validates :code, format: { without: /\ATax/ }
 
   TYPES = ["Vouchers::FlatRate", "Vouchers::PercentageRate"].freeze
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index efeaadafd3..3a62412ef2 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -117,6 +117,10 @@ en:
             using_producer_stock_settings_but_count_on_hand_set: "must be blank because using producer stock settings"
             on_demand_but_count_on_hand_set: "must be blank if on demand"
             limited_stock_but_no_count_on_hand: "must be specified because forcing limited stock"
+        voucher:
+          attributes:
+            code:
+              invalid: "can't start with 'Tax'"
       messages:
         confirmation: "doesn't match %{attribute}"
         blank: "can't be blank"
diff --git a/spec/models/voucher_spec.rb b/spec/models/voucher_spec.rb
index 4e3c294af9..da47d8905e 100644
--- a/spec/models/voucher_spec.rb
+++ b/spec/models/voucher_spec.rb
@@ -28,6 +28,11 @@ describe Voucher do
 
     it { is_expected.to validate_presence_of(:code) }
     it { is_expected.to validate_uniqueness_of(:code).scoped_to(:enterprise_id) }
+    it "doesn't accept code starting with 'Tax'" do
+      voucher = build(:voucher_flat_rate, code: 'Taxnew_code', enterprise: enterprise)
+
+      expect(voucher.valid?).to be(false)
+    end
   end
 
   describe '#display_value' do

From 9e49da8faef0e29f2b1fc26c28efdca9e5e8aac0 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Thu, 21 Sep 2023 14:56:37 +0200
Subject: [PATCH 09/19] Add unit test for
 Reporting::Reports::SalesTax::SalesTaxTotalsByOrder

And a small refactoring.
Currently it only covers tax excluded from price
---
 .../sales_tax/sales_tax_totals_by_order.rb    |   7 +-
 .../reports/sales_tax_totals_by_order_spec.rb | 258 ++++++++++++++++++
 2 files changed, 262 insertions(+), 3 deletions(-)
 create mode 100644 spec/lib/reports/sales_tax_totals_by_order_spec.rb

diff --git a/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb b/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
index 7e3d7cc01d..60c0dcdf16 100644
--- a/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
+++ b/lib/reporting/reports/sales_tax/sales_tax_totals_by_order.rb
@@ -138,7 +138,7 @@ module Reporting
         end
 
         def tax_rate_total(query_result_row)
-          total_tax = non_adjusted_tax_rate_total(query_result_row)
+          total_tax = filtered_tax_rate_total(query_result_row)
 
           # Tax adjustment is already a negative value
           order = order(query_result_row)
@@ -148,10 +148,11 @@ module Reporting
         def total_incl_tax(query_result_row)
           order(query_result_row).total -
             order(query_result_row).total_tax +
-            non_adjusted_tax_rate_total(query_result_row)
+            filtered_tax_rate_total(query_result_row)
         end
 
-        def non_adjusted_tax_rate_total(query_result_row)
+        # filtered tax total, relevant when there are more than one tax rate
+        def filtered_tax_rate_total(query_result_row)
           order(query_result_row).all_adjustments
             .tax
             .where(originator_id: tax_rate_id(query_result_row))
diff --git a/spec/lib/reports/sales_tax_totals_by_order_spec.rb b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
new file mode 100644
index 0000000000..b59a34f1fe
--- /dev/null
+++ b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
@@ -0,0 +1,258 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
+  subject(:report) { Reporting::Reports::SalesTax::SalesTaxTotalsByOrder.new(user, {}) }
+
+  let(:user) { create(:user) }
+  let(:state_zone) { create(:zone_with_state_member) }
+  let(:country_zone) { create(:zone_with_member) }
+  let(:tax_category) { create(:tax_category, name: 'GST Food') }
+  let!(:state_tax_rate) do
+    create(:tax_rate, zone: state_zone, tax_category:, name: 'State', amount: 0.02)
+  end
+  let!(:country_tax_rate) do
+    create(:tax_rate, zone: country_zone, tax_category:, name: 'Country', amount: 0.01)
+  end
+  let(:ship_address) do
+    create(:ship_address, state: state_zone.members.first.zoneable,
+                          country: country_zone.members.first.zoneable)
+  end
+  let(:variant) { create(:variant) }
+  let(:product) { variant.product }
+  let(:supplier) do
+    create(:supplier_enterprise, name: 'SupplierEnterprise', charges_sales_tax: true)
+  end
+  let(:distributor) do
+    create(:distributor_enterprise_with_tax, name: 'DistributorEnterpriseWithTax',
+                                             charges_sales_tax: true)
+  end
+  let(:payment_method) { create(:payment_method, :flat_rate) }
+  let(:shipping_method) do
+    create(:shipping_method, :flat_rate, amount: 10, tax_category_id: tax_category.id)
+  end
+  let(:order) { create(:order_with_distributor, distributor:) }
+  let(:order_cycle) do
+    create(:simple_order_cycle, name: 'oc1', suppliers: [supplier], distributors: [distributor],
+                                variants: [variant])
+  end
+  let(:customer1) do
+    create(:customer, enterprise: create(:enterprise), user: create(:user),
+                      first_name: 'cfname', last_name: 'clname', code: 'ABC123')
+  end
+
+  before do
+    distributor.shipping_methods << shipping_method
+    distributor.payment_methods << payment_method
+
+    product.update!(supplier_id: supplier.id)
+    variant.update!(tax_category_id: tax_category.id)
+
+    order.update!(
+      number: 'ORDER_NUMBER_1',
+      order_cycle_id: order_cycle.id,
+      ship_address_id: ship_address.id,
+      customer_id: customer1.id,
+      email: 'order1@example.com'
+    )
+    order.line_items.create(variant:, quantity: 1, price: 100)
+
+    # the enterprise fees can be known only when the user selects the variants
+    # we'll need to create them by calling recreate_all_fees!
+    order.recreate_all_fees!
+  end
+
+  describe "#filtered_tax_rate_total" do
+    let(:query_row) do
+      [
+        [country_tax_rate.id, order.id],
+        order
+      ]
+    end
+
+    # TODO see if we can simplify setup
+    # sould be able to only create the adjustment we need, or maybe mock them ?
+    it "returns tax amount filtered by tax rate in query_row" do
+      OrderWorkflow.new(order).complete!
+
+      filtered_tax_total = report.filtered_tax_rate_total(query_row)
+
+      expect(filtered_tax_total).not_to eq(order.total_tax)
+
+      # 10 % of 10.00 shipment cost + 10 % of 100.00 line item price
+      expect(filtered_tax_total).to eq(0.1 + 1)
+    end
+  end
+
+  describe "#tax_rate_total" do
+    let(:query_row) do
+      [
+        [state_tax_rate.id, order.id],
+        order
+      ]
+    end
+
+    it "returns the tax amount filtered by tax rate in the query_row" do
+      OrderWorkflow.new(order).complete!
+
+      tax_total = report.tax_rate_total(query_row)
+
+      expect(tax_total).not_to eq(order.total_tax)
+
+      # 20 % of 10.00 shipment cost + 20 % of 100.00 line item price
+      expect(tax_total).to eq(0.2 + 2)
+    end
+
+    context "with a voucher" do
+      let(:voucher) do
+        create(:voucher_flat_rate, code: 'some_code', enterprise: order.distributor, amount: 10)
+      end
+
+      it "returns the tax amount adjusted with voucher tax discount" do
+        add_voucher(order, voucher)
+
+        tax_total = report.tax_rate_total(query_row)
+
+        # 20 % of 10.00 shipment cost + 20 % of 100.00 line item price - voucher tax
+        expect(tax_total).to eq(0.2 + 2 - 0.29)
+      end
+    end
+  end
+
+  describe "#total_excl_tax" do
+    let(:query_row) do
+      [
+        [state_tax_rate.id, order.id],
+        order
+      ]
+    end
+
+    it "returns the total excluding tax specified in query_row" do
+      OrderWorkflow.new(order).complete!
+
+      total = report.total_excl_tax(query_row)
+
+      # order.total - order.total_tax
+      expect(total).to eq(113.3 - 3.3)
+    end
+
+    context "with a voucher" do
+      let(:voucher) do
+        create(:voucher_flat_rate, code: 'some_code', enterprise: order.distributor, amount: 10)
+      end
+
+      it "returns the total exluding tax and indcluding voucher tax discount" do
+        add_voucher(order, voucher)
+
+        total = report.total_excl_tax(query_row)
+
+        # order.total - order.total_tax + voucher tax
+        expect(total).to eq(103.3 - 3.3 + 0.29)
+      end
+    end
+  end
+
+  describe "#total_incl_tax" do
+    let(:query_row) do
+      [
+        [state_tax_rate.id, order.id], order
+      ]
+    end
+
+    it "returns the total including the tax specified in query_row" do
+      OrderWorkflow.new(order).complete!
+
+      total = report.total_incl_tax(query_row)
+
+      # order.total - order.total_tax + filtered tax
+      expect(total).to eq(113.3 - 3.3 + 2.2)
+    end
+  end
+
+  describe "#rules" do
+    before do
+      OrderWorkflow.new(order).complete!
+    end
+
+    it "returns rules" do
+      expected_rules = [
+        {
+          group_by: :distributor,
+        },
+        {
+          group_by: :order_cycle,
+        },
+        {
+          group_by: :order_number,
+          summary_row: an_instance_of(Proc)
+        }
+      ]
+
+      expect(report.rules).to match(expected_rules)
+    end
+
+    describe "summary_row" do
+      it "returns expected totals" do
+        expected_summary = {
+          total_excl_tax: 110.00,
+          tax: 3.3,
+          total_incl_tax: 113.30,
+          first_name: "cfname",
+          last_name: "clname",
+          code: "ABC123",
+          email: "order1@example.com"
+        }
+
+        rules = report.rules
+
+        # Running the "summary row" Proc
+        item = [[state_tax_rate.id, order.id], order]
+        summary_row = rules.third[:summary_row].call(nil, [item], nil)
+
+        expect(summary_row).to eq(expected_summary)
+      end
+
+      context "with a voucher" do
+        let(:voucher) do
+          create(:voucher_flat_rate, code: 'some_code', enterprise: order.distributor, amount: 10)
+        end
+
+        it "adjusts total_excl_tax and tax with voucher tax" do
+          add_voucher(order, voucher)
+
+          # total_excl_tax = order.total - (order.total_tax - voucher_tax)
+          # tax = order.total_tax - voucher_tax
+          expected_summary = {
+            total_excl_tax: 100.29,
+            tax: 3.01,
+            total_incl_tax: 103.30,
+            first_name: "cfname",
+            last_name: "clname",
+            code: "ABC123",
+            email: "order1@example.com"
+          }
+
+          rules = report.rules
+
+          # Running the "summary row" Proc
+          item = [[state_tax_rate.id, order.id], order]
+          summary_row = rules.third[:summary_row].call(nil, [item], nil)
+
+          expect(summary_row).to eq(expected_summary)
+        end
+      end
+    end
+  end
+
+  def add_voucher(order, voucher)
+    Flipper.enable :vouchers
+
+    # Add voucher to the order
+    voucher.create_adjustment(voucher.code, order)
+    VoucherAdjustmentsService.new(order).update
+    order.update_totals_and_states
+
+    OrderWorkflow.new(order).complete!
+  end
+end

From f54846829daa8b5df6dcd8e131857454debf6027 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Thu, 21 Sep 2023 15:49:20 +0200
Subject: [PATCH 10/19] Add test for #voucher_tax_adjustment

And mock calls to VoucherAdjustmentsService
---
 .../reports/sales_tax_totals_by_order_spec.rb | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/spec/lib/reports/sales_tax_totals_by_order_spec.rb b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
index b59a34f1fe..2fb7c5ef81 100644
--- a/spec/lib/reports/sales_tax_totals_by_order_spec.rb
+++ b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
@@ -75,6 +75,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
     # sould be able to only create the adjustment we need, or maybe mock them ?
     it "returns tax amount filtered by tax rate in query_row" do
       OrderWorkflow.new(order).complete!
+      mock_voucher_adjustment_service
 
       filtered_tax_total = report.filtered_tax_rate_total(query_row)
 
@@ -95,6 +96,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
     it "returns the tax amount filtered by tax rate in the query_row" do
       OrderWorkflow.new(order).complete!
+      mock_voucher_adjustment_service
 
       tax_total = report.tax_rate_total(query_row)
 
@@ -111,6 +113,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
       it "returns the tax amount adjusted with voucher tax discount" do
         add_voucher(order, voucher)
+        mock_voucher_adjustment_service(excluded_tax: -0.29)
 
         tax_total = report.tax_rate_total(query_row)
 
@@ -130,6 +133,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
     it "returns the total excluding tax specified in query_row" do
       OrderWorkflow.new(order).complete!
+      mock_voucher_adjustment_service
 
       total = report.total_excl_tax(query_row)
 
@@ -144,6 +148,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
       it "returns the total exluding tax and indcluding voucher tax discount" do
         add_voucher(order, voucher)
+        mock_voucher_adjustment_service(excluded_tax: -0.29)
 
         total = report.total_excl_tax(query_row)
 
@@ -162,6 +167,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
     it "returns the total including the tax specified in query_row" do
       OrderWorkflow.new(order).complete!
+      mock_voucher_adjustment_service
 
       total = report.total_incl_tax(query_row)
 
@@ -176,6 +182,8 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
     end
 
     it "returns rules" do
+      mock_voucher_adjustment_service
+
       expected_rules = [
         {
           group_by: :distributor,
@@ -194,6 +202,8 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
     describe "summary_row" do
       it "returns expected totals" do
+        mock_voucher_adjustment_service
+
         expected_summary = {
           total_excl_tax: 110.00,
           tax: 3.3,
@@ -220,6 +230,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
         it "adjusts total_excl_tax and tax with voucher tax" do
           add_voucher(order, voucher)
+          mock_voucher_adjustment_service(excluded_tax: -0.29)
 
           # total_excl_tax = order.total - (order.total_tax - voucher_tax)
           # tax = order.total_tax - voucher_tax
@@ -245,6 +256,32 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
     end
   end
 
+  describe "#voucher_tax_adjustment" do
+    context "with tax excluded from price" do
+      it "returns the tax related voucher adjustment" do
+        mock_voucher_adjustment_service(excluded_tax: -0.1)
+
+        expect(report.voucher_tax_adjustment(order)).to eq(-0.1)
+      end
+    end
+
+    context "with tax included in price" do
+      it "returns the tax part of the voucher adjustment" do
+        mock_voucher_adjustment_service(included_tax: -0.2)
+
+        expect(report.voucher_tax_adjustment(order)).to eq(-0.2)
+      end
+    end
+
+    context "with both type of tax" do
+      it "returns sum of the tax part of voucher adjustment and tax related voucher adjusment" do
+        mock_voucher_adjustment_service(included_tax: -0.5, excluded_tax: -0.1)
+
+        expect(report.voucher_tax_adjustment(order)).to eq(-0.6)
+      end
+    end
+  end
+
   def add_voucher(order, voucher)
     Flipper.enable :vouchers
 
@@ -255,4 +292,14 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
     OrderWorkflow.new(order).complete!
   end
+
+  def mock_voucher_adjustment_service(included_tax: 0.0, excluded_tax: 0.0)
+    service = instance_double(
+      VoucherAdjustmentsService,
+      voucher_included_tax: included_tax,
+      voucher_excluded_tax: excluded_tax
+    )
+
+    allow(VoucherAdjustmentsService).to receive(:new).and_return(service)
+  end
 end

From 25adaaa6ea6140c5bbf8deedba171a37e680f21a Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Fri, 22 Sep 2023 10:13:24 +0200
Subject: [PATCH 11/19] Simplify test set up

---
 .../reports/sales_tax_totals_by_order_spec.rb  | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/spec/lib/reports/sales_tax_totals_by_order_spec.rb b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
index 2fb7c5ef81..8d0b98fe87 100644
--- a/spec/lib/reports/sales_tax_totals_by_order_spec.rb
+++ b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
@@ -19,14 +19,20 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
     create(:ship_address, state: state_zone.members.first.zoneable,
                           country: country_zone.members.first.zoneable)
   end
-  let(:variant) { create(:variant) }
+  let(:variant) { create(:variant, tax_category: ) }
   let(:product) { variant.product }
   let(:supplier) do
     create(:supplier_enterprise, name: 'SupplierEnterprise', charges_sales_tax: true)
   end
   let(:distributor) do
-    create(:distributor_enterprise_with_tax, name: 'DistributorEnterpriseWithTax',
-                                             charges_sales_tax: true)
+    create(
+      :distributor_enterprise_with_tax,
+      name: 'DistributorEnterpriseWithTax',
+      charges_sales_tax: true
+    ).tap do |distributor|
+      distributor.shipping_methods << shipping_method
+      distributor.payment_methods << payment_method
+    end
   end
   let(:payment_method) { create(:payment_method, :flat_rate) }
   let(:shipping_method) do
@@ -43,11 +49,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
   end
 
   before do
-    distributor.shipping_methods << shipping_method
-    distributor.payment_methods << payment_method
-
     product.update!(supplier_id: supplier.id)
-    variant.update!(tax_category_id: tax_category.id)
 
     order.update!(
       number: 'ORDER_NUMBER_1',
@@ -71,8 +73,6 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
       ]
     end
 
-    # TODO see if we can simplify setup
-    # sould be able to only create the adjustment we need, or maybe mock them ?
     it "returns tax amount filtered by tax rate in query_row" do
       OrderWorkflow.new(order).complete!
       mock_voucher_adjustment_service

From 0a68300e40369fec96fc107a0b7f67d233d79ef2 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Fri, 22 Sep 2023 10:22:47 +0200
Subject: [PATCH 12/19] Remove Voucher scenario

It is now covered by unit test
---
 .../sales_tax_totals_by_order_spec.rb         | 152 ------------------
 1 file changed, 152 deletions(-)

diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
index 08ec770633..475efc4568 100644
--- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
+++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
@@ -153,82 +153,6 @@ describe "Sales Tax Totals By order" do
         "order1@example.com"
       ].join(" "))
     end
-
-    context "with a voucher" do
-      let(:voucher) do
-        create(:voucher_flat_rate, code: 'some_code', enterprise: distributor, amount: 10)
-      end
-
-      before do
-        Flipper.enable :vouchers
-
-        # Add voucher to the order
-        voucher.create_adjustment(voucher.code, order)
-        VoucherAdjustmentsService.new(order).update
-
-        # the enterprise fees can be known only when the user selects the variants
-        # we'll need to create them by calling recreate_all_fees!
-        order.recreate_all_fees!
-
-        order.update_totals_and_states
-        OrderWorkflow.new(order).complete!
-      end
-
-      it "generates the report with the correct tax amount" do
-        visit_sales_tax_totals_by_order
-
-        expect(page).to have_button("Go")
-        click_on "Go"
-        expect(page.find("table.report__table thead tr").text).to have_content(table_header)
-
-        # Calculation details :
-        # Price excl tax = order.total - total_tax + voucher tax (negative number)
-        # Tax amount = Non adjusted tax - voucher tax
-        # Price incl tac = Price excl tax + non adjusted tax
-        expect(page.find("table.report__table tbody").text).to have_content([
-          "DistributorEnterpriseWithTax",
-          "oc1",
-          "ORDER_NUMBER_1",
-          "GST Food",
-          "State",
-          "1.5 %",
-          "105.39", # 109.61 - 4.61 + 0.39
-          "1.34", # 1.73 (State tax) - 0.39 (Voucher tax)
-          "106.73", # 109.61 - 4.61 + 1.73
-          "cfname",
-          "clname",
-          "ABC123",
-          "order1@example.com"
-        ].join(" "))
-
-        expect(page.find("table.report__table tbody").text).to have_content([
-          "DistributorEnterpriseWithTax",
-          "oc1",
-          "ORDER_NUMBER_1",
-          "GST Food",
-          "Country",
-          "2.5 %",
-          "105.39", # 109.61 - 4.61 + 0.39
-          "2.49", # 2.88 (country tax) - 0.39 (voucher tax)
-          "107.88", # 109.61 - 4.61 + 2.88
-          "cfname",
-          "clname",
-          "ABC123",
-          "order1@example.com"
-        ].join(" "))
-
-        expect(page.find("table.report__table tbody").text).to have_content([
-          "TOTAL",
-          "105.39", # 109.61 - 4.61 + 0.39
-          "4.22", # 1.73 + 2.88 (state + country tax) - 0.39  (Voucher tax)
-          "109.61",
-          "cfname",
-          "clname",
-          "ABC123",
-          "order1@example.com"
-        ].join(" "))
-      end
-    end
   end
 
   context 'included tax' do
@@ -290,82 +214,6 @@ describe "Sales Tax Totals By order" do
         "order1@example.com"
       ].join(" "))
     end
-
-    context "with a voucher" do
-      let(:voucher) do
-        create(:voucher_flat_rate, code: 'some_code', enterprise: distributor, amount: 10)
-      end
-
-      before do
-        Flipper.enable :vouchers
-
-        # Add voucher to the order
-        voucher.create_adjustment(voucher.code, order)
-        VoucherAdjustmentsService.new(order).update
-
-        # the enterprise fees can be known only when the user selects the variants
-        # we'll need to create them by calling recreate_all_fees!
-        order.recreate_all_fees!
-
-        order.update_totals_and_states
-        OrderWorkflow.new(order).complete!
-      end
-
-      it "generates the report with the correct tax amount" do
-        visit_sales_tax_totals_by_order
-
-        expect(page).to have_button("Go")
-        click_on "Go"
-        expect(page.find("table.report__table thead tr").text).to have_content(table_header)
-
-        # Calculation details :
-        # Price excl tax = order.total - total_tax + voucher included tax (negative number)
-        # Tax amount = Non adjusted tax - voucher included tax
-        # Price incl tac = Price excl tax + non adjusted tax
-        expect(page.find("table.report__table tbody").text).to have_content([
-          "DistributorEnterpriseWithTax",
-          "oc1",
-          "ORDER_NUMBER_1",
-          "GST Food",
-          "State",
-          "1.5 %",
-          "100.89", # 105 - 4.5 + 0.39
-          "1.31", # 1.7 (State tax) - 0.39 (Voucher tax)
-          "102.2", # 105 - 4.5 + 1.7 TODO ?
-          "cfname",
-          "clname",
-          "ABC123",
-          "order1@example.com"
-        ].join(" "))
-
-        expect(page.find("table.report__table tbody").text).to have_content([
-          "DistributorEnterpriseWithTax",
-          "oc1",
-          "ORDER_NUMBER_1",
-          "GST Food",
-          "Country",
-          "2.5 %",
-          "100.89", # 105 - 4.5 + 0.39
-          "2.41", # 2.8 (country tax) - 0.39 (voucher tax)
-          "103.3",  # 105 - 4.5 + 2.8
-          "cfname",
-          "clname",
-          "ABC123",
-          "order1@example.com"
-        ].join(" "))
-
-        expect(page.find("table.report__table tbody").text).to have_content([
-          "TOTAL",
-          "100.89", # 105 - 4.5 + 0.39
-          "4.11", # 1.7 + 2.8 (state + country tax) - 0.39  (Voucher tax)
-          "105.0",
-          "cfname",
-          "clname",
-          "ABC123",
-          "order1@example.com"
-        ].join(" "))
-      end
-    end
   end
 
   context 'should filter by customer' do

From 03ce39d5c536fc0db41951dd1d185b2c423c883e Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Fri, 22 Sep 2023 11:16:21 +0200
Subject: [PATCH 13/19] Add metadata to tax related voucher adjustment

In the scenario where you have tax excluded from price, when adding
a voucher to an order, we create 2 voucher adjusments. One of them
represent the tax part of the voucher, and has a label starting with
"Tax". To better differentiate them and allow a reliable way to
query it, we add a metadata entry.
---
 app/models/spree/adjustment.rb                 |  3 +++
 app/services/voucher_adjustments_service.rb    | 14 ++++++++++----
 .../voucher_adjustments_service_spec.rb        | 18 +++++++++++++++---
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index 8c8bfb848e..7bbea45d76 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -70,6 +70,9 @@ module Spree
     scope :credit, -> { where('amount < 0') }
     scope :return_authorization, -> { where(originator_type: "Spree::ReturnAuthorization") }
     scope :voucher, -> { where(originator_type: "Voucher") }
+    scope :voucher_tax, -> {
+      voucher.joins(:metadata).where(metadata: { fee_name: "Tax", fee_type: "Voucher" })
+    }
     scope :non_voucher, -> { where.not(originator_type: "Voucher") }
     scope :inclusive, -> { where(included: true) }
     scope :additional, -> { where(included: false) }
diff --git a/app/services/voucher_adjustments_service.rb b/app/services/voucher_adjustments_service.rb
index 5462985d18..f464bc749e 100644
--- a/app/services/voucher_adjustments_service.rb
+++ b/app/services/voucher_adjustments_service.rb
@@ -44,11 +44,9 @@ class VoucherAdjustmentsService
   end
 
   def voucher_excluded_tax
-    return 0.0 if @order.voucher_adjustments.empty?
+    return 0.0 if @order.voucher_adjustments.voucher_tax.empty?
 
-    return 0.0 if @order.voucher_adjustments.where("label LIKE 'Tax%'").empty?
-
-    @order.voucher_adjustments.where("label LIKE 'Tax%'").first.amount
+    @order.voucher_adjustments.voucher_tax.first.amount
   end
 
   private
@@ -86,6 +84,14 @@ class VoucherAdjustmentsService
     tax_adjustment = @order.adjustments.find_or_initialize_by(adjustment_attributes)
     tax_adjustment.amount = tax_amount
     tax_adjustment.save
+
+    # Add metada so we know which voucher adjustment is Tax related
+    AdjustmentMetadata.create(
+      adjustment: tax_adjustment,
+      enterprise: adjustment.originator.enterprise,
+      fee_name: "Tax",
+      fee_type: "Voucher"
+    )
   end
 
   def handle_tax_included_in_price(amount, voucher)
diff --git a/spec/services/voucher_adjustments_service_spec.rb b/spec/services/voucher_adjustments_service_spec.rb
index 68ddbaf0fb..01bb2e5175 100644
--- a/spec/services/voucher_adjustments_service_spec.rb
+++ b/spec/services/voucher_adjustments_service_spec.rb
@@ -136,6 +136,12 @@ describe VoucherAdjustmentsService do
           # -0.058479532 * 11 = -0.64
           expect(tax_adjustment.amount.to_f).to eq(-0.64)
           expect(tax_adjustment.label).to match("Tax")
+
+          expect(tax_adjustment.metadata.enterprise_id).to eq(
+            tax_adjustment.originator.enterprise.id
+          )
+          expect(tax_adjustment.metadata.fee_name).to eq("Tax")
+          expect(tax_adjustment.metadata.fee_type).to eq("Voucher")
         end
 
         context "when re calculating" do
@@ -330,9 +336,9 @@ describe VoucherAdjustmentsService do
 
     it "returns the amount from the tax voucher adjustment" do
       voucher_adjustment = voucher.create_adjustment(voucher.code, order)
-      # Manually add voucher tax adjustment, so we don't have to do a big data setup to be able to
-      # use VoucherAdjustmentsService.update
-      order.adjustments.create!(
+      # Manually add voucher tax adjustment and metadata, so we don't have to do a big data setup
+      # to be able to use VoucherAdjustmentsService.update
+      tax_adjustment = order.adjustments.create!(
         originator: voucher_adjustment.originator,
         order: order,
         label: "Tax #{voucher_adjustment.label}",
@@ -342,6 +348,12 @@ describe VoucherAdjustmentsService do
         included_tax: 0,
         amount: 0.5
       )
+      AdjustmentMetadata.create(
+        adjustment: tax_adjustment,
+        enterprise: tax_adjustment.originator.enterprise,
+        fee_name: "Tax",
+        fee_type: "Voucher"
+      )
 
       expect(voucher_excluded_tax).to eq(0.5)
     end

From 19fc1fab8c2a12b4484c8723e1b8883273862f92 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Fri, 22 Sep 2023 11:35:56 +0200
Subject: [PATCH 14/19] As per review, visit page directly to save time

We still visit the page as user would do once to make sure it is
working.
---
 .../sales_tax/sales_tax_totals_by_order_spec.rb      | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
index 475efc4568..20664687c6 100644
--- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
+++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb
@@ -104,7 +104,11 @@ describe "Sales Tax Totals By order" do
     end
 
     it "generates the report" do
-      visit_sales_tax_totals_by_order
+      # Check we can access the report as user would do.
+      # For speed sake we'll use `visit_sales_tax_totals_by_order` helper for the rest of the spec
+      login_as admin
+      visit admin_reports_path
+      click_on "Sales Tax Totals By Order"
 
       expect(page).to have_button("Go")
       click_on "Go"
@@ -464,8 +468,10 @@ describe "Sales Tax Totals By order" do
 
   def visit_sales_tax_totals_by_order
     login_as admin
-    visit admin_reports_path
-    click_on "Sales Tax Totals By Order"
+    visit admin_report_path(
+      report_type: :sales_tax,
+      report_subtype: :sales_tax_totals_by_order
+    )
   end
 
   def generate_report

From 097b775fa048380fc26f00b1cfb7ef783e888187 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <40413322+rioug@users.noreply.github.com>
Date: Mon, 25 Sep 2023 17:44:16 +1000
Subject: [PATCH 15/19] Add metadata before saving adjustment

Rails may put it all in one transaction

Co-authored-by: Maikel <maikel@email.org.au>
---
 app/services/voucher_adjustments_service.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/services/voucher_adjustments_service.rb b/app/services/voucher_adjustments_service.rb
index f464bc749e..f3566ae7f7 100644
--- a/app/services/voucher_adjustments_service.rb
+++ b/app/services/voucher_adjustments_service.rb
@@ -83,15 +83,15 @@ class VoucherAdjustmentsService
     # Update the amount if tax adjustment already exist, create if not
     tax_adjustment = @order.adjustments.find_or_initialize_by(adjustment_attributes)
     tax_adjustment.amount = tax_amount
-    tax_adjustment.save
 
     # Add metada so we know which voucher adjustment is Tax related
-    AdjustmentMetadata.create(
-      adjustment: tax_adjustment,
+    tax_adjustment.metadata ||= AdjustmentMetadata.new(
       enterprise: adjustment.originator.enterprise,
       fee_name: "Tax",
       fee_type: "Voucher"
     )
+
+    tax_adjustment.save
   end
 
   def handle_tax_included_in_price(amount, voucher)

From 02aaf6f69254eb536805ea3332b84bc2f2c491ee Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 25 Sep 2023 09:49:55 +0200
Subject: [PATCH 16/19] Remove voucher validation on code format

The code doesn't rely on label match any more so this restriction
is not necessary anymore
---
 app/models/voucher.rb       | 3 ---
 config/locales/en.yml       | 4 ----
 spec/models/voucher_spec.rb | 5 -----
 3 files changed, 12 deletions(-)

diff --git a/app/models/voucher.rb b/app/models/voucher.rb
index bb841e9555..9ba6a95c3b 100644
--- a/app/models/voucher.rb
+++ b/app/models/voucher.rb
@@ -13,9 +13,6 @@ class Voucher < ApplicationRecord
            dependent: :nullify
 
   validates :code, presence: true, uniqueness: { scope: :enterprise_id }
-  # We store the tax portion of a voucher in a separate adjustment with a label starting by "Tax"
-  # when tax are exluded from price. To avoid any issue we a code stating by Tax in not allowed
-  validates :code, format: { without: /\ATax/ }
 
   TYPES = ["Vouchers::FlatRate", "Vouchers::PercentageRate"].freeze
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 3a62412ef2..efeaadafd3 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -117,10 +117,6 @@ en:
             using_producer_stock_settings_but_count_on_hand_set: "must be blank because using producer stock settings"
             on_demand_but_count_on_hand_set: "must be blank if on demand"
             limited_stock_but_no_count_on_hand: "must be specified because forcing limited stock"
-        voucher:
-          attributes:
-            code:
-              invalid: "can't start with 'Tax'"
       messages:
         confirmation: "doesn't match %{attribute}"
         blank: "can't be blank"
diff --git a/spec/models/voucher_spec.rb b/spec/models/voucher_spec.rb
index da47d8905e..4e3c294af9 100644
--- a/spec/models/voucher_spec.rb
+++ b/spec/models/voucher_spec.rb
@@ -28,11 +28,6 @@ describe Voucher do
 
     it { is_expected.to validate_presence_of(:code) }
     it { is_expected.to validate_uniqueness_of(:code).scoped_to(:enterprise_id) }
-    it "doesn't accept code starting with 'Tax'" do
-      voucher = build(:voucher_flat_rate, code: 'Taxnew_code', enterprise: enterprise)
-
-      expect(voucher.valid?).to be(false)
-    end
   end
 
   describe '#display_value' do

From e9051f5c58d1c159da56d1bf8c05df12ad72c6d6 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 26 Sep 2023 10:26:06 +0200
Subject: [PATCH 17/19] As per review comment, clarify specs

And some DRYing
---
 .../reports/sales_tax_totals_by_order_spec.rb | 48 +++++++------------
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/spec/lib/reports/sales_tax_totals_by_order_spec.rb b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
index 8d0b98fe87..98d46e8c8a 100644
--- a/spec/lib/reports/sales_tax_totals_by_order_spec.rb
+++ b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
@@ -47,6 +47,12 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
     create(:customer, enterprise: create(:enterprise), user: create(:user),
                       first_name: 'cfname', last_name: 'clname', code: 'ABC123')
   end
+  let(:query_row) do
+    [
+      [state_tax_rate.id, order.id],
+      order
+    ]
+  end
 
   before do
     product.update!(supplier_id: supplier.id)
@@ -87,13 +93,6 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
   end
 
   describe "#tax_rate_total" do
-    let(:query_row) do
-      [
-        [state_tax_rate.id, order.id],
-        order
-      ]
-    end
-
     it "returns the tax amount filtered by tax rate in the query_row" do
       OrderWorkflow.new(order).complete!
       mock_voucher_adjustment_service
@@ -124,13 +123,6 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
   end
 
   describe "#total_excl_tax" do
-    let(:query_row) do
-      [
-        [state_tax_rate.id, order.id],
-        order
-      ]
-    end
-
     it "returns the total excluding tax specified in query_row" do
       OrderWorkflow.new(order).complete!
       mock_voucher_adjustment_service
@@ -152,19 +144,13 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
 
         total = report.total_excl_tax(query_row)
 
-        # order.total - order.total_tax + voucher tax
-        expect(total).to eq(103.3 - 3.3 + 0.29)
+        # discounted order total - discounted order tax
+        expect(total).to eq((113.3 - 10) - (3.3 - 0.29))
       end
     end
   end
 
   describe "#total_incl_tax" do
-    let(:query_row) do
-      [
-        [state_tax_rate.id, order.id], order
-      ]
-    end
-
     it "returns the total including the tax specified in query_row" do
       OrderWorkflow.new(order).complete!
       mock_voucher_adjustment_service
@@ -204,7 +190,13 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
       it "returns expected totals" do
         mock_voucher_adjustment_service
 
-        expected_summary = {
+        rules = report.rules
+
+        # Running the "summary row" Proc
+        item = [[state_tax_rate.id, order.id], order]
+        summary_row = rules.third[:summary_row].call(nil, [item], nil)
+
+        expect(summary_row).to eq({
           total_excl_tax: 110.00,
           tax: 3.3,
           total_incl_tax: 113.30,
@@ -212,15 +204,7 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
           last_name: "clname",
           code: "ABC123",
           email: "order1@example.com"
-        }
-
-        rules = report.rules
-
-        # Running the "summary row" Proc
-        item = [[state_tax_rate.id, order.id], order]
-        summary_row = rules.third[:summary_row].call(nil, [item], nil)
-
-        expect(summary_row).to eq(expected_summary)
+        })
       end
 
       context "with a voucher" do

From b129b9f1d019d658f7e50a2ba12bae22728e3357 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 26 Sep 2023 10:50:58 +0200
Subject: [PATCH 18/19] DRY up specs

---
 .../voucher_adjustments_service_spec.rb       | 77 +++++++------------
 1 file changed, 28 insertions(+), 49 deletions(-)

diff --git a/spec/services/voucher_adjustments_service_spec.rb b/spec/services/voucher_adjustments_service_spec.rb
index 01bb2e5175..d31d616b44 100644
--- a/spec/services/voucher_adjustments_service_spec.rb
+++ b/spec/services/voucher_adjustments_service_spec.rb
@@ -42,13 +42,7 @@ describe VoucherAdjustmentsService do
         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!
+          add_voucher(order, voucher)
 
           VoucherAdjustmentsService.new(order).update
         end
@@ -110,13 +104,7 @@ describe VoucherAdjustmentsService do
         let(:tax_adjustment) { order.voucher_adjustments.second }
 
         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!
+          add_voucher(order, voucher)
 
           VoucherAdjustmentsService.new(order).update
         end
@@ -220,13 +208,7 @@ describe VoucherAdjustmentsService do
         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!
+          add_voucher(order, voucher)
 
           VoucherAdjustmentsService.new(order).update
         end
@@ -254,13 +236,7 @@ describe VoucherAdjustmentsService do
         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!
+          add_voucher(order, voucher)
 
           VoucherAdjustmentsService.new(order).update
         end
@@ -328,34 +304,28 @@ describe VoucherAdjustmentsService do
 
   describe "#voucher_excluded_tax" do
     subject(:voucher_excluded_tax) { VoucherAdjustmentsService.new(order).voucher_excluded_tax }
-    let(:order) { create(:order_with_totals) }
+    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
     let(:enterprise) { build(:enterprise) }
     let(:voucher) do
       create(:voucher_flat_rate, code: 'new_code', enterprise: enterprise, amount: 10)
     end
 
     it "returns the amount from the tax voucher adjustment" do
-      voucher_adjustment = voucher.create_adjustment(voucher.code, order)
-      # Manually add voucher tax adjustment and metadata, so we don't have to do a big data setup
-      # to be able to use VoucherAdjustmentsService.update
-      tax_adjustment = order.adjustments.create!(
-        originator: voucher_adjustment.originator,
-        order: order,
-        label: "Tax #{voucher_adjustment.label}",
-        mandatory: false,
-        state: 'open',
-        tax_category: nil,
-        included_tax: 0,
-        amount: 0.5
-      )
-      AdjustmentMetadata.create(
-        adjustment: tax_adjustment,
-        enterprise: tax_adjustment.originator.enterprise,
-        fee_name: "Tax",
-        fee_type: "Voucher"
-      )
+      add_voucher(order, voucher)
 
-      expect(voucher_excluded_tax).to eq(0.5)
+      VoucherAdjustmentsService.new(order).update
+
+      expect(voucher_excluded_tax).to eq(-0.64)
     end
 
     context "when no voucher adjustment tax" do
@@ -372,4 +342,13 @@ describe VoucherAdjustmentsService do
       end
     end
   end
+
+  def add_voucher(order, voucher)
+    voucher.create_adjustment(voucher.code, order)
+
+    # Update taxes
+    order.create_tax_charge!
+    order.update_shipping_fees!
+    order.update_order!
+  end
 end

From 33f3c660a4b5f68618dd58f0916770a9da68b9a4 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 26 Sep 2023 11:21:05 +0200
Subject: [PATCH 19/19] Fix rubocop warning

---
 .../reports/sales_tax_totals_by_order_spec.rb | 20 ++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/spec/lib/reports/sales_tax_totals_by_order_spec.rb b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
index 98d46e8c8a..7fea0dfa12 100644
--- a/spec/lib/reports/sales_tax_totals_by_order_spec.rb
+++ b/spec/lib/reports/sales_tax_totals_by_order_spec.rb
@@ -196,15 +196,17 @@ describe "Reporting::Reports::SalesTax::SalesTaxTotalsByOrder" do
         item = [[state_tax_rate.id, order.id], order]
         summary_row = rules.third[:summary_row].call(nil, [item], nil)
 
-        expect(summary_row).to eq({
-          total_excl_tax: 110.00,
-          tax: 3.3,
-          total_incl_tax: 113.30,
-          first_name: "cfname",
-          last_name: "clname",
-          code: "ABC123",
-          email: "order1@example.com"
-        })
+        expect(summary_row).to eq(
+          {
+            total_excl_tax: 110.00,
+            tax: 3.3,
+            total_incl_tax: 113.30,
+            first_name: "cfname",
+            last_name: "clname",
+            code: "ABC123",
+            email: "order1@example.com"
+          }
+        )
       end
 
       context "with a voucher" do