#465: Update specs for Projects::BuildListsController

app/controllers/projects/build_lists_controller.rb

@@ -7,18 +7,12 @@ class Projects::BuildListsController < Projects::BaseController
   before_action :authenticate_user!
   skip_before_action :authenticate_user!, only: [:show, :index, :log] if APP_CONFIG['anonymous_access']
 
-  before_action :find_build_list, only: [:show, :publish, :cancel, :update, :log, :create_container, :dependent_projects]
+  before_action :load_build_list, except: NESTED_ACTIONS
-
-  load_and_authorize_resource :project, only: [:new, :create]
-  load_resource :project, only: :index, parent: false
-  load_and_authorize_resource :build_list, through: :project, only: NESTED_ACTIONS, shallow: true
-  load_and_authorize_resource except: NESTED_ACTIONS
 
   before_action :create_from_build_list, only: :new
 
   def index
     authorize :build_list
-    authorize @project, :show? if @project
     params[:filter].each{|k,v| params[:filter].delete(k) if v.blank? } if params[:filter]
 
     respond_to do |format|
@@ -47,6 +41,7 @@ class Projects::BuildListsController < Projects::BaseController
   end
 
   def new
+    authorize @build_list = @project.build_lists.build
     if params[:show] == 'inline' && params[:build_list_id].present?
       render json: new_build_list_data(@build_list, @project, params), layout: false
     else
@@ -74,7 +69,8 @@ class Projects::BuildListsController < Projects::BaseController
         @build_list.priority = current_user.build_priority # User builds more priority than mass rebuild with zero priority
 
         flash_options = { project_version: @build_list.project_version, arch: arch.name, build_for_platform: build_for_platform.name }
-        if authorize!(:create, @build_list) && @build_list.save
+        authorize @build_list
+        if @build_list.save
           build_lists << @build_list
           notices << t('flash.build_list.saved', flash_options)
         else
@@ -126,8 +122,6 @@ class Projects::BuildListsController < Projects::BaseController
   end
 
   def dependent_projects
-    raise Pundit::NotAuthorizedError if @build_list.save_to_platform.personal?
-
     if request.post?
       prs = params[:build_list]
       if prs.present? && prs[:projects].present? && prs[:arches].present?
@@ -212,6 +206,16 @@ class Projects::BuildListsController < Projects::BaseController
 
   protected
 
+  # Private: before_action hook which loads BuidList.
+  def load_build_list
+    authorize @build_list =
+      if @project
+        @project.build_lists
+      else
+        BuildList
+      end.find(params[:id])
+  end
+
   def do_and_back(action, prefix, success = 'success', fail = 'fail')
     result  = @build_list.send("can_#{action}?") && @build_list.send(action)
     message = result ? success : fail
@@ -219,10 +223,6 @@ class Projects::BuildListsController < Projects::BaseController
     redirect_to :back
   end
 
-  def find_build_list
-    @build_list = BuildList.find(params[:id])
-  end
-
   def create_from_build_list
     return if params[:build_list_id].blank?
     build_list = @project.build_lists.find(params[:build_list_id])

app/policies/build_list_policy.rb

@@ -21,6 +21,10 @@ class BuildListPolicy < ApplicationPolicy
   end
   alias_method :rerun_tests?, :create?
 
+  def dependent_projects?
+    record.save_to_platform.main? && create?
+  end
+
   def publish_into_testing?
     return false unless record.new_core?
     return false unless record.can_publish_into_testing?

spec/controllers/projects/build_lists_controller_spec.rb

@@ -5,24 +5,24 @@ describe Projects::BuildListsController, type: :controller do
   shared_examples_for 'show build list' do
     it 'should be able to perform show action' do
       get :show, @show_params
-      response.should be_success
+      expect(response).to be_success
     end
 
     it 'should be able to perform index action in project scope' do
       get :index, name_with_owner: @project.name_with_owner
-      response.should be_success
+      expect(response).to be_success
     end
   end
 
   shared_examples_for 'not show build list' do
     it 'should not be able to perform show action' do
       get :show, @show_params
-      response.should redirect_to(forbidden_url)
+      expect(response).to redirect_to(forbidden_url)
     end
 
     it 'should not be able to perform index action in project scope' do
       get :index, name_with_owner: @project.name_with_owner
-      response.should redirect_to(forbidden_url)
+      expect(response).to redirect_to(forbidden_url)
     end
   end
 
@@ -33,31 +33,35 @@ describe Projects::BuildListsController, type: :controller do
 
     it 'should be able to perform new action' do
       get :new, name_with_owner: @project.name_with_owner
-      response.should render_template(:new)
+      expect(response).to render_template(:new)
     end
 
     it 'should be able to perform create action' do
       post :create, { name_with_owner: @project.name_with_owner }.merge(@create_params)
-      response.should redirect_to project_build_lists_path(@project)
+      expect(response).to redirect_to project_build_lists_path(@project)
     end
 
     it 'should save correct commit_hash for branch based build' do
       post :create, { name_with_owner: @project.name_with_owner }.merge(@create_params).deep_merge(build_list: { project_version: "master" })
-      @project.build_lists.last.commit_hash.should == @project.repo.commits('master').first.id
+      expect(@project.build_lists.last.commit_hash).to eq @project.repo.commits('master').first.id
     end
 
     it 'should save correct commit_hash for tag based build' do
       system("cd #{@project.repo.path} && git tag 4.7.5.3") # TODO REDO through grit
       post :create, { name_with_owner: @project.name_with_owner }.merge(@create_params).deep_merge(build_list: { project_version: "4.7.5.3" })
-      @project.build_lists.last.commit_hash.should == @project.repo.commits('4.7.5.3').first.id
+      expect(@project.build_lists.last.commit_hash).to eq @project.repo.commits('4.7.5.3').first.id
     end
 
     it 'should not be able to create with wrong project version' do
-      lambda{ post :create, { name_with_owner: @project.name_with_owner }.merge(@create_params).deep_merge(build_list: { project_version: "wrong", commit_hash: nil })}.should change{ @project.build_lists.count }.by(0)
+      expect do
+        post :create, { name_with_owner: @project.name_with_owner }.merge(@create_params).deep_merge(build_list: { project_version: "wrong", commit_hash: nil })
+      end.to_not change{ @project.build_lists.count }
     end
 
     it 'should not be able to create with wrong git hash' do
-      lambda{ post :create, { name_with_owner: @project.name_with_owner }.merge(@create_params).deep_merge(build_list: { commit_hash: 'wrong' }) }.should change{ @project.build_lists.count }.by(0)
+      expect do
+        post :create, { name_with_owner: @project.name_with_owner }.merge(@create_params).deep_merge(build_list: { commit_hash: 'wrong' })
+      end.to_not change{ @project.build_lists.count }
     end
   end
 
@@ -68,12 +72,12 @@ describe Projects::BuildListsController, type: :controller do
 
     it 'should not be able to perform new action' do
       get :new, name_with_owner: @project.name_with_owner
-      response.should redirect_to(forbidden_url)
+      expect(response).to redirect_to(forbidden_url)
     end unless skip_new
 
     it 'should not be able to perform create action' do
       post :create, { name_with_owner: @project.name_with_owner }.merge(@create_params)
-      response.should redirect_to(forbidden_url)
+      expect(response).to redirect_to(forbidden_url)
     end
   end
 
@@ -98,12 +102,12 @@ describe Projects::BuildListsController, type: :controller do
     context 'for guest' do
       it 'should be able to perform index action', anonymous_access: true do
         get :index
-        response.should be_success
+        expect(response).to be_success
       end
 
       it 'should not be able to perform index action', anonymous_access: false do
         get :index
-        response.should redirect_to(new_user_session_path)
+        expect(response).to redirect_to(new_user_session_path)
       end
 
     end
@@ -199,11 +203,11 @@ describe Projects::BuildListsController, type: :controller do
 
           context "if it has :success status" do
             it 'should return 302 response code' do
-              response.status.should == 302
+              expect(response.status).to eq 302
             end
 
             it "should reject publish build list" do
-              @build_list.reload.status.should == BuildList::REJECTED_PUBLISH
+              expect(@build_list.reload.status).to eq BuildList::REJECTED_PUBLISH
             end
           end
 
@@ -214,7 +218,7 @@ describe Projects::BuildListsController, type: :controller do
             end
 
             it "should not change status of build list" do
-              @build_list.reload.status.should == BuildList::BUILD_ERROR
+              expect(@build_list.reload.status).to eq BuildList::BUILD_ERROR
             end
           end
         end
@@ -227,12 +231,12 @@ describe Projects::BuildListsController, type: :controller do
           end
 
           it "should redirect to forbidden page" do
-            response.should redirect_to(forbidden_url)
+            expect(response).to redirect_to(forbidden_url)
           end
 
           it "should not change status of build list" do
             do_reject_publish
-            @build_list.reload.status.should == BuildList::SUCCESS
+            expect(@build_list.reload.status).to eq BuildList::SUCCESS
           end
         end
 
@@ -247,12 +251,12 @@ describe Projects::BuildListsController, type: :controller do
           end
 
           it "should redirect to forbidden page" do
-            response.should redirect_to(forbidden_url)
+            expect(response).to redirect_to(forbidden_url)
           end
 
           it "should not change status of build list" do
             do_reject_publish
-            @build_list.reload.status.should == BuildList::SUCCESS
+            expect(@build_list.reload.status).to eq BuildList::SUCCESS
           end
         end
 
@@ -267,11 +271,11 @@ describe Projects::BuildListsController, type: :controller do
           end
 
           it 'should return 302 response code' do
-            response.status.should == 302
+            expect(response.status).to eq 302
           end
 
           it "should reject publish build list" do
-            @build_list.reload.status.should == BuildList::REJECTED_PUBLISH
+            expect(@build_list.reload.status).to eq BuildList::REJECTED_PUBLISH
           end
         end
       end
@@ -293,15 +297,15 @@ describe Projects::BuildListsController, type: :controller do
 
         it 'should be able to perform index action' do
           get :index
-          response.should be_success
+          expect(response).to be_success
         end
 
         it 'should show only accessible build_lists' do
-          get :index, filter: {ownership: 'everything'}
+          get :index, filter: {ownership: 'everything'}, format: :json
-          assigns(:build_lists).should include(@build_list1)
+          expect(assigns(:build_lists)).to include(@build_list1)
-          assigns(:build_lists).should_not include(@build_list2)
+          expect(assigns(:build_lists)).to_not include(@build_list2)
-          assigns(:build_lists).should include(@build_list3)
+          expect(assigns(:build_lists)).to include(@build_list3)
-          assigns(:build_lists).should include(@build_list4)
+          expect(assigns(:build_lists)).to include(@build_list4)
         end
       end
 
@@ -393,15 +397,15 @@ describe Projects::BuildListsController, type: :controller do
 
         it 'should be able to perform index action' do
           get :index
-          response.should be_success
+          expect(response).to be_success
         end
 
         it 'should show only accessible build_lists' do
-          get :index, filter: {ownership: 'everything'}
+          get :index, filter: {ownership: 'everything'}, format: :json
-          assigns(:build_lists).should include(@build_list1)
+          expect(assigns(:build_lists)).to include(@build_list1)
-          assigns(:build_lists).should_not include(@build_list2)
+          expect(assigns(:build_lists)).to_not include(@build_list2)
-          assigns(:build_lists).should include(@build_list3)
+          expect(assigns(:build_lists)).to include(@build_list3)
-          assigns(:build_lists).should include(@build_list4)
+          expect(assigns(:build_lists)).to include(@build_list4)
         end
       end
 
@@ -462,26 +466,26 @@ describe Projects::BuildListsController, type: :controller do
 
     it 'should filter by id' do
       get :index, filter: {id: @build_list1.id, project_name: 'fdsfdf', any_other_field: 'do not matter'}, format: :json
-      assigns[:build_lists].should include(@build_list1)
+      expect(assigns[:build_lists]).to include(@build_list1)
-      assigns[:build_lists].should_not include(@build_list2)
+      expect(assigns[:build_lists]).to_not include(@build_list2)
-      assigns[:build_lists].should_not include(@build_list3)
+      expect(assigns[:build_lists]).to_not include(@build_list3)
     end
 
     it 'should filter by project_name' do
       # Project.where(id: build_list2.project.id).update_all(name: 'project_name')
       get :index, filter: {project_name: @build_list2.project.name, ownership: 'everything'}, format: :json
-      assigns[:build_lists].should_not include(@build_list1)
+      expect(assigns[:build_lists]).to_not include(@build_list1)
-      assigns[:build_lists].should include(@build_list2)
+      expect(assigns[:build_lists]).to include(@build_list2)
-      assigns[:build_lists].should_not include(@build_list3)
+      expect(assigns[:build_lists]).to_not include(@build_list3)
     end
 
     it 'should filter by project_name and update_date' do
       get :index, filter: {project_name: @build_list3.project.name, ownership: 'everything',
                             "updated_at_start" => @build_list3.updated_at.strftime('%d/%m/%Y')}, format: :json
-      assigns[:build_lists].should_not include(@build_list1)
+      expect(assigns[:build_lists]).to_not include(@build_list1)
-      assigns[:build_lists].should_not include(@build_list2)
+      expect(assigns[:build_lists]).to_not include(@build_list2)
-      assigns[:build_lists].should include(@build_list3)
+      expect(assigns[:build_lists]).to include(@build_list3)
-      assigns[:build_lists].should_not include(@build_list4)
+      expect(assigns[:build_lists]).to_not include(@build_list4)
     end
   end