From 33feb7f63f4e6f673e7625641ca1637a17ae263e Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 30 Aug 2022 11:10:40 +1000
Subject: [PATCH 1/6] Test for explicit values, not reusing faulty code

---
 spec/system/admin/order_print_ticket_spec.rb | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/spec/system/admin/order_print_ticket_spec.rb b/spec/system/admin/order_print_ticket_spec.rb
index 18c2c2aede..0024a93478 100644
--- a/spec/system/admin/order_print_ticket_spec.rb
+++ b/spec/system/admin/order_print_ticket_spec.rb
@@ -92,10 +92,7 @@ describe '
       end
 
       def taxes_in_print_data
-        display_checkout_taxes_hash(order).map { |tax_rate, tax_value|
-          [tax_rate,
-           tax_value.format(with_currency: false)]
-        }
+        [["10.0%", "$11.00"]]
       end
     end
   end

From f4466b1aad3d12a836e55abd58314d8858bd82da Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 30 Aug 2022 11:51:30 +1000
Subject: [PATCH 2/6] Spec problem with tax rate summary display

---
 spec/helpers/checkout_helper_spec.rb | 58 ++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb
index 41bacae7bc..b09d7d01a9 100644
--- a/spec/helpers/checkout_helper_spec.rb
+++ b/spec/helpers/checkout_helper_spec.rb
@@ -24,6 +24,64 @@ describe CheckoutHelper, type: :helper do
     end
   end
 
+  describe "#display_checkout_taxes_hash" do
+    let(:order) { build(:order_with_totals) }
+    let(:tax_rate10) { build(:tax_rate, amount: 0.1) }
+    let(:tax_rate20) { build(:tax_rate, amount: 0.2) }
+    let(:other_tax_rate20) { build(:tax_rate, amount: 0.2) }
+    let(:adjustment1) {
+      build(:adjustment, amount: 1, label: "10% tax", originator: tax_rate10)
+    }
+    let(:adjustment2) {
+      build(:adjustment, amount: 2, label: "20% tax", originator: tax_rate20)
+    }
+    let(:other_adjustment2) {
+      build(:adjustment, amount: 2, label: "20% tax", originator: other_tax_rate20)
+    }
+
+    it "produces an empty hash without taxes" do
+      expect(helper.display_checkout_taxes_hash(order)).to eq({})
+    end
+
+    it "shows a single tax adjustment" do
+      order.all_adjustments << adjustment1
+      order.save!
+
+      expect(helper.display_checkout_taxes_hash(order)).to eq(
+        "10.0%" => Spree::Money.new(1, currency: order.currency)
+      )
+    end
+
+    it "shows multiple tax adjustments" do
+      order.all_adjustments << adjustment1
+      order.all_adjustments << adjustment2
+      order.save!
+
+      expect(helper.display_checkout_taxes_hash(order)).to eq(
+        "10.0%" => Spree::Money.new(1, currency: order.currency),
+        "20.0%" => Spree::Money.new(2, currency: order.currency),
+      )
+    end
+
+    it "shows multiple tax adjustments with same percentage" do
+      order.all_adjustments << adjustment2
+      order.all_adjustments << other_adjustment2
+      order.save!
+
+      # This passes because we override the hash entry exactly
+      # like the original code.
+      expect(helper.display_checkout_taxes_hash(order)).to eq(
+        "20.0%" => Spree::Money.new(2, currency: order.currency),
+        "20.0%" => Spree::Money.new(2, currency: order.currency),
+      )
+
+      pending "https://github.com/openfoodfoundation/openfoodnetwork/issues/9605"
+
+      # This fails. We got only one result when we should have two.
+      expect(helper.display_checkout_taxes_hash(order).size).to eq 2
+    end
+  end
+
   it "knows if guests can checkout" do
     distributor = create(:distributor_enterprise)
     order = create(:order, distributor: distributor)

From f37b681facebe2ed56d253ff85536b942b32b2e7 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 30 Aug 2022 17:58:45 +1000
Subject: [PATCH 3/6] Allow more tax data in the summary

I converted the value to a hash so that we can put all information in
there and we won't need the current hash key as percentage any more.
---
 app/helpers/checkout_helper.rb                       |  4 +++-
 .../spree/admin/orders/_invoice_table2.html.haml     |  4 ++--
 app/views/spree/admin/orders/ticket.html.haml        |  4 ++--
 spec/helpers/checkout_helper_spec.rb                 | 12 +++++++-----
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb
index 780c27c595..1ba2097d80 100644
--- a/app/helpers/checkout_helper.rb
+++ b/app/helpers/checkout_helper.rb
@@ -72,7 +72,9 @@ module CheckoutHelper
 
     totals.each_with_object({}) do |(tax_rate, tax_amount), hash|
       hash[number_to_percentage(tax_rate.amount * 100, precision: 1)] =
-        Spree::Money.new tax_amount, currency: order.currency
+        {
+          amount: Spree::Money.new(tax_amount, currency: order.currency),
+        }
     end
   end
 
diff --git a/app/views/spree/admin/orders/_invoice_table2.html.haml b/app/views/spree/admin/orders/_invoice_table2.html.haml
index eb83714747..ad2c97977d 100644
--- a/app/views/spree/admin/orders/_invoice_table2.html.haml
+++ b/app/views/spree/admin/orders/_invoice_table2.html.haml
@@ -47,12 +47,12 @@
         %strong= @order.has_taxes_included ? t(:total_incl_tax) : t(:total_excl_tax)
       %td{:align => "right", :colspan => "2"}
         %strong= @order.has_taxes_included ? @order.display_total : display_checkout_total_less_tax(@order)
-    - display_checkout_taxes_hash(@order).each do |tax_rate, tax_value|
+    - display_checkout_taxes_hash(@order).each do |tax_rate, tax|
       %tr
         %td{:align => "right", :colspan => "3"}
           = t(:tax_total, rate: tax_rate)
         %td{:align => "right", :colspan => "2"}
-          = tax_value
+          = tax[:amount]
     %tr
       %td{:align => "right", :colspan => "3"}
         = @order.has_taxes_included ? t(:total_excl_tax) : t(:total_incl_tax)
diff --git a/app/views/spree/admin/orders/ticket.html.haml b/app/views/spree/admin/orders/ticket.html.haml
index 415fb97adb..07810ca7d4 100644
--- a/app/views/spree/admin/orders/ticket.html.haml
+++ b/app/views/spree/admin/orders/ticket.html.haml
@@ -58,10 +58,10 @@
       j(@order.display_total.format(with_currency: false))]}",
       '\x1B' + '\x45' + '\x0A', // bold off
       '\x0A',
-      "#{display_checkout_taxes_hash(@order).map { |tax_rate, tax_value|
+      "#{display_checkout_taxes_hash(@order).map { |tax_rate, tax|
       '%31s%10s' %
       [j(t(:tax_total, rate: tax_rate)),
-      j(tax_value.format(with_currency: false))] +
+      j(tax[:amount].format(with_currency: false))] +
       '" + \'\x0A\' + "'}.join }",
       "#{'%31s%10s' %
       [j(t(:total_excl_tax)),
diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb
index b09d7d01a9..6851383879 100644
--- a/spec/helpers/checkout_helper_spec.rb
+++ b/spec/helpers/checkout_helper_spec.rb
@@ -48,7 +48,9 @@ describe CheckoutHelper, type: :helper do
       order.save!
 
       expect(helper.display_checkout_taxes_hash(order)).to eq(
-        "10.0%" => Spree::Money.new(1, currency: order.currency)
+        "10.0%" => {
+          amount: Spree::Money.new(1, currency: order.currency),
+        }
       )
     end
 
@@ -58,8 +60,8 @@ describe CheckoutHelper, type: :helper do
       order.save!
 
       expect(helper.display_checkout_taxes_hash(order)).to eq(
-        "10.0%" => Spree::Money.new(1, currency: order.currency),
-        "20.0%" => Spree::Money.new(2, currency: order.currency),
+        "10.0%" => { amount: Spree::Money.new(1, currency: order.currency) },
+        "20.0%" => { amount: Spree::Money.new(2, currency: order.currency) },
       )
     end
 
@@ -71,8 +73,8 @@ describe CheckoutHelper, type: :helper do
       # This passes because we override the hash entry exactly
       # like the original code.
       expect(helper.display_checkout_taxes_hash(order)).to eq(
-        "20.0%" => Spree::Money.new(2, currency: order.currency),
-        "20.0%" => Spree::Money.new(2, currency: order.currency),
+        "20.0%" => { amount: Spree::Money.new(2, currency: order.currency) },
+        "20.0%" => { amount: Spree::Money.new(2, currency: order.currency) },
       )
 
       pending "https://github.com/openfoodfoundation/openfoodnetwork/issues/9605"

From 2a57dc976645a990af666688cf918140036a49ea Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 31 Aug 2022 16:34:37 +1000
Subject: [PATCH 4/6] Index tax totals correctly for invoice display

---
 app/helpers/checkout_helper.rb                |  3 +-
 .../admin/orders/_invoice_table2.html.haml    |  2 +-
 app/views/spree/admin/orders/ticket.html.haml |  2 +-
 spec/helpers/checkout_helper_spec.rb          | 28 ++++++++++++-------
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb
index 1ba2097d80..a4fd0b13f1 100644
--- a/app/helpers/checkout_helper.rb
+++ b/app/helpers/checkout_helper.rb
@@ -71,9 +71,10 @@ module CheckoutHelper
     totals = OrderTaxAdjustmentsFetcher.new(order).totals
 
     totals.each_with_object({}) do |(tax_rate, tax_amount), hash|
-      hash[number_to_percentage(tax_rate.amount * 100, precision: 1)] =
+      hash[tax_rate] =
         {
           amount: Spree::Money.new(tax_amount, currency: order.currency),
+          percentage: number_to_percentage(tax_rate.amount * 100, precision: 1),
         }
     end
   end
diff --git a/app/views/spree/admin/orders/_invoice_table2.html.haml b/app/views/spree/admin/orders/_invoice_table2.html.haml
index ad2c97977d..e94485afc7 100644
--- a/app/views/spree/admin/orders/_invoice_table2.html.haml
+++ b/app/views/spree/admin/orders/_invoice_table2.html.haml
@@ -50,7 +50,7 @@
     - display_checkout_taxes_hash(@order).each do |tax_rate, tax|
       %tr
         %td{:align => "right", :colspan => "3"}
-          = t(:tax_total, rate: tax_rate)
+          = t(:tax_total, rate: tax[:percentage])
         %td{:align => "right", :colspan => "2"}
           = tax[:amount]
     %tr
diff --git a/app/views/spree/admin/orders/ticket.html.haml b/app/views/spree/admin/orders/ticket.html.haml
index 07810ca7d4..9cf318a409 100644
--- a/app/views/spree/admin/orders/ticket.html.haml
+++ b/app/views/spree/admin/orders/ticket.html.haml
@@ -60,7 +60,7 @@
       '\x0A',
       "#{display_checkout_taxes_hash(@order).map { |tax_rate, tax|
       '%31s%10s' %
-      [j(t(:tax_total, rate: tax_rate)),
+      [j(t(:tax_total, rate: tax[:percentage])),
       j(tax[:amount].format(with_currency: false))] +
       '" + \'\x0A\' + "'}.join }",
       "#{'%31s%10s' %
diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb
index 6851383879..c1c5eada0a 100644
--- a/spec/helpers/checkout_helper_spec.rb
+++ b/spec/helpers/checkout_helper_spec.rb
@@ -48,8 +48,9 @@ describe CheckoutHelper, type: :helper do
       order.save!
 
       expect(helper.display_checkout_taxes_hash(order)).to eq(
-        "10.0%" => {
+        tax_rate10 => {
           amount: Spree::Money.new(1, currency: order.currency),
+          percentage: "10.0%",
         }
       )
     end
@@ -60,8 +61,14 @@ describe CheckoutHelper, type: :helper do
       order.save!
 
       expect(helper.display_checkout_taxes_hash(order)).to eq(
-        "10.0%" => { amount: Spree::Money.new(1, currency: order.currency) },
-        "20.0%" => { amount: Spree::Money.new(2, currency: order.currency) },
+        tax_rate10 => {
+          amount: Spree::Money.new(1, currency: order.currency),
+          percentage: "10.0%",
+        },
+        tax_rate20 => {
+          amount: Spree::Money.new(2, currency: order.currency),
+          percentage: "20.0%",
+        },
       )
     end
 
@@ -70,16 +77,17 @@ describe CheckoutHelper, type: :helper do
       order.all_adjustments << other_adjustment2
       order.save!
 
-      # This passes because we override the hash entry exactly
-      # like the original code.
       expect(helper.display_checkout_taxes_hash(order)).to eq(
-        "20.0%" => { amount: Spree::Money.new(2, currency: order.currency) },
-        "20.0%" => { amount: Spree::Money.new(2, currency: order.currency) },
+        tax_rate20 => {
+          amount: Spree::Money.new(2, currency: order.currency),
+          percentage: "20.0%",
+        },
+        other_tax_rate20 => {
+          amount: Spree::Money.new(2, currency: order.currency),
+          percentage: "20.0%",
+        },
       )
 
-      pending "https://github.com/openfoodfoundation/openfoodnetwork/issues/9605"
-
-      # This fails. We got only one result when we should have two.
       expect(helper.display_checkout_taxes_hash(order).size).to eq 2
     end
   end

From 91eaa522cf098c3fa76afd4acad53d8578b278a0 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 31 Aug 2022 16:46:52 +1000
Subject: [PATCH 5/6] Simplify tax totals helper

---
 app/helpers/checkout_helper.rb                | 11 ++++----
 .../admin/orders/_invoice_table2.html.haml    |  2 +-
 app/views/spree/admin/orders/ticket.html.haml |  2 +-
 spec/helpers/checkout_helper_spec.rb          | 26 +++++++++----------
 4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb
index a4fd0b13f1..51f58bf52e 100644
--- a/app/helpers/checkout_helper.rb
+++ b/app/helpers/checkout_helper.rb
@@ -70,12 +70,11 @@ module CheckoutHelper
   def display_checkout_taxes_hash(order)
     totals = OrderTaxAdjustmentsFetcher.new(order).totals
 
-    totals.each_with_object({}) do |(tax_rate, tax_amount), hash|
-      hash[tax_rate] =
-        {
-          amount: Spree::Money.new(tax_amount, currency: order.currency),
-          percentage: number_to_percentage(tax_rate.amount * 100, precision: 1),
-        }
+    totals.map do |tax_rate, tax_amount|
+      {
+        amount: Spree::Money.new(tax_amount, currency: order.currency),
+        percentage: number_to_percentage(tax_rate.amount * 100, precision: 1),
+      }
     end
   end
 
diff --git a/app/views/spree/admin/orders/_invoice_table2.html.haml b/app/views/spree/admin/orders/_invoice_table2.html.haml
index e94485afc7..c9a19b7956 100644
--- a/app/views/spree/admin/orders/_invoice_table2.html.haml
+++ b/app/views/spree/admin/orders/_invoice_table2.html.haml
@@ -47,7 +47,7 @@
         %strong= @order.has_taxes_included ? t(:total_incl_tax) : t(:total_excl_tax)
       %td{:align => "right", :colspan => "2"}
         %strong= @order.has_taxes_included ? @order.display_total : display_checkout_total_less_tax(@order)
-    - display_checkout_taxes_hash(@order).each do |tax_rate, tax|
+    - display_checkout_taxes_hash(@order).each do |tax|
       %tr
         %td{:align => "right", :colspan => "3"}
           = t(:tax_total, rate: tax[:percentage])
diff --git a/app/views/spree/admin/orders/ticket.html.haml b/app/views/spree/admin/orders/ticket.html.haml
index 9cf318a409..d86ba0d1b6 100644
--- a/app/views/spree/admin/orders/ticket.html.haml
+++ b/app/views/spree/admin/orders/ticket.html.haml
@@ -58,7 +58,7 @@
       j(@order.display_total.format(with_currency: false))]}",
       '\x1B' + '\x45' + '\x0A', // bold off
       '\x0A',
-      "#{display_checkout_taxes_hash(@order).map { |tax_rate, tax|
+      "#{display_checkout_taxes_hash(@order).map { |tax|
       '%31s%10s' %
       [j(t(:tax_total, rate: tax[:percentage])),
       j(tax[:amount].format(with_currency: false))] +
diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb
index c1c5eada0a..2bc56b83e6 100644
--- a/spec/helpers/checkout_helper_spec.rb
+++ b/spec/helpers/checkout_helper_spec.rb
@@ -39,20 +39,20 @@ describe CheckoutHelper, type: :helper do
       build(:adjustment, amount: 2, label: "20% tax", originator: other_tax_rate20)
     }
 
-    it "produces an empty hash without taxes" do
-      expect(helper.display_checkout_taxes_hash(order)).to eq({})
+    it "produces an empty array without taxes" do
+      expect(helper.display_checkout_taxes_hash(order)).to eq([])
     end
 
     it "shows a single tax adjustment" do
       order.all_adjustments << adjustment1
       order.save!
 
-      expect(helper.display_checkout_taxes_hash(order)).to eq(
-        tax_rate10 => {
+      expect(helper.display_checkout_taxes_hash(order)).to eq [
+        {
           amount: Spree::Money.new(1, currency: order.currency),
           percentage: "10.0%",
         }
-      )
+      ]
     end
 
     it "shows multiple tax adjustments" do
@@ -60,16 +60,16 @@ describe CheckoutHelper, type: :helper do
       order.all_adjustments << adjustment2
       order.save!
 
-      expect(helper.display_checkout_taxes_hash(order)).to eq(
-        tax_rate10 => {
+      expect(helper.display_checkout_taxes_hash(order)).to match_array [
+        {
           amount: Spree::Money.new(1, currency: order.currency),
           percentage: "10.0%",
         },
-        tax_rate20 => {
+        {
           amount: Spree::Money.new(2, currency: order.currency),
           percentage: "20.0%",
         },
-      )
+      ]
     end
 
     it "shows multiple tax adjustments with same percentage" do
@@ -77,16 +77,16 @@ describe CheckoutHelper, type: :helper do
       order.all_adjustments << other_adjustment2
       order.save!
 
-      expect(helper.display_checkout_taxes_hash(order)).to eq(
-        tax_rate20 => {
+      expect(helper.display_checkout_taxes_hash(order)).to match_array [
+        {
           amount: Spree::Money.new(2, currency: order.currency),
           percentage: "20.0%",
         },
-        other_tax_rate20 => {
+        {
           amount: Spree::Money.new(2, currency: order.currency),
           percentage: "20.0%",
         },
-      )
+      ]
 
       expect(helper.display_checkout_taxes_hash(order).size).to eq 2
     end

From 326b0af35ff3955f308abf70ea301646255eecf6 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Thu, 1 Sep 2022 11:19:02 +1000
Subject: [PATCH 6/6] Sort tax total by percentage on invoice

---
 app/helpers/checkout_helper.rb       |  3 ++-
 spec/helpers/checkout_helper_spec.rb | 28 ++++++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb
index 51f58bf52e..c4754e667e 100644
--- a/app/helpers/checkout_helper.rb
+++ b/app/helpers/checkout_helper.rb
@@ -74,8 +74,9 @@ module CheckoutHelper
       {
         amount: Spree::Money.new(tax_amount, currency: order.currency),
         percentage: number_to_percentage(tax_rate.amount * 100, precision: 1),
+        rate_amount: tax_rate.amount,
       }
-    end
+    end.sort_by { |tax| tax[:rate_amount] }
   end
 
   def display_line_item_tax_rates(line_item)
diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb
index 2bc56b83e6..ef64fec69a 100644
--- a/spec/helpers/checkout_helper_spec.rb
+++ b/spec/helpers/checkout_helper_spec.rb
@@ -51,6 +51,7 @@ describe CheckoutHelper, type: :helper do
         {
           amount: Spree::Money.new(1, currency: order.currency),
           percentage: "10.0%",
+          rate_amount: 0.1,
         }
       ]
     end
@@ -60,14 +61,35 @@ describe CheckoutHelper, type: :helper do
       order.all_adjustments << adjustment2
       order.save!
 
-      expect(helper.display_checkout_taxes_hash(order)).to match_array [
+      expect(helper.display_checkout_taxes_hash(order)).to eq [
         {
           amount: Spree::Money.new(1, currency: order.currency),
           percentage: "10.0%",
+          rate_amount: 0.1,
         },
         {
           amount: Spree::Money.new(2, currency: order.currency),
           percentage: "20.0%",
+          rate_amount: 0.2,
+        },
+      ]
+    end
+
+    it "sorts adjustments by percentage" do
+      order.all_adjustments << adjustment2
+      order.all_adjustments << adjustment1
+      order.save!
+
+      expect(helper.display_checkout_taxes_hash(order)).to eq [
+        {
+          amount: Spree::Money.new(1, currency: order.currency),
+          percentage: "10.0%",
+          rate_amount: 0.1,
+        },
+        {
+          amount: Spree::Money.new(2, currency: order.currency),
+          percentage: "20.0%",
+          rate_amount: 0.2,
         },
       ]
     end
@@ -77,14 +99,16 @@ describe CheckoutHelper, type: :helper do
       order.all_adjustments << other_adjustment2
       order.save!
 
-      expect(helper.display_checkout_taxes_hash(order)).to match_array [
+      expect(helper.display_checkout_taxes_hash(order)).to eq [
         {
           amount: Spree::Money.new(2, currency: order.currency),
           percentage: "20.0%",
+          rate_amount: 0.2,
         },
         {
           amount: Spree::Money.new(2, currency: order.currency),
           percentage: "20.0%",
+          rate_amount: 0.2,
         },
       ]