From 51bca7ce2fbf53158137c8f8592a165215280e68 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 27 Nov 2019 21:47:10 +0000 Subject: [PATCH 01/12] Add customer_details_controller from spree_backend so that we can now merge it with the OFN's decorator --- .../orders/customer_details_controller.rb | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 app/controllers/spree/admin/orders/customer_details_controller.rb diff --git a/app/controllers/spree/admin/orders/customer_details_controller.rb b/app/controllers/spree/admin/orders/customer_details_controller.rb new file mode 100644 index 0000000000..151e3e4d5a --- /dev/null +++ b/app/controllers/spree/admin/orders/customer_details_controller.rb @@ -0,0 +1,43 @@ +module Spree + module Admin + module Orders + class CustomerDetailsController < Spree::Admin::BaseController + before_filter :load_order + + def show + edit + render :action => :edit + end + + def edit + country_id = Address.default.country.id + @order.build_bill_address(:country_id => country_id) if @order.bill_address.nil? + @order.build_ship_address(:country_id => country_id) if @order.ship_address.nil? + end + + def update + if @order.update_attributes(params[:order]) + if params[:guest_checkout] == "false" + @order.associate_user!(Spree.user_class.find_by_email(@order.email)) + end + while @order.next; end + + @order.shipments.map &:refresh_rates + flash[:success] = Spree.t('customer_details_updated') + redirect_to admin_order_customer_path(@order) + else + render :action => :edit + end + + end + + private + + def load_order + @order = Order.find_by_number!(params[:order_id], :include => :adjustments) + end + + end + end + end +end From a1d4b4ee984b8c4e7010da334bc4dfdace6ac267 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 27 Nov 2019 21:47:22 +0000 Subject: [PATCH 02/12] Add orders_controller from spree_backend so that we can now merge it with the OFN's decorator --- .../spree/admin/orders_controller.rb | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 app/controllers/spree/admin/orders_controller.rb diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb new file mode 100644 index 0000000000..eecae8592a --- /dev/null +++ b/app/controllers/spree/admin/orders_controller.rb @@ -0,0 +1,141 @@ +module Spree + module Admin + class OrdersController < Spree::Admin::BaseController + require 'spree/core/gateway_error' + before_filter :initialize_order_events + before_filter :load_order, :only => [:edit, :update, :fire, :resend, :open_adjustments, :close_adjustments] + + respond_to :html + + def index + params[:q] ||= {} + params[:q][:completed_at_not_null] ||= '1' if Spree::Config[:show_only_complete_orders_by_default] + @show_only_completed = params[:q][:completed_at_not_null].present? + params[:q][:s] ||= @show_only_completed ? 'completed_at desc' : 'created_at desc' + + # As date params are deleted if @show_only_completed, store + # the original date so we can restore them into the params + # after the search + created_at_gt = params[:q][:created_at_gt] + created_at_lt = params[:q][:created_at_lt] + + params[:q].delete(:inventory_units_shipment_id_null) if params[:q][:inventory_units_shipment_id_null] == "0" + + if !params[:q][:created_at_gt].blank? + params[:q][:created_at_gt] = Time.zone.parse(params[:q][:created_at_gt]).beginning_of_day rescue "" + end + + if !params[:q][:created_at_lt].blank? + params[:q][:created_at_lt] = Time.zone.parse(params[:q][:created_at_lt]).end_of_day rescue "" + end + + if @show_only_completed + params[:q][:completed_at_gt] = params[:q].delete(:created_at_gt) + params[:q][:completed_at_lt] = params[:q].delete(:created_at_lt) + end + + @search = Order.accessible_by(current_ability, :index).ransack(params[:q]) + @orders = @search.result.includes([:user, :shipments, :payments]). + page(params[:page]). + per(params[:per_page] || Spree::Config[:orders_per_page]) + + # Restore dates + params[:q][:created_at_gt] = created_at_gt + params[:q][:created_at_lt] = created_at_lt + end + + def new + @order = Order.create + @order.created_by = try_spree_current_user + @order.save + redirect_to edit_admin_order_url(@order) + end + + def edit + @order.shipments.map &:refresh_rates + # Transition as far as we can go + while @order.next; end + # The payment step shows an error of 'No pending payments' + # Clearing the errors from the order object will stop this error + # appearing on the edit page where we don't want it to. + @order.errors.clear + end + + def update + return_path = nil + if @order.update_attributes(params[:order]) && @order.line_items.present? + @order.update! + unless @order.complete? + # Jump to next step if order is not complete. + return_path = admin_order_customer_path(@order) + else + # Otherwise, go back to first page since all necessary information has been filled out. + return_path = admin_order_path(@order) + end + else + @order.errors.add(:line_items, Spree.t('errors.messages.blank')) if @order.line_items.empty? + end + + if return_path + redirect_to return_path + else + render :action => :edit + end + end + + def fire + # TODO - possible security check here but right now any admin can before any transition (and the state machine + # itself will make sure transitions are not applied in the wrong state) + event = params[:e] + if @order.send("#{event}") + flash[:success] = Spree.t(:order_updated) + else + flash[:error] = Spree.t(:cannot_perform_operation) + end + rescue Spree::Core::GatewayError => ge + flash[:error] = "#{ge.message}" + ensure + redirect_to :back + end + + def resend + OrderMailer.confirm_email(@order.id, true).deliver + flash[:success] = Spree.t(:order_email_resent) + + redirect_to :back + end + + def open_adjustments + adjustments = @order.adjustments.where(:state => 'closed') + adjustments.update_all(:state => 'open') + flash[:success] = Spree.t(:all_adjustments_opened) + + respond_with(@order) { |format| format.html { redirect_to :back } } + end + + def close_adjustments + adjustments = @order.adjustments.where(:state => 'open') + adjustments.update_all(:state => 'closed') + flash[:success] = Spree.t(:all_adjustments_closed) + + respond_with(@order) { |format| format.html { redirect_to :back } } + end + + private + + def load_order + @order = Order.find_by_number!(params[:id], :include => :adjustments) if params[:id] + authorize! action, @order + end + + # Used for extensions which need to provide their own custom event links on the order details view. + def initialize_order_events + @order_events = %w{cancel resume} + end + + def model_class + Spree::Order + end + end + end +end From 76b6a85509a6ac2978c0159ecb3d0e124d9447ea Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 11 Nov 2019 16:29:37 +0000 Subject: [PATCH 03/12] Merge spree/admin/orders_controller and its decorator and fix a few rubocop issues --- .../spree/admin/orders_controller.rb | 193 +++++++++++------- .../admin/orders_controller_decorator.rb | 120 ----------- 2 files changed, 117 insertions(+), 196 deletions(-) delete mode 100644 app/controllers/spree/admin/orders_controller_decorator.rb diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index eecae8592a..ec2a547e2b 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -1,47 +1,37 @@ +require 'open_food_network/spree_api_key_loader' + module Spree module Admin class OrdersController < Spree::Admin::BaseController require 'spree/core/gateway_error' - before_filter :initialize_order_events - before_filter :load_order, :only => [:edit, :update, :fire, :resend, :open_adjustments, :close_adjustments] + include OpenFoodNetwork::SpreeApiKeyLoader + helper CheckoutHelper - respond_to :html + before_filter :initialize_order_events + before_filter :load_order, only: [:edit, :update, :fire, :resend, + :open_adjustments, :close_adjustments] + + before_filter :load_order, only: %i[show edit update fire resend invoice print print_ticket] + + before_filter :load_distribution_choices, only: [:new, :edit, :update] + + # Ensure that the distributor is set for an order when + before_filter :ensure_distribution, only: :new + + # After updating an order, the fees should be updated as well + # Currently, adding or deleting line items does not trigger updating the + # fees! This is a quick fix for that. + # TODO: update fees when adding/removing line items + # instead of the update_distribution_charge method. + after_filter :update_distribution_charge, only: :update + + before_filter :require_distributor_abn, only: :invoice + + respond_to :html, :json def index - params[:q] ||= {} - params[:q][:completed_at_not_null] ||= '1' if Spree::Config[:show_only_complete_orders_by_default] - @show_only_completed = params[:q][:completed_at_not_null].present? - params[:q][:s] ||= @show_only_completed ? 'completed_at desc' : 'created_at desc' - - # As date params are deleted if @show_only_completed, store - # the original date so we can restore them into the params - # after the search - created_at_gt = params[:q][:created_at_gt] - created_at_lt = params[:q][:created_at_lt] - - params[:q].delete(:inventory_units_shipment_id_null) if params[:q][:inventory_units_shipment_id_null] == "0" - - if !params[:q][:created_at_gt].blank? - params[:q][:created_at_gt] = Time.zone.parse(params[:q][:created_at_gt]).beginning_of_day rescue "" - end - - if !params[:q][:created_at_lt].blank? - params[:q][:created_at_lt] = Time.zone.parse(params[:q][:created_at_lt]).end_of_day rescue "" - end - - if @show_only_completed - params[:q][:completed_at_gt] = params[:q].delete(:created_at_gt) - params[:q][:completed_at_lt] = params[:q].delete(:created_at_lt) - end - - @search = Order.accessible_by(current_ability, :index).ransack(params[:q]) - @orders = @search.result.includes([:user, :shipments, :payments]). - page(params[:page]). - per(params[:per_page] || Spree::Config[:orders_per_page]) - - # Restore dates - params[:q][:created_at_gt] = created_at_gt - params[:q][:created_at_lt] = created_at_lt + # Overriding the action so we only render the page template. An angular request + # within the page then fetches the data it needs from Api::OrdersController end def new @@ -52,9 +42,10 @@ module Spree end def edit - @order.shipments.map &:refresh_rates - # Transition as far as we can go - while @order.next; end + @order.shipments.map(&:refresh_rates) + + AdvanceOrderService.new(@order).call + # The payment step shows an error of 'No pending payments' # Clearing the errors from the order object will stop this error # appearing on the edit page where we don't want it to. @@ -62,60 +53,82 @@ module Spree end def update - return_path = nil - if @order.update_attributes(params[:order]) && @order.line_items.present? - @order.update! - unless @order.complete? - # Jump to next step if order is not complete. - return_path = admin_order_customer_path(@order) - else - # Otherwise, go back to first page since all necessary information has been filled out. - return_path = admin_order_path(@order) + unless @order.update_attributes(params[:order]) && @order.line_items.present? + if @order.line_items.empty? + @order.errors.add(:line_items, Spree.t('errors.messages.blank')) end - else - @order.errors.add(:line_items, Spree.t('errors.messages.blank')) if @order.line_items.empty? + return redirect_to(edit_admin_order_path(@order), + flash: { error: @order.errors.full_messages.join(', ') }) end - if return_path - redirect_to return_path + @order.update! + if @order.complete? + redirect_to edit_admin_order_path(@order) else - render :action => :edit + # Jump to next step if order is not complete + redirect_to admin_order_customer_path(@order) end end + def bulk_management + load_spree_api_key + end + def fire - # TODO - possible security check here but right now any admin can before any transition (and the state machine - # itself will make sure transitions are not applied in the wrong state) + # TODO - Possible security check here + # Right now any admin can before any transition (and the state machine + # itself will make sure transitions are not applied in the wrong state) event = params[:e] - if @order.send("#{event}") + if @order.public_send(event.to_s) flash[:success] = Spree.t(:order_updated) else flash[:error] = Spree.t(:cannot_perform_operation) end - rescue Spree::Core::GatewayError => ge - flash[:error] = "#{ge.message}" + rescue Spree::Core::GatewayError => e + flash[:error] = e.message.to_s ensure redirect_to :back end def resend - OrderMailer.confirm_email(@order.id, true).deliver - flash[:success] = Spree.t(:order_email_resent) + Spree::OrderMailer.confirm_email_for_customer(@order.id, true).deliver + flash[:success] = t(:order_email_resent) - redirect_to :back + respond_with(@order) { |format| format.html { redirect_to :back } } + end + + def invoice + pdf = InvoiceRenderer.new.render_to_string(@order) + + Spree::OrderMailer.invoice_email(@order.id, pdf).deliver + flash[:success] = t('admin.orders.invoice_email_sent') + + respond_with(@order) { |format| format.html { redirect_to edit_admin_order_path(@order) } } + end + + def print + render InvoiceRenderer.new.args(@order) + end + + def print_ticket + render template: "spree/admin/orders/ticket", layout: false + end + + def update_distribution_charge + @order.update_distribution_charge! end def open_adjustments - adjustments = @order.adjustments.where(:state => 'closed') - adjustments.update_all(:state => 'open') + adjustments = @order.adjustments.where(state: 'closed') + adjustments.update_all(state: 'open') flash[:success] = Spree.t(:all_adjustments_opened) respond_with(@order) { |format| format.html { redirect_to :back } } end def close_adjustments - adjustments = @order.adjustments.where(:state => 'open') - adjustments.update_all(:state => 'closed') + adjustments = @order.adjustments.where(state: 'open') + adjustments.update_all(state: 'closed') flash[:success] = Spree.t(:all_adjustments_closed) respond_with(@order) { |format| format.html { redirect_to :back } } @@ -123,19 +136,47 @@ module Spree private - def load_order - @order = Order.find_by_number!(params[:id], :include => :adjustments) if params[:id] - authorize! action, @order - end + def load_order + @order = Order.find_by_number!(params[:id], include: :adjustments) if params[:id] + authorize! action, @order + end - # Used for extensions which need to provide their own custom event links on the order details view. - def initialize_order_events - @order_events = %w{cancel resume} - end + def initialize_order_events + @order_events = %w{cancel resume} + end - def model_class - Spree::Order + def model_class + Spree::Order + end + + def require_distributor_abn + return if @order.distributor.abn.present? + + flash[:error] = t(:must_have_valid_business_number, + enterprise_name: @order.distributor.name) + respond_with(@order) { |format| format.html { redirect_to edit_admin_order_path(@order) } } + end + + def load_distribution_choices + @shops = Enterprise.is_distributor.managed_by(spree_current_user).by_name + + ocs = OrderCycle.managed_by(spree_current_user) + @order_cycles = ocs.soonest_closing + + ocs.soonest_opening + + 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 diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb deleted file mode 100644 index 8483956024..0000000000 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ /dev/null @@ -1,120 +0,0 @@ -require 'open_food_network/spree_api_key_loader' - -Spree::Admin::OrdersController.class_eval do - include OpenFoodNetwork::SpreeApiKeyLoader - helper CheckoutHelper - before_filter :load_order, only: %i[show edit update fire resend invoice print print_ticket] - - before_filter :load_distribution_choices, only: [:new, :edit, :update] - - # Ensure that the distributor is set for an order when - before_filter :ensure_distribution, only: :new - - # After updating an order, the fees should be updated as well - # Currently, adding or deleting line items does not trigger updating the - # fees! This is a quick fix for that. - # TODO: update fees when adding/removing line items - # instead of the update_distribution_charge method. - after_filter :update_distribution_charge, only: :update - - before_filter :require_distributor_abn, only: :invoice - - respond_to :html, :json - - def index - # Overriding the action so we only render the page template. An angular request - # within the page then fetches the data it needs from Api::OrdersController - end - - def bulk_management - load_spree_api_key - end - - def edit - @order.shipments.map &:refresh_rates - - AdvanceOrderService.new(@order).call - - # The payment step shows an error of 'No pending payments' - # Clearing the errors from the order object will stop this error - # appearing on the edit page where we don't want it to. - @order.errors.clear - end - - # Re-implement spree method so that it redirects to edit instead of rendering edit - # This allows page reloads while adding variants to the order (/edit), without being redirected to customer details page (/update) - def update - unless @order.update_attributes(params[:order]) && @order.line_items.present? - @order.errors.add(:line_items, Spree.t('errors.messages.blank')) if @order.line_items.empty? - return redirect_to edit_admin_order_path(@order), flash: { error: @order.errors.full_messages.join(', ') } - end - - @order.update! - if @order.complete? - redirect_to edit_admin_order_path(@order) - else - # Jump to next step if order is not complete - redirect_to admin_order_customer_path(@order) - end - end - - # Overwrite to use confirm_email_for_customer instead of confirm_email. - # This uses a new template. See mailers/spree/order_mailer_decorator.rb. - def resend - Spree::OrderMailer.confirm_email_for_customer(@order.id, true).deliver - flash[:success] = t(:order_email_resent) - - respond_with(@order) { |format| format.html { redirect_to :back } } - end - - def invoice - pdf = InvoiceRenderer.new.render_to_string(@order) - - Spree::OrderMailer.invoice_email(@order.id, pdf).deliver - flash[:success] = t('admin.orders.invoice_email_sent') - - respond_with(@order) { |format| format.html { redirect_to edit_admin_order_path(@order) } } - end - - def print - render InvoiceRenderer.new.args(@order) - end - - def print_ticket - render template: "spree/admin/orders/ticket", layout: false - end - - def update_distribution_charge - @order.update_distribution_charge! - end - - private - - def require_distributor_abn - if @order.distributor.abn.blank? - flash[:error] = t(:must_have_valid_business_number, enterprise_name: @order.distributor.name) - respond_with(@order) { |format| format.html { redirect_to edit_admin_order_path(@order) } } - end - end - - def load_distribution_choices - @shops = Enterprise.is_distributor.managed_by(spree_current_user).by_name - - ocs = OrderCycle.managed_by(spree_current_user) - @order_cycles = ocs.soonest_closing + - ocs.soonest_opening + - ocs.closed + - ocs.undated - end - - def ensure_distribution - unless @order - @order = Spree::Order.new - @order.generate_order_number - @order.save! - end - unless @order.distribution_set? - render 'set_distribution', locals: { order: @order } - end - end -end From b93af37ea95ddaefae40fbf227f6e931c021c5f1 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 11 Nov 2019 21:47:13 +0000 Subject: [PATCH 04/12] Merge customer details decorator with controller --- .../orders/customer_details_controller.rb | 37 ++++++++++++-- .../customer_details_controller_decorator.rb | 49 ------------------- 2 files changed, 32 insertions(+), 54 deletions(-) delete mode 100644 app/controllers/spree/admin/orders/customer_details_controller_decorator.rb diff --git a/app/controllers/spree/admin/orders/customer_details_controller.rb b/app/controllers/spree/admin/orders/customer_details_controller.rb index 151e3e4d5a..081d76a754 100644 --- a/app/controllers/spree/admin/orders/customer_details_controller.rb +++ b/app/controllers/spree/admin/orders/customer_details_controller.rb @@ -3,6 +3,8 @@ module Spree module Orders class CustomerDetailsController < Spree::Admin::BaseController before_filter :load_order + before_filter :check_authorization + before_filter :set_guest_checkout_status, only: :update def show edit @@ -20,23 +22,48 @@ module Spree if params[:guest_checkout] == "false" @order.associate_user!(Spree.user_class.find_by_email(@order.email)) end - while @order.next; end + + AdvanceOrderService.new(@order).call @order.shipments.map &:refresh_rates flash[:success] = Spree.t('customer_details_updated') redirect_to admin_order_customer_path(@order) else - render :action => :edit + render action: :edit end + end + # Inherit CanCan permissions for the current order + def model_class + load_order unless @order + @order end private - def load_order - @order = Order.find_by_number!(params[:order_id], :include => :adjustments) - end + def load_order + @order = Order.find_by_number!(params[:order_id], :include => :adjustments) + end + def check_authorization + load_order + session[:access_token] ||= params[:token] + + resource = @order + action = params[:action].to_sym + action = :edit if action == :show # show route renders :edit for this controller + + authorize! action, resource, session[:access_token] + end + + def set_guest_checkout_status + registered_user = Spree::User.find_by_email(params[:order][:email]) + + params[:order][:guest_checkout] = registered_user.nil? + + return unless registered_user + @order.user_id = registered_user.id + end end end end diff --git a/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb b/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb deleted file mode 100644 index 996e9a72fb..0000000000 --- a/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb +++ /dev/null @@ -1,49 +0,0 @@ -Spree::Admin::Orders::CustomerDetailsController.class_eval do - before_filter :check_authorization - before_filter :set_guest_checkout_status, only: :update - - def update - if @order.update_attributes(params[:order]) - if params[:guest_checkout] == "false" - @order.associate_user!(Spree.user_class.find_by_email(@order.email)) - end - - AdvanceOrderService.new(@order).call - - @order.shipments.map &:refresh_rates - flash[:success] = Spree.t('customer_details_updated') - redirect_to admin_order_customer_path(@order) - else - render action: :edit - end - end - - # Inherit CanCan permissions for the current order - def model_class - load_order unless @order - @order - end - - private - - def check_authorization - load_order - session[:access_token] ||= params[:token] - - resource = @order - action = params[:action].to_sym - action = :edit if action == :show # show route renders :edit for this controller - - authorize! action, resource, session[:access_token] - end - - def set_guest_checkout_status - registered_user = Spree::User.find_by_email(params[:order][:email]) - - params[:order][:guest_checkout] = registered_user.nil? - - return unless registered_user - - @order.user_id = registered_user.id - end -end From 7009cd89e0b76ae6b317cb5a73002da195a2e641 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 11 Nov 2019 21:47:59 +0000 Subject: [PATCH 05/12] Fix rubocop issues in customer details controller and update rubocop manual todo accordingly --- .rubocop_manual_todo.yml | 8 +++++--- .../spree/admin/orders/customer_details_controller.rb | 11 ++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 011f30f66c..6d2a56baaa 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -360,8 +360,8 @@ Metrics/AbcSize: - app/controllers/enterprises_controller.rb - app/controllers/spree/admin/adjustments_controller_decorator.rb - app/controllers/spree/admin/image_settings_controller.rb - - app/controllers/spree/admin/orders/customer_details_controller_decorator.rb - - app/controllers/spree/admin/orders_controller_decorator.rb + - app/controllers/spree/admin/orders/customer_details_controller.rb + - app/controllers/spree/admin/orders_controller.rb - app/controllers/spree/admin/overview_controller.rb - app/controllers/spree/admin/payment_methods_controller.rb - app/controllers/spree/admin/payments_controller.rb @@ -565,7 +565,8 @@ Metrics/MethodLength: - app/controllers/checkout_controller.rb - app/controllers/shop_controller.rb - app/controllers/spree/admin/image_settings_controller.rb - - app/controllers/spree/admin/orders/customer_details_controller_decorator.rb + - app/controllers/spree/admin/orders/customer_details_controller.rb + - app/controllers/spree/admin/orders_controller.rb - app/controllers/spree/admin/payment_methods_controller.rb - app/controllers/spree/admin/payments_controller.rb - app/controllers/spree/admin/reports_controller.rb @@ -652,6 +653,7 @@ Metrics/ClassLength: - app/controllers/api/products_controller.rb - app/controllers/checkout_controller.rb - app/controllers/spree/admin/base_controller.rb + - app/controllers/spree/admin/orders_controller.rb - app/controllers/spree/admin/payment_methods_controller.rb - app/controllers/spree/admin/reports_controller.rb - app/controllers/spree/admin/resource_controller.rb diff --git a/app/controllers/spree/admin/orders/customer_details_controller.rb b/app/controllers/spree/admin/orders/customer_details_controller.rb index 081d76a754..2a22c1e086 100644 --- a/app/controllers/spree/admin/orders/customer_details_controller.rb +++ b/app/controllers/spree/admin/orders/customer_details_controller.rb @@ -8,13 +8,13 @@ module Spree def show edit - render :action => :edit + render action: :edit end def edit country_id = Address.default.country.id - @order.build_bill_address(:country_id => country_id) if @order.bill_address.nil? - @order.build_ship_address(:country_id => country_id) if @order.ship_address.nil? + @order.build_bill_address(country_id: country_id) if @order.bill_address.nil? + @order.build_ship_address(country_id: country_id) if @order.ship_address.nil? end def update @@ -25,7 +25,7 @@ module Spree AdvanceOrderService.new(@order).call - @order.shipments.map &:refresh_rates + @order.shipments.map(&:refresh_rates) flash[:success] = Spree.t('customer_details_updated') redirect_to admin_order_customer_path(@order) else @@ -42,7 +42,7 @@ module Spree private def load_order - @order = Order.find_by_number!(params[:order_id], :include => :adjustments) + @order = Order.find_by_number!(params[:order_id], include: :adjustments) end def check_authorization @@ -62,6 +62,7 @@ module Spree params[:order][:guest_checkout] = registered_user.nil? return unless registered_user + @order.user_id = registered_user.id end end From ab60c4a9dd62bf6bcbba476b6b8c4341cdb31436 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 14 Dec 2019 15:20:26 +0000 Subject: [PATCH 06/12] Remove unnecessary index action override because this is no longer a decorator Remove useless todo comment --- app/controllers/spree/admin/orders_controller.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index ec2a547e2b..80a3b7f6f6 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -29,11 +29,6 @@ module Spree respond_to :html, :json - def index - # Overriding the action so we only render the page template. An angular request - # within the page then fetches the data it needs from Api::OrdersController - end - def new @order = Order.create @order.created_by = try_spree_current_user @@ -75,9 +70,6 @@ module Spree end def fire - # TODO - Possible security check here - # Right now any admin can before any transition (and the state machine - # itself will make sure transitions are not applied in the wrong state) event = params[:e] if @order.public_send(event.to_s) flash[:success] = Spree.t(:order_updated) From 6e2684181728483ac8a03a87d6a09f0f791ac30c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 14 Dec 2019 15:25:41 +0000 Subject: [PATCH 07/12] Fix load_order before filter and remove unused open and close adjustments actions --- .../spree/admin/orders_controller.rb | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index 80a3b7f6f6..a6d8e250b7 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -9,10 +9,7 @@ module Spree before_filter :initialize_order_events before_filter :load_order, only: [:edit, :update, :fire, :resend, - :open_adjustments, :close_adjustments] - - before_filter :load_order, only: %i[show edit update fire resend invoice print print_ticket] - + :invoice, :print, :print_ticket] before_filter :load_distribution_choices, only: [:new, :edit, :update] # Ensure that the distributor is set for an order when @@ -110,22 +107,6 @@ module Spree @order.update_distribution_charge! end - def open_adjustments - adjustments = @order.adjustments.where(state: 'closed') - adjustments.update_all(state: 'open') - flash[:success] = Spree.t(:all_adjustments_opened) - - respond_with(@order) { |format| format.html { redirect_to :back } } - end - - def close_adjustments - adjustments = @order.adjustments.where(state: 'open') - adjustments.update_all(state: 'closed') - flash[:success] = Spree.t(:all_adjustments_closed) - - respond_with(@order) { |format| format.html { redirect_to :back } } - end - private def load_order From e5f089610c7545c1cd971cd94dc7c2d023fc7dc7 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 14 Dec 2019 15:27:05 +0000 Subject: [PATCH 08/12] Bring admin orders routes from spree_backend --- config/routes/spree.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/config/routes/spree.rb b/config/routes/spree.rb index e7fcf0d2cb..27f530b5cd 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -85,12 +85,18 @@ Spree::Core::Engine.routes.draw do get '/variants/search', :to => "variants#search", :as => :search_variants resources :orders do - get :invoice, on: :member - get :print, on: :member - get :print_ticket, on: :member - get :managed, on: :collection + member do + put :fire + get :fire + post :resend + get :invoice + get :print + get :print_ticket + end collection do + get :managed + resources :invoices, only: [:create, :show] do get :poll end @@ -103,6 +109,8 @@ Spree::Core::Engine.routes.draw do put :fire end end + + resource :customer, :controller => "orders/customer_details" end resources :users do From 914244a1eee0b9147cc9cf2005b1e377f123202b Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 14 Dec 2019 21:15:05 +0000 Subject: [PATCH 09/12] Bring orders helper from spree_backend --- app/helpers/spree/admin/orders_helper.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 app/helpers/spree/admin/orders_helper.rb diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb new file mode 100644 index 0000000000..b258a4c69e --- /dev/null +++ b/app/helpers/spree/admin/orders_helper.rb @@ -0,0 +1,23 @@ +module Spree + module Admin + module OrdersHelper + # Renders all the extension partials that may have been specified in the extensions + def event_links + links = [] + @order_events.sort.each do |event| + if @order.send("can_#{event}?") + links << button_link_to(Spree.t(event), fire_admin_order_url(@order, :e => event), + :method => :put, + :icon => "icon-#{event}", + :data => { :confirm => Spree.t(:order_sure_want_to, :event => Spree.t(event)) }) + end + end + links.join(' ').html_safe + end + + def line_item_shipment_price(line_item, quantity) + Spree::Money.new(line_item.price * quantity, { currency: line_item.currency }) + end + end + end +end From 08dd992344ad12f049b95ade3cd5b6e50d819270 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 14 Dec 2019 21:15:57 +0000 Subject: [PATCH 10/12] Merge orders_helper with decorator --- app/helpers/spree/admin/orders_helper.rb | 100 +++++++++++++++++ .../spree/admin/orders_helper_decorator.rb | 105 ------------------ 2 files changed, 100 insertions(+), 105 deletions(-) delete mode 100644 app/helpers/spree/admin/orders_helper_decorator.rb diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb index b258a4c69e..324e73fd51 100644 --- a/app/helpers/spree/admin/orders_helper.rb +++ b/app/helpers/spree/admin/orders_helper.rb @@ -18,6 +18,106 @@ module Spree def line_item_shipment_price(line_item, quantity) Spree::Money.new(line_item.price * quantity, { currency: line_item.currency }) end + + def order_links(order) + @order ||= order + links = [] + links << edit_order_link unless action_name == "edit" + links.concat(complete_order_links) if @order.complete? + links << ship_order_link if @order.ready_to_ship? + links << cancel_order_link if @order.can_cancel? + links + end + + private + + def complete_order_links + [resend_confirmation_link] + invoice_links + ticket_links + end + + def invoice_links + return [] unless Spree::Config[:enable_invoices?] + + [send_invoice_link, print_invoice_link] + end + + def send_invoice_link + if @order.distributor.can_invoice? + send_invoice_link_with_url + else + send_invoice_link_without_url + end + end + + def ticket_links + return [] unless Spree::Config[:enable_receipt_printing?] + + [print_ticket_link, select_ticket_printer_link] + end + + def edit_order_link + { name: t(:edit_order), + url: edit_admin_order_path(@order), + icon: 'icon-edit' } + end + + def resend_confirmation_link + { name: t(:resend_confirmation), + url: resend_admin_order_path(@order), + icon: 'icon-email', + method: 'post', + confirm: t(:confirm_resend_order_confirmation) } + end + + def send_invoice_link_with_url + { name: t(:send_invoice), + url: invoice_admin_order_path(@order), + icon: 'icon-email', + confirm: t(:confirm_send_invoice) } + end + + def send_invoice_link_without_url + { name: t(:send_invoice), + url: "#", + icon: 'icon-email', + confirm: t(:must_have_valid_business_number, enterprise_name: @order.distributor.name) } + end + + def print_invoice_link + { name: t(:print_invoice), + url: print_admin_order_path(@order), + icon: 'icon-print', + target: "_blank" } + end + + def print_ticket_link + { name: t(:print_ticket), + url: print_ticket_admin_order_path(@order), + icon: 'icon-print', + target: "_blank" } + end + + def select_ticket_printer_link + { name: t(:select_ticket_printer), + url: "#{print_ticket_admin_order_path(@order)}#select-printer", + icon: 'icon-print', + target: "_blank" } + end + + def ship_order_link + { name: t(:ship_order), + url: fire_admin_order_path(@order, e: 'ship'), + method: 'put', + icon: 'icon-truck', + confirm: t(:are_you_sure) } + end + + def cancel_order_link + { name: t(:cancel_order), + url: fire_admin_order_path(@order.number, e: 'cancel'), + icon: 'icon-trash', + confirm: t(:are_you_sure) } + end end end end diff --git a/app/helpers/spree/admin/orders_helper_decorator.rb b/app/helpers/spree/admin/orders_helper_decorator.rb deleted file mode 100644 index 0d62c2caa0..0000000000 --- a/app/helpers/spree/admin/orders_helper_decorator.rb +++ /dev/null @@ -1,105 +0,0 @@ -module Spree - module Admin - module OrdersHelper - def order_links(order) - @order ||= order - links = [] - links << edit_order_link unless action_name == "edit" - links.concat(complete_order_links) if @order.complete? - links << ship_order_link if @order.ready_to_ship? - links << cancel_order_link if @order.can_cancel? - links - end - - private - - def complete_order_links - [resend_confirmation_link] + invoice_links + ticket_links - end - - def invoice_links - return [] unless Spree::Config[:enable_invoices?] - - [send_invoice_link, print_invoice_link] - end - - def send_invoice_link - if @order.distributor.can_invoice? - send_invoice_link_with_url - else - send_invoice_link_without_url - end - end - - def ticket_links - return [] unless Spree::Config[:enable_receipt_printing?] - - [print_ticket_link, select_ticket_printer_link] - end - - def edit_order_link - { name: t(:edit_order), - url: edit_admin_order_path(@order), - icon: 'icon-edit' } - end - - def resend_confirmation_link - { name: t(:resend_confirmation), - url: resend_admin_order_path(@order), - icon: 'icon-email', - method: 'post', - confirm: t(:confirm_resend_order_confirmation) } - end - - def send_invoice_link_with_url - { name: t(:send_invoice), - url: invoice_admin_order_path(@order), - icon: 'icon-email', - confirm: t(:confirm_send_invoice) } - end - - def send_invoice_link_without_url - { name: t(:send_invoice), - url: "#", - icon: 'icon-email', - confirm: t(:must_have_valid_business_number, enterprise_name: @order.distributor.name) } - end - - def print_invoice_link - { name: t(:print_invoice), - url: print_admin_order_path(@order), - icon: 'icon-print', - target: "_blank" } - end - - def print_ticket_link - { name: t(:print_ticket), - url: print_ticket_admin_order_path(@order), - icon: 'icon-print', - target: "_blank" } - end - - def select_ticket_printer_link - { name: t(:select_ticket_printer), - url: "#{print_ticket_admin_order_path(@order)}#select-printer", - icon: 'icon-print', - target: "_blank" } - end - - def ship_order_link - { name: t(:ship_order), - url: fire_admin_order_path(@order, e: 'ship'), - method: 'put', - icon: 'icon-truck', - confirm: t(:are_you_sure) } - end - - def cancel_order_link - { name: t(:cancel_order), - url: fire_admin_order_path(@order.number, e: 'cancel'), - icon: 'icon-trash', - confirm: t(:are_you_sure) } - end - end - end -end From bbb3748d3c296c581eb2944887f73b8ec88ab42d Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 15 Dec 2019 16:11:42 +0000 Subject: [PATCH 11/12] Simplify orders events logic --- .rubocop_todo.yml | 1 + .../spree/admin/orders_controller.rb | 5 ---- app/helpers/spree/admin/orders_helper.rb | 23 ++++++++++--------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bc0d0d90c4..33970eb805 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -335,6 +335,7 @@ Rails/OutputSafety: - 'app/helpers/spree/admin/zones_helper.rb' - 'app/helpers/spree/reports_helper.rb' - 'app/helpers/spree/admin/navigation_helper.rb' + - 'app/helpers/spree/admin/orders_helper.rb' - 'app/serializers/api/product_serializer.rb' - 'lib/spree/money_decorator.rb' - 'spec/features/admin/orders_spec.rb' diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index a6d8e250b7..2ddc9dccc7 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -7,7 +7,6 @@ module Spree include OpenFoodNetwork::SpreeApiKeyLoader helper CheckoutHelper - before_filter :initialize_order_events before_filter :load_order, only: [:edit, :update, :fire, :resend, :invoice, :print, :print_ticket] before_filter :load_distribution_choices, only: [:new, :edit, :update] @@ -114,10 +113,6 @@ module Spree authorize! action, @order end - def initialize_order_events - @order_events = %w{cancel resume} - end - def model_class Spree::Order end diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb index 324e73fd51..6f464f63fb 100644 --- a/app/helpers/spree/admin/orders_helper.rb +++ b/app/helpers/spree/admin/orders_helper.rb @@ -1,22 +1,15 @@ module Spree module Admin module OrdersHelper - # Renders all the extension partials that may have been specified in the extensions def event_links links = [] - @order_events.sort.each do |event| - if @order.send("can_#{event}?") - links << button_link_to(Spree.t(event), fire_admin_order_url(@order, :e => event), - :method => :put, - :icon => "icon-#{event}", - :data => { :confirm => Spree.t(:order_sure_want_to, :event => Spree.t(event)) }) - end - end + links << event_link("cancel") if @order.can_cancel? + links << event_link("resume") if @order.can_resume? links.join(' ').html_safe end def line_item_shipment_price(line_item, quantity) - Spree::Money.new(line_item.price * quantity, { currency: line_item.currency }) + Spree::Money.new(line_item.price * quantity, currency: line_item.currency) end def order_links(order) @@ -117,7 +110,15 @@ module Spree url: fire_admin_order_path(@order.number, e: 'cancel'), icon: 'icon-trash', confirm: t(:are_you_sure) } - end + end + + def event_link(event) + button_link_to(Spree.t(event), + fire_admin_order_url(@order, e: event), + method: :put, + icon: "icon-#{event}", + data: { confirm: Spree.t(:order_sure_want_to, event: Spree.t(event)) }) + end end end end From d32e106bf0c3101df0e22d228195fadb6dc067e0 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 15 Dec 2019 16:28:59 +0000 Subject: [PATCH 12/12] Add partials needed in the orders edit page --- .../spree/admin/shared/_address_form.html.erb | 72 +++++++++++++++++++ .../spree/admin/shared/_update_order_state.js | 7 ++ 2 files changed, 79 insertions(+) create mode 100644 app/views/spree/admin/shared/_address_form.html.erb create mode 100644 app/views/spree/admin/shared/_update_order_state.js diff --git a/app/views/spree/admin/shared/_address_form.html.erb b/app/views/spree/admin/shared/_address_form.html.erb new file mode 100644 index 0000000000..012c40d9e8 --- /dev/null +++ b/app/views/spree/admin/shared/_address_form.html.erb @@ -0,0 +1,72 @@ +<% if use_billing %> +
+ + <%= check_box_tag 'order[use_billing]', '1', (!(@order.bill_address.empty? && @order.ship_address.empty?) && @order.bill_address.same_as?(@order.ship_address)) %> + <%= label_tag 'order[use_billing]', Spree.t(:use_billing_address) %> + +
+<% end %> + +<% is_shipping_address = name == Spree.t(:shipping_address) %> +<% shipping_or_billing = is_shipping_address ? 'shipping' : 'billing' %> +<% s_or_b = is_shipping_address ? 's' : 'b' %> + +
+
"> + <%= f.label :firstname, Spree.t(:first_name) + ':' %> + <%= f.text_field :firstname, :class => 'fullwidth' %> +
+
"> + <%= f.label :lastname, Spree.t(:last_name) + ':' %> + <%= f.text_field :lastname, :class => 'fullwidth' %> +
+ <% if Spree::Config[:company] %> +
"> + <%= f.label :company, Spree.t(:company) + ':' %> + <%= f.text_field :company, :class => 'fullwidth' %> +
+ <% end %> +
"> + <%= f.label :address1, Spree.t(:street_address) + ':' %> + <%= f.text_field :address1, :class => 'fullwidth' %> +
+
"> + <%= f.label :address2, Spree.t(:street_address_2) + ':' %> + <%= f.text_field :address2, :class => 'fullwidth' %> +
+
"> + <%= f.label :city, Spree.t(:city) + ':' %> + <%= f.text_field :city, :class => 'fullwidth' %> +
+
"> + <%= f.label :zipcode, Spree.t(:zip) + ':' %> + <%= f.text_field :zipcode, :class => 'fullwidth' %> +
+
"> + <%= f.label :country_id, Spree.t(:country) + ':' %> + + <%= f.collection_select :country_id, available_countries, :id, :name, {}, {:class => 'select2 fullwidth'} %> + +
+
"> + <%= f.label :state_id, Spree.t(:state) + ':' %> + + <%= f.text_field :state_name, + :style => "display: #{f.object.country.states.empty? ? 'block' : 'none' };", + :disabled => !f.object.country.states.empty?, :class => 'fullwidth state_name' %> + <%= f.collection_select :state_id, f.object.country.states.sort, :id, :name, {:include_blank => true}, {:class => 'select2 fullwidth', :style => "display: #{f.object.country.states.empty? ? 'none' : 'block' };", :disabled => f.object.country.states.empty?} %> + +
+
"> + <%= f.label :phone, Spree.t(:phone) + ':' %> + <%= f.phone_field :phone, :class => 'fullwidth' %> +
+
+ +<% content_for :head do %> + <%= javascript_tag do -%> + $(document).ready(function(){ + $('span#<%= s_or_b %>country .select2').on('change', function() { update_state('<%= s_or_b %>'); }); + }); + <% end -%> +<% end %> diff --git a/app/views/spree/admin/shared/_update_order_state.js b/app/views/spree/admin/shared/_update_order_state.js new file mode 100644 index 0000000000..32b2d4d574 --- /dev/null +++ b/app/views/spree/admin/shared/_update_order_state.js @@ -0,0 +1,7 @@ +$('#order_tab_summary h5#order_status').html('<%= j Spree.t(:status) %>: <%= j Spree.t(@order.state, :scope => :order_state) %>'); +$('#order_tab_summary h5#order_total').html('<%= j Spree.t(:total) %>: <%= j @order.display_total.to_html %>'); + +<% if @order.completed? %> + $('#order_tab_summary h5#payment_status').html('<%= j Spree.t(:payment) %>: <%= j Spree.t(@order.payment_state, :scope => :payment_states, :default => [:missing, "none"]) %>'); + $('#order_tab_summary h5#shipment_status').html('<%= j Spree.t(:shipment) %>: <%= j Spree.t(@order.shipment_state, :scope => :shipment_state, :default => [:missing, "none"]) %>'); +<% end %>