From 4d66019e576d9ea1d168c3dc1cbb313f578a7e38 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= <jarka@gitlab.com>
Date: Wed, 27 May 2020 13:56:26 +0200
Subject: [PATCH] Redirect to the referrer when updating a service

- if a referrer is not empty
---
 .../projects/services_controller.rb           |  5 ++-
 app/views/projects/services/_form.html.haml   |  1 +
 .../projects/services_controller_spec.rb      | 40 +++++++++++++++----
 .../projects/services/_form.haml_spec.rb      |  4 +-
 4 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index 92c6ce324f7..5ed9baf04f6 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -2,6 +2,7 @@
 
 class Projects::ServicesController < Projects::ApplicationController
   include ServiceParams
+  include InternalRedirect
 
   # Authorize
   before_action :authorize_admin_project!
@@ -26,8 +27,8 @@ class Projects::ServicesController < Projects::ApplicationController
     respond_to do |format|
       format.html do
         if saved
-          redirect_to project_settings_integrations_path(@project),
-            notice: success_message
+          target_url = safe_redirect_path(params[:redirect_to]).presence || project_settings_integrations_path(@project)
+          redirect_to target_url, notice: success_message
         else
           render 'edit'
         end
diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml
index 05c49a7b6ce..e6761807409 100644
--- a/app/views/projects/services/_form.html.haml
+++ b/app/views/projects/services/_form.html.haml
@@ -13,6 +13,7 @@
     = form_for(@service, as: :service, url: scoped_integration_path(@service), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'can-test' => @service.can_test?, 'test-url' => test_project_service_path(@project, @service) } }) do |form|
       = render 'shared/service_settings', form: form, service: @service
       .footer-block.row-content-block
+        %input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referrer }
         = service_save_button
         &nbsp;
         = link_to _('Cancel'), project_settings_integrations_path(@project), class: 'btn btn-cancel'
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb
index c669119fa4e..b591e52d45c 100644
--- a/spec/controllers/projects/services_controller_spec.rb
+++ b/spec/controllers/projects/services_controller_spec.rb
@@ -134,24 +134,50 @@ describe Projects::ServicesController do
   describe 'PUT #update' do
     describe 'as HTML' do
       let(:service_params) { { active: true } }
+      let(:params)         { project_params(service: service_params) }
+
+      let(:message) { 'Jira activated.' }
+      let(:redirect_url) { project_settings_integrations_path(project) }
 
       before do
-        put :update, params: project_params(service: service_params)
+        put :update, params: params
+      end
+
+      shared_examples 'service update' do
+        it 'redirects to the correct url with a flash message' do
+          expect(response).to redirect_to(redirect_url)
+          expect(flash[:notice]).to eq(message)
+        end
       end
 
       context 'when param `active` is set to true' do
-        it 'activates the service and redirects to integrations paths' do
-          expect(response).to redirect_to(project_settings_integrations_path(project))
-          expect(flash[:notice]).to eq 'Jira activated.'
+        let(:params) { project_params(service: service_params, redirect_to: redirect) }
+
+        context 'when redirect_to param is present' do
+          let(:redirect)     { '/redirect_here' }
+          let(:redirect_url) { redirect }
+
+          it_behaves_like 'service update'
+        end
+
+        context 'when redirect_to is an external domain' do
+          let(:redirect) { 'http://examle.com' }
+
+          it_behaves_like 'service update'
+        end
+
+        context 'when redirect_to param is an empty string' do
+          let(:redirect) { '' }
+
+          it_behaves_like 'service update'
         end
       end
 
       context 'when param `active` is set to false' do
         let(:service_params) { { active: false } }
+        let(:message)        { 'Jira settings saved, but not activated.' }
 
-        it 'does not activate the service but saves the settings' do
-          expect(flash[:notice]).to eq 'Jira settings saved, but not activated.'
-        end
+        it_behaves_like 'service update'
       end
     end
 
diff --git a/spec/views/projects/services/_form.haml_spec.rb b/spec/views/projects/services/_form.haml_spec.rb
index a3faa92b50e..720e0aaf450 100644
--- a/spec/views/projects/services/_form.haml_spec.rb
+++ b/spec/views/projects/services/_form.haml_spec.rb
@@ -15,7 +15,8 @@ describe 'projects/services/_form' do
 
     allow(view).to receive_messages(current_user: user,
                                     can?: true,
-                                    current_application_settings: Gitlab::CurrentSettings.current_application_settings)
+                                    current_application_settings: Gitlab::CurrentSettings.current_application_settings,
+                                    request: double(referrer: '/services'))
   end
 
   context 'commit_events and merge_request_events' do
@@ -30,6 +31,7 @@ describe 'projects/services/_form' do
 
       expect(rendered).to have_content('Event will be triggered when a commit is created/updated')
       expect(rendered).to have_content('Event will be triggered when a merge request is created/updated/merged')
+      expect(rendered).to have_css("input[name='redirect_to'][value='/services']", count: 1, visible: false)
     end
   end
 end
-- 
2.30.9