Add feature flag to guard controller

- Use feature flag to render 404 Not Found while
feature is WIP
- Refactor private methods to be more readable
- Apply the refactor to projects/services_controller

app/controllers/admin/integrations_controller.rb

@@ -3,6 +3,7 @@
 class Admin::IntegrationsController < Admin::ApplicationController
   include ServiceParams
 
+  before_action :not_found, unless: :instance_level_integrations_enabled?
   before_action :service, only: [:edit, :update, :test]
 
   def edit
@@ -28,6 +29,10 @@ class Admin::IntegrationsController < Admin::ApplicationController
 
   private
 
+  def instance_level_integrations_enabled?
+    Feature.enabled?(:instance_level_integrations)
+  end
+
   def project
     # TODO: Change to something more meaningful
     Project.first
@@ -38,26 +43,24 @@ class Admin::IntegrationsController < Admin::ApplicationController
   end
 
   def success_message
-    if @service.active?
-      _('%{service_title} activated.') % { service_title: @service.title }
-    else
-      _('%{service_title} settings saved, but not activated.') % { service_title: @service.title }
-    end
+    message = @service.active? ? _('activated') : _('settings saved, but not activated')
+
+    _('%{service_title} %{message}.') % { service_title: @service.title, message: message }
   end
 
   def service_test_response
-    if @service.update(service_params[:service])
-      data = @service.test_data(project, current_user)
-      outcome = @service.test(data)
-
-      if outcome[:success]
-        {}
-      else
-        { error: true, message: _('Test failed.'), service_response: outcome[:result].to_s, test_failed: true }
-      end
-    else
-      { error: true, message: _('Validations failed.'), service_response: @service.errors.full_messages.join(','), test_failed: false }
+    unless @service.update(service_params[:service])
+      return { error: true, message: _('Validations failed.'), service_response: @service.errors.full_messages.join(','), test_failed: false }
     end
+
+    data = @service.test_data(project, current_user)
+    outcome = @service.test(data)
+
+    unless outcome[:success]
+      return { error: true, message: _('Test failed.'), service_response: outcome[:result].to_s, test_failed: true }
+    end
+
+    {}
   rescue Gitlab::HTTP::BlockedUrlError => e
     { error: true, message: _('Test failed.'), service_response: e.message, test_failed: true }
   end

app/controllers/projects/services_controller.rb

@@ -52,28 +52,26 @@ class Projects::ServicesController < Projects::ApplicationController
   private
 
   def service_test_response
-    if @service.update(service_params[:service])
-      data = @service.test_data(project, current_user)
-      outcome = @service.test(data)
-
-      if outcome[:success]
-        {}
-      else
-        { error: true, message: _('Test failed.'), service_response: outcome[:result].to_s, test_failed: true }
-      end
-    else
-      { error: true, message: _('Validations failed.'), service_response: @service.errors.full_messages.join(','), test_failed: false }
+    unless @service.update(service_params[:service])
+      return { error: true, message: _('Validations failed.'), service_response: @service.errors.full_messages.join(','), test_failed: false }
+    end
+
+    data = @service.test_data(project, current_user)
+    outcome = @service.test(data)
+
+    unless outcome[:success]
+      return { error: true, message: _('Test failed.'), service_response: outcome[:result].to_s, test_failed: true }
     end
+
+    {}
   rescue Gitlab::HTTP::BlockedUrlError => e
     { error: true, message: _('Test failed.'), service_response: e.message, test_failed: true }
   end
 
   def success_message
-    if @service.active?
-      _("%{service_title} activated.") % { service_title: @service.title }
-    else
-      _("%{service_title} settings saved, but not activated.") % { service_title: @service.title }
-    end
+    message = @service.active? ? _('activated') : _('settings saved, but not activated')
+
+    _('%{service_title} %{message}.') % { service_title: @service.title, message: message }
   end
 
   def service

locale/gitlab.pot

@@ -401,10 +401,7 @@ msgstr ""
 msgid "%{screenreaderOnlyStart}Keyboard shorcuts%{screenreaderOnlyEnd} Enabled"
 msgstr ""
 
-msgid "%{service_title} activated."
-msgstr ""
-
-msgid "%{service_title} settings saved, but not activated."
+msgid "%{service_title} %{message}."
 msgstr ""
 
 msgid "%{size} GiB"
@@ -23165,6 +23162,9 @@ msgid_plural "about %d hours"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "activated"
+msgstr ""
+
 msgid "added %{created_at_timeago}"
 msgstr ""
 
@@ -24220,6 +24220,9 @@ msgstr ""
 msgid "security Reports|There was an error creating the merge request"
 msgstr ""
 
+msgid "settings saved, but not activated"
+msgstr ""
+
 msgid "severity|Critical"
 msgstr ""
 

spec/controllers/admin/integrations_controller_spec.rb

@@ -11,6 +11,16 @@ describe Admin::IntegrationsController do
   end
 
   describe '#edit' do
+    context 'when instance_level_integrations not enabled' do
+      it 'returns not_found' do
+        allow(Feature).to receive(:enabled?).with(:instance_level_integrations) { false }
+
+        get :edit, params: { id: Service.available_services_names.sample }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
     Service.available_services_names.each do |integration_name|
       context "#{integration_name}" do
         it 'successfully displays the template' do