diff --git a/app/controllers/projects/build_lists_controller.rb b/app/controllers/projects/build_lists_controller.rb index 1d79f05c8..df7e61ce6 100644 --- a/app/controllers/projects/build_lists_controller.rb +++ b/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] - - 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 :load_build_list, 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]) diff --git a/app/policies/build_list_policy.rb b/app/policies/build_list_policy.rb index 67be69299..ab757cf58 100644 --- a/app/policies/build_list_policy.rb +++ b/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? diff --git a/spec/controllers/projects/build_lists_controller_spec.rb b/spec/controllers/projects/build_lists_controller_spec.rb index 66bc1cffb..ff5145ebb 100644 --- a/spec/controllers/projects/build_lists_controller_spec.rb +++ b/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'} - assigns(:build_lists).should include(@build_list1) - assigns(:build_lists).should_not include(@build_list2) - assigns(:build_lists).should include(@build_list3) - assigns(:build_lists).should include(@build_list4) + get :index, filter: {ownership: 'everything'}, format: :json + expect(assigns(:build_lists)).to include(@build_list1) + expect(assigns(:build_lists)).to_not include(@build_list2) + expect(assigns(:build_lists)).to include(@build_list3) + 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'} - assigns(:build_lists).should include(@build_list1) - assigns(:build_lists).should_not include(@build_list2) - assigns(:build_lists).should include(@build_list3) - assigns(:build_lists).should include(@build_list4) + get :index, filter: {ownership: 'everything'}, format: :json + expect(assigns(:build_lists)).to include(@build_list1) + expect(assigns(:build_lists)).to_not include(@build_list2) + expect(assigns(:build_lists)).to include(@build_list3) + 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) - assigns[:build_lists].should_not include(@build_list2) - assigns[:build_lists].should_not include(@build_list3) + expect(assigns[:build_lists]).to include(@build_list1) + expect(assigns[:build_lists]).to_not include(@build_list2) + 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) - assigns[:build_lists].should include(@build_list2) - assigns[:build_lists].should_not include(@build_list3) + expect(assigns[:build_lists]).to_not include(@build_list1) + expect(assigns[:build_lists]).to include(@build_list2) + 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) - assigns[:build_lists].should_not include(@build_list2) - assigns[:build_lists].should include(@build_list3) - assigns[:build_lists].should_not include(@build_list4) + expect(assigns[:build_lists]).to_not include(@build_list1) + expect(assigns[:build_lists]).to_not include(@build_list2) + expect(assigns[:build_lists]).to include(@build_list3) + expect(assigns[:build_lists]).to_not include(@build_list4) end end