diff --git a/app/assets/javascripts/admin/resources/resources/line_item_resource.js.coffee b/app/assets/javascripts/admin/resources/resources/line_item_resource.js.coffee index 4301f8df82..2ec5703c82 100644 --- a/app/assets/javascripts/admin/resources/resources/line_item_resource.js.coffee +++ b/app/assets/javascripts/admin/resources/resources/line_item_resource.js.coffee @@ -1,5 +1,5 @@ angular.module("admin.resources").factory 'LineItemResource', ($resource) -> - $resource('/admin/:orders/:order_number/line_items/:id.json', {}, { + $resource('/admin/bulk_line_items/:id.json', {}, { 'index': method: 'GET' isArray: true diff --git a/app/assets/javascripts/admin/resources/services/line_items.js.coffee b/app/assets/javascripts/admin/resources/services/line_items.js.coffee index 182ef81e6a..7c96caecd8 100644 --- a/app/assets/javascripts/admin/resources/services/line_items.js.coffee +++ b/app/assets/javascripts/admin/resources/services/line_items.js.coffee @@ -26,7 +26,7 @@ angular.module("admin.resources").factory 'LineItems', ($q, LineItemResource) -> save: (lineItem) -> deferred = $q.defer() lineItem.errors = {} - lineItem.$update({id: lineItem.id, orders: "orders", order_number: lineItem.order.number}) + lineItem.$update({id: lineItem.id}) .then( (data) => @pristineByID[lineItem.id] = angular.copy(lineItem) deferred.resolve(data) @@ -54,7 +54,7 @@ angular.module("admin.resources").factory 'LineItems', ($q, LineItemResource) -> delete: (lineItem, callback=null) -> deferred = $q.defer() - lineItem.$delete({id: lineItem.id, orders: "orders", order_number: lineItem.order.number}) + lineItem.$delete({id: lineItem.id}) .then( (data) => delete @byID[lineItem.id] delete @pristineByID[lineItem.id] diff --git a/app/controllers/admin/bulk_line_items_controller.rb b/app/controllers/admin/bulk_line_items_controller.rb new file mode 100644 index 0000000000..044038bbfb --- /dev/null +++ b/app/controllers/admin/bulk_line_items_controller.rb @@ -0,0 +1,53 @@ +module Admin + class BulkLineItemsController < Spree::Admin::BaseController + + # GET /admin/bulk_line_items.json + # + def index + order_params = params[:q].andand.delete :order + orders = OpenFoodNetwork::Permissions.new(spree_current_user).editable_orders.ransack(order_params).result + line_items = OpenFoodNetwork::Permissions.new(spree_current_user).editable_line_items.where(order_id: orders).ransack(params[:q]) + render_as_json line_items.result.reorder('order_id ASC, id ASC') + end + + # PUT /admin/bulk_line_items/:id.json + # + def update + load_line_item + authorize! :update, @line_item.order + + if @line_item.update_attributes(params[:line_item]) + render nothing: true, status: 204 # No Content, does not trigger ng resource auto-update + else + render json: { errors: @line_item.errors }, status: 412 + end + end + + # DELETE /admin/bulk_line_items/:id.json + # + def destroy + load_line_item + authorize! :update, @line_item.order + + @line_item.destroy + render nothing: true, status: 204 # No Content, does not trigger ng resource auto-update + end + + private + + def load_line_item + @line_item = Spree::LineItem.find(params[:id]) + end + + def model_class + Spree::LineItem + end + + # Returns the appropriate serializer for this controller + # + # @return [Api::Admin::LineItemSerializer] + def serializer(_ams_prefix) + Api::Admin::LineItemSerializer + end + end +end diff --git a/app/controllers/spree/admin/line_items_controller_decorator.rb b/app/controllers/spree/admin/line_items_controller_decorator.rb index 7d76cda245..637fc82a11 100644 --- a/app/controllers/spree/admin/line_items_controller_decorator.rb +++ b/app/controllers/spree/admin/line_items_controller_decorator.rb @@ -2,19 +2,6 @@ Spree::Admin::LineItemsController.class_eval do prepend_before_filter :load_order, except: :index around_filter :apply_enterprise_fees_with_lock, only: :update - # TODO make updating line items faster by creating a bulk update method - - def index - respond_to do |format| - format.json do - order_params = params[:q].andand.delete :order - orders = OpenFoodNetwork::Permissions.new(spree_current_user).editable_orders.ransack(order_params).result - line_items = OpenFoodNetwork::Permissions.new(spree_current_user).editable_line_items.where(order_id: orders).ransack(params[:q]) - render_as_json line_items.result.reorder('order_id ASC, id ASC') - end - end - end - def create variant = Spree::Variant.find(params[:line_item][:variant_id]) OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope(variant) @@ -34,7 +21,6 @@ Spree::Admin::LineItemsController.class_eval do # TODO: simplify this, 3 formats per action is too much # we need `js` format for admin/orders/edit (jquery-rails gem) - # we need `json` format for admin/orders/bulk_management (angular) # we don't know if `html` format is needed def update respond_to do |format| @@ -46,19 +32,11 @@ Spree::Admin::LineItemsController.class_eval do render json: { errors: @line_item.errors }, status: 412 end } - format.json { - if @line_item.update_attributes(params[:line_item]) - render nothing: true, status: 204 # No Content, does not trigger ng resource auto-update - else - render json: { errors: @line_item.errors }, status: 412 - end - } end end # TODO: simplify this, 3 formats per action is too much: # we need `js` format for admin/orders/edit (jquery-rails gem) - # we need `json` format for admin/orders/bulk_management (angular) # we don't know if `html` format is needed def destroy @line_item.destroy @@ -66,7 +44,6 @@ Spree::Admin::LineItemsController.class_eval do respond_to do |format| format.html { render_order_form } format.js { render nothing: true, status: 204 } # No Content - format.json { render nothing: true, status: 204 } # No Content end end diff --git a/config/routes.rb b/config/routes.rb index 02c459cea0..b91779166c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -53,6 +53,10 @@ Openfoodnetwork::Application.routes.draw do end end + namespace :admin do + resources :bulk_line_items + end + get '/checkout', :to => 'checkout#edit' , :as => :checkout put '/checkout', :to => 'checkout#update' , :as => :update_checkout get '/checkout/paypal_payment/:order_id', to: 'checkout#paypal_payment', as: :paypal_payment @@ -265,8 +269,6 @@ Spree::Core::Engine.routes.prepend do get :print_ticket, on: :member get :managed, on: :collection end - - resources :line_items, only: [:index], format: :json end resources :orders do diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb new file mode 100644 index 0000000000..a993285c59 --- /dev/null +++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb @@ -0,0 +1,249 @@ +require 'spec_helper' + +describe Admin::BulkLineItemsController do + include AuthenticationWorkflow + + describe '#index' do + render_views + + let(:line_item_attributes) { [:id, :quantity, :max_quantity, :price, :supplier, :final_weight_volume, :units_product, :units_variant, :order] } + let!(:dist1) { FactoryGirl.create(:distributor_enterprise) } + let!(:order1) { FactoryGirl.create(:order, state: 'complete', completed_at: 1.day.ago, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:order2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.zone.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:order3) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.zone.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1) } + let!(:line_item2) { FactoryGirl.create(:line_item, order: order2) } + let!(:line_item3) { FactoryGirl.create(:line_item, order: order2) } + let!(:line_item4) { FactoryGirl.create(:line_item, order: order3) } + + context "as a normal user" do + before { controller.stub spree_current_user: create_enterprise_user } + + it "should deny me access to the index action" do + spree_get :index, :format => :json + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "as an administrator" do + + before do + controller.stub spree_current_user: quick_login_as_admin + end + + context "when no ransack params are passed in" do + before do + spree_get :index, :format => :json + end + + it "retrieves a list of line_items with appropriate attributes, including line items with appropriate attributes" do + keys = json_response.first.keys.map{ |key| key.to_sym } + line_item_attributes.all?{ |attr| keys.include? attr }.should == true + end + + it "sorts line_items in ascending id line_item" do + ids = json_response.map{ |line_item| line_item['id'] } + ids[0].should < ids[1] + ids[1].should < ids[2] + end + + it "formats final_weight_volume as a float" do + json_response.map{ |line_item| line_item['final_weight_volume'] }.all?{ |fwv| fwv.is_a?(Float) }.should == true + end + + it "returns distributor object with id key" do + json_response.map{ |line_item| line_item['supplier'] }.all?{ |d| d.has_key?('id') }.should == true + end + end + + context "when ransack params are passed in for line items" do + before do + spree_get :index, :format => :json, q: { order_id_eq: order2.id } + end + + it "retrives a list of line items which match the criteria" do + expect(json_response.map{ |line_item| line_item['id'] }).to eq [line_item2.id, line_item3.id] + end + end + + context "when ransack params are passed in for orders" do + before do + spree_get :index, :format => :json, q: { order: { completed_at_gt: 2.hours.ago } } + end + + it "retrives a list of line items whose orders match the criteria" do + expect(json_response.map{ |line_item| line_item['id'] }).to eq [line_item2.id, line_item3.id, line_item4.id] + end + 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) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let!(:line_item2) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let!(:order2) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor2, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item3) { FactoryGirl.create(:line_item, order: order2, product: FactoryGirl.create(:product, supplier: supplier)) } + + context "producer enterprise" do + 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 line_items" do + keys = json_response.first.keys.map{ |key| key.to_sym } + line_item_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 line_items" do + keys = json_response.first.keys.map{ |key| key.to_sym } + line_item_attributes.all?{ |attr| keys.include? attr }.should == true + end + end + end + end + + describe '#update' do + let(:supplier) { create(:supplier_enterprise) } + let(:distributor1) { create(:distributor_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) } + let!(:order1) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let(:line_item_params) { { quantity: 3, final_weight_volume: 3000, price: 3.00 } } + let(:params) { { id: line_item1.id, order_id: order1.number, line_item: line_item_params } } + + context "as an enterprise user" do + context "producer enterprise" do + before do + controller.stub spree_current_user: supplier.owner + spree_put :update, params + end + + it "does not allow access" do + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "coordinator enterprise" do + render_views + + before do + controller.stub spree_current_user: coordinator.owner + end + + # Used in admin/orders/bulk_management + context 'when the request is JSON (angular)' do + before { params[:format] = :json } + + it "updates the line item" do + spree_put :update, params + line_item1.reload + expect(line_item1.quantity).to eq 3 + expect(line_item1.final_weight_volume).to eq 3000 + expect(line_item1.price).to eq 3.00 + end + + it "returns an empty JSON response" do + spree_put :update, params + expect(response.body).to eq ' ' + end + + it 'returns a 204 response' do + spree_put :update, params + expect(response.status).to eq 204 + end + + context 'when the line item params are not correct' do + let(:line_item_params) { { price: 'hola' } } + let(:errors) { { 'price' => ['is not a number'] } } + + it 'returns a JSON with the errors' do + spree_put :update, params + expect(JSON.parse(response.body)['errors']).to eq(errors) + end + + it 'returns a 412 response' do + spree_put :update, params + expect(response.status).to eq 412 + end + end + end + end + + context "hub enterprise" do + before do + controller.stub spree_current_user: distributor1.owner + xhr :put, :update, params + end + + it "updates the line item" do + line_item1.reload + expect(line_item1.quantity).to eq 3 + expect(line_item1.final_weight_volume).to eq 3000 + expect(line_item1.price).to eq 3.00 + end + end + end + end + + describe '#destroy' do + render_views + + let(:supplier) { create(:supplier_enterprise) } + let(:distributor1) { create(:distributor_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) } + let!(:order1) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let(:params) { { id: line_item1.id, order_id: order1.number } } + + before do + controller.stub spree_current_user: coordinator.owner + end + + # Used in admin/orders/bulk_management + context 'when the request is JSON (angular)' do + before { params[:format] = :json } + + it 'destroys the line item' do + expect { + spree_delete :destroy, params + }.to change { Spree::LineItem.where(id: line_item1).count }.from(1).to(0) + end + + it 'returns an empty JSON response' do + spree_delete :destroy, params + expect(response.body).to eq ' ' + end + + it 'returns a 204 response' do + spree_delete :destroy, params + expect(response.status).to eq 204 + end + end + end +end diff --git a/spec/controllers/spree/admin/line_items_controller_spec.rb b/spec/controllers/spree/admin/line_items_controller_spec.rb index 2aced0ad0c..0c9a4e4835 100644 --- a/spec/controllers/spree/admin/line_items_controller_spec.rb +++ b/spec/controllers/spree/admin/line_items_controller_spec.rb @@ -3,129 +3,6 @@ require 'spec_helper' describe Spree::Admin::LineItemsController do include AuthenticationWorkflow - describe "#index" do - render_views - - let(:line_item_attributes) { [:id, :quantity, :max_quantity, :price, :supplier, :final_weight_volume, :units_product, :units_variant, :order] } - let!(:dist1) { FactoryGirl.create(:distributor_enterprise) } - let!(:order1) { FactoryGirl.create(:order, state: 'complete', completed_at: 1.day.ago, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } - let!(:order2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.zone.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } - let!(:order3) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.zone.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } - let!(:line_item1) { FactoryGirl.create(:line_item, order: order1) } - let!(:line_item2) { FactoryGirl.create(:line_item, order: order2) } - let!(:line_item3) { FactoryGirl.create(:line_item, order: order2) } - let!(:line_item4) { FactoryGirl.create(:line_item, order: order3) } - - context "as a normal user" do - before { controller.stub spree_current_user: create_enterprise_user } - - it "should deny me access to the index action" do - spree_get :index, :format => :json - expect(response).to redirect_to spree.unauthorized_path - end - end - - context "as an administrator" do - - before do - controller.stub spree_current_user: quick_login_as_admin - end - - context "when no ransack params are passed in" do - before do - spree_get :index, :format => :json - end - - it "retrieves a list of line_items with appropriate attributes, including line items with appropriate attributes" do - keys = json_response.first.keys.map{ |key| key.to_sym } - line_item_attributes.all?{ |attr| keys.include? attr }.should == true - end - - it "sorts line_items in ascending id line_item" do - ids = json_response.map{ |line_item| line_item['id'] } - ids[0].should < ids[1] - ids[1].should < ids[2] - end - - it "formats final_weight_volume as a float" do - json_response.map{ |line_item| line_item['final_weight_volume'] }.all?{ |fwv| fwv.is_a?(Float) }.should == true - end - - it "returns distributor object with id key" do - json_response.map{ |line_item| line_item['supplier'] }.all?{ |d| d.has_key?('id') }.should == true - end - end - - context "when ransack params are passed in for line items" do - before do - spree_get :index, :format => :json, q: { order_id_eq: order2.id } - end - - it "retrives a list of line items which match the criteria" do - expect(json_response.map{ |line_item| line_item['id'] }).to eq [line_item2.id, line_item3.id] - end - end - - context "when ransack params are passed in for orders" do - before do - spree_get :index, :format => :json, q: { order: { completed_at_gt: 2.hours.ago } } - end - - it "retrives a list of line items whose orders match the criteria" do - expect(json_response.map{ |line_item| line_item['id'] }).to eq [line_item2.id, line_item3.id, line_item4.id] - end - 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) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor1, billing_address: FactoryGirl.create(:address) ) } - let!(:line_item1) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } - let!(:line_item2) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } - let!(:order2) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor2, billing_address: FactoryGirl.create(:address) ) } - let!(:line_item3) { FactoryGirl.create(:line_item, order: order2, product: FactoryGirl.create(:product, supplier: supplier)) } - - context "producer enterprise" do - 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 line_items" do - keys = json_response.first.keys.map{ |key| key.to_sym } - line_item_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 line_items" do - keys = json_response.first.keys.map{ |key| key.to_sym } - line_item_attributes.all?{ |attr| keys.include? attr }.should == true - end - end - end - end - describe "#create" do let!(:variant) { create(:variant, price: 88) } let!(:vo) { create(:variant_override, hub: distributor, variant: variant, price: 11.11) }