From 8813edaed88b2afca480419e88c89d0dfdc56963 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 6 Jun 2023 14:59:03 +0100
Subject: [PATCH 01/16] Fix variables in form partial

---
 app/views/spree/admin/orders/_form.html.haml | 9 +++++----
 app/views/spree/admin/orders/edit.html.haml  | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml
index 9f8b00c6e2..02969f99e0 100644
--- a/app/views/spree/admin/orders/_form.html.haml
+++ b/app/views/spree/admin/orders/_form.html.haml
@@ -5,17 +5,18 @@
   - if !@order.completed? && @order.insufficient_stock_lines.any?
     = render 'spree/admin/orders/insufficient_stock_lines', insufficient_stock_lines: @order.insufficient_stock_lines
 
-  = render :partial => "spree/admin/orders/shipment", :collection => @order.shipments, :locals => { :order => order }
-  - if order.line_items.exists?
+  = render :partial => "spree/admin/orders/shipment", :collection => @order.shipments, :locals => { :order => @order }
+
+  - if @order.line_items.exists?
     = render partial: "spree/admin/orders/note", locals: { order: @order }
 
   = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => @order.line_item_adjustments, :title => t(".line_item_adjustments")}
   = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => order_adjustments_for_display(@order), :title => t(".order_adjustments")}
 
-  - if order.line_items.exists?
+  - if @order.line_items.exists?
     %fieldset#order-total.no-border-bottom.order-details-total
       %legend{ align: 'center' }= t(".order_total")
-      %span.order-total= order.display_total
+      %span.order-total= @order.display_total
 
   = form_for @order, url: spree.admin_order_url(@order), method: :put do |f|
     = render partial: 'spree/admin/orders/_form/distribution_fields'
diff --git a/app/views/spree/admin/orders/edit.html.haml b/app/views/spree/admin/orders/edit.html.haml
index b8556d861a..6a88c85ebd 100644
--- a/app/views/spree/admin/orders/edit.html.haml
+++ b/app/views/spree/admin/orders/edit.html.haml
@@ -32,4 +32,4 @@
         = Spree.t(:your_order_is_empty_add_product)
 
     %div.admin-order-edit-form
-      = render :partial => 'form', :locals => { :order => @order }
+      = render partial: 'form'

From 2bfb57a1a170fa0cc3b9471631f3e9c9cf0dd1df Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 6 Jun 2023 14:59:53 +0100
Subject: [PATCH 02/16] Clarify shipments partial inclusion

---
 app/views/spree/admin/orders/_form.html.haml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml
index 02969f99e0..7a6adfbe15 100644
--- a/app/views/spree/admin/orders/_form.html.haml
+++ b/app/views/spree/admin/orders/_form.html.haml
@@ -5,7 +5,8 @@
   - if !@order.completed? && @order.insufficient_stock_lines.any?
     = render 'spree/admin/orders/insufficient_stock_lines', insufficient_stock_lines: @order.insufficient_stock_lines
 
-  = render :partial => "spree/admin/orders/shipment", :collection => @order.shipments, :locals => { :order => @order }
+  - if @order.shipments.any?
+    = render :partial => "spree/admin/orders/shipment", :collection => @order.shipments, :locals => { :order => @order }
 
   - if @order.line_items.exists?
     = render partial: "spree/admin/orders/note", locals: { order: @order }

From 67c3e09dba735fd9bd5ae37c3c73d27d6414e93e Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 6 Jun 2023 15:03:55 +0100
Subject: [PATCH 03/16] Use RESTful routes for orders controller actions

---
 .../spree/admin/orders_controller.rb          | 32 ++++++++-----------
 app/models/spree/order.rb                     |  5 +--
 .../spree/admin/orders/distribution.html.haml | 27 ----------------
 app/views/spree/admin/orders/new.html.haml    | 19 ++++-------
 app/views/spree/admin/shared/_routes.html.erb |  2 +-
 config/routes/spree.rb                        |  2 --
 spec/system/admin/order_spec.rb               | 27 ----------------
 7 files changed, 24 insertions(+), 90 deletions(-)
 delete mode 100644 app/views/spree/admin/orders/distribution.html.haml

diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb
index 80c3707ca3..0e5cb4b825 100644
--- a/app/controllers/spree/admin/orders_controller.rb
+++ b/app/controllers/spree/admin/orders_controller.rb
@@ -8,9 +8,8 @@ module Spree
       include OpenFoodNetwork::SpreeApiKeyLoader
       helper CheckoutHelper
 
-      before_action :load_order, only: [:edit, :update, :fire, :resend,
-                                        :invoice, :print, :distribution]
-      before_action :load_distribution_choices, only: [:new, :edit, :update, :distribution]
+      before_action :load_order, only: [:edit, :update, :fire, :resend, :invoice, :print]
+      before_action :load_distribution_choices, only: [:new, :create, :edit, :update]
       before_action :require_distributor_abn, only: :invoice
       before_action :restore_saved_query!, only: :index
 
@@ -24,22 +23,7 @@ module Spree
       end
 
       def new
-        @order = Order.create
-        @order.created_by = spree_current_user
-        @order.generate_order_number
-        @order.save
-        redirect_to spree.distribution_admin_order_path(@order)
-      end
-
-      def distribution
-        return if order_params.blank?
-
-        on_update
-
-        @order.assign_attributes(order_params)
-        return unless @order.save(context: :set_distribution_step)
-
-        redirect_to spree.admin_order_customer_path(@order)
+        @order = Spree::Order.new
       end
 
       def edit
@@ -47,6 +31,16 @@ module Spree
         @order.errors.clear
       end
 
+      def create
+        @order = Spree::Order.new(order_params.merge(created_by: spree_current_user))
+
+        if @order.save(context: :require_distribution)
+          redirect_to spree.admin_order_customer_path(@order)
+        else
+          render :new
+        end
+      end
+
       def update
         on_update
 
diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 74a4f332d9..aaab5875ff 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -104,8 +104,9 @@ module Spree
     validates :email, presence: true,
                       format: /\A([\w.%+\-']+)@([\w\-]+\.)+(\w{2,})\z/i,
                       if: :require_email
-    validates :order_cycle, presence: true, on: :set_distribution_step
-    validates :distributor, presence: true, on: :set_distribution_step
+
+    validates :order_cycle, presence: true, on: :require_distribution
+    validates :distributor, presence: true, on: :require_distribution
 
     make_permalink field: :number
 
diff --git a/app/views/spree/admin/orders/distribution.html.haml b/app/views/spree/admin/orders/distribution.html.haml
deleted file mode 100644
index ac577f0dd4..0000000000
--- a/app/views/spree/admin/orders/distribution.html.haml
+++ /dev/null
@@ -1,27 +0,0 @@
-- content_for :page_actions do
-  %li= button_link_to t(:back_to_orders_list), spree.admin_orders_path, :icon => 'icon-arrow-left'
-
-= admin_inject_shops(module: 'admin.orders')
-= admin_inject_order_cycles
-
-- content_for :page_title do
-  = t(:new_order)
-  \#
-  = @order.number
-
-= render 'spree/admin/shared/order_tabs', :current => 'Customer Details'
-
-= csrf_meta_tags
-
-%div
-  = render 'spree/shared/error_messages', :target => @order
-
-%div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl"}
-  = form_for @order, url: distribution_admin_order_path(@order), method: :put do |f|
-    = render 'spree/admin/orders/_form/distribution_fields'
-    -# This param passed to stop validation error in next page due to no line items in order yet:
-    = hidden_field_tag 'suppress_error_msg', "true"
-    = hidden_field_tag "set_distribution_step", "true"
-    = button_tag :class => 'secondary radius expand small', :id => 'update-button' do
-      %i.icon-arrow-right
-      = t(:next)
diff --git a/app/views/spree/admin/orders/new.html.haml b/app/views/spree/admin/orders/new.html.haml
index 556e464cd4..b43d50fc9e 100644
--- a/app/views/spree/admin/orders/new.html.haml
+++ b/app/views/spree/admin/orders/new.html.haml
@@ -1,7 +1,5 @@
 - content_for :page_title do
   = t(:new_order)
-  \#
-  = @order.number
 
 - content_for :page_actions do
   %li= button_link_to t(:back_to_orders_list), spree.admin_orders_path, :icon => 'icon-arrow-left'
@@ -9,17 +7,14 @@
 = admin_inject_shops(module: 'admin.orders')
 = admin_inject_order_cycles
 
-= render 'spree/admin/shared/order_tabs', :current => 'Order Details'
-
-= csrf_meta_tags
+= render 'spree/admin/shared/order_tabs', current: 'Order Details'
 
 %div
-  = render 'spree/shared/error_messages', :target => @order
+  = render 'spree/shared/error_messages', target: @order
 
 %div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl"}
-  %div{"ng-show" => "distributionChosen()"}
-    = render 'add_product'
-
-  - unless @order.line_items.any?
-    %div
-      = render 'form'
+  = form_for @order, url: spree.admin_orders_path, method: :post do
+    = render 'spree/admin/orders/_form/distribution_fields'
+    = button_tag class: 'secondary radius expand small', id: 'update-button' do
+      %i.icon-arrow-right
+      = t(:next)
diff --git a/app/views/spree/admin/shared/_routes.html.erb b/app/views/spree/admin/shared/_routes.html.erb
index dc8fac6b9d..27adba0559 100644
--- a/app/views/spree/admin/shared/_routes.html.erb
+++ b/app/views/spree/admin/shared/_routes.html.erb
@@ -10,6 +10,6 @@
     :taxons_search         => main_app.api_v0_taxons_url(:format => 'json'),
     :orders_api            => main_app.api_v0_orders_url,
     :states_search         => main_app.api_v0_states_url(:format => 'json'),
-    :cancel_order          => spree.fire_admin_order_url(id: @order ? @order.number : "", e: 'cancel')
+    :cancel_order          => spree.fire_admin_order_url(id: @order&.number ? @order.number : "", e: 'cancel')
   }.to_json %>;
 </script>
diff --git a/config/routes/spree.rb b/config/routes/spree.rb
index 28834db308..a6e4bab5ed 100644
--- a/config/routes/spree.rb
+++ b/config/routes/spree.rb
@@ -89,8 +89,6 @@ Spree::Core::Engine.routes.draw do
         get :resend
         get :invoice
         get :print
-        get :distribution
-        put :distribution
       end
 
       collection do
diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb
index 366c2f2a9a..8f77ff4f9d 100644
--- a/spec/system/admin/order_spec.rb
+++ b/spec/system/admin/order_spec.rb
@@ -116,33 +116,6 @@ describe '
     end
   end
 
-  context "when order has a distributor and order cycle" do
-    before do
-      order.distributor = distributor
-      order.order_cycle = order_cycle
-      order.save!
-      login_as_admin
-      visit spree.distribution_admin_order_path(order)
-    end
-
-    it "can access the `/distribution` step" do
-      expect(current_path).to eq spree.distribution_admin_order_path(order)
-      expect(page).to have_content "DISTRIBUTION"
-    end
-
-    it "shows the distributor and order cycle" do
-      expect(page).to have_content distributor.name
-      expect(page).to have_content order_cycle.name
-    end
-
-    it "shows links to other steps" do
-      expect(page).to have_content "CUSTOMER DETAILS"
-      expect(page).to have_content "ORDER DETAILS"
-      expect(page).to have_content "PAYMENTS"
-      expect(page).to have_content "ADJUSTMENTS"
-    end
-  end
-
   context "when creating an order with a customer-only" do
     let(:customer2) { create(:customer, enterprise: distributor) }
     let(:customer3) { create(:customer, enterprise: distributor) }

From e88843c733e23e43ff459dab2ab523f1f00d6634 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 6 Jun 2023 15:04:58 +0100
Subject: [PATCH 04/16] Ensure order number is generated nicely on new records

---
 app/models/spree/order.rb       | 5 +++--
 spec/models/spree/order_spec.rb | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index aaab5875ff..64bb68fa93 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -89,7 +89,7 @@ module Spree
 
     # Needs to happen before save_permalink is called
     before_validation :set_currency
-    before_validation :generate_order_number, on: :create
+    before_validation :generate_order_number, if: :new_record?
     before_validation :clone_billing_address, if: :use_billing?
     before_validation :ensure_customer
 
@@ -291,8 +291,9 @@ module Spree
                                                    created_by_id: created_by_id)
     end
 
-    # FIXME refactor this method and implement validation using validates_* utilities
     def generate_order_number
+      return if number.present?
+
       record = true
       while record
         random = "R#{Array.new(9){ rand(9) }.join}"
diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 5c250f970e..4de8da94c1 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -40,6 +40,7 @@ describe Spree::Order do
   context "#generate_order_number" do
     it "should generate a random string" do
       expect(order.generate_order_number.is_a?(String)).to be_truthy
+      order.number = nil
       expect(!order.generate_order_number.to_s.empty?).to be_truthy
     end
   end

From c7bb24e2a025fee1e5ab41c6b7145389374ca84d Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 6 Jun 2023 15:23:38 +0100
Subject: [PATCH 05/16] Remove dead code and clarify update logic

---
 .../spree/admin/orders_controller.rb           | 18 +++++++++---------
 app/views/spree/admin/orders/edit.html.haml    |  5 +----
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb
index 0e5cb4b825..9e141d7674 100644
--- a/app/controllers/spree/admin/orders_controller.rb
+++ b/app/controllers/spree/admin/orders_controller.rb
@@ -44,16 +44,9 @@ module Spree
       def update
         on_update
 
-        if params[:set_distribution_step] && @order.update(order_params)
-          OrderWorkflow.new(@order).advance_to_payment if @order.state.in? ["cart", "address", "delivery"]
-          return redirect_to spree.admin_order_customer_path(@order)
-        end
-
-        unless order_params.present? && @order.update(order_params) && @order.line_items.present?
-          if @order.line_items.empty? && !params[:suppress_error_msg]
-            @order.errors.add(:line_items, Spree.t('errors.messages.blank'))
-          end
+        order_updated = order_params.present? && @order.update(order_params)
 
+        unless order_updated && line_items_present?
           flash[:error] = @order.errors.full_messages.join(', ') if @order.errors.present?
           return redirect_to spree.edit_admin_order_path(@order)
         end
@@ -111,6 +104,13 @@ module Spree
 
       private
 
+      def line_items_present?
+        return true if @order.line_items.any?
+
+        @order.errors.add(:line_items, Spree.t('errors.messages.blank'))
+        false
+      end
+
       def update_search_results
         session[:admin_orders_search] = search_params
 
diff --git a/app/views/spree/admin/orders/edit.html.haml b/app/views/spree/admin/orders/edit.html.haml
index 6a88c85ebd..6e290d3502 100644
--- a/app/views/spree/admin/orders/edit.html.haml
+++ b/app/views/spree/admin/orders/edit.html.haml
@@ -16,10 +16,7 @@
 = render partial: "spree/admin/shared/order_tabs", locals: { current: 'Order Details' }
 
 %div
-  -# Suppress errors when manually creating a new order - needs to proceed to edit page
-  -# without having line items (which otherwise gives a validation error)
-  - unless params["suppress_error_msg"]
-    = render partial: "spree/shared/error_messages", :locals => { :target => @order }
+  = render partial: "spree/shared/error_messages", locals: { target: @order }
 
   = admin_inject_shops(module: 'admin.orders')
   = admin_inject_order_cycles

From 6e8ed1f61206158b9182928210f853490a9338b1 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 6 Jun 2023 16:32:49 +0100
Subject: [PATCH 06/16] Hide secondary form until the order has line items

---
 app/views/spree/admin/orders/_form.html.haml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml
index 7a6adfbe15..4d43acf8dd 100644
--- a/app/views/spree/admin/orders/_form.html.haml
+++ b/app/views/spree/admin/orders/_form.html.haml
@@ -19,12 +19,12 @@
       %legend{ align: 'center' }= t(".order_total")
       %span.order-total= @order.display_total
 
-  = form_for @order, url: spree.admin_order_url(@order), method: :put do |f|
-    = render partial: 'spree/admin/orders/_form/distribution_fields'
+    = form_for @order, url: spree.admin_order_url(@order), method: :put do |f|
+      = render partial: 'spree/admin/orders/_form/distribution_fields'
 
-    .filter-actions.actions{"ng-show" => "distributionChosen()"}
-      = button t(:update_and_recalculate_fees), 'icon-refresh'
-      = link_to_with_icon 'button icon-arrow-left', t(:back), spree.admin_orders_url
+      .filter-actions.actions{"ng-show" => "distributionChosen()"}
+        = button t(:update_and_recalculate_fees), 'icon-refresh'
+        = link_to_with_icon 'button icon-arrow-left', t(:back), spree.admin_orders_url
 
   = javascript_tag do
     var order_number = '#{@order.number}';

From e1b37090be15403658a1a3ef471ed4b667705850 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 6 Jun 2023 18:13:17 +0100
Subject: [PATCH 07/16] Use _path helper instead of _url

---
 app/views/spree/admin/orders/customer_details/edit.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/spree/admin/orders/customer_details/edit.html.haml b/app/views/spree/admin/orders/customer_details/edit.html.haml
index fe68760d93..28d5f0fd27 100644
--- a/app/views/spree/admin/orders/customer_details/edit.html.haml
+++ b/app/views/spree/admin/orders/customer_details/edit.html.haml
@@ -22,6 +22,6 @@
 
 = render :partial => 'spree/shared/error_messages', :locals => { :target => @order }
 
-= form_for @order, :url => admin_order_customer_url(@order) do |f|
+= form_for @order, :url => admin_order_customer_path(@order) do |f|
   = render 'form', :f => f
   = f.hidden_field :customer_id, value: @order.customer_id, id: "customer_id"

From acc34d1deb726c5d1e2694e497de8eae4fefb33a Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 00:26:46 +0100
Subject: [PATCH 08/16] Update advance to payment logic

---
 .../spree/admin/orders/customer_details_controller.rb           | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/spree/admin/orders/customer_details_controller.rb b/app/controllers/spree/admin/orders/customer_details_controller.rb
index 0ce7ccc166..22f90d858a 100644
--- a/app/controllers/spree/admin/orders/customer_details_controller.rb
+++ b/app/controllers/spree/admin/orders/customer_details_controller.rb
@@ -25,7 +25,7 @@ module Spree
 
             refresh_shipment_rates
             recalculate_taxes
-            OrderWorkflow.new(@order).advance_to_payment
+            OrderWorkflow.new(@order).advance_to_payment if @order.state.in? ["cart", "address", "delivery"]
 
             flash[:success] = Spree.t('customer_details_updated')
             redirect_to spree.admin_order_customer_path(@order)

From cf0f148dba972055a548bf3b62fbf179e60ce74b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 00:50:08 +0100
Subject: [PATCH 09/16] Advance order when creating a shipment

This action gets called from the order edit page when adding line items and it's one of the places that needs to advance the order *before* redirecting back to the order edit action
---
 app/controllers/api/v0/shipments_controller.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb
index 0905abe954..98a705e277 100644
--- a/app/controllers/api/v0/shipments_controller.rb
+++ b/app/controllers/api/v0/shipments_controller.rb
@@ -21,6 +21,10 @@ module Api
         @shipment.refresh_rates
         @shipment.save!
 
+        if @order.line_items.any? && @order.state.in?(["cart", "address", "delivery"])
+          OrderWorkflow.new(@order).advance_to_payment
+        end
+
         render json: @shipment.reload, serializer: Api::ShipmentSerializer, status: :ok
       end
 

From f3ee10dd5ab0a7d75b0359477d9ab46c12a12946 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 00:42:16 +0100
Subject: [PATCH 10/16] Simplify by moving conditional to guard clause

---
 app/controllers/api/v0/shipments_controller.rb              | 6 ++----
 .../spree/admin/orders/customer_details_controller.rb       | 2 +-
 app/controllers/spree/admin/orders_controller.rb            | 3 ++-
 app/services/order_workflow.rb                              | 2 ++
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb
index 98a705e277..b1be516316 100644
--- a/app/controllers/api/v0/shipments_controller.rb
+++ b/app/controllers/api/v0/shipments_controller.rb
@@ -21,11 +21,9 @@ module Api
         @shipment.refresh_rates
         @shipment.save!
 
-        if @order.line_items.any? && @order.state.in?(["cart", "address", "delivery"])
-          OrderWorkflow.new(@order).advance_to_payment
-        end
+        OrderWorkflow.new(@order).advance_to_payment if @order.line_items.any?
 
-        render json: @shipment.reload, serializer: Api::ShipmentSerializer, status: :ok
+        render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok
       end
 
       def update
diff --git a/app/controllers/spree/admin/orders/customer_details_controller.rb b/app/controllers/spree/admin/orders/customer_details_controller.rb
index 22f90d858a..0ce7ccc166 100644
--- a/app/controllers/spree/admin/orders/customer_details_controller.rb
+++ b/app/controllers/spree/admin/orders/customer_details_controller.rb
@@ -25,7 +25,7 @@ module Spree
 
             refresh_shipment_rates
             recalculate_taxes
-            OrderWorkflow.new(@order).advance_to_payment if @order.state.in? ["cart", "address", "delivery"]
+            OrderWorkflow.new(@order).advance_to_payment
 
             flash[:success] = Spree.t('customer_details_updated')
             redirect_to spree.admin_order_customer_path(@order)
diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb
index 9e141d7674..263e60ae9a 100644
--- a/app/controllers/spree/admin/orders_controller.rb
+++ b/app/controllers/spree/admin/orders_controller.rb
@@ -51,7 +51,8 @@ module Spree
           return redirect_to spree.edit_admin_order_path(@order)
         end
 
-        OrderWorkflow.new(@order).advance_to_payment if @order.state.in? ["cart", "address", "delivery"]
+        OrderWorkflow.new(@order).advance_to_payment
+
         if @order.complete?
           redirect_to spree.edit_admin_order_path(@order)
         else
diff --git a/app/services/order_workflow.rb b/app/services/order_workflow.rb
index 46596b67ce..53d7813e57 100644
--- a/app/services/order_workflow.rb
+++ b/app/services/order_workflow.rb
@@ -24,6 +24,8 @@ class OrderWorkflow
   end
 
   def advance_to_payment
+    return unless order.state.in? ["cart", "address", "delivery"]
+
     advance_to_state("payment", advance_order_options)
   end
 

From 6daf29400f88a0068e1ebdd5110a254c07373d03 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 00:27:21 +0100
Subject: [PATCH 11/16] Improve distribution and line items tests

---
 spec/system/admin/order_spec.rb | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb
index 8f77ff4f9d..ef81d6f3e0 100644
--- a/spec/system/admin/order_spec.rb
+++ b/spec/system/admin/order_spec.rb
@@ -66,17 +66,13 @@ describe '
     expect(page).not_to have_content "Line items can't be blank"
 
     expect(page).to have_selector 'h1', text: 'Customer Details'
-    o = Spree::Order.last
-    expect(o.distributor).to eq(distributor)
-    expect(o.order_cycle).to eq(order_cycle)
+
+    order = Spree::Order.last
+    expect(order.distributor).to eq(distributor)
+    expect(order.order_cycle).to eq(order_cycle)
+    expect(order.line_items.count).to be_zero
 
     click_link "Order Details"
-    click_button "Update And Recalculate Fees"
-    expect(page).to have_selector '.flash.error'
-    expect(page).to have_content "Line items can't be blank"
-
-    # it suppresses validation errors when setting distribution
-    expect(page).not_to have_selector '#errorExplanation'
     expect(page).to have_content 'ADD PRODUCT'
     select2_select product.name, from: 'add_variant_id', search: true
     find('button.add_variant').click
@@ -84,7 +80,7 @@ describe '
     page.has_selector?("table.index tbody tr")
     expect(page).to have_selector 'td', text: product.name
 
-    click_button 'Update'
+    expect(order.reload.line_items.count).to be_positive
   end
 
   context "can't create an order without selecting a distributor nor an order cycle" do

From 7a7ab17db4907aac8633b3518c704f64bc092774 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 00:28:43 +0100
Subject: [PATCH 12/16] Improve customer tests

---
 spec/system/admin/order_spec.rb | 44 ++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb
index ef81d6f3e0..6869c5593b 100644
--- a/spec/system/admin/order_spec.rb
+++ b/spec/system/admin/order_spec.rb
@@ -113,33 +113,49 @@ describe '
   end
 
   context "when creating an order with a customer-only" do
+    let!(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) }
     let(:customer2) { create(:customer, enterprise: distributor) }
     let(:customer3) { create(:customer, enterprise: distributor) }
 
     before do
       login_as_admin
-      visit spree.new_admin_order_path
-      select2_select distributor.name, from: 'order_distributor_id'
-      select2_select order_cycle.name, from: 'order_order_cycle_id'
-      click_button 'Next'
-      expect(Spree::Order.last.customer_id).to be_nil
+      visit spree.admin_order_customer_path(order)
+    end
+
+    it "sets the customer on the order" do
+      expect(order.customer_id).to be_nil
+
       tomselect_search_and_select customer2.email, from: 'customer_search_override'
       check 'order_use_billing'
       click_button 'Update'
       expect(page).to have_content 'Customer Details updated'
+
+      expect(order.reload.customer).to eq customer2
     end
 
-    it "set the right customer attached to the order" do
-      expect(Spree::Order.last.reload.customer).to eq customer2
-    end
+    context "when changing the attached customer" do
+      before do
+        order.update(
+          customer: customer2,
+          email: customer2.email,
+          ship_address: customer2.ship_address,
+          bill_address: customer2.bill_address
+        )
+        visit spree.admin_order_customer_path(order)
+      end
+
+      it "should update the order customer (not only its details)" do
+        expect(page).to have_field 'order_email', with: customer2.email
+        tomselect_search_and_select customer3.email, from: 'customer_search_override'
+        check 'order_use_billing'
 
-    it "when changing the attached customer,"\
-       "it should update the order customer (not only its details)" do
-      tomselect_search_and_select customer3.email, from: 'customer_search_override'
-      expect do
-        click_button 'Update'
         expect(page).to have_field 'order_email', with: customer3.email
-      end.to change { Spree::Order.last.reload.customer }.from(customer2).to(customer3)
+
+        expect do
+          click_button 'Update'
+          expect(page).to have_content 'Customer Details updated'
+        end.to change { order.reload.customer }.from(customer2).to(customer3)
+      end
     end
   end
 

From daf243996b1a602ab1c38c04e253e7e9caaf6ec1 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 00:28:54 +0100
Subject: [PATCH 13/16] Improve flaky spec

---
 spec/system/admin/order_spec.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb
index 6869c5593b..6bc7a63b13 100644
--- a/spec/system/admin/order_spec.rb
+++ b/spec/system/admin/order_spec.rb
@@ -313,7 +313,6 @@ describe '
         within(".modal") do
           expect do
             click_on("OK")
-            expect(page).not_to have_css(".modal")
           end.to change { order.reload.line_items.length }.by(-1)
         end
       end

From 968bf882c402eb6259d5b8350a6a05eb3674a767 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 00:33:27 +0100
Subject: [PATCH 14/16] Fix flaky spec

---
 spec/system/admin/order_spec.rb | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb
index 6bc7a63b13..93d098b31c 100644
--- a/spec/system/admin/order_spec.rb
+++ b/spec/system/admin/order_spec.rb
@@ -409,14 +409,7 @@ describe '
       fill_in "order_bill_address_attributes_city", with: "Kansas"
       fill_in "order_bill_address_attributes_zipcode", with: "SP1 M11"
 
-      # The country is already selected and we avoid re-selecting it here
-      # because it would trigger an API call which we would need to wait for
-      # while we have no visual indicator for the wait.
-      #
-      # Ideally, we would implement a visual cue like disable the state field
-      # while the data is fetched from the API.
-      #
-      # select "Australia", from: "order_bill_address_attributes_country_id"
+      select "Australia", from: "order_bill_address_attributes_country_id"
       select "Victoria", from: "order_bill_address_attributes_state_id"
       fill_in "order_bill_address_attributes_phone", with: "111 1111 1111"
 

From 983353e078e375d95132adad4e9ac5c7f07d16b7 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 01:09:55 +0100
Subject: [PATCH 15/16] Remove dead code

I'm pretty sure this was here because advancing the order was adding order errors in some cases
---
 app/controllers/spree/admin/orders_controller.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb
index 263e60ae9a..0122319adf 100644
--- a/app/controllers/spree/admin/orders_controller.rb
+++ b/app/controllers/spree/admin/orders_controller.rb
@@ -28,7 +28,6 @@ module Spree
 
       def edit
         @order.shipments.map(&:refresh_rates)
-        @order.errors.clear
       end
 
       def create

From b58b555e385e876da382dfb0b03549fd687fe369 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 7 Jun 2023 10:57:58 +0100
Subject: [PATCH 16/16] Improve flaky spec

---
 spec/system/admin/order_spec.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb
index 93d098b31c..1a788d201b 100644
--- a/spec/system/admin/order_spec.rb
+++ b/spec/system/admin/order_spec.rb
@@ -488,6 +488,7 @@ describe '
 
     # And I select that customer's email address and save the order
     tomselect_search_and_select customer.email, from: 'customer_search_override'
+    expect(page).to have_field "order_email", with: customer.email
     click_button 'Update'
 
     # Then their addresses should be associated with the order