Further tweaks to LineItemsController

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

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