Merge branch 'admin-integrations-remove-call-to-es' into 'master'

Admin integrations page: remove a call to es

See merge request gitlab-org/gitlab!25724

app/controllers/admin/application_settings_controller.rb

@@ -244,6 +244,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   def render_update_error
     action = valid_setting_panels.include?(action_name) ? action_name : :general
 
+    flash[:alert] = _('Application settings update failed')
+
     render action
   end
 

ee/app/controllers/admin/elasticsearch_controller.rb

@@ -4,17 +4,21 @@ class Admin::ElasticsearchController < Admin::ApplicationController
   before_action :check_elasticsearch_web_indexing_feature_flag!
 
   def check_elasticsearch_web_indexing_feature_flag!
-    render_404 unless Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && Gitlab::Elastic::Helper.index_exists?
+    render_404 unless Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true)
   end
 
   # POST
   # Scheduling indexing jobs
   def enqueue_index
+    if Gitlab::Elastic::Helper.index_exists?
       ::Elastic::IndexProjectsService.new.execute
 
       notice = _('Elasticsearch indexing started')
       queue_link = helpers.link_to(_('(check progress)'), sidekiq_path + '/queues/elastic_full_index')
       flash[:notice] = "#{notice} #{queue_link}".html_safe
+    else
+      flash[:warning] = _('Please create an index before enabling indexing')
+    end
 
     redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
   end

ee/app/services/ee/application_settings/update_service.rb

@@ -8,6 +8,8 @@ module EE
 
       override :execute
       def execute
+        return false if prevent_elasticsearch_indexing_update?
+
         # Repository size limit comes as MB from the view
         limit = params.delete(:repository_size_limit)
         application_setting.repository_size_limit = ::Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
@@ -38,6 +40,14 @@ module EE
         # Add new containers
         new_container_ids.each { |id| klass.create!(klass.target_attr_name => id) }
       end
+
+      private
+
+      def prevent_elasticsearch_indexing_update?
+        !application_setting.elasticsearch_indexing &&
+          params[:elasticsearch_indexing].to_i.positive? &&
+          !::Gitlab::Elastic::Helper.index_exists?
+      end
     end
   end
 end

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

@@ -22,19 +22,16 @@
         .sub-section
           .form-group
             .form-check
-              - indexing_enabled = Gitlab::CurrentSettings.elasticsearch_indexing?
-              - index_exists = Gitlab::Elastic::Helper.index_exists? rescue false
-              - disable_indexing_checkbox = !indexing_enabled && !index_exists # Disable this checkbox only when we haven't enabled indexing and we don't have an index
-              = f.check_box :elasticsearch_indexing, class: 'form-check-input', data: { qa_selector: 'indexing_checkbox' }, disabled: disable_indexing_checkbox
+              = f.check_box :elasticsearch_indexing, class: 'form-check-input', data: { qa_selector: 'indexing_checkbox' }
               = f.label :elasticsearch_indexing, class: 'form-check-label' do
                 Elasticsearch indexing
-              - unless index_exists
-                .form-text.text-warning
+              - unless Gitlab::CurrentSettings.elasticsearch_indexing?
+                .form-text.text-muted
                   = _('Please create an index before enabling indexing')
-              - if Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && indexing_enabled
+              - if Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && Gitlab::CurrentSettings.elasticsearch_indexing?
                 .form-text
                   = link_to _('Index all projects'), admin_elasticsearch_enqueue_index_path,
-                      class: ['btn', 'btn-success', ('disabled' unless index_exists)], method: :post
+                      class: ['btn', 'btn-success'], method: :post
 
           .form-group
             .form-check

ee/spec/controllers/admin/elasticsearch_controller_spec.rb

@@ -32,7 +32,8 @@ describe Admin::ElasticsearchController do
 
         post :enqueue_index
 
-        expect(response).to have_gitlab_http_status(:not_found)
+        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')
       end
     end
 

ee/spec/services/application_settings/update_service_spec.rb

@@ -34,6 +34,30 @@ describe ApplicationSettings::UpdateService do
       end
     end
 
+    context 'index dependent' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:index_exists, :indexing_enabled, :input_value, :result) do
+        false  | false | '1' | false
+        false  | true  | '1' | true
+        true   | false | '1' | true
+        true   | true  | '1' | true
+      end
+
+      with_them do
+        before do
+          allow(Gitlab::Elastic::Helper).to(receive(:index_exists?)).and_return(index_exists)
+          allow(service.application_setting).to(receive(:elasticsearch_indexing)).and_return(indexing_enabled)
+        end
+
+        let(:opts) { { elasticsearch_indexing: input_value } }
+
+        it 'returns correct result' do
+          expect(service.execute).to be(result)
+        end
+      end
+    end
+
     context 'repository_size_limit assignment as Bytes' do
       let(:service) { described_class.new(setting, user, opts) }
 

ee/spec/views/admin/application_settings/_elasticsearch_form.html.haml_spec.rb

@@ -12,53 +12,31 @@ describe 'admin/application_settings/_elasticsearch_form' do
     allow(view).to receive(:expanded) { true }
   end
 
-  context 'es index dependent' do
+  context 'es indexing' do
+    let(:application_setting) { build(:application_setting) }
+    let(:button_text) { 'Index all projects' }
+
     before do
       allow(Gitlab::CurrentSettings).to(receive(:elasticsearch_indexing?)).and_return(es_indexing)
-      allow(Gitlab::Elastic::Helper).to(receive(:index_exists?)).and_return(index_exists)
     end
 
-    let(:warning_msg) { 'create an index before enabling indexing' }
-    let(:button_text) { 'Index all projects' }
-    let(:application_setting) { build(:application_setting, elasticsearch_indexing: es_indexing) }
-
-    context 'when elasticsearch index does not exist with indexing enabled' do
+    context 'indexing is enabled' do
       let(:es_indexing) { true }
-      let(:index_exists) { false }
 
-      it 'shows a warning and disables a button' do
+      it 'hides index button when indexing is disabled' do
         render
 
-        expect(rendered).to have_content(warning_msg)
-        expect(rendered).to have_css('a.btn-success.disabled', text: button_text)
-        expect(rendered).to have_css('#application_setting_elasticsearch_indexing')
-        expect(rendered).not_to have_css('#application_setting_elasticsearch_indexing[disabled="disabled"]')
+        expect(rendered).to have_css('a.btn-success', text: button_text)
       end
     end
 
-    context 'when elasticsearch index does not exist with indexing disabled' do
+    context 'indexing is disabled' do
       let(:es_indexing) { false }
-      let(:index_exists) { false }
 
-      it 'shows a warning and disables a checkbox and hides an indexing button' do
+      it 'shows index button when indexing is enabled' do
         render
 
-        expect(rendered).to have_content(warning_msg)
         expect(rendered).not_to have_css('a.btn-success', text: button_text)
-        expect(rendered).to have_css('#application_setting_elasticsearch_indexing[disabled="disabled"]')
-      end
-    end
-
-    context 'when elasticsearch index exists' do
-      let(:es_indexing) { true }
-      let(:index_exists) { true }
-
-      it 'shows non-disabled index button without a warning' do
-        render
-
-        expect(rendered).to have_css('a.btn-success', text: button_text)
-        expect(rendered).not_to have_css('a.btn-success.disabled', text: button_text)
-        expect(rendered).not_to have_content(warning_msg)
       end
     end
   end

locale/gitlab.pot

@@ -2066,6 +2066,9 @@ msgstr ""
 msgid "Application settings saved successfully"
 msgstr ""
 
+msgid "Application settings update failed"
+msgstr ""
+
 msgid "Application uninstalled but failed to destroy: %{error_message}"
 msgstr ""
 

spec/views/admin/application_settings/integrations.html.haml_spec.rb

@@ -11,7 +11,6 @@ describe 'admin/application_settings/integrations.html.haml' do
     before do
       assign(:application_setting, app_settings)
       allow(Gitlab::Sourcegraph).to receive(:feature_available?).and_return(sourcegraph_flag)
-      allow(License).to receive(:feature_available?).with(:elastic_search).and_return(false) if defined?(License)
     end
 
     context 'when sourcegraph feature is enabled' do