Commit 18104e9a authored by Eugenia Grieff's avatar Eugenia Grieff

Revert include_parent_milestones in milestones API

- Revert changes that added the param
include_parent_milestones to project milestones
parent 84cb8798
---
title: Add include_parent_milestones param to milestones API
merge_request: 36944
author:
type: added
......@@ -25,14 +25,13 @@ 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`. Will be ignored if `include_parent_milestones` is set to `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 milestones from parent group and ancestors. Introduced in [GitLab 13.2](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36944) |
| 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 |
```shell
curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/milestones"
......
......@@ -31,14 +31,12 @@ module API
end
def list_milestones_for(parent)
finder_params = params.merge(milestones_finder_params(parent))
milestones = MilestonesFinder.new(finder_params).execute
milestones = parent.milestones.order_id_desc
milestones = Milestone.filter_by_state(milestones, params[:state])
milestones = filter_by_iid(milestones, params[:iids]) if params[:iids].present?
milestones = filter_by_title(milestones, params[:title]) if params[:title]
milestones = filter_by_search(milestones, params[:search]) if params[:search]
if params[:iids].present? && !params[:include_parent_milestones]
milestones = filter_by_iid(milestones, params[:iids])
end
present paginate(milestones), with: Entities::Milestone
end
......@@ -98,25 +96,6 @@ module API
[MergeRequestsFinder, Entities::MergeRequestBasic]
end
end
def milestones_finder_params(parent)
if parent.is_a?(Group)
{ group_ids: parent.id }
else
{
project_ids: parent.id,
group_ids: parent_group_ids(parent)
}
end
end
def parent_group_ids(parent)
return unless params[:include_parent_milestones].present?
parent.group.self_and_ancestors
.public_or_visible_to_user(current_user)
.select(:id)
end
end
end
end
......
......@@ -16,8 +16,6 @@ module API
end
params do
use :list_params
optional :include_parent_milestones, type: Boolean, default: false,
desc: 'Include milestones from parent group and ancestors'
end
get ":id/milestones" do
authorize! :read_milestone, user_project
......
......@@ -3,10 +3,10 @@
require 'spec_helper'
RSpec.describe API::ProjectMilestones do
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') }
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') }
before do
project.add_developer(user)
......@@ -16,65 +16,6 @@ RSpec.describe API::ProjectMilestones do
let(:route) { "/projects/#{project.id}/milestones" }
end
describe 'GET /projects/:id/milestones' do
context 'when include_parent_milestones is true' do
let_it_be(:group) { create(:group, :public) }
let_it_be(:child_group) { create(:group, :public, parent: group) }
let_it_be(:child_project) { create(:project, group: child_group) }
let_it_be(:project_milestone) { create(:milestone, project: child_project) }
let_it_be(:group_milestone) { create(:milestone, group: group) }
let_it_be(:child_group_milestone) { create(:milestone, group: child_group) }
before do
child_project.add_developer(user)
end
it 'includes parent groups milestones' do
milestones = [child_group_milestone, group_milestone, project_milestone]
get api("/projects/#{child_project.id}/milestones", user),
params: { include_parent_milestones: true }
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
context 'when user has no access to an ancestor group' do
before do
[child_group, group].each do |group|
group.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
end
end
it 'does not show ancestor group milestones' do
milestones = [child_group_milestone, project_milestone]
get api("/projects/#{child_project.id}/milestones", user),
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
context 'when filtering by iids' do
it 'does not filter by iids' do
milestones = [child_group_milestone, group_milestone, project_milestone]
get api("/projects/#{child_project.id}/milestones", user),
params: { include_parent_milestones: true, iids: [group_milestone.iid] }
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
end
end
describe 'DELETE /projects/:id/milestones/:milestone_id' do
let(:guest) { create(:user) }
let(:reporter) { create(:user) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment