Commit 4615c56d authored by Nick Thomas's avatar Nick Thomas

Merge branch '14724-show-ancestor-group-epics-in-epics-select' into 'master'

Show ancestor group epics in epic select

See merge request gitlab-org/gitlab!16218
parents 7540c54c 85d2c4df
...@@ -49,6 +49,8 @@ GET /groups/:id/epics?state=opened ...@@ -49,6 +49,8 @@ GET /groups/:id/epics?state=opened
| `created_before` | datetime | no | Return epics created on or before the given time | | `created_before` | datetime | no | Return epics created on or before the given time |
| `updated_after` | datetime | no | Return epics updated on or after the given time | | `updated_after` | datetime | no | Return epics updated on or after the given time |
| `updated_before` | datetime | no | Return epics updated on or before the given time | | `updated_before` | datetime | no | Return epics updated on or before the given time |
| `include_ancestor_groups` | boolean | no | Include epics from the requested group's ancestors. Default is `false` |
| `include_descendant_groups` | boolean | no | Include epics from the requested group's descendants. Default is `true` |
```bash ```bash
curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/groups/1/epics curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/groups/1/epics
......
...@@ -7,7 +7,8 @@ export default { ...@@ -7,7 +7,8 @@ export default {
ldapGroupsPath: '/api/:version/ldap/:provider/groups.json', ldapGroupsPath: '/api/:version/ldap/:provider/groups.json',
subscriptionPath: '/api/:version/namespaces/:id/gitlab_subscription', subscriptionPath: '/api/:version/namespaces/:id/gitlab_subscription',
childEpicPath: '/api/:version/groups/:id/epics/:epic_iid/epics', childEpicPath: '/api/:version/groups/:id/epics/:epic_iid/epics',
groupEpicsPath: '/api/:version/groups/:id/epics', groupEpicsPath:
'/api/:version/groups/:id/epics?include_ancestor_groups=:includeAncestorGroups&include_descendant_groups=:includeDescendantGroups',
epicIssuePath: '/api/:version/groups/:id/epics/:epic_iid/issues/:issue_id', epicIssuePath: '/api/:version/groups/:id/epics/:epic_iid/issues/:issue_id',
userSubscription(namespaceId) { userSubscription(namespaceId) {
...@@ -43,8 +44,11 @@ export default { ...@@ -43,8 +44,11 @@ export default {
}); });
}, },
groupEpics({ groupId }) { groupEpics({ groupId, includeAncestorGroups = false, includeDescendantGroups = true }) {
const url = Api.buildUrl(this.groupEpicsPath).replace(':id', groupId); const url = Api.buildUrl(this.groupEpicsPath)
.replace(':id', groupId)
.replace(':includeAncestorGroups', includeAncestorGroups)
.replace(':includeDescendantGroups', includeDescendantGroups);
return axios.get(url); return axios.get(url);
}, },
......
...@@ -17,11 +17,9 @@ export const setSelectedEpic = ({ commit }, selectedEpic) => ...@@ -17,11 +17,9 @@ export const setSelectedEpic = ({ commit }, selectedEpic) =>
commit(types.SET_SELECTED_EPIC, selectedEpic); commit(types.SET_SELECTED_EPIC, selectedEpic);
export const requestEpics = ({ commit }) => commit(types.REQUEST_EPICS); export const requestEpics = ({ commit }) => commit(types.REQUEST_EPICS);
export const receiveEpicsSuccess = ({ state, commit }, data) => { export const receiveEpicsSuccess = ({ commit }, data) => {
const epics = data const epics = data.map(rawEpic =>
.filter(rawEpic => rawEpic.group_id === state.groupId) convertObjectPropsToCamelCase(Object.assign({}, rawEpic, { url: rawEpic.web_edit_url }), {
.map(rawEpic =>
convertObjectPropsToCamelCase(Object.assign(rawEpic, { url: rawEpic.web_edit_url }), {
dropKeys: ['web_edit_url'], dropKeys: ['web_edit_url'],
}), }),
); );
...@@ -37,6 +35,8 @@ export const fetchEpics = ({ state, dispatch }) => { ...@@ -37,6 +35,8 @@ export const fetchEpics = ({ state, dispatch }) => {
Api.groupEpics({ Api.groupEpics({
groupId: state.groupId, groupId: state.groupId,
includeDescendantGroups: false,
includeAncestorGroups: true,
}) })
.then(({ data }) => { .then(({ data }) => {
dispatch('receiveEpicsSuccess', data); dispatch('receiveEpicsSuccess', data);
......
# frozen_string_literal: true # frozen_string_literal: true
# Params:
# iids: integer[]
# state: 'open' or 'closed' or 'all'
# group_id: integer
# parent_id: integer
# author_id: integer
# author_username: string
# label_name: string
# search: string
# sort: string
# start_date: datetime
# end_date: datetime
# created_after: datetime
# created_before: datetime
# updated_after: datetime
# updated_before: datetime
# include_ancestor_groups: boolean
# include_descendant_groups: boolean
class EpicsFinder < IssuableFinder class EpicsFinder < IssuableFinder
def self.scalar_params def self.scalar_params
@scalar_params ||= %i[ @scalar_params ||= %i[
...@@ -59,7 +78,7 @@ class EpicsFinder < IssuableFinder ...@@ -59,7 +78,7 @@ class EpicsFinder < IssuableFinder
# The `group` method takes care of checking permissions # The `group` method takes care of checking permissions
[group] [group]
else else
groups_user_can_read_epics(group.self_and_descendants) groups_user_can_read_epics(related_groups)
end end
Epic.where(group: groups) Epic.where(group: groups)
...@@ -68,6 +87,21 @@ class EpicsFinder < IssuableFinder ...@@ -68,6 +87,21 @@ class EpicsFinder < IssuableFinder
private private
def related_groups
include_ancestors = params.fetch(:include_ancestor_groups, false)
include_descendants = params.fetch(:include_descendant_groups, true)
if include_ancestors && include_descendants
group.self_and_hierarchy
elsif include_ancestors
group.self_and_ancestors
elsif include_descendants
group.self_and_descendants
else
Group.id_in(group.id)
end
end
def count_key(value) def count_key(value)
last_value = Array(value).last last_value = Array(value).last
......
...@@ -34,6 +34,8 @@ module API ...@@ -34,6 +34,8 @@ module API
optional :created_before, type: DateTime, desc: 'Return epics created before the specified time' optional :created_before, type: DateTime, desc: 'Return epics created before the specified time'
optional :updated_after, type: DateTime, desc: 'Return epics updated after the specified time' optional :updated_after, type: DateTime, desc: 'Return epics updated after the specified time'
optional :updated_before, type: DateTime, desc: 'Return epics updated before the specified time' optional :updated_before, type: DateTime, desc: 'Return epics updated before the specified time'
optional :include_ancestor_groups, type: Boolean, default: false, desc: 'Include epics from ancestor groups'
optional :include_descendant_groups, type: Boolean, default: true, desc: 'Include epics from descendant groups'
use :pagination use :pagination
end end
get ':id/(-/)epics' do get ':id/(-/)epics' do
......
...@@ -12,7 +12,7 @@ describe EpicsFinder do ...@@ -12,7 +12,7 @@ describe EpicsFinder do
describe '#execute' do describe '#execute' do
def epics(params = {}) def epics(params = {})
params[:group_id] = group.id params[:group_id] ||= group.id
described_class.new(search_user, params).execute described_class.new(search_user, params).execute
end end
...@@ -113,11 +113,49 @@ describe EpicsFinder do ...@@ -113,11 +113,49 @@ describe EpicsFinder do
context 'when subgroups are supported' do context 'when subgroups are supported' do
let(:subgroup) { create(:group, :private, parent: group) } let(:subgroup) { create(:group, :private, parent: group) }
let(:subgroup2) { create(:group, :private, parent: subgroup) } let(:subgroup2) { create(:group, :private, parent: subgroup) }
let!(:subepic1) { create(:epic, group: subgroup) } let!(:subgroup_epic) { create(:epic, group: subgroup) }
let!(:subepic2) { create(:epic, group: subgroup2) } let!(:subgroup2_epic) { create(:epic, group: subgroup2) }
it 'returns all epics that belong to the given group and its subgroups' do it 'returns all epics that belong to the given group and its subgroups' do
expect(epics).to contain_exactly(epic1, epic2, epic3, subepic1, subepic2) expect(epics).to contain_exactly(epic1, epic2, epic3, subgroup_epic, subgroup2_epic)
end
describe 'hierarchy params' do
let(:finder_params) { {} }
subject { epics(finder_params.merge(group_id: subgroup.id)) }
it 'excludes ancestor groups and includes descendant groups by default' do
is_expected.to contain_exactly(subgroup_epic, subgroup2_epic)
end
context 'when include_descendant_groups is false' do
context 'and include_ancestor_groups is false' do
let(:finder_params) { { include_descendant_groups: false, include_ancestor_groups: false } }
it { is_expected.to contain_exactly(subgroup_epic) }
end
context 'and include_ancestor_groups is true' do
let(:finder_params) { { include_descendant_groups: false, include_ancestor_groups: true } }
it { is_expected.to contain_exactly(subgroup_epic, epic1, epic2, epic3) }
end
end
context 'when include_descendant_groups is true' do
context 'and include_ancestor_groups is false' do
let(:finder_params) { { include_descendant_groups: true, include_ancestor_groups: false } }
it { is_expected.to contain_exactly(subgroup_epic, subgroup2_epic) }
end
context 'and include_ancestor_groups is true' do
let(:finder_params) { { include_descendant_groups: true, include_ancestor_groups: true } }
it { is_expected.to contain_exactly(subgroup_epic, subgroup2_epic, epic1, epic2, epic3) }
end
end
end end
it 'does not execute more than 14 SQL queries' do it 'does not execute more than 14 SQL queries' do
......
...@@ -88,7 +88,7 @@ describe('Api', () => { ...@@ -88,7 +88,7 @@ describe('Api', () => {
describe('groupEpics', () => { describe('groupEpics', () => {
it('calls `axios.get` using param `groupId`', done => { it('calls `axios.get` using param `groupId`', done => {
const groupId = 2; const groupId = 2;
const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${groupId}/epics`; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${groupId}/epics?include_ancestor_groups=false&include_descendant_groups=true`;
mock.onGet(expectedUrl).reply(200, mockEpics); mock.onGet(expectedUrl).reply(200, mockEpics);
......
...@@ -184,7 +184,11 @@ describe('EpicsSelect', () => { ...@@ -184,7 +184,11 @@ describe('EpicsSelect', () => {
dispatch: () => {}, dispatch: () => {},
}); });
expect(Api.groupEpics).toHaveBeenCalledWith({ groupId: state.groupId }); expect(Api.groupEpics).toHaveBeenCalledWith({
groupId: state.groupId,
includeDescendantGroups: false,
includeAncestorGroups: true,
});
}); });
}); });
......
...@@ -330,6 +330,33 @@ describe API::Epics do ...@@ -330,6 +330,33 @@ describe API::Epics do
end end
end end
context 'with hierarchy params' do
let(:subgroup) { create(:group, parent: group) }
let(:subgroup2) { create(:group, parent: subgroup) }
let!(:subgroup_epic) { create(:epic, group: subgroup) }
let!(:subgroup2_epic) { create(:epic, group: subgroup2) }
let(:url) { "/groups/#{subgroup.id}/epics" }
before do
stub_licensed_features(epics: true)
epic
end
it 'excludes descendant group epics' do
get api(url), params: { include_descendant_groups: false }
expect_paginated_array_response(subgroup_epic.id)
end
it 'includes ancestor group epics' do
get api(url), params: { include_ancestor_groups: true }
expect_paginated_array_response([epic.id, subgroup2_epic.id, subgroup_epic.id])
end
end
context 'with pagination params' do context 'with pagination params' do
let(:page) { 1 } let(:page) { 1 }
let(:per_page) { 2 } let(:per_page) { 2 }
......
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