Add include_parent_group_milestones param

- Add include_parent_group_milestones param
to project milestones API endpoint.
This param is an optional boolean value that is
false by default.

Add param to group milestones endpoint

- Rename param to include_parent_milestones
- Add tests
- Update documentation

Update documentation to include note in iids

changelogs/unreleased/196066-add-milestone-expired-info-be.yml

+---
+title: Add include_parent_milestones param to project and group milestones API endpoints
+merge_request: 38800
+author:
+type: added

doc/api/group_milestones.md

@@ -27,13 +27,14 @@ GET /groups/:id/milestones?search=version
 
 Parameters:
 
-| Attribute | Type   | Required | Description |
-| --------- | ------ | -------- | ----------- |
-| `id`      | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user |
-| `iids[]`  | integer array | no | Return only the milestones having the given `iid` |
-| `state`   | string | no | Return only `active` or `closed` milestones |
-| `title`   | string | no | Return only the milestones having the given `title` |
-| `search`  | string | no | Return only milestones with a title or description matching the provided string |
+| Attribute                   | Type   | Required | Description |
+| ---------                   | ------ | -------- | ----------- |
+| `id`                        | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user |
+| `iids[]`                    | integer array | no | Return only the milestones having the given `iid` (Note: ignored if `include_parent_milestones` is set as `true`) |
+| `state`                     | string | no | Return only `active` or `closed` milestones |
+| `title`                     | string | no | Return only the milestones having the given `title` |
+| `search`                    | string | no | Return only milestones with a title or description matching the provided string |
+| `include_parent_milestones` | boolean | optional | Include milestones from parent group and its ancestors. Introduced in [GitLab 13.3](https://gitlab.com/gitlab-org/gitlab/-/issues/196066) |
 
 ```shell
 curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/5/milestones"

doc/api/milestones.md

@@ -25,13 +25,14 @@ GET /projects/:id/milestones?search=version
 
 Parameters:
 
-| Attribute | Type   | Required | Description |
-| --------- | ------ | -------- | ----------- |
-| `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
-| `iids[]`  | integer array | optional | Return only the milestones having the given `iid` |
-| `state`   | string | optional | Return only `active` or `closed` milestones |
-| `title`   | string | optional | Return only the milestones having the given `title` |
-| `search`  | string | optional | Return only milestones with a title or description matching the provided string |
+| Attribute                         | Type   | Required | Description |
+| ----------------------------      | ------ | -------- | ----------- |
+| `id`                              | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
+| `iids[]`                          | integer array | optional | Return only the milestones having the given `iid` (Note: ignored if `include_parent_milestones` is set as `true`) |
+| `state`                           | string | optional | Return only `active` or `closed` milestones |
+| `title`                           | string | optional | Return only the milestones having the given `title` |
+| `search`                          | string | optional | Return only milestones with a title or description matching the provided string |
+| `include_parent_milestones`       | boolean | optional | Include group milestones from parent group and its ancestors. Introduced in [GitLab 13.3](https://gitlab.com/gitlab-org/gitlab/-/issues/196066) |
 
 ```shell
 curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/milestones"

lib/api/milestone_responses.rb

@@ -14,10 +14,12 @@ module API
 
         params :list_params do
           optional :state, type: String, values: %w[active closed all], default: 'all',
-                           desc: 'Return "active", "closed", or "all" milestones'
+                          desc: 'Return "active", "closed", or "all" milestones'
           optional :iids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IIDs of the milestones'
           optional :title, type: String, desc: 'The title of the milestones'
           optional :search, type: String, desc: 'The search criteria for the title or description of the milestone'
+          optional :include_parent_milestones, type: Grape::API::Boolean, default: false,
+                    desc: 'Include group milestones from parent and its ancestors'
           use :pagination
         end
 
@@ -25,21 +27,36 @@ module API
           requires :milestone_id, type: Integer, desc: 'The milestone ID number'
           optional :title, type: String, desc: 'The title of the milestone'
           optional :state_event, type: String, values: %w[close activate],
-                                 desc: 'The state event of the milestone '
+                                desc: 'The state event of the milestone '
           use :optional_params
           at_least_one_of :title, :description, :start_date, :due_date, :state_event
         end
 
         def list_milestones_for(parent)
-          milestones = parent.milestones.order_id_desc
+          milestones = init_milestones_collection(parent)
           milestones = Milestone.filter_by_state(milestones, params[:state])
-          milestones = filter_by_iid(milestones, params[:iids]) if params[:iids].present?
+          if params[:iids].present? && !params[:include_parent_milestones]
+            milestones = filter_by_iid(milestones, params[:iids])
+          end
+
           milestones = filter_by_title(milestones, params[:title]) if params[:title]
           milestones = filter_by_search(milestones, params[:search]) if params[:search]
 
           present paginate(milestones), with: Entities::Milestone
         end
 
+        def filter_milestones(milestones)
+          milestones = Milestone.filter_by_state(milestones, params[:state])
+          if params[:iids].present? && !params[:include_parent_milestones]
+            milestones = filter_by_iid(milestones, params[:iids])
+          end
+
+          milestones = filter_by_title(milestones, params[:title]) if params[:title]
+          milestones = filter_by_search(milestones, params[:search]) if params[:search]
+
+          milestones
+        end
+
         def get_milestone_for(parent)
           milestone = parent.milestones.find(params[:milestone_id])
           present milestone, with: Entities::Milestone
@@ -96,6 +113,30 @@ module API
             [MergeRequestsFinder, Entities::MergeRequestBasic]
           end
         end
+
+        def init_milestones_collection(parent)
+          milestones = if params[:include_parent_milestones].present?
+                         parent_and_ancestors_milestones(parent)
+                       else
+                         parent.milestones
+                       end
+
+          milestones.order_id_desc
+        end
+
+        def parent_and_ancestors_milestones(parent)
+          project_id = parent.is_a?(Project) ? parent.id : nil
+
+          Milestone.for_projects_and_groups(project_id, group_ids(parent))
+        end
+
+        def group_ids(parent)
+          group = parent.is_a?(Project) ? parent.group : parent
+
+          group.self_and_ancestors
+            .public_or_visible_to_user(current_user)
+            .select(:id)
+        end
       end
     end
   end

spec/requests/api/group_milestones_spec.rb

@@ -3,17 +3,73 @@
 require 'spec_helper'
 
 RSpec.describe API::GroupMilestones do
-  let(:user) { create(:user) }
-  let(:group) { create(:group, :private) }
-  let(:project) { create(:project, namespace: group) }
-  let!(:group_member) { create(:group_member, group: group, user: user) }
-  let!(:closed_milestone) { create(:closed_milestone, group: group, title: 'version1', description: 'closed milestone') }
-  let!(:milestone) { create(:milestone, group: group, title: 'version2', description: 'open milestone') }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group, :private) }
+  let_it_be(:project) { create(:project, namespace: group) }
+  let_it_be(:group_member) { create(:group_member, group: group, user: user) }
+  let_it_be(:closed_milestone) { create(:closed_milestone, group: group, title: 'version1', description: 'closed milestone') }
+  let_it_be(:milestone) { create(:milestone, group: group, title: 'version2', description: 'open milestone') }
 
   it_behaves_like 'group and project milestones', "/groups/:id/milestones" do
     let(:route) { "/groups/#{group.id}/milestones" }
   end
 
+  describe 'GET /groups/:id/milestones' do
+    context 'when include_parent_milestones is true' do
+      let_it_be(:subgroup) { create(:group, :private, parent: group) }
+      let_it_be(:subgroup_milestone) { create(:milestone, group: subgroup) }
+      let_it_be(:route) { "/groups/#{subgroup.id}/milestones" }
+      let_it_be(:params) { { include_parent_milestones: true } }
+
+      shared_examples 'lists all milestones' do
+        it 'includes parent and ancestors milestones' do
+          milestones = [subgroup_milestone, milestone, closed_milestone]
+
+          get api(route, user), params: params
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response.size).to eq(3)
+          expect(json_response.map { |entry| entry["id"] }).to eq(milestones.map(&:id))
+        end
+      end
+
+      context 'when user has access to all groups' do
+        before do
+          group.add_developer(user)
+          subgroup.add_developer(user)
+        end
+
+        it_behaves_like 'lists all milestones'
+
+        context 'when iids param is present' do
+          before do
+            params.merge(iids: [milestone.iid])
+          end
+
+          it_behaves_like 'lists all milestones'
+        end
+      end
+
+      context 'when user has no access to an ancestor group' do
+        let_it_be(:user2) { create(:user) }
+
+        before do
+          subgroup.add_developer(user2)
+        end
+
+        it 'does not show ancestor group milestones' do
+          milestones = [subgroup_milestone]
+
+          get api(route, user2), params: { include_parent_milestones: true }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response.size).to eq(1)
+          expect(json_response.map { |entry| entry["id"] }).to eq(milestones.map(&:id))
+        end
+      end
+    end
+  end
+
   def setup_for_group
     context_group.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
     context_group.add_developer(user)

spec/requests/api/project_milestones_spec.rb

@@ -3,17 +3,81 @@
 require 'spec_helper'
 
 RSpec.describe API::ProjectMilestones do
-  let(:user) { create(:user) }
-  let!(:project) { create(:project, namespace: user.namespace ) }
-  let!(:closed_milestone) { create(:closed_milestone, project: project, title: 'version1', description: 'closed milestone') }
-  let!(:milestone) { create(:milestone, project: project, title: 'version2', description: 'open milestone') }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, namespace: user.namespace ) }
+  let_it_be(:closed_milestone) { create(:closed_milestone, project: project, title: 'version1', description: 'closed milestone') }
+  let_it_be(:milestone) { create(:milestone, project: project, title: 'version2', description: 'open milestone') }
 
   before do
     project.add_developer(user)
   end
 
   it_behaves_like 'group and project milestones', "/projects/:id/milestones" do
-    let(:route) { "/projects/#{project.id}/milestones" }
+    let_it_be(:route) { "/projects/#{project.id}/milestones" }
+  end
+
+  describe 'GET /projects/:id/milestones' do
+    context 'when include_parent_milestones is true' do
+      let_it_be(:parent_group) { create(:group, :private) }
+      let_it_be(:subgroup) { create(:group, :private, parent: parent_group) }
+      let_it_be(:sub_project) { create(:project, group: subgroup) }
+      let_it_be(:sub_project_milestone) { create(:milestone, project: sub_project) }
+      let_it_be(:parent_group_milestone) { create(:milestone, group: parent_group) }
+      let_it_be(:subgroup_milestone) { create(:milestone, group: subgroup) }
+      let_it_be(:route) { "/projects/#{sub_project.id}/milestones" }
+      let_it_be(:params) { { include_parent_milestones: true } }
+
+      before do
+        sub_project.add_developer(user)
+      end
+
+      shared_examples 'lists all milestones' do
+        it 'includes parent and ancestors milestones' do
+          milestones = [subgroup_milestone, parent_group_milestone, sub_project_milestone]
+
+          get api(route, user), params: params
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response.size).to eq(3)
+          expect(json_response.map { |entry| entry["id"] }).to eq(milestones.map(&:id))
+        end
+      end
+
+      context 'when user has access to all groups' do
+        before do
+          parent_group.add_developer(user)
+          subgroup.add_developer(user)
+        end
+
+        it_behaves_like 'lists all milestones'
+
+        context 'when iids param is present' do
+          before do
+            params.merge(iids: [sub_project_milestone.iid])
+          end
+
+          it_behaves_like 'lists all milestones'
+        end
+      end
+
+      context 'when user has no access to an ancestor group' do
+        let(:user2) { create(:user) }
+
+        before do
+          sub_project.add_developer(user2)
+        end
+
+        it 'does not show ancestor group milestones' do
+          milestones = [subgroup_milestone, sub_project_milestone]
+
+          get api(route, user2), params: { include_parent_milestones: true }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response.size).to eq(2)
+          expect(json_response.map { |entry| entry["id"] }).to eq(milestones.map(&:id))
+        end
+      end
+    end
   end
 
   describe 'DELETE /projects/:id/milestones/:milestone_id' do