Merge branch...

Merge branch '34418-admin-setting-integrations-incorrectly-report-validation-errors-in-ui' into 'master'

Remove unrelated errors from individual integrations

Closes #34418

See merge request gitlab-org/gitlab!18814

.haml-lint_todo.yml

@@ -416,9 +416,6 @@ linters:
       - 'app/views/u2f/_register.html.haml'
       - 'app/views/users/_deletion_guidance.html.haml'
       - 'ee/app/views/admin/_namespace_plan_info.html.haml'
-      - 'ee/app/views/admin/application_settings/_elasticsearch_form.html.haml'
-      - 'ee/app/views/admin/application_settings/_slack.html.haml'
-      - 'ee/app/views/admin/application_settings/_snowplow.html.haml'
       - 'ee/app/views/admin/application_settings/_templates.html.haml'
       - 'ee/app/views/admin/audit_logs/index.html.haml'
       - 'ee/app/views/admin/dashboard/stats.html.haml'

app/helpers/application_settings_helper.rb

@@ -3,11 +3,11 @@
 module ApplicationSettingsHelper
   extend self
 
-  delegate  :allow_signup?,
-            :gravatar_enabled?,
-            :password_authentication_enabled_for_web?,
-            :akismet_enabled?,
-            to: :'Gitlab::CurrentSettings.current_application_settings'
+  delegate :allow_signup?,
+           :gravatar_enabled?,
+           :password_authentication_enabled_for_web?,
+           :akismet_enabled?,
+           to: :'Gitlab::CurrentSettings.current_application_settings'
 
   def user_oauth_applications?
     Gitlab::CurrentSettings.user_oauth_applications
@@ -315,6 +315,10 @@ module ApplicationSettingsHelper
     Rails.env.test?
   end
 
+  def integration_expanded?(substring)
+    @application_setting.errors.any? { |k| k.to_s.start_with?(substring) }
+  end
+
   def instance_clusters_enabled?
     can?(current_user, :read_cluster, Clusters::Instance.new)
   end

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

-= form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-plantuml-settings'), html: { class: 'fieldset-form' } do |f|
-  = form_errors(@application_setting)
+- expanded = integration_expanded?('plantuml_')
+%section.settings.as-plantuml.no-animate#js-plantuml-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('PlantUML')
+    %button.btn.btn-default.js-settings-toggle{ type: 'button' }
+      = expanded ? _('Collapse') : _('Expand')
+    %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_errors(@application_setting) if expanded
 
-  %fieldset
-    .form-group
-      .form-check
-        = f.check_box :plantuml_enabled, class: 'form-check-input'
-        = f.label :plantuml_enabled, class: 'form-check-label' do
-          Enable PlantUML
-    .form-group
-      = f.label :plantuml_url, 'PlantUML URL', class: 'label-bold'
-      = f.text_field :plantuml_url, class: 'form-control', placeholder: 'http://gitlab.your-plantuml-instance.com:8080'
-      .form-text.text-muted
-        Allow rendering of
-        = link_to "PlantUML", "http://plantuml.com"
-        diagrams in Asciidoc documents using an external PlantUML service.
+      %fieldset
+        .form-group
+          .form-check
+            = f.check_box :plantuml_enabled, class: 'form-check-input'
+            = f.label :plantuml_enabled, _('Enable PlantUML'), class: 'form-check-label'
+        .form-group
+          = f.label :plantuml_url, 'PlantUML URL', class: 'label-bold'
+          = f.text_field :plantuml_url, class: 'form-control', placeholder: 'http://gitlab.your-plantuml-instance.com:8080'
+          .form-text.text-muted
+            Allow rendering of
+            = link_to "PlantUML", "http://plantuml.com"
+            diagrams in Asciidoc documents using an external PlantUML service.
 
-  = f.submit 'Save changes', class: "btn btn-success"
+      = f.submit _('Save changes'), class: "btn btn-success"

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

-- expanded = true if !@application_setting.valid? && @application_setting.errors.any? { |k| k.to_s.start_with?('snowplow_') }
+- expanded = integration_expanded?('snowplow_')
 %section.settings.as-snowplow.no-animate#js-snowplow-settings{ class: ('expanded' if expanded) }
   .settings-header
     %h4
@@ -10,7 +10,7 @@
   .settings-content
 
     = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-snowplow-settings'), html: { class: 'fieldset-form' } do |f|
-      = form_errors(@application_setting)
+      = form_errors(@application_setting) if expanded
 
       %fieldset
         .form-group

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

-- application_setting = local_assigns.fetch(:application_setting)
+- expanded = integration_expanded?('hide_third_party_')
+%section.settings.as-third-party-offers.no-animate#js-third-party-offers-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Third party offers')
+    %button.btn.btn-default.js-settings-toggle{ type: 'button' }
+      = expanded ? _('Collapse') : _('Expand')
+    %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_errors(application_setting)
+    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-third-party-offers-settings'), html: { class: 'fieldset-form' } do |f|
+      = form_errors(@application_setting) if expanded
 
-  %fieldset
-    .form-group
-      .form-check
-        = f.check_box :hide_third_party_offers, class: 'form-check-input'
-        = f.label :hide_third_party_offers, class: 'form-check-label' do
-          Do not display offers from third parties within GitLab
+      %fieldset
+        .form-group
+          .form-check
+            = f.check_box :hide_third_party_offers, class: 'form-check-input'
+            = f.label :hide_third_party_offers, _('Do not display offers from third parties within GitLab'), class: 'form-check-label'
 
-  = f.submit 'Save changes', class: "btn btn-success"
+      = f.submit _('Save changes'), class: "btn btn-success"

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

@@ -2,31 +2,10 @@
 - page_title _("Integrations")
 - @content_class = "limit-container-width" unless fluid_layout
 
-= render_if_exists 'admin/application_settings/elasticsearch_form', expanded: expanded_by_default?
-
-%section.settings.as-plantuml.no-animate#js-plantuml-settings{ class: ('expanded' if expanded_by_default?) }
-  .settings-header
-    %h4
-      = _('PlantUML')
-    %button.btn.btn-default.js-settings-toggle{ type: 'button' }
-      = expanded_by_default? ? _('Collapse') : _('Expand')
-    %p
-      = _('Allow rendering of PlantUML diagrams in Asciidoc documents.')
-  .settings-content
-    = render 'plantuml'
-
-= render_if_exists 'admin/application_settings/slack', expanded: expanded_by_default?
-
-%section.settings.as-third-party-offers.no-animate#js-third-party-offers-settings{ class: ('expanded' if expanded_by_default?) }
-  .settings-header
-    %h4
-      = _('Third party offers')
-    %button.btn.btn-default.js-settings-toggle{ type: 'button' }
-      = expanded_by_default? ? _('Collapse') : _('Expand')
-    %p
-      = _('Control the display of third party offers.')
-  .settings-content
-    = render 'third_party_offers', application_setting: @application_setting
-
-= render 'admin/application_settings/snowplow', expanded: expanded_by_default?
+= render_if_exists 'admin/application_settings/elasticsearch_form'
+= render 'admin/application_settings/plantuml'
+= render_if_exists 'admin/application_settings/slack'
+= render 'admin/application_settings/third_party_offers'
+= render 'admin/application_settings/snowplow'
 = render_if_exists 'admin/application_settings/pendo'
+

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

@@ -3,6 +3,7 @@
 - recreate_index_url = help_page_url('integration/elasticsearch.md')
 - recreate_index_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: recreate_index_url }
 - recreate_index_text = s_("Changes won't take place until the index is %{link_start}recreated%{link_end}.").html_safe % { link_start: recreate_index_link_start, link_end: '</a>'.html_safe }
+- expanded = integration_expanded?('elasticsearch_')
 
 %section.settings.as-elasticsearch.no-animate#js-elasticsearch-settings{ class: ('expanded' if expanded), data: { qa_selector: 'elasticsearch_tab' } }
   .settings-header
@@ -15,7 +16,7 @@
   .settings-content
 
     = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings'), html: { class: 'fieldset-form' } do |f|
-      = form_errors(@application_setting)
+      = form_errors(@application_setting) if expanded
 
       %fieldset
         .sub-section
@@ -40,7 +41,7 @@
             = f.label :elasticsearch_url, 'URL', class: 'label-bold'
             = f.text_field :elasticsearch_url, value: @application_setting.elasticsearch_url.join(', '), class: 'form-control', placeholder: 'http://localhost:9200', data: { qa_selector: 'url_field' }
             .form-text.text-muted
-              The URL to use for connecting to Elasticsearch. Use a comma-separated list to support clustering (e.g., "http://localhost:9200, http://localhost:9201").
+              = _('The URL to use for connecting to Elasticsearch. Use a comma-separated list to support clustering (e.g., "http://localhost:9200, http://localhost:9201").')
 
           .form-group
             = f.label :elasticsearch_shards, _('Number of Elasticsearch shards'), class: 'label-bold'
@@ -73,7 +74,7 @@
             = f.text_field :elasticsearch_project_ids, class: 'js-elasticsearch-projects', value: elasticsearch_project_ids, data: { selected: elasticsearch_objects_options(@application_setting.elasticsearch_limited_projects(true)).to_json }
 
         .sub-section
-          %h4 Elasticsearch AWS IAM credentials
+          %h4= _('Elasticsearch AWS IAM credentials')
           .form-group
             .form-check
               = f.check_box :elasticsearch_aws, class: 'form-check-input'
@@ -83,18 +84,18 @@
             = f.label :elasticsearch_aws_region, 'AWS region', class: 'label-bold'
             = f.text_field :elasticsearch_aws_region, value: @application_setting.elasticsearch_aws_region, class: 'form-control'
             .form-text.text-muted
-              Region that elasticsearch is configured
+              = _('Region that Elasticsearch is configured')
 
           .form-group
-            = f.label :elasticsearch_aws_access_key, 'AWS Access Key', class: 'label-bold'
+            = f.label :elasticsearch_aws_access_key, _('AWS Access Key'), class: 'label-bold'
             = f.text_field :elasticsearch_aws_access_key, value: @application_setting.elasticsearch_aws_access_key, class: 'form-control'
             .form-text.text-muted
-              AWS Access Key.  Only required if not using role instance credentials
+              = _('AWS Access Key.  Only required if not using role instance credentials')
 
           .form-group
-            = f.label :elasticsearch_aws_secret_access_key, 'AWS Secret Access Key', class: 'label-bold'
+            = f.label :elasticsearch_aws_secret_access_key, _('AWS Secret Access Key'), class: 'label-bold'
             = f.password_field :elasticsearch_aws_secret_access_key, value: @application_setting.elasticsearch_aws_secret_access_key, class: 'form-control'
             .form-text.text-muted
-              AWS Secret Access Key.  Only required if not using role instance credentials
+              = _('AWS Secret Access Key.  Only required if not using role instance credentials')
 
-      = f.submit 'Save changes', class: "btn btn-success", data: { qa_selector: 'submit_button' }
+      = f.submit _('Save changes'), class: "btn btn-success", data: { qa_selector: 'submit_button' }

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

 - return unless Gitlab.dev_env_or_com?
 
+- expanded = integration_expanded?('slack_app_')
 %section.settings.as-slack.no-animate#js-slack-settings{ class: ('expanded' if expanded) }
   .settings-header
     %h4
@@ -11,7 +12,7 @@
   .settings-content
 
     = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-slack-settings'), html: { class: 'fieldset-form' } do |f|
-      = form_errors(@application_setting)
+      = form_errors(@application_setting) if expanded
 
       %fieldset
         .form-group
@@ -20,7 +21,7 @@
             = f.label :slack_app_enabled, class: 'form-check-label' do
               Enable Slack application
             .form-text.text-muted
-              This option is only available on GitLab.com
+              = _('This option is only available on GitLab.com')
         .form-group
           = f.label :slack_app_id, 'APP_ID', class: 'label-bold'
           = f.text_field :slack_app_id, class: 'form-control'
@@ -31,5 +32,5 @@
           = f.label :slack_app_verification_token, 'Verification token', class: 'label-bold'
           = f.text_field :slack_app_verification_token, class: 'form-control'
 
-      = f.submit 'Save changes', class: "btn btn-success"
+      = f.submit _('Save changes'), class: "btn btn-success"
 

ee/spec/features/admin/admin_settings_spec.rb

@@ -52,6 +52,9 @@ describe 'Admin updates EE-only settings' do
   context 'Elasticsearch settings' do
     before do
       visit integrations_admin_application_settings_path
+      page.within('.as-elasticsearch') do
+        click_button 'Expand'
+      end
     end
 
     it 'changes elasticsearch settings' do
@@ -73,7 +76,7 @@ describe 'Admin updates EE-only settings' do
       end
     end
 
-    it 'Allows limiting projects and namespaces to index', :js do
+    it 'Allows limiting projects and namespaces to index', :aggregate_failures, :js do
       project = create(:project)
       namespace = create(:namespace)
 
@@ -119,7 +122,7 @@ describe 'Admin updates EE-only settings' do
       expect(page).to have_content 'Application settings saved successfully'
     end
 
-    it 'Allows removing all namespaces and projects', :js do
+    it 'Allows removing all namespaces and projects', :aggregate_failures, :js do
       stub_ee_application_setting(elasticsearch_limit_indexing: true)
 
       namespace = create(:elasticsearch_indexed_namespace).namespace
@@ -131,6 +134,8 @@ describe 'Admin updates EE-only settings' do
       expect(ElasticsearchIndexedProject.count).to be > 0
 
       page.within('.as-elasticsearch') do
+        click_button 'Expand'
+
         expect(page).to have_content('Namespaces to index')
         expect(page).to have_content('Projects to index')
         expect(page).to have_content(namespace.full_name)

locale/gitlab.pot

@@ -704,6 +704,18 @@ msgstr ""
 msgid "API Token"
 msgstr ""
 
+msgid "AWS Access Key"
+msgstr ""
+
+msgid "AWS Access Key.  Only required if not using role instance credentials"
+msgstr ""
+
+msgid "AWS Secret Access Key"
+msgstr ""
+
+msgid "AWS Secret Access Key.  Only required if not using role instance credentials"
+msgstr ""
+
 msgid "Abort"
 msgstr ""
 
@@ -5739,6 +5751,9 @@ msgstr ""
 msgid "Display name"
 msgstr ""
 
+msgid "Do not display offers from third parties within GitLab"
+msgstr ""
+
 msgid "Do you want to customize how Google Code email addresses and usernames are imported into GitLab?"
 msgstr ""
 
@@ -5925,6 +5940,9 @@ msgstr ""
 msgid "Elasticsearch"
 msgstr ""
 
+msgid "Elasticsearch AWS IAM credentials"
+msgstr ""
+
 msgid "Elasticsearch indexing restrictions"
 msgstr ""
 
@@ -6033,6 +6051,9 @@ msgstr ""
 msgid "Enable Incident Management inbound alert limit"
 msgstr ""
 
+msgid "Enable PlantUML"
+msgstr ""
+
 msgid "Enable Pseudonymizer data collection"
 msgstr ""
 
@@ -13709,6 +13730,9 @@ msgstr ""
 msgid "Regex pattern"
 msgstr ""
 
+msgid "Region that Elasticsearch is configured"
+msgstr ""
+
 msgid "Register"
 msgstr ""
 
@@ -16501,6 +16525,9 @@ msgstr ""
 msgid "The Issue Tracker is the place to add things that need to be improved or solved in a project. You can register or sign in to create issues for this project."
 msgstr ""
 
+msgid "The URL to use for connecting to Elasticsearch. Use a comma-separated list to support clustering (e.g., \"http://localhost:9200, http://localhost:9201\")."
+msgstr ""
+
 msgid "The X509 Certificate to use when mutual TLS is required to communicate with the external authorization service. If left blank, the server certificate is still validated when accessing over HTTPS."
 msgstr ""
 
@@ -17131,6 +17158,9 @@ msgstr ""
 msgid "This option is disabled as you don't have write permissions for the current branch"
 msgstr ""
 
+msgid "This option is only available on GitLab.com"
+msgstr ""
+
 msgid "This page is unavailable because you are not allowed to read information across multiple projects."
 msgstr ""
 

spec/helpers/application_settings_helper_spec.rb

@@ -38,7 +38,26 @@ describe ApplicationSettingsHelper do
   it_behaves_like 'when HTTP protocol is in use', 'http'
 
   context 'with tracking parameters' do
-    it { expect(visible_attributes).to include(*%i(snowplow_collector_hostname snowplow_cookie_domain snowplow_enabled snowplow_site_id))}
-    it { expect(visible_attributes).to include(*%i(pendo_enabled pendo_url))}
+    it { expect(visible_attributes).to include(*%i(snowplow_collector_hostname snowplow_cookie_domain snowplow_enabled snowplow_site_id)) }
+    it { expect(visible_attributes).to include(*%i(pendo_enabled pendo_url)) }
+  end
+
+  describe '.integration_expanded?' do
+    let(:application_setting) { build(:application_setting) }
+
+    it 'is expanded' do
+      application_setting.plantuml_enabled = true
+      application_setting.valid?
+      helper.instance_variable_set(:@application_setting, application_setting)
+
+      expect(helper.integration_expanded?('plantuml_')).to be_truthy
+    end
+
+    it 'is not expanded' do
+      application_setting.valid?
+      helper.instance_variable_set(:@application_setting, application_setting)
+
+      expect(helper.integration_expanded?('plantuml_')).to be_falsey
+    end
   end
 end