diff --git a/changelogs/unreleased/23061-consolidate-project-lists.yml b/changelogs/unreleased/23061-consolidate-project-lists.yml new file mode 100644 index 00000000000..dbb8fed55c0 --- /dev/null +++ b/changelogs/unreleased/23061-consolidate-project-lists.yml @@ -0,0 +1,4 @@ +--- +title: 'API: Consolidate /projects endpoint' +merge_request: 8962 +author: diff --git a/doc/api/groups.md b/doc/api/groups.md index a3a43ca7f1c..4a39dbc5555 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -73,6 +73,8 @@ Parameters: | `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | | `search` | string | no | Return list of authorized projects matching the search criteria | | `simple` | boolean | no | Return only the ID, URL, name, and path of each project | +| `owned` | boolean | no | Limit by projects owned by the current user | +| `starred` | boolean | no | Limit by projects starred by the current user | Example response: diff --git a/doc/api/projects.md b/doc/api/projects.md index e579b89d836..fa51158956a 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -36,6 +36,8 @@ Parameters: | `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | | `search` | string | no | Return list of authorized projects matching the search criteria | | `simple` | boolean | no | Return only the ID, URL, name, and path of each project | +| `owned` | boolean | no | Limit by projects owned by the current user | +| `starred` | boolean | no | Limit by projects starred by the current user | ```json [ @@ -152,190 +154,6 @@ Parameters: ] ``` -Get a list of projects which the authenticated user can see. - -``` -GET /projects/visible -``` - -Parameters: - -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `archived` | boolean | no | Limit by archived status | -| `visibility` | string | no | Limit by visibility `public`, `internal`, or `private` | -| `order_by` | string | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. Default is `created_at` | -| `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | -| `search` | string | no | Return list of authorized projects matching the search criteria | -| `simple` | boolean | no | Return only the ID, URL, name, and path of each project | - -```json -[ - { - "id": 4, - "description": null, - "default_branch": "master", - "public": false, - "visibility_level": 0, - "ssh_url_to_repo": "git@example.com:diaspora/diaspora-client.git", - "http_url_to_repo": "http://example.com/diaspora/diaspora-client.git", - "web_url": "http://example.com/diaspora/diaspora-client", - "tag_list": [ - "example", - "disapora client" - ], - "owner": { - "id": 3, - "name": "Diaspora", - "created_at": "2013-09-30T13:46:02Z" - }, - "name": "Diaspora Client", - "name_with_namespace": "Diaspora / Diaspora Client", - "path": "diaspora-client", - "path_with_namespace": "diaspora/diaspora-client", - "issues_enabled": true, - "open_issues_count": 1, - "merge_requests_enabled": true, - "builds_enabled": true, - "wiki_enabled": true, - "snippets_enabled": false, - "container_registry_enabled": false, - "created_at": "2013-09-30T13:46:02Z", - "last_activity_at": "2013-09-30T13:46:02Z", - "creator_id": 3, - "namespace": { - "id": 3, - "name": "Diaspora", - "path": "diaspora", - "kind": "group" - }, - "archived": false, - "avatar_url": "http://example.com/uploads/project/avatar/4/uploads/avatar.png", - "shared_runners_enabled": true, - "forks_count": 0, - "star_count": 0, - "runners_token": "b8547b1dc37721d05889db52fa2f02", - "public_builds": true, - "shared_with_groups": [] - }, - { - "id": 6, - "description": null, - "default_branch": "master", - "public": false, - "visibility_level": 0, - "ssh_url_to_repo": "git@example.com:brightbox/puppet.git", - "http_url_to_repo": "http://example.com/brightbox/puppet.git", - "web_url": "http://example.com/brightbox/puppet", - "tag_list": [ - "example", - "puppet" - ], - "owner": { - "id": 4, - "name": "Brightbox", - "created_at": "2013-09-30T13:46:02Z" - }, - "name": "Puppet", - "name_with_namespace": "Brightbox / Puppet", - "path": "puppet", - "path_with_namespace": "brightbox/puppet", - "issues_enabled": true, - "open_issues_count": 1, - "merge_requests_enabled": true, - "builds_enabled": true, - "wiki_enabled": true, - "snippets_enabled": false, - "container_registry_enabled": false, - "created_at": "2013-09-30T13:46:02Z", - "last_activity_at": "2013-09-30T13:46:02Z", - "creator_id": 3, - "namespace": { - "id": 4, - "name": "Brightbox", - "path": "brightbox", - "kind": "group" - }, - "permissions": { - "project_access": { - "access_level": 10, - "notification_level": 3 - }, - "group_access": { - "access_level": 50, - "notification_level": 3 - } - }, - "archived": false, - "avatar_url": null, - "shared_runners_enabled": true, - "forks_count": 0, - "star_count": 0, - "runners_token": "b8547b1dc37721d05889db52fa2f02", - "public_builds": true, - "shared_with_groups": [] - } -] -``` - -### List owned projects - -Get a list of projects which are owned by the authenticated user. - -``` -GET /projects/owned -``` - -Parameters: - -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `archived` | boolean | no | Limit by archived status | -| `visibility` | string | no | Limit by visibility `public`, `internal`, or `private` | -| `order_by` | string | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. Default is `created_at` | -| `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | -| `search` | string | no | Return list of authorized projects matching the search criteria | -| `simple` | boolean | no | Return only the ID, URL, name, and path of each project | -| `statistics` | boolean | no | Include project statistics | - -### List starred projects - -Get a list of projects which are starred by the authenticated user. - -``` -GET /projects/starred -``` - -Parameters: - -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `archived` | boolean | no | Limit by archived status | -| `visibility` | string | no | Limit by visibility `public`, `internal`, or `private` | -| `order_by` | string | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. Default is `created_at` | -| `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | -| `search` | string | no | Return list of authorized projects matching the search criteria | -| `simple` | boolean | no | Return only the ID, URL, name, and path of each project | - -### List ALL projects - -Get a list of all GitLab projects (admin only). - -``` -GET /projects/all -``` - -Parameters: - -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `archived` | boolean | no | Limit by archived status | -| `visibility` | string | no | Limit by visibility `public`, `internal`, or `private` | -| `order_by` | string | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. Default is `created_at` | -| `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | -| `search` | string | no | Return list of authorized projects matching the search criteria | -| `statistics` | boolean | no | Include project statistics | - ### Get single project Get a specific project, identified by project ID or NAMESPACE/PROJECT_NAME, which is owned by the authenticated user. diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 7811025f34e..84ff72bc36c 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -23,4 +23,4 @@ changes are in V4: - `/gitlab_ci_ymls/:key` - `/dockerfiles/:key` - Moved `/projects/fork/:id` to `/projects/:id/fork` - +- Endpoints `/projects/owned`, `/projects/visible`, `/projects/starred` & `/projects/all` are consolidated into `/projects` using query parameters diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 5c132bdd6f9..9f29c4466ab 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -143,6 +143,9 @@ module API desc: 'Return projects sorted in ascending and descending order' optional :simple, type: Boolean, default: false, desc: 'Return only the ID, URL, name, and path of each project' + optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user' + optional :starred, type: Boolean, default: false, desc: 'Limit by starred status' + use :pagination end get ":id/projects" do diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index dfab60f7fa5..13896dd91b9 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -256,6 +256,14 @@ module API # project helpers def filter_projects(projects) + if params[:owned] + projects = projects.merge(current_user.owned_projects) + end + + if params[:starred] + projects = projects.merge(current_user.starred_projects) + end + if params[:search].present? projects = projects.search(params[:search]) end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 2cacb246db8..68c2732ec80 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -50,6 +50,8 @@ module API optional :visibility, type: String, values: %w[public internal private], desc: 'Limit by visibility' optional :search, type: String, desc: 'Return list of authorized projects matching the search criteria' + optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user' + optional :starred, type: Boolean, default: false, desc: 'Limit by starred status' end params :statistics_params do @@ -82,62 +84,9 @@ module API params do use :collection_params end - get '/visible' do - entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails - present_projects ProjectsFinder.new.execute(current_user), with: entity - end - - desc 'Get a projects list for authenticated user' do - success Entities::BasicProjectDetails - end - params do - use :collection_params - end get do - authenticate! - - present_projects current_user.authorized_projects, - with: Entities::ProjectWithAccess - end - - desc 'Get an owned projects list for authenticated user' do - success Entities::BasicProjectDetails - end - params do - use :collection_params - use :statistics_params - end - get '/owned' do - authenticate! - - present_projects current_user.owned_projects, - with: Entities::ProjectWithAccess, - statistics: params[:statistics] - end - - desc 'Gets starred project for the authenticated user' do - success Entities::BasicProjectDetails - end - params do - use :collection_params - end - get '/starred' do - authenticate! - - present_projects current_user.viewable_starred_projects - end - - desc 'Get all projects for admin user' do - success Entities::BasicProjectDetails - end - params do - use :collection_params - use :statistics_params - end - get '/all' do - authenticated_as_admin! - - present_projects Project.all, with: Entities::ProjectWithAccess, statistics: params[:statistics] + entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails + present_projects ProjectsFinder.new.execute(current_user), with: entity, statistics: params[:statistics] end desc 'Create new project' do diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index f78bde6f53a..ccd7898586c 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -338,6 +338,26 @@ describe API::Groups, api: true do expect(json_response.length).to eq(1) expect(json_response.first['name']).to eq(project3.name) end + + it 'only returns the projects owned by user' do + project2.group.add_owner(user3) + + get api("/groups/#{project2.group.id}/projects", user3), owned: true + + expect(response).to have_http_status(200) + expect(json_response.length).to eq(1) + expect(json_response.first['name']).to eq(project2.name) + end + + it 'only returns the projects starred by user' do + user1.starred_projects = [project1] + + get api("/groups/#{group1.id}/projects", user1), starred: true + + expect(response).to have_http_status(200) + expect(json_response.length).to eq(1) + expect(json_response.first['name']).to eq(project1.name) + end end context "when authenticated as admin" do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 17b5e372bdc..bca7642b6fc 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -41,26 +41,40 @@ describe API::Projects, api: true do end describe 'GET /projects' do - before { project } + shared_examples_for 'projects response' do + it 'returns an array of projects' do + get api('/projects', current_user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.map { |p| p['id'] }).to contain_exactly(*projects.map(&:id)) + end + end + + let!(:public_project) { create(:empty_project, :public, name: 'public_project') } + before do + project + project2 + project3 + project4 + end context 'when unauthenticated' do - it 'returns authentication error' do - get api('/projects') - expect(response).to have_http_status(401) + it_behaves_like 'projects response' do + let(:current_user) { nil } + let(:projects) { [public_project] } end end context 'when authenticated as regular user' do - it 'returns an array of projects' do - get api('/projects', user) - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.first['name']).to eq(project.name) - expect(json_response.first['owner']['username']).to eq(user.username) + it_behaves_like 'projects response' do + let(:current_user) { user } + let(:projects) { [public_project, project, project2, project3] } end it 'includes the project labels as the tag_list' do get api('/projects', user) + expect(response.status).to eq 200 expect(json_response).to be_an Array expect(json_response.first.keys).to include('tag_list') @@ -68,21 +82,39 @@ describe API::Projects, api: true do it 'includes open_issues_count' do get api('/projects', user) + expect(response.status).to eq 200 expect(json_response).to be_an Array expect(json_response.first.keys).to include('open_issues_count') end - it 'does not include open_issues_count' do + it 'does not include open_issues_count if issues are disabled' do project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) get api('/projects', user) + expect(response.status).to eq 200 expect(json_response).to be_an Array - expect(json_response.first.keys).not_to include('open_issues_count') + expect(json_response.find { |hash| hash['id'] == project.id }.keys).not_to include('open_issues_count') end - context 'GET /projects?simple=true' do + it "does not include statistics by default" do + get api('/projects', user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first).not_to include('statistics') + end + + it "includes statistics if requested" do + get api('/projects', user), statistics: true + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first).to include 'statistics' + end + + context 'and with simple=true' do it 'returns a simplified version of all the projects' do expected_keys = ["id", "http_url_to_repo", "web_url", "name", "name_with_namespace", "path", "path_with_namespace"] @@ -97,6 +129,7 @@ describe API::Projects, api: true do context 'and using search' do it 'returns searched project' do get api('/projects', user), { search: project.name } + expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -106,196 +139,109 @@ describe API::Projects, api: true do context 'and using the visibility filter' do it 'filters based on private visibility param' do get api('/projects', user), { visibility: 'private' } + expect(response).to have_http_status(200) expect(json_response).to be_an Array - expect(json_response.length).to eq(user.namespace.projects.where(visibility_level: Gitlab::VisibilityLevel::PRIVATE).count) + expect(json_response.map { |p| p['id'] }).to contain_exactly(project.id, project2.id, project3.id) end it 'filters based on internal visibility param' do + project2.update_attribute(:visibility_level, Gitlab::VisibilityLevel::INTERNAL) + get api('/projects', user), { visibility: 'internal' } + expect(response).to have_http_status(200) expect(json_response).to be_an Array - expect(json_response.length).to eq(user.namespace.projects.where(visibility_level: Gitlab::VisibilityLevel::INTERNAL).count) + expect(json_response.map { |p| p['id'] }).to contain_exactly(project2.id) end it 'filters based on public visibility param' do get api('/projects', user), { visibility: 'public' } + expect(response).to have_http_status(200) expect(json_response).to be_an Array - expect(json_response.length).to eq(user.namespace.projects.where(visibility_level: Gitlab::VisibilityLevel::PUBLIC).count) + expect(json_response.map { |p| p['id'] }).to contain_exactly(public_project.id) end end context 'and using sorting' do - before do - project2 - project3 - end - it 'returns the correct order when sorted by id' do get api('/projects', user), { order_by: 'id', sort: 'desc' } + expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.first['id']).to eq(project3.id) end end - end - end - describe 'GET /projects/all' do - before { project } + context 'and with owned=true' do + it 'returns an array of projects the user owns' do + get api('/projects', user4), owned: true - context 'when unauthenticated' do - it 'returns authentication error' do - get api('/projects/all') - expect(response).to have_http_status(401) - end - end - - context 'when authenticated as regular user' do - it 'returns authentication error' do - get api('/projects/all', user) - expect(response).to have_http_status(403) - end - end - - context 'when authenticated as admin' do - it 'returns an array of all projects' do - get api('/projects/all', admin) - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - - expect(json_response).to satisfy do |response| - response.one? do |entry| - entry.has_key?('permissions') && - entry['name'] == project.name && - entry['owner']['username'] == user.username - end + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(project4.name) + expect(json_response.first['owner']['username']).to eq(user4.username) end end - it "does not include statistics by default" do - get api('/projects/all', admin) + context 'and with starred=true' do + let(:public_project) { create(:empty_project, :public) } - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.first).not_to include('statistics') + before do + project_member2 + user3.update_attributes(starred_projects: [project, project2, project3, public_project]) + end + + it 'returns the starred projects viewable by the user' do + get api('/projects', user3), starred: true + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }).to contain_exactly(project.id, public_project.id) + end end - it "includes statistics if requested" do - get api('/projects/all', admin), statistics: true + context 'and with all query parameters' do + # | | project5 | project6 | project7 | project8 | project9 | + # |---------+----------+----------+----------+----------+----------| + # | search | x | | x | x | x | + # | starred | x | x | | x | x | + # | public | x | x | x | | x | + # | owned | x | x | x | x | | + let!(:project5) { create(:empty_project, :public, path: 'gitlab5', namespace: user.namespace) } + let!(:project6) { create(:empty_project, :public, path: 'project6', namespace: user.namespace) } + let!(:project7) { create(:empty_project, :public, path: 'gitlab7', namespace: user.namespace) } + let!(:project8) { create(:empty_project, path: 'gitlab8', namespace: user.namespace) } + let!(:project9) { create(:empty_project, :public, path: 'gitlab9') } - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.first).to include 'statistics' - end - end - end + before do + user.update_attributes(starred_projects: [project5, project6, project8, project9]) + end - describe 'GET /projects/owned' do - before do - project3 - project4 - end + it 'returns only projects that satify all query parameters' do + get api('/projects', user), { visibility: 'public', owned: true, starred: true, search: 'gitlab' } - context 'when unauthenticated' do - it 'returns authentication error' do - get api('/projects/owned') - expect(response).to have_http_status(401) - end - end - - context 'when authenticated as project owner' do - it 'returns an array of projects the user owns' do - get api('/projects/owned', user4) - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.first['name']).to eq(project4.name) - expect(json_response.first['owner']['username']).to eq(user4.username) - end - - it "does not include statistics by default" do - get api('/projects/owned', user4) - - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.first).not_to include('statistics') - end - - it "includes statistics if requested" do - attributes = { - commit_count: 23, - storage_size: 702, - repository_size: 123, - lfs_objects_size: 234, - build_artifacts_size: 345, - } - - project4.statistics.update!(attributes) - - get api('/projects/owned', user4), statistics: true - - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.first['statistics']).to eq attributes.stringify_keys - end - end - end - - describe 'GET /projects/visible' do - shared_examples_for 'visible projects response' do - it 'returns the visible projects' do - get api('/projects/visible', current_user) - - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.map { |p| p['id'] }).to contain_exactly(*projects.map(&:id)) - end - end - - let!(:public_project) { create(:empty_project, :public) } - before do - project - project2 - project3 - project4 - end - - context 'when unauthenticated' do - it_behaves_like 'visible projects response' do - let(:current_user) { nil } - let(:projects) { [public_project] } - end - end - - context 'when authenticated' do - it_behaves_like 'visible projects response' do - let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3] } + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response.first['id']).to eq(project5.id) + end end end context 'when authenticated as a different user' do - it_behaves_like 'visible projects response' do + it_behaves_like 'projects response' do let(:current_user) { user2 } let(:projects) { [public_project] } end end - end - describe 'GET /projects/starred' do - let(:public_project) { create(:empty_project, :public) } - - before do - project_member2 - user3.update_attributes(starred_projects: [project, project2, project3, public_project]) - end - - it 'returns the starred projects viewable by the user' do - get api('/projects/starred', user3) - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.map { |project| project['id'] }).to contain_exactly(project.id, public_project.id) + context 'when authenticated as admin' do + it_behaves_like 'projects response' do + let(:current_user) { admin } + let(:projects) { Project.all } + end end end