diff --git a/app/controllers/line_items_controller.rb b/app/controllers/line_items_controller.rb index 7ae5cb7251..45f0d497f2 100644 --- a/app/controllers/line_items_controller.rb +++ b/app/controllers/line_items_controller.rb @@ -1,22 +1,25 @@ class LineItemsController < BaseController respond_to :json - # Taken from Spree::Api::BaseController - rescue_from ActiveRecord::RecordNotFound, :with => :not_found + before_filter :load_line_item, only: :destroy def bought respond_with bought_items, each_serializer: Api::LineItemSerializer end def destroy - item = Spree::LineItem.find(params[:id]) - authorize! :destroy, item - destroy_with_lock item - respond_with(item) + authorize! :destroy, @line_item + destroy_with_lock @line_item + respond_with(@line_item) end private + def load_line_item + @line_item = Spree::LineItem.find_by_id(params[:id]) + not_found unless @line_item + end + # List all items the user already ordered in the current order cycle def bought_items return [] unless current_order_cycle && spree_current_user && current_distributor @@ -24,7 +27,8 @@ class LineItemsController < BaseController end def unauthorized - render nothing: true, status: 401 and return + status = spree_current_user ? 403 : 401 + render nothing: true, status: status and return end def not_found diff --git a/spec/controllers/line_items_controller_spec.rb b/spec/controllers/line_items_controller_spec.rb index 9eff3982a2..1db2d96dea 100644 --- a/spec/controllers/line_items_controller_spec.rb +++ b/spec/controllers/line_items_controller_spec.rb @@ -47,11 +47,10 @@ describe LineItemsController do context "with a line item id" do let(:params) { { format: :json, id: item } } - context "without a user" do + context "where the item's order is not associated with the user" do it "denies deletion" do delete :destroy, params - expect(response.status).to eq 401 - expect(item.reload).to be_a Spree::LineItem + expect(response.status).to eq 403 end end @@ -61,8 +60,7 @@ describe LineItemsController do context "without an order cycle" do it "denies deletion" do delete :destroy, params - expect(response.status).to eq 401 - expect(item.reload).to be_a Spree::LineItem + expect(response.status).to eq 403 end end @@ -72,8 +70,7 @@ describe LineItemsController do context "without a distributor" do it "denies deletion" do delete :destroy, params - expect(response.status).to eq 401 - expect(item.reload).to be_a Spree::LineItem + expect(response.status).to eq 403 end end @@ -82,8 +79,7 @@ describe LineItemsController do context "where changes are not allowed" do it "denies deletion" do delete :destroy, params - expect(response.status).to eq 401 - expect(item.reload).to be_a Spree::LineItem + expect(response.status).to eq 403 end end