Merge branch '258980-feature-flag-rollout-of-admin-approval-for-new-user-signups' into 'master'

Remove admin_approval_for_new_user_signups feature flag

See merge request gitlab-org/gitlab!46051

app/controllers/admin/users_controller.rb

@@ -6,7 +6,6 @@ class Admin::UsersController < Admin::ApplicationController
   before_action :user, except: [:index, :new, :create]
   before_action :check_impersonation_availability, only: :impersonate
   before_action :ensure_destroy_prerequisites_met, only: [:destroy]
-  before_action :check_admin_approval_feature_available!, only: [:approve]
 
   feature_category :users
 
@@ -298,10 +297,6 @@ class Admin::UsersController < Admin::ApplicationController
   def log_impersonation_event
     Gitlab::AppLogger.info(_("User %{current_user_username} has started impersonating %{username}") % { current_user_username: current_user.username, username: user.username })
   end
-
-  def check_admin_approval_feature_available!
-    access_denied! unless Feature.enabled?(:admin_approval_for_new_user_signups, default_enabled: true)
-  end
 end
 
 Admin::UsersController.prepend_if_ee('EE::Admin::UsersController')

app/controllers/registrations_controller.rb

@@ -218,7 +218,6 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   def set_user_state
-    return unless Feature.enabled?(:admin_approval_for_new_user_signups, default_enabled: true)
     return unless Gitlab::CurrentSettings.require_admin_approval_after_user_signup
 
     resource.state = BLOCKED_PENDING_APPROVAL_STATE

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

@@ -9,14 +9,13 @@
           Sign-up enabled
           .form-text.text-muted
             = _("When enabled, any user visiting %{host} will be able to create an account.") % { host: "#{new_user_session_url(host: Gitlab.config.gitlab.host)}" }
-    - if Feature.enabled?(:admin_approval_for_new_user_signups, default_enabled: true)
-      .form-group
-        .form-check
-          = f.check_box :require_admin_approval_after_user_signup, class: 'form-check-input'
-          = f.label :require_admin_approval_after_user_signup, class: 'form-check-label' do
-            = _('Require admin approval for new sign-ups')
-            .form-text.text-muted
-              = _("When enabled, any user visiting %{host} and creating an account will have to be explicitly approved by an admin before they can sign in. This setting is effective only if sign-ups are enabled.") % { host: "#{new_user_session_url(host: Gitlab.config.gitlab.host)}" }
+    .form-group
+      .form-check
+        = f.check_box :require_admin_approval_after_user_signup, class: 'form-check-input'
+        = f.label :require_admin_approval_after_user_signup, class: 'form-check-label' do
+          = _('Require admin approval for new sign-ups')
+          .form-text.text-muted
+            = _("When enabled, any user visiting %{host} and creating an account will have to be explicitly approved by an admin before they can sign in. This setting is effective only if sign-ups are enabled.") % { host: "#{new_user_session_url(host: Gitlab.config.gitlab.host)}" }
     .form-group
       .form-check
         = f.check_box :send_user_confirmation_email, class: 'form-check-input'

app/views/admin/users/index.html.haml

@@ -30,11 +30,10 @@
       = link_to admin_users_path(filter: "blocked") do
         = s_('AdminUsers|Blocked')
         %small.badge.badge-pill= limited_counter_with_delimiter(User.blocked)
-    - if Feature.enabled?(:admin_approval_for_new_user_signups, default_enabled: true)
-      = nav_link(html_options: { class: "#{active_when(params[:filter] == 'blocked_pending_approval')} filter-blocked-pending-approval" }) do
-        = link_to admin_users_path(filter: "blocked_pending_approval") do
-          = s_('AdminUsers|Pending approval')
-          %small.badge.badge-pill= limited_counter_with_delimiter(User.blocked_pending_approval)
+    = nav_link(html_options: { class: "#{active_when(params[:filter] == 'blocked_pending_approval')} filter-blocked-pending-approval" }) do
+      = link_to admin_users_path(filter: "blocked_pending_approval") do
+        = s_('AdminUsers|Pending approval')
+        %small.badge.badge-pill= limited_counter_with_delimiter(User.blocked_pending_approval)
     = nav_link(html_options: { class: active_when(params[:filter] == 'deactivated') }) do
       = link_to admin_users_path(filter: "deactivated") do
         = s_('AdminUsers|Deactivated')

changelogs/unreleased/258980-feature-flag-rollout-of-admin-approval-for-new-user-signups.yml

+---
+title: Remove admin_approval_for_new_user_signups feature flag
+merge_request: 46051
+author:
+type: changed

spec/controllers/admin/users_controller_spec.rb

@@ -107,49 +107,31 @@ RSpec.describe Admin::UsersController do
 
     subject { put :approve, params: { id: user.username } }
 
-    context 'when feature is disabled' do
-      before do
-        stub_feature_flags(admin_approval_for_new_user_signups: false)
-      end
-
-      it 'responds with access denied' do
+    context 'when successful' do
+      it 'activates the user' do
         subject
 
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
+        user.reload
 
-    context 'when feature is enabled' do
-      before do
-        stub_feature_flags(admin_approval_for_new_user_signups: true)
+        expect(user).to be_active
+        expect(flash[:notice]).to eq('Successfully approved')
       end
+    end
 
-      context 'when successful' do
-        it 'activates the user' do
-          subject
+    context 'when unsuccessful' do
+      let(:user) { create(:user, :blocked) }
 
-          user.reload
+      it 'displays the error' do
+        subject
 
-          expect(user).to be_active
-          expect(flash[:notice]).to eq('Successfully approved')
-        end
+        expect(flash[:alert]).to eq('The user you are trying to approve is not pending an approval')
       end
 
-      context 'when unsuccessful' do
-        let(:user) { create(:user, :blocked) }
-
-        it 'displays the error' do
-          subject
-
-          expect(flash[:alert]).to eq('The user you are trying to approve is not pending an approval')
-        end
-
-        it 'does not activate the user' do
-          subject
+      it 'does not activate the user' do
+        subject
 
-          user.reload
-          expect(user).not_to be_active
-        end
+        user.reload
+        expect(user).not_to be_active
       end
     end
   end

spec/controllers/registrations_controller_spec.rb

@@ -46,102 +46,76 @@ RSpec.describe RegistrationsController do
     subject { post(:create, params: user_params) }
 
     context '`blocked_pending_approval` state' do
-      context 'when the feature is enabled' do
+      context 'when the `require_admin_approval_after_user_signup` setting is turned on' do
         before do
-          stub_feature_flags(admin_approval_for_new_user_signups: true)
+          stub_application_setting(require_admin_approval_after_user_signup: true)
         end
 
-        context 'when the `require_admin_approval_after_user_signup` setting is turned on' do
-          before do
-            stub_application_setting(require_admin_approval_after_user_signup: true)
-          end
-
-          it 'signs up the user in `blocked_pending_approval` state' do
-            subject
-            created_user = User.find_by(email: 'new@user.com')
-
-            expect(created_user).to be_present
-            expect(created_user.blocked_pending_approval?).to eq(true)
-          end
-
-          it 'does not log in the user after sign up' do
-            subject
-
-            expect(controller.current_user).to be_nil
-          end
-
-          it 'shows flash message after signing up' do
-            subject
-
-            expect(response).to redirect_to(new_user_session_path(anchor: 'login-pane'))
-            expect(flash[:notice])
-              .to eq('You have signed up successfully. However, we could not sign you in because your account is awaiting approval from your GitLab administrator.')
-          end
-
-          context 'email confirmation' do
-            context 'when `send_user_confirmation_email` is true' do
-              before do
-                stub_application_setting(send_user_confirmation_email: true)
-              end
+        it 'signs up the user in `blocked_pending_approval` state' do
+          subject
+          created_user = User.find_by(email: 'new@user.com')
 
-              it 'does not send a confirmation email' do
-                expect { subject }
-                  .not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
-              end
-            end
-          end
+          expect(created_user).to be_present
+          expect(created_user.blocked_pending_approval?).to eq(true)
         end
 
-        context 'when the `require_admin_approval_after_user_signup` setting is turned off' do
-          before do
-            stub_application_setting(require_admin_approval_after_user_signup: false)
-          end
-
-          it 'signs up the user in `active` state' do
-            subject
-            created_user = User.find_by(email: 'new@user.com')
+        it 'does not log in the user after sign up' do
+          subject
 
-            expect(created_user).to be_present
-            expect(created_user.active?).to eq(true)
-          end
+          expect(controller.current_user).to be_nil
+        end
 
-          it 'does not show any flash message after signing up' do
-            subject
+        it 'shows flash message after signing up' do
+          subject
 
-            expect(flash[:notice]).to be_nil
-          end
+          expect(response).to redirect_to(new_user_session_path(anchor: 'login-pane'))
+          expect(flash[:notice])
+            .to eq('You have signed up successfully. However, we could not sign you in because your account is awaiting approval from your GitLab administrator.')
+        end
 
-          context 'email confirmation' do
-            context 'when `send_user_confirmation_email` is true' do
-              before do
-                stub_application_setting(send_user_confirmation_email: true)
-              end
+        context 'email confirmation' do
+          context 'when `send_user_confirmation_email` is true' do
+            before do
+              stub_application_setting(send_user_confirmation_email: true)
+            end
 
-              it 'sends a confirmation email' do
-                expect { subject }
-                  .to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
-              end
+            it 'does not send a confirmation email' do
+              expect { subject }
+                .not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
             end
           end
         end
       end
 
-      context 'when the feature is disabled' do
+      context 'when the `require_admin_approval_after_user_signup` setting is turned off' do
         before do
-          stub_feature_flags(admin_approval_for_new_user_signups: false)
+          stub_application_setting(require_admin_approval_after_user_signup: false)
         end
 
-        context 'when the `require_admin_approval_after_user_signup` setting is turned on' do
-          before do
-            stub_application_setting(require_admin_approval_after_user_signup: true)
-          end
+        it 'signs up the user in `active` state' do
+          subject
+          created_user = User.find_by(email: 'new@user.com')
 
-          it 'signs up the user in `active` state' do
-            subject
+          expect(created_user).to be_present
+          expect(created_user.active?).to eq(true)
+        end
 
-            created_user = User.find_by(email: 'new@user.com')
-            expect(created_user).to be_present
-            expect(created_user.active?).to eq(true)
+        it 'does not show any flash message after signing up' do
+          subject
+
+          expect(flash[:notice]).to be_nil
+        end
+
+        context 'email confirmation' do
+          context 'when `send_user_confirmation_email` is true' do
+            before do
+              stub_application_setting(send_user_confirmation_email: true)
+            end
+
+            it 'sends a confirmation email' do
+              expect { subject }
+                .to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
+            end
           end
         end
       end

spec/features/admin/admin_settings_spec.rb

@@ -132,32 +132,14 @@ RSpec.describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_n
 
       context 'Change Sign-up restrictions' do
         context 'Require Admin approval for new signup setting' do
-          context 'when feature is enabled' do
-            before do
-              stub_feature_flags(admin_approval_for_new_user_signups: true)
-            end
-
-            it 'changes the setting' do
-              page.within('.as-signup') do
-                check 'Require admin approval for new sign-ups'
-                click_button 'Save changes'
-              end
-
-              expect(current_settings.require_admin_approval_after_user_signup).to be_truthy
-              expect(page).to have_content "Application settings saved successfully"
-            end
-          end
-
-          context 'when feature is disabled' do
-            before do
-              stub_feature_flags(admin_approval_for_new_user_signups: false)
+          it 'changes the setting' do
+            page.within('.as-signup') do
+              check 'Require admin approval for new sign-ups'
+              click_button 'Save changes'
             end
 
-            it 'does not show the the setting' do
-              page.within('.as-signup') do
-                expect(page).not_to have_selector('.application_setting_require_admin_approval_after_user_signup')
-              end
-            end
+            expect(current_settings.require_admin_approval_after_user_signup).to be_truthy
+            expect(page).to have_content "Application settings saved successfully"
           end
         end
       end

spec/features/admin/admin_users_spec.rb

@@ -75,26 +75,12 @@ RSpec.describe "Admin::Users" do
       end
 
       context '`Pending approval` tab' do
-        context 'feature is enabled' do
-          before do
-            stub_feature_flags(admin_approval_for_new_user_signups: true)
-            visit admin_users_path
-          end
-
-          it 'shows the `Pending approval` tab' do
-            expect(page).to have_link('Pending approval', href: admin_users_path(filter: 'blocked_pending_approval'))
-          end
+        before do
+          visit admin_users_path
         end
 
-        context 'feature is disabled' do
-          before do
-            stub_feature_flags(admin_approval_for_new_user_signups: false)
-            visit admin_users_path
-          end
-
-          it 'does not show the `Pending approval` tab' do
-            expect(page).not_to have_link('Pending approval', href: admin_users_path(filter: 'blocked_pending_approval'))
-          end
+        it 'shows the `Pending approval` tab' do
+          expect(page).to have_link('Pending approval', href: admin_users_path(filter: 'blocked_pending_approval'))
         end
       end
     end