raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-01-31 21:37:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2912 from Matt-Yorkley/admin_orders_refactor

Browse Source 
Admin orders refactor
This commit is contained in:
Maikel
2018-11-02 16:57:28 +11:00
committed by GitHub
parent cbfe1b535d 443515dae4
commit 3259b3ecf1
11 changed files with 291 additions and 196 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
app/assets/javascripts/admin/resources/resources/order_resource.js.coffee
View File

@@ -1,6 +1,7 @@
angular.module("admin.resources").factory 'OrderResource', ($resource) ->
$resource('/admin/orders/:id/:action.json', {}, {
'index':
url: '/api/orders.json'
method: 'GET'
'update':
method: 'PUT'

23
app/controllers/api/orders_controller.rb Normal file
View File

@@ -0,0 +1,23 @@
module Api
class OrdersController < BaseController
def index
authorize! :admin, Spree::Order
search_results = SearchOrders.new(params, spree_current_user)
render json: {
orders: serialized_orders(search_results.orders),
pagination: search_results.pagination_data
}
end
private
def serialized_orders(orders)
ActiveModel::ArraySerializer.new(
orders,
each_serializer: Api::Admin::OrderSerializer
)
end
end
end

83
app/controllers/spree/admin/orders_controller_decorator.rb
View File

@@ -20,53 +20,11 @@ Spree::Admin::OrdersController.class_eval do
before_filter :require_distributor_abn, only: :invoice
respond_to :html, :json
# Mostly the original Spree method, tweaked to allow us to ransack with completed_at in a sane way
def index
params[:q] ||= {}
params[:q][:completed_at_not_null] ||= '1' if Spree::Config[:show_only_complete_orders_by_default]
@show_only_completed = params[:q][:completed_at_not_null].present?
params[:q][:s] ||= @show_only_completed ? 'completed_at desc' : 'created_at desc'
# As date params are deleted if @show_only_completed, store
# the original date so we can restore them into the params
# after the search
created_at_gt = params[:q][:created_at_gt]
created_at_lt = params[:q][:created_at_lt]
params[:q].delete(:inventory_units_shipment_id_null) if params[:q][:inventory_units_shipment_id_null] == "0"
if !params[:q][:created_at_gt].blank?
params[:q][:created_at_gt] = Time.zone.parse(params[:q][:created_at_gt]).beginning_of_day rescue ""
end
if !params[:q][:created_at_lt].blank?
params[:q][:created_at_lt] = Time.zone.parse(params[:q][:created_at_lt]).end_of_day rescue ""
end
# Changed this to stop completed_at being overriden when present
if @show_only_completed
params[:q][:completed_at_gt] = params[:q].delete(:created_at_gt) unless params[:q][:completed_at_gt]
params[:q][:completed_at_lt] = params[:q].delete(:created_at_lt) unless params[:q][:completed_at_gt]
end
@orders = orders
# Restore dates
params[:q][:created_at_gt] = created_at_gt
params[:q][:created_at_lt] = created_at_lt
respond_with(@orders) do |format|
format.html
format.json do
render json: {
orders: ActiveModel::ArraySerializer.new(@orders, each_serializer: Api::Admin::OrderSerializer),
pagination: pagination_data
}
end
end
# Overriding the action so we only render the page template. An angular request
# within the page then fetches the data it needs from Api::OrdersController
end
# Overwrite to use confirm_email_for_customer instead of confirm_email.
@@ -102,42 +60,6 @@ Spree::Admin::OrdersController.class_eval do
private
def orders
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)
end
search_results
end
def search_results
if using_pagination?
@search.result.page(params[:page]).per(params[:per_page] || Spree::Config[:orders_per_page])
else
@search.result
end
end
def using_pagination?
params[:per_page]
end
def pagination_data
if using_pagination?
{
results: @orders.total_count,
pages: @orders.num_pages,
page: params[:page].to_i,
per_page: params[:per_page].to_i
}
end
end
def require_distributor_abn
unless @order.distributor.abn.present?
flash[:error] = t(:must_have_valid_business_number, enterprise_name: @order.distributor.name)
@@ -165,5 +87,4 @@ Spree::Admin::OrdersController.class_eval do
render 'set_distribution', locals: { order: @order }
end
end
end

41
app/services/search_orders.rb Normal file
View File

@@ -0,0 +1,41 @@
class SearchOrders
attr_reader :orders
def initialize(params, current_user)
@params = params
@current_user = current_user
@orders = fetch_orders
end
def pagination_data
return unless using_pagination?
{
results: @orders.total_count,
pages: @orders.num_pages,
page: params[:page].to_i,
per_page: params[:per_page].to_i
}
end
private
attr_reader :params, :current_user
def fetch_orders
@search = OpenFoodNetwork::Permissions.new(current_user).editable_orders.ransack(params[:q])
return paginated_results if using_pagination?
@search.result
end
def paginated_results
@search.result
.page(params[:page])
.per(params[:per_page])
end
def using_pagination?
params[:per_page]
end
end

22
app/views/spree/admin/orders/_filters.html.haml
View File

@@ -1,38 +1,40 @@
%div{"data-hook" => "admin_orders_index_search"}
= search_form_for [:admin, @search], html: { name: "orders_form", "ng-submit" => "fetchResults()"} do |f|
= form_tag false, {name: "orders_form", "ng-submit" => "fetchResults()"} do
.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)
= text_field_tag "q[created_at_gt]", nil, class: 'datepicker', datepicker: 'q.created_at_gt', 'ng-model' => '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)
= text_field_tag "q[created_at_lt]", nil, class: 'datepicker', datepicker: 'q.created_at_lt', 'ng-model' => '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'
= select_tag("q[state_eq]",
options_for_select(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'
= text_field_tag "q[number_cont]", nil, 'ng-model' => 'q.number_cont'
.field
= label_tag nil, t(:email)
= f.email_field :email_cont, 'ng-model' => 'q.email_cont'
= email_field_tag "q[email_cont", nil, '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'
= text_field_tag "q[bill_address_firstname_start]", nil, 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'
= text_field_tag "q[bill_address_lastname_start]", nil, 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', ''
= check_box_tag "q[completed_at_not_null]", 1, true, {'ng-model' => 'q.completed_at_not_null'}
= 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'
= check_box_tag "q[inventory_units_shipment_id_null]", 1, false, {'ng-model' => 'q.inventory_units_shipment_id_null'}
= t(:show_only_unfulfilled_orders)
.field-block.alpha.eight.columns
= label_tag nil, t(:distributors)

2
config/routes.rb
View File

@@ -103,6 +103,8 @@ Openfoodnetwork::Application.routes.draw do
get :accessible, on: :collection
end
resources :orders, only: [:index]
resource :status do
get :job_queue
end

158
spec/controllers/api/orders_controller_spec.rb Normal file
View File

@@ -0,0 +1,158 @@
require 'spec_helper'
require 'spree/api/testing_support/helpers'
module Api
describe OrdersController, type: :controller do
include AuthenticationWorkflow
render_views
describe '#index' do
let!(:distributor) { create(:distributor_enterprise) }
let!(:distributor2) { create(:distributor_enterprise) }
let!(:supplier) { create(:supplier_enterprise) }
let!(:coordinator) { create(:distributor_enterprise) }
let!(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) }
let!(:order1) do
create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now,
distributor: distributor, billing_address: create(:address) )
end
let!(:order2) do
create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now,
distributor: distributor2, billing_address: create(:address) )
end
let!(:order3) do
create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now,
distributor: distributor, billing_address: create(:address) )
end
let!(:line_item1) do
create(:line_item, order: order1,
product: create(:product, supplier: supplier))
end
let!(:line_item2) do
create(:line_item, order: order2,
product: create(:product, supplier: supplier))
end
let!(:line_item3) do
create(:line_item, order: order2,
product: create(:product, supplier: supplier))
end
let!(:line_item4) do
create(:line_item, order: order3,
product: create(:product, supplier: supplier))
end
let!(:regular_user) { create(:user) }
let!(:admin_user) { create(:admin_user) }
context 'as a regular user' do
before do
allow(controller).to receive(:spree_current_user) { regular_user }
get :index
end
it "returns unauthorized" do
assert_unauthorized!
end
end
context 'as an admin user' do
before do
allow(controller).to receive(:spree_current_user) { admin_user }
get :index
end
it "retrieves a list of orders with appropriate attributes,
including line items with appropriate attributes" do
returns_orders(json_response)
end
it "formats completed_at to 'yyyy-mm-dd hh:mm'" do
completed_dates = json_response['orders'].map{ |order| order['completed_at'] }
correct_formats = completed_dates.all?{ |a| a == order1.completed_at.strftime('%B %d, %Y') }
expect(correct_formats).to be_truthy
end
it "returns distributor object with id key" do
distributors = json_response['orders'].map{ |order| order['distributor'] }
expect(distributors.all?{ |d| d.key?('id') }).to be_truthy
end
it "returns the order number" do
order_numbers = json_response['orders'].map{ |order| order['number'] }
expect(order_numbers.all?{ |number| number.match("^R\\d{5,10}$") }).to be_truthy
end
end
context 'as an enterprise user' do
context 'producer enterprise' do
before do
allow(controller).to receive(:spree_current_user) { supplier.owner }
get :index
end
it "does not display line items for which my enterprise is a supplier" do
assert_unauthorized!
end
end
context 'coordinator enterprise' do
before do
allow(controller).to receive(:spree_current_user) { coordinator.owner }
get :index
end
it "retrieves a list of orders" do
returns_orders(json_response)
end
end
context 'hub enterprise' do
before do
allow(controller).to receive(:spree_current_user) { distributor.owner }
get :index
end
it "retrieves a list of orders" do
returns_orders(json_response)
end
end
end
context 'with pagination' do
before do
allow(controller).to receive(:spree_current_user) { distributor.owner }
end
it 'returns pagination data when query params contain :per_page]' do
get :index, per_page: 15, page: 1
pagination_data = {
'results' => 2,
'pages' => 1,
'page' => 1,
'per_page' => 15
}
expect(json_response['pagination']).to eq pagination_data
end
end
end
private
def returns_orders(response)
keys = response['orders'].first.keys.map(&:to_sym)
expect(order_attributes.all?{ |attr| keys.include? attr }).to be_truthy
end
def order_attributes
[
:id, :number, :full_name, :email, :phone, :completed_at, :display_total,
:show_path, :edit_path, :state, :payment_state, :shipment_state,
:payments_path, :shipments_path, :ship_path, :ready_to_ship, :created_at,
:distributor_name, :special_instructions, :payment_capture_path
]
end
end
end

118
spec/controllers/spree/admin/orders_controller_spec.rb
View File

@@ -30,112 +30,22 @@ describe Spree::Admin::OrdersController, type: :controller do
end
describe "#index" do
render_views
let(:order_attributes) { [:id, :full_name, :email, :phone, :completed_at, :distributor, :order_cycle, :number] }
def self.make_simple_data!
let!(:dist1) { FactoryBot.create(:distributor_enterprise) }
let!(:order1) { FactoryBot.create(:order, state: 'complete', completed_at: Time.zone.now, distributor: dist1, billing_address: FactoryBot.create(:address) ) }
let!(:order2) { FactoryBot.create(:order, state: 'complete', completed_at: Time.zone.now, distributor: dist1, billing_address: FactoryBot.create(:address) ) }
let!(:order3) { FactoryBot.create(:order, state: 'complete', completed_at: Time.zone.now, distributor: dist1, billing_address: FactoryBot.create(:address) ) }
let!(:line_item1) { FactoryBot.create(:line_item, order: order1) }
let!(:line_item2) { FactoryBot.create(:line_item, order: order2) }
let!(:line_item3) { FactoryBot.create(:line_item, order: order2) }
let!(:line_item4) { FactoryBot.create(:line_item, order: order3) }
let(:line_item_attributes) { [:id, :quantity, :max_quantity, :supplier, :units_product, :units_variant] }
end
context "as a normal user" do
before { controller.stub spree_current_user: create_enterprise_user }
make_simple_data!
context "as a regular user" do
before { allow(controller).to receive(:spree_current_user) { create_enterprise_user } }
it "should deny me access to the index action" do
spree_get :index, :format => :json
spree_get :index
expect(response).to redirect_to spree.unauthorized_path
end
end
context "as an administrator" do
make_simple_data!
before do
controller.stub spree_current_user: quick_login_as_admin
spree_get :index, :format => :json
end
it "retrieves a list of orders with appropriate attributes, including line items with appropriate attributes" do
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 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
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['orders'].map{ |order| order['distributor'] }.all?{ |d| d.has_key?('id') }.should == true
end
it "retrieves the order number" do
json_response['orders'].map{ |order| order['number'] }.all?{ |number| number.match("^R\\d{5,10}$") }.should == true
end
end
context "as an enterprise user" do
let(:supplier) { create(:supplier_enterprise) }
let(:distributor1) { create(:distributor_enterprise) }
let(:distributor2) { create(:distributor_enterprise) }
let(:coordinator) { create(:distributor_enterprise) }
let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) }
let!(:order1) { FactoryBot.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor1, billing_address: FactoryBot.create(:address) ) }
let!(:line_item1) { FactoryBot.create(:line_item, order: order1, product: FactoryBot.create(:product, supplier: supplier)) }
let!(:line_item2) { FactoryBot.create(:line_item, order: order1, product: FactoryBot.create(:product, supplier: supplier)) }
let!(:order2) { FactoryBot.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor2, billing_address: FactoryBot.create(:address) ) }
let!(:line_item3) { FactoryBot.create(:line_item, order: order2, product: FactoryBot.create(:product, supplier: supplier)) }
let!(:order) { create(:order_with_distributor) }
context "producer enterprise" do
before { allow(controller).to receive(:spree_current_user) { order.distributor.owner } }
before do
controller.stub spree_current_user: supplier.owner
spree_get :index, :format => :json
end
it "does not display line items for which my enterprise is a supplier" do
expect(response).to redirect_to spree.unauthorized_path
end
end
context "coordinator enterprise" do
before do
controller.stub spree_current_user: coordinator.owner
spree_get :index, :format => :json
end
it "retrieves a list of orders" do
keys = json_response['orders'].first.keys.map{ |key| key.to_sym }
order_attributes.all?{ |attr| keys.include? attr }.should == true
end
end
context "hub enterprise" do
before do
controller.stub spree_current_user: distributor1.owner
spree_get :index, :format => :json
end
it "retrieves a list of orders" do
keys = json_response['orders'].first.keys.map{ |key| key.to_sym }
order_attributes.all?{ |attr| keys.include? attr }.should == true
end
it "should allow access" do
expect(response.status).to eq 200
end
end
end
@@ -148,7 +58,7 @@ describe Spree::Admin::OrdersController, type: :controller do
let(:params) { { id: order.number } }
context "as a normal user" do
before { controller.stub spree_current_user: user }
before { allow(controller).to receive(:spree_current_user) { user } }
it "should prevent me from sending order invoices" do
spree_get :invoice, params
@@ -158,7 +68,8 @@ describe Spree::Admin::OrdersController, type: :controller do
context "as an enterprise user" do
context "which is not a manager of the distributor for an order" do
before { controller.stub spree_current_user: user }
before { allow(controller).to receive(:spree_current_user) { user } }
it "should prevent me from sending order invoices" do
spree_get :invoice, params
expect(response).to redirect_to spree.unauthorized_path
@@ -166,7 +77,8 @@ describe Spree::Admin::OrdersController, type: :controller do
end
context "which is a manager of the distributor for an order" do
before { controller.stub spree_current_user: distributor.owner }
before { allow(controller).to receive(:spree_current_user) { distributor.owner } }
context "when the distributor's ABN has not been set" do
before { distributor.update_attribute(:abn, "") }
it "should allow me to send order invoices" do
@@ -205,7 +117,7 @@ describe Spree::Admin::OrdersController, type: :controller do
let(:params) { { id: order.number } }
context "as a normal user" do
before { controller.stub spree_current_user: user }
before { allow(controller).to receive(:spree_current_user) { user } }
it "should prevent me from sending order invoices" do
spree_get :print, params
@@ -215,7 +127,7 @@ describe Spree::Admin::OrdersController, type: :controller do
context "as an enterprise user" do
context "which is not a manager of the distributor for an order" do
before { controller.stub spree_current_user: user }
before { allow(controller).to receive(:spree_current_user) { user } }
it "should prevent me from sending order invoices" do
spree_get :print, params
expect(response).to redirect_to spree.unauthorized_path
@@ -223,7 +135,7 @@ describe Spree::Admin::OrdersController, type: :controller do
end
context "which is a manager of the distributor for an order" do
before { controller.stub spree_current_user: distributor.owner }
before { allow(controller).to receive(:spree_current_user) { distributor.owner } }
it "should allow me to send order invoices" do
spree_get :print, params
expect(response).to render_template :invoice

2
spec/javascripts/unit/admin/line_items/controllers/line_items_controller_spec.js.coffee
View File

@@ -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 {orders: [order], pagination: {page: 1, pages: 1, results: 1}}
httpBackend.expectGET("/api/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]

2
spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee
View File

@@ -19,7 +19,7 @@ describe "Orders service", ->
beforeEach ->
response = { orders: [{ id: 5, name: 'Order 1'}], pagination: {page: 1, pages: 1, results: 1} }
$httpBackend.expectGET('/admin/orders.json').respond 200, response
$httpBackend.expectGET('/api/orders.json').respond 200, response
result = Orders.index()
$httpBackend.flush()

35
spec/services/search_orders_spec.rb Normal file
View File

@@ -0,0 +1,35 @@
require 'spec_helper'
describe SearchOrders do
let!(:distributor) { create(:distributor_enterprise) }
let!(:order1) { create(:order, distributor: distributor) }
let!(:order2) { create(:order, distributor: distributor) }
let!(:order3) { create(:order, distributor: distributor) }
let(:enterprise_user) { distributor.owner }
describe '#orders' do
let(:params) { {} }
let(:service) { SearchOrders.new(params, enterprise_user) }
it 'returns orders' do
expect(service.orders.count).to eq 3
end
end
describe '#pagination_data' do
let(:params) { { per_page: 15, page: 1 } }
let(:service) { SearchOrders.new(params, enterprise_user) }
it 'returns pagination data' do
pagination_data = {
results: 3,
pages: 1,
page: 1,
per_page: 15
}
expect(service.pagination_data).to eq pagination_data
end
end
end