Merge branch 'justin_ho-fix-old-integration-settings' into 'master'

Move old integrations to Settings > General and introduce instance-level integrations

See merge request gitlab-org/gitlab!38488

app/controllers/admin/application_settings_controller.rb

@@ -16,7 +16,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
     push_frontend_feature_flag(:ci_instance_variables_ui, default_enabled: true)
   end
 
-  VALID_SETTING_PANELS = %w(general integrations repository
+  VALID_SETTING_PANELS = %w(general repository
                             ci_cd reporting metrics_and_profiling
                             network preferences).freeze
 
@@ -32,12 +32,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   end
 
   def integrations
-    if Feature.enabled?(:instance_level_integrations)
-      @integrations = Service.find_or_initialize_instances.sort_by(&:title)
-    else
-      set_application_setting
-      perform_update if submitted?
-    end
+    @integrations = Service.find_or_initialize_instances.sort_by(&:title)
   end
 
   def update

app/controllers/admin/integrations_controller.rb

@@ -12,7 +12,7 @@ class Admin::IntegrationsController < Admin::ApplicationController
   end
 
   def integrations_enabled?
-    Feature.enabled?(:instance_level_integrations)
+    true
   end
 
   def scoped_edit_integration_path(integration)

app/views/admin/application_settings/_eks.html.haml

@@ -9,7 +9,7 @@
       = _('Amazon EKS integration allows you to provision EKS clusters from GitLab.')
 
   .settings-content
-    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-eks-settings'), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-eks-settings'), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting)
 
       %fieldset

app/views/admin/application_settings/_plantuml.html.haml

@@ -8,7 +8,7 @@
     %p
       = _('Allow rendering of PlantUML diagrams in Asciidoc documents.')
   .settings-content
-    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-plantuml-settings'), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-plantuml-settings'), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting) if expanded
 
       %fieldset

app/views/admin/application_settings/_snowplow.html.haml

@@ -8,8 +8,7 @@
     %p
       = _('Configure the %{link} integration.').html_safe % { link: link_to('Snowplow', 'https://snowplowanalytics.com/', target: '_blank') }
   .settings-content
-
-    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-snowplow-settings'), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-snowplow-settings'), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting) if expanded
 
       %fieldset

app/views/admin/application_settings/_sourcegraph.html.haml

@@ -16,7 +16,7 @@
 
 
   .settings-content
-    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-sourcegraph-settings'), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-sourcegraph-settings'), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting)
 
       %fieldset

app/views/admin/application_settings/_third_party_offers.html.haml

@@ -8,8 +8,7 @@
     %p
       = _('Control the display of third party offers.')
   .settings-content
-
-    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-third-party-offers-settings'), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-third-party-offers-settings'), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting) if expanded
 
       %fieldset

app/views/admin/application_settings/general.html.haml

@@ -116,11 +116,10 @@
     .settings-content
       #js-maintenance-mode-settings
 
-- if Feature.enabled?(:instance_level_integrations)
-  = render_if_exists 'admin/application_settings/elasticsearch_form'
-  = render 'admin/application_settings/plantuml'
-  = render 'admin/application_settings/sourcegraph'
-  = render_if_exists 'admin/application_settings/slack'
-  = render 'admin/application_settings/third_party_offers'
-  = render 'admin/application_settings/snowplow'
-  = render 'admin/application_settings/eks'
+= render_if_exists 'admin/application_settings/elasticsearch_form'
+= render 'admin/application_settings/plantuml'
+= render 'admin/application_settings/sourcegraph'
+= render_if_exists 'admin/application_settings/slack'
+= render 'admin/application_settings/third_party_offers'
+= render 'admin/application_settings/snowplow'
+= render 'admin/application_settings/eks'

app/views/admin/application_settings/integrations.html.haml

@@ -2,29 +2,19 @@
 - page_title _('Integrations')
 - @content_class = 'limit-container-width' unless fluid_layout
 
-- if Feature.enabled?(:instance_level_integrations)
-  - if show_admin_integrations_moved?
-    .gl-alert.gl-alert-info.js-admin-integrations-moved.mt-3{ role: 'alert', data: { feature_id: UserCalloutsHelper::ADMIN_INTEGRATIONS_MOVED, dismiss_endpoint: user_callouts_path } }
-      = sprite_icon('information-o', css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
-      %button.js-close.gl-alert-dismiss{ type: 'button', 'aria-label' => _('Dismiss') }
-        = sprite_icon('close', css_class: 'gl-icon')
-      .gl-alert-body
-        %h4.gl-alert-title= s_('AdminSettings|Some settings have moved')
-        = html_escape_once(s_('AdminSettings|Elasticsearch, PlantUML, Slack application, Third party offers, Snowplow, Amazon EKS have moved to Settings > General.')).html_safe
-      .gl-alert-actions
-        = link_to s_('AdminSettings|Go to General Settings'), general_admin_application_settings_path, class: 'btn gl-alert-action btn-info new-gl-button'
+- if show_admin_integrations_moved?
+  .gl-alert.gl-alert-info.js-admin-integrations-moved.mt-3{ role: 'alert', data: { feature_id: UserCalloutsHelper::ADMIN_INTEGRATIONS_MOVED, dismiss_endpoint: user_callouts_path } }
+    = sprite_icon('information-o', css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
+    %button.js-close.gl-alert-dismiss{ type: 'button', 'aria-label' => _('Dismiss') }
+      = sprite_icon('close', css_class: 'gl-icon')
+    .gl-alert-body
+      %h4.gl-alert-title= s_('AdminSettings|Some settings have moved')
+      = html_escape_once(s_('AdminSettings|Elasticsearch, PlantUML, Slack application, Third party offers, Snowplow, Amazon EKS have moved to Settings > General.')).html_safe
+    .gl-alert-actions
+      = link_to s_('AdminSettings|Go to General Settings'), general_admin_application_settings_path, class: 'btn gl-alert-action btn-info new-gl-button'
 
-  %h4= s_('AdminSettings|Apply integration settings to all Projects')
-  %p
-    = s_('AdminSettings|Integrations configured here will automatically apply to all projects on this instance.')
-    = link_to _('Learn more'), '#'
-  = render 'shared/integrations/index', integrations: @integrations
-
-- else
-  = render_if_exists 'admin/application_settings/elasticsearch_form'
-  = render 'admin/application_settings/plantuml'
-  = render 'admin/application_settings/sourcegraph'
-  = render_if_exists 'admin/application_settings/slack'
-  = render 'admin/application_settings/third_party_offers'
-  = render 'admin/application_settings/snowplow'
-  = render 'admin/application_settings/eks'
+%h4= s_('AdminSettings|Apply integration settings to all Projects')
+%p
+  = s_('AdminSettings|Integrations configured here will automatically apply to all projects on this instance.')
+  = link_to _('Learn more'), '#'
+= render 'shared/integrations/index', integrations: @integrations

changelogs/unreleased/justin_ho-fix-old-integration-settings.yml

+---
+title: Move old integrations to Settings > General and introduce instance-level integrations
+merge_request: 38488
+author:
+type: added

ee/app/controllers/admin/elasticsearch_controller.rb

@@ -14,7 +14,7 @@ class Admin::ElasticsearchController < Admin::ApplicationController
       flash[:warning] = _('Please create an index before enabling indexing')
     end
 
-    redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+    redirect_to redirect_path
   end
 
   # POST
@@ -27,6 +27,12 @@ class Admin::ElasticsearchController < Admin::ApplicationController
       flash[:notice] = _('Elasticsearch reindexing triggered')
     end
 
-    redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+    redirect_to redirect_path
+  end
+
+  private
+
+  def redirect_path
+    general_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
   end
 end

ee/app/views/admin/application_settings/_elasticsearch_form.html.haml

@@ -14,8 +14,7 @@
     %p
       = _('Elasticsearch integration. Elasticsearch AWS IAM.')
   .settings-content
-
-    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings'), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-elasticsearch-settings'), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting) if expanded
 
       %fieldset

ee/app/views/admin/application_settings/_slack.html.haml

@@ -10,8 +10,7 @@
     %p
       = _('Slack integration allows you to interact with GitLab via slash commands in a chat window.')
   .settings-content
-
-    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-slack-settings'), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-slack-settings'), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting) if expanded
 
       %fieldset
@@ -33,4 +32,3 @@
           = f.text_field :slack_app_verification_token, class: 'form-control'
 
       = f.submit _('Save changes'), class: "btn btn-success"
-

ee/app/views/admin/dashboard/_elastic_and_geo.html.haml

 = feature_entry(_('Elasticsearch'),
-  href: integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings'),
+  href: general_admin_application_settings_path(anchor: 'js-elasticsearch-settings'),
   doc_href: help_page_path('integration/elasticsearch'),
   enabled: Gitlab::CurrentSettings.elasticsearch_search?)
 

ee/spec/controllers/admin/elasticsearch_controller_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe Admin::ElasticsearchController do
       post :enqueue_index
 
       expect(controller).to set_flash[:notice].to include('/admin/sidekiq/queues/elastic_full_index')
-      expect(response).to redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+      expect(response).to redirect_to general_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
     end
 
     context 'without an index' do
@@ -33,7 +33,7 @@ RSpec.describe Admin::ElasticsearchController do
         post :enqueue_index
 
         expect(controller).to set_flash[:warning].to include('create an index before enabling indexing')
-        expect(response).to redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+        expect(response).to redirect_to general_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
       end
     end
   end
@@ -49,7 +49,7 @@ RSpec.describe Admin::ElasticsearchController do
       post :trigger_reindexing
 
       expect(controller).to set_flash[:notice].to include('reindexing triggered')
-      expect(response).to redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+      expect(response).to redirect_to general_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
     end
 
     it 'does not create a reindexing task if there is another one' do
@@ -59,7 +59,7 @@ RSpec.describe Admin::ElasticsearchController do
       post :trigger_reindexing
 
       expect(controller).to set_flash[:warning].to include('already in progress')
-      expect(response).to redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
+      expect(response).to redirect_to general_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
     end
   end
 end

ee/spec/features/admin/admin_settings_spec.rb

@@ -6,7 +6,6 @@ RSpec.describe 'Admin updates EE-only settings' do
   include StubENV
 
   before do
-    stub_feature_flags(instance_level_integrations: false)
     stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
     sign_in(create(:admin))
     allow(License).to receive(:feature_available?).and_return(true)
@@ -53,7 +52,7 @@ RSpec.describe 'Admin updates EE-only settings' do
 
   context 'Elasticsearch settings' do
     before do
-      visit integrations_admin_application_settings_path
+      visit general_admin_application_settings_path
       page.within('.as-elasticsearch') do
         click_button 'Expand'
       end
@@ -115,7 +114,7 @@ RSpec.describe 'Admin updates EE-only settings' do
       end
 
       page.within('#select2-drop') do
-        expect(page).to have_content(project.full_path)
+        expect(page).to have_content(project.name_with_namespace)
       end
 
       page.within('.as-elasticsearch') do
@@ -135,7 +134,7 @@ RSpec.describe 'Admin updates EE-only settings' do
       namespace = create(:elasticsearch_indexed_namespace).namespace
       project = create(:elasticsearch_indexed_project).project
 
-      visit integrations_admin_application_settings_path
+      visit general_admin_application_settings_path
 
       expect(ElasticsearchIndexedNamespace.count).to be > 0
       expect(ElasticsearchIndexedProject.count).to be > 0
@@ -165,7 +164,7 @@ RSpec.describe 'Admin updates EE-only settings' do
 
   it 'Enable Slack application' do
     allow(Gitlab).to receive(:com?).and_return(true)
-    visit integrations_admin_application_settings_path
+    visit general_admin_application_settings_path
 
     page.within('.as-slack') do
       check 'Enable Slack application'

qa/qa/ee.rb

@@ -71,7 +71,7 @@ module QA
 
         module Settings
           autoload :Templates, 'qa/ee/page/admin/settings/templates'
-          autoload :Integration, 'qa/ee/page/admin/settings/integration'
+          autoload :Elasticsearch, 'qa/ee/page/admin/settings/elasticsearch'
           autoload :Preferences, 'qa/ee/page/admin/settings/preferences'
 
           module Component

qa/qa/ee/page/admin/settings/integration.rb → qa/qa/ee/page/admin/settings/elasticsearch.rb

@@ -5,7 +5,7 @@ module QA
     module Page
       module Admin
         module Settings
-          class Integration < QA::Page::Base
+          class Elasticsearch < QA::Page::Base
             include QA::Page::Settings::Common
 
             view 'ee/app/views/admin/application_settings/_elasticsearch_form.html.haml' do

qa/qa/ee/resource/settings/elasticsearch.rb

@@ -17,9 +17,9 @@ module QA
 
           def fabricate!
             QA::Page::Main::Menu.perform(&:go_to_admin_area)
-            QA::Page::Admin::Menu.perform(&:go_to_integration_settings)
-            QA::EE::Page::Admin::Settings::Integration.perform do |integration|
-              integration.expand_elasticsearch do |es|
+            QA::Page::Admin::Menu.perform(&:go_to_general_settings)
+            QA::EE::Page::Admin::Settings::Elasticsearch.perform do |settings|
+              settings.expand_elasticsearch do |es|
                 es.check_indexing if @es_indexing
                 es.check_search if @es_enabled
                 es.enter_link(@es_url)

spec/controllers/admin/application_settings_controller_spec.rb

@@ -142,23 +142,12 @@ RSpec.describe Admin::ApplicationSettingsController do
     end
 
     describe 'verify panel actions' do
-      before do
-        stub_feature_flags(instance_level_integrations: false)
-      end
-
       Admin::ApplicationSettingsController::VALID_SETTING_PANELS.each do |valid_action|
         it_behaves_like 'renders correct panels' do
           let(:action) { valid_action }
         end
       end
     end
-  end
-
-  describe 'PATCH #integrations' do
-    before do
-      stub_feature_flags(instance_level_integrations: false)
-      sign_in(admin)
-    end
 
     describe 'EKS integration' do
       let(:application_setting) { ApplicationSetting.current }
@@ -172,7 +161,7 @@ RSpec.describe Admin::ApplicationSettingsController do
       end
 
       it 'updates EKS settings' do
-        patch :integrations, params: { application_setting: settings_params }
+        put :update, params: { application_setting: settings_params }
 
         expect(application_setting.eks_integration_enabled).to be_truthy
         expect(application_setting.eks_account_id).to eq '123456789012'
@@ -186,7 +175,7 @@ RSpec.describe Admin::ApplicationSettingsController do
         it 'does not update the secret key' do
           application_setting.update!(eks_secret_access_key: 'dummy secret key')
 
-          patch :integrations, params: { application_setting: settings_params }
+          put :update, params: { application_setting: settings_params }
 
           expect(application_setting.reload.eks_secret_access_key).to eq 'dummy secret key'
         end

spec/controllers/admin/integrations_controller_spec.rb

@@ -10,16 +10,6 @@ RSpec.describe Admin::IntegrationsController do
   end
 
   describe '#edit' do
-    context 'when instance_level_integrations not enabled' do
-      it 'returns not_found' do
-        stub_feature_flags(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

spec/features/admin/admin_settings_spec.rb

@@ -209,8 +209,7 @@ RSpec.describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_n
 
     context 'Integrations page' do
       before do
-        stub_feature_flags(instance_level_integrations: false)
-        visit integrations_admin_application_settings_path
+        visit general_admin_application_settings_path
       end
 
       it 'Enable hiding third party offers' do

spec/features/projects/clusters/gcp_spec.rb

@@ -205,11 +205,10 @@ RSpec.describe 'Gcp Cluster', :js, :do_not_mock_admin_mode do
     let(:admin) { create(:admin) }
 
     before do
-      stub_feature_flags(instance_level_integrations: false)
       stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
       sign_in(admin)
       gitlab_enable_admin_mode_sign_in(admin)
-      visit integrations_admin_application_settings_path
+      visit general_admin_application_settings_path
     end
 
     it 'user does not see the offer' do