Merge pull request #10712 from jibees/6133-backoffice-orders-endless-…

…spinner-and-error-500-after-skipping-oc-selector

Admin, create new order: distributor and order cycle are now mandatory on step 1 (aka the set_distribution step)

app/controllers/spree/admin/orders_controller.rb

@@ -9,20 +9,30 @@ class OrdersController < Spree::Admin::BaseController
       helper CheckoutHelper
 
       before_action :load_order, only: [:edit, :update, :fire, :resend,
-                                        :invoice, :print]
-      before_action :load_distribution_choices, only: [:new, :edit, :update]
+                                        :invoice, :print, :distribution]
+      before_action :load_distribution_choices, only: [:new, :edit, :update, :distribution]
 
-      # Ensure that the distributor is set for an order when
-      before_action :ensure_distribution, only: :new
       before_action :require_distributor_abn, only: :invoice
 
       respond_to :html, :json
 
       def new
         @order = Order.create
         @order.created_by = spree_current_user
+        @order.generate_order_number
         @order.save
-        redirect_to spree.edit_admin_order_url(@order)
+        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)
       end
 
       def edit
@@ -33,12 +43,7 @@ def edit
       end
 
       def update
-        @order.recreate_all_fees!
-
-        unless @order.cart?
-          @order.create_tax_charge!
-          @order.update_order!
-        end
+        on_update
 
         if params[:set_distribution_step] && @order.update(order_params)
           return redirect_to spree.admin_order_customer_path(@order)
@@ -105,6 +110,15 @@ def print
 
       private
 
+      def on_update
+        @order.recreate_all_fees!
+
+        return if @order.cart?
+
+        @order.create_tax_charge!
+        @order.update_order!
+      end
+
       def order_params
         return params[:order] if params[:order].blank?
 
@@ -142,17 +156,6 @@ def load_distribution_choices
                         ocs.closed +
                         ocs.undated
       end
-
-      def ensure_distribution
-        unless @order
-          @order = Spree::Order.new
-          @order.generate_order_number
-          @order.save!
-        end
-        return if @order.distribution_set?
-
-        render 'set_distribution', locals: { order: @order }
-      end
     end
   end
 end

app/models/spree/ability.rb

@@ -274,7 +274,7 @@ def add_order_management_abilities(user)
           # Enterprise User can access orders that are placed inside a OC they coordinate
           order.order_cycle&.coordinated_by?(user)
       end
-      can [:admin, :bulk_management, :managed], Spree::Order do
+      can [:admin, :bulk_management, :managed, :distribution], Spree::Order do
         user.admin? || user.enterprises.any?(&:is_distributor)
       end
       can [:admin, :create, :show, :poll], :invoice

app/models/spree/order.rb

@@ -97,6 +97,8 @@ def states
     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
 
     make_permalink field: :number
 

app/views/spree/admin/orders/set_distribution.html.haml → app/views/spree/admin/orders/distribution.html.haml

@@ -17,7 +17,7 @@
   = render 'spree/shared/error_messages', :target => @order
 
 %div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl"}
-  = form_for @order, url: admin_order_url(@order), method: :put do |f|
+  = 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"

app/views/spree/admin/shared/_order_tabs.html.haml

@@ -42,25 +42,26 @@
         %dd#date_complete
           = pretty_time(@order.completed_at)
 
-  %nav.menu
-    %ul
-      - customer_details_classes = "active" if current == "Customer Details"
-      %li{ class: customer_details_classes }
-        = link_to_with_icon 'icon-user', t(:customer_details), spree.admin_order_customer_url(@order)
+  - if @order.distribution_set?
+    %nav.menu
+      %ul
+        - customer_details_classes = "active" if current == "Customer Details"
+        %li{ class: customer_details_classes }
+          = link_to_with_icon 'icon-user', t(:customer_details), spree.admin_order_customer_url(@order)
 
-      - order_details_classes = "active" if current == "Order Details"
-      %li{ class: order_details_classes }
-        = link_to_with_icon 'icon-edit', t(:order_details), spree.edit_admin_order_url(@order)
+        - order_details_classes = "active" if current == "Order Details"
+        %li{ class: order_details_classes }
+          = link_to_with_icon 'icon-edit', t(:order_details), spree.edit_admin_order_url(@order)
 
-      - payments_classes = "active" if current == "Payments"
-      %li{ class: payments_classes }
-        = link_to_with_icon 'icon-credit-card', t(:payments), spree.admin_order_payments_url(@order)
+        - payments_classes = "active" if current == "Payments"
+        %li{ class: payments_classes }
+          = link_to_with_icon 'icon-credit-card', t(:payments), spree.admin_order_payments_url(@order)
 
-      - adjustments_classes = "active" if current == "Adjustments"
-      %li{ class: adjustments_classes }
-        = link_to_with_icon 'icon-cogs', t(:adjustments), spree.admin_order_adjustments_url(@order)
+        - adjustments_classes = "active" if current == "Adjustments"
+        %li{ class: adjustments_classes }
+          = link_to_with_icon 'icon-cogs', t(:adjustments), spree.admin_order_adjustments_url(@order)
 
-      - if @order.completed?
-        - authorizations_classes = "active" if current == "Return Authorizations"
-        %li{ class: authorizations_classes }
-          = link_to_with_icon 'icon-share-alt', t(:return_authorizations), spree.admin_order_return_authorizations_url(@order)
+        - if @order.completed?
+          - authorizations_classes = "active" if current == "Return Authorizations"
+          %li{ class: authorizations_classes }
+            = link_to_with_icon 'icon-share-alt', t(:return_authorizations), spree.admin_order_return_authorizations_url(@order)

config/routes/spree.rb

@@ -89,6 +89,8 @@
         post :resend
         get :invoice
         get :print
+        get :distribution
+        put :distribution
       end
 
       collection do

spec/system/admin/order_spec.rb

@@ -89,6 +89,62 @@ def new_order_with_distribution(distributor, order_cycle)
     click_button 'Update'
   end
 
+  context "can't create an order without selecting a distributor nor an order cycle" do
+    before do
+      login_as_admin
+      visit spree.admin_orders_path
+      click_link 'New Order'
+    end
+
+    it 'shows error when distributor is not selected' do
+      click_button 'Next'
+
+      expect(page).to have_content "Order cycle can't be blank"
+      expect(page).to have_content "Distributor can't be blank"
+    end
+
+    it 'shows error when order cycle is not selected' do
+      select2_select distributor.name, from: 'order_distributor_id'
+      click_button 'Next'
+
+      expect(page).to have_content "Order cycle can't be blank"
+    end
+
+    it "doesn't show links to other steps" do
+      expect(page).not_to have_content "CUSTOMER DETAILS"
+      expect(page).not_to have_content "ORDER DETAILS"
+      expect(page).not_to have_content "PAYMENTS"
+      expect(page).not_to have_content "ADJUSTMENTS"
+    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
+
   it "can add a product to an existing order" do
     login_as_admin
     visit spree.edit_admin_order_path(order)