diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb index 0905abe954..b1be516316 100644 --- a/app/controllers/api/v0/shipments_controller.rb +++ b/app/controllers/api/v0/shipments_controller.rb @@ -21,7 +21,9 @@ module Api @shipment.refresh_rates @shipment.save! - render json: @shipment.reload, serializer: Api::ShipmentSerializer, status: :ok + OrderWorkflow.new(@order).advance_to_payment if @order.line_items.any? + + render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok end def update diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index 80c3707ca3..0122319adf 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,47 +23,35 @@ 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 @order.shipments.map(&:refresh_rates) - @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 - 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 - 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 @@ -117,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/models/spree/order.rb b/app/models/spree/order.rb index 74a4f332d9..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 @@ -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 @@ -290,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/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 diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml index 9f8b00c6e2..4d43acf8dd 100644 --- a/app/views/spree/admin/orders/_form.html.haml +++ b/app/views/spree/admin/orders/_form.html.haml @@ -5,24 +5,26 @@ - 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? + - 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 } = 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' + = 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}'; 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" 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/edit.html.haml b/app/views/spree/admin/orders/edit.html.haml index b8556d861a..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 @@ -32,4 +29,4 @@ = Spree.t(:your_order_is_empty_add_product) %div.admin-order-edit-form - = render :partial => 'form', :locals => { :order => @order } + = render partial: 'form' 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 %>; 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/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 diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb index 366c2f2a9a..1a788d201b 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 @@ -116,61 +112,50 @@ 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!(: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 @@ -328,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 @@ -425,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" @@ -511,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