Merge branch '241144-fj-add-non-internal-filter-to-users-rest-endpoint' into 'master'

Add filter to exclude non internal users in REST API

See merge request gitlab-org/gitlab!40372

app/finders/users_finder.rb

@@ -17,6 +17,7 @@
 #     without_projects: boolean
 #     sort: string
 #     id: integer
+#     non_internal: boolean
 #
 class UsersFinder
   include CreatedAtFilter
@@ -42,6 +43,7 @@ class UsersFinder
     users = by_created_at(users)
     users = by_without_projects(users)
     users = by_custom_attributes(users)
+    users = by_non_internal(users)
 
     order(users)
   end
@@ -112,6 +114,12 @@ class UsersFinder
     users.without_projects
   end
 
+  def by_non_internal(users)
+    return users unless params[:non_internal]
+
+    users.non_internal
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def order(users)
     return users unless params[:sort]

changelogs/unreleased/241144-fj-add-non-internal-filter-to-users-rest-endpoint.yml

+---
+title: Add filter to exclude non internal users in REST API
+merge_request: 40372
+author:
+type: changed

doc/api/users.md

@@ -61,6 +61,15 @@ GET /users?active=true
 GET /users?blocked=true
 ```
 
+GitLab supports bot users such as the [alert bot](../operations/incident_management/generic_alerts.md)
+or the [support bot](../user/project/service_desk.md#support-bot-user).
+To exclude these users from the users' list, you can use the parameter `exclude_internal=true`
+([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/241144) in GitLab 13.4).
+
+```plaintext
+GET /users?exclude_internal=true
+```
+
 NOTE: **Note:**
 Username search is case insensitive.
 

ee/spec/finders/users_finder_spec.rb

@@ -13,13 +13,13 @@ RSpec.describe UsersFinder do
         it 'returns ldap users by default' do
           users = described_class.new(normal_user).execute
 
-          expect(users).to contain_exactly(normal_user, blocked_user, omniauth_user, ldap_user)
+          expect(users).to contain_exactly(normal_user, blocked_user, omniauth_user, ldap_user, internal_user)
         end
 
         it 'returns only non-ldap users with skip_ldap: true' do
           users = described_class.new(normal_user, skip_ldap: true).execute
 
-          expect(users).to contain_exactly(normal_user, blocked_user, omniauth_user)
+          expect(users).to contain_exactly(normal_user, blocked_user, omniauth_user, internal_user)
         end
       end
     end

lib/api/users.rb

@@ -84,6 +84,7 @@ module API
         optional :created_after, type: DateTime, desc: 'Return users created after the specified time'
         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'
         all_or_none_of :extern_uid, :provider
 
         use :sort_params

spec/finders/users_finder_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe UsersFinder do
       it 'returns all users' do
         users = described_class.new(user).execute
 
-        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user)
+        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user)
       end
 
       it 'filters by username' do
@@ -54,7 +54,7 @@ RSpec.describe UsersFinder do
       it 'returns no external users' do
         users = described_class.new(user, external: true).execute
 
-        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user)
+        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user)
       end
 
       it 'filters by created_at' do
@@ -68,19 +68,25 @@ RSpec.describe UsersFinder do
         expect(users.map(&:username)).not_to include([filtered_user_before.username, filtered_user_after.username])
       end
 
+      it 'filters by non internal users' do
+        users = described_class.new(user, non_internal: true).execute
+
+        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user)
+      end
+
       it 'does not filter by custom attributes' do
         users = described_class.new(
           user,
           custom_attributes: { foo: 'bar' }
         ).execute
 
-        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user)
+        expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user)
       end
 
       it 'orders returned results' do
         users = described_class.new(user, sort: 'id_asc').execute
 
-        expect(users).to eq([normal_user, blocked_user, omniauth_user, user])
+        expect(users).to eq([normal_user, blocked_user, omniauth_user, internal_user, user])
       end
     end
 
@@ -96,13 +102,14 @@ 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)
+        expect(users).to contain_exactly(admin, normal_user, blocked_user, external_user, omniauth_user, internal_user)
       end
 
       it 'filters by custom attributes' do
         create :user_custom_attribute, user: normal_user, key: 'foo', value: 'foo'
         create :user_custom_attribute, user: normal_user, key: 'bar', value: 'bar'
         create :user_custom_attribute, user: blocked_user, key: 'foo', value: 'foo'
+        create :user_custom_attribute, user: internal_user, key: 'foo', value: 'foo'
 
         users = described_class.new(
           admin,

spec/requests/api/users_spec.rb

@@ -348,6 +348,26 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do
         expect(response).to match_response_schema('public_api/v4/user/basics')
         expect(json_response.first.keys).not_to include 'is_admin'
       end
+
+      context 'exclude_internal param' do
+        let_it_be(:internal_user) { User.alert_bot }
+
+        it 'returns all users when it is not set' do
+          get api("/users?exclude_internal=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(internal_user.id)
+        end
+
+        it 'returns all non internal users when it is set' do
+          get api("/users?exclude_internal=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(internal_user.id)
+        end
+      end
     end
 
     context "when admin" do

spec/support/shared_contexts/finders/users_finder_shared_contexts.rb

@@ -5,4 +5,5 @@ RSpec.shared_context 'UsersFinder#execute filter by project context' do
   let_it_be(:blocked_user) { create(:user, :blocked, username: 'notsorandom') }
   let_it_be(:external_user) { create(:user, :external) }
   let_it_be(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') }
+  let_it_be(:internal_user) { User.alert_bot }
 end