From 73a81310f8e9e976fa84bdc6500a51a422cbb842 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 17 Apr 2023 17:18:22 +0200 Subject: [PATCH 1/9] Create a new url `/orders/ORDER_ID/distribution` That handles the step when distributor and order_cycle must be selected --- app/controllers/spree/admin/orders_controller.rb | 9 ++++++--- app/models/spree/ability.rb | 2 +- config/routes/spree.rb | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index 8f8408ab42..e1f22068f3 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -9,8 +9,8 @@ module Spree helper CheckoutHelper before_action :load_order, only: [:edit, :update, :fire, :resend, - :invoice, :print] - before_action :load_distribution_choices, only: [:new, :edit, :update] + :invoice, :print, :set_distribution] + before_action :load_distribution_choices, only: [:new, :edit, :update, :set_distribution] # Ensure that the distributor is set for an order when before_action :ensure_distribution, only: :new @@ -21,10 +21,13 @@ module Spree 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 set_distribution; end + def edit @order.shipments.map(&:refresh_rates) diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index dee02528e4..4ac5f8b399 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -274,7 +274,7 @@ module Spree # 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, :set_distribution], Spree::Order do user.admin? || user.enterprises.any?(&:is_distributor) end can [:admin, :create, :show, :poll], :invoice diff --git a/config/routes/spree.rb b/config/routes/spree.rb index 9844b800e5..c58d365606 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -89,6 +89,7 @@ Spree::Core::Engine.routes.draw do post :resend get :invoice get :print + get :distribution, to: 'orders#set_distribution' end collection do From 05a040b4c30ea0d33a5e8327a6b15bec4f677ed9 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 17 Apr 2023 17:19:09 +0200 Subject: [PATCH 2/9] No need to ensure if the distribution is set on `new` action This will be handled by validation on set_distribution action --- app/controllers/spree/admin/orders_controller.rb | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index e1f22068f3..e6a2355c8a 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -12,8 +12,6 @@ module Spree :invoice, :print, :set_distribution] before_action :load_distribution_choices, only: [:new, :edit, :update, :set_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 @@ -145,17 +143,6 @@ module Spree 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 From 6172d1f2e5b7bc5b18d1107fc05ba176e215d9a5 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 18 Apr 2023 11:21:46 +0200 Subject: [PATCH 3/9] Extract method `on_update` that will be used elsewhere --- app/controllers/spree/admin/orders_controller.rb | 16 ++++++++++------ config/routes/spree.rb | 1 + 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index e6a2355c8a..d77f61d668 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -34,12 +34,7 @@ module Spree 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) @@ -106,6 +101,15 @@ module Spree private + def on_update + @order.recreate_all_fees! + + unless @order.cart? + @order.create_tax_charge! + @order.update_order! + end + end + def order_params return params[:order] if params[:order].blank? diff --git a/config/routes/spree.rb b/config/routes/spree.rb index c58d365606..18ba8f3698 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -90,6 +90,7 @@ Spree::Core::Engine.routes.draw do get :invoice get :print get :distribution, to: 'orders#set_distribution' + put :distribution, to: 'orders#set_distribution' end collection do From 0f28d318aca57b3023d8927da92c0dd2989cbd4b Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 18 Apr 2023 11:56:03 +0200 Subject: [PATCH 4/9] Fix rubocop error and prefer using a return clause --- app/controllers/spree/admin/orders_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index d77f61d668..3d4b7c1c49 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -104,10 +104,10 @@ module Spree def on_update @order.recreate_all_fees! - unless @order.cart? - @order.create_tax_charge! - @order.update_order! - end + return if @order.cart? + + @order.create_tax_charge! + @order.update_order! end def order_params From ba1d985932dc1b806a9d29f459b943cd85abd2c4 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 18 Apr 2023 11:22:41 +0200 Subject: [PATCH 5/9] update the set_distribution form on the same path and use the same method `OrdersController#set_distribution` --- app/controllers/spree/admin/orders_controller.rb | 10 +++++++++- .../spree/admin/orders/set_distribution.html.haml | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index 3d4b7c1c49..1127f4eb45 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -24,7 +24,15 @@ module Spree redirect_to spree.distribution_admin_order_path(@order) end - def set_distribution; end + def set_distribution + return if order_params.blank? + + on_update + + @order.update(order_params) + @order.save + redirect_to spree.admin_order_customer_path(@order) + end def edit @order.shipments.map(&:refresh_rates) diff --git a/app/views/spree/admin/orders/set_distribution.html.haml b/app/views/spree/admin/orders/set_distribution.html.haml index 8cd9038788..ac577f0dd4 100644 --- a/app/views/spree/admin/orders/set_distribution.html.haml +++ b/app/views/spree/admin/orders/set_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" From ac666a6fce5a4a31f4c7fcdbcfabee9b6620102c Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 18 Apr 2023 11:23:29 +0200 Subject: [PATCH 6/9] Validate presence of distributor and order_cycle in set_distribution step --- .../spree/admin/orders_controller.rb | 2 ++ app/models/spree/order.rb | 2 ++ spec/system/admin/order_spec.rb | 22 +++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index 1127f4eb45..c62a5b017a 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -30,6 +30,8 @@ module Spree on_update @order.update(order_params) + return unless @order.valid?(:set_distribution_step) + @order.save redirect_to spree.admin_order_customer_path(@order) end diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index b3e4b8f935..40dfbcb093 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -97,6 +97,8 @@ 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 make_permalink field: :number diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb index d9e3dae850..3b932aec74 100644 --- a/spec/system/admin/order_spec.rb +++ b/spec/system/admin/order_spec.rb @@ -89,6 +89,28 @@ describe ' 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 + end + it "can add a product to an existing order" do login_as_admin visit spree.edit_admin_order_path(order) From 722c66069777d85df1bd51197b1ea6cb1b2a2134 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 18 Apr 2023 14:36:29 +0200 Subject: [PATCH 7/9] Do not show any links to other steps when no distribution set distribution means tuple: distributor + order cycle --- .../spree/admin/shared/_order_tabs.html.haml | 37 ++++++++++--------- spec/system/admin/order_spec.rb | 34 +++++++++++++++++ 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/app/views/spree/admin/shared/_order_tabs.html.haml b/app/views/spree/admin/shared/_order_tabs.html.haml index 8b46243b08..40cc8e323c 100644 --- a/app/views/spree/admin/shared/_order_tabs.html.haml +++ b/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) diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb index 3b932aec74..96211327b5 100644 --- a/spec/system/admin/order_spec.rb +++ b/spec/system/admin/order_spec.rb @@ -109,6 +109,40 @@ describe ' 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 From 7e306693a8004fbb441521205fa13ebe9b8dcf69 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 19 Apr 2023 10:06:44 +0200 Subject: [PATCH 8/9] Use Rails standard naming: same name for the route and the method --- app/controllers/spree/admin/orders_controller.rb | 6 +++--- app/models/spree/ability.rb | 2 +- .../{set_distribution.html.haml => distribution.html.haml} | 0 config/routes/spree.rb | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) rename app/views/spree/admin/orders/{set_distribution.html.haml => distribution.html.haml} (100%) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index c62a5b017a..bd4b9185f7 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -9,8 +9,8 @@ module Spree helper CheckoutHelper before_action :load_order, only: [:edit, :update, :fire, :resend, - :invoice, :print, :set_distribution] - before_action :load_distribution_choices, only: [:new, :edit, :update, :set_distribution] + :invoice, :print, :distribution] + before_action :load_distribution_choices, only: [:new, :edit, :update, :distribution] before_action :require_distributor_abn, only: :invoice @@ -24,7 +24,7 @@ module Spree redirect_to spree.distribution_admin_order_path(@order) end - def set_distribution + def distribution return if order_params.blank? on_update diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 4ac5f8b399..f0ae8fda0c 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -274,7 +274,7 @@ module Spree # 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, :set_distribution], 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 diff --git a/app/views/spree/admin/orders/set_distribution.html.haml b/app/views/spree/admin/orders/distribution.html.haml similarity index 100% rename from app/views/spree/admin/orders/set_distribution.html.haml rename to app/views/spree/admin/orders/distribution.html.haml diff --git a/config/routes/spree.rb b/config/routes/spree.rb index 18ba8f3698..6959454e22 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -89,8 +89,8 @@ Spree::Core::Engine.routes.draw do post :resend get :invoice get :print - get :distribution, to: 'orders#set_distribution' - put :distribution, to: 'orders#set_distribution' + get :distribution + put :distribution end collection do From 0f54d3950d3ffadbc18ec4227b5876c921fac2af Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 19 Apr 2023 10:15:29 +0200 Subject: [PATCH 9/9] Save can have a context: simplify then --- app/controllers/spree/admin/orders_controller.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index bd4b9185f7..51a21a8b60 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -29,10 +29,9 @@ module Spree on_update - @order.update(order_params) - return unless @order.valid?(:set_distribution_step) + @order.assign_attributes(order_params) + return unless @order.save(context: :set_distribution_step) - @order.save redirect_to spree.admin_order_customer_path(@order) end