Merge pull request #6362 from Ana06/staging-workflow-policy

 Authorize RequestExclusion instead of StagingWorkflow &&  Improve ExcludeRequestsController tests

src/api/app/controllers/webui/staging/excluded_requests_controller.rb

@@ -15,7 +15,6 @@ def index
       end
 
       def create
-        authorize @staging_workflow
         staging_request_exclusion = params[:staging_request_exclusion]
 
         request = @staging_workflow.target_of_bs_requests.find_by(number: staging_request_exclusion[:number])
@@ -25,6 +24,7 @@ def create
         end
 
         request_exclusion = @staging_workflow.request_exclusions.build(bs_request: request, description: staging_request_exclusion[:description])
+        authorize request_exclusion
 
         if request_exclusion.save
           flash[:success] = 'The request was successfully excluded'
@@ -35,7 +35,7 @@ def create
       end
 
       def destroy
-        authorize @staging_workflow
+        authorize @request_exclusion
 
         if @request_exclusion.destroy
           flash[:success] = 'The request is not excluded anymore'

src/api/spec/controllers/webui/staging/excluded_requests_controller_spec.rb

@@ -2,6 +2,7 @@
 
 RSpec.describe Webui::Staging::ExcludedRequestsController, type: :controller do
   let(:user) { create(:confirmed_user, login: 'tom') }
+  let(:another_user) { create(:confirmed_user, login: 'another_user') }
   let(:project) { user.home_project }
   let(:staging_workflow) { create(:staging_workflow_with_staging_projects, project: project) }
 
@@ -22,49 +23,99 @@
   describe '#create' do
     let(:description) { Faker::Lorem.sentence }
 
-    context 'success' do
-      before do
-        post :create, params: { staging_workflow_id: staging_workflow, staging_request_exclusion: { number: bs_request, description: description } }
+    context 'succeeds' do
+      subject { post :create, params: { staging_workflow_id: staging_workflow, staging_request_exclusion: { number: bs_request, description: description } } }
+
+      it { expect { subject }.to(change { staging_workflow.request_exclusions.count }.by(1)) }
+
+      context 'response' do
+        before { subject }
+
+        it { expect(staging_workflow.request_exclusions.first.description).to eq(description) }
+        it { expect(response).to redirect_to(staging_workflow_excluded_requests_path(staging_workflow)) }
+        it { expect(flash[:success]).not_to be_nil }
       end
+    end
+
+    context 'fails: invalid exclusion request' do
+      subject { post :create, params: { staging_workflow_id: staging_workflow, staging_request_exclusion: { number: bs_request } } }
+
+      it { expect { subject }.not_to(change { staging_workflow.request_exclusions.count }) }
 
-      it { expect(Staging::RequestExclusion.count).to eq(1) }
-      it { expect(staging_workflow.request_exclusions.first.description).to eq(description) }
-      it { expect(response).to redirect_to(staging_workflow_excluded_requests_path(staging_workflow)) }
-      it { expect(flash[:success]).not_to be_nil }
+      context 'response' do
+        before { subject }
+
+        it { expect(response).to redirect_to(staging_workflow_excluded_requests_path(staging_workflow)) }
+        it { expect(flash[:error]).not_to be_nil }
+      end
     end
 
-    context "doesn't exist" do
+    context "fails: user doesn't have permissions" do
+      subject { post :create, params: { staging_workflow_id: staging_workflow, staging_request_exclusion: { number: bs_request, description: description } } }
+
       before do
-        allow_any_instance_of(Staging::RequestExclusion).to receive(:save).and_return(false)
-        post :create, params: { staging_workflow_id: staging_workflow, staging_request_exclusion: { number: bs_request, description: description } }
+        staging_workflow
+        login(another_user)
       end
 
-      it { expect(response).to redirect_to(staging_workflow_excluded_requests_path(staging_workflow)) }
-      it { expect(flash[:error]).not_to be_nil }
+      it { expect { subject }.not_to(change { staging_workflow.request_exclusions.count }) }
+
+      context 'response' do
+        before { subject }
+
+        it { expect(response).to redirect_to(root_path) }
+        it { expect(flash[:error]).not_to be_nil }
+      end
     end
   end
 
   describe '#destroy' do
-    let(:request_exclusion) { create(:request_exclusion, staging_workflow: staging_workflow, bs_request: bs_request) }
+    let!(:request_exclusion) { create(:request_exclusion, staging_workflow: staging_workflow, bs_request: bs_request) }
 
-    context 'success' do
-      before do
-        delete :destroy, params: { staging_workflow_id: staging_workflow, id: request_exclusion }
+    context 'succeeds' do
+      subject { delete :destroy, params: { staging_workflow_id: staging_workflow, id: request_exclusion } }
+
+      it { expect { subject }.to(change { staging_workflow.request_exclusions.count }.by(-1)) }
+
+      context 'response' do
+        before { subject }
+
+        it { expect(response).to redirect_to(staging_workflow_excluded_requests_path(staging_workflow)) }
+        it { expect(flash[:success]).not_to be_nil }
       end
+    end
+
+    context 'fails: destroy not possible' do
+      subject { delete :destroy, params: { staging_workflow_id: staging_workflow, id: request_exclusion } }
+
+      before { allow_any_instance_of(Staging::RequestExclusion).to receive(:destroy).and_return(false) }
+
+      it { expect { subject }.not_to(change { staging_workflow.request_exclusions.count }) }
 
-      it { expect(Staging::RequestExclusion.count).to eq(0) }
-      it { expect(response).to redirect_to(staging_workflow_excluded_requests_path(staging_workflow)) }
-      it { expect(flash[:success]).not_to be_nil }
+      context 'response' do
+        before { subject }
+
+        it { expect(flash[:error]).not_to be_nil }
+        it { expect(response).to redirect_to(staging_workflow_excluded_requests_path(staging_workflow)) }
+      end
     end
 
-    context 'error' do
+    context "fails: users doesn't have permissions" do
+      subject { delete :destroy, params: { staging_workflow_id: staging_workflow, id: request_exclusion } }
+
       before do
-        allow_any_instance_of(Staging::RequestExclusion).to receive(:destroy).and_return(false)
-        delete :destroy, params: { staging_workflow_id: staging_workflow, id: request_exclusion }
+        staging_workflow
+        login(another_user)
       end
 
-      it { expect(flash[:error]).not_to be_nil }
-      it { expect(response).to redirect_to(staging_workflow_excluded_requests_path(staging_workflow)) }
+      it { expect { subject }.not_to(change { staging_workflow.request_exclusions.count }) }
+
+      context 'response' do
+        before { subject }
+
+        it { expect(response).to redirect_to(root_path) }
+        it { expect(flash[:error]).not_to be_nil }
+      end
     end
   end
 end