Merge branch '41673-blank-query-members-api' into 'master'

Resolve "Project/#/Members?query= blank causes 500 error"

Closes #41673

See merge request gitlab-org/gitlab-ce!16235

7 files changed, 47 insertions, 2 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 09aa5a7b318..9403da98268 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -318,6 +318,8 @@ class User < ActiveRecord::Base
     #
     # Returns an ActiveRecord::Relation.
     def search(query)
+      return none if query.blank?
+
       query = query.downcase
 
       order = <<~SQL
@@ -341,6 +343,8 @@ class User < ActiveRecord::Base
     # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
 
     def search_with_secondary_emails(query)
+      return none if query.blank?
+
       query = query.downcase
 
       email_table = Email.arel_table
diff --git a/changelogs/unreleased/41673-blank-query-members-api.yml b/changelogs/unreleased/41673-blank-query-members-api.yml
new file mode 100644
index 00000000000..677c5e250c8
--- /dev/null
+++ b/changelogs/unreleased/41673-blank-query-members-api.yml
@@ -0,0 +1,5 @@
+---
+title: Fix error on empty query for Members API
+merge_request: 16235
+author:
+type: fixed
diff --git a/lib/api/members.rb b/lib/api/members.rb
index 5446f6b54b1..130c6d6da71 100644
--- a/lib/api/members.rb
+++ b/lib/api/members.rb
@@ -22,7 +22,7 @@ module API
           source = find_source(source_type, params[:id])
 
           users = source.users
-          users = users.merge(User.search(params[:query])) if params[:query]
+          users = users.merge(User.search(params[:query])) if params[:query].present?
 
           present paginate(users), with: Entities::Member, source: source
         end
diff --git a/lib/api/v3/members.rb b/lib/api/v3/members.rb
index de226e4e573..46145cac7a5 100644
--- a/lib/api/v3/members.rb
+++ b/lib/api/v3/members.rb
@@ -23,7 +23,7 @@ module API
             source = find_source(source_type, params[:id])
 
             users = source.users
-            users = users.merge(User.search(params[:query])) if params[:query]
+            users = users.merge(User.search(params[:query])) if params[:query].present?
 
             present paginate(users), with: ::API::Entities::Member, source: source
           end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 8d0eaf565a7..762cec9b95e 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -966,6 +966,14 @@ describe User do
         expect(described_class.search(user3.username.upcase)).to eq([user3])
       end
     end
+
+    it 'returns no matches for an empty string' do
+      expect(described_class.search('')).to be_empty
+    end
+
+    it 'returns no matches for nil' do
+      expect(described_class.search(nil)).to be_empty
+    end
   end
 
   describe '.search_with_secondary_emails' do
@@ -1020,6 +1028,14 @@ describe User do
     it 'does not return users with a matching part of secondary email' do
       expect(search_with_secondary_emails(email.email[1..4])).not_to include([email.user])
     end
+
+    it 'returns no matches for an empty string' do
+      expect(search_with_secondary_emails('')).to be_empty
+    end
+
+    it 'returns no matches for nil' do
+      expect(search_with_secondary_emails(nil)).to be_empty
+    end
   end
 
   describe '.find_by_ssh_key_id' do
diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb
index 5d4f81e07a6..73bd4785b11 100644
--- a/spec/requests/api/members_spec.rb
+++ b/spec/requests/api/members_spec.rb
@@ -65,6 +65,16 @@ describe API::Members do
         expect(json_response.count).to eq(1)
         expect(json_response.first['username']).to eq(master.username)
       end
+
+      it 'finds all members with no query specified' do
+        get api("/#{source_type.pluralize}/#{source.id}/members", developer), query: ''
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.count).to eq(2)
+        expect(json_response.map { |u| u['id'] }).to match_array [master.id, developer.id]
+      end
     end
   end
 
diff --git a/spec/requests/api/v3/members_spec.rb b/spec/requests/api/v3/members_spec.rb
index b91782ae511..de4339ecb8b 100644
--- a/spec/requests/api/v3/members_spec.rb
+++ b/spec/requests/api/v3/members_spec.rb
@@ -58,6 +58,16 @@ describe API::V3::Members do
         expect(json_response.count).to eq(1)
         expect(json_response.first['username']).to eq(master.username)
       end
+
+      it 'finds all members with no query specified' do
+        get v3_api("/#{source_type.pluralize}/#{source.id}/members", developer), query: ''
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.count).to eq(2)
+        expect(json_response.map { |u| u['id'] }).to match_array [master.id, developer.id]
+      end
     end
   end