Commit f6966cfa authored by Stan Hu's avatar Stan Hu

Address some comments with running a pipeline schedule

parent 8e7f19c6
class Projects::PipelineSchedulesController < Projects::ApplicationController class Projects::PipelineSchedulesController < Projects::ApplicationController
before_action :schedule, except: [:index, :new, :create] before_action :schedule, except: [:index, :new, :create]
before_action :authorize_create_pipeline!, only: [:run] before_action :authorize_create_pipeline!, only: [:play]
before_action :authorize_read_pipeline_schedule! before_action :authorize_read_pipeline_schedule!
before_action :authorize_create_pipeline_schedule!, only: [:new, :create] before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :run] before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :play]
before_action :authorize_admin_pipeline_schedule!, only: [:destroy] before_action :authorize_admin_pipeline_schedule!, only: [:destroy]
def index def index
...@@ -41,7 +41,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController ...@@ -41,7 +41,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
end end
end end
def run def play
job_id = RunPipelineScheduleWorker.perform_async(schedule.id, current_user.id) job_id = RunPipelineScheduleWorker.perform_async(schedule.id, current_user.id)
flash[:notice] = flash[:notice] =
......
...@@ -182,9 +182,9 @@ module GitlabRoutingHelper ...@@ -182,9 +182,9 @@ module GitlabRoutingHelper
edit_project_pipeline_schedule_path(project, schedule) edit_project_pipeline_schedule_path(project, schedule)
end end
def run_pipeline_schedule_path(schedule, *args) def play_pipeline_schedule_path(schedule, *args)
project = schedule.project project = schedule.project
run_project_pipeline_schedule_path(project, schedule, *args) play_project_pipeline_schedule_path(project, schedule, *args)
end end
def take_ownership_pipeline_schedule_path(schedule, *args) def take_ownership_pipeline_schedule_path(schedule, *args)
......
...@@ -5,8 +5,10 @@ class RunPipelineScheduleWorker ...@@ -5,8 +5,10 @@ class RunPipelineScheduleWorker
enqueue_in group: :creation enqueue_in group: :creation
def perform(schedule_id, user_id) def perform(schedule_id, user_id)
schedule = Ci::PipelineSchedule.find(schedule_id) schedule = Ci::PipelineSchedule.find_by(id: schedule_id)
user = User.find(user_id) user = User.find_by(id: user_id)
return unless schedule && user
run_pipeline_schedule(schedule, user) run_pipeline_schedule(schedule, user)
end end
......
...@@ -179,7 +179,7 @@ constraints(ProjectUrlConstrainer.new) do ...@@ -179,7 +179,7 @@ constraints(ProjectUrlConstrainer.new) do
resources :pipeline_schedules, except: [:show] do resources :pipeline_schedules, except: [:show] do
member do member do
post :run post :play
post :take_ownership post :take_ownership
end end
end end
......
...@@ -364,7 +364,7 @@ describe Projects::PipelineSchedulesController do ...@@ -364,7 +364,7 @@ describe Projects::PipelineSchedulesController do
end end
end end
describe 'POST #run' do describe 'POST #play' do
set(:user) { create(:user) } set(:user) { create(:user) }
context 'when a developer makes the request' do context 'when a developer makes the request' do
...@@ -384,7 +384,7 @@ describe Projects::PipelineSchedulesController do ...@@ -384,7 +384,7 @@ describe Projects::PipelineSchedulesController do
end end
def go def go
post :run, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
end end
end end
......
...@@ -2,27 +2,30 @@ require 'spec_helper' ...@@ -2,27 +2,30 @@ require 'spec_helper'
describe RunPipelineScheduleWorker do describe RunPipelineScheduleWorker do
describe '#perform' do describe '#perform' do
let(:project) { create(:project) } set(:project) { create(:project) }
set(:user) { create(:user) }
set(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) }
let(:worker) { described_class.new } let(:worker) { described_class.new }
let(:user) { create(:user) }
let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) }
context 'when a project not found' do context 'when a project not found' do
it 'does not call the Service' do it 'does not call the Service' do
expect(Ci::CreatePipelineService).not_to receive(:new) expect(Ci::CreatePipelineService).not_to receive(:new)
expect { worker.perform(100000, user.id) }.to raise_error(ActiveRecord::RecordNotFound) expect(worker).not_to receive(:run_pipeline_schedule)
worker.perform(100000, user.id)
end end
end end
context 'when a user not found' do context 'when a user not found' do
it 'does not call the Service' do it 'does not call the Service' do
expect(Ci::CreatePipelineService).not_to receive(:new) expect(Ci::CreatePipelineService).not_to receive(:new)
expect { worker.perform(pipeline_schedule.id, 10000) }.to raise_error(ActiveRecord::RecordNotFound) expect(worker).not_to receive(:run_pipeline_schedule)
worker.perform(pipeline_schedule.id, 10000)
end end
end end
context 'when everything is ok' do context 'when everything is ok' do
let(:project) { create(:project) }
let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) } let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) }
it 'calls the Service' do it 'calls the Service' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment