Merge branch '337829-deduplicate-network-rate-limit-form' into 'master'

Deduplicate the network rate limits form

See merge request gitlab-org/gitlab!70400

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

-= gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-files-limits-settings'), html: { class: 'fieldset-form' } do |f|
-  = form_errors(@application_setting)
-
-  %fieldset
-    %legend.h5.gl-border-none
-      = _('Unauthenticated API request rate limit')
-    .form-group
-      = f.gitlab_ui_checkbox_component :throttle_unauthenticated_files_api_enabled,
-        _('Enable unauthenticated API request rate limit'),
-        help_text: _('Helps reduce request volume (e.g. from crawlers or abusive bots)'),
-        checkbox_options: { data: { qa_selector: 'throttle_unauthenticated_files_api_checkbox' } }
-    .form-group
-      = f.label :throttle_unauthenticated_files_api_requests_per_period, 'Max unauthenticated API requests per period per IP', class: 'label-bold'
-      = f.number_field :throttle_unauthenticated_files_api_requests_per_period, class: 'form-control gl-form-input'
-    .form-group
-      = f.label :throttle_unauthenticated_files_api_period_in_seconds, 'Unauthenticated API rate limit period in seconds', class: 'label-bold'
-      = f.number_field :throttle_unauthenticated_files_api_period_in_seconds, class: 'form-control gl-form-input'
-
-  %fieldset
-    %legend.h5.gl-border-none
-      = _('Authenticated API request rate limit')
-    .form-group
-      = f.gitlab_ui_checkbox_component :throttle_authenticated_files_api_enabled,
-        _('Enable authenticated API request rate limit'),
-        help_text: _('Helps reduce request volume (e.g. from crawlers or abusive bots)'),
-        checkbox_options: { data: { qa_selector: 'throttle_authenticated_files_api_checkbox' } }
-    .form-group
-      = f.label :throttle_authenticated_files_api_requests_per_period, 'Max authenticated API requests per period per user', class: 'label-bold'
-      = f.number_field :throttle_authenticated_files_api_requests_per_period, class: 'form-control gl-form-input'
-    .form-group
-      = f.label :throttle_authenticated_files_api_period_in_seconds, 'Authenticated API rate limit period in seconds', class: 'label-bold'
-      = f.number_field :throttle_authenticated_files_api_period_in_seconds, class: 'form-control gl-form-input'
-
-  = f.submit 'Save changes', class: "gl-button btn btn-confirm", data: { qa_selector: 'save_changes_button' }

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

+= gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: anchor), html: { class: 'fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    = _("Rate limits can help reduce request volume (like from crawlers or abusive bots).")
+
+  %fieldset
+    .form-group
+      = f.gitlab_ui_checkbox_component :"throttle_unauthenticated_#{setting_fragment}_enabled",
+        _('Enable unauthenticated API request rate limit'),
+        checkbox_options: { data: { qa_selector: "throttle_unauthenticated_#{setting_fragment}_checkbox" } },
+        label_options: { class: 'label-bold' }
+    .form-group
+      = f.label :"throttle_unauthenticated_#{setting_fragment}_requests_per_period", _('Maximum unauthenticated API requests per rate limit period per IP'), class: 'label-bold'
+      = f.number_field :"throttle_unauthenticated_#{setting_fragment}_requests_per_period", class: 'form-control gl-form-input'
+    .form-group
+      = f.label :"throttle_unauthenticated_#{setting_fragment}_period_in_seconds", _('Unauthenticated API rate limit period in seconds'), class: 'label-bold'
+      = f.number_field :"throttle_unauthenticated_#{setting_fragment}_period_in_seconds", class: 'form-control gl-form-input'
+
+  %fieldset
+    .form-group
+      = f.gitlab_ui_checkbox_component :"throttle_authenticated_#{setting_fragment}_enabled",
+        _('Enable authenticated API request rate limit'),
+        checkbox_options: { data: { qa_selector: "throttle_authenticated_#{setting_fragment}_checkbox" } },
+        label_options: { class: 'label-bold' }
+    .form-group
+      = f.label :"throttle_authenticated_#{setting_fragment}_requests_per_period", _('Maximum authenticated API requests per rate limit period per user'), class: 'label-bold'
+      = f.number_field :"throttle_authenticated_#{setting_fragment}_requests_per_period", class: 'form-control gl-form-input'
+    .form-group
+      = f.label :"throttle_authenticated_#{setting_fragment}_period_in_seconds", _('Authenticated API rate limit period in seconds'), class: 'label-bold'
+      = f.number_field :"throttle_authenticated_#{setting_fragment}_period_in_seconds", class: 'form-control gl-form-input'
+
+  = f.submit _('Save changes'), class: "gl-button btn btn-confirm", data: { qa_selector: 'save_changes_button' }

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

-= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-packages-limits-settings'), html: { class: 'fieldset-form' } do |f|
-  = form_errors(@application_setting)
-
-  %fieldset
-    = _("The package registry rate limits can help reduce request volume (like from crawlers or abusive bots).")
-
-  %fieldset
-    .form-group
-      .form-check
-        = f.check_box :throttle_unauthenticated_packages_api_enabled, class: 'form-check-input', data: { qa_selector: 'throttle_unauthenticated_packages_api_checkbox' }
-        = f.label :throttle_unauthenticated_packages_api_enabled, class: 'form-check-label label-bold' do
-          = _('Enable unauthenticated API request rate limit')
-    .form-group
-      = f.label :throttle_unauthenticated_packages_api_requests_per_period, _('Maximum unauthenticated API requests per rate limit period per IP'), class: 'label-bold'
-      = f.number_field :throttle_unauthenticated_packages_api_requests_per_period, class: 'form-control gl-form-input'
-    .form-group
-      = f.label :throttle_unauthenticated_packages_api_period_in_seconds, _('Unauthenticated API rate limit period in seconds'), class: 'label-bold'
-      = f.number_field :throttle_unauthenticated_packages_api_period_in_seconds, class: 'form-control gl-form-input'
-    %hr
-    .form-group
-      .form-check
-        = f.check_box :throttle_authenticated_packages_api_enabled, class: 'form-check-input', data: { qa_selector: 'throttle_authenticated_packages_api_checkbox' }
-        = f.label :throttle_authenticated_packages_api_enabled, class: 'form-check-label label-bold' do
-          = _('Enable authenticated API request rate limit')
-    .form-group
-      = f.label :throttle_authenticated_packages_api_requests_per_period, _('Maximum authenticated API requests per rate limit period per user'), class: 'label-bold'
-      = f.number_field :throttle_authenticated_packages_api_requests_per_period, class: 'form-control gl-form-input'
-    .form-group
-      = f.label :throttle_authenticated_packages_api_period_in_seconds, _('Authenticated API rate limit period in seconds'), class: 'label-bold'
-      = f.number_field :throttle_authenticated_packages_api_period_in_seconds, class: 'form-control gl-form-input'
-
-  = f.submit _('Save changes'), class: "gl-button btn btn-confirm", data: { qa_selector: 'save_changes_button' }

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

@@ -35,9 +35,10 @@
       = _('Set rate limits for package registry API requests that supersede the general user and IP rate limits.')
       = link_to _('Learn more.'), help_page_path('user/admin_area/settings/package_registry_rate_limits.md'), target: '_blank', rel: 'noopener noreferrer'
   .settings-content
-    = render 'package_registry_limits'
+    = render partial: 'network_rate_limits', locals: { anchor: 'js-packages-limits-settings', setting_fragment: 'packages_api' }
+
 - if Feature.enabled?(:files_api_throttling, default_enabled: :yaml)
-  %section.settings.as-files-limits.no-animate#js-files-limits-settings{ class: ('expanded' if expanded_by_default?), data: { testid: 'files-limits-settings' } }
+  %section.settings.as-files-limits.no-animate#js-files-limits-settings{ class: ('expanded' if expanded_by_default?) }
     .settings-header
       %h4
         = _('Files API Rate Limits')
@@ -46,7 +47,7 @@
       %p
         = _('Configure specific limits for Files API requests that supersede the general user and IP rate limits.')
     .settings-content
-      = render 'files_limits'
+      = render partial: 'network_rate_limits', locals: { anchor: 'js-files-limits-settings', setting_fragment: 'files_api' }
 
 %section.settings.as-git-lfs-limits.no-animate#js-git-lfs-limits-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'git_lfs_limits_content' } }
   .settings-header

locale/gitlab.pot

@@ -4765,9 +4765,6 @@ msgstr ""
 msgid "Authenticated API rate limit period in seconds"
 msgstr ""
 
-msgid "Authenticated API request rate limit"
-msgstr ""
-
 msgid "Authenticated API requests"
 msgstr ""
 
@@ -16681,9 +16678,6 @@ msgstr ""
 msgid "Helps prevent malicious users hide their activity"
 msgstr ""
 
-msgid "Helps reduce request volume (e.g. from crawlers or abusive bots)"
-msgstr ""
-
 msgid "Helps reduce request volume (for example, from crawlers or abusive bots)"
 msgstr ""
 
@@ -33764,9 +33758,6 @@ msgstr ""
 msgid "The number of times an upload record could not find its file"
 msgstr ""
 
-msgid "The package registry rate limits can help reduce request volume (like from crawlers or abusive bots)."
-msgstr ""
-
 msgid "The page could not be displayed because it timed out."
 msgstr ""
 
@@ -36038,9 +36029,6 @@ msgstr ""
 msgid "Unauthenticated API rate limit period in seconds"
 msgstr ""
 
-msgid "Unauthenticated API request rate limit"
-msgstr ""
-
 msgid "Unauthenticated requests"
 msgstr ""
 

spec/features/admin/admin_settings_spec.rb

@@ -602,18 +602,47 @@ RSpec.describe 'Admin updates settings' do
         expect(current_settings.issues_create_limit).to eq(0)
       end
 
-      it 'changes Files API rate limits settings' do
-        visit network_admin_application_settings_path
+      shared_examples 'regular throttle rate limit settings' do
+        it 'changes rate limit settings' do
+          visit network_admin_application_settings_path
 
-        page.within('[data-testid="files-limits-settings"]') do
-          check 'Enable unauthenticated API request rate limit'
-          fill_in 'Max unauthenticated API requests per period per IP', with: 10
-          click_button 'Save changes'
+          page.within(".#{selector}") do
+            check 'Enable unauthenticated API request rate limit'
+            fill_in 'Maximum unauthenticated API requests per rate limit period per IP', with: 12
+            fill_in 'Unauthenticated API rate limit period in seconds', with: 34
+
+            check 'Enable authenticated API request rate limit'
+            fill_in 'Maximum authenticated API requests per rate limit period per user', with: 56
+            fill_in 'Authenticated API rate limit period in seconds', with: 78
+
+            click_button 'Save changes'
+          end
+
+          expect(page).to have_content "Application settings saved successfully"
+
+          expect(current_settings).to have_attributes(
+            "throttle_unauthenticated_#{fragment}_enabled" => true,
+            "throttle_unauthenticated_#{fragment}_requests_per_period" => 12,
+            "throttle_unauthenticated_#{fragment}_period_in_seconds" => 34,
+            "throttle_authenticated_#{fragment}_enabled" => true,
+            "throttle_authenticated_#{fragment}_requests_per_period" => 56,
+            "throttle_authenticated_#{fragment}_period_in_seconds" => 78
+          )
         end
+      end
 
-        expect(page).to have_content "Application settings saved successfully"
-        expect(current_settings.throttle_unauthenticated_files_api_enabled).to be true
-        expect(current_settings.throttle_unauthenticated_files_api_requests_per_period).to eq(10)
+      context 'Package Registry API rate limits' do
+        let(:selector) { 'as-packages-limits' }
+        let(:fragment) { :packages_api }
+
+        include_examples 'regular throttle rate limit settings'
+      end
+
+      context 'Files API rate limits' do
+        let(:selector) { 'as-files-limits' }
+        let(:fragment) { :files_api }
+
+        include_examples 'regular throttle rate limit settings'
       end
     end