Merge branch 'expose_is_using_seat_for_group_members' into 'master'

Expose is_using_seat attribute for Member in API

See merge request gitlab-org/gitlab!21496

app/models/user.rb

@@ -101,6 +101,7 @@ class User < ApplicationRecord
 
   # Groups
   has_many :members
+  has_one  :max_access_level_membership, -> { select(:id, :user_id, :access_level).order(access_level: :desc).readonly }, class_name: 'Member'
   has_many :group_members, -> { where(requested_at: nil) }, source: 'GroupMember'
   has_many :groups, through: :group_members
   has_many :owned_groups, -> { where(members: { access_level: Gitlab::Access::OWNER }) }, through: :group_members, source: :group
@@ -1027,7 +1028,7 @@ class User < ApplicationRecord
   end
 
   def highest_role
-    members.maximum(:access_level) || Gitlab::Access::NO_ACCESS
+    max_access_level_membership&.access_level || Gitlab::Access::NO_ACCESS
   end
 
   def accessible_deploy_keys

changelogs/unreleased/expose_is_using_seat_for_group_members.yml

+---
+title: Expose is_using_seat attribute for Member in API
+merge_request: 21496
+author:
+type: added

ee/lib/ee/api/entities.rb

@@ -93,6 +93,9 @@ module EE
           expose :group_saml_identity,
                  using: ::API::Entities::Identity,
                  if: -> (member, options) { Ability.allowed?(options[:current_user], :read_group_saml_identity, member.source) }
+          expose :is_using_seat, if: -> (member, options) { options[:show_seat_info] } do |member, _options|
+            !!member.user&.using_license_seat?
+          end
         end
       end
 

ee/lib/ee/api/helpers/members_helpers.rb

@@ -17,6 +17,7 @@ module EE
         override :retrieve_members
         def retrieve_members(source, params:, deep: false)
           members = super
+          members = members.includes(user: :max_access_level_membership)
 
           if can_view_group_identity?(source)
             members = members.includes(user: :group_saml_identities)

ee/spec/requests/api/members_spec.rb

@@ -29,6 +29,13 @@ describe API::Members do
   end
 
   describe 'GET /groups/:id/members' do
+    it 'matches json schema' do
+      get api("/groups/#{group.to_param}/members", owner)
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(response).to match_response_schema('public_api/v4/members')
+    end
+
     context 'when a group has SAML provider configured' do
       let(:maintainer) { create(:user) }
 
@@ -74,6 +81,40 @@ describe API::Members do
         end
       end
     end
+
+    context 'with is_using_seat' do
+      shared_examples 'seat information not included' do
+        it 'returns a list of users that does not contain the is_using_seat attribute' do
+          get api(api_url, owner)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response.size).to eq(1)
+          expect(json_response.first.keys).not_to include('is_using_seat')
+        end
+      end
+
+      context 'with show_seat_info set to true' do
+        it 'returns a list of users that contains the is_using_seat attribute' do
+          get api("/groups/#{group.to_param}/members?show_seat_info=true", owner)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response.size).to eq(1)
+          expect(json_response.first['is_using_seat']).to be_truthy
+        end
+      end
+
+      context 'with show_seat_info set to false' do
+        let(:api_url) { "/groups/#{group.to_param}/members?show_seat_info=false" }
+
+        it_behaves_like 'seat information not included'
+      end
+
+      context 'with no show_seat_info set' do
+        let(:api_url) { "/groups/#{group.to_param}/members" }
+
+        it_behaves_like 'seat information not included'
+      end
+    end
   end
 
   shared_examples 'POST /:source_type/:id/members' do |source_type|

lib/api/helpers/members_helpers.rb

@@ -46,7 +46,7 @@ module API
       end
 
       def present_members(members)
-        present members, with: Entities::Member, current_user: current_user
+        present members, with: Entities::Member, current_user: current_user, show_seat_info: params[:show_seat_info]
       end
     end
   end

lib/api/members.rb

@@ -19,6 +19,7 @@ module API
         params do
           optional :query, type: String, desc: 'A query string to search for members'
           optional :user_ids, type: Array[Integer], desc: 'Array of user ids to look up for membership'
+          optional :show_seat_info, type: Boolean, desc: 'Show seat information for members'
           use :optional_filter_params_ee
           use :pagination
         end
@@ -37,6 +38,7 @@ module API
         params do
           optional :query, type: String, desc: 'A query string to search for members'
           optional :user_ids, type: Array[Integer], desc: 'Array of user ids to look up for membership'
+          optional :show_seat_info, type: Boolean, desc: 'Show seat information for members'
           use :pagination
         end
 

spec/fixtures/api/schemas/public_api/v4/members.json

+{
+  "type": "array",
+  "items": {
+    "type": "object",
+    "properties" : {
+      "id": { "type": "integer" },
+      "name": { "type": "string" },
+      "username": { "type": "string" },
+      "state": { "type": "string" },
+      "avatar_url": { "type": ["string", "null"] },
+      "web_url": { "type": ["string", "null"] },
+      "access_level": { "type": "integer" },
+      "expires_at": { "type": ["date", "null"] },
+      "is_using_seat": { "type": "boolean" }
+    },
+    "required": [
+      "id", "name", "username", "state",
+      "web_url", "access_level", "expires_at"
+    ],
+    "additionalProperties": false
+  }
+}

spec/models/user_spec.rb

@@ -25,6 +25,7 @@ describe User, :do_not_mock_admin_mode do
   describe 'associations' do
     it { is_expected.to have_one(:namespace) }
     it { is_expected.to have_one(:status) }
+    it { is_expected.to have_one(:max_access_level_membership) }
     it { is_expected.to have_many(:snippets).dependent(:destroy) }
     it { is_expected.to have_many(:members) }
     it { is_expected.to have_many(:project_members) }
@@ -839,9 +840,36 @@ describe User, :do_not_mock_admin_mode do
 
   describe '#highest_role' do
     let(:user) { create(:user) }
-
     let(:group) { create(:group) }
 
+    context 'with association :max_access_level_membership' do
+      let(:another_user) { create(:user) }
+
+      before do
+        create(:project, group: group) do |project|
+          group.add_user(user, GroupMember::GUEST)
+          group.add_user(another_user, GroupMember::DEVELOPER)
+        end
+
+        create(:project, group: create(:group)) do |project|
+          project.add_guest(another_user)
+        end
+
+        create(:project, group: create(:group)) do |project|
+          project.add_maintainer(user)
+        end
+      end
+
+      it 'returns the correct highest role' do
+        users = User.includes(:max_access_level_membership).where(id: [user.id, another_user.id])
+
+        expect(users.collect { |u| [u.id, u.highest_role] }).to contain_exactly(
+          [user.id, Gitlab::Access::MAINTAINER],
+          [another_user.id, Gitlab::Access::DEVELOPER]
+        )
+      end
+    end
+
     it 'returns NO_ACCESS if none has been set' do
       expect(user.highest_role).to eq(Gitlab::Access::NO_ACCESS)
     end