kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Merge branch '32783-api-all-members-with-ancestors' into 'master'

Browse source 
Resolve "API endpoint that returns all members, including the inherited membership through ancestor group"

Closes #32783

See merge request gitlab-org/gitlab-ce!19748
This commit is contained in:
Robert Speicher 2018-07-25 21:45:42 +00:00
commit ac202fff47
5 changed files with 203 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
changelogs/unreleased/32783-api-all-members-with-ancestors.yml Normal file
View file

@ -0,0 +1,6 @@
---
title: Adds API endpoint /api/v4/(project/group)/:id/members/all to list also inherited
members
merge_request: 19748
author: Jacopo Beschi @jacopo-beschi
type: added

75
doc/api/members.md
View file

@ -40,7 +40,9 @@ Example response:
"username": "raymond_smith",
"name": "Raymond Smith",
"state": "active",
"created_at": "2012-10-22T14:13:35Z",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root",
"expires_at": "2012-10-22T14:13:35Z",
"access_level": 30
},
{
@ -48,7 +50,65 @@ Example response:
"username": "john_doe",
"name": "John Doe",
"state": "active",
"created_at": "2012-10-22T14:13:35Z",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root",
"expires_at": "2012-10-22T14:13:35Z",
"access_level": 30
}
]
```
## List all members of a group or project including inherited members
Gets a list of group or project members viewable by the authenticated user, including inherited members through ancestor groups.
```
GET /groups/:id/members/all
GET /projects/:id/members/all
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project or group](README.md#namespaced-path-encoding) owned by the authenticated user |
| `query` | string | no | A query string to search for members |
```bash
curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/:id/members/all
curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/:id/members/all
```
Example response:
```json
[
{
"id": 1,
"username": "raymond_smith",
"name": "Raymond Smith",
"state": "active",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root",
"expires_at": "2012-10-22T14:13:35Z",
"access_level": 30
},
{
"id": 2,
"username": "john_doe",
"name": "John Doe",
"state": "active",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root",
"expires_at": "2012-10-22T14:13:35Z",
"access_level": 30
},
{
"id": 3,
"username": "foo_bar",
"name": "Foo bar",
"state": "active",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root",
"expires_at": "2012-11-22T14:13:35Z",
"access_level": 30
}
]
@ -81,7 +141,8 @@ Example response:
"username": "raymond_smith",
"name": "Raymond Smith",
"state": "active",
"created_at": "2012-10-22T14:13:35Z",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root",
"access_level": 30,
"expires_at": null
}
@ -116,7 +177,9 @@ Example response:
"username": "raymond_smith",
"name": "Raymond Smith",
"state": "active",
"created_at": "2012-10-22T14:13:35Z",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root",
"expires_at": "2012-10-22T14:13:35Z",
"access_level": 30
}
```
@ -150,7 +213,9 @@ Example response:
"username": "raymond_smith",
"name": "Raymond Smith",
"state": "active",
"created_at": "2012-10-22T14:13:35Z",
"avatar_url": "https://www.gravatar.com/avatar/c2525a7f58ae3776070e44c106c48e15?s=80&d=identicon",
"web_url": "http://192.168.1.8:3000/root",
"expires_at": "2012-10-22T14:13:35Z",
"access_level": 40
}
```

25
lib/api/helpers/members_helpers.rb
View file

@ -10,6 +10,31 @@ module API
def authorize_admin_source!(source_type, source)
authorize! :"admin_#{source_type}", source
end
def find_all_members(source_type, source)
members = source_type == 'project' ? find_all_members_for_project(source) : find_all_members_for_group(source)
members.non_invite
.non_request
end
def find_all_members_for_project(project)
shared_group_ids = project.project_group_links.pluck(:group_id)
project_group_ids = project.group&.self_and_ancestors&.pluck(:id)
source_ids = [project.id, project_group_ids, shared_group_ids]
.flatten
.compact
Member.includes(:user)
.joins(user: :project_authorizations)
.where(project_authorizations: { project_id: project.id })
.where(source_id: source_ids)
end
def find_all_members_for_group(group)
source_ids = group.self_and_ancestors.pluck(:id)
Member.includes(:user)
.where(source_id: source_ids)
.where(source_type: 'Namespace')
end
end
end
end

17
lib/api/members.rb
View file

@ -28,6 +28,23 @@ module API
present members, with: Entities::Member
end
desc 'Gets a list of group or project members viewable by the authenticated user, including those who gained membership through ancestor group.' do
success Entities::Member
end
params do
optional :query, type: String, desc: 'A query string to search for members'
use :pagination
end
get ":id/members/all" do
source = find_source(source_type, params[:id])
members = find_all_members(source_type, source)
members = members.includes(:user).references(:user).merge(User.search(params[:query])) if params[:query].present?
members = paginate(members)
present members, with: Entities::Member
end
desc 'Gets a member of a group or project.' do
success Entities::Member
end

119
spec/requests/api/members_spec.rb
View file

@ -22,10 +22,16 @@ describe API::Members do
end
end
shared_examples 'GET /:sources/:id/members' do |source_type|
context "with :sources == #{source_type.pluralize}" do
shared_examples 'GET /:source_type/:id/members/(all)' do |source_type, all|
let(:members_url) do
"/#{source_type.pluralize}/#{source.id}/members".tap do |url|
url << "/all" if all
end
end
context "with :source_type == #{source_type.pluralize}" do
it_behaves_like 'a 404 response when source is private' do
let(:route) { get api("/#{source_type.pluralize}/#{source.id}/members", stranger) }
let(:route) { get api(members_url, stranger) }
end
%i[maintainer developer access_requester stranger].each do |type|
@ -33,7 +39,7 @@ describe API::Members do
it 'returns 200' do
user = public_send(type)
get api("/#{source_type.pluralize}/#{source.id}/members", user)
get api(members_url, user)
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
@ -46,23 +52,23 @@ describe API::Members do
it 'avoids N+1 queries' do
# Establish baseline
get api("/#{source_type.pluralize}/#{source.id}/members", maintainer)
get api(members_url, maintainer)
control = ActiveRecord::QueryRecorder.new do
get api("/#{source_type.pluralize}/#{source.id}/members", maintainer)
get api(members_url, maintainer)
end
project.add_developer(create(:user))
expect do
get api("/#{source_type.pluralize}/#{source.id}/members", maintainer)
get api(members_url, maintainer)
end.not_to exceed_query_limit(control)
end
it 'does not return invitees' do
create(:"#{source_type}_member", invite_token: '123', invite_email: 'test@abc.com', source: source, user: nil)
get api("/#{source_type.pluralize}/#{source.id}/members", developer)
get api(members_url, developer)
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
@ -72,7 +78,7 @@ describe API::Members do
end
it 'finds members with query string' do
get api("/#{source_type.pluralize}/#{source.id}/members", developer), query: maintainer.username
get api(members_url, developer), query: maintainer.username
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
@ -82,7 +88,7 @@ describe API::Members do
end
it 'finds all members with no query specified' do
get api("/#{source_type.pluralize}/#{source.id}/members", developer), query: ''
get api(members_url, developer), query: ''
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
@ -93,8 +99,51 @@ describe API::Members do
end
end
shared_examples 'GET /:sources/:id/members/:user_id' do |source_type|
context "with :sources == #{source_type.pluralize}" do
describe 'GET /:source_type/:id/members/all', :nested_groups do
let(:nested_user) { create(:user) }
let(:project_user) { create(:user) }
let(:linked_group_user) { create(:user) }
let!(:project_group_link) { create(:project_group_link, project: project, group: linked_group) }
let(:project) do
create(:project, :public, group: nested_group) do |project|
project.add_developer(project_user)
end
end
let(:linked_group) do
create(:group) do |linked_group|
linked_group.add_developer(linked_group_user)
end
end
let(:nested_group) do
create(:group, parent: group) do |nested_group|
nested_group.add_developer(nested_user)
end
end
it 'finds all project members including inherited members' do
get api("/projects/#{project.id}/members/all", developer)
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
expect(json_response.map { |u| u['id'] }).to match_array [maintainer.id, developer.id, nested_user.id, project_user.id, linked_group_user.id]
end
it 'finds all group members including inherited members' do
get api("/groups/#{nested_group.id}/members/all", developer)
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
expect(json_response.map { |u| u['id'] }).to match_array [maintainer.id, developer.id, nested_user.id]
end
end
shared_examples 'GET /:source_type/:id/members/:user_id' do |source_type|
context "with :source_type == #{source_type.pluralize}" do
it_behaves_like 'a 404 response when source is private' do
let(:route) { get api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) }
end
@ -124,8 +173,8 @@ describe API::Members do
end
end
shared_examples 'POST /:sources/:id/members' do |source_type|
context "with :sources == #{source_type.pluralize}" do
shared_examples 'POST /:source_type/:id/members' do |source_type|
context "with :source_type == #{source_type.pluralize}" do
it_behaves_like 'a 404 response when source is private' do
let(:route) do
post api("/#{source_type.pluralize}/#{source.id}/members", stranger),
@ -205,8 +254,8 @@ describe API::Members do
end
end
shared_examples 'PUT /:sources/:id/members/:user_id' do |source_type|
context "with :sources == #{source_type.pluralize}" do
shared_examples 'PUT /:source_type/:id/members/:user_id' do |source_type|
context "with :source_type == #{source_type.pluralize}" do
it_behaves_like 'a 404 response when source is private' do
let(:route) do
put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger),
@ -262,8 +311,8 @@ describe API::Members do
end
end
shared_examples 'DELETE /:sources/:id/members/:user_id' do |source_type|
context "with :sources == #{source_type.pluralize}" do
shared_examples 'DELETE /:source_type/:id/members/:user_id' do |source_type|
context "with :source_type == #{source_type.pluralize}" do
it_behaves_like 'a 404 response when source is private' do
let(:route) { delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) }
end
@ -323,43 +372,45 @@ describe API::Members do
end
end
it_behaves_like 'GET /:sources/:id/members', 'project' do
[false, true].each do |all|
it_behaves_like 'GET /:source_type/:id/members/(all)', 'project', all do
let(:source) { project }
end
it_behaves_like 'GET /:source_type/:id/members/(all)', 'group', all do
let(:source) { group }
end
end
it_behaves_like 'GET /:source_type/:id/members/:user_id', 'project' do
let(:source) { project }
end
it_behaves_like 'GET /:sources/:id/members', 'group' do
it_behaves_like 'GET /:source_type/:id/members/:user_id', 'group' do
let(:source) { group }
end
it_behaves_like 'GET /:sources/:id/members/:user_id', 'project' do
it_behaves_like 'POST /:source_type/:id/members', 'project' do
let(:source) { project }
end
it_behaves_like 'GET /:sources/:id/members/:user_id', 'group' do
it_behaves_like 'POST /:source_type/:id/members', 'group' do
let(:source) { group }
end
it_behaves_like 'POST /:sources/:id/members', 'project' do
it_behaves_like 'PUT /:source_type/:id/members/:user_id', 'project' do
let(:source) { project }
end
it_behaves_like 'POST /:sources/:id/members', 'group' do
it_behaves_like 'PUT /:source_type/:id/members/:user_id', 'group' do
let(:source) { group }
end
it_behaves_like 'PUT /:sources/:id/members/:user_id', 'project' do
it_behaves_like 'DELETE /:source_type/:id/members/:user_id', 'project' do
let(:source) { project }
end
it_behaves_like 'PUT /:sources/:id/members/:user_id', 'group' do
let(:source) { group }
end
it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'project' do
let(:source) { project }
end
it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'group' do
it_behaves_like 'DELETE /:source_type/:id/members/:user_id', 'group' do
let(:source) { group }
end