From 3d44f4bc98c5f7a82e09da91795781e878e690b6 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 1 Jun 2023 19:09:54 +0100
Subject: [PATCH 01/28] Introduce "zero priced order" concept with no required
 payment

This applies to cases where an order contains items with zero price or where applied vouchers have reduced the order's total to zero.
---
 app/models/spree/order.rb                       |  8 +++++++-
 app/views/spree/shared/_order_details.html.haml | 14 +++++++++-----
 config/locales/en.yml                           |  1 +
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 91c70e8c84..d87b14b3bb 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -23,7 +23,7 @@ module Spree
       go_to_state :delivery
       go_to_state :payment, if: ->(order) {
         order.update_totals
-        order.payment_required?
+        order.payment_required? || order.zero_priced_order?
       }
       go_to_state :confirmation
       go_to_state :complete
@@ -219,6 +219,12 @@ module Spree
       total.to_f > 0.0 && !skip_payment_for_subscription?
     end
 
+    # There are items present in the order, but either the items have zero price,
+    # or the order's total has been modified (maybe discounted) to zero.
+    def zero_priced_order?
+      valid? && line_items.count.positive? && total.zero?
+    end
+
     # Returns the relevant zone (if any) to be used for taxation purposes.
     # Uses default tax zone unless there is a specific match
     def tax_zone
diff --git a/app/views/spree/shared/_order_details.html.haml b/app/views/spree/shared/_order_details.html.haml
index b559074bf5..b3e5650984 100644
--- a/app/views/spree/shared/_order_details.html.haml
+++ b/app/views/spree/shared/_order_details.html.haml
@@ -11,11 +11,15 @@
         %strong
           = order.display_total.to_html
     .pad
-      .text-big
-        = t :order_payment
-        %strong=  last_payment_method(order)&.name
-      %p.text-small.text-skinny.pre-line.word-wrap
-        %em= last_payment_method(order)&.description
+      - if (order_payment_method = last_payment_method(order))
+        .text-big
+          = t :order_payment
+          %strong= order_payment_method&.name
+        %p.text-small.text-skinny.pre-line.word-wrap
+          %em= order_payment_method&.description
+      - else
+        .text-big
+          = t(:no_payment_required)
 
     .order-summary.text-small
       %strong
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 22acda77a9..5f39fb8b57 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -2120,6 +2120,7 @@ en:
   order_not_paid: NOT PAID
   order_total: Total order
   order_payment: "Paying via:"
+  no_payment_required: "No payment required"
   order_billing_address: Billing address
   order_delivery_on: Delivery on
   order_delivery_address: Delivery address

From 75e6ccd580d46e48ad9eaacbdd17b47f6b24f1ad Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 1 Jun 2023 19:11:16 +0100
Subject: [PATCH 02/28] Simplify voucher controller

---
 app/controllers/admin/vouchers_controller.rb | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/app/controllers/admin/vouchers_controller.rb b/app/controllers/admin/vouchers_controller.rb
index efd9ac9b7d..3f190a8001 100644
--- a/app/controllers/admin/vouchers_controller.rb
+++ b/app/controllers/admin/vouchers_controller.rb
@@ -9,14 +9,11 @@ module Admin
     end
 
     def create
-      voucher_params = permitted_resource_params.merge(enterprise: @enterprise)
-      @voucher = Voucher.create(voucher_params)
+      @voucher = Voucher.create(permitted_resource_params.merge(enterprise: @enterprise))
 
       if @voucher.save
-        redirect_to(
-          "#{edit_admin_enterprise_path(@enterprise)}#vouchers_panel",
-          flash: { success: flash_message_for(@voucher, :successfully_created) }
-        )
+        flash[:success] = flash_message_for(@voucher, :successfully_created)
+        redirect_to edit_admin_enterprise_path(@enterprise, anchor: :vouchers_panel)
       else
         flash[:error] = @voucher.errors.full_messages.to_sentence
         render :new

From e04a3f6b4d5cb0bc386f96cc57c7c68f24974fa8 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 00:12:33 +0100
Subject: [PATCH 03/28] Move form definition down into each checkout step

---
 app/views/split_checkout/_details.html.haml | 269 ++++++++++----------
 app/views/split_checkout/_form.html.haml    |   7 +-
 app/views/split_checkout/_payment.html.haml |  67 ++---
 app/views/split_checkout/_summary.html.haml | 181 ++++++-------
 4 files changed, 262 insertions(+), 262 deletions(-)

diff --git a/app/views/split_checkout/_details.html.haml b/app/views/split_checkout/_details.html.haml
index e81c8c08a7..53b4e29fdd 100644
--- a/app/views/split_checkout/_details.html.haml
+++ b/app/views/split_checkout/_details.html.haml
@@ -1,155 +1,156 @@
-.medium-6
-  = f.fields :bill_address, model: @order.bill_address do |bill_address|
-    %div.checkout-substep
-      -# YOUR DETAILS
-      %div.checkout-title
-        = t("split_checkout.step1.contact_information.title")
+= form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
+  .medium-6
+    = f.fields :bill_address, model: @order.bill_address do |bill_address|
+      %div.checkout-substep
+        -# YOUR DETAILS
+        %div.checkout-title
+          = t("split_checkout.step1.contact_information.title")
 
-      .two-columns-inputs
-        %div.checkout-input.with-floating-label{ "data-controller": "floating-label" }
-          = f.label :email, t("split_checkout.step1.contact_information.email.label")
-          = f.text_field :email, { placeholder: " " }
-          = f.error_message_on :email
+        .two-columns-inputs
+          %div.checkout-input.with-floating-label{ "data-controller": "floating-label" }
+            = f.label :email, t("split_checkout.step1.contact_information.email.label")
+            = f.text_field :email, { placeholder: " " }
+            = f.error_message_on :email
 
-        %div.checkout-input.with-floating-label{ "data-controller": "floating-label" }
-          = bill_address.label :phone, t("split_checkout.step1.contact_information.phone.label")
-          = bill_address.text_field :phone, { placeholder: " " }
-          = f.error_message_on "bill_address.phone"
+          %div.checkout-input.with-floating-label{ "data-controller": "floating-label" }
+            = bill_address.label :phone, t("split_checkout.step1.contact_information.phone.label")
+            = bill_address.text_field :phone, { placeholder: " " }
+            = f.error_message_on "bill_address.phone"
 
-    %div.checkout-substep
-      -# BILLING ADDRESS
-      %div.checkout-title
-        = t("split_checkout.step1.billing_address.title")
+      %div.checkout-substep
+        -# BILLING ADDRESS
+        %div.checkout-title
+          = t("split_checkout.step1.billing_address.title")
 
-      .two-columns-inputs
-        %div.checkout-input.with-floating-label{ "data-controller": "floating-label" }
-          = bill_address.label :firstname, t("split_checkout.step1.billing_address.first_name.label")
-          = bill_address.text_field :firstname, { placeholder: " " }
-          = f.error_message_on "bill_address.firstname"
+        .two-columns-inputs
+          %div.checkout-input.with-floating-label{ "data-controller": "floating-label" }
+            = bill_address.label :firstname, t("split_checkout.step1.billing_address.first_name.label")
+            = bill_address.text_field :firstname, { placeholder: " " }
+            = f.error_message_on "bill_address.firstname"
 
-        %div.checkout-input.with-floating-label{ "data-controller": "floating-label" }
-          = bill_address.label :lastname, t("split_checkout.step1.billing_address.last_name.label")
-          = bill_address.text_field :lastname, { placeholder: " " }
-          = f.error_message_on "bill_address.lastname"
-
-      %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
-        = bill_address.label :address1, t("split_checkout.step1.address.address1.label")
-        = bill_address.text_field :address1, { placeholder: " " }
-        = f.error_message_on "bill_address.address1"
-
-      %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
-        = bill_address.label :address2, t("split_checkout.step1.address.address2.label")
-        = bill_address.text_field :address2, { placeholder: " " }
-        = f.error_message_on "bill_address.address2"
-
-      %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
-        = bill_address.label :city, t("split_checkout.step1.address.city.label")
-        = bill_address.text_field :city, { placeholder: " " }
-        = f.error_message_on "bill_address.city"
-
-      %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
-        = bill_address.label :zipcode, t("split_checkout.step1.address.zipcode.label")
-        = bill_address.text_field :zipcode, { placeholder: " " }
-        = f.error_message_on "bill_address.zipcode"
-
-      %div{ "data-controller": "dependent-select", "data-dependent-select-options-value": countries_with_states }
-        - bill_address_country = @order.bill_address.country || DefaultCountry.country
-
-        %div.checkout-input
-          = bill_address.label :country_id, t("split_checkout.step1.address.country_id.label")
-          = bill_address.select :country_id, countries, { selected: bill_address_country.id }, { "data-dependent-select-target": "source", "data-action": "dependent-select#handleSelectChange" }
-
-        %div.checkout-input
-          = bill_address.label :state_id, t("split_checkout.step1.address.state_id.label")
-          = bill_address.select :state_id, states_for_country(bill_address_country), { selected: @order.bill_address&.state_id }, { "data-dependent-select-target": "select" }
-
-      - if spree_current_user
-        %div.checkout-input
-          = f.check_box :save_bill_address
-          = f.label :save_bill_address, t(:checkout_default_bill_address)
-
-  %div.checkout-substep{ "data-controller": "toggle shippingmethod" }
-    - selected_shipping_method = @order.shipping_method&.id || params[:shipping_method_id]
-    %div.checkout-title
-      = t("split_checkout.step1.shipping_info.title")
-
-    - display_ship_address = false
-    - ship_method_description = nil
-
-    - selected_shipping_method ||= @shipping_methods[0].id if @shipping_methods.length == 1
-    - @shipping_methods.each do |shipping_method|
-      - ship_method_is_selected = shipping_method.id == selected_shipping_method.to_i
-      %div.checkout-input.checkout-input-radio
-        = fields_for shipping_method do |shipping_method_form|
-          = shipping_method_form.radio_button :name, shipping_method.id,
-            id: "shipping_method_#{shipping_method.id}",
-            checked: ship_method_is_selected,
-            name: "shipping_method_id",
-            "data-requireAddress": shipping_method.require_ship_address,
-            "data-action": "toggle#toggle shippingmethod#selectShippingMethod",
-            "data-toggle-show": shipping_method.require_ship_address
-          = shipping_method_form.label shipping_method.id, shipping_method.name, {for: "shipping_method_" + shipping_method.id.to_s }
-          %em.fees= payment_or_shipping_price(shipping_method, @order)
-          - display_ship_address = display_ship_address || (ship_method_is_selected && shipping_method.require_ship_address)
-      %div.checkout-input{"data-shippingmethod-target": "shippingMethodDescription", "data-shippingmethodid": shipping_method.id , style: "display: #{ship_method_is_selected ? 'block' : 'none'}" }
-        #distributor_address.panel
-          - if shipping_method.description.present?
-            %span #{shipping_method.description}
-            %br/
-            %br/
-          - if @order.order_cycle.pickup_time_for(@order.distributor)
-            = t :checkout_ready_for
-          = @order.order_cycle.pickup_time_for(@order.distributor)
-      
-    = f.error_message_on :shipping_method, standalone: true
-    
-    %div.checkout-input{ "data-toggle-target": "content", style: "display: #{display_ship_address ? 'block' : 'none'}" }
-      = f.check_box :ship_address_same_as_billing, { id: "ship_address_same_as_billing", name: "ship_address_same_as_billing", "data-action": "shippingmethod#showHideShippingAddress", "data-shippingmethod-target": "shippingAddressCheckbox", checked: shipping_and_billing_match?(@order) }, 1, nil
-      = f.label :ship_address_same_as_billing, t(:checkout_address_same), { for: "ship_address_same_as_billing" }
-
-    %div{"data-shippingmethod-target": "shippingMethodAddress", style: "display: #{!display_ship_address || shipping_and_billing_match?(@order) ? 'none' : 'block'}" }
-      = f.fields :ship_address, model: @order.ship_address do |ship_address|
-        %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
-          = ship_address.label :address1, t("split_checkout.step1.address.address1.label")
-          = ship_address.text_field :address1, { placeholder: " " }
-          = f.error_message_on "ship_address.address1"
+          %div.checkout-input.with-floating-label{ "data-controller": "floating-label" }
+            = bill_address.label :lastname, t("split_checkout.step1.billing_address.last_name.label")
+            = bill_address.text_field :lastname, { placeholder: " " }
+            = f.error_message_on "bill_address.lastname"
 
         %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
-          = ship_address.label :address2, t("split_checkout.step1.address.address2.label")
-          = ship_address.text_field :address2, { placeholder: " " }
-          = f.error_message_on "ship_address.address2"
+          = bill_address.label :address1, t("split_checkout.step1.address.address1.label")
+          = bill_address.text_field :address1, { placeholder: " " }
+          = f.error_message_on "bill_address.address1"
 
         %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
-          = ship_address.label :city, t("split_checkout.step1.address.city.label")
-          = ship_address.text_field :city, { placeholder: " " }
-          = f.error_message_on "ship_address.city"
+          = bill_address.label :address2, t("split_checkout.step1.address.address2.label")
+          = bill_address.text_field :address2, { placeholder: " " }
+          = f.error_message_on "bill_address.address2"
 
         %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
-          = ship_address.label :zipcode, t("split_checkout.step1.address.zipcode.label")
-          = ship_address.text_field :zipcode, { placeholder: " " }
-          = f.error_message_on "ship_address.zipcode"
+          = bill_address.label :city, t("split_checkout.step1.address.city.label")
+          = bill_address.text_field :city, { placeholder: " " }
+          = f.error_message_on "bill_address.city"
+
+        %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
+          = bill_address.label :zipcode, t("split_checkout.step1.address.zipcode.label")
+          = bill_address.text_field :zipcode, { placeholder: " " }
+          = f.error_message_on "bill_address.zipcode"
 
         %div{ "data-controller": "dependent-select", "data-dependent-select-options-value": countries_with_states }
-          - ship_address_country = @order.ship_address.country || DefaultCountry.country
+          - bill_address_country = @order.bill_address.country || DefaultCountry.country
 
           %div.checkout-input
-            = ship_address.label :country_id, t("split_checkout.step1.address.country_id.label")
-            = ship_address.select :country_id, countries, { selected: ship_address_country.id }, { "data-dependent-select-target": "source", "data-action": "dependent-select#handleSelectChange" }
+            = bill_address.label :country_id, t("split_checkout.step1.address.country_id.label")
+            = bill_address.select :country_id, countries, { selected: bill_address_country.id }, { "data-dependent-select-target": "source", "data-action": "dependent-select#handleSelectChange" }
 
           %div.checkout-input
-            = ship_address.label :state_id, t("split_checkout.step1.address.state_id.label")
-            = ship_address.select :state_id, states_for_country(ship_address_country), { selected: @order.ship_address&.state_id }, { "data-dependent-select-target": "select" }
+            = bill_address.label :state_id, t("split_checkout.step1.address.state_id.label")
+            = bill_address.select :state_id, states_for_country(bill_address_country), { selected: @order.bill_address&.state_id }, { "data-dependent-select-target": "select" }
+
+        - if spree_current_user
+          %div.checkout-input
+            = f.check_box :save_bill_address
+            = f.label :save_bill_address, t(:checkout_default_bill_address)
+
+    %div.checkout-substep{ "data-controller": "toggle shippingmethod" }
+      - selected_shipping_method = @order.shipping_method&.id || params[:shipping_method_id]
+      %div.checkout-title
+        = t("split_checkout.step1.shipping_info.title")
+
+      - display_ship_address = false
+      - ship_method_description = nil
+
+      - selected_shipping_method ||= @shipping_methods[0].id if @shipping_methods.length == 1
+      - @shipping_methods.each do |shipping_method|
+        - ship_method_is_selected = shipping_method.id == selected_shipping_method.to_i
+        %div.checkout-input.checkout-input-radio
+          = fields_for shipping_method do |shipping_method_form|
+            = shipping_method_form.radio_button :name, shipping_method.id,
+              id: "shipping_method_#{shipping_method.id}",
+              checked: ship_method_is_selected,
+              name: "shipping_method_id",
+              "data-requireAddress": shipping_method.require_ship_address,
+              "data-action": "toggle#toggle shippingmethod#selectShippingMethod",
+              "data-toggle-show": shipping_method.require_ship_address
+            = shipping_method_form.label shipping_method.id, shipping_method.name, {for: "shipping_method_" + shipping_method.id.to_s }
+            %em.fees= payment_or_shipping_price(shipping_method, @order)
+            - display_ship_address = display_ship_address || (ship_method_is_selected && shipping_method.require_ship_address)
+        %div.checkout-input{"data-shippingmethod-target": "shippingMethodDescription", "data-shippingmethodid": shipping_method.id , style: "display: #{ship_method_is_selected ? 'block' : 'none'}" }
+          #distributor_address.panel
+            - if shipping_method.description.present?
+              %span #{shipping_method.description}
+              %br/
+              %br/
+            - if @order.order_cycle.pickup_time_for(@order.distributor)
+              = t :checkout_ready_for
+            = @order.order_cycle.pickup_time_for(@order.distributor)
+
+      = f.error_message_on :shipping_method, standalone: true
 
-    - if spree_current_user
       %div.checkout-input{ "data-toggle-target": "content", style: "display: #{display_ship_address ? 'block' : 'none'}" }
-        = f.check_box :save_ship_address
-        = f.label :save_ship_address, t(:checkout_default_ship_address)
+        = f.check_box :ship_address_same_as_billing, { id: "ship_address_same_as_billing", name: "ship_address_same_as_billing", "data-action": "shippingmethod#showHideShippingAddress", "data-shippingmethod-target": "shippingAddressCheckbox", checked: shipping_and_billing_match?(@order) }, 1, nil
+        = f.label :ship_address_same_as_billing, t(:checkout_address_same), { for: "ship_address_same_as_billing" }
 
-    .div.checkout-input
-      = f.label :special_instructions, t(:checkout_instructions)
-      = f.text_area :special_instructions, size: "60x4"
+      %div{"data-shippingmethod-target": "shippingMethodAddress", style: "display: #{!display_ship_address || shipping_and_billing_match?(@order) ? 'none' : 'block'}" }
+        = f.fields :ship_address, model: @order.ship_address do |ship_address|
+          %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
+            = ship_address.label :address1, t("split_checkout.step1.address.address1.label")
+            = ship_address.text_field :address1, { placeholder: " " }
+            = f.error_message_on "ship_address.address1"
 
-  %div.checkout-submit
-    = f.submit t("split_checkout.step1.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false
-    %a.button.cancel{href: main_app.cart_path}
-      = t("split_checkout.step1.cancel")
+          %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
+            = ship_address.label :address2, t("split_checkout.step1.address.address2.label")
+            = ship_address.text_field :address2, { placeholder: " " }
+            = f.error_message_on "ship_address.address2"
+
+          %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
+            = ship_address.label :city, t("split_checkout.step1.address.city.label")
+            = ship_address.text_field :city, { placeholder: " " }
+            = f.error_message_on "ship_address.city"
+
+          %div.checkout-input.with-floating-label{ "data-controller": "floating-label"}
+            = ship_address.label :zipcode, t("split_checkout.step1.address.zipcode.label")
+            = ship_address.text_field :zipcode, { placeholder: " " }
+            = f.error_message_on "ship_address.zipcode"
+
+          %div{ "data-controller": "dependent-select", "data-dependent-select-options-value": countries_with_states }
+            - ship_address_country = @order.ship_address.country || DefaultCountry.country
+
+            %div.checkout-input
+              = ship_address.label :country_id, t("split_checkout.step1.address.country_id.label")
+              = ship_address.select :country_id, countries, { selected: ship_address_country.id }, { "data-dependent-select-target": "source", "data-action": "dependent-select#handleSelectChange" }
+
+            %div.checkout-input
+              = ship_address.label :state_id, t("split_checkout.step1.address.state_id.label")
+              = ship_address.select :state_id, states_for_country(ship_address_country), { selected: @order.ship_address&.state_id }, { "data-dependent-select-target": "select" }
+
+      - if spree_current_user
+        %div.checkout-input{ "data-toggle-target": "content", style: "display: #{display_ship_address ? 'block' : 'none'}" }
+          = f.check_box :save_ship_address
+          = f.label :save_ship_address, t(:checkout_default_ship_address)
+
+      .div.checkout-input
+        = f.label :special_instructions, t(:checkout_instructions)
+        = f.text_area :special_instructions, size: "60x4"
+
+    %div.checkout-submit
+      = f.submit t("split_checkout.step1.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false
+      %a.button.cancel{href: main_app.cart_path}
+        = t("split_checkout.step1.cancel")
diff --git a/app/views/split_checkout/_form.html.haml b/app/views/split_checkout/_form.html.haml
index 0d32cac7c5..afedf10ebd 100644
--- a/app/views/split_checkout/_form.html.haml
+++ b/app/views/split_checkout/_form.html.haml
@@ -1,8 +1,5 @@
 - content_for :injection_data do
   = inject_saved_credit_cards
 
-%div.checkout-step{"class": if checkout_step?(:summary) then "checkout-summary" end}
-  = form_with url: checkout_update_path(checkout_step), model: @order, method: :put,
-    data: { remote: "true" } do |form|
-
-    = render "split_checkout/#{checkout_step}", f: form
+%div.checkout-step{ class: "#{'checkout-summary' if checkout_step?(:summary)}" }
+  = render "split_checkout/#{checkout_step}"
diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml
index 82d3adcb3a..a85fe0d922 100644
--- a/app/views/split_checkout/_payment.html.haml
+++ b/app/views/split_checkout/_payment.html.haml
@@ -1,37 +1,38 @@
-.medium-6
-  %div.checkout-substep{"data-controller": "paymentmethod"}
-    = render partial: "split_checkout/voucher_section", formats: [:cable_ready], locals: { order: @order, voucher_adjustment: @voucher_adjustment }
-        
-    %div.checkout-title
-      = t("split_checkout.step2.payment_method.title")
+= form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
+  .medium-6
+    %div.checkout-substep{"data-controller": "paymentmethod"}
+      = render partial: "split_checkout/voucher_section", formats: [:cable_ready], locals: { order: @order, voucher_adjustment: @voucher_adjustment }
 
-    - selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id
-    - selected_payment_method ||= available_payment_methods[0].id if available_payment_methods.length == 1
-    - available_payment_methods.each do |payment_method| 
-      %div.checkout-input.checkout-input-radio
-        = f.radio_button :payment_method_id, payment_method.id,
-          id: "payment_method_#{payment_method.id}",
-          name: "order[payments_attributes][][payment_method_id]",
-          checked: (payment_method.id == selected_payment_method),
-          "data-action": "paymentmethod#selectPaymentMethod",
-          "data-paymentmethod-id": "#{payment_method.id}",
-          "data-paymentmethod-target": "input"
-        = f.label :payment_method_id, "#{payment_method.name}", for: "payment_method_#{payment_method.id}"
-        %em.fees=payment_or_shipping_price(payment_method, @order)
+      %div.checkout-title
+        = t("split_checkout.step2.payment_method.title")
 
-      .paymentmethod-container{"data-paymentmethod-id": "#{payment_method.id}", style: "display: #{payment_method.id == selected_payment_method ? "block" : "none"}"}
-        - if payment_method.description && !payment_method.description.empty?
-          .paymentmethod-description.panel
-            #{payment_method.description}
-        .paymentmethod-form
-          = render partial: "split_checkout/payment/#{payment_method.method_type}", locals: { payment_method: payment_method, f: f }
+      - selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id
+      - selected_payment_method ||= available_payment_methods[0].id if available_payment_methods.length == 1
+      - available_payment_methods.each do |payment_method|
+        %div.checkout-input.checkout-input-radio
+          = f.radio_button :payment_method_id, payment_method.id,
+            id: "payment_method_#{payment_method.id}",
+            name: "order[payments_attributes][][payment_method_id]",
+            checked: (payment_method.id == selected_payment_method),
+            "data-action": "paymentmethod#selectPaymentMethod",
+            "data-paymentmethod-id": "#{payment_method.id}",
+            "data-paymentmethod-target": "input"
+          = f.label :payment_method_id, "#{payment_method.name}", for: "payment_method_#{payment_method.id}"
+          %em.fees=payment_or_shipping_price(payment_method, @order)
 
-    = f.error_message_on :payment_method, standalone: true
-      
-  %div.checkout-substep
-    = t("split_checkout.step2.explaination")
+        .paymentmethod-container{"data-paymentmethod-id": "#{payment_method.id}", style: "display: #{payment_method.id == selected_payment_method ? "block" : "none"}"}
+          - if payment_method.description && !payment_method.description.empty?
+            .paymentmethod-description.panel
+              #{payment_method.description}
+          .paymentmethod-form
+            = render partial: "split_checkout/payment/#{payment_method.method_type}", locals: { payment_method: payment_method, f: f }
 
-  %div.checkout-submit
-    = f.submit t("split_checkout.step2.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false
-    %a.button.cancel{href: main_app.checkout_step_path(:details)}
-      = t("split_checkout.step2.cancel")
+      = f.error_message_on :payment_method, standalone: true
+
+    %div.checkout-substep
+      = t("split_checkout.step2.explaination")
+
+    %div.checkout-submit
+      = f.submit t("split_checkout.step2.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false
+      %a.button.cancel{href: main_app.checkout_step_path(:details)}
+        = t("split_checkout.step2.cancel")
diff --git a/app/views/split_checkout/_summary.html.haml b/app/views/split_checkout/_summary.html.haml
index e395bd7411..bd14fd3e7d 100644
--- a/app/views/split_checkout/_summary.html.haml
+++ b/app/views/split_checkout/_summary.html.haml
@@ -1,103 +1,104 @@
-.summary-main
-  = render partial: "split_checkout/already_ordered" if show_bought_items? && checkout_step?(:summary)
-  .checkout-substep
-    .checkout-title
-      = t("split_checkout.step3.delivery_details.title")
-      %a.summary-edit{href: main_app.checkout_step_path(:details)}
-        = t("split_checkout.step3.delivery_details.edit")
+= form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
+  .summary-main
+    = render partial: "split_checkout/already_ordered" if show_bought_items? && checkout_step?(:summary)
+    .checkout-substep
+      .checkout-title
+        = t("split_checkout.step3.delivery_details.title")
+        %a.summary-edit{href: main_app.checkout_step_path(:details)}
+          = t("split_checkout.step3.delivery_details.edit")
 
-    .summary-subtitle
-      = @order.shipping_method.name
-      %em.fees= payment_or_shipping_price(@order.shipping_method, @order)
-    .two-columns
-      %div
-        .summary-subtitle
-          = t("split_checkout.step3.delivery_details.address")
-        %span
-          = @order.bill_address.firstname
-          = @order.bill_address.lastname
-        %div 
-          = @order.bill_address.phone
+      .summary-subtitle
+        = @order.shipping_method.name
+        %em.fees= payment_or_shipping_price(@order.shipping_method, @order)
+      .two-columns
         %div
-          = @order.user.email if @order.user
-        %br
-        %div
-          = @order.bill_address.address1
-        - unless @order.bill_address.address2.blank?
+          .summary-subtitle
+            = t("split_checkout.step3.delivery_details.address")
+          %span
+            = @order.bill_address.firstname
+            = @order.bill_address.lastname
           %div
-            = @order.bill_address.address2
-        %div
-          = @order.bill_address.city
-        %div
-          = @order.bill_address.state
-        %div
-          = @order.bill_address.zipcode
-        %div
-          = @order.bill_address.country
-        - if @order.special_instructions.present?
+            = @order.bill_address.phone
+          %div
+            = @order.user.email if @order.user
           %br
-          %em
-            = @order.special_instructions
-      - if @order.shipping_method.description.present?
-        %div
-          .summary-subtitle
-            = t("split_checkout.step3.delivery_details.instructions")
           %div
-            = @order.shipping_method.description
-    
-  %hr
-
-  .checkout-substep
-    .checkout-title
-      = t("split_checkout.step3.payment_method.title")
-      %a.summary-edit{href: main_app.checkout_step_path(:payment)}
-        = t("split_checkout.step3.payment_method.edit")
-    .two-columns
-      %div
-        - payment_method = last_payment_method(@order)
-        = payment_method&.name
-        %em.fees=payment_or_shipping_price(payment_method, @order)
-      - if payment_method&.description.present? 
-        %div
-          .summary-subtitle
-            = t("split_checkout.step3.payment_method.instructions")
+            = @order.bill_address.address1
+          - unless @order.bill_address.address2.blank?
+            %div
+              = @order.bill_address.address2
           %div
-            = last_payment_method(@order)&.description
+            = @order.bill_address.city
+          %div
+            = @order.bill_address.state
+          %div
+            = @order.bill_address.zipcode
+          %div
+            = @order.bill_address.country
+          - if @order.special_instructions.present?
+            %br
+            %em
+              = @order.special_instructions
+        - if @order.shipping_method.description.present?
+          %div
+            .summary-subtitle
+              = t("split_checkout.step3.delivery_details.instructions")
+            %div
+              = @order.shipping_method.description
+
+    %hr
+
+    .checkout-substep
+      .checkout-title
+        = t("split_checkout.step3.payment_method.title")
+        %a.summary-edit{href: main_app.checkout_step_path(:payment)}
+          = t("split_checkout.step3.payment_method.edit")
+      .two-columns
+        %div
+          - payment_method = last_payment_method(@order)
+          = payment_method&.name
+          %em.fees=payment_or_shipping_price(payment_method, @order)
+        - if payment_method&.description.present?
+          %div
+            .summary-subtitle
+              = t("split_checkout.step3.payment_method.instructions")
+            %div
+              = last_payment_method(@order)&.description
 
 
-  %div.checkout-substep
-    %div.checkout-title
-      = t("split_checkout.step3.order.title")
-      %a.summary-edit{href: main_app.cart_path}
-        = t("split_checkout.step3.order.edit")
-    
-    = render 'spree/orders/summary', order: @order, display_footer: false
+    %div.checkout-substep
+      %div.checkout-title
+        = t("split_checkout.step3.order.title")
+        %a.summary-edit{href: main_app.cart_path}
+          = t("split_checkout.step3.order.edit")
+
+      = render 'spree/orders/summary', order: @order, display_footer: false
 
 
-.summary-right{ "data-controller": "sticky", "data-sticky-target": "container" }
-  .summary-right-line.total
-    .summary-right-line-label= t :order_total_price
-    .summary-right-line-value#order_total= @order.display_total.to_html
-  
-  .summary-right-line
-    .summary-right-line-label= t :order_produce
-    .summary-right-line-value= display_checkout_subtotal(@order)
+  .summary-right{ "data-controller": "sticky", "data-sticky-target": "container" }
+    .summary-right-line.total
+      .summary-right-line-label= t :order_total_price
+      .summary-right-line-value#order_total= @order.display_total.to_html
 
-  - checkout_adjustments_for(@order, exclude: [:line_item]).reverse_each do |adjustment|
     .summary-right-line
-      -if adjustment.originator_type == 'Voucher'
-        .summary-right-line-label.voucher= adjustment.label
-        .summary-right-line-value.voucher= adjustment.display_amount.to_html
-      -else
-        .summary-right-line-label= adjustment.label
-        .summary-right-line-value= adjustment.display_amount.to_html
+      .summary-right-line-label= t :order_produce
+      .summary-right-line-value= display_checkout_subtotal(@order)
 
-  - if @order.total_tax > 0
-    .summary-right-line
-      .summary-right-line-label= t :order_includes_tax
-      .summary-right-line-value#tax-row= display_checkout_tax_total(@order)
-  
-  .checkout-submit
-    - if any_terms_required?(@order.distributor)
-      = render partial: "terms_and_conditions", locals: { f: f }
-    = f.submit t("split_checkout.step3.submit"), name: "confirm_order", class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false
+    - checkout_adjustments_for(@order, exclude: [:line_item]).reverse_each do |adjustment|
+      .summary-right-line
+        -if adjustment.originator_type == 'Voucher'
+          .summary-right-line-label.voucher= adjustment.label
+          .summary-right-line-value.voucher= adjustment.display_amount.to_html
+        -else
+          .summary-right-line-label= adjustment.label
+          .summary-right-line-value= adjustment.display_amount.to_html
+
+    - if @order.total_tax > 0
+      .summary-right-line
+        .summary-right-line-label= t :order_includes_tax
+        .summary-right-line-value#tax-row= display_checkout_tax_total(@order)
+
+    .checkout-submit
+      - if any_terms_required?(@order.distributor)
+        = render partial: "terms_and_conditions", locals: { f: f }
+      = f.submit t("split_checkout.step3.submit"), name: "confirm_order", class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false

From 3f83da992838f9a5bdf2abf0e1e650226828ac60 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 00:20:19 +0100
Subject: [PATCH 04/28] Move voucher section out of main checkout form

---
 app/views/split_checkout/_payment.html.haml   |  8 +++--
 .../_voucher_section.cable_ready.haml         | 35 +++++++++----------
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml
index a85fe0d922..fb626ae4f9 100644
--- a/app/views/split_checkout/_payment.html.haml
+++ b/app/views/split_checkout/_payment.html.haml
@@ -1,8 +1,10 @@
-= form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
-  .medium-6
-    %div.checkout-substep{"data-controller": "paymentmethod"}
+.medium-6
+  - if @order.distributor.vouchers.present?
+    %div.checkout-substep
       = render partial: "split_checkout/voucher_section", formats: [:cable_ready], locals: { order: @order, voucher_adjustment: @voucher_adjustment }
 
+  = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
+    %div.checkout-substep{"data-controller": "paymentmethod"}
       %div.checkout-title
         = t("split_checkout.step2.payment_method.title")
 
diff --git a/app/views/split_checkout/_voucher_section.cable_ready.haml b/app/views/split_checkout/_voucher_section.cable_ready.haml
index a005b5698b..059acfa00a 100644
--- a/app/views/split_checkout/_voucher_section.cable_ready.haml
+++ b/app/views/split_checkout/_voucher_section.cable_ready.haml
@@ -1,19 +1,18 @@
 %div#voucher-section
-  - if order.distributor.vouchers.present?
-    .checkout-title
-      = t("split_checkout.step2.voucher.apply_voucher")
-    .checkout-input
-      .two-columns-inputs.voucher{"data-controller": "toggle-button-disabled"}
-        - if voucher_adjustment.present?
-          %span.button.voucher-added
-            %i.ofn-i_051-check-big
-              = t("split_checkout.step2.voucher.voucher", voucher_amount: voucher_adjustment.originator.display_value)
-          = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") }
-          - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ?
-          - if voucher_adjustment.originator.amount > order.total
-            .checkout-input
-              %span.formError.standalone
-                = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")
-        - else
-          = text_field_tag "[order][voucher_code]", params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher"
-          = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }
+  .checkout-title
+    = t("split_checkout.step2.voucher.apply_voucher")
+  .checkout-input
+    .two-columns-inputs.voucher{"data-controller": "toggle-button-disabled"}
+      - if voucher_adjustment.present?
+        %span.button.voucher-added
+          %i.ofn-i_051-check-big
+            = t("split_checkout.step2.voucher.voucher", voucher_amount: voucher_adjustment.originator.display_value)
+        = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") }
+        - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ?
+        - if voucher_adjustment.originator.amount > order.total
+          .checkout-input
+            %span.formError.standalone
+              = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")
+      - else
+        = text_field_tag "[order][voucher_code]", params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher"
+        = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }

From 91004588f6a4d0deba8c51549df8bb5a494b52ba Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 00:35:16 +0100
Subject: [PATCH 05/28] Add separate voucher form

---
 app/views/split_checkout/_payment.html.haml                  | 2 +-
 ...r_section.cable_ready.haml => _voucher_section.html.haml} | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)
 rename app/views/split_checkout/{_voucher_section.cable_ready.haml => _voucher_section.html.haml} (66%)

diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml
index fb626ae4f9..2b0fd81aa0 100644
--- a/app/views/split_checkout/_payment.html.haml
+++ b/app/views/split_checkout/_payment.html.haml
@@ -1,7 +1,7 @@
 .medium-6
   - if @order.distributor.vouchers.present?
     %div.checkout-substep
-      = render partial: "split_checkout/voucher_section", formats: [:cable_ready], locals: { order: @order, voucher_adjustment: @voucher_adjustment }
+      = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @voucher_adjustment }
 
   = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
     %div.checkout-substep{"data-controller": "paymentmethod"}
diff --git a/app/views/split_checkout/_voucher_section.cable_ready.haml b/app/views/split_checkout/_voucher_section.html.haml
similarity index 66%
rename from app/views/split_checkout/_voucher_section.cable_ready.haml
rename to app/views/split_checkout/_voucher_section.html.haml
index 059acfa00a..d50aad3933 100644
--- a/app/views/split_checkout/_voucher_section.cable_ready.haml
+++ b/app/views/split_checkout/_voucher_section.html.haml
@@ -14,5 +14,6 @@
             %span.formError.standalone
               = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")
       - else
-        = text_field_tag "[order][voucher_code]", params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher"
-        = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }
+        = form_with url: voucher_adjustments_path, method: :post, data: { remote: true } do |form|
+          = form.text_field :voucher_code, value: params[:voucher_code], data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher"
+          = form.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }

From 9e5061fc31926777feabc1b1c15ee18943f9d493 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 00:58:06 +0100
Subject: [PATCH 06/28] Move voucher processing out of checkout controller

---
 app/controllers/split_checkout_controller.rb  | 57 ----------------
 .../voucher_adjustments_controller.rb         | 68 ++++++++++++++-----
 config/routes.rb                              |  2 +-
 spec/requests/voucher_adjustments_spec.rb     | 44 +++++-------
 4 files changed, 69 insertions(+), 102 deletions(-)

diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb
index 31f2368b69..7b3d57977b 100644
--- a/app/controllers/split_checkout_controller.rb
+++ b/app/controllers/split_checkout_controller.rb
@@ -29,8 +29,6 @@ class SplitCheckoutController < ::BaseController
   end
 
   def update
-    return process_voucher if params[:apply_voucher].present?
-
     if confirm_order || update_order
       return if performed?
 
@@ -60,27 +58,6 @@ class SplitCheckoutController < ::BaseController
       replace("#flashes", partial("shared/flashes", locals: { flashes: flash }))
   end
 
-  def render_voucher_section_or_redirect
-    respond_to do |format|
-      format.cable_ready { render_voucher_section }
-      format.html { redirect_to checkout_step_path(:payment) }
-    end
-  end
-
-  # Using the power of cable_car we replace only the #voucher_section instead of reloading the page
-  def render_voucher_section
-    render(
-      status: :ok,
-      cable_ready: cable_car.replace(
-        "#voucher-section",
-        partial(
-          "split_checkout/voucher_section",
-          locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first }
-        )
-      )
-    )
-  end
-
   def order_error_messages
     # Remove ship_address.* errors if no shipping method is not selected
     remove_ship_address_errors if no_ship_address_needed?
@@ -201,40 +178,6 @@ class SplitCheckoutController < ::BaseController
     selected_shipping_method.first.require_ship_address == false
   end
 
-  def process_voucher
-    if add_voucher
-      VoucherAdjustmentsService.calculate(@order)
-      render_voucher_section_or_redirect
-    elsif @order.errors.present?
-      render_error
-    end
-  end
-
-  def add_voucher
-    if params.dig(:order, :voucher_code).blank?
-      @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found'))
-      return false
-    end
-
-    # Fetch Voucher
-    voucher = Voucher.find_by(code: params[:order][:voucher_code], enterprise: @order.distributor)
-
-    if voucher.nil?
-      @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found'))
-      return false
-    end
-
-    adjustment = voucher.create_adjustment(voucher.code, @order)
-
-    unless adjustment.valid?
-      @order.errors.add(:voucher, I18n.t('split_checkout.errors.add_voucher_error'))
-      adjustment.errors.each { |error| @order.errors.import(error) }
-      return false
-    end
-
-    true
-  end
-
   def summary_step?
     params[:step] == "summary"
   end
diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb
index b45b60fd9b..b9f5c0524e 100644
--- a/app/controllers/voucher_adjustments_controller.rb
+++ b/app/controllers/voucher_adjustments_controller.rb
@@ -1,32 +1,68 @@
 # frozen_string_literal: true
 
 class VoucherAdjustmentsController < BaseController
-  include CablecarResponses
+  before_action :set_order
+
+  def create
+    if add_voucher
+      VoucherAdjustmentsService.calculate(@order)
+
+      render_voucher_section
+    elsif @order.errors.present?
+      render_error
+    end
+  end
 
   def destroy
-    @order = current_order
-
     @order.voucher_adjustments.find_by(id: params[:id])&.destroy
 
-    respond_to do |format|
-      format.cable_ready { render_voucher_section }
-      format.html { redirect_to checkout_step_path(:payment) }
-    end
+    render_voucher_section
   end
 
   private
 
-  # Using the power of cable_car we replace only the #voucher_section instead of reloading the page
+  def set_order
+    @order = current_order
+  end
+
+  def add_voucher
+    if params[:voucher_code].blank?
+      @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found'))
+      return false
+    end
+
+    voucher = Voucher.find_by(code: params[:voucher_code], enterprise: @order.distributor)
+
+    if voucher.nil?
+      @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found'))
+      return false
+    end
+
+    adjustment = voucher.create_adjustment(voucher.code, @order)
+
+    if !adjustment.valid?
+      @order.errors.add(:voucher, I18n.t('split_checkout.errors.add_voucher_error'))
+      adjustment.errors.each { |error| @order.errors.import(error) }
+      return false
+    end
+
+    true
+  end
+
   def render_voucher_section
-    render(
-      status: :ok,
-      cable_ready: cable_car.replace(
-        "#voucher-section",
-        partial(
-          "split_checkout/voucher_section",
-          locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first }
-        )
+    render cable_ready: cable_car.replace(
+      selector: "#voucher-section",
+      html: render_to_string(
+        partial: "split_checkout/voucher_section",
+        locals: { order: @order,voucher_adjustment: @order.voucher_adjustments.first }
       )
     )
   end
+
+  def render_error
+    flash.now[:error] = @order.errors.full_messages.to_sentence
+
+    render status: :unprocessable_entity, cable_ready: cable_car.
+      replace("#flashes", partial("shared/flashes", locals: { flashes: flash }))
+  end
 end
diff --git a/config/routes.rb b/config/routes.rb
index 1d933363ac..c92481cb64 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -112,7 +112,7 @@ Openfoodnetwork::Application.routes.draw do
   get '/:id/shop', to: 'enterprises#shop', as: 'enterprise_shop'
   get "/enterprises/:permalink", to: redirect("/") # Legacy enterprise URL
 
-  resources :voucher_adjustments, only: [:destroy]
+  resources :voucher_adjustments, only: [:create, :destroy]
 
   get 'sitemap.xml', to: 'sitemap#index', defaults: { format: 'xml' }
 
diff --git a/spec/requests/voucher_adjustments_spec.rb b/spec/requests/voucher_adjustments_spec.rb
index 9154f7ea32..fc533473b8 100644
--- a/spec/requests/voucher_adjustments_spec.rb
+++ b/spec/requests/voucher_adjustments_spec.rb
@@ -18,42 +18,30 @@ describe VoucherAdjustmentsController, type: :request do
   end
 
   describe "DELETE voucher_adjustments/:id" do
-    let(:cable_ready_header) { { accept: "text/vnd.cable-ready.json" } }
+    it "deletes the voucher adjustment" do
+      delete "/voucher_adjustments/#{adjustment.id}"
 
-    context "with a cable ready request" do
-      it "deletes the voucher adjustment" do
-        delete("/voucher_adjustments/#{adjustment.id}", headers: cable_ready_header)
+      expect(order.voucher_adjustments.length).to eq(0)
+    end
 
-        expect(order.voucher_adjustments.length).to eq(0)
+    it "render a succesful response" do
+      delete "/voucher_adjustments/#{adjustment.id}"
+
+      expect(response).to be_successful
+    end
+
+    context "when adjustment doesn't exits" do
+      it "does nothing" do
+        delete "/voucher_adjustments/-1"
+
+        expect(order.voucher_adjustments.length).to eq(1)
       end
 
       it "render a succesful response" do
-        delete("/voucher_adjustments/#{adjustment.id}", headers: cable_ready_header)
+        delete "/voucher_adjustments/-1"
 
         expect(response).to be_successful
       end
-
-      context "when adjustment doesn't exits" do
-        it "does nothing" do
-          delete "/voucher_adjustments/-1", headers: cable_ready_header
-
-          expect(order.voucher_adjustments.length).to eq(1)
-        end
-
-        it "render a succesful response" do
-          delete "/voucher_adjustments/-1", headers: cable_ready_header
-
-          expect(response).to be_successful
-        end
-      end
-    end
-
-    context "with an html request" do
-      it "redirect to checkout payment step" do
-        delete "/voucher_adjustments/#{adjustment.id}"
-
-        expect(response).to redirect_to(checkout_step_path(:payment))
-      end
     end
   end
 end

From 47b5a3fb1d014b1aa6e9a5d5bf75fb8cc646487c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 14:33:51 +0100
Subject: [PATCH 07/28] Don't apply tax calculations if there's no tax

---
 app/services/voucher_adjustments_service.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/services/voucher_adjustments_service.rb b/app/services/voucher_adjustments_service.rb
index 2569010d95..ccadb8eb75 100644
--- a/app/services/voucher_adjustments_service.rb
+++ b/app/services/voucher_adjustments_service.rb
@@ -19,8 +19,10 @@ class VoucherAdjustmentsService
     # For now we just assume it is either all tax included in price or all tax excluded from price.
     if order.additional_tax_total.positive?
       handle_tax_excluded_from_price(order, amount)
-    else
+    elsif order.included_tax_total.positive?
       handle_tax_included_in_price(order, amount)
+    else
+      adjustment.amount = amount
     end
 
     # Move to closed state

From d7950617ecf3f5e38100acf404134032909aaf5f Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 15:15:15 +0100
Subject: [PATCH 08/28] Drop superfluous method

---
 app/controllers/split_checkout_controller.rb | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb
index 7b3d57977b..c74b3f0ba1 100644
--- a/app/controllers/split_checkout_controller.rb
+++ b/app/controllers/split_checkout_controller.rb
@@ -25,7 +25,9 @@ class SplitCheckoutController < ::BaseController
     redirect_to_step_based_on_order unless params[:step]
     check_step if params[:step]
 
-    flash_error_when_no_shipping_method_available if available_shipping_methods.none?
+    return if available_shipping_methods.any?
+
+    flash[:error] = I18n.t('split_checkout.errors.no_shipping_methods_available')
   end
 
   def update
@@ -102,10 +104,6 @@ class SplitCheckoutController < ::BaseController
     end
   end
 
-  def flash_error_when_no_shipping_method_available
-    flash[:error] = I18n.t('split_checkout.errors.no_shipping_methods_available')
-  end
-
   def check_payments_adjustments
     @order.payments.each(&:ensure_correct_adjustment)
   end

From a65b3b8b60b58c7874c34c69a970a036278d2b6e Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 15:41:39 +0100
Subject: [PATCH 09/28] Extract voucher tests to separate controller spec

---
 .../split_checkout_controller_spec.rb         | 67 ----------------
 .../voucher_adjustments_controller_spec.rb    | 78 +++++++++++++++++++
 2 files changed, 78 insertions(+), 67 deletions(-)
 create mode 100644 spec/controllers/voucher_adjustments_controller_spec.rb

diff --git a/spec/controllers/split_checkout_controller_spec.rb b/spec/controllers/split_checkout_controller_spec.rb
index 114a947c39..7783717e28 100644
--- a/spec/controllers/split_checkout_controller_spec.rb
+++ b/spec/controllers/split_checkout_controller_spec.rb
@@ -233,73 +233,6 @@ describe SplitCheckoutController, type: :controller do
           expect(order.payments.first.source.id).to eq saved_card.id
         end
       end
-
-      describe "Vouchers" do
-        let(:voucher) { create(:voucher, code: 'some_code', enterprise: distributor) }
-
-        describe "adding a voucher" do
-          let(:checkout_params) do
-            {
-              apply_voucher: "true",
-              order: {
-                voucher_code: voucher.code
-              }
-            }
-          end
-
-          it "adds a voucher to the order" do
-            # Set the headers to simulate a cable_ready request
-            request.headers["accept"] = "text/vnd.cable-ready.json"
-
-            put :update, params: params
-
-            expect(response.status).to eq(200)
-            expect(order.reload.voucher_adjustments.length).to eq(1)
-          end
-
-          context "when voucher doesn't exist" do
-            let(:checkout_params) do
-              {
-                apply_voucher: "true",
-                order: {
-                  voucher_code: "non_voucher"
-                }
-              }
-            end
-
-            it "returns 422 and an error message" do
-              put :update, params: params
-
-              expect(response.status).to eq 422
-              expect(flash[:error]).to match "Voucher Not found"
-            end
-          end
-
-          context "when adding fails" do
-            it "returns 422 and an error message" do
-              # Create a non valid adjustment
-              adjustment = build(:adjustment, label: nil)
-              allow(voucher).to receive(:create_adjustment).and_return(adjustment)
-              allow(Voucher).to receive(:find_by).and_return(voucher)
-
-              put :update, params: params
-
-              expect(response.status).to eq 422
-              expect(flash[:error]).to match(
-                "There was an error while adding the voucher and Label can't be blank"
-              )
-            end
-          end
-
-          context "with an html request" do
-            it "redirects to the payment step" do
-              put :update, params: params
-
-              expect(response).to redirect_to(checkout_step_path(:payment))
-            end
-          end
-        end
-      end
     end
 
     context "summary step" do
diff --git a/spec/controllers/voucher_adjustments_controller_spec.rb b/spec/controllers/voucher_adjustments_controller_spec.rb
new file mode 100644
index 0000000000..d9366dae17
--- /dev/null
+++ b/spec/controllers/voucher_adjustments_controller_spec.rb
@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe VoucherAdjustmentsController, type: :controller do
+  let(:user) { order.user }
+  let(:address) { create(:address) }
+  let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) }
+  let(:order_cycle) { create(:order_cycle, distributors: [distributor]) }
+  let(:exchange) { order_cycle.exchanges.outgoing.first }
+  let(:order) {
+    create(:order_with_line_items, line_items_count: 1, distributor: distributor,
+                                   order_cycle: order_cycle, bill_address: address,
+                                   ship_address: address)
+  }
+  let(:payment_method) { distributor.payment_methods.first }
+  let(:shipping_method) { distributor.shipping_methods.first }
+
+  let(:voucher) { create(:voucher, code: 'some_code', enterprise: distributor) }
+
+  before do
+    exchange.variants << order.line_items.first.variant
+    order.select_shipping_method shipping_method.id
+    OrderWorkflow.new(order).advance_to_payment
+
+    allow(controller).to receive(:current_order) { order }
+    allow(controller).to receive(:spree_current_user) { user }
+  end
+
+  describe "#create" do
+    describe "adding a voucher" do
+      let(:params) { { voucher_code: voucher.code } }
+
+      it "adds a voucher to the user's current order" do
+        post :create, params: params
+
+        expect(response.status).to eq(200)
+        expect(order.reload.voucher_adjustments.length).to eq(1)
+      end
+
+      context "when voucher doesn't exist" do
+        let(:params) { { voucher_code: "non_voucher" } }
+
+        it "returns 422 and an error message" do
+          post :create, params: params
+
+          expect(response.status).to eq 422
+          expect(flash[:error]).to match "Voucher Not found"
+        end
+      end
+
+      context "when adding fails" do
+        it "returns 422 and an error message" do
+          # Create a non valid adjustment
+          adjustment = build(:adjustment, label: nil)
+          allow(voucher).to receive(:create_adjustment).and_return(adjustment)
+          allow(Voucher).to receive(:find_by).and_return(voucher)
+
+          post :create, params: params
+
+          expect(response.status).to eq 422
+          expect(flash[:error]).to match(
+            "There was an error while adding the voucher and Label can't be blank"
+          )
+        end
+      end
+    end
+  end
+
+  describe "#destroy" do
+    it "removes the voucher from the current order" do
+      put :destroy, params: { id: voucher.id }
+
+      expect(order.reload.voucher_adjustments.count).to eq 0
+      expect(response.status).to eq 200
+    end
+  end
+end

From bd29a9acdefceb888d274763ac40b2c3c3711331 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 20:18:14 +0100
Subject: [PATCH 10/28] Introduce "zero priced orders" to checkout UI and order
 state flow

---
 app/controllers/split_checkout_controller.rb |  1 +
 app/helpers/checkout_helper.rb               |  2 +
 app/models/spree/order.rb                    |  4 ++
 app/views/split_checkout/_payment.html.haml  | 44 +++++++++++---------
 app/views/split_checkout/_summary.html.haml  | 13 ++++--
 5 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb
index c74b3f0ba1..6d714c258d 100644
--- a/app/controllers/split_checkout_controller.rb
+++ b/app/controllers/split_checkout_controller.rb
@@ -203,6 +203,7 @@ class SplitCheckoutController < ::BaseController
 
   def validate_payment!
     return true if params.dig(:order, :payments_attributes, 0, :payment_method_id).present?
+    return true if @order.zero_priced_order?
 
     @order.errors.add :payment_method, I18n.t('split_checkout.errors.select_a_payment_method')
   end
diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb
index 371b92dfff..c6c2f8d0e7 100644
--- a/app/helpers/checkout_helper.rb
+++ b/app/helpers/checkout_helper.rb
@@ -120,6 +120,8 @@ module CheckoutHelper
   end
 
   def payment_or_shipping_price(method, order)
+    return unless method
+
     price = method.compute_amount(order)
     if price.zero?
       t('checkout_method_free')
diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index d87b14b3bb..21904aa05d 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -621,6 +621,10 @@ module Spree
       raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty?
 
       pending_payments.each do |payment|
+        if payment.amount.zero? && zero_priced_order?
+          payment.update_columns(state: "completed", captured_at: Time.zone.now)
+        end
+
         break if payment_total >= total
 
         yield payment
diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml
index 2b0fd81aa0..12a94405ee 100644
--- a/app/views/split_checkout/_payment.html.haml
+++ b/app/views/split_checkout/_payment.html.haml
@@ -8,28 +8,32 @@
       %div.checkout-title
         = t("split_checkout.step2.payment_method.title")
 
-      - selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id
-      - selected_payment_method ||= available_payment_methods[0].id if available_payment_methods.length == 1
-      - available_payment_methods.each do |payment_method|
-        %div.checkout-input.checkout-input-radio
-          = f.radio_button :payment_method_id, payment_method.id,
-            id: "payment_method_#{payment_method.id}",
-            name: "order[payments_attributes][][payment_method_id]",
-            checked: (payment_method.id == selected_payment_method),
-            "data-action": "paymentmethod#selectPaymentMethod",
-            "data-paymentmethod-id": "#{payment_method.id}",
-            "data-paymentmethod-target": "input"
-          = f.label :payment_method_id, "#{payment_method.name}", for: "payment_method_#{payment_method.id}"
-          %em.fees=payment_or_shipping_price(payment_method, @order)
+      - if @order.zero_priced_order?
+        %h3= t(:no_payment_required)
+        = hidden_field_tag "order[payments_attributes][][amount]", 0
+      - else
+        - selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id
+        - selected_payment_method ||= available_payment_methods[0].id if available_payment_methods.length == 1
+        - available_payment_methods.each do |payment_method|
+          %div.checkout-input.checkout-input-radio
+            = f.radio_button :payment_method_id, payment_method.id,
+              id: "payment_method_#{payment_method.id}",
+              name: "order[payments_attributes][][payment_method_id]",
+              checked: (payment_method.id == selected_payment_method),
+              "data-action": "paymentmethod#selectPaymentMethod",
+              "data-paymentmethod-id": "#{payment_method.id}",
+              "data-paymentmethod-target": "input"
+            = f.label :payment_method_id, "#{payment_method.name}", for: "payment_method_#{payment_method.id}"
+            %em.fees=payment_or_shipping_price(payment_method, @order)
 
-        .paymentmethod-container{"data-paymentmethod-id": "#{payment_method.id}", style: "display: #{payment_method.id == selected_payment_method ? "block" : "none"}"}
-          - if payment_method.description && !payment_method.description.empty?
-            .paymentmethod-description.panel
-              #{payment_method.description}
-          .paymentmethod-form
-            = render partial: "split_checkout/payment/#{payment_method.method_type}", locals: { payment_method: payment_method, f: f }
+          .paymentmethod-container{"data-paymentmethod-id": "#{payment_method.id}", style: "display: #{payment_method.id == selected_payment_method ? "block" : "none"}"}
+            - if payment_method.description && !payment_method.description.empty?
+              .paymentmethod-description.panel
+                #{payment_method.description}
+            .paymentmethod-form
+              = render partial: "split_checkout/payment/#{payment_method.method_type}", locals: { payment_method: payment_method, f: f }
 
-      = f.error_message_on :payment_method, standalone: true
+        = f.error_message_on :payment_method, standalone: true
 
     %div.checkout-substep
       = t("split_checkout.step2.explaination")
diff --git a/app/views/split_checkout/_summary.html.haml b/app/views/split_checkout/_summary.html.haml
index bd14fd3e7d..0317ee6232 100644
--- a/app/views/split_checkout/_summary.html.haml
+++ b/app/views/split_checkout/_summary.html.haml
@@ -54,16 +54,21 @@
         %a.summary-edit{href: main_app.checkout_step_path(:payment)}
           = t("split_checkout.step3.payment_method.edit")
       .two-columns
+        - payment_method = last_payment_method(@order)
         %div
-          - payment_method = last_payment_method(@order)
-          = payment_method&.name
-          %em.fees=payment_or_shipping_price(payment_method, @order)
+          - if payment_method
+            = payment_method.name
+            %em.fees
+              = payment_or_shipping_price(payment_method, @order)
+          - elsif @order.zero_priced_order?
+            %h4= t(:no_payment_required)
+
         - if payment_method&.description.present?
           %div
             .summary-subtitle
               = t("split_checkout.step3.payment_method.instructions")
             %div
-              = last_payment_method(@order)&.description
+              = payment_method&.description
 
 
     %div.checkout-substep

From 1cd38c957df846dfbb774c3897c7b80d0078ac79 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 20:31:49 +0100
Subject: [PATCH 11/28] Introduce "zero priced orders" in admin order payments
 UI and helper

---
 app/helpers/spree/payment_methods_helper.rb    | 5 +++--
 app/views/spree/admin/payments/_list.html.haml | 4 +++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/helpers/spree/payment_methods_helper.rb b/app/helpers/spree/payment_methods_helper.rb
index 50fe34d457..531848ab14 100644
--- a/app/helpers/spree/payment_methods_helper.rb
+++ b/app/helpers/spree/payment_methods_helper.rb
@@ -4,12 +4,13 @@ module Spree
   module PaymentMethodsHelper
     def payment_method(payment)
       # hack to allow us to retrieve the name of a "deleted" payment method
-      id = payment.payment_method_id
+      return unless (id = payment.payment_method_id)
+
       Spree::PaymentMethod.find_with_destroyed(id)
     end
 
     def payment_method_name(payment)
-      payment_method(payment).name
+      payment_method(payment)&.name
     end
   end
 end
diff --git a/app/views/spree/admin/payments/_list.html.haml b/app/views/spree/admin/payments/_list.html.haml
index 4e1a1ec066..cbae3b95c6 100644
--- a/app/views/spree/admin/payments/_list.html.haml
+++ b/app/views/spree/admin/payments/_list.html.haml
@@ -11,7 +11,9 @@
       %tr{class: "#{cycle('odd', 'even')}"}
         %td= pretty_time(payment.created_at)
         %td.align-center= payment.display_amount.to_html
-        %td.align-center= link_to payment_method_name(payment), spree.admin_order_payment_path(@order, payment)
+        %td.align-center
+          - if payment.payment_method_id
+            = link_to payment_method_name(payment), spree.admin_order_payment_path(@order, payment)
         %td.align-center
           %span{class: "state #{payment.state}"}= t(payment.state, scope: "spree.payment_states", default: payment.state.capitalize)
         %td.actions

From 7a0b83076b7a28d386b8a63ae36932ba79881de9 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 21:30:42 +0100
Subject: [PATCH 12/28] Move loading of saved cards out of checkout concern

---
 app/controllers/concerns/checkout_callbacks.rb         | 7 +------
 app/views/split_checkout/payment/_stripe_sca.html.haml | 8 +++++---
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb
index 159c6041ea..18f413d0e2 100644
--- a/app/controllers/concerns/checkout_callbacks.rb
+++ b/app/controllers/concerns/checkout_callbacks.rb
@@ -14,7 +14,7 @@ module CheckoutCallbacks
     prepend_before_action :require_order_cycle
     prepend_before_action :require_distributor_chosen
 
-    before_action :load_order, :associate_user, :load_saved_addresses, :load_saved_credit_cards
+    before_action :load_order, :associate_user, :load_saved_addresses
     before_action :load_shipping_methods, if: -> { params[:step] == "details" }
 
     before_action :ensure_order_not_completed
@@ -43,11 +43,6 @@ module CheckoutCallbacks
     @order.ship_address ||= finder.ship_address
   end
 
-  def load_saved_credit_cards
-    @saved_credit_cards = spree_current_user&.credit_cards&.with_payment_profile.to_a
-    @selected_card = nil
-  end
-
   def load_shipping_methods
     @shipping_methods = available_shipping_methods.sort { |a, b| a.name.casecmp(b.name) }
   end
diff --git a/app/views/split_checkout/payment/_stripe_sca.html.haml b/app/views/split_checkout/payment/_stripe_sca.html.haml
index 144ca2559a..4503c2829e 100644
--- a/app/views/split_checkout/payment/_stripe_sca.html.haml
+++ b/app/views/split_checkout/payment/_stripe_sca.html.haml
@@ -1,15 +1,17 @@
+- saved_credit_cards = spree_current_user&.credit_cards&.with_payment_profile.to_a
+
 %div{"data-controller": "stripe-cards", "data-paymentmethod-id": "#{payment_method.id}" }
-  - if @saved_credit_cards.any?
+  - if saved_credit_cards.any?
     .checkout-input
       %label
         = t('split_checkout.step2.form.stripe.use_saved_card')
       = select_tag :existing_card_id,
-        options_for_select(stripe_card_options(@saved_credit_cards) + [[t('split_checkout.step2.form.stripe.create_new_card'), ""]], @selected_card),
+        options_for_select(stripe_card_options(saved_credit_cards) + [[t('split_checkout.step2.form.stripe.create_new_card'), ""]], nil),
         { "data-action": "change->stripe-cards#onSelectCard", "data-stripe-cards-target": "select" }
 
   %div{"data-stripe-cards-target": "stripeelements"}
     .checkout-input
-      - if @saved_credit_cards.none?
+      - if saved_credit_cards.none?
         %label
           = t('split_checkout.step2.form.stripe.use_new_card')
 

From 55bce9f1b288346ebcca5e6f986bbcfa10cf17d4 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 21:31:20 +0100
Subject: [PATCH 13/28] Remove @voucher_adjustment instance variable

---
 app/controllers/concerns/checkout_callbacks.rb | 2 --
 app/views/split_checkout/_payment.html.haml    | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb
index 18f413d0e2..7da81a592b 100644
--- a/app/controllers/concerns/checkout_callbacks.rb
+++ b/app/controllers/concerns/checkout_callbacks.rb
@@ -30,8 +30,6 @@ module CheckoutCallbacks
     @order.manual_shipping_selection = true
     @order.checkout_processing = true
 
-    @voucher_adjustment = @order.voucher_adjustments.first
-
     redirect_to(main_app.shop_path) && return if redirect_to_shop?
     redirect_to_cart_path && return unless valid_order_line_items?
   end
diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml
index 12a94405ee..e142b894e6 100644
--- a/app/views/split_checkout/_payment.html.haml
+++ b/app/views/split_checkout/_payment.html.haml
@@ -1,7 +1,7 @@
 .medium-6
   - if @order.distributor.vouchers.present?
     %div.checkout-substep
-      = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @voucher_adjustment }
+      = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first }
 
   = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
     %div.checkout-substep{"data-controller": "paymentmethod"}

From de97e69e7d11da4afe42a35a2848a6aac2044ac4 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 21:34:30 +0100
Subject: [PATCH 14/28] Show/hide payment methods if voucher changes order
 total to zero

---
 app/controllers/voucher_adjustments_controller.rb | 14 ++++++--------
 app/views/split_checkout/_payment.html.haml       |  4 ++--
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb
index b9f5c0524e..4ee0c3d3fe 100644
--- a/app/controllers/voucher_adjustments_controller.rb
+++ b/app/controllers/voucher_adjustments_controller.rb
@@ -6,8 +6,9 @@ class VoucherAdjustmentsController < BaseController
   def create
     if add_voucher
       VoucherAdjustmentsService.calculate(@order)
+      @order.update_totals_and_states
 
-      render_voucher_section
+      update_payment_section
     elsif @order.errors.present?
       render_error
     end
@@ -16,7 +17,7 @@ class VoucherAdjustmentsController < BaseController
   def destroy
     @order.voucher_adjustments.find_by(id: params[:id])&.destroy
 
-    render_voucher_section
+    update_payment_section
   end
 
   private
@@ -49,13 +50,10 @@ class VoucherAdjustmentsController < BaseController
     true
   end
 
-  def render_voucher_section
+  def update_payment_section
     render cable_ready: cable_car.replace(
-      selector: "#voucher-section",
-      html: render_to_string(
-        partial: "split_checkout/voucher_section",
-        locals: { order: @order,voucher_adjustment: @order.voucher_adjustments.first }
-      )
+      selector: "#checkout-payment-methods",
+      html: render_to_string(partial: "split_checkout/payment", locals: { step: "payment" })
     )
   end
 
diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml
index e142b894e6..e32e65b245 100644
--- a/app/views/split_checkout/_payment.html.haml
+++ b/app/views/split_checkout/_payment.html.haml
@@ -1,9 +1,9 @@
-.medium-6
+.medium-6#checkout-payment-methods
   - if @order.distributor.vouchers.present?
     %div.checkout-substep
       = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first }
 
-  = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
+  = form_with url: checkout_update_path(local_assigns[:step] || checkout_step), model: @order, method: :put, data: { remote: "true" } do |f|
     %div.checkout-substep{"data-controller": "paymentmethod"}
       %div.checkout-title
         = t("split_checkout.step2.payment_method.title")

From 28795effc3818601f02ca2c3ad010d01652dcdcc Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 2 Jun 2023 21:38:41 +0100
Subject: [PATCH 15/28] Clarify named vouchers in UI

---
 app/views/split_checkout/_summary.html.haml     | 8 +++++---
 app/views/spree/orders/_totals_footer.html.haml | 2 ++
 config/locales/en.yml                           | 1 +
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/app/views/split_checkout/_summary.html.haml b/app/views/split_checkout/_summary.html.haml
index 0317ee6232..e1e230043d 100644
--- a/app/views/split_checkout/_summary.html.haml
+++ b/app/views/split_checkout/_summary.html.haml
@@ -91,10 +91,12 @@
 
     - checkout_adjustments_for(@order, exclude: [:line_item]).reverse_each do |adjustment|
       .summary-right-line
-        -if adjustment.originator_type == 'Voucher'
-          .summary-right-line-label.voucher= adjustment.label
+        - if adjustment.originator_type == 'Voucher'
+          .summary-right-line-label.voucher
+            = "#{t(:voucher)}:"
+            = adjustment.label
           .summary-right-line-value.voucher= adjustment.display_amount.to_html
-        -else
+        - else
           .summary-right-line-label= adjustment.label
           .summary-right-line-value= adjustment.display_amount.to_html
 
diff --git a/app/views/spree/orders/_totals_footer.html.haml b/app/views/spree/orders/_totals_footer.html.haml
index ab8130210e..556160804b 100644
--- a/app/views/spree/orders/_totals_footer.html.haml
+++ b/app/views/spree/orders/_totals_footer.html.haml
@@ -12,6 +12,8 @@
       %tr.total
         %td.text-right{:colspan => "3"}
           %strong
+            - if adjustment.originator_type == "Voucher"
+              = "#{t(:voucher)}:"
             = adjustment.label
         %td.text-right.total
           %span= adjustment.display_amount.to_html
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 5f39fb8b57..ababeaa9df 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -450,6 +450,7 @@ en:
   none: None
   notes: Notes
   error: Error
+  voucher: Voucher
   processing_payment: "Processing payment..."
   no_pending_payments: "No pending payments"
   invalid_payment_state: "Invalid payment state: %{state}"

From 2f0f3e08206b4a7064e77ecffe91555a913ecd8e Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 5 Jun 2023 18:14:38 +0100
Subject: [PATCH 16/28] Re-enable voucher test

---
 spec/system/consumer/split_checkout_tax_not_incl_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
index d4a7ddda41..1109c1c600 100644
--- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
+++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
@@ -115,7 +115,7 @@ describe "As a consumer, I want to see adjustment breakdown" do
         expect(page).to have_selector('#tax-row', text: with_currency(1.30))
       end
 
-      pending "when using a voucher" do
+      context "when using a voucher" do
         let!(:voucher) do
           create(:voucher, code: 'some_code', enterprise: distributor, amount: 10)
         end

From 9b45b71696d5b8b24e3f7f49d689dc8863ca37ec Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 8 Jun 2023 19:33:26 +0100
Subject: [PATCH 17/28] Improve feature toggling

---
 app/views/split_checkout/_payment.html.haml              | 2 +-
 spec/system/consumer/split_checkout_spec.rb              | 2 ++
 spec/system/consumer/split_checkout_tax_incl_spec.rb     | 2 ++
 spec/system/consumer/split_checkout_tax_not_incl_spec.rb | 1 +
 4 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml
index e32e65b245..675fbfd820 100644
--- a/app/views/split_checkout/_payment.html.haml
+++ b/app/views/split_checkout/_payment.html.haml
@@ -1,5 +1,5 @@
 .medium-6#checkout-payment-methods
-  - if @order.distributor.vouchers.present?
+  - if feature?(:vouchers, spree_current_user) && @order.distributor.vouchers.present?
     %div.checkout-substep
       = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first }
 
diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb
index 2bcc9f1cd0..abf8a6f2f2 100644
--- a/spec/system/consumer/split_checkout_spec.rb
+++ b/spec/system/consumer/split_checkout_spec.rb
@@ -707,6 +707,8 @@ describe "As a consumer, I want to checkout my order" do
       end
 
       describe "vouchers" do
+        before { Flipper.enable :vouchers }
+
         context "with no voucher available" do
           before do
             visit checkout_step_path(:payment)
diff --git a/spec/system/consumer/split_checkout_tax_incl_spec.rb b/spec/system/consumer/split_checkout_tax_incl_spec.rb
index 66edeff008..6663c6a970 100644
--- a/spec/system/consumer/split_checkout_tax_incl_spec.rb
+++ b/spec/system/consumer/split_checkout_tax_incl_spec.rb
@@ -106,6 +106,8 @@ describe "As a consumer, I want to see adjustment breakdown" do
       end
 
       context "when using a voucher" do
+        before { Flipper.enable :vouchers }
+
         let!(:voucher) do
           create(:voucher, code: 'some_code', enterprise: distributor, amount: 10)
         end
diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
index 1109c1c600..5d9f9bcd5b 100644
--- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
+++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
@@ -63,6 +63,7 @@ describe "As a consumer, I want to see adjustment breakdown" do
   before do
     # assures tax is charged in dependence of shipping address
     Spree::Config.set(tax_using_ship_address: true)
+    Flipper.enable :vouchers
   end
 
   describe "a not-included tax" do

From 37a4c73a12b2dcba4c5869ab92d43fcbe01a641e Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 8 Jun 2023 19:34:21 +0100
Subject: [PATCH 18/28] Fix rubocop complaint

---
 app/controllers/voucher_adjustments_controller.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb
index 4ee0c3d3fe..267e2d367b 100644
--- a/app/controllers/voucher_adjustments_controller.rb
+++ b/app/controllers/voucher_adjustments_controller.rb
@@ -41,8 +41,8 @@ class VoucherAdjustmentsController < BaseController
 
     adjustment = voucher.create_adjustment(voucher.code, @order)
 
-    if !adjustment.valid?
-      @order.errors.add(:voucher, I18n.t('split_checkout.errors.add_voucher_error'))
+    unless adjustment.valid?
+      @order.errors.add(:voucher_code, I18n.t('split_checkout.errors.add_voucher_error'))
       adjustment.errors.each { |error| @order.errors.import(error) }
       return false
     end

From 672400192f641a10c1220fa865eb2eaba61775b0 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 8 Jun 2023 19:37:23 +0100
Subject: [PATCH 19/28] Update use of params

---
 app/controllers/voucher_adjustments_controller.rb    | 12 ++++++++----
 app/views/split_checkout/_voucher_section.html.haml  |  4 ++--
 .../voucher_adjustments_controller_spec.rb           |  6 +++---
 spec/system/consumer/split_checkout_spec.rb          |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb
index 267e2d367b..9923733fbb 100644
--- a/app/controllers/voucher_adjustments_controller.rb
+++ b/app/controllers/voucher_adjustments_controller.rb
@@ -27,15 +27,15 @@ class VoucherAdjustmentsController < BaseController
   end
 
   def add_voucher
-    if params[:voucher_code].blank?
-      @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found'))
+    if voucher_params[:voucher_code].blank?
+      @order.errors.add(:voucher_code, I18n.t('split_checkout.errors.voucher_not_found'))
       return false
     end
 
-    voucher = Voucher.find_by(code: params[:voucher_code], enterprise: @order.distributor)
+    voucher = Voucher.find_by(code: voucher_params[:voucher_code], enterprise: @order.distributor)
 
     if voucher.nil?
-      @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found'))
+      @order.errors.add(:voucher_code, I18n.t('split_checkout.errors.voucher_not_found'))
       return false
     end
 
@@ -63,4 +63,8 @@ class VoucherAdjustmentsController < BaseController
     render status: :unprocessable_entity, cable_ready: cable_car.
       replace("#flashes", partial("shared/flashes", locals: { flashes: flash }))
   end
+
+  def voucher_params
+    params.require(:order).permit(:voucher_code)
+  end
 end
diff --git a/app/views/split_checkout/_voucher_section.html.haml b/app/views/split_checkout/_voucher_section.html.haml
index d50aad3933..c46dce4ec0 100644
--- a/app/views/split_checkout/_voucher_section.html.haml
+++ b/app/views/split_checkout/_voucher_section.html.haml
@@ -14,6 +14,6 @@
             %span.formError.standalone
               = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")
       - else
-        = form_with url: voucher_adjustments_path, method: :post, data: { remote: true } do |form|
-          = form.text_field :voucher_code, value: params[:voucher_code], data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher"
+        = form_with url: voucher_adjustments_path, model: @order, method: :post, data: { remote: true } do |form|
+          = form.text_field :voucher_code, value: params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher"
           = form.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }
diff --git a/spec/controllers/voucher_adjustments_controller_spec.rb b/spec/controllers/voucher_adjustments_controller_spec.rb
index d9366dae17..df11d9c06b 100644
--- a/spec/controllers/voucher_adjustments_controller_spec.rb
+++ b/spec/controllers/voucher_adjustments_controller_spec.rb
@@ -29,7 +29,7 @@ describe VoucherAdjustmentsController, type: :controller do
 
   describe "#create" do
     describe "adding a voucher" do
-      let(:params) { { voucher_code: voucher.code } }
+      let(:params) { { order: { voucher_code: voucher.code } } }
 
       it "adds a voucher to the user's current order" do
         post :create, params: params
@@ -39,13 +39,13 @@ describe VoucherAdjustmentsController, type: :controller do
       end
 
       context "when voucher doesn't exist" do
-        let(:params) { { voucher_code: "non_voucher" } }
+        let(:params) { { order: { voucher_code: "non_voucher" } } }
 
         it "returns 422 and an error message" do
           post :create, params: params
 
           expect(response.status).to eq 422
-          expect(flash[:error]).to match "Voucher Not found"
+          expect(flash[:error]).to match "Voucher code Not found"
         end
       end
 
diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb
index abf8a6f2f2..db6fbb3236 100644
--- a/spec/system/consumer/split_checkout_spec.rb
+++ b/spec/system/consumer/split_checkout_spec.rb
@@ -771,7 +771,7 @@ describe "As a consumer, I want to checkout my order" do
                 fill_in "Enter voucher code", with: "non_code"
                 click_button("Apply")
 
-                expect(page).to have_content("Voucher Not found")
+                expect(page).to have_content("Voucher code Not found")
               end
             end
           end

From f7912a2240a75a36d6e99682cbe29712fa53fd9c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 8 Jun 2023 19:38:46 +0100
Subject: [PATCH 20/28] Fix CSS/layout issues

---
 .../voucher_adjustments_controller.rb         |  9 ++++-
 .../split_checkout/_voucher_section.html.haml | 34 +++++++++++--------
 .../css/darkswarm/split-checkout.scss         | 20 +++++------
 spec/system/consumer/split_checkout_spec.rb   |  3 +-
 4 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb
index 9923733fbb..c34570269c 100644
--- a/app/controllers/voucher_adjustments_controller.rb
+++ b/app/controllers/voucher_adjustments_controller.rb
@@ -61,7 +61,14 @@ class VoucherAdjustmentsController < BaseController
     flash.now[:error] = @order.errors.full_messages.to_sentence
 
     render status: :unprocessable_entity, cable_ready: cable_car.
-      replace("#flashes", partial("shared/flashes", locals: { flashes: flash }))
+      replace("#flashes", partial("shared/flashes", locals: { flashes: flash })).
+      replace(
+        "#voucher-section",
+        partial(
+          "split_checkout/voucher_section",
+          locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first }
+        )
+      )
   end
 
   def voucher_params
diff --git a/app/views/split_checkout/_voucher_section.html.haml b/app/views/split_checkout/_voucher_section.html.haml
index c46dce4ec0..19298da409 100644
--- a/app/views/split_checkout/_voucher_section.html.haml
+++ b/app/views/split_checkout/_voucher_section.html.haml
@@ -1,19 +1,25 @@
 %div#voucher-section
   .checkout-title
     = t("split_checkout.step2.voucher.apply_voucher")
-  .checkout-input
-    .two-columns-inputs.voucher{"data-controller": "toggle-button-disabled"}
+  .checkout-input{"data-controller": "toggle-button-disabled"}
+    = form_with url: voucher_adjustments_path, model: @order, method: :post, data: { remote: true } do |form|
       - if voucher_adjustment.present?
-        %span.button.voucher-added
-          %i.ofn-i_051-check-big
-            = t("split_checkout.step2.voucher.voucher", voucher_amount: voucher_adjustment.originator.display_value)
-        = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") }
-        - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ?
-        - if voucher_adjustment.originator.amount > order.total
-          .checkout-input
-            %span.formError.standalone
-              = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")
+        .two-columns-inputs.voucher
+          %span.button.voucher-added
+            %i.ofn-i_051-check-big
+              = t("split_checkout.step2.voucher.voucher", voucher_amount: voucher_adjustment.originator.display_value)
+          = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") }
+
+          - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ?
+          - if voucher_adjustment.originator.amount > order.total
+            .checkout-input
+              %span.formError.standalone
+                = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")
       - else
-        = form_with url: voucher_adjustments_path, model: @order, method: :post, data: { remote: true } do |form|
-          = form.text_field :voucher_code, value: params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher"
-          = form.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }
+        .two-columns-inputs
+          %div.checkout-input
+            = form.text_field :voucher_code, value: params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher"
+            = form.error_message_on :voucher_code
+
+          %div.checkout-input
+            = form.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher-button", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" }
diff --git a/app/webpacker/css/darkswarm/split-checkout.scss b/app/webpacker/css/darkswarm/split-checkout.scss
index 3d9c05b929..1fc90b9815 100644
--- a/app/webpacker/css/darkswarm/split-checkout.scss
+++ b/app/webpacker/css/darkswarm/split-checkout.scss
@@ -412,22 +412,18 @@
     justify-content: normal;
     align-items: center;
 
-    input {
-      width: 50%;
-    }
-
     a {
       color: inherit;
     }
+  }
 
-    .button {
-      &.cancel {
-        width: 30%;
-        border-radius: 0.5em;
-        padding:0;
-        height: 2.5em;
-        background-color: $teal-400
-      }
+  .voucher-button {
+    &.cancel {
+      width: 30%;
+      border-radius: 0.35em;
+      padding:0;
+      height: 2.5em;
+      background-color: $teal-400
     }
   }
   
diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb
index db6fbb3236..148f2a212c 100644
--- a/spec/system/consumer/split_checkout_spec.rb
+++ b/spec/system/consumer/split_checkout_spec.rb
@@ -788,9 +788,10 @@ describe "As a consumer, I want to checkout my order" do
                 click_on "Remove code"
               end
 
-              within '.voucher' do
+              within '#voucher-section' do
                 expect(page).to have_button("Apply", disabled: true)
               end
+
               expect(order.voucher_adjustments.length).to eq(0)
             end
           end

From 2aa3f8eb892ceb02671f472a941617f09a3b91cd Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 8 Jun 2023 20:05:51 +0100
Subject: [PATCH 21/28] Fix flaky test

---
 spec/system/consumer/split_checkout_tax_not_incl_spec.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
index 5d9f9bcd5b..046a6ac483 100644
--- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
+++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb
@@ -127,9 +127,11 @@ describe "As a consumer, I want to see adjustment breakdown" do
           choose "Delivery"
 
           click_button "Next - Payment method"
+
           # add Voucher
           fill_in "Enter voucher code", with: voucher.code
           click_button("Apply")
+          expect(page).to have_selector ".voucher-added"
 
           click_on "Next - Order summary"
           click_on "Complete order"

From ad6d0c1c73660254d9088ae54d6e80bdfd2415c1 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 8 Jun 2023 22:31:00 +0100
Subject: [PATCH 22/28] Add nil safety in reports for zero priced orders with
 no payment method

---
 lib/reporting/reports/payments/payment_totals.rb           | 2 +-
 lib/reporting/reports/payments/payments_by_payment_type.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/reporting/reports/payments/payment_totals.rb b/lib/reporting/reports/payments/payment_totals.rb
index 06a6d1cdb4..4e1b561994 100644
--- a/lib/reporting/reports/payments/payment_totals.rb
+++ b/lib/reporting/reports/payments/payment_totals.rb
@@ -23,7 +23,7 @@ module Reporting
 
         def total_by_payment_method(orders, pay_method)
           orders.map(&:payments).flatten.select { |payment|
-            payment.completed? && payment.payment_method.name.to_s.include?(pay_method)
+            payment.completed? && payment.payment_method&.name.to_s.include?(pay_method)
           }.sum(&:amount)
         end
       end
diff --git a/lib/reporting/reports/payments/payments_by_payment_type.rb b/lib/reporting/reports/payments/payments_by_payment_type.rb
index 67d96236be..89003205df 100644
--- a/lib/reporting/reports/payments/payments_by_payment_type.rb
+++ b/lib/reporting/reports/payments/payments_by_payment_type.rb
@@ -17,7 +17,7 @@ module Reporting
           {
             payment_state: proc { |payments| payment_state(payments.first.order) },
             distributor: proc { |payments| payments.first.order.distributor.name },
-            payment_type: proc { |payments| payments.first.payment_method.name },
+            payment_type: proc { |payments| payments.first.payment_method&.name },
             total_price: proc { |payments| payments.sum(&:amount) }
           }
         end

From 55cc57cf87d723bd6d5c5be8f19a8b090acfd0cc Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 8 Jun 2023 23:37:08 +0100
Subject: [PATCH 23/28] Use pre_discount_total when comparing to voucher amount

---
 app/views/split_checkout/_voucher_section.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/split_checkout/_voucher_section.html.haml b/app/views/split_checkout/_voucher_section.html.haml
index 19298da409..fcbebda20b 100644
--- a/app/views/split_checkout/_voucher_section.html.haml
+++ b/app/views/split_checkout/_voucher_section.html.haml
@@ -11,7 +11,7 @@
           = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") }
 
           - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ?
-          - if voucher_adjustment.originator.amount > order.total
+          - if voucher_adjustment.originator.amount > order.pre_discount_total
             .checkout-input
               %span.formError.standalone
                 = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")

From 74073946e6ab43d88745acd91ce0c958bbfed341 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Fri, 30 Jun 2023 16:07:31 +1000
Subject: [PATCH 24/28] Move VoucherAdjustmentsController to request specs

---
 .../voucher_adjustments_controller_spec.rb    | 78 -------------------
 spec/requests/voucher_adjustments_spec.rb     | 71 ++++++++++++++---
 2 files changed, 62 insertions(+), 87 deletions(-)
 delete mode 100644 spec/controllers/voucher_adjustments_controller_spec.rb

diff --git a/spec/controllers/voucher_adjustments_controller_spec.rb b/spec/controllers/voucher_adjustments_controller_spec.rb
deleted file mode 100644
index df11d9c06b..0000000000
--- a/spec/controllers/voucher_adjustments_controller_spec.rb
+++ /dev/null
@@ -1,78 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe VoucherAdjustmentsController, type: :controller do
-  let(:user) { order.user }
-  let(:address) { create(:address) }
-  let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) }
-  let(:order_cycle) { create(:order_cycle, distributors: [distributor]) }
-  let(:exchange) { order_cycle.exchanges.outgoing.first }
-  let(:order) {
-    create(:order_with_line_items, line_items_count: 1, distributor: distributor,
-                                   order_cycle: order_cycle, bill_address: address,
-                                   ship_address: address)
-  }
-  let(:payment_method) { distributor.payment_methods.first }
-  let(:shipping_method) { distributor.shipping_methods.first }
-
-  let(:voucher) { create(:voucher, code: 'some_code', enterprise: distributor) }
-
-  before do
-    exchange.variants << order.line_items.first.variant
-    order.select_shipping_method shipping_method.id
-    OrderWorkflow.new(order).advance_to_payment
-
-    allow(controller).to receive(:current_order) { order }
-    allow(controller).to receive(:spree_current_user) { user }
-  end
-
-  describe "#create" do
-    describe "adding a voucher" do
-      let(:params) { { order: { voucher_code: voucher.code } } }
-
-      it "adds a voucher to the user's current order" do
-        post :create, params: params
-
-        expect(response.status).to eq(200)
-        expect(order.reload.voucher_adjustments.length).to eq(1)
-      end
-
-      context "when voucher doesn't exist" do
-        let(:params) { { order: { voucher_code: "non_voucher" } } }
-
-        it "returns 422 and an error message" do
-          post :create, params: params
-
-          expect(response.status).to eq 422
-          expect(flash[:error]).to match "Voucher code Not found"
-        end
-      end
-
-      context "when adding fails" do
-        it "returns 422 and an error message" do
-          # Create a non valid adjustment
-          adjustment = build(:adjustment, label: nil)
-          allow(voucher).to receive(:create_adjustment).and_return(adjustment)
-          allow(Voucher).to receive(:find_by).and_return(voucher)
-
-          post :create, params: params
-
-          expect(response.status).to eq 422
-          expect(flash[:error]).to match(
-            "There was an error while adding the voucher and Label can't be blank"
-          )
-        end
-      end
-    end
-  end
-
-  describe "#destroy" do
-    it "removes the voucher from the current order" do
-      put :destroy, params: { id: voucher.id }
-
-      expect(order.reload.voucher_adjustments.count).to eq 0
-      expect(response.status).to eq 200
-    end
-  end
-end
diff --git a/spec/requests/voucher_adjustments_spec.rb b/spec/requests/voucher_adjustments_spec.rb
index fc533473b8..8c93fb3f69 100644
--- a/spec/requests/voucher_adjustments_spec.rb
+++ b/spec/requests/voucher_adjustments_spec.rb
@@ -4,27 +4,80 @@ require 'spec_helper'
 
 describe VoucherAdjustmentsController, type: :request do
   let(:user) { order.user }
+  let(:address) { create(:address) }
   let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) }
-  let(:order) { create( :order_with_line_items, line_items_count: 1, distributor: distributor) }
+  let(:order_cycle) { create(:order_cycle, distributors: [distributor]) }
+  let(:exchange) { order_cycle.exchanges.outgoing.first }
+  let(:order) do
+    create(
+      :order_with_line_items,
+      line_items_count: 1,
+      distributor: distributor,
+      order_cycle: order_cycle,
+      bill_address: address,
+      ship_address: address
+    )
+  end
+  let(:shipping_method) { distributor.shipping_methods.first }
   let(:voucher) { create(:voucher, code: 'some_code', enterprise: distributor) }
-  let!(:adjustment) { voucher.create_adjustment(voucher.code, order) }
 
   before do
-    # Make sure the order is created by the order user, the factory doesn't set ip properly
-    order.created_by = user
-    order.save!
+    order.update!(created_by: user)
+
+    order.select_shipping_method shipping_method.id
+    OrderWorkflow.new(order).advance_to_payment
 
     sign_in user
   end
 
+  describe "POST voucher_adjustments" do
+    let(:params) { { order: { voucher_code: voucher.code } } }
+
+    it "adds a voucher to the user's current order" do
+      post "/voucher_adjustments", params: params
+
+      expect(response).to be_successful
+      expect(order.reload.voucher_adjustments.length).to eq(1)
+    end
+
+    context "when voucher doesn't exist" do
+      let(:params) { { order: { voucher_code: "non_voucher" } } }
+
+      it "returns 422 and an error message" do
+        post "/voucher_adjustments", params: params
+
+        expect(response).to be_unprocessable
+        expect(flash[:error]).to match "Voucher code Not found"
+      end
+    end
+
+    context "when adding fails" do
+      it "returns 422 and an error message" do
+        # Create a non valid adjustment
+        bad_adjustment = build(:adjustment, label: nil)
+        allow(voucher).to receive(:create_adjustment).and_return(bad_adjustment)
+        allow(Voucher).to receive(:find_by).and_return(voucher)
+
+        post "/voucher_adjustments", params: params
+
+        expect(response).to be_unprocessable
+        expect(flash[:error]).to match(
+          "There was an error while adding the voucher and Label can't be blank"
+        )
+      end
+    end
+  end
+
   describe "DELETE voucher_adjustments/:id" do
+    let!(:adjustment) { voucher.create_adjustment(voucher.code, order) }
+
     it "deletes the voucher adjustment" do
       delete "/voucher_adjustments/#{adjustment.id}"
 
-      expect(order.voucher_adjustments.length).to eq(0)
+      expect(order.voucher_adjustments.reload.length).to eq(0)
     end
 
-    it "render a succesful response" do
+    it "render a success response" do
       delete "/voucher_adjustments/#{adjustment.id}"
 
       expect(response).to be_successful
@@ -34,10 +87,10 @@ describe VoucherAdjustmentsController, type: :request do
       it "does nothing" do
         delete "/voucher_adjustments/-1"
 
-        expect(order.voucher_adjustments.length).to eq(1)
+        expect(order.voucher_adjustments.reload.length).to eq(1)
       end
 
-      it "render a succesful response" do
+      it "render a success response" do
         delete "/voucher_adjustments/-1"
 
         expect(response).to be_successful

From 66a546027f7f00df2c44791ab5be8f653a945e9a Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 4 Jul 2023 13:03:41 +0100
Subject: [PATCH 25/28] Add test coverage for handling zero priced orders

---
 .../split_checkout_controller_spec.rb         | 20 +++++++++++++++++++
 spec/models/spree/order/payment_spec.rb       | 17 ++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/spec/controllers/split_checkout_controller_spec.rb b/spec/controllers/split_checkout_controller_spec.rb
index 7783717e28..b493f4ab21 100644
--- a/spec/controllers/split_checkout_controller_spec.rb
+++ b/spec/controllers/split_checkout_controller_spec.rb
@@ -212,6 +212,26 @@ describe SplitCheckoutController, type: :controller do
         end
       end
 
+      context "with a zero-priced order" do
+        let(:params) do
+          { step: "payment", order: { payments_attributes: [{ amount: 0 }] } }
+        end
+
+        before do
+          order.line_items.first.update(price: 0)
+          order.update_totals_and_states
+        end
+
+        it "allows proceeding to confirmation" do
+          put :update, params: params
+
+          expect(response).to redirect_to checkout_step_path(:summary)
+          expect(order.reload.state).to eq "confirmation"
+          expect(order.payments.count).to eq 1
+          expect(order.payments.first.amount).to eq 0
+        end
+      end
+
       context "with a saved credit card" do
         let!(:saved_card) { create(:stored_credit_card, user: user) }
         let(:checkout_params) do
diff --git a/spec/models/spree/order/payment_spec.rb b/spec/models/spree/order/payment_spec.rb
index 224f0be0dc..c7eaa673b3 100644
--- a/spec/models/spree/order/payment_spec.rb
+++ b/spec/models/spree/order/payment_spec.rb
@@ -50,5 +50,22 @@ module Spree
 
       order.process_payments!
     end
+
+    context "with a zero-priced order" do
+      let!(:zero_order) {
+        create(:order, state: "payment", line_items: [create(:line_item, price: 0)])
+      }
+      let!(:zero_payment) { create(:payment, order: zero_order, amount: 0, state: "checkout") }
+      let(:updater) { OrderManagement::Order::Updater.new(zero_order) }
+
+      it "processes payments successfully" do
+        zero_order.process_payments!
+        updater.update_payment_state
+
+        expect(zero_order.payment_state).to eq "paid"
+        expect(zero_payment.reload.state).to eq "completed"
+        expect(zero_payment.captured_at).to_not be_nil
+      end
+    end
   end
 end

From 671dfc7082f864f1c8274e1dad664a610294b2d5 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 4 Jul 2023 14:42:31 +0100
Subject: [PATCH 26/28] Don't flush errors when checking zero priced order
 validity

---
 app/models/spree/order.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 21904aa05d..ab62fb0372 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -222,7 +222,7 @@ module Spree
     # There are items present in the order, but either the items have zero price,
     # or the order's total has been modified (maybe discounted) to zero.
     def zero_priced_order?
-      valid? && line_items.count.positive? && total.zero?
+      dup.valid? && line_items.count.positive? && total.zero?
     end
 
     # Returns the relevant zone (if any) to be used for taxation purposes.

From 7e0042ea1d6338ed31b827d0e93270a6a4b9ea26 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 5 Jul 2023 11:27:03 +1000
Subject: [PATCH 27/28] Apply code suggestion

Co-authored-by: Gaetan Craig-Riou <gaetan.riou@gmail.com>

Co-authored-by: Gaetan Craig-Riou <40413322+rioug@users.noreply.github.com>
---
 app/helpers/spree/payment_methods_helper.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/helpers/spree/payment_methods_helper.rb b/app/helpers/spree/payment_methods_helper.rb
index 531848ab14..bfd4a72e26 100644
--- a/app/helpers/spree/payment_methods_helper.rb
+++ b/app/helpers/spree/payment_methods_helper.rb
@@ -4,7 +4,8 @@ module Spree
   module PaymentMethodsHelper
     def payment_method(payment)
       # hack to allow us to retrieve the name of a "deleted" payment method
-      return unless (id = payment.payment_method_id)
+      id = payment.payment_method_id
+      return if id.nil?
 
       Spree::PaymentMethod.find_with_destroyed(id)
     end

From 652beac73f92619ed9f3bbda8089ff545615c4dc Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 5 Jul 2023 15:31:05 +0100
Subject: [PATCH 28/28] Drop valid? check from zero_priced_order? method

---
 app/models/spree/order.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index ab62fb0372..dead0129fc 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -222,7 +222,7 @@ module Spree
     # There are items present in the order, but either the items have zero price,
     # or the order's total has been modified (maybe discounted) to zero.
     def zero_priced_order?
-      dup.valid? && line_items.count.positive? && total.zero?
+      line_items.count.positive? && total.zero?
     end
 
     # Returns the relevant zone (if any) to be used for taxation purposes.