Merge pull request #11543 from rioug/11364-Voucher-fix-tax-report

[Vouchers] Fix Sales Tax Totals By Order report

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) }

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.voucher_tax.empty?
+
+    @order.voucher_adjustments.voucher_tax.first.amount
+  end
+
+  private
+
   def handle_tax_excluded_from_price(voucher)
     voucher_rate = voucher.rate(@order)
     adjustment = @order.voucher_adjustments.first
@@ -64,6 +83,14 @@ 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
+
+    # Add metada so we know which voucher adjustment is Tax related
+    tax_adjustment.metadata ||= AdjustmentMetadata.new(
+      enterprise: adjustment.originator.enterprise,
+      fee_name: "Tax",
+      fee_type: "Voucher"
+    )
+
     tax_adjustment.save
   end
 

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,38 @@ 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 = filtered_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 +
+            filtered_tax_rate_total(query_result_row)
+        end
+
+        # 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))
             .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)
+          VoucherAdjustmentsService.new(order).voucher_included_tax +
+            VoucherAdjustmentsService.new(order).voucher_excluded_tax
         end
 
         def first_name(query_result_row)

spec/lib/reports/sales_tax_totals_by_order_spec.rb

@@ -0,0 +1,291 @@
+# 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, 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
+    ).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
+    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
+  let(:query_row) do
+    [
+      [state_tax_rate.id, order.id],
+      order
+    ]
+  end
+
+  before do
+    product.update!(supplier_id: supplier.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
+
+    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)
+
+      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
+    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)
+
+      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)
+        mock_voucher_adjustment_service(excluded_tax: -0.29)
+
+        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
+    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)
+
+      # 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)
+        mock_voucher_adjustment_service(excluded_tax: -0.29)
+
+        total = report.total_excl_tax(query_row)
+
+        # 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
+    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)
+
+      # 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
+      mock_voucher_adjustment_service
+
+      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
+        mock_voucher_adjustment_service
+
+        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,
+            first_name: "cfname",
+            last_name: "clname",
+            code: "ABC123",
+            email: "order1@example.com"
+          }
+        )
+      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)
+          mock_voucher_adjustment_service(excluded_tax: -0.29)
+
+          # 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
+
+  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
+
+    # 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
+
+  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

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
@@ -136,6 +124,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
@@ -214,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
@@ -248,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
@@ -294,4 +276,79 @@ 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) 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
+      add_voucher(order, voucher)
+
+      VoucherAdjustmentsService.new(order).update
+
+      expect(voucher_excluded_tax).to eq(-0.64)
+    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
+
+  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

spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb

@@ -104,6 +104,8 @@ describe "Sales Tax Totals By order" do
     end
 
     it "generates the report" do
+      # 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"
@@ -161,14 +163,13 @@ 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 })
+    end
 
+    it "generates the report" 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 +335,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 +466,14 @@ describe "Sales Tax Totals By order" do
     end
   end
 
+  def visit_sales_tax_totals_by_order
+    login_as admin
+    visit admin_report_path(
+      report_type: :sales_tax,
+      report_subtype: :sales_tax_totals_by_order
+    )
+  end
+
   def generate_report
     click_on "Go"
     wait_for_download