Adds email types for api/users/:id/emails53618-admin-api-get-users-id-emails-is-inconsistent-w-documentation

By using `types[]` parameter the user can obtain the
`primary`, `secondary` or both types of emails.
The `primary` is user.email and `secondary` are user.emails.

10 files changed, 134 insertions, 8 deletions

diff --git a/app/finders/user_emails_finder.rb b/app/finders/user_emails_finder.rb
new file mode 100644
index 00000000000..4f0215a3707
--- /dev/null
+++ b/app/finders/user_emails_finder.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+# Finds the user emails.
+#
+# Arguments:
+#   user - which user use
+#   types: Array - email types (primary/secondary) the primary is user.email,
+#   the secondary are user.emails.
+#
+# Returns an Email(id, email attributes only) ActiveRecord::Relation with the id set to nil
+# for the primary email.
+#
+# Using pluck in this finder is not supported because we are using select NULL as id and pluck
+# overrides the select statement.
+class UserEmailsFinder < UnionFinder
+  attr_reader :user, :params
+
+  def initialize(user, params = {})
+    @user = user
+    @params = params
+  end
+
+  def execute
+    return Email.none unless params[:types].present?
+
+    find_union(all_emails_by_types, Email).order_id_asc_nulls_first
+  end
+
+  private
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def all_emails_by_types
+    emails = []
+
+    emails << user.class.where(id: user).select('NULL as id, email') if params[:types].include?('primary')
+    emails << user.emails.select('id, email') if params[:types].include?('secondary')
+
+    emails
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+end
diff --git a/app/models/concerns/null_sorting.rb b/app/models/concerns/null_sorting.rb
new file mode 100644
index 00000000000..294e6b7d073
--- /dev/null
+++ b/app/models/concerns/null_sorting.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module NullSorting
+  extend ActiveSupport::Concern
+
+  included do
+    scope :order_id_asc_nulls_first, -> { order(Gitlab::Database.nulls_first_order('id', 'ASC')) }
+  end
+end
diff --git a/app/models/email.rb b/app/models/email.rb
index 0ddaa049c3b..10424128943 100644
--- a/app/models/email.rb
+++ b/app/models/email.rb
@@ -1,8 +1,12 @@
 # frozen_string_literal: true
 
 class Email < ApplicationRecord
+  EMAIL_TYPES = %w[primary secondary].freeze
+
   include Sortable
+  include NullSorting
   include Gitlab::SQL::Pattern
+  include FromUnion
 
   belongs_to :user
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 60f69659a6b..b82bffd02ac 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -11,6 +11,7 @@ class User < ApplicationRecord
   include Avatarable
   include Referable
   include Sortable
+  include NullSorting
   include CaseSensitivity
   include TokenAuthenticatable
   include IgnorableColumn
diff --git a/changelogs/unreleased/53618-admin-api-get-users-id-emails-is-inconsistent-w-documentation.yml b/changelogs/unreleased/53618-admin-api-get-users-id-emails-is-inconsistent-w-documentation.yml
new file mode 100644
index 00000000000..c62b52967f7
--- /dev/null
+++ b/changelogs/unreleased/53618-admin-api-get-users-id-emails-is-inconsistent-w-documentation.yml
@@ -0,0 +1,5 @@
+---
+title: Adds email types for api/users/:id/emails
+merge_request: 27904
+author: Jacopo Beschi @jacopo-beschi
+type: changed
diff --git a/doc/api/users.md b/doc/api/users.md
index d3e67d3d510..0b051c13b5e 100644
--- a/doc/api/users.md
+++ b/doc/api/users.md
@@ -918,6 +918,7 @@ GET /users/:id/emails
 Parameters:
 
 - `id` (required) - id of specified user
+- `types` (optional) - the email types: `primary` or `secondary` (default)
 
 ## Single email
 
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 625fada4f08..442d3cd456b 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -99,7 +99,8 @@ module API
     end
 
     class Email < Grape::Entity
-      expose :id, :email
+      expose :id, unless: ->(email) { email.id.nil? }
+      expose :email
     end
 
     class Hook < Grape::Entity
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 2f23e33bd4a..821f13dfd45 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -418,17 +418,18 @@ module API
       end
       params do
         requires :id, type: Integer, desc: 'The ID of the user'
+        optional :types, type: Array, values: Email::EMAIL_TYPES, desc: "Type of the email: #{Email::EMAIL_TYPES.join(',')}", default: 'secondary'
         use :pagination
       end
-      # rubocop: disable CodeReuse/ActiveRecord
       get ':id/emails' do
         authenticated_as_admin!
-        user = User.find_by(id: params[:id])
+        user = User.find_by(id: params[:id]) # rubocop: disable CodeReuse/ActiveRecord
         not_found!('User') unless user
 
-        present paginate(user.emails), with: Entities::Email
+        emails = UserEmailsFinder.new(user, types: params[:types]).execute
+
+        present paginate(emails), with: Entities::Email
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       desc 'Delete an email address of a specified user. Available only for admins.' do
         success Entities::Email
diff --git a/spec/finders/user_emails_finder_spec.rb b/spec/finders/user_emails_finder_spec.rb
new file mode 100644
index 00000000000..b9f512b3a6b
--- /dev/null
+++ b/spec/finders/user_emails_finder_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe UserEmailsFinder do
+  describe '#execute' do
+    let(:user) { create(:user) }
+    let!(:email1) { create(:email, user: user) }
+    let!(:email2) { create(:email, user: user) }
+
+    def slice_email_attributes(emails)
+      emails.map { |email| email.slice('id', 'email') }.map(&:symbolize_keys)
+    end
+
+    it 'returns the primary email with nil id when types is primary' do
+      emails = described_class.new(user.reload, types: %w[primary]).execute
+
+      expect(slice_email_attributes(emails)).to eq([{ id: nil, email: user.email }])
+    end
+
+    it 'returns the secondary emails when types is secondary' do
+      emails = described_class.new(user.reload, types: %w[secondary]).execute
+
+      expect(slice_email_attributes(emails)).to eq([{ id: email1.id, email: email1.email },
+                                                    { id: email2.id, email: email2.email }])
+    end
+
+    it 'returns both primary and secondary emails when type is primary,secondary sorted by id asc null first' do
+      emails = described_class.new(user.reload, types: %w[primary secondary]).execute
+
+      expect(slice_email_attributes(emails)).to eq([{ id: nil, email: user.email },
+                                                    { id: email1.id, email: email1.email },
+                                                    { id: email2.id, email: email2.email }])
+    end
+
+    it 'returns an empty relation when types is empty' do
+      emails = described_class.new(user.reload, types: []).execute
+
+      expect(emails).to be_empty
+    end
+  end
+end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index b84202364e1..0cea02fcceb 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -1146,18 +1146,39 @@ describe API::Users do
         expect(json_response['message']).to eq('404 User Not Found')
       end
 
-      it 'returns array of emails' do
-        user.emails << email
-        user.save
+      it 'returns array of secondary emails' do
+        email
 
         get api("/users/#{user.id}/emails", admin)
 
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
         expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
         expect(json_response.first['email']).to eq(email.email)
       end
 
+      it 'returns only the primary email when types[]=primary' do
+        email
+
+        get api("/users/#{user.id}/emails", admin), params: { types: ['primary'] }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response.length).to eq(1)
+        expect(json_response.first).to eq({ 'email' => user.email })
+      end
+
+      it 'returns both primary and secondary emails when types[]=primary,secondary' do
+        email
+
+        get api("/users/#{user.id}/emails", admin), params: { types: %w[primary secondary] }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response.length).to eq(2)
+        expect(json_response.first).to  eq({ 'email' => user.email })
+        expect(json_response.second).to eq({ 'id' => email.id, 'email' => email.email })
+      end
+
       it "returns a 404 for invalid ID" do
         get api("/users/ASDF/emails", admin)