Add spec for signup approval

Also Refactor signup into a flow and create requried page objects

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

@@ -11,7 +11,7 @@
             = _("When enabled, any user visiting %{host} will be able to create an account.") % { 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.check_box :require_admin_approval_after_user_signup, class: 'form-check-input', data: { qa_selector: 'require_admin_approval_after_user_signup_checkbox' }
         = f.label :require_admin_approval_after_user_signup, class: 'form-check-label' do
           = _('Require admin approval for new sign-ups')
           .form-text.text-muted
@@ -77,4 +77,4 @@
       = f.label :after_sign_up_text, class: 'label-bold'
       = f.text_area :after_sign_up_text, class: 'form-control', rows: 4
       .form-text.text-muted Markdown enabled
-  = f.submit 'Save changes', class: "gl-button btn btn-success"
+  = f.submit 'Save changes', class: "gl-button btn btn-success", data: { qa_selector: 'save_changes_button' }

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

@@ -35,7 +35,7 @@
   .settings-content
     = render 'diff_limits'
 
-%section.settings.as-signup.no-animate#js-signup-settings{ class: ('expanded' if expanded_by_default?) }
+%section.settings.as-signup.no-animate#js-signup-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'sign_up_restrictions_settings_content' } }
   .settings-header
     %h4
       = _('Sign-up restrictions')

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

@@ -4,4 +4,4 @@
   .card-body
     = render partial: 'admin/users/user_approve_effects'
     %br
-    = link_to s_('AdminUsers|Approve user'), approve_admin_user_path(user), method: :put, class: "btn gl-button btn-info", data: { confirm: s_('AdminUsers|Are you sure?') }
+    = link_to s_('AdminUsers|Approve user'), approve_admin_user_path(user), method: :put, class: "btn gl-button btn-info", data: { confirm: s_('AdminUsers|Are you sure?'), qa_selector: 'approve_user_button' }

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

@@ -31,7 +31,7 @@
         = s_('AdminUsers|Blocked')
         %small.badge.badge-pill= limited_counter_with_delimiter(User.blocked)
     = 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
+      = link_to admin_users_path(filter: "blocked_pending_approval"), data: { qa_selector: 'pending_approval_tab' } 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

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

@@ -42,7 +42,7 @@
               = sprite_icon('close', size: 16, css_class: 'gl-icon')
         %li
           %span.light ID:
-          %strong
+          %strong{ data: { qa_selector: 'user_id_content' } }
             = @user.id
         %li
           %span.light= _('Namespace ID:')

qa/qa.rb

@@ -20,6 +20,7 @@ module QA
     autoload :User, 'qa/flow/user'
     autoload :MergeRequest, 'qa/flow/merge_request'
     autoload :Pipeline, 'qa/flow/pipeline'
+    autoload :SignUp, 'qa/flow/sign_up'
   end
 
   ##
@@ -181,10 +182,14 @@ module QA
       autoload :Menu, 'qa/page/main/menu'
       autoload :OAuth, 'qa/page/main/oauth'
       autoload :TwoFactorAuth, 'qa/page/main/two_factor_auth'
-      autoload :SignUp, 'qa/page/main/sign_up'
       autoload :Terms, 'qa/page/main/terms'
     end
 
+    module Registration
+      autoload :SignUp, 'qa/page/registration/sign_up'
+      autoload :Welcome, 'qa/page/registration/welcome'
+    end
+
     module Settings
       autoload :Common, 'qa/page/settings/common'
     end
@@ -431,6 +436,7 @@ module QA
           autoload :OutboundRequests, 'qa/page/admin/settings/component/outbound_requests'
           autoload :AccountAndLimit, 'qa/page/admin/settings/component/account_and_limit'
           autoload :PerformanceBar, 'qa/page/admin/settings/component/performance_bar'
+          autoload :SignUpRestrictions, 'qa/page/admin/settings/component/sign_up_restrictions'
         end
       end
 

qa/qa/ee.rb

@@ -58,7 +58,10 @@ module QA
       module Main
         autoload :Banner, 'qa/ee/page/main/banner'
         autoload :Menu, 'qa/ee/page/main/menu'
-        autoload :SignUp, 'qa/ee/page/main/sign_up'
+      end
+
+      module Registration
+        autoload :Welcome, 'qa/ee/page/registration/welcome'
       end
 
       module Admin

qa/qa/ee/page/main/sign_up.rb → qa/qa/ee/page/registration/welcome.rb

@@ -3,8 +3,8 @@
 module QA
   module EE
     module Page
-      module Main
-        module SignUp
+      module Registration
+        module Welcome
           extend QA::Page::PageConcern
 
           def self.included(base)

qa/qa/flow/sign_up.rb

+# frozen_string_literal: true
+
+module QA
+  module Flow
+    module SignUp
+      module_function
+
+      def sign_up!(user)
+        Page::Main::Login.perform(&:switch_to_register_page)
+
+        success = Support::Retrier.retry_until(raise_on_failure: false) do
+          Page::Registration::SignUp.perform do |sign_up|
+            sign_up.fill_new_user_first_name_field(user.first_name)
+            sign_up.fill_new_user_last_name_field(user.last_name)
+            sign_up.fill_new_user_username_field(user.username)
+            sign_up.fill_new_user_email_field(user.email)
+            sign_up.fill_new_user_password_field(user.password)
+            sign_up.click_new_user_register_button
+          end
+
+          # Because invisible_captcha would prevent submitting this form
+          # within 4 seconds, sleep here. This can be removed once we
+          # implement invisible_captcha as an application setting instead
+          # of a feature flag, so we can turn it off while testing.
+          # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/284113
+          sleep 4
+
+          Page::Registration::Welcome.perform(&:click_get_started_button_if_available)
+
+          if user.expect_fabrication_success
+            Page::Main::Menu.perform(&:has_personal_area?)
+          else
+            Page::Main::Menu.perform(&:not_signed_in?)
+          end
+        end
+
+        raise "Failed to register the user" unless success
+      end
+    end
+  end
+end

qa/qa/page/admin/overview/users/index.rb

@@ -8,6 +8,7 @@ module QA
           class Index < QA::Page::Base
             view 'app/views/admin/users/index.html.haml' do
               element :user_search_field
+              element :pending_approval_tab
             end
 
             view 'app/views/admin/users/_user.html.haml' do
@@ -22,6 +23,10 @@ module QA
               find_element(:user_search_field).set(username).send_keys(:return)
             end
 
+            def click_pending_approval_tab
+              click_element :pending_approval_tab
+            end
+
             def click_user(username)
               within_element(:user_row_content, text: username) do
                 click_element(:username_link)

qa/qa/page/admin/overview/users/show.rb

@@ -12,17 +12,32 @@ module QA
 
             view 'app/views/admin/users/show.html.haml' do
               element :confirm_user_button
+              element :user_id_content
+            end
+
+            view 'app/views/admin/users/_approve_user.html.haml' do
+              element :approve_user_button
             end
 
             def click_impersonate_user
               click_element(:impersonate_user_link)
             end
 
+            def user_id
+              find_element(:user_id_content).text
+            end
+
             def confirm_user
               accept_confirm do
                 click_element :confirm_user_button
               end
             end
+
+            def approve_user
+              accept_confirm do
+                click_element :approve_user_button
+              end
+            end
           end
         end
       end

qa/qa/page/admin/settings/component/sign_up_restrictions.rb

+# frozen_string_literal: true
+
+module QA
+  module Page
+    module Admin
+      module Settings
+        module Component
+          class SignUpRestrictions < Page::Base
+            view 'app/views/admin/application_settings/_signup.html.haml' do
+              element :require_admin_approval_after_user_signup_checkbox
+              element :save_changes_button
+            end
+
+            def require_admin_approval_after_user_signup
+              check_element :require_admin_approval_after_user_signup_checkbox
+              click_element :save_changes_button
+            end
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/page/admin/settings/general.rb

@@ -9,6 +9,7 @@ module QA
 
           view 'app/views/admin/application_settings/general.html.haml' do
             element :account_and_limit_settings_content
+            element :sign_up_restrictions_settings_content
           end
 
           def expand_account_and_limit(&block)
@@ -16,6 +17,12 @@ module QA
               Component::AccountAndLimit.perform(&block)
             end
           end
+
+          def expand_sign_up_restrictions(&block)
+            expand_content(:sign_up_restrictions_settings_content) do
+              Component::SignUpRestrictions.perform(&block)
+            end
+          end
         end
       end
     end

qa/qa/page/main/menu.rb

@@ -93,6 +93,10 @@ module QA
           has_personal_area?(wait: 0)
         end
 
+        def not_signed_in?
+          has_no_personal_area?
+        end
+
         def sign_out
           retry_until do
             wait_if_retry_later
@@ -129,6 +133,10 @@ module QA
           has_element?(:user_avatar, wait: wait)
         end
 
+        def has_no_personal_area?(wait: Capybara.default_max_wait_time)
+          has_no_element?(:user_avatar, wait: wait)
+        end
+
         def has_admin_area_link?(wait: Capybara.default_max_wait_time)
           has_element?(:admin_area_link, wait: wait)
         end

qa/qa/page/main/sign_up.rb → qa/qa/page/registration/sign_up.rb

@@ -2,7 +2,7 @@
 
 module QA
   module Page
-    module Main
+    module Registration
       class SignUp < Page::Base
         view 'app/views/devise/shared/_signup_box.html.haml' do
           element :new_user_first_name_field
@@ -17,32 +17,30 @@ module QA
           element :get_started_button
         end
 
-        def sign_up!(user)
-          signed_in = retry_until(raise_on_failure: false) do
-            fill_element :new_user_first_name_field, user.first_name
-            fill_element :new_user_last_name_field, user.last_name
-            fill_element :new_user_username_field, user.username
-            fill_element :new_user_email_field, user.email
-            fill_element :new_user_password_field, user.password
+        def fill_new_user_first_name_field(first_name)
+          fill_element :new_user_first_name_field, first_name
+        end
 
-            # Because invisible_captcha would prevent submitting this form
-            # within 4 seconds, sleep here. This can be removed once we
-            # implement invisible_captcha as an application setting instead
-            # of a feature flag, so we can turn it off while testing.
-            # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/284113
-            sleep 4
+        def fill_new_user_last_name_field(last_name)
+          fill_element :new_user_last_name_field, last_name
+        end
 
-            click_element :new_user_register_button if has_element?(:new_user_register_button)
-            click_element :get_started_button if has_element?(:get_started_button)
+        def fill_new_user_username_field(username)
+          fill_element :new_user_username_field, username
+        end
 
-            Page::Main::Menu.perform(&:has_personal_area?)
-          end
+        def fill_new_user_email_field(email)
+          fill_element :new_user_email_field, email
+        end
 
-          raise "Failed to register and sign in" unless signed_in
+        def fill_new_user_password_field(password)
+          fill_element :new_user_password_field, password
+        end
+
+        def click_new_user_register_button
+          click_element :new_user_register_button if has_element?(:new_user_register_button)
         end
       end
     end
   end
 end
-
-QA::Page::Main::SignUp.prepend_if_ee('QA::EE::Page::Main::SignUp')

qa/qa/page/registration/welcome.rb

+# frozen_string_literal: true
+
+module QA
+  module Page
+    module Registration
+      class Welcome < Page::Base
+        view 'app/views/registrations/welcome/show.html.haml' do
+          element :get_started_button
+        end
+
+        def click_get_started_button_if_available
+          click_element :get_started_button if has_element?(:get_started_button)
+        end
+      end
+    end
+  end
+end
+
+QA::Page::Registration::Welcome.prepend_if_ee('QA::EE::Page::Registration::Welcome')

qa/qa/resource/user.rb

@@ -7,7 +7,7 @@ module QA
     class User < Base
       attr_reader :unique_id
       attr_writer :username, :password
-      attr_accessor :admin, :provider, :extern_uid
+      attr_accessor :admin, :provider, :extern_uid, :expect_fabrication_success
 
       attribute :id
       attribute :name
@@ -18,6 +18,7 @@ module QA
       def initialize
         @admin = false
         @unique_id = SecureRandom.hex(8)
+        @expect_fabrication_success = true
       end
 
       def admin?
@@ -74,12 +75,7 @@ module QA
             login.sign_in_using_credentials(user: self)
           end
         else
-          Page::Main::Login.perform do |login|
-            login.switch_to_register_page
-          end
-          Page::Main::SignUp.perform do |signup|
-            signup.sign_up!(self)
-          end
+          Flow::SignUp.sign_up!(self)
         end
       end
 

qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb

@@ -26,7 +26,7 @@ module QA
         ldap_username = Runtime::Env.ldap_username
         Runtime::Env.ldap_username = nil
 
-        disable_require_admin_approval_after_user_signup
+        set_require_admin_approval_after_user_signup_via_api(false)
 
         Runtime::Env.ldap_username = ldap_username
       end
@@ -39,60 +39,135 @@ module QA
     end
 
     describe 'standard', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/936' do
-      before(:all) do
-        disable_require_admin_approval_after_user_signup
-      end
+      context 'when admin approval is not required' do
+        before(:all) do
+          set_require_admin_approval_after_user_signup_via_api(false)
+        end
 
-      it_behaves_like 'registration and login'
+        it_behaves_like 'registration and login'
+
+        context 'when user account is deleted' do
+          let(:user) do
+            Resource::User.fabricate_via_api! do |resource|
+              resource.api_client = admin_api_client
+            end
+          end
+
+          before do
+            # Use the UI instead of API to delete the account since
+            # this is the only test that exercise this UI.
+            # Other tests should use the API for this purpose.
+            Flow::Login.sign_in(as: user)
+            Page::Main::Menu.perform(&:click_settings_link)
+            Page::Profile::Menu.perform(&:click_account)
+            Page::Profile::Accounts::Show.perform do |show|
+              show.delete_account(user.password)
+            end
+          end
+
+          it 'allows recreating with same credentials', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/937' do
+            expect(Page::Main::Menu.perform(&:signed_in?)).to be_falsy
+
+            Flow::Login.sign_in(as: user, skip_page_validation: true)
 
-      context 'when user account is deleted' do
-        let(:user) do
-          Resource::User.fabricate_via_api! do |resource|
-            resource.api_client = admin_api_client
+            expect(page).to have_text("Invalid Login or password")
+
+            @recreated_user = Resource::User.fabricate_via_browser_ui! do |resource|
+              resource.name = user.name
+              resource.username = user.username
+              resource.email = user.email
+            end
+
+            expect(Page::Main::Menu.perform(&:signed_in?)).to be_truthy
+          end
+
+          after do
+            @recreated_user.remove_via_api!
+          end
+
+          def admin_api_client
+            @admin_api_client ||= Runtime::API::Client.as_admin
           end
         end
+      end
+
+      context 'when admin approval is required' do
+        let(:signed_up_waiting_approval_text) { 'You have signed up successfully. However, we could not sign you in because your account is awaiting approval from your GitLab administrator.' }
+        let(:pending_approval_blocked_text) { 'Your account is pending approval from your GitLab administrator and hence blocked. Please contact your GitLab administrator if you think this is an error.' }
 
         before do
-          # Use the UI instead of API to delete the account since
-          # this is the only test that exercise this UI.
-          # Other tests should use the API for this purpose.
-          Flow::Login.sign_in(as: user)
-          Page::Main::Menu.perform(&:click_settings_link)
-          Page::Profile::Menu.perform(&:click_account)
-          Page::Profile::Accounts::Show.perform do |show|
-            show.delete_account(user.password)
+          enable_require_admin_approval_after_user_signup_via_ui
+
+          @user = Resource::User.fabricate_via_browser_ui! do |user|
+            user.expect_fabrication_success = false
           end
         end
 
-        it 'allows recreating with same credentials', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/937' do
-          expect(Page::Main::Menu.perform(&:signed_in?)).to be_falsy
+        it 'allows user login after approval', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1076' do
+          expect(page).to have_text(signed_up_waiting_approval_text)
 
-          Flow::Login.sign_in(as: user, skip_page_validation: true)
+          Flow::Login.sign_in(as: @user, skip_page_validation: true)
 
-          expect(page).to have_text("Invalid Login or password")
+          expect(page).to have_text(pending_approval_blocked_text)
 
-          @recreated_user = Resource::User.fabricate_via_browser_ui! do |resource|
-            resource.name = user.name
-            resource.username = user.username
-            resource.email = user.email
-          end
+          approve_user(@user)
+
+          Flow::Login.sign_in(as: @user, skip_page_validation: true)
+
+          Page::Registration::Welcome.perform(&:click_get_started_button_if_available)
 
-          expect(Page::Main::Menu.perform(&:signed_in?)).to be_truthy
+          Page::Main::Menu.perform do |menu|
+            expect(menu).to have_personal_area
+          end
         end
 
         after do
-          @recreated_user.remove_via_api!
+          @user.remove_via_api! if @user
+        end
+      end
+    end
+
+    def approve_user(user)
+      Flow::Login.while_signed_in_as_admin do
+        Page::Main::Menu.perform(&:go_to_admin_area)
+        Page::Admin::Menu.perform(&:go_to_users_overview)
+        Page::Admin::Overview::Users::Index.perform do |index|
+          index.click_pending_approval_tab
+          index.search_user(user.username)
+          index.click_user(user.username)
         end
 
-        def admin_api_client
-          @admin_api_client ||= Runtime::API::Client.as_admin
+        Page::Admin::Overview::Users::Show.perform do |show|
+          user.id = show.user_id.to_i
+          show.approve_user
         end
+
+        expect(page).to have_text('Successfully approved')
       end
     end
 
-    def disable_require_admin_approval_after_user_signup
-      Runtime::ApplicationSettings.set_application_settings(require_admin_approval_after_user_signup: false)
+    def set_require_admin_approval_after_user_signup_via_api(enable_or_disable)
+      return if Runtime::ApplicationSettings.get_application_settings[:require_admin_approval_after_user_signup] == enable_or_disable
+
+      Runtime::ApplicationSettings.set_application_settings(require_admin_approval_after_user_signup: enable_or_disable)
+
       sleep 10 # It takes a moment for the setting to come into effect
     end
+
+    def enable_require_admin_approval_after_user_signup_via_ui
+      unless Runtime::ApplicationSettings.get_application_settings[:require_admin_approval_after_user_signup]
+        Flow::Login.while_signed_in_as_admin do
+          Page::Main::Menu.perform(&:go_to_admin_area)
+          QA::Page::Admin::Menu.perform(&:go_to_general_settings)
+          Page::Admin::Settings::General.perform do |setting|
+            setting.expand_sign_up_restrictions do |settings|
+              settings.require_admin_approval_after_user_signup
+            end
+          end
+        end
+
+        sleep 10 # It takes a moment for the setting to come into effect
+      end
+    end
   end
 end