From 538741f2303f03e520d0bbfea150da6754f5a995 Mon Sep 17 00:00:00 2001 From: Thiago Presa Date: Fri, 22 Mar 2019 09:54:03 +0000 Subject: Add highest_role method to User --- app/models/user.rb | 4 ++ app/views/admin/users/show.html.haml | 5 ++ .../unreleased/tpresa-add-highest-role-to-user.yml | 5 ++ doc/api/users.md | 3 +- lib/api/entities.rb | 4 ++ lib/api/users.rb | 2 +- lib/gitlab/access.rb | 14 +++++ locale/gitlab.pot | 3 ++ spec/models/user_spec.rb | 62 ++++++++++++++++++++++ spec/requests/api/users_spec.rb | 20 +++++++ 10 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/tpresa-add-highest-role-to-user.yml diff --git a/app/models/user.rb b/app/models/user.rb index 0ebfb9a0ccb..d2be26370ff 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -917,6 +917,10 @@ class User < ApplicationRecord DeployKey.unscoped.in_projects(authorized_projects.pluck(:id)).distinct(:id) end + def highest_role + members.maximum(:access_level) || Gitlab::Access::NO_ACCESS + end + def accessible_deploy_keys @accessible_deploy_keys ||= begin key_ids = project_deploy_keys.pluck(:id) diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index a74e052707f..4ffa8d89504 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -117,6 +117,11 @@ %strong = @user.sign_in_count + %li + %span.light= _("Highest role:") + %strong + = Gitlab::Access.human_access_with_none(@user.highest_role) + - if @user.ldap_user? %li %span.light LDAP uid: diff --git a/changelogs/unreleased/tpresa-add-highest-role-to-user.yml b/changelogs/unreleased/tpresa-add-highest-role-to-user.yml new file mode 100644 index 00000000000..9714d8dcc99 --- /dev/null +++ b/changelogs/unreleased/tpresa-add-highest-role-to-user.yml @@ -0,0 +1,5 @@ +--- +title: Adding highest role property to admin's user details page +merge_request: +author: +type: added diff --git a/doc/api/users.md b/doc/api/users.md index b0977810120..606003a75e2 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -140,7 +140,8 @@ GET /users "can_create_project": true, "two_factor_enabled": true, "external": false, - "private_profile": false + "private_profile": false, + "highest_role":10 } ] ``` diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 2cd0d93b205..611523a2444 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -86,6 +86,10 @@ module API expose :admin?, as: :is_admin end + class UserDetailsWithAdmin < UserWithAdmin + expose :highest_role + end + class UserStatus < Grape::Entity expose :emoji expose :message diff --git a/lib/api/users.rb b/lib/api/users.rb index 7d88880d412..a3d4acc63d5 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -124,7 +124,7 @@ module API user = User.find_by(id: params[:id]) not_found!('User') unless user && can?(current_user, :read_user, user) - opts = { with: current_user&.admin? ? Entities::UserWithAdmin : Entities::User, current_user: current_user } + opts = { with: current_user&.admin? ? Entities::UserDetailsWithAdmin : Entities::User, current_user: current_user } user, opts = with_custom_attributes(user, opts) present user, opts diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index ec090aea784..6c8ca8f219c 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -46,6 +46,12 @@ module Gitlab ) end + def options_with_none + options_with_owner.merge( + "None" => NO_ACCESS + ) + end + def sym_options { guest: GUEST, @@ -75,12 +81,20 @@ module Gitlab def human_access(access) options_with_owner.key(access) end + + def human_access_with_none(access) + options_with_none.key(access) + end end def human_access Gitlab::Access.human_access(access_field) end + def human_access_with_none + Gitlab::Access.human_access_with_none(access_field) + end + def owner? access_field == OWNER end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fe607e36808..177bd189817 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4121,6 +4121,9 @@ msgstr[1] "" msgid "Hide values" msgstr "" +msgid "Highest role:" +msgstr "" + msgid "History" msgstr "" diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 85b157a9435..1be29d039a7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -660,6 +660,68 @@ describe User do end end + describe '#highest_role' do + let(:user) { create(:user) } + + let(:group) { create(:group) } + + it 'returns NO_ACCESS if none has been set' do + expect(user.highest_role).to eq(Gitlab::Access::NO_ACCESS) + end + + it 'returns MAINTAINER if user is maintainer of a project' do + create(:project, group: group) do |project| + project.add_maintainer(user) + end + + expect(user.highest_role).to eq(Gitlab::Access::MAINTAINER) + end + + it 'returns the highest role if user is member of multiple projects' do + create(:project, group: group) do |project| + project.add_maintainer(user) + end + + create(:project, group: group) do |project| + project.add_developer(user) + end + + expect(user.highest_role).to eq(Gitlab::Access::MAINTAINER) + end + + it 'returns MAINTAINER if user is maintainer of a group' do + create(:group) do |group| + group.add_user(user, GroupMember::MAINTAINER) + end + + expect(user.highest_role).to eq(Gitlab::Access::MAINTAINER) + end + + it 'returns the highest role if user is member of multiple groups' do + create(:group) do |group| + group.add_user(user, GroupMember::MAINTAINER) + end + + create(:group) do |group| + group.add_user(user, GroupMember::DEVELOPER) + end + + expect(user.highest_role).to eq(Gitlab::Access::MAINTAINER) + end + + it 'returns the highest role if user is member of multiple groups and projects' do + create(:group) do |group| + group.add_user(user, GroupMember::DEVELOPER) + end + + create(:project, group: group) do |project| + project.add_maintainer(user) + end + + expect(user.highest_role).to eq(Gitlab::Access::MAINTAINER) + end + end + describe '#update_tracked_fields!', :clean_gitlab_redis_shared_state do let(:request) { OpenStruct.new(remote_ip: "127.0.0.1") } let(:user) { create(:user) } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index a879426589d..b84202364e1 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -68,6 +68,13 @@ describe API::Users do expect(json_response.size).to eq(0) end + it "does not return the highest role" do + get api("/users"), params: { username: user.username } + + expect(response).to match_response_schema('public_api/v4/user/basics') + expect(json_response.first.keys).not_to include 'highest_role' + end + context "when public level is restricted" do before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) @@ -286,6 +293,13 @@ describe API::Users do expect(json_response.keys).not_to include 'is_admin' end + it "does not return the user's `highest_role`" do + get api("/users/#{user.id}", user) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).not_to include 'highest_role' + end + context 'when authenticated as admin' do it 'includes the `is_admin` field' do get api("/users/#{user.id}", admin) @@ -300,6 +314,12 @@ describe API::Users do expect(response).to match_response_schema('public_api/v4/user/admin') expect(json_response.keys).to include 'created_at' end + it 'includes the `highest_role` field' do + get api("/users/#{user.id}", admin) + + expect(response).to match_response_schema('public_api/v4/user/admin') + expect(json_response['highest_role']).to be(0) + end end context 'for an anonymous user' do -- cgit v1.2.1