Merge pull request #76 from dumparkltd/destroy-own-actors

Apply authorisation changes to actors controller

app/controllers/actors_controller.rb

@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class ActorsController < ApplicationController
-  before_action :set_and_authorize_actor, only: [:show, :update, :destroy]
-
   # GET /actors
   def index
     @actors = policy_scope(base_object).order(created_at: :desc).page(params[:page])
@@ -35,7 +33,7 @@ def update
       return render json: '{"error":"Record outdated"}', status: :unprocessable_entity
     end
     if @actor.update!(permitted_attributes(@actor))
-      set_and_authorize_actor
+
       render json: serialize(@actor)
     end
   end
@@ -52,11 +50,13 @@ def base_object
   end
 
   # Use callbacks to share common setup or constraints between actions.
-  def set_and_authorize_actor
-    @actor = policy_scope(base_object).find(params[:id])
-    authorize @actor
+  def authorize!
+    @actor = policy_scope(base_object)&.find(params[:id]) if params[:id]
+
+    authorize @actor || base_object
   end
 
+
   def serialize(target, serializer: ActorSerializer)
     super
   end

spec/controllers/actors_controller_spec.rb

@@ -231,7 +231,7 @@
   end
 
   describe "PUT update" do
-    let(:actor) { FactoryBot.create(:actor) }
+    let(:actor) { FactoryBot.create(:actor, :not_draft) }
     subject do
       put :update,
         format: :json,
@@ -327,34 +327,91 @@
     end
   end
 
-  describe "Delete destroy" do
+  describe "DELETE destroy" do
     let(:actor) { FactoryBot.create(:actor) }
     subject { delete :destroy, format: :json, params: {id: actor} }
 
-    context "when not signed in" do
-      it "not allow deleting an actor" do
-        expect(subject).to be_unauthorized
+    context "when signed in" do
+      before { sign_in user }
+
+      context "as a guest" do
+        let(:user) { FactoryBot.create(:user) }
+
+        context "with an actor not belonging to the signed in user" do
+          let(:actor) { FactoryBot.create(:actor) }
+
+          it "will not allow you to delete an actor" do
+            expect(subject).to be_forbidden
+          end
+        end
+
+        context "with an actor belonging to the signed in user" do
+          let(:actor) { FactoryBot.create(:actor, created_by: user) }
+
+          it "will not allow you to delete an actor" do
+            expect(subject).to be_forbidden
+          end
+        end
       end
-    end
 
-    context "when user signed in" do
-      let(:admin) { FactoryBot.create(:user, :admin) }
-      let(:guest) { FactoryBot.create(:user) }
-      let(:user) { FactoryBot.create(:user, :manager) }
+      context "as a manager" do
+        let(:user) { FactoryBot.create(:user, :manager) }
 
-      it "will not allow a guest to delete an actor" do
-        sign_in guest
-        expect(subject).to be_forbidden
+        context "with an actor not belonging to the signed in user" do
+          let(:actor) { FactoryBot.create(:actor) }
+
+          it "will not allow you to delete an actor" do
+            expect(subject).to be_forbidden
+          end
+        end
+
+        context "with an actor belonging to the signed in user" do
+          let(:actor) { FactoryBot.create(:actor, created_by: user) }
+
+          it "will allow you to delete an actor" do
+            expect(subject).to be_no_content
+          end
+        end
       end
 
-      it "will not allow a manager to delete an actor" do
-        sign_in manager
-        expect(subject).to be_forbidden
+      context "as a coordinator" do
+        let(:user) { FactoryBot.create(:user, :coordinator) }
+
+        context "with an actor not belonging to the signed in user" do
+          let(:actor) { FactoryBot.create(:actor) }
+
+          it "will not allow you to delete an actor" do
+            expect(subject).to be_forbidden
+          end
+        end
+
+        context "with an actor belonging to the signed in user" do
+          let(:actor) { FactoryBot.create(:actor, created_by: user) }
+
+          it "will allow you to delete an actor" do
+            expect(subject).to be_no_content
+          end
+        end
       end
 
-      it "will allow an admin to delete an actor" do
-        sign_in admin
-        expect(subject).to be_no_content
+      context "as an admin" do
+        let(:user) { FactoryBot.create(:user, :admin) }
+
+        context "with an actor not belonging to the signed in user" do
+          let(:actor) { FactoryBot.create(:actor) }
+
+          it "will allow you to delete an actor" do
+            expect(subject).to be_no_content
+          end
+        end
+
+        context "with an actor belonging to the signed in user" do
+          let(:actor) { FactoryBot.create(:actor, created_by: user) }
+
+          it "will allow you to delete an actor" do
+            expect(subject).to be_no_content
+          end
+        end
       end
     end
   end