From d09e0cd44ac448975a13cd8909a16e49a097cde8 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Fri, 14 Jul 2017 12:30:34 +0200 Subject: [PATCH] Deal with both JS and JSON format --- .../admin/line_items_controller_decorator.rb | 16 ++ .../spree/admin/line_items_controller_spec.rb | 139 +++++++++++++----- 2 files changed, 118 insertions(+), 37 deletions(-) diff --git a/app/controllers/spree/admin/line_items_controller_decorator.rb b/app/controllers/spree/admin/line_items_controller_decorator.rb index 4b31f4d749..7d76cda245 100644 --- a/app/controllers/spree/admin/line_items_controller_decorator.rb +++ b/app/controllers/spree/admin/line_items_controller_decorator.rb @@ -32,6 +32,10 @@ Spree::Admin::LineItemsController.class_eval do 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 update respond_to do |format| format.html { render_order_form } @@ -42,15 +46,27 @@ 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 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/spec/controllers/spree/admin/line_items_controller_spec.rb b/spec/controllers/spree/admin/line_items_controller_spec.rb index dc82b005b5..2aced0ad0c 100644 --- a/spec/controllers/spree/admin/line_items_controller_spec.rb +++ b/spec/controllers/spree/admin/line_items_controller_spec.rb @@ -172,41 +172,82 @@ describe Spree::Admin::LineItemsController do controller.stub spree_current_user: coordinator.owner end - it "updates the line item" do - xhr :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 - xhr :put, :update, params - expect(response.body).to eq ' ' - end - - it 'returns a 204 response' do - xhr :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 + # Used in admin/orders/edit + context 'when the request is JS/XHR (jquery-rails gem)' do + it "updates the line item" do xhr :put, :update, params - expect(JSON.parse(response.body)['errors']).to eq(errors) + 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 a 412 response' do + it "returns an empty JSON response" do xhr :put, :update, params - expect(response.status).to eq 412 + expect(response.body).to eq ' ' + end + + it 'returns a 204 response' do + xhr :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 + xhr :put, :update, params + expect(JSON.parse(response.body)['errors']).to eq(errors) + end + + it 'returns a 412 response' do + xhr :put, :update, params + expect(response.status).to eq 412 + end + end + 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 context 'when the request is HTML' do - before { params.merge(format: :html) } + before { params[:format] = :html } it 'returns an HTML response with the order form' do spree_put :update, params @@ -246,20 +287,44 @@ describe Spree::Admin::LineItemsController do controller.stub spree_current_user: coordinator.owner end - it 'destroys the line item' do - expect { + # Used in admin/orders/edit + context 'when the request is JS/XHR (jquery-rails gem)' do + it 'destroys the line item' do + expect { + xhr :delete, :destroy, params + }.to change { Spree::LineItem.where(id: line_item1).count }.from(1).to(0) + end + + it 'returns an empty JSON response' do xhr :delete, :destroy, params - }.to change { Spree::LineItem.where(id: line_item1).count }.from(1).to(0) + expect(response.body).to eq ' ' + end + + it 'returns a 204 response' do + xhr :delete, :destroy, params + expect(response.status).to eq 204 + end end - it 'returns an empty JSON response' do - xhr :delete, :destroy, params - expect(response.body).to eq ' ' - end + # Used in admin/orders/bulk_management + context 'when the request is JSON (angular)' do + before { params[:format] = :json } - it 'returns a 204 response' do - xhr :delete, :destroy, params - expect(response.status).to eq 204 + 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 context 'when the request is HTML' do