From 4899dfcafe55ca2935d699f85c13fc35a8e16545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eva=20Kadlecova=CC=81?= Date: Sun, 26 Aug 2018 17:57:51 +0200 Subject: [PATCH] Redirect to the pipeline builds page when a build is canceled --- app/controllers/projects/jobs_controller.rb | 8 ++- app/views/projects/ci/builds/_build.html.haml | 2 +- .../unreleased/Fix-pipeline-redirect.yml | 5 ++ .../projects/jobs_controller_spec.rb | 65 ++++++++++++++----- 4 files changed, 62 insertions(+), 18 deletions(-) create mode 100644 changelogs/unreleased/Fix-pipeline-redirect.yml diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index e69faae754a..009d8afd523 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -1,5 +1,6 @@ class Projects::JobsController < Projects::ApplicationController include SendFileUpload + include ContinueParams before_action :build, except: [:index, :cancel_all] before_action :authorize_read_build! @@ -101,7 +102,12 @@ class Projects::JobsController < Projects::ApplicationController return respond_422 unless @build.cancelable? @build.cancel - redirect_to build_path(@build) + + if continue_params + redirect_to continue_params[:to] + else + redirect_to builds_project_pipeline_path(@project, @build.pipeline.id) + end end def status diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 44c1453e239..44446f15ddc 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -99,7 +99,7 @@ = sprite_icon('download') - if can?(current_user, :update_build, job) - if job.active? - = link_to cancel_project_job_path(job.project, job, return_to: request.original_url), method: :post, title: 'Cancel', class: 'btn btn-build' do + = link_to cancel_project_job_path(job.project, job, continue: { to: request.fullpath }), method: :post, title: 'Cancel', class: 'btn btn-build' do = icon('remove', class: 'cred') - elsif allow_retry - if job.playable? && !admin && can?(current_user, :update_build, job) diff --git a/changelogs/unreleased/Fix-pipeline-redirect.yml b/changelogs/unreleased/Fix-pipeline-redirect.yml new file mode 100644 index 00000000000..459273c7740 --- /dev/null +++ b/changelogs/unreleased/Fix-pipeline-redirect.yml @@ -0,0 +1,5 @@ +--- +title: Redirect to the pipeline builds page when a build is canceled +merge_request: +author: Eva Kadlecova +type: fixed diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 1aca44c6e74..44c4d3ac3ce 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -356,35 +356,68 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do before do project.add_developer(user) sign_in(user) - - post_cancel end - context 'when job is cancelable' do + context 'when continue url is present' do let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) } - it 'redirects to the canceled job page' do - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(namespace_project_job_path(id: job.id)) + context 'when continue to is a safe url' do + let(:url) { '/test' } + + before do + post_cancel(continue: { to: url }) + end + + it 'redirects to the continue url' do + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(url) + end + + it 'transits to canceled' do + expect(job.reload).to be_canceled + end end - it 'transits to canceled' do - expect(job.reload).to be_canceled + context 'when continue to is not a safe url' do + let(:url) { 'http://example.com' } + + it 'raises an error' do + expect { cancel_with_redirect(url) }.to raise_error + end end end - context 'when job is not cancelable' do - let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } + context 'when continue url is not present' do + before do + post_cancel + end - it 'returns unprocessable_entity' do - expect(response).to have_gitlab_http_status(:unprocessable_entity) + context 'when job is cancelable' do + let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) } + + it 'redirects to the builds page' do + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(builds_namespace_project_pipeline_path(id: pipeline.id)) + end + + it 'transits to canceled' do + expect(job.reload).to be_canceled + end + end + + context 'when job is not cancelable' do + let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } + + it 'returns unprocessable_entity' do + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end end end - def post_cancel - post :cancel, namespace_id: project.namespace, - project_id: project, - id: job.id + def post_cancel(additional_params = {}) + post :cancel, { namespace_id: project.namespace, + project_id: project, + id: job.id }.merge(additional_params) end end