Merge branch 'led/327852-migrate-some-unit-to-feature-tests' into 'master'

Move Sign-up restrictions confirmation modal tests from unit to feature

See merge request gitlab-org/gitlab!71058

app/helpers/application_settings_helper.rb

@@ -477,8 +477,6 @@ module ApplicationSettingsHelper
   end
 
   def pending_user_count
-    return 0 if Gitlab::CurrentSettings.new_user_signups_cap.blank?
-
     User.blocked_pending_approval.count
   end
 end

ee/spec/features/admin/admin_settings_spec.rb

@@ -286,6 +286,10 @@ RSpec.describe 'Admin updates EE-only settings' do
   end
 
   context 'sign up settings', :js do
+    before do
+      visit general_admin_application_settings_path
+    end
+
     context 'when license has active user count' do
       let(:license) { create(:license, restrictions: { active_user_count: 1 }) }
 
@@ -294,8 +298,6 @@ RSpec.describe 'Admin updates EE-only settings' do
       end
 
       it 'disallows entering user cap greater then license allows' do
-        visit general_admin_application_settings_path
-
         page.within('#js-signup-settings') do
           fill_in 'application_setting[new_user_signups_cap]', with: 5
 
@@ -309,8 +311,6 @@ RSpec.describe 'Admin updates EE-only settings' do
     end
 
     it 'changes the user cap from unlimited to 5' do
-      visit general_admin_application_settings_path
-
       expect(current_settings.new_user_signups_cap).to be_nil
 
       page.within('#js-signup-settings') do
@@ -328,8 +328,6 @@ RSpec.describe 'Admin updates EE-only settings' do
       end
 
       it 'changes the user cap to unlimited' do
-        visit general_admin_application_settings_path
-
         page.within('#js-signup-settings') do
           fill_in 'application_setting[new_user_signups_cap]', with: nil
 
@@ -342,11 +340,10 @@ RSpec.describe 'Admin updates EE-only settings' do
       context 'with pending users' do
         before do
           create(:user, :blocked_pending_approval)
+          visit general_admin_application_settings_path
         end
 
         it 'displays a modal confirmation when removing the cap' do
-          visit general_admin_application_settings_path
-
           page.within('#js-signup-settings') do
             fill_in 'application_setting[new_user_signups_cap]', with: nil
 
@@ -361,6 +358,74 @@ RSpec.describe 'Admin updates EE-only settings' do
         end
       end
     end
+
+    context 'form submit button confirmation modal for side-effect of possibly adding unwanted new users' do
+      [
+        [:unchanged_true, :unchanged, false, :submits_form],
+        [:unchanged_false, :unchanged, false, :submits_form],
+        [:toggled_off, :unchanged, true, :shows_confirmation_modal],
+        [:toggled_off, :unchanged, false, :submits_form],
+        [:toggled_on, :unchanged, false, :submits_form],
+        [:unchanged_false, :increased, true, :shows_confirmation_modal],
+        [:unchanged_true, :increased, false, :submits_form],
+        [:toggled_off, :increased, true, :shows_confirmation_modal],
+        [:toggled_off, :increased, false, :submits_form],
+        [:toggled_on, :increased, true, :shows_confirmation_modal],
+        [:toggled_on, :increased, false, :submits_form],
+        [:toggled_on, :decreased, false, :submits_form],
+        [:toggled_on, :decreased, true, :submits_form],
+        [:unchanged_false, :changed_from_limited_to_unlimited, true, :shows_confirmation_modal],
+        [:unchanged_false, :changed_from_limited_to_unlimited, false, :submits_form],
+        [:unchanged_false, :changed_from_unlimited_to_limited, false, :submits_form],
+        [:unchanged_false, :unchanged_unlimited, false, :submits_form]
+      ].each do |(require_admin_approval_action, user_cap_action, add_pending_user, button_effect)|
+        it "#{button_effect} if 'require admin approval for new sign-ups' is #{require_admin_approval_action} and the user cap is #{user_cap_action} and #{add_pending_user ? "has" : "doesn't have"} pending user count" do
+          user_cap_default = 5
+          require_admin_approval_value = [:unchanged_true, :toggled_off].include?(require_admin_approval_action)
+
+          current_settings.update_attribute(:require_admin_approval_after_user_signup, require_admin_approval_value)
+
+          unless [:changed_from_unlimited_to_limited, :unchanged_unlimited].include?(user_cap_action)
+            current_settings.update_attribute(:new_user_signups_cap, user_cap_default)
+          end
+
+          if add_pending_user
+            create(:user, :blocked_pending_approval)
+            visit general_admin_application_settings_path
+          end
+
+          page.within('#js-signup-settings') do
+            case require_admin_approval_action
+            when :toggled_on
+              find('[data-testid="require-admin-approval-checkbox"]').set(true)
+            when :toggled_off
+              find('[data-testid="require-admin-approval-checkbox"]').set(false)
+            end
+
+            case user_cap_action
+            when :increased
+              fill_in 'application_setting[new_user_signups_cap]', with: user_cap_default + 1
+            when :decreased
+              fill_in 'application_setting[new_user_signups_cap]', with: user_cap_default - 1
+            when :changed_from_limited_to_unlimited
+              fill_in 'application_setting[new_user_signups_cap]', with: nil
+            when :changed_from_unlimited_to_limited
+              fill_in 'application_setting[new_user_signups_cap]', with: user_cap_default
+            end
+
+            click_button 'Save changes'
+          end
+
+          case button_effect
+          when :shows_confirmation_modal
+            expect(page).to have_selector('.modal')
+            expect(page).to have_css('.modal .modal-body', text: 'By making this change, you will automatically approve 1 user with the pending approval status.')
+          when :submits_form
+            expect(page).to have_content 'Application settings saved successfully'
+          end
+        end
+      end
+    end
   end
 
   def current_settings

spec/frontend/admin/signup_restrictions/components/signup_form_spec.js

@@ -35,9 +35,6 @@ describe('Signup Form', () => {
 
   const findDenyListRawInputGroup = () => wrapper.findByTestId('domain-denylist-raw-input-group');
   const findDenyListFileInputGroup = () => wrapper.findByTestId('domain-denylist-file-input-group');
-
-  const findRequireAdminApprovalCheckbox = () =>
-    wrapper.findByTestId('require-admin-approval-checkbox');
   const findUserCapInput = () => wrapper.findByTestId('user-cap-input');
   const findModal = () => wrapper.find(GlModal);
 
@@ -191,125 +188,6 @@ describe('Signup Form', () => {
   });
 
   describe('form submit button confirmation modal for side-effect of adding possibly unwanted new users', () => {
-    it.each`
-      requireAdminApprovalAction | userCapAction                          | pendingUserCount | buttonEffect
-      ${'unchanged from true'}   | ${'unchanged'}                         | ${0}             | ${'submits form'}
-      ${'unchanged from false'}  | ${'unchanged'}                         | ${0}             | ${'submits form'}
-      ${'toggled off'}           | ${'unchanged'}                         | ${1}             | ${'shows confirmation modal'}
-      ${'toggled off'}           | ${'unchanged'}                         | ${0}             | ${'submits form'}
-      ${'toggled on'}            | ${'unchanged'}                         | ${0}             | ${'submits form'}
-      ${'unchanged from false'}  | ${'increased'}                         | ${1}             | ${'shows confirmation modal'}
-      ${'unchanged from true'}   | ${'increased'}                         | ${0}             | ${'submits form'}
-      ${'toggled off'}           | ${'increased'}                         | ${1}             | ${'shows confirmation modal'}
-      ${'toggled off'}           | ${'increased'}                         | ${0}             | ${'submits form'}
-      ${'toggled on'}            | ${'increased'}                         | ${1}             | ${'shows confirmation modal'}
-      ${'toggled on'}            | ${'increased'}                         | ${0}             | ${'submits form'}
-      ${'toggled on'}            | ${'decreased'}                         | ${0}             | ${'submits form'}
-      ${'toggled on'}            | ${'decreased'}                         | ${1}             | ${'submits form'}
-      ${'unchanged from false'}  | ${'changed from limited to unlimited'} | ${1}             | ${'shows confirmation modal'}
-      ${'unchanged from false'}  | ${'changed from limited to unlimited'} | ${0}             | ${'submits form'}
-      ${'unchanged from false'}  | ${'changed from unlimited to limited'} | ${0}             | ${'submits form'}
-      ${'unchanged from false'}  | ${'unchanged from unlimited'}          | ${0}             | ${'submits form'}
-    `(
-      '$buttonEffect if require admin approval for new sign-ups is $requireAdminApprovalAction and the user cap is $userCapAction and pending user count is $pendingUserCount',
-      async ({ requireAdminApprovalAction, userCapAction, pendingUserCount, buttonEffect }) => {
-        let isModalDisplayed;
-
-        switch (buttonEffect) {
-          case 'shows confirmation modal':
-            isModalDisplayed = true;
-            break;
-          case 'submits form':
-            isModalDisplayed = false;
-            break;
-          default:
-            isModalDisplayed = false;
-            break;
-        }
-
-        const isFormSubmittedWhenClickingFormSubmitButton = !isModalDisplayed;
-
-        const injectedProps = {
-          pendingUserCount,
-        };
-
-        const USER_CAP_DEFAULT = 5;
-
-        switch (userCapAction) {
-          case 'changed from unlimited to limited':
-            injectedProps.newUserSignupsCap = '';
-            break;
-          case 'unchanged from unlimited':
-            injectedProps.newUserSignupsCap = '';
-            break;
-          default:
-            injectedProps.newUserSignupsCap = USER_CAP_DEFAULT;
-            break;
-        }
-
-        switch (requireAdminApprovalAction) {
-          case 'unchanged from true':
-            injectedProps.requireAdminApprovalAfterUserSignup = true;
-            break;
-          case 'unchanged from false':
-            injectedProps.requireAdminApprovalAfterUserSignup = false;
-            break;
-          case 'toggled off':
-            injectedProps.requireAdminApprovalAfterUserSignup = true;
-            break;
-          case 'toggled on':
-            injectedProps.requireAdminApprovalAfterUserSignup = false;
-            break;
-          default:
-            injectedProps.requireAdminApprovalAfterUserSignup = false;
-            break;
-        }
-
-        formSubmitSpy = jest.spyOn(HTMLFormElement.prototype, 'submit').mockImplementation();
-
-        await mountComponent({
-          injectedProps,
-          stubs: { GlButton, GlModal: stubComponent(GlModal) },
-        });
-
-        findModal().vm.show = jest.fn();
-
-        if (
-          requireAdminApprovalAction === 'toggled off' ||
-          requireAdminApprovalAction === 'toggled on'
-        ) {
-          await findRequireAdminApprovalCheckbox().vm.$emit('input', false);
-        }
-
-        switch (userCapAction) {
-          case 'increased':
-            await findUserCapInput().vm.$emit('input', USER_CAP_DEFAULT + 1);
-            break;
-          case 'decreased':
-            await findUserCapInput().vm.$emit('input', USER_CAP_DEFAULT - 1);
-            break;
-          case 'changed from limited to unlimited':
-            await findUserCapInput().vm.$emit('input', '');
-            break;
-          case 'changed from unlimited to limited':
-            await findUserCapInput().vm.$emit('input', USER_CAP_DEFAULT);
-            break;
-          default:
-            break;
-        }
-
-        await findFormSubmitButton().trigger('click');
-
-        if (isFormSubmittedWhenClickingFormSubmitButton) {
-          expect(formSubmitSpy).toHaveBeenCalled();
-          expect(findModal().vm.show).not.toHaveBeenCalled();
-        } else {
-          expect(formSubmitSpy).not.toHaveBeenCalled();
-          expect(findModal().vm.show).toHaveBeenCalled();
-        }
-      },
-    );
-
     describe('modal actions', () => {
       beforeEach(async () => {
         const INITIAL_USER_CAP = 5;

spec/helpers/application_settings_helper_spec.rb

@@ -271,18 +271,6 @@ RSpec.describe ApplicationSettingsHelper do
         expect(pending_user_count).to eq 1
       end
     end
-
-    context 'when the new_user_signups_cap is not present' do
-      let(:user_cap) { nil }
-
-      it { is_expected.to eq 0 }
-
-      it 'does not query users unnecessarily' do
-        expect(User).not_to receive(:blocked_pending_approval)
-
-        pending_user_count
-      end
-    end
   end
 
   describe '#sidekiq_job_limiter_modes_for_select' do