Merge branch '23061-consolidate-project-lists' into 'master'

API: consolidate project lists

Closes #23061

See merge request !8962

changelogs/unreleased/23061-consolidate-project-lists.yml

+---
+title: 'API: Consolidate /projects endpoint'
+merge_request: 8962
+author:

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:
 

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.

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

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

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

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

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

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
+
+      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
 
-      context 'GET /projects?simple=true' do
+      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')
-      end
-
-      it "includes statistics if requested" do
-        get api('/projects/all', admin), statistics: true
-
-        expect(response).to have_http_status(200)
-        expect(json_response).to be_an Array
-        expect(json_response.first).to include 'statistics'
-      end
-    end
-  end
-
-  describe 'GET /projects/owned' do
-    before do
-      project3
-      project4
-    end
-
-    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)
+        before do
+          project_member2
+          user3.update_attributes(starred_projects: [project, project2, project3, public_project])
+        end
 
-        get api('/projects/owned', user4), statistics: true
+        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.first['statistics']).to eq attributes.stringify_keys
+          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
-    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)
+      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.map { |p| p['id'] }).to contain_exactly(*projects.map(&:id))
-      end
-    end
+        before do
+          user.update_attributes(starred_projects: [project5, project6, project8, project9])
+        end
 
-    let!(:public_project) { create(:empty_project, :public) }
-    before do
-      project
-      project2
-      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_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