Merge branch...

Merge branch '355659-inviting-project-bots-project-access-token-users-to-be-members-of-projects-groups' into 'master'

Filters out project bots from invite member selections

See merge request gitlab-org/gitlab!83651

app/assets/javascripts/invite_members/components/members_token_select.vue

@@ -117,7 +117,7 @@ export default {
       this.$emit('clear');
     },
   },
-  defaultQueryOptions: { exclude_internal: true, active: true },
+  defaultQueryOptions: { without_project_bots: true, active: true },
   i18n: {
     inviteTextMessage: __('Invite "%{email}" by email'),
   },

app/finders/users_finder.rb

@@ -47,6 +47,7 @@ class UsersFinder
     users = by_without_projects(users)
     users = by_custom_attributes(users)
     users = by_non_internal(users)
+    users = by_without_project_bots(users)
 
     order(users)
   end
@@ -138,6 +139,12 @@ class UsersFinder
     users.non_internal
   end
 
+  def by_without_project_bots(users)
+    return users unless params[:without_project_bots]
+
+    users.without_project_bot
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def order(users)
     return users unless params[:sort]

doc/api/users.md

@@ -97,6 +97,14 @@ In addition, to exclude external users from the users' list, you can use the par
 GET /users?exclude_external=true
 ```
 
+To exclude [bot users for projects](../user/project/settings/project_access_tokens.md#bot-users-for-projects)
+and [bot users for groups](../user/group/settings/group_access_tokens.md#bot-users-for-groups), you can use the
+parameter `without_project_bots=true`.
+
+```plaintext
+GET /users?without_project_bots=true
+```
+
 ### For admins
 
 > The `namespace_id` field in the response was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82045) in GitLab 14.10.

ee/spec/features/groups/members/list_members_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 
 RSpec.describe 'Groups > Members > List members' do
   include Spec::Support::Helpers::Features::MembersHelpers
+  include Spec::Support::Helpers::Features::InviteMembersModalHelper
 
   let_it_be(:user1) { create(:user, name: 'John Doe') }
   let_it_be(:user2) { create(:user, name: 'Mary Jane') }
@@ -66,8 +67,8 @@ RSpec.describe 'Groups > Members > List members' do
 
       click_on 'Invite members'
 
-      page.within '[data-testid="invite-modal"]' do
-        field = find('[data-testid="members-token-select-input"]')
+      page.within invite_modal_selector do
+        field = find(member_dropdown_selector)
         field.native.send_keys :tab
         field.click
 

ee/spec/features/groups/members/manage_members_spec.rb

@@ -58,7 +58,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do
 
         click_on 'Invite members'
 
-        page.within '[data-testid="invite-modal"]' do
+        page.within invite_modal_selector do
           add_user_to_input(user2.name)
           add_user_to_input(user3.name)
 
@@ -118,7 +118,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do
 
     click_on 'Invite members'
 
-    page.within '[data-testid="invite-modal"]' do
+    page.within invite_modal_selector do
       add_user_to_input(name)
       choose_options(role, nil)
 
@@ -127,7 +127,7 @@ RSpec.describe 'Groups > Members > Manage members', :saas, :js do
   end
 
   def add_user_to_input(name)
-    find('[data-testid="members-token-select-input"]').set(name)
+    find(member_dropdown_selector).set(name)
 
     wait_for_requests
     click_button name

lib/api/users.rb

@@ -89,6 +89,7 @@ module API
         optional :created_before, type: DateTime, desc: 'Return users created before the specified time'
         optional :without_projects, type: Boolean, default: false, desc: 'Filters only users without projects'
         optional :exclude_internal, as: :non_internal, type: Boolean, default: false, desc: 'Filters only non internal users'
+        optional :without_project_bots, type: Boolean, default: false, desc: 'Filters users without project bots'
         optional :admins, type: Boolean, default: false, desc: 'Filters only admin users'
         all_or_none_of :extern_uid, :provider
 

spec/features/groups/members/manage_members_spec.rb

@@ -63,25 +63,6 @@ RSpec.describe 'Groups > Members > Manage members' do
     )
   end
 
-  it 'do not disclose email addresses', :js do
-    group.add_owner(user1)
-    create(:user, email: 'undisclosed_email@gitlab.com', name: "Jane 'invisible' Doe")
-
-    visit group_group_members_path(group)
-
-    click_on 'Invite members'
-    find('[data-testid="members-token-select-input"]').set('@gitlab.com')
-
-    wait_for_requests
-
-    expect(page).to have_content('No matches found')
-
-    find('[data-testid="members-token-select-input"]').set('undisclosed_email@gitlab.com')
-    wait_for_requests
-
-    expect(page).to have_content('Invite "undisclosed_email@gitlab.com" by email')
-  end
-
   it 'remove user from group', :js do
     group.add_owner(user1)
     group.add_developer(user2)
@@ -169,4 +150,57 @@ RSpec.describe 'Groups > Members > Manage members' do
       end
     end
   end
+
+  describe 'member search results', :js do
+    before do
+      group.add_owner(user1)
+    end
+
+    it 'does not disclose email addresses' do
+      create(:user, email: 'undisclosed_email@gitlab.com', name: "Jane 'invisible' Doe")
+
+      visit group_group_members_path(group)
+
+      click_on 'Invite members'
+      find(member_dropdown_selector).set('@gitlab.com')
+
+      wait_for_requests
+
+      expect(page).to have_content('No matches found')
+
+      find(member_dropdown_selector).set('undisclosed_email@gitlab.com')
+      wait_for_requests
+
+      expect(page).to have_content('Invite "undisclosed_email@gitlab.com" by email')
+    end
+
+    it 'does not show project_bots', :aggregate_failures do
+      internal_project_bot = create(:user, :project_bot, name: '_internal_project_bot_')
+      project = create(:project, group: group)
+      project.add_maintainer(internal_project_bot)
+
+      external_group = create(:group)
+      external_project_bot = create(:user, :project_bot, name: '_external_project_bot_')
+      external_project = create(:project, group: external_group)
+      external_project.add_maintainer(external_project_bot)
+      external_project.add_maintainer(user1)
+
+      visit group_group_members_path(group)
+
+      click_on 'Invite members'
+
+      page.within invite_modal_selector do
+        field = find(member_dropdown_selector)
+        field.native.send_keys :tab
+        field.click
+
+        wait_for_requests
+
+        expect(page).to have_content(user1.name)
+        expect(page).to have_content(user2.name)
+        expect(page).not_to have_content(internal_project_bot.name)
+        expect(page).not_to have_content(external_project_bot.name)
+      end
+    end
+  end
 end

spec/features/projects/members/list_spec.rb → spec/features/projects/members/manage_members_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Project members list', :js do
+RSpec.describe 'Projects > Members > Manage members', :js do
   include Spec::Support::Helpers::Features::MembersHelpers
   include Spec::Support::Helpers::Features::InviteMembersModalHelper
   include Spec::Support::Helpers::ModalHelpers
@@ -124,6 +124,36 @@ RSpec.describe 'Project members list', :js do
     )
   end
 
+  describe 'member search results' do
+    it 'does not show project_bots', :aggregate_failures do
+      internal_project_bot = create(:user, :project_bot, name: '_internal_project_bot_')
+      project.add_maintainer(internal_project_bot)
+
+      external_group = create(:group)
+      external_project_bot = create(:user, :project_bot, name: '_external_project_bot_')
+      external_project = create(:project, group: external_group)
+      external_project.add_maintainer(external_project_bot)
+      external_project.add_maintainer(user1)
+
+      visit_members_page
+
+      click_on 'Invite members'
+
+      page.within invite_modal_selector do
+        field = find(member_dropdown_selector)
+        field.native.send_keys :tab
+        field.click
+
+        wait_for_requests
+
+        expect(page).to have_content(user1.name)
+        expect(page).to have_content(user2.name)
+        expect(page).not_to have_content(internal_project_bot.name)
+        expect(page).not_to have_content(external_project_bot.name)
+      end
+    end
+  end
+
   context 'as a signed out visitor viewing a public project' do
     let_it_be(:project) { create(:project, :public) }
 

spec/finders/users_finder_spec.rb

@@ -6,13 +6,15 @@ RSpec.describe UsersFinder do
   describe '#execute' do
     include_context 'UsersFinder#execute filter by project context'
 
+    let_it_be(:project_bot) { create(:user, :project_bot) }
+
     context 'with a normal user' do
-      let(:user) { create(:user) }
+      let_it_be(:user) { create(:user) }
 
       it 'returns all users' do
         users = described_class.new(user).execute
 
-        expect(users).to contain_exactly(user, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user)
+        expect(users).to contain_exactly(user, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user, project_bot)
       end
 
       it 'filters by username' do
@@ -54,7 +56,7 @@ RSpec.describe UsersFinder do
       it 'filters by active users' do
         users = described_class.new(user, active: true).execute
 
-        expect(users).to contain_exactly(user, normal_user, external_user, omniauth_user, admin_user)
+        expect(users).to contain_exactly(user, normal_user, external_user, omniauth_user, admin_user, project_bot)
       end
 
       it 'filters by external users' do
@@ -66,7 +68,7 @@ RSpec.describe UsersFinder do
       it 'filters by non external users' do
         users = described_class.new(user, non_external: true).execute
 
-        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user, admin_user)
+        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user, admin_user, project_bot)
       end
 
       it 'filters by created_at' do
@@ -83,7 +85,13 @@ RSpec.describe UsersFinder do
       it 'filters by non internal users' do
         users = described_class.new(user, non_internal: true).execute
 
-        expect(users).to contain_exactly(user, normal_user, external_user, blocked_user, omniauth_user, admin_user)
+        expect(users).to contain_exactly(user, normal_user, external_user, blocked_user, omniauth_user, admin_user, project_bot)
+      end
+
+      it 'filters by without project bots' do
+        users = described_class.new(user, without_project_bots: true).execute
+
+        expect(users).to contain_exactly(user, normal_user, external_user, blocked_user, omniauth_user, internal_user, admin_user)
       end
 
       it 'does not filter by custom attributes' do
@@ -92,23 +100,23 @@ RSpec.describe UsersFinder do
           custom_attributes: { foo: 'bar' }
         ).execute
 
-        expect(users).to contain_exactly(user, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user)
+        expect(users).to contain_exactly(user, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user, project_bot)
       end
 
       it 'orders returned results' do
         users = described_class.new(user, sort: 'id_asc').execute
 
-        expect(users).to eq([normal_user, admin_user, blocked_user, external_user, omniauth_user, internal_user, user])
+        expect(users).to eq([normal_user, admin_user, blocked_user, external_user, omniauth_user, internal_user, project_bot, user])
       end
 
       it 'does not filter by admins' do
         users = described_class.new(user, admins: true).execute
-        expect(users).to contain_exactly(user, normal_user, external_user, admin_user, blocked_user, omniauth_user, internal_user)
+        expect(users).to contain_exactly(user, normal_user, external_user, admin_user, blocked_user, omniauth_user, internal_user, project_bot)
       end
     end
 
     context 'with an admin user', :enable_admin_mode do
-      let(:admin) { create(:admin) }
+      let_it_be(:admin) { create(:admin) }
 
       it 'filters by external users' do
         users = described_class.new(admin, external: true).execute
@@ -119,7 +127,7 @@ RSpec.describe UsersFinder do
       it 'returns all users' do
         users = described_class.new(admin).execute
 
-        expect(users).to contain_exactly(admin, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user)
+        expect(users).to contain_exactly(admin, normal_user, blocked_user, external_user, omniauth_user, internal_user, admin_user, project_bot)
       end
 
       it 'returns only admins' do

spec/frontend/invite_members/components/members_token_select_spec.js

@@ -95,7 +95,7 @@ describe('MembersTokenSelect', () => {
 
         expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, {
           active: true,
-          exclude_internal: true,
+          without_project_bots: true,
         });
         expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false);
       });
@@ -172,7 +172,7 @@ describe('MembersTokenSelect', () => {
 
       expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, {
         active: true,
-        exclude_internal: true,
+        without_project_bots: true,
         saml_provider_id: samlProviderId,
       });
     });

spec/requests/api/users_spec.rb

@@ -359,6 +359,26 @@ RSpec.describe API::Users do
         end
       end
 
+      context 'without_project_bots param' do
+        let_it_be(:project_bot) { create(:user, :project_bot) }
+
+        it 'returns all users when it is not set' do
+          get api("/users?without_project_bots=false", user)
+
+          expect(response).to match_response_schema('public_api/v4/user/basics')
+          expect(response).to include_pagination_headers
+          expect(json_response.map { |u| u['id'] }).to include(project_bot.id)
+        end
+
+        it 'returns all non project_bot users when it is set' do
+          get api("/users?without_project_bots=true", user)
+
+          expect(response).to match_response_schema('public_api/v4/user/basics')
+          expect(response).to include_pagination_headers
+          expect(json_response.map { |u| u['id'] }).not_to include(project_bot.id)
+        end
+      end
+
       context 'admins param' do
         it 'returns all users' do
           get api("/users?admins=true", user)

spec/support/helpers/features/invite_members_modal_helper.rb

@@ -8,8 +8,8 @@ module Spec
           def invite_member(name, role: 'Guest', expires_at: nil)
             click_on 'Invite members'
 
-            page.within '[data-testid="invite-modal"]' do
-              find('[data-testid="members-token-select-input"]').set(name)
+            page.within invite_modal_selector do
+              find(member_dropdown_selector).set(name)
 
               wait_for_requests
               click_button name
@@ -53,6 +53,14 @@ module Spec
             '[data-testid="group-select-dropdown"]'
           end
 
+          def member_dropdown_selector
+            '[data-testid="members-token-select-input"]'
+          end
+
+          def invite_modal_selector
+            '[data-testid="invite-modal"]'
+          end
+
           def expect_to_have_group(group)
             expect(page).to have_selector("[entity-id='#{group.id}']")
           end