From e577bcb46fcec3024aec9dda7f4cf11484f488f1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 12 Sep 2018 12:49:16 +0100 Subject: [PATCH 01/21] Prepare angular controller and serialized data --- .../controllers/orders_controller.js.coffee | 6 +++++- .../admin/orders_controller_decorator.rb | 21 +++++++++---------- app/serializers/api/admin/order_serializer.rb | 2 +- app/views/spree/admin/orders/index.html.haml | 9 +++++++- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee index 060b23bed9..afe3e87d2e 100644 --- a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee +++ b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.orders").controller "ordersCtrl", ($scope, $compile, $attrs, shops, orderCycles) -> +angular.module("admin.orders").controller "ordersCtrl", ($scope, $compile, $attrs, Orders) -> $scope.$compile = $compile $scope.shops = shops $scope.orderCycles = orderCycles @@ -6,6 +6,10 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, $compile, $attr $scope.distributor_id = parseInt($attrs.ofnDistributorId) $scope.order_cycle_id = parseInt($attrs.ofnOrderCycleId) + $scope.orders = Orders.all + + Orders.index(per_page: 15) + $scope.validOrderCycle = (oc) -> $scope.orderCycleHasDistributor oc, parseInt($scope.distributor_id) diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb index 49f017930d..43d1ce16b8 100644 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ b/app/controllers/spree/admin/orders_controller_decorator.rb @@ -100,18 +100,17 @@ Spree::Admin::OrdersController.class_eval do private def orders - if json_request? - @search = OpenFoodNetwork::Permissions.new(spree_current_user).editable_orders.ransack(params[:q]) - @search.result.reorder('id ASC') - else - @search = Spree::Order.accessible_by(current_ability, :index).ransack(params[:q]) + @search = if json_request? + OpenFoodNetwork::Permissions.new(spree_current_user).editable_orders.ransack(params[:q]) + else + Spree::Order.accessible_by(current_ability, :index).ransack(params[:q]) + end - # Replaced this search to filter orders to only show those distributed by current user (or all for admin user) - @search.result.includes([:user, :shipments, :payments]). - distributed_by_user(spree_current_user). - page(params[:page]). - per(params[:per_page] || Spree::Config[:orders_per_page]) - end + # Replaced this search to filter orders to only show those distributed by current user (or all for admin user) + @search.result.includes([:user, :shipments, :payments]). + distributed_by_user(spree_current_user). + page(params[:page]). + per(params[:per_page] || Spree::Config[:orders_per_page]) end def require_distributor_abn diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb index 6f22ba1e94..a010636307 100644 --- a/app/serializers/api/admin/order_serializer.rb +++ b/app/serializers/api/admin/order_serializer.rb @@ -1,5 +1,5 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer - attributes :id, :number, :full_name, :email, :phone, :completed_at + attributes :id, :number, :full_name, :email, :phone, :completed_at, :payment_total has_one :distributor, serializer: Api::Admin::IdSerializer has_one :order_cycle, serializer: Api::Admin::IdSerializer diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 3b233424cc..68e1763d99 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -1,11 +1,18 @@ - content_for :page_title do = t(:listing_orders) + - content_for :page_actions do %li = button_link_to t(:new_order), new_admin_order_url, :icon => 'icon-plus', :id => 'admin_new_order' + = render partial: 'spree/admin/shared/order_sub_menu' + +- content_for :app_wrapper_attrs do + = "ng-app='admin.orders'" + - content_for :table_filter_title do = t(:search) + - content_for :table_filter do %div{"data-hook" => "admin_orders_index_search"} = search_form_for [:admin, @search] do |f| @@ -59,7 +66,7 @@ = button t(:filter_results), 'icon-search' - unless @orders.empty? - %table#listing_orders.index.responsive{"data-hook" => "", width: "100%", "ng-app" => "ofn.admin"} + %table#listing_orders.index.responsive{"data-hook" => "", width: "100%", "ng-controller" => "ordersCtrl"} %colgroup %col{style: "width: 10%"} %thead From 93d273f94a7293a5910097e10261a5621202912b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 12 Sep 2018 18:00:21 +0100 Subject: [PATCH 02/21] Convert orders index table to use angular ng-repeat --- .../controllers/orders_controller.js.coffee | 10 +- .../admin/resources/services/orders.js.coffee | 6 +- app/serializers/api/admin/order_serializer.rb | 75 ++++++++++++- .../spree/admin/orders/_capture.html.haml | 7 -- app/views/spree/admin/orders/index.html.haml | 105 ++++++++++-------- config/locales/en.yml | 2 + 6 files changed, 143 insertions(+), 62 deletions(-) delete mode 100644 app/views/spree/admin/orders/_capture.html.haml diff --git a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee index afe3e87d2e..abbf84a06f 100644 --- a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee +++ b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.orders").controller "ordersCtrl", ($scope, $compile, $attrs, Orders) -> +angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, $compile, $attrs, Orders) -> $scope.$compile = $compile $scope.shops = shops $scope.orderCycles = orderCycles @@ -6,9 +6,15 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, $compile, $attr $scope.distributor_id = parseInt($attrs.ofnDistributorId) $scope.order_cycle_id = parseInt($attrs.ofnOrderCycleId) + $scope.RequestMonitor = RequestMonitor $scope.orders = Orders.all + $scope.per_page = 15 + $scope.page = 1 - Orders.index(per_page: 15) + Orders.index( + per_page: $scope.per_page, + page: $scope.page + ) $scope.validOrderCycle = (oc) -> $scope.orderCycleHasDistributor oc, parseInt($scope.distributor_id) diff --git a/app/assets/javascripts/admin/resources/services/orders.js.coffee b/app/assets/javascripts/admin/resources/services/orders.js.coffee index da3f409149..22377d3a8e 100644 --- a/app/assets/javascripts/admin/resources/services/orders.js.coffee +++ b/app/assets/javascripts/admin/resources/services/orders.js.coffee @@ -1,12 +1,14 @@ -angular.module("admin.resources").factory 'Orders', ($q, OrderResource) -> +angular.module("admin.resources").factory 'Orders', ($q, OrderResource, RequestMonitor) -> new class Orders byID: {} pristineByID: {} index: (params={}, callback=null) -> - OrderResource.index params, (data) => + request = OrderResource.index params, (data) => @load(data) (callback || angular.noop)(data) + RequestMonitor.load(request.$promise) + request load: (orders) -> for order in orders diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb index a010636307..065025d22f 100644 --- a/app/serializers/api/admin/order_serializer.rb +++ b/app/serializers/api/admin/order_serializer.rb @@ -1,5 +1,8 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer - attributes :id, :number, :full_name, :email, :phone, :completed_at, :payment_total + attributes :id, :number, :full_name, :email, :phone, :completed_at, :display_total + attributes :show_path, :edit_path, :state, :payment_state, :shipment_state + attributes :payments_path, :shipments_path, :ship_path, :ready_to_ship, :created_at + attributes :distributor_name, :special_instructions, :pending_payments, :capture_path has_one :distributor, serializer: Api::Admin::IdSerializer has_one :order_cycle, serializer: Api::Admin::IdSerializer @@ -8,6 +11,48 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer object.billing_address.nil? ? "" : ( object.billing_address.full_name || "" ) end + def distributor_name + object.distributor.andand.name + end + + def show_path + return '' unless object.id + Spree::Core::Engine.routes_url_helpers.admin_order_path(object) + end + + def edit_path + return '' unless object.id + Spree::Core::Engine.routes_url_helpers.edit_admin_order_path(object) + end + + def payments_path + return '' unless object.payment_state + Spree::Core::Engine.routes_url_helpers.admin_order_payments_path(object) + end + + def shipments_path + return '' unless object.shipment_state + Spree::Core::Engine.routes_url_helpers.admin_order_shipments_path(object) + end + + def ship_path + Spree::Core::Engine.routes_url_helpers.fire_admin_order_path(object, e: 'ship') + end + + def capture_path + return '' unless ready_for_payment? + return unless payment_to_capture + Spree::Core::Engine.routes_url_helpers.fire_admin_order_payment_path(object, payment_to_capture.id, e: 'capture') + end + + def ready_to_ship + object.ready_to_ship? + end + + def display_total + object.display_total.to_html + end + def email object.email || "" end @@ -16,7 +61,33 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer object.billing_address.nil? ? "a" : ( object.billing_address.phone || "" ) end + def created_at + object.created_at.blank? ? "" : I18n.l(object.created_at, format: '%B %d, %Y') + end + def completed_at - object.completed_at.blank? ? "" : object.completed_at.strftime("%F %T") + object.completed_at.blank? ? "" : I18n.l(object.completed_at, format: '%B %d, %Y') + end + + def pending_payments + return if object.payments.blank? + payment = object.payments.select{ |p| p if p.state == 'checkout' }.first + return unless can_be_captured? payment + + payment.id + end + + private + + def ready_for_payment? + object.payment_required? && object.payments.present? + end + + def payment_to_capture + object.payments.select{ |p| p if p.state == 'checkout' }.first + end + + def can_be_captured?(payment) + payment && payment.actions.include?('capture') end end diff --git a/app/views/spree/admin/orders/_capture.html.haml b/app/views/spree/admin/orders/_capture.html.haml deleted file mode 100644 index b1267badbb..0000000000 --- a/app/views/spree/admin/orders/_capture.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -- # Get the payment in 'checkout' state if any, and show capture button -- if order.payments.present? - - payment = order.payments.select{|p| p if p.state == 'checkout'}.first - - if !payment.nil? - - payment.actions.grep(/^capture$/).each do |action| - - # copied from backend/app/views/spree/admin/payments/_list.html.erb - = link_to_with_icon "icon-#{action}", t('admin.orders.index.capture'), fire_admin_order_payment_path(order, payment, :e => action), :method => :put, :no_text => true, :data => {:action => action} diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 68e1763d99..44f5bd3432 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -8,7 +8,7 @@ = render partial: 'spree/admin/shared/order_sub_menu' - content_for :app_wrapper_attrs do - = "ng-app='admin.orders'" + = "ng-app='admin.orders' ng-controller='ordersCtrl'" - content_for :table_filter_title do = t(:search) @@ -65,53 +65,60 @@ %div{"data-hook" => "admin_orders_index_search_buttons"} = button t(:filter_results), 'icon-search' -- unless @orders.empty? - %table#listing_orders.index.responsive{"data-hook" => "", width: "100%", "ng-controller" => "ordersCtrl"} - %colgroup - %col{style: "width: 10%"} - %thead - %tr{"data-hook" => "admin_orders_index_headers"} - %th - = t(:products_distributor) - - if @show_only_completed - %th= sort_link @search, :completed_at, t(:completed_at, :scope => 'activerecord.attributes.spree/order') - - else - %th= sort_link @search, :created_at, t(:created_at, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :number, t(:number, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :state, t(:state, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :payment_state, t(:payment_state, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :shipment_state, t(:shipment_state, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :email, t(:email, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :total, t(:total, :scope => 'activerecord.attributes.spree/order') - %th.actions{"data-hook" => "admin_orders_index_header_actions"} - %tbody - - @orders.each do |order| - %tr{class: "state-#{order.state.downcase} #{cycle('odd', 'even')}", "data-hook" => "admin_orders_index_rows"} - %td.align-center - = order.distributor.andand.name - %td.align-center= l (@show_only_completed ? order.completed_at : order.created_at).to_date - %td - = link_to order.number, admin_order_path(order) - - if order.special_instructions.present? - %br - %span{class: "icon-warning-sign", "ofn-with-tip" => order.special_instructions} - notes - %td.align-center - %span{class: "state #{order.state.downcase}"}= t("order_state.#{order.state.downcase}") - %td.align-center - %span{class: "state #{order.payment_state}"}= link_to t("payment_states.#{order.payment_state}"), admin_order_payments_path(order) if order.payment_state - %td.align-center - %span{class: "state #{order.shipment_state}"}= link_to t("shipment_states.#{order.shipment_state}"), admin_order_shipments_path(order) if order.shipment_state - %td= mail_to order.email - %td.align-center= order.display_total.to_html - %td.actions{"data-hook" => "admin_orders_index_row_actions"} - = link_to_edit_url edit_admin_order_path(order), :title => "admin_edit_#{dom_id(order)}", :no_text => true - - if order.ready_to_ship? - - # copied from backend/app/views/spree/admin/payments/_list.html.erb - = link_to_with_icon "icon-road", t('admin.orders.index.ship'), fire_admin_order_url(order, :e => 'ship'), :method => :put, :no_text => true, :data => {:action => 'ship', :confirm => t(:are_you_sure)} - = render partial: 'spree/admin/orders/capture', locals: {order: order} -- else - .no-objects-found - = t(:no_orders_found) +%table#listing_orders.index.responsive{"data-hook" => "", width: "100%", 'ng-show' => "!RequestMonitor.loading && orders.length > 0" } + %colgroup + %col{style: "width: 10%"} + %thead + %tr{"data-hook" => "admin_orders_index_headers"} + %th + = t(:products_distributor) + - if @show_only_completed + %th= sort_link @search, :completed_at, t(:completed_at, :scope => 'activerecord.attributes.spree/order') + - else + %th= sort_link @search, :created_at, t(:created_at, :scope => 'activerecord.attributes.spree/order') + %th= sort_link @search, :number, t(:number, :scope => 'activerecord.attributes.spree/order') + %th= sort_link @search, :state, t(:state, :scope => 'activerecord.attributes.spree/order') + %th= sort_link @search, :payment_state, t(:payment_state, :scope => 'activerecord.attributes.spree/order') + %th= sort_link @search, :shipment_state, t(:shipment_state, :scope => 'activerecord.attributes.spree/order') + %th= sort_link @search, :email, t(:email, :scope => 'activerecord.attributes.spree/order') + %th= sort_link @search, :total, t(:total, :scope => 'activerecord.attributes.spree/order') + %th.actions{"data-hook" => "admin_orders_index_header_actions"} + %tbody + %tr{ng: {repeat: 'order in orders track by $index', class: {even: "'even'", odd: "'odd'"}}, 'ng-class' => "'state-{{order.state}}'", "data-hook" => "admin_orders_index_rows"} + %td.align-center + {{::order.distributor_name}} + %td.align-center + = @show_only_completed ? '{{::order.completed_at}}' : '{{::order.created_at}}' + %td + %a{'ng-href' => '{{::order.show_path}}'} + {{order.number}} + %div{'ng-if' => 'order.special_instructions'} + %br + %span.icon-warning-sign{'ofn-with-tip' => "{{::order.special_instructions}}"} + = t(:note) + %td.align-center + %span.state{'ng-class' => 'order.state'} + {{'order_state.' + order.state | t}} + %td.align-center + %span.state{'ng-class' => 'order.payment_state', 'ng-if' => 'order.payment_state'} + %a{'ng-href' => '{{::order.payments_path}}' } + {{'payment_states.' + order.payment_state | t}} + %td.align-center + %span.state{'ng-class' => 'order.shipment_state', 'ng-if' => 'order.shipment_state'} + %a{'ng-href' => '{{::order.shipments_path}}' } + {{'shipment_states.' + order.shipment_state | t}} + %td + = mail_to "{{::order.email}}" + %td.align-center + %span{'ng-bind-html' => '::order.display_total'} + %td.actions{"data-hook" => "admin_orders_index_row_actions"} + %a.icon_link.with-tip.icon-edit.no-text{'ng-href' => '{{::order.edit_path}}', 'data-action' => 'edit', 'ofn-with-tip' => t(:edit)} + %div{'ng-if' => 'order.ready_to_ship'} + %a.icon-road.icon_link.with-tip.no-text{'ng-href' => '{{::order.ship_path}}', 'data-action' => 'ship', 'data-confirm' => t(:are_you_sure), 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:ship)} + %div{'ng-if' => 'order.pending_payments'} + %a.icon-capture.icon_link.no-text{'ng-href' => '{{::order.capture_path}}', 'data-action' => 'capture', 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:capture)} + +.no-objects-found{'ng-show' => "!RequestMonitor.loading && orders.length == 0"} + = t(:no_orders_found) = paginate @orders diff --git a/config/locales/en.yml b/config/locales/en.yml index b9dabc2129..8250e549d3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -616,6 +616,8 @@ en: index: capture: "Capture" ship: "Ship" + edit: "Edit" + note: "Note" invoice_email_sent: 'Invoice email has been sent' order_email_resent: 'Order email has been resent' bulk_management: From 3b9d9db16b058e57b8eb6ac8e59a8515f2374d0f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 13 Sep 2018 11:00:05 +0100 Subject: [PATCH 03/21] Add pagination --- .../controllers/orders_controller.js.coffee | 20 +++++++++++++------ .../resources/order_resource.js.coffee | 1 - .../admin/resources/services/orders.js.coffee | 6 ++++-- .../admin/components/pagination.scss | 20 +++++++++++++++++++ .../admin/orders_controller_decorator.rb | 9 ++++++++- .../orders/_angular_pagination.html.haml | 17 ++++++++++++++++ app/views/spree/admin/orders/index.html.haml | 7 ++++--- config/locales/en.yml | 4 ++++ 8 files changed, 71 insertions(+), 13 deletions(-) create mode 100644 app/assets/stylesheets/admin/components/pagination.scss create mode 100644 app/views/spree/admin/orders/_angular_pagination.html.haml diff --git a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee index abbf84a06f..7243c0f178 100644 --- a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee +++ b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee @@ -8,13 +8,16 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, $scope.RequestMonitor = RequestMonitor $scope.orders = Orders.all - $scope.per_page = 15 - $scope.page = 1 + $scope.pagination = Orders.pagination - Orders.index( - per_page: $scope.per_page, - page: $scope.page - ) + $scope.initialise = -> + $scope.fetchResults() + + $scope.fetchResults = -> + Orders.index({ + per_page: $scope.per_page || 15, + page: $scope.page || 1 + }) $scope.validOrderCycle = (oc) -> $scope.orderCycleHasDistributor oc, parseInt($scope.distributor_id) @@ -29,6 +32,11 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, $scope.distributionChosen = -> $scope.distributor_id && $scope.order_cycle_id + $scope.changePage = (newPage) -> + $scope.page = newPage + Orders.resetData() + $scope.fetchResults() + for oc in $scope.orderCycles oc.name_and_status = "#{oc.name} (#{oc.status})" diff --git a/app/assets/javascripts/admin/resources/resources/order_resource.js.coffee b/app/assets/javascripts/admin/resources/resources/order_resource.js.coffee index 317b384485..d5679c629e 100644 --- a/app/assets/javascripts/admin/resources/resources/order_resource.js.coffee +++ b/app/assets/javascripts/admin/resources/resources/order_resource.js.coffee @@ -2,7 +2,6 @@ angular.module("admin.resources").factory 'OrderResource', ($resource) -> $resource('/admin/orders/:id/:action.json', {}, { 'index': method: 'GET' - isArray: true 'update': method: 'PUT' }) diff --git a/app/assets/javascripts/admin/resources/services/orders.js.coffee b/app/assets/javascripts/admin/resources/services/orders.js.coffee index 22377d3a8e..c4ec6b4a80 100644 --- a/app/assets/javascripts/admin/resources/services/orders.js.coffee +++ b/app/assets/javascripts/admin/resources/services/orders.js.coffee @@ -2,6 +2,7 @@ angular.module("admin.resources").factory 'Orders', ($q, OrderResource, RequestM new class Orders byID: {} pristineByID: {} + pagination: {} index: (params={}, callback=null) -> request = OrderResource.index params, (data) => @@ -10,8 +11,9 @@ angular.module("admin.resources").factory 'Orders', ($q, OrderResource, RequestM RequestMonitor.load(request.$promise) request - load: (orders) -> - for order in orders + load: (data) -> + angular.extend(@pagination, data.pagination) + for order in data.orders @byID[order.id] = order @pristineByID[order.id] = angular.copy(order) diff --git a/app/assets/stylesheets/admin/components/pagination.scss b/app/assets/stylesheets/admin/components/pagination.scss new file mode 100644 index 0000000000..fd3705a6fd --- /dev/null +++ b/app/assets/stylesheets/admin/components/pagination.scss @@ -0,0 +1,20 @@ +@import "admin/variables"; + +.pagination { + text-align: center; + margin: 2em 0 1em; + + button { + margin: 0 0.35em; + + &.active { + background-color: darken($spree-blue, 15%); + cursor: default; + } + + &.disabled { + background-color: #ccc; + cursor: default; + } + } +} diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb index 43d1ce16b8..8c0c914d9e 100644 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ b/app/controllers/spree/admin/orders_controller_decorator.rb @@ -61,7 +61,14 @@ Spree::Admin::OrdersController.class_eval do respond_with(@orders) do |format| format.html format.json do - render_as_json @orders + render json: { + orders: ActiveModel::ArraySerializer.new(@orders, each_serializer: Api::Admin::OrderSerializer), + pagination: { + results: @orders.total_count, + pages: @orders.num_pages, + page: params[:page].to_i + } + } end end end diff --git a/app/views/spree/admin/orders/_angular_pagination.html.haml b/app/views/spree/admin/orders/_angular_pagination.html.haml new file mode 100644 index 0000000000..53a28af885 --- /dev/null +++ b/app/views/spree/admin/orders/_angular_pagination.html.haml @@ -0,0 +1,17 @@ +.pagination + %button{'ng-click' => 'changePage(1)', 'ng-class' => "{'disabled': pagination.page == 1}", 'ng-disabled' => "pagination.page == 1"} + = "«".html_safe + = t(:first) + %button{'ng-click' => 'changePage((pagination.page)-1)', 'ng-class' => "{'disabled': pagination.page == 1}", 'ng-disabled' => "pagination.page == 1"} + = t(:previous) + %span{'ng-show' => 'pagination.page > 3'} + = "…".html_safe + %button{'ng-repeat' => 'i in [].constructor(pagination.pages) track by $index', 'ng-show' =>'($index+1 > pagination.page-3 || (pagination.page > pagination.pages-2 && $index+1 > pagination.pages-5)) && ($index+1 < pagination.page+3 || (pagination.page < 3 && $index+1 < 6))', 'ng-class' => "{'active': pagination.page == $index+1}", 'ng-click' => 'changePage($index+1)', 'ng-disabled' => "pagination.page == $index+1"} + {{$index+1}} + %span{'ng-show' => 'pagination.page < pagination.pages-2'} + = "…".html_safe + %button{'ng-click' => 'changePage((pagination.page)+1)', 'ng-class' => "{'disabled': pagination.page == pagination.pages}", 'ng-disabled' => "pagination.page == pagination.pages"} + = t(:next) + %button{'ng-click' => 'changePage(pagination.pages)', 'ng-class' => "{'disabled': pagination.page == pagination.pages}", 'ng-disabled' => "pagination.page == pagination.pages"} + = t(:last) + = "»".html_safe diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 44f5bd3432..f7f4d90a11 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -65,7 +65,7 @@ %div{"data-hook" => "admin_orders_index_search_buttons"} = button t(:filter_results), 'icon-search' -%table#listing_orders.index.responsive{"data-hook" => "", width: "100%", 'ng-show' => "!RequestMonitor.loading && orders.length > 0" } +%table#listing_orders.index.responsive{"data-hook" => "", width: "100%", 'ng-init' => 'initialise()', 'ng-show' => "!RequestMonitor.loading && orders.length > 0" } %colgroup %col{style: "width: 10%"} %thead @@ -118,7 +118,8 @@ %div{'ng-if' => 'order.pending_payments'} %a.icon-capture.icon_link.no-text{'ng-href' => '{{::order.capture_path}}', 'data-action' => 'capture', 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:capture)} +%div{'ng-show' => "!RequestMonitor.loading && orders.length > 0" } + = render partial: 'angular_pagination' + .no-objects-found{'ng-show' => "!RequestMonitor.loading && orders.length == 0"} = t(:no_orders_found) - -= paginate @orders diff --git a/config/locales/en.yml b/config/locales/en.yml index 8250e549d3..16a85986b1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -618,6 +618,10 @@ en: ship: "Ship" edit: "Edit" note: "Note" + first: "First" + last: "Last" + previous: "Previous" + next: "Next" invoice_email_sent: 'Invoice email has been sent' order_email_resent: 'Order email has been resent' bulk_management: From 2112f296e42e6e955f770d6164e60c8e7eab816a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 13 Sep 2018 12:33:10 +0100 Subject: [PATCH 04/21] Angularise filters --- .../controllers/orders_controller.js.coffee | 24 +++++++-- .../admin/resources/services/orders.js.coffee | 8 +++ .../spree/admin/orders/_filters.html.haml | 51 ++++++++++++++++++ app/views/spree/admin/orders/index.html.haml | 53 +------------------ 4 files changed, 80 insertions(+), 56 deletions(-) create mode 100644 app/views/spree/admin/orders/_filters.html.haml diff --git a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee index 7243c0f178..21a5d42b91 100644 --- a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee +++ b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee @@ -7,16 +7,31 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, $scope.order_cycle_id = parseInt($attrs.ofnOrderCycleId) $scope.RequestMonitor = RequestMonitor - $scope.orders = Orders.all $scope.pagination = Orders.pagination + $scope.orders = Orders.all $scope.initialise = -> + $scope.q = { + completed_at_not_null: true + } $scope.fetchResults() - $scope.fetchResults = -> + $scope.fetchResults = (page=1) -> Orders.index({ + 'q[created_at_lt]': $scope['q']['created_at_lt'], + 'q[created_at_gt]': $scope['q']['created_at_gt'], + 'q[state_eq]': $scope['q']['state_eq'], + 'q[number_cont]': $scope['q']['number_cont'], + 'q[email_cont]': $scope['q']['email_cont'], + 'q[bill_address_firstname_start]': $scope['q']['bill_address_firstname_start'], + 'q[bill_address_lastname_start]': $scope['q']['bill_address_lastname_start'], + 'q[completed_at_not_null]': $scope['q']['completed_at_not_null'], + 'q[inventory_units_shipment_id_null]': $scope['q']['inventory_units_shipment_id_null'], + 'q[distributor_id_in]': $scope['q']['distributor_id_in'], + 'q[order_cycle_id_in]': $scope['q']['order_cycle_id_in'], + 'q[order_cycle_id_in]': $scope['q']['order_cycle_id_in'], per_page: $scope.per_page || 15, - page: $scope.page || 1 + page: page }) $scope.validOrderCycle = (oc) -> @@ -34,8 +49,7 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, $scope.changePage = (newPage) -> $scope.page = newPage - Orders.resetData() - $scope.fetchResults() + $scope.fetchResults(newPage) for oc in $scope.orderCycles oc.name_and_status = "#{oc.name} (#{oc.status})" diff --git a/app/assets/javascripts/admin/resources/services/orders.js.coffee b/app/assets/javascripts/admin/resources/services/orders.js.coffee index c4ec6b4a80..332dc44796 100644 --- a/app/assets/javascripts/admin/resources/services/orders.js.coffee +++ b/app/assets/javascripts/admin/resources/services/orders.js.coffee @@ -1,5 +1,6 @@ angular.module("admin.resources").factory 'Orders', ($q, OrderResource, RequestMonitor) -> new class Orders + all: [] byID: {} pristineByID: {} pagination: {} @@ -13,10 +14,17 @@ angular.module("admin.resources").factory 'Orders', ($q, OrderResource, RequestM load: (data) -> angular.extend(@pagination, data.pagination) + @clearData() for order in data.orders + @all.push order @byID[order.id] = order @pristineByID[order.id] = angular.copy(order) + clearData: -> + @all.length = 0 + @byID = {} + @pristineByID = {} + save: (order) -> deferred = $q.defer() order.$update({id: order.number}) diff --git a/app/views/spree/admin/orders/_filters.html.haml b/app/views/spree/admin/orders/_filters.html.haml new file mode 100644 index 0000000000..5ebea2a4cd --- /dev/null +++ b/app/views/spree/admin/orders/_filters.html.haml @@ -0,0 +1,51 @@ +%div{"data-hook" => "admin_orders_index_search"} + = search_form_for [:admin, @search], html: { name: "orders_form", "ng-submit" => "fetchResults()"} do |f| + .field-block.alpha.four.columns + .date-range-filter.field + = label_tag nil, t(:date_range) + .date-range-fields + = f.text_field :created_at_gt, class: 'datepicker', datepicker: 'q.created_at_gt', 'ng-model' => 'q.created_at_gt', :value => params[:q][:created_at_gt], :placeholder => t(:start) + %span.range-divider + %i.icon-arrow-right + = f.text_field :created_at_lt, class: 'datepicker', datepicker: 'q.created_at_lt', 'ng-model' => 'q.created_at_lt', :value => params[:q][:created_at_lt], :placeholder => t(:stop) + .field + = label_tag nil, t(:status) + = f.select :state_eq, Spree::Order.state_machines[:state].states.collect {|s| [t("order_state.#{s.name}"), s.value]}, {:include_blank => true}, :class => 'select2', 'ng-model' => 'q.state_eq' + .four.columns + .field + = label_tag nil, t(:order_number) + = f.text_field :number_cont, 'ng-model' => 'q.number_cont' + .field + = label_tag nil, t(:email) + = f.email_field :email_cont, 'ng-model' => 'q.email_cont' + .four.columns + .field + = label_tag nil, t(:first_name_begins_with) + = f.text_field :bill_address_firstname_start, :size => 25, 'ng-model' => 'q.bill_address_firstname_start' + .field + = label_tag nil, t(:last_name_begins_with) + = f.text_field :bill_address_lastname_start, :size => 25, 'ng-model' => 'q.bill_address_lastname_start' + .omega.four.columns + .field.checkbox + %label + = f.check_box :completed_at_not_null, {:checked => @show_only_completed, 'ng-model' => 'q.completed_at_not_null'}, '1', '' + = t(:show_only_complete_orders) + .field.checkbox + %label + = f.check_box :inventory_units_shipment_id_null, {'ng-model' => 'q.inventory_units_shipment_id_null'}, '1', '0' + = t(:show_only_unfulfilled_orders) + .field-block.alpha.eight.columns + = label_tag nil, t(:distributors) + = select_tag("q[distributor_id_in]", + options_for_select(Enterprise.is_distributor.managed_by(spree_current_user).map {|e| [e.name, e.id]}, params[:distributor_ids]), + {class: "select2 fullwidth", multiple: true, 'ng-model' => 'q.distributor_id_in'}) + .field-block.omega.eight.columns + = label_tag nil, t(:order_cycles) + = select_tag("q[order_cycle_id_in]", + options_for_select(OrderCycle.managed_by(spree_current_user).where('order_cycles.orders_close_at is not null').order('order_cycles.orders_close_at DESC').map {|oc| [oc.name, oc.id]}, params[:order_cycle_ids]), + {class: "select2 fullwidth", multiple: true, 'ng-model' => 'q.order_cycle_id_in'}) + .clearfix + .actions.filter-actions + %div + %a.button.icon-search{'ng-click' => 'fetchResults()'} + = t(:filter_results) diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index f7f4d90a11..209c67b36e 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -14,57 +14,8 @@ = t(:search) - content_for :table_filter do - %div{"data-hook" => "admin_orders_index_search"} - = search_form_for [:admin, @search] do |f| - .field-block.alpha.four.columns - .date-range-filter.field - = label_tag nil, t(:date_range) - .date-range-fields - = f.text_field :created_at_gt, :class => 'datepicker datepicker-from', :value => params[:q][:created_at_gt], :placeholder => t(:start) - %span.range-divider - %i.icon-arrow-right - = f.text_field :created_at_lt, :class => 'datepicker datepicker-to', :value => params[:q][:created_at_lt], :placeholder => t(:stop) - .field - = label_tag nil, t(:status) - = f.select :state_eq, Spree::Order.state_machines[:state].states.collect {|s| [t("order_state.#{s.name}"), s.value]}, {:include_blank => true}, :class => 'select2' - .four.columns - .field - = label_tag nil, t(:order_number) - = f.text_field :number_cont - .field - = label_tag nil, t(:email) - = f.email_field :email_cont - .four.columns - .field - = label_tag nil, t(:first_name_begins_with) - = f.text_field :bill_address_firstname_start, :size => 25 - .field - = label_tag nil, t(:last_name_begins_with) - = f.text_field :bill_address_lastname_start, :size => 25 - .omega.four.columns - .field.checkbox - %label - = f.check_box :completed_at_not_null, {:checked => @show_only_completed}, '1', '' - = t(:show_only_complete_orders) - .field.checkbox - %label - = f.check_box :inventory_units_shipment_id_null, { }, '1', '0' - = t(:show_only_unfulfilled_orders) - .field-block.alpha.eight.columns - = label_tag nil, t(:distributors) - = select_tag("q[distributor_id_in]", - options_for_select(Enterprise.is_distributor.managed_by(spree_current_user).map {|e| [e.name, e.id]}, params[:distributor_ids]), - {class: "select2 fullwidth", multiple: true}) - .field-block.omega.eight.columns - = label_tag nil, t(:order_cycles) - = select_tag("q[order_cycle_id_in]", - options_for_select(OrderCycle.managed_by(spree_current_user).where('order_cycles.orders_close_at is not null').order('order_cycles.orders_close_at DESC').map {|oc| [oc.name, oc.id]}, params[:order_cycle_ids]), - {class: "select2 fullwidth", multiple: true}) - .clearfix - .actions.filter-actions - %div{"data-hook" => "admin_orders_index_search_buttons"} - = button t(:filter_results), 'icon-search' - + = render partial: 'filters' + %table#listing_orders.index.responsive{"data-hook" => "", width: "100%", 'ng-init' => 'initialise()', 'ng-show' => "!RequestMonitor.loading && orders.length > 0" } %colgroup %col{style: "width: 10%"} From 9da6a5a9b38fe04a19c8a85b890ddc9768599e9b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 17 Sep 2018 12:13:57 +0100 Subject: [PATCH 05/21] Add column sorting to table --- .../controllers/orders_controller.js.coffee | 11 ++++- app/views/spree/admin/orders/index.html.haml | 48 +++++++++++++++---- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee index 21a5d42b91..8382eb0332 100644 --- a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee +++ b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, $compile, $attrs, Orders) -> +angular.module("admin.orders").controller "ordersCtrl", ($scope, $injector, RequestMonitor, $compile, $attrs, Orders, SortOptions) -> $scope.$compile = $compile $scope.shops = shops $scope.orderCycles = orderCycles @@ -9,6 +9,7 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, $scope.RequestMonitor = RequestMonitor $scope.pagination = Orders.pagination $scope.orders = Orders.all + $scope.sortOptions = SortOptions $scope.initialise = -> $scope.q = { @@ -30,10 +31,18 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, 'q[distributor_id_in]': $scope['q']['distributor_id_in'], 'q[order_cycle_id_in]': $scope['q']['order_cycle_id_in'], 'q[order_cycle_id_in]': $scope['q']['order_cycle_id_in'], + 'q[s]': $scope.sorting || 'id desc', per_page: $scope.per_page || 15, page: page }) + $scope.$watch 'sortOptions', (sort) -> + if sort.predicate != "" + $scope.sorting = sort.predicate + ' desc' if sort.reverse + $scope.sorting = sort.predicate + ' asc' if !sort.reverse + $scope.fetchResults() + , true + $scope.validOrderCycle = (oc) -> $scope.orderCycleHasDistributor oc, parseInt($scope.distributor_id) diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 209c67b36e..35c0ad5009 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -24,15 +24,47 @@ %th = t(:products_distributor) - if @show_only_completed - %th= sort_link @search, :completed_at, t(:completed_at, :scope => 'activerecord.attributes.spree/order') + %th + %a{'ng-click' => "sortOptions.toggle('completed_at')"} + = t(:completed_at, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == 'completed_at asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == 'completed_at desc' || sorting === undefined"}= "▼".html_safe - else - %th= sort_link @search, :created_at, t(:created_at, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :number, t(:number, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :state, t(:state, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :payment_state, t(:payment_state, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :shipment_state, t(:shipment_state, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :email, t(:email, :scope => 'activerecord.attributes.spree/order') - %th= sort_link @search, :total, t(:total, :scope => 'activerecord.attributes.spree/order') + %th + %a{'ng-click' => "sortOptions.toggle('created_at')"} + = t(:created_at, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == 'created_at asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == 'created_at desc'"}= "▼".html_safe + %th + %a{'ng-click' => "sortOptions.toggle('number')"} + = t(:number, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == 'number asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == 'number desc'"}= "▼".html_safe + %th + %a{'ng-click' => "sortOptions.toggle('state')"} + = t(:state, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == 'state asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == 'state desc'"}= "▼".html_safe + %th + %a{'ng-click' => "sortOptions.toggle('payment_state')"} + = t(:payment_state, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == 'payment_state asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == 'payment_state desc'"}= "▼".html_safe + %th + %a{'ng-click' => "sortOptions.toggle('shipment_state')"} + = t(:shipment_state, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == 'shipment_state asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == 'shipment_state desc'"}= "▼".html_safe + %th + %a{'ng-click' => "sortOptions.toggle('email')"} + = t(:email, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == 'email asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == 'email desc'"}= "▼".html_safe + %th + %a{'ng-click' => "sortOptions.toggle('total')"} + = t(:total, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == 'total asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == 'total desc'"}= "▼".html_safe %th.actions{"data-hook" => "admin_orders_index_header_actions"} %tbody %tr{ng: {repeat: 'order in orders track by $index', class: {even: "'even'", odd: "'odd'"}}, 'ng-class' => "'state-{{order.state}}'", "data-hook" => "admin_orders_index_rows"} From 68f0c800168c636afea92eedeb6b10eb7f416e96 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 17 Sep 2018 12:45:26 +0100 Subject: [PATCH 06/21] Add loading message and spinner --- app/assets/stylesheets/admin/orders.css.scss | 12 ++++++++++++ app/views/spree/admin/orders/index.html.haml | 8 ++++++++ config/locales/en.yml | 1 + 3 files changed, 21 insertions(+) diff --git a/app/assets/stylesheets/admin/orders.css.scss b/app/assets/stylesheets/admin/orders.css.scss index 33c5a4a3e8..0955d9fd46 100644 --- a/app/assets/stylesheets/admin/orders.css.scss +++ b/app/assets/stylesheets/admin/orders.css.scss @@ -91,3 +91,15 @@ th.actions { table.index td.actions { text-align: left; } + +.orders-loading { + margin-top: 1em; + + img { + width: 85px; + } + + span { + font-size: 1.2em; + } +} diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 35c0ad5009..1191846869 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -101,6 +101,14 @@ %div{'ng-if' => 'order.pending_payments'} %a.icon-capture.icon_link.no-text{'ng-href' => '{{::order.capture_path}}', 'data-action' => 'capture', 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:capture)} +.orders-loading{'ng-show' => 'RequestMonitor.loading'} + .row + .small-12.columns.fullwidth.text-center + %img.spinner{ src: "/assets/spinning-circles.svg" } + .row + .small-12.columns.fullwidth.text-center + %span= t(:loading) + %div{'ng-show' => "!RequestMonitor.loading && orders.length > 0" } = render partial: 'angular_pagination' diff --git a/config/locales/en.yml b/config/locales/en.yml index 16a85986b1..e0a9863c03 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -622,6 +622,7 @@ en: last: "Last" previous: "Previous" next: "Next" + loading: "Loading" invoice_email_sent: 'Invoice email has been sent' order_email_resent: 'Order email has been resent' bulk_management: From b2551b4e0bd0574ea2d417c32c4100b829e1dd34 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 17 Sep 2018 16:34:52 +0100 Subject: [PATCH 07/21] Rewrite existing specs --- .../controllers/orders_controller.js.coffee | 2 +- .../admin/orders_controller_decorator.rb | 19 ++++++++--------- .../spree/admin/orders_controller_spec.rb | 21 ++++++++++--------- spec/features/admin/adjustments_spec.rb | 16 +++++++------- .../admin/bulk_order_management_spec.rb | 4 ++-- spec/features/admin/orders_spec.rb | 2 +- .../line_items_controller_spec.js.coffee | 4 ++-- .../orders_controller_spec.js.coffee | 18 +++++++++++++--- .../orders/services/orders_spec.js.coffee | 6 +++--- 9 files changed, 52 insertions(+), 40 deletions(-) diff --git a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee index 8382eb0332..1ce3da7595 100644 --- a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee +++ b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee @@ -37,7 +37,7 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, $injector, Requ }) $scope.$watch 'sortOptions', (sort) -> - if sort.predicate != "" + if sort && sort.predicate != "" $scope.sorting = sort.predicate + ' desc' if sort.reverse $scope.sorting = sort.predicate + ' asc' if !sort.reverse $scope.fetchResults() diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb index 8c0c914d9e..6df8943966 100644 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ b/app/controllers/spree/admin/orders_controller_decorator.rb @@ -107,17 +107,16 @@ Spree::Admin::OrdersController.class_eval do private def orders - @search = if json_request? - OpenFoodNetwork::Permissions.new(spree_current_user).editable_orders.ransack(params[:q]) - else - Spree::Order.accessible_by(current_ability, :index).ransack(params[:q]) - end + if json_request? + @search = OpenFoodNetwork::Permissions.new(spree_current_user).editable_orders.ransack(params[:q]) + else + @search = Spree::Order.accessible_by(current_ability, :index).ransack(params[:q]) - # Replaced this search to filter orders to only show those distributed by current user (or all for admin user) - @search.result.includes([:user, :shipments, :payments]). - distributed_by_user(spree_current_user). - page(params[:page]). - per(params[:per_page] || Spree::Config[:orders_per_page]) + # Replaced this search to filter orders to only show those distributed by current user (or all for admin user) + @search.result.includes([:user, :shipments, :payments]).distributed_by_user(spree_current_user) + end + + @search.result.page(params[:page]).per(params[:per_page] || Spree::Config[:orders_per_page]) end def require_distributor_abn diff --git a/spec/controllers/spree/admin/orders_controller_spec.rb b/spec/controllers/spree/admin/orders_controller_spec.rb index 25c2683d4d..8d76ea3a8d 100644 --- a/spec/controllers/spree/admin/orders_controller_spec.rb +++ b/spec/controllers/spree/admin/orders_controller_spec.rb @@ -66,26 +66,27 @@ describe Spree::Admin::OrdersController, type: :controller do end it "retrieves a list of orders with appropriate attributes, including line items with appropriate attributes" do - keys = json_response.first.keys.map{ |key| key.to_sym } + keys = json_response['orders'].first.keys.map{ |key| key.to_sym } order_attributes.all?{ |attr| keys.include? attr }.should == true end - it "sorts orders in ascending id order" do - ids = json_response.map{ |order| order['id'] } - ids[0].should < ids[1] - ids[1].should < ids[2] + it "sorts orders in descending id order" do + ids = json_response['orders'].map{ |order| order['id'] } + ids[0].should > ids[1] + ids[1].should > ids[2] end it "formats completed_at to 'yyyy-mm-dd hh:mm'" do - json_response.map{ |order| order['completed_at'] }.all?{ |a| a.match("^\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}$") }.should == true + pp json_response + json_response['orders'].map{ |order| order['completed_at'] }.all?{ |a| a == order1.completed_at.strftime('%B %d, %Y') }.should == true end it "returns distributor object with id key" do - json_response.map{ |order| order['distributor'] }.all?{ |d| d.has_key?('id') }.should == true + json_response['orders'].map{ |order| order['distributor'] }.all?{ |d| d.has_key?('id') }.should == true end it "retrieves the order number" do - json_response.map{ |order| order['number'] }.all?{ |number| number.match("^R\\d{5,10}$") }.should == true + json_response['orders'].map{ |order| order['number'] }.all?{ |number| number.match("^R\\d{5,10}$") }.should == true end end @@ -120,7 +121,7 @@ describe Spree::Admin::OrdersController, type: :controller do end it "retrieves a list of orders" do - keys = json_response.first.keys.map{ |key| key.to_sym } + keys = json_response['orders'].first.keys.map{ |key| key.to_sym } order_attributes.all?{ |attr| keys.include? attr }.should == true end end @@ -132,7 +133,7 @@ describe Spree::Admin::OrdersController, type: :controller do end it "retrieves a list of orders" do - keys = json_response.first.keys.map{ |key| key.to_sym } + keys = json_response['orders'].first.keys.map{ |key| key.to_sym } order_attributes.all?{ |attr| keys.include? attr }.should == true end end diff --git a/spec/features/admin/adjustments_spec.rb b/spec/features/admin/adjustments_spec.rb index a051d28b2b..992ee7c47c 100644 --- a/spec/features/admin/adjustments_spec.rb +++ b/spec/features/admin/adjustments_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" feature %q{ As an administrator I want to manage adjustments on orders -} do +}, js: true do include AuthenticationWorkflow include WebHelper @@ -30,7 +30,7 @@ feature %q{ click_link 'New Adjustment' fill_in 'adjustment_amount', with: 110 fill_in 'adjustment_label', with: 'Late fee' - select 'GST', from: 'tax_rate_id' + select2_select 'GST', from: 'tax_rate_id' click_button 'Continue' # Then I should see the adjustment, with the correct tax @@ -51,11 +51,11 @@ feature %q{ page.find('tr', text: 'Shipping').find('a.icon-edit').click # Then I should see the uneditable included tax and our tax rate as the default - page.should have_field :adjustment_included_tax, with: '10.00', disabled: true - page.should have_select :tax_rate_id, selected: 'GST' + expect(page).to have_field :adjustment_included_tax, with: '10.00', disabled: true + expect(page).to have_select2 :tax_rate_id, selected: 'GST' # When I edit the adjustment, removing the tax - select 'Remove tax', from: :tax_rate_id + select2_select 'Remove tax', from: :tax_rate_id click_button 'Continue' # Then the adjustment tax should be cleared @@ -75,11 +75,11 @@ feature %q{ page.find('tr', text: 'Shipping').find('a.icon-edit').click # Then I should see the uneditable included tax and 'Remove tax' as the default tax rate - page.should have_field :adjustment_included_tax, with: '0.00', disabled: true - page.should have_select :tax_rate_id, selected: [] + expect(page).to have_field :adjustment_included_tax, with: '0.00', disabled: true + expect(page).to have_select2 :tax_rate_id, selected: [] # When I edit the adjustment, setting a tax rate - select 'GST', from: :tax_rate_id + select2_select 'GST', from: :tax_rate_id click_button 'Continue' # Then the adjustment tax should be recalculated diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index 602fb99f8c..0c436014b2 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -54,8 +54,8 @@ feature %q{ it "displays a column for order date" do expect(page).to have_selector "th.date", text: "ORDER DATE", :visible => true - expect(page).to have_selector "td.date", text: o1.completed_at.strftime("%F %T"), :visible => true - expect(page).to have_selector "td.date", text: o2.completed_at.strftime("%F %T"), :visible => true + expect(page).to have_selector "td.date", text: o1.completed_at.strftime('%B %d, %Y'), :visible => true + expect(page).to have_selector "td.date", text: o2.completed_at.strftime('%B %d, %Y'), :visible => true end it "displays a column for producer" do diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index 0b9a6cb602..99fa8ebb23 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -109,7 +109,7 @@ feature %q{ quick_login_as_admin visit '/admin/orders' uncheck 'Only show complete orders' - click_button 'Filter Results' + page.find('a.icon-search').click click_edit diff --git a/spec/javascripts/unit/admin/line_items/controllers/line_items_controller_spec.js.coffee b/spec/javascripts/unit/admin/line_items/controllers/line_items_controller_spec.js.coffee index 7e63ba7284..1b23dea59d 100644 --- a/spec/javascripts/unit/admin/line_items/controllers/line_items_controller_spec.js.coffee +++ b/spec/javascripts/unit/admin/line_items/controllers/line_items_controller_spec.js.coffee @@ -37,7 +37,7 @@ describe "LineItemsCtrl", -> order = { id: 9, order_cycle: { id: 4 }, distributor: { id: 5 }, number: "R123456" } lineItem = { id: 7, quantity: 3, order: { id: 9 }, supplier: { id: 1 } } - httpBackend.expectGET("/admin/orders.json?q%5Bcompleted_at_gteq%5D=SomeDate&q%5Bcompleted_at_lt%5D=SomeDate&q%5Bcompleted_at_not_null%5D=true&q%5Bstate_not_eq%5D=canceled").respond [order] + httpBackend.expectGET("/admin/orders.json?q%5Bcompleted_at_gteq%5D=SomeDate&q%5Bcompleted_at_lt%5D=SomeDate&q%5Bcompleted_at_not_null%5D=true&q%5Bstate_not_eq%5D=canceled").respond {orders: [order], pagination: {page: 1, pages: 1, results: 1}} httpBackend.expectGET("/admin/bulk_line_items.json?q%5Border%5D%5Bcompleted_at_gteq%5D=SomeDate&q%5Border%5D%5Bcompleted_at_lt%5D=SomeDate&q%5Border%5D%5Bcompleted_at_not_null%5D=true&q%5Border%5D%5Bstate_not_eq%5D=canceled").respond [lineItem] httpBackend.expectGET("/admin/enterprises/visible.json?ams_prefix=basic&q%5Bsells_in%5D%5B%5D=own&q%5Bsells_in%5D%5B%5D=any").respond [distributor] httpBackend.expectGET("/admin/order_cycles.json?ams_prefix=basic&as=distributor&q%5Borders_close_at_gt%5D=SomeDate").respond [orderCycle] @@ -68,7 +68,7 @@ describe "LineItemsCtrl", -> describe "initialisation", -> it "gets suppliers", -> - expect(scope.suppliers).toDeepEqual [supplier ] + expect(scope.suppliers).toDeepEqual [ supplier ] it "gets distributors", -> expect(scope.distributors).toDeepEqual [ distributor ] diff --git a/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee b/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee index 4ceeea277b..0324d79182 100644 --- a/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee +++ b/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee @@ -7,13 +7,25 @@ describe "ordersCtrl", -> {id: 10, name: 'Ten', status: 'open', distributors: [{id: 1, name: 'One'}]} {id: 20, name: 'Twenty', status: 'closed', distributors: [{id: 2, name: 'Two', status: 'closed'}]} ] + SortOptions = { + predicate: "", + reverse: false + } beforeEach -> scope = {} + shops = [] + orderCycles = [ + {id: 10, name: 'Ten', status: 'open', distributors: [{id: 1, name: 'One'}]} + {id: 20, name: 'Twenty', status: 'closed', distributors: [{id: 2, name: 'Two', status: 'closed'}]} + ] - module('admin.orders') - inject ($controller) -> - ctrl = $controller 'ordersCtrl', {$scope: scope, $attrs: attrs, shops: shops, orderCycles: orderCycles} + module 'admin.orders', ($provide)-> + $provide.provider('shops', shops) + $provide.provider('orderCycles', orderCycles) + inject (_$injector_, $controller) -> + $injector = _$injector_ + ctrl = $controller 'ordersCtrl', {$scope: scope, $attrs: attrs, $injector: $injector, SortOptions: SortOptions} it "initialises name_and_status", -> expect(scope.orderCycles[0].name_and_status).toEqual "Ten (open)" diff --git a/spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee b/spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee index 3d41ded40a..bb22689d73 100644 --- a/spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee +++ b/spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee @@ -18,17 +18,17 @@ describe "Orders service", -> result = response = null beforeEach -> - response = [{ id: 5, name: 'Order 1'}] + response = { orders: [{ id: 5, name: 'Order 1'}], pagination: {page: 1, pages: 1, results: 1} } $httpBackend.expectGET('/admin/orders.json').respond 200, response result = Orders.index() $httpBackend.flush() it "stores returned data in @byID, with ids as keys", -> # OrderResource returns instances of Resource rather than raw objects - expect(Orders.byID).toDeepEqual { 5: response[0] } + expect(Orders.byID).toDeepEqual { 5: response.orders[0] } it "stores returned data in @pristineByID, with ids as keys", -> - expect(Orders.pristineByID).toDeepEqual { 5: response[0] } + expect(Orders.pristineByID).toDeepEqual { 5: response.orders[0] } it "returns an array of orders", -> expect(result).toDeepEqual response From 6768055b4ddadff70f63f0c67ca4b010c61360f1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 18 Sep 2018 01:09:31 +0100 Subject: [PATCH 08/21] Split orders into 2 angular controllers --- .../controllers/order_controller.js.coffee | 26 +++++++++++++++++ .../controllers/orders_controller.js.coffee | 28 +------------------ app/views/spree/admin/orders/edit.html.haml | 2 +- app/views/spree/admin/orders/new.html.haml | 2 +- .../admin/orders/set_distribution.html.haml | 2 +- ...coffee => order_controller_spec.js.coffee} | 20 +++---------- 6 files changed, 34 insertions(+), 46 deletions(-) create mode 100644 app/assets/javascripts/admin/orders/controllers/order_controller.js.coffee rename spec/javascripts/unit/admin/orders/controllers/{orders_controller_spec.js.coffee => order_controller_spec.js.coffee} (66%) diff --git a/app/assets/javascripts/admin/orders/controllers/order_controller.js.coffee b/app/assets/javascripts/admin/orders/controllers/order_controller.js.coffee new file mode 100644 index 0000000000..822e81eb49 --- /dev/null +++ b/app/assets/javascripts/admin/orders/controllers/order_controller.js.coffee @@ -0,0 +1,26 @@ +angular.module("admin.orders").controller "orderCtrl", ($scope, shops, orderCycles, $compile, $attrs, Orders) -> + $scope.$compile = $compile + $scope.shops = shops + $scope.orderCycles = orderCycles + + $scope.distributor_id = parseInt($attrs.ofnDistributorId) + $scope.order_cycle_id = parseInt($attrs.ofnOrderCycleId) + + $scope.validOrderCycle = (oc) -> + $scope.orderCycleHasDistributor oc, parseInt($scope.distributor_id) + + $scope.distributorHasOrderCycles = (distributor) -> + (oc for oc in $scope.orderCycles when @orderCycleHasDistributor(oc, distributor.id)).length > 0 + + $scope.orderCycleHasDistributor = (oc, distributor_id) -> + distributor_ids = (d.id for d in oc.distributors) + distributor_ids.indexOf(distributor_id) != -1 + + $scope.distributionChosen = -> + $scope.distributor_id && $scope.order_cycle_id + + for oc in $scope.orderCycles + oc.name_and_status = "#{oc.name} (#{oc.status})" + + for shop in $scope.shops + shop.disabled = !$scope.distributorHasOrderCycles(shop) diff --git a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee index 1ce3da7595..96bfd63ab2 100644 --- a/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee +++ b/app/assets/javascripts/admin/orders/controllers/orders_controller.js.coffee @@ -1,11 +1,4 @@ -angular.module("admin.orders").controller "ordersCtrl", ($scope, $injector, RequestMonitor, $compile, $attrs, Orders, SortOptions) -> - $scope.$compile = $compile - $scope.shops = shops - $scope.orderCycles = orderCycles - - $scope.distributor_id = parseInt($attrs.ofnDistributorId) - $scope.order_cycle_id = parseInt($attrs.ofnOrderCycleId) - +angular.module("admin.orders").controller "ordersCtrl", ($scope, RequestMonitor, Orders, SortOptions) -> $scope.RequestMonitor = RequestMonitor $scope.pagination = Orders.pagination $scope.orders = Orders.all @@ -43,25 +36,6 @@ angular.module("admin.orders").controller "ordersCtrl", ($scope, $injector, Requ $scope.fetchResults() , true - $scope.validOrderCycle = (oc) -> - $scope.orderCycleHasDistributor oc, parseInt($scope.distributor_id) - - $scope.distributorHasOrderCycles = (distributor) -> - (oc for oc in orderCycles when @orderCycleHasDistributor(oc, distributor.id)).length > 0 - - $scope.orderCycleHasDistributor = (oc, distributor_id) -> - distributor_ids = (d.id for d in oc.distributors) - distributor_ids.indexOf(distributor_id) != -1 - - $scope.distributionChosen = -> - $scope.distributor_id && $scope.order_cycle_id - $scope.changePage = (newPage) -> $scope.page = newPage $scope.fetchResults(newPage) - - for oc in $scope.orderCycles - oc.name_and_status = "#{oc.name} (#{oc.status})" - - for shop in $scope.shops - shop.disabled = !$scope.distributorHasOrderCycles(shop) diff --git a/app/views/spree/admin/orders/edit.html.haml b/app/views/spree/admin/orders/edit.html.haml index f53cca42ef..f897065224 100644 --- a/app/views/spree/admin/orders/edit.html.haml +++ b/app/views/spree/admin/orders/edit.html.haml @@ -13,7 +13,7 @@ %div{"data-hook" => "admin_order_edit_header"} = render 'spree/shared/error_messages', target: @order -%div{"ng-app" => "admin.orders", "ng-controller" => "ordersCtrl", "ofn-distributor-id" => @order.distributor_id, "ofn-order-cycle-id" => @order.order_cycle_id} +%div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl", "ofn-distributor-id" => @order.distributor_id, "ofn-order-cycle-id" => @order.order_cycle_id} = render 'add_product' %div{"data-hook" => "admin_order_edit_form"} diff --git a/app/views/spree/admin/orders/new.html.haml b/app/views/spree/admin/orders/new.html.haml index 24190f3190..b653439230 100644 --- a/app/views/spree/admin/orders/new.html.haml +++ b/app/views/spree/admin/orders/new.html.haml @@ -14,7 +14,7 @@ %div{"data-hook" => "admin_order_new_header"} = render 'spree/shared/error_messages', :target => @order -%div{"ng-app" => "admin.orders", "ng-controller" => "ordersCtrl"} +%div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl"} %div{"ng-show" => "distributionChosen()"} = render 'add_product' diff --git a/app/views/spree/admin/orders/set_distribution.html.haml b/app/views/spree/admin/orders/set_distribution.html.haml index 51c7b97e8d..89d036b93a 100644 --- a/app/views/spree/admin/orders/set_distribution.html.haml +++ b/app/views/spree/admin/orders/set_distribution.html.haml @@ -14,7 +14,7 @@ %div{"data-hook" => "admin_order_new_header"} = render 'spree/shared/error_messages', :target => @order -%div{"ng-app" => "admin.orders", "ng-controller" => "ordersCtrl"} +%div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl"} = form_for @order, url: admin_order_url(@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: diff --git a/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee b/spec/javascripts/unit/admin/orders/controllers/order_controller_spec.js.coffee similarity index 66% rename from spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee rename to spec/javascripts/unit/admin/orders/controllers/order_controller_spec.js.coffee index 0324d79182..297eac32f6 100644 --- a/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee +++ b/spec/javascripts/unit/admin/orders/controllers/order_controller_spec.js.coffee @@ -1,4 +1,4 @@ -describe "ordersCtrl", -> +describe "orderCtrl", -> ctrl = null scope = {} attrs = {} @@ -7,25 +7,13 @@ describe "ordersCtrl", -> {id: 10, name: 'Ten', status: 'open', distributors: [{id: 1, name: 'One'}]} {id: 20, name: 'Twenty', status: 'closed', distributors: [{id: 2, name: 'Two', status: 'closed'}]} ] - SortOptions = { - predicate: "", - reverse: false - } beforeEach -> scope = {} - shops = [] - orderCycles = [ - {id: 10, name: 'Ten', status: 'open', distributors: [{id: 1, name: 'One'}]} - {id: 20, name: 'Twenty', status: 'closed', distributors: [{id: 2, name: 'Two', status: 'closed'}]} - ] - module 'admin.orders', ($provide)-> - $provide.provider('shops', shops) - $provide.provider('orderCycles', orderCycles) - inject (_$injector_, $controller) -> - $injector = _$injector_ - ctrl = $controller 'ordersCtrl', {$scope: scope, $attrs: attrs, $injector: $injector, SortOptions: SortOptions} + module 'admin.orders' + inject ($controller) -> + ctrl = $controller 'orderCtrl', {$scope: scope, $attrs: attrs, shops: shops, orderCycles: orderCycles} it "initialises name_and_status", -> expect(scope.orderCycles[0].name_and_status).toEqual "Ten (open)" From 64620c279749c3c5c35e1355ad44091833b7cb92 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 18 Sep 2018 01:16:37 +0100 Subject: [PATCH 09/21] Tidy up response formats for easier testing --- .../javascripts/admin/resources/services/orders.js.coffee | 2 +- .../unit/admin/orders/services/orders_spec.js.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/resources/services/orders.js.coffee b/app/assets/javascripts/admin/resources/services/orders.js.coffee index 332dc44796..5eef10eecb 100644 --- a/app/assets/javascripts/admin/resources/services/orders.js.coffee +++ b/app/assets/javascripts/admin/resources/services/orders.js.coffee @@ -10,7 +10,7 @@ angular.module("admin.resources").factory 'Orders', ($q, OrderResource, RequestM @load(data) (callback || angular.noop)(data) RequestMonitor.load(request.$promise) - request + @all load: (data) -> angular.extend(@pagination, data.pagination) diff --git a/spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee b/spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee index bb22689d73..68bc3beb1f 100644 --- a/spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee +++ b/spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee @@ -31,7 +31,7 @@ describe "Orders service", -> expect(Orders.pristineByID).toDeepEqual { 5: response.orders[0] } it "returns an array of orders", -> - expect(result).toDeepEqual response + expect(result).toDeepEqual response.orders describe "#save", -> From 9ce32e3c141d9f7101c8f5b7fbe9f8bc5bf54e46 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 18 Sep 2018 09:51:11 +0100 Subject: [PATCH 10/21] Add new ordersCtrl spec --- .../orders_controller_spec.js.coffee | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee diff --git a/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee b/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee new file mode 100644 index 0000000000..4aed599140 --- /dev/null +++ b/spec/javascripts/unit/admin/orders/controllers/orders_controller_spec.js.coffee @@ -0,0 +1,37 @@ +describe "ordersCtrl", -> + ctrl = null + Orders = null + $scope = null + orders = [ + { id: 8, order_cycle: { id: 4 }, distributor: { id: 5 }, number: "R123456" } + { id: 9, order_cycle: { id: 5 }, distributor: { id: 7 }, number: "R213776" } + ] + form = { + q: { + created_at_lt: '' + created_at_gt: '' + completed_at_not_null: true + } + } + + beforeEach -> + module 'admin.orders' + inject ($controller, $rootScope, RequestMonitor, SortOptions) -> + $scope = $rootScope.$new() + Orders = + index: jasmine.createSpy('index').and.returnValue(orders) + all: orders + ctrl = $controller 'ordersCtrl', { $scope: $scope, RequestMonitor: RequestMonitor, SortOptions: SortOptions, Orders: Orders } + $scope.q = form.q + + describe "initialising the controller", -> + it "fetches orders", -> + $scope.initialise() + expect(Orders.index).toHaveBeenCalled() + expect($scope.orders).toEqual orders + + describe "using pagination", -> + it "changes the page", -> + $scope.changePage(2) + expect($scope.page).toEqual 2 + expect(Orders.index).toHaveBeenCalled() From ba254802f814ce378ddd5d42f2b4153e1c2d450c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 19 Sep 2018 09:27:44 +0100 Subject: [PATCH 11/21] Move angular_pagination to /views/admin/shared/ --- .../admin/orders => admin/shared}/_angular_pagination.html.haml | 0 app/views/spree/admin/orders/index.html.haml | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename app/views/{spree/admin/orders => admin/shared}/_angular_pagination.html.haml (100%) diff --git a/app/views/spree/admin/orders/_angular_pagination.html.haml b/app/views/admin/shared/_angular_pagination.html.haml similarity index 100% rename from app/views/spree/admin/orders/_angular_pagination.html.haml rename to app/views/admin/shared/_angular_pagination.html.haml diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 1191846869..750a7d7958 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -110,7 +110,7 @@ %span= t(:loading) %div{'ng-show' => "!RequestMonitor.loading && orders.length > 0" } - = render partial: 'angular_pagination' + = render partial: 'admin/shared/angular_pagination' .no-objects-found{'ng-show' => "!RequestMonitor.loading && orders.length == 0"} = t(:no_orders_found) From 3cbb576b4f8e0ff150b9e30f076ab39f3d2ca9d1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 19 Sep 2018 10:21:01 +0100 Subject: [PATCH 12/21] Move payment object logic out of order serializer and delete code --- app/serializers/api/admin/order_serializer.rb | 30 +++---------------- app/services/pending_payments.rb | 15 ++++++++++ app/views/spree/admin/orders/index.html.haml | 2 +- 3 files changed, 20 insertions(+), 27 deletions(-) create mode 100644 app/services/pending_payments.rb diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb index 065025d22f..7b7c5018c1 100644 --- a/app/serializers/api/admin/order_serializer.rb +++ b/app/serializers/api/admin/order_serializer.rb @@ -2,7 +2,7 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer attributes :id, :number, :full_name, :email, :phone, :completed_at, :display_total attributes :show_path, :edit_path, :state, :payment_state, :shipment_state attributes :payments_path, :shipments_path, :ship_path, :ready_to_ship, :created_at - attributes :distributor_name, :special_instructions, :pending_payments, :capture_path + attributes :distributor_name, :special_instructions, :capture_path has_one :distributor, serializer: Api::Admin::IdSerializer has_one :order_cycle, serializer: Api::Admin::IdSerializer @@ -40,9 +40,9 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer end def capture_path - return '' unless ready_for_payment? - return unless payment_to_capture - Spree::Core::Engine.routes_url_helpers.fire_admin_order_payment_path(object, payment_to_capture.id, e: 'capture') + payment_due = PendingPayments.new(object) + return '' unless object.payment_required? && payment_due.payment_object + Spree::Core::Engine.routes_url_helpers.fire_admin_order_payment_path(object, payment_due.payment_object.id, e: 'capture') end def ready_to_ship @@ -68,26 +68,4 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer def completed_at object.completed_at.blank? ? "" : I18n.l(object.completed_at, format: '%B %d, %Y') end - - def pending_payments - return if object.payments.blank? - payment = object.payments.select{ |p| p if p.state == 'checkout' }.first - return unless can_be_captured? payment - - payment.id - end - - private - - def ready_for_payment? - object.payment_required? && object.payments.present? - end - - def payment_to_capture - object.payments.select{ |p| p if p.state == 'checkout' }.first - end - - def can_be_captured?(payment) - payment && payment.actions.include?('capture') - end end diff --git a/app/services/pending_payments.rb b/app/services/pending_payments.rb new file mode 100644 index 0000000000..729af49e47 --- /dev/null +++ b/app/services/pending_payments.rb @@ -0,0 +1,15 @@ +# Returns the capturable payment object for an order with balance due + +class PendingPayments + def initialize(order) + @order = order + end + + def payment_object + @order.payments.select{ |p| p if p.state == 'checkout' }.first + end + + def can_be_captured? + payment_object && payment_object.actions.include?('capture') + end +end diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 750a7d7958..ed94800e02 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -98,7 +98,7 @@ %a.icon_link.with-tip.icon-edit.no-text{'ng-href' => '{{::order.edit_path}}', 'data-action' => 'edit', 'ofn-with-tip' => t(:edit)} %div{'ng-if' => 'order.ready_to_ship'} %a.icon-road.icon_link.with-tip.no-text{'ng-href' => '{{::order.ship_path}}', 'data-action' => 'ship', 'data-confirm' => t(:are_you_sure), 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:ship)} - %div{'ng-if' => 'order.pending_payments'} + %div{'ng-if' => 'order.capture_path'} %a.icon-capture.icon_link.no-text{'ng-href' => '{{::order.capture_path}}', 'data-action' => 'capture', 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:capture)} .orders-loading{'ng-show' => 'RequestMonitor.loading'} From 897e43fe0b082080319daf66fc141040fe266391 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 20 Sep 2018 11:33:22 +0100 Subject: [PATCH 13/21] Remove Spree's Deface data-hooks from new view --- app/views/spree/admin/orders/index.html.haml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index ed94800e02..526c4c5ee4 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -16,11 +16,11 @@ - content_for :table_filter do = render partial: 'filters' -%table#listing_orders.index.responsive{"data-hook" => "", width: "100%", 'ng-init' => 'initialise()', 'ng-show' => "!RequestMonitor.loading && orders.length > 0" } +%table#listing_orders.index.responsive{width: "100%", 'ng-init' => 'initialise()', 'ng-show' => "!RequestMonitor.loading && orders.length > 0" } %colgroup %col{style: "width: 10%"} %thead - %tr{"data-hook" => "admin_orders_index_headers"} + %tr %th = t(:products_distributor) - if @show_only_completed @@ -65,9 +65,9 @@ = t(:total, scope: 'activerecord.attributes.spree/order') %span{'ng-show' => "sorting == 'total asc'"}= "▲".html_safe %span{'ng-show' => "sorting == 'total desc'"}= "▼".html_safe - %th.actions{"data-hook" => "admin_orders_index_header_actions"} + %th.actions %tbody - %tr{ng: {repeat: 'order in orders track by $index', class: {even: "'even'", odd: "'odd'"}}, 'ng-class' => "'state-{{order.state}}'", "data-hook" => "admin_orders_index_rows"} + %tr{ng: {repeat: 'order in orders track by $index', class: {even: "'even'", odd: "'odd'"}}, 'ng-class' => "'state-{{order.state}}'"} %td.align-center {{::order.distributor_name}} %td.align-center @@ -94,7 +94,7 @@ = mail_to "{{::order.email}}" %td.align-center %span{'ng-bind-html' => '::order.display_total'} - %td.actions{"data-hook" => "admin_orders_index_row_actions"} + %td.actions %a.icon_link.with-tip.icon-edit.no-text{'ng-href' => '{{::order.edit_path}}', 'data-action' => 'edit', 'ofn-with-tip' => t(:edit)} %div{'ng-if' => 'order.ready_to_ship'} %a.icon-road.icon_link.with-tip.no-text{'ng-href' => '{{::order.ship_path}}', 'data-action' => 'ship', 'data-confirm' => t(:are_you_sure), 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:ship)} From 9f57b43a13db3927ae5af38701583932d763d151 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 23 Sep 2018 22:14:12 +0100 Subject: [PATCH 14/21] Move sortble header elements to a partial --- .../admin/orders/_sortable_header.html.haml | 4 ++ app/views/spree/admin/orders/index.html.haml | 38 ++----------------- 2 files changed, 8 insertions(+), 34 deletions(-) create mode 100644 app/views/spree/admin/orders/_sortable_header.html.haml diff --git a/app/views/spree/admin/orders/_sortable_header.html.haml b/app/views/spree/admin/orders/_sortable_header.html.haml new file mode 100644 index 0000000000..97d24165c0 --- /dev/null +++ b/app/views/spree/admin/orders/_sortable_header.html.haml @@ -0,0 +1,4 @@ +%a{'ng-click' => "sortOptions.toggle('#{column_name}')"} + = t(column_name.to_s, scope: 'activerecord.attributes.spree/order') + %span{'ng-show' => "sorting == '#{column_name} asc'"}= "▲".html_safe + %span{'ng-show' => "sorting == '#{column_name} desc'"}= "▼".html_safe \ No newline at end of file diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 526c4c5ee4..dec3c2c576 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -31,40 +31,10 @@ %span{'ng-show' => "sorting == 'completed_at desc' || sorting === undefined"}= "▼".html_safe - else %th - %a{'ng-click' => "sortOptions.toggle('created_at')"} - = t(:created_at, scope: 'activerecord.attributes.spree/order') - %span{'ng-show' => "sorting == 'created_at asc'"}= "▲".html_safe - %span{'ng-show' => "sorting == 'created_at desc'"}= "▼".html_safe - %th - %a{'ng-click' => "sortOptions.toggle('number')"} - = t(:number, scope: 'activerecord.attributes.spree/order') - %span{'ng-show' => "sorting == 'number asc'"}= "▲".html_safe - %span{'ng-show' => "sorting == 'number desc'"}= "▼".html_safe - %th - %a{'ng-click' => "sortOptions.toggle('state')"} - = t(:state, scope: 'activerecord.attributes.spree/order') - %span{'ng-show' => "sorting == 'state asc'"}= "▲".html_safe - %span{'ng-show' => "sorting == 'state desc'"}= "▼".html_safe - %th - %a{'ng-click' => "sortOptions.toggle('payment_state')"} - = t(:payment_state, scope: 'activerecord.attributes.spree/order') - %span{'ng-show' => "sorting == 'payment_state asc'"}= "▲".html_safe - %span{'ng-show' => "sorting == 'payment_state desc'"}= "▼".html_safe - %th - %a{'ng-click' => "sortOptions.toggle('shipment_state')"} - = t(:shipment_state, scope: 'activerecord.attributes.spree/order') - %span{'ng-show' => "sorting == 'shipment_state asc'"}= "▲".html_safe - %span{'ng-show' => "sorting == 'shipment_state desc'"}= "▼".html_safe - %th - %a{'ng-click' => "sortOptions.toggle('email')"} - = t(:email, scope: 'activerecord.attributes.spree/order') - %span{'ng-show' => "sorting == 'email asc'"}= "▲".html_safe - %span{'ng-show' => "sorting == 'email desc'"}= "▼".html_safe - %th - %a{'ng-click' => "sortOptions.toggle('total')"} - = t(:total, scope: 'activerecord.attributes.spree/order') - %span{'ng-show' => "sorting == 'total asc'"}= "▲".html_safe - %span{'ng-show' => "sorting == 'total desc'"}= "▼".html_safe + = render partial: 'sortable_header', locals: {column_name: 'created_at'} + - ['number', 'state', 'payment_state', 'shipment_state', 'email', 'total'].each do |column_name| + %th + = render partial: 'sortable_header', locals: {column_name: column_name} %th.actions %tbody %tr{ng: {repeat: 'order in orders track by $index', class: {even: "'even'", odd: "'odd'"}}, 'ng-class' => "'state-{{order.state}}'"} From 6f2760cf928b2391201bdd09f899cb17c607aaeb Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 24 Sep 2018 15:54:05 +0100 Subject: [PATCH 15/21] Move translations into their namespace and use '.key' format --- app/views/spree/admin/orders/index.html.haml | 16 +++++++------- config/locales/en.yml | 23 +++++++++++--------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index dec3c2c576..5aa6f161b8 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -1,9 +1,9 @@ - content_for :page_title do - = t(:listing_orders) + = t('.listing_orders') - content_for :page_actions do %li - = button_link_to t(:new_order), new_admin_order_url, :icon => 'icon-plus', :id => 'admin_new_order' + = button_link_to t('.new_order'), new_admin_order_url, icon: 'icon-plus', id: 'admin_new_order' = render partial: 'spree/admin/shared/order_sub_menu' @@ -48,7 +48,7 @@ %div{'ng-if' => 'order.special_instructions'} %br %span.icon-warning-sign{'ofn-with-tip' => "{{::order.special_instructions}}"} - = t(:note) + = t('.note') %td.align-center %span.state{'ng-class' => 'order.state'} {{'order_state.' + order.state | t}} @@ -65,11 +65,11 @@ %td.align-center %span{'ng-bind-html' => '::order.display_total'} %td.actions - %a.icon_link.with-tip.icon-edit.no-text{'ng-href' => '{{::order.edit_path}}', 'data-action' => 'edit', 'ofn-with-tip' => t(:edit)} + %a.icon_link.with-tip.icon-edit.no-text{'ng-href' => '{{::order.edit_path}}', 'data-action' => 'edit', 'ofn-with-tip' => t('.edit')} %div{'ng-if' => 'order.ready_to_ship'} - %a.icon-road.icon_link.with-tip.no-text{'ng-href' => '{{::order.ship_path}}', 'data-action' => 'ship', 'data-confirm' => t(:are_you_sure), 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:ship)} + %a.icon-road.icon_link.with-tip.no-text{'ng-href' => '{{::order.ship_path}}', 'data-action' => 'ship', 'data-confirm' => t(:are_you_sure), 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t('.ship')} %div{'ng-if' => 'order.capture_path'} - %a.icon-capture.icon_link.no-text{'ng-href' => '{{::order.capture_path}}', 'data-action' => 'capture', 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t(:capture)} + %a.icon-capture.icon_link.no-text{'ng-href' => '{{::order.capture_path}}', 'data-action' => 'capture', 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t('.capture')} .orders-loading{'ng-show' => 'RequestMonitor.loading'} .row @@ -77,10 +77,10 @@ %img.spinner{ src: "/assets/spinning-circles.svg" } .row .small-12.columns.fullwidth.text-center - %span= t(:loading) + %span= t('.loading') %div{'ng-show' => "!RequestMonitor.loading && orders.length > 0" } = render partial: 'admin/shared/angular_pagination' .no-objects-found{'ng-show' => "!RequestMonitor.loading && orders.length == 0"} - = t(:no_orders_found) + = t('.no_orders_found') diff --git a/config/locales/en.yml b/config/locales/en.yml index e0a9863c03..da68db37b9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -613,16 +613,6 @@ en: controls: back_to_my_inventory: Back to my inventory orders: - index: - capture: "Capture" - ship: "Ship" - edit: "Edit" - note: "Note" - first: "First" - last: "Last" - previous: "Previous" - next: "Next" - loading: "Loading" invoice_email_sent: 'Invoice email has been sent' order_email_resent: 'Order email has been resent' bulk_management: @@ -2655,6 +2645,19 @@ See the %{link} to find out more about %{sitename}'s features and to start using index: inherits_properties_checkbox_hint: "Inherit properties from %{supplier}? (unless overridden above)" orders: + index: + listing_orders: "Listing Orders" + new_order: "New Order" + capture: "Capture" + ship: "Ship" + edit: "Edit" + note: "Note" + first: "First" + last: "Last" + previous: "Previous" + next: "Next" + loading: "Loading" + no_orders_found: "No Orders Found" invoice: issued_on: Issued on tax_invoice: TAX INVOICE From 1d9243af190cfff109db22d468cd0be59f017b33 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 24 Sep 2018 16:58:43 +0100 Subject: [PATCH 16/21] DRY and clarify serializer and service --- app/serializers/api/admin/order_serializer.rb | 18 ++++++++++++------ app/services/pending_payments.rb | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb index 7b7c5018c1..2a579b2910 100644 --- a/app/serializers/api/admin/order_serializer.rb +++ b/app/serializers/api/admin/order_serializer.rb @@ -17,32 +17,32 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer def show_path return '' unless object.id - Spree::Core::Engine.routes_url_helpers.admin_order_path(object) + spree_routes_helper.admin_order_path(object) end def edit_path return '' unless object.id - Spree::Core::Engine.routes_url_helpers.edit_admin_order_path(object) + spree_routes_helper.edit_admin_order_path(object) end def payments_path return '' unless object.payment_state - Spree::Core::Engine.routes_url_helpers.admin_order_payments_path(object) + spree_routes_helper.admin_order_payments_path(object) end def shipments_path return '' unless object.shipment_state - Spree::Core::Engine.routes_url_helpers.admin_order_shipments_path(object) + spree_routes_helper.admin_order_shipments_path(object) end def ship_path - Spree::Core::Engine.routes_url_helpers.fire_admin_order_path(object, e: 'ship') + spree_routes_helper.fire_admin_order_path(object, e: 'ship') end def capture_path payment_due = PendingPayments.new(object) return '' unless object.payment_required? && payment_due.payment_object - Spree::Core::Engine.routes_url_helpers.fire_admin_order_payment_path(object, payment_due.payment_object.id, e: 'capture') + spree_routes_helper.fire_admin_order_payment_path(object, payment_due.payment_object.id, e: 'capture') end def ready_to_ship @@ -68,4 +68,10 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer def completed_at object.completed_at.blank? ? "" : I18n.l(object.completed_at, format: '%B %d, %Y') end + + private + + def spree_routes_helper + Spree::Core::Engine.routes_url_helpers + end end diff --git a/app/services/pending_payments.rb b/app/services/pending_payments.rb index 729af49e47..8a0d660c2d 100644 --- a/app/services/pending_payments.rb +++ b/app/services/pending_payments.rb @@ -6,7 +6,7 @@ class PendingPayments end def payment_object - @order.payments.select{ |p| p if p.state == 'checkout' }.first + @order.payments.select{ |payment| payment if payment.state == 'checkout' }.first end def can_be_captured? From e93d46e75acc7da4cfd7eb3013fc9100714eabf9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 25 Sep 2018 09:35:05 +0100 Subject: [PATCH 17/21] Use .find instead of .select().first --- app/services/pending_payments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/pending_payments.rb b/app/services/pending_payments.rb index 8a0d660c2d..9569500dda 100644 --- a/app/services/pending_payments.rb +++ b/app/services/pending_payments.rb @@ -6,7 +6,7 @@ class PendingPayments end def payment_object - @order.payments.select{ |payment| payment if payment.state == 'checkout' }.first + @order.payments.find{ |payment| payment.state == 'checkout' } end def can_be_captured? From 2dcc8ea4bb5ce4a25daf8083be9818ee76dc32e0 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 25 Sep 2018 10:05:32 +0100 Subject: [PATCH 18/21] Add spec for pending payments service --- .../services/pending_payments_service_spec.rb | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 spec/services/pending_payments_service_spec.rb diff --git a/spec/services/pending_payments_service_spec.rb b/spec/services/pending_payments_service_spec.rb new file mode 100644 index 0000000000..eb3af6007c --- /dev/null +++ b/spec/services/pending_payments_service_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe PendingPayments do + let(:order1) { + create(:order_with_totals_and_distribution, + completed_at: 1.day.ago, state: "complete", + payments: [create(:payment, state: 'checkout')]) + } + let(:order2) { + create(:order_with_totals_and_distribution, + completed_at: 1.day.ago, state: "complete", + payments: [create(:payment, state: 'completed')]) + } + + describe "#can_be_captured?" do + it "responds with a boolean; if an order has payments that can be captured or not" do + expect(PendingPayments.new(order1).can_be_captured?).to be_truthy + expect(PendingPayments.new(order2).can_be_captured?).to_not be_truthy + end + end + + describe "#payment_object" do + it "returns a capturable payment object if there is one present" do + expect(PendingPayments.new(order1).payment_object).to be_a Spree::Payment + expect(PendingPayments.new(order2).payment_object).to be_nil + end + end +end From 54b17ac7018449960a6c959ef551d5b7c5b7fbd7 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 27 Sep 2018 13:04:56 +0100 Subject: [PATCH 19/21] Use Spree::Order.pending_payments and remove service --- app/serializers/api/admin/order_serializer.rb | 6 ++-- app/services/pending_payments.rb | 15 ---------- .../services/pending_payments_service_spec.rb | 28 ------------------- 3 files changed, 3 insertions(+), 46 deletions(-) delete mode 100644 app/services/pending_payments.rb delete mode 100644 spec/services/pending_payments_service_spec.rb diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb index 2a579b2910..7ea181da5d 100644 --- a/app/serializers/api/admin/order_serializer.rb +++ b/app/serializers/api/admin/order_serializer.rb @@ -40,9 +40,9 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer end def capture_path - payment_due = PendingPayments.new(object) - return '' unless object.payment_required? && payment_due.payment_object - spree_routes_helper.fire_admin_order_payment_path(object, payment_due.payment_object.id, e: 'capture') + pending_payment = object.pending_payments.first + return '' unless object.payment_required? && pending_payment + spree_routes_helper.fire_admin_order_payment_path(object, pending_payment.id, e: 'capture') end def ready_to_ship diff --git a/app/services/pending_payments.rb b/app/services/pending_payments.rb deleted file mode 100644 index 9569500dda..0000000000 --- a/app/services/pending_payments.rb +++ /dev/null @@ -1,15 +0,0 @@ -# Returns the capturable payment object for an order with balance due - -class PendingPayments - def initialize(order) - @order = order - end - - def payment_object - @order.payments.find{ |payment| payment.state == 'checkout' } - end - - def can_be_captured? - payment_object && payment_object.actions.include?('capture') - end -end diff --git a/spec/services/pending_payments_service_spec.rb b/spec/services/pending_payments_service_spec.rb deleted file mode 100644 index eb3af6007c..0000000000 --- a/spec/services/pending_payments_service_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -require 'spec_helper' - -describe PendingPayments do - let(:order1) { - create(:order_with_totals_and_distribution, - completed_at: 1.day.ago, state: "complete", - payments: [create(:payment, state: 'checkout')]) - } - let(:order2) { - create(:order_with_totals_and_distribution, - completed_at: 1.day.ago, state: "complete", - payments: [create(:payment, state: 'completed')]) - } - - describe "#can_be_captured?" do - it "responds with a boolean; if an order has payments that can be captured or not" do - expect(PendingPayments.new(order1).can_be_captured?).to be_truthy - expect(PendingPayments.new(order2).can_be_captured?).to_not be_truthy - end - end - - describe "#payment_object" do - it "returns a capturable payment object if there is one present" do - expect(PendingPayments.new(order1).payment_object).to be_a Spree::Payment - expect(PendingPayments.new(order2).payment_object).to be_nil - end - end -end From 017e3d14dfcebf78e939d9dcf7368de3f8afd5ab Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 2 Oct 2018 11:10:21 +0100 Subject: [PATCH 20/21] Use variable colour assignment --- app/assets/stylesheets/admin/components/pagination.scss | 2 +- app/assets/stylesheets/admin/variables.css.scss | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/admin/components/pagination.scss b/app/assets/stylesheets/admin/components/pagination.scss index fd3705a6fd..05cbf34ca7 100644 --- a/app/assets/stylesheets/admin/components/pagination.scss +++ b/app/assets/stylesheets/admin/components/pagination.scss @@ -13,7 +13,7 @@ } &.disabled { - background-color: #ccc; + background-color: $disabled_button; cursor: default; } } diff --git a/app/assets/stylesheets/admin/variables.css.scss b/app/assets/stylesheets/admin/variables.css.scss index f8c12e0d4d..162ad93c6c 100644 --- a/app/assets/stylesheets/admin/variables.css.scss +++ b/app/assets/stylesheets/admin/variables.css.scss @@ -8,8 +8,10 @@ $warning-red: #da5354; $warning-orange: #da7f52; $medium-grey: #919191; $pale-blue: #cee1f4; +$light-grey: #ccc; $admin-table-border: $pale-blue; $modal-close-button-color: #de6060; $modal-close-button-hover-color: #bf4545; +$disabled-button: $light-grey; From 55d7d5d1e4876f35798cdfa63c94ea7148e095c4 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 2 Oct 2018 11:10:51 +0100 Subject: [PATCH 21/21] Rename #capture_path to #payment_capture_path --- app/serializers/api/admin/order_serializer.rb | 4 ++-- app/views/spree/admin/orders/index.html.haml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb index 7ea181da5d..e3207b30fd 100644 --- a/app/serializers/api/admin/order_serializer.rb +++ b/app/serializers/api/admin/order_serializer.rb @@ -2,7 +2,7 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer attributes :id, :number, :full_name, :email, :phone, :completed_at, :display_total attributes :show_path, :edit_path, :state, :payment_state, :shipment_state attributes :payments_path, :shipments_path, :ship_path, :ready_to_ship, :created_at - attributes :distributor_name, :special_instructions, :capture_path + attributes :distributor_name, :special_instructions, :payment_capture_path has_one :distributor, serializer: Api::Admin::IdSerializer has_one :order_cycle, serializer: Api::Admin::IdSerializer @@ -39,7 +39,7 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer spree_routes_helper.fire_admin_order_path(object, e: 'ship') end - def capture_path + def payment_capture_path pending_payment = object.pending_payments.first return '' unless object.payment_required? && pending_payment spree_routes_helper.fire_admin_order_payment_path(object, pending_payment.id, e: 'capture') diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 5aa6f161b8..5c8f0e8d14 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -68,8 +68,8 @@ %a.icon_link.with-tip.icon-edit.no-text{'ng-href' => '{{::order.edit_path}}', 'data-action' => 'edit', 'ofn-with-tip' => t('.edit')} %div{'ng-if' => 'order.ready_to_ship'} %a.icon-road.icon_link.with-tip.no-text{'ng-href' => '{{::order.ship_path}}', 'data-action' => 'ship', 'data-confirm' => t(:are_you_sure), 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t('.ship')} - %div{'ng-if' => 'order.capture_path'} - %a.icon-capture.icon_link.no-text{'ng-href' => '{{::order.capture_path}}', 'data-action' => 'capture', 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t('.capture')} + %div{'ng-if' => 'order.payment_capture_path'} + %a.icon-capture.icon_link.no-text{'ng-href' => '{{::order.payment_capture_path}}', 'data-action' => 'capture', 'data-method' => 'put', rel: 'nofollow', 'ofn-with-tip' => t('.capture')} .orders-loading{'ng-show' => 'RequestMonitor.loading'} .row