Commit 54faed69 authored by Alexandru Croitor's avatar Alexandru Croitor Committed by Michael Kozono

Scope iteration report to subgroups

When navigating to an iteration in subgroup it redirects to
the source group instead of scoping the issues related to an
iteration to given subgroup.
parent 54c5a90e
......@@ -20,7 +20,7 @@ class GroupChildEntity < Grape::Entity
# We know `type` will be one either `project` or `group`.
# The `edit_polymorphic_path` helper would try to call the path helper
# with a plural: `edit_groups_path(instance)` or `edit_projects_path(instance)`
# while our methods are `edit_group_path` or `edit_group_path`
# while our methods are `edit_group_path` or `edit_project_path`
public_send("edit_#{type}_path", instance) # rubocop:disable GitlabSecurity/PublicSend
end
......
......@@ -13,6 +13,7 @@ import BurnCharts from 'ee/burndown_chart/components/burn_charts.vue';
import { formatDate } from '~/lib/utils/datetime_utility';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { __ } from '~/locale';
import { convertToGraphQLId } from '~/graphql_shared/utils';
import query from '../queries/iteration.query.graphql';
import { Namespace } from '../constants';
import IterationForm from './iteration_form.vue';
......@@ -45,17 +46,17 @@ export default {
apollo: {
iteration: {
query,
/* eslint-disable @gitlab/require-i18n-strings */
variables() {
return {
fullPath: this.fullPath,
id: `gid://gitlab/Iteration/${this.iterationId}`,
iid: this.iterationIid,
hasId: Boolean(this.iterationId),
hasIid: Boolean(this.iterationIid),
id: convertToGraphQLId('Iteration', this.iterationId),
isGroup: this.namespaceType === Namespace.Group,
};
},
/* eslint-enable @gitlab/require-i18n-strings */
update(data) {
return data.group?.iterations?.nodes[0] || data.iteration || {};
return data[this.namespaceType]?.iterations?.nodes[0] || {};
},
error(err) {
this.error = err.message;
......@@ -73,11 +74,6 @@ export default {
required: false,
default: undefined,
},
iterationIid: {
type: String,
required: false,
default: undefined,
},
canEdit: {
type: Boolean,
required: false,
......
......@@ -60,7 +60,6 @@ export function initIterationReport({ namespaceType, initiallyEditing } = {}) {
const {
fullPath,
iterationId,
iterationIid,
labelsFetchPath,
editIterationPath,
previewMarkdownPath,
......@@ -75,7 +74,6 @@ export function initIterationReport({ namespaceType, initiallyEditing } = {}) {
props: {
fullPath,
iterationId,
iterationIid,
labelsFetchPath,
canEdit,
editIterationPath,
......
#import "./iteration_report.fragment.graphql"
query Iteration(
$fullPath: ID!
$id: IterationID!
$iid: ID
$hasId: Boolean = false
$hasIid: Boolean = false
) {
iteration(id: $id) @include(if: $hasId) {
...IterationReport
query Iteration($fullPath: ID!, $id: ID!, $isGroup: Boolean = true) {
group(fullPath: $fullPath) @include(if: $isGroup) {
iterations(id: $id, first: 1, includeAncestors: true) {
nodes {
...IterationReport
}
}
}
group(fullPath: $fullPath) @include(if: $hasIid) {
iterations(iid: $iid, first: 1, includeAncestors: false) {
project(fullPath: $fullPath) @skip(if: $isGroup) {
iterations(id: $id, first: 1, includeAncestors: true) {
nodes {
...IterationReport
}
......
# frozen_string_literal: true
class Projects::Iterations::InheritedController < Projects::ApplicationController
before_action :check_iterations_available!
before_action :authorize_show_iteration!
feature_category :issue_tracking
def show; end
private
def check_iterations_available!
render_404 unless project.feature_available?(:iterations)
end
def authorize_show_iteration!
render_404 unless can?(current_user, :read_iteration, project)
end
end
......@@ -8,6 +8,8 @@ class Projects::IterationsController < Projects::ApplicationController
def index; end
def show; end
private
def check_iterations_available!
......
......@@ -4,32 +4,18 @@ module EE
module TimeboxesRoutingHelper
def iteration_path(iteration, *args)
if iteration.group_timebox?
group_iteration_path(iteration.group, iteration, *args)
group_iteration_path(iteration.group, iteration.id, *args)
elsif iteration.project_timebox?
# We don't have project iteration routes yet, so for now send users to the project itself
project_path(iteration.project, *args)
project_iteration_path(iteration.project, iteration.id, *args)
end
end
def iteration_url(iteration, *args)
if iteration.group_timebox?
group_iteration_url(iteration.group, iteration, *args)
group_iteration_url(iteration.group, iteration.id, *args)
elsif iteration.project_timebox?
# We don't have project iteration routes yet, so for now send users to the project itself
project_url(iteration.project, *args)
project_iteration_url(iteration.project, iteration.id, *args)
end
end
def inherited_iteration_path(project, iteration, *args)
return unless iteration.group_timebox?
project_iterations_inherited_path(project, iteration.id, *args)
end
def inherited_iteration_url(project, iteration, *args)
return unless iteration.group_timebox?
project_iterations_inherited_url(project, iteration.id, *args)
end
end
end
......@@ -12,14 +12,22 @@ class IterationPresenter < Gitlab::View::Presenter::Delegated
end
def scoped_iteration_path(parent:)
return unless parent[:parent_object]&.is_a?(Project)
parent_object = parent[:parent_object] || iteration.resource_parent
url_builder.inherited_iteration_path(parent[:parent_object], iteration)
if parent_object&.is_a?(Project)
project_iteration_path(parent_object, iteration.id, only_path: true)
else
group_iteration_path(parent_object, iteration.id, only_path: true)
end
end
def scoped_iteration_url(parent:)
return unless parent[:parent_object]&.is_a?(Project)
parent_object = parent[:parent_object] || iteration.resource_parent
url_builder.inherited_iteration_url(parent[:parent_object], iteration)
if parent_object&.is_a?(Project)
project_iteration_url(parent_object, iteration.id)
else
group_iteration_url(parent_object, iteration.id)
end
end
end
......@@ -5,6 +5,6 @@
- if Feature.enabled?(:group_iterations, @group, default_enabled: true)
.js-iteration{ data: { full_path: @group.full_path,
can_edit: can?(current_user, :admin_iteration, @group).to_s,
iteration_iid: params[:id],
iteration_id: params[:id],
preview_markdown_path: preview_markdown_path(@group) } }
......@@ -5,6 +5,6 @@
- if Feature.enabled?(:group_iterations, @group, default_enabled: true)
.js-iteration{ data: { full_path: @group.full_path,
can_edit: can?(current_user, :admin_iteration, @group).to_s,
iteration_iid: params[:id],
iteration_id: params[:id],
labels_fetch_path: group_labels_path(@group, format: :json, include_ancestor_groups: true),
preview_markdown_path: preview_markdown_path(@group) } }
---
title: Scope iteration report to subgroups
merge_request: 52926
author:
type: fixed
......@@ -114,11 +114,12 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
resources :iterations, only: [:index]
# Added for backward compatibility with https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39543
# TODO: Cleanup https://gitlab.com/gitlab-org/gitlab/-/issues/320814
get 'iterations/inherited/:id', to: redirect('%{namespace_id}/%{project_id}/-/iterations/%{id}'),
as: :legacy_project_iterations_inherited
namespace :iterations do
resources :inherited, only: [:show], constraints: { id: /\d+/ }
end
resources :iterations, only: [:index, :show], constraints: { id: /\d+/ }
namespace :incident_management, path: '' do
resources :oncall_schedules, only: [:index], path: 'oncall_schedules'
......
......@@ -55,7 +55,7 @@ RSpec.describe 'Iterations list', :js do
wait_for_requests
expect(page).to have_current_path(group_iteration_path(group, started_iteration.iid))
expect(page).to have_current_path(group_iteration_path(group, started_iteration.id))
end
end
end
......
......@@ -7,7 +7,7 @@ RSpec.describe 'User views iteration' do
let_it_be(:group) { create(:group) }
let_it_be(:user) { create(:group_member, :maintainer, user: create(:user), group: group ).user }
let_it_be(:guest_user) { create(:group_member, :guest, user: create(:user), group: group ).user }
let_it_be(:iteration) { create(:iteration, :skip_future_date_validation, iid: 1, id: 2, group: group, title: 'Correct Iteration', description: 'Iteration description', start_date: now - 1.day, due_date: now) }
let_it_be(:iteration) { create(:iteration, :skip_future_date_validation, group: group, title: 'Correct Iteration', description: 'Iteration description', start_date: now - 1.day, due_date: now) }
dropdown_selector = '[data-testid="actions-dropdown"]'
context 'with license' do
......@@ -22,7 +22,7 @@ RSpec.describe 'User views iteration' do
context 'load edit page directly', :js do
before do
visit edit_group_iteration_path(group, iteration)
visit edit_group_iteration_path(group, iteration.id)
end
it 'prefills fields and allows updating all values' do
......@@ -49,14 +49,14 @@ RSpec.describe 'User views iteration' do
expect(page).to have_content(updated_desc)
expect(page).to have_content(updated_start_date.strftime('%b %-d, %Y'))
expect(page).to have_content(updated_due_date.strftime('%b %-d, %Y'))
expect(page).to have_current_path(group_iteration_path(group, iteration))
expect(page).to have_current_path(group_iteration_path(group, iteration.id))
end
end
end
context 'load edit page from report', :js do
before do
visit group_iteration_path(iteration.group, iteration)
visit group_iteration_path(iteration.group, iteration.id)
end
it 'prefills fields and updates URL' do
......@@ -68,7 +68,7 @@ RSpec.describe 'User views iteration' do
expect(description_input.value).to eq(iteration.description)
expect(start_date_input.value).to have_content(iteration.start_date)
expect(due_date_input.value).to have_content(iteration.due_date)
expect(page).to have_current_path(edit_group_iteration_path(iteration.group, iteration))
expect(page).to have_current_path(edit_group_iteration_path(iteration.group, iteration.id))
end
end
end
......@@ -80,14 +80,14 @@ RSpec.describe 'User views iteration' do
end
it 'does not show edit dropdown', :js do
visit group_iteration_path(iteration.group, iteration)
visit group_iteration_path(iteration.group, iteration.id)
expect(page).to have_content(iteration.title)
expect(page).not_to have_selector(dropdown_selector)
end
it '404s when loading edit page directly' do
visit edit_group_iteration_path(iteration.group, iteration)
visit edit_group_iteration_path(iteration.group, iteration.id)
expect(page).to have_gitlab_http_status(:not_found)
end
......
......@@ -29,7 +29,7 @@ RSpec.describe 'User views iteration' do
before do
sign_in(current_user)
visit group_iteration_path(iteration.group, iteration.iid)
visit group_iteration_path(iteration.group, iteration.id)
end
it 'shows iteration info' do
......@@ -85,7 +85,7 @@ RSpec.describe 'User views iteration' do
before do
sign_in(user)
visit group_iteration_path(iteration.group, iteration.iid)
visit group_iteration_path(iteration.group, iteration.id)
end
it_behaves_like 'iteration report group by label'
......@@ -99,7 +99,7 @@ RSpec.describe 'User views iteration' do
end
it 'shows page not found' do
visit group_iteration_path(iteration.group, iteration.iid)
visit group_iteration_path(iteration.group, iteration.id)
expect(page).to have_title('Not Found')
expect(page).to have_content('Page Not Found')
......
......@@ -17,21 +17,21 @@ RSpec.describe 'Iterations list', :js do
end
it 'shows iterations on each tab', :aggregate_failures do
expect(page).to have_link(started_iteration.title, href: project_iterations_inherited_path(project, started_iteration.id))
expect(page).to have_link(upcoming_iteration.title, href: project_iterations_inherited_path(project, upcoming_iteration.id))
expect(page).to have_link(started_iteration.title, href: project_iteration_path(project, started_iteration.id))
expect(page).to have_link(upcoming_iteration.title, href: project_iteration_path(project, upcoming_iteration.id))
expect(page).not_to have_link(closed_iteration.title)
click_link('Closed')
expect(page).to have_link(closed_iteration.title, href: project_iterations_inherited_path(project, closed_iteration.id))
expect(page).to have_link(closed_iteration.title, href: project_iteration_path(project, closed_iteration.id))
expect(page).not_to have_link(started_iteration.title)
expect(page).not_to have_link(upcoming_iteration.title)
click_link('All')
expect(page).to have_link(started_iteration.title, href: project_iterations_inherited_path(project, started_iteration.id))
expect(page).to have_link(upcoming_iteration.title, href: project_iterations_inherited_path(project, upcoming_iteration.id))
expect(page).to have_link(closed_iteration.title, href: project_iterations_inherited_path(project, closed_iteration.id))
expect(page).to have_link(started_iteration.title, href: project_iteration_path(project, started_iteration.id))
expect(page).to have_link(upcoming_iteration.title, href: project_iteration_path(project, upcoming_iteration.id))
expect(page).to have_link(closed_iteration.title, href: project_iteration_path(project, closed_iteration.id))
end
end
......
......@@ -17,13 +17,7 @@ RSpec.describe 'User views iteration' do
let_it_be(:other_iteration_issue) { create(:issue, project: project, iteration: other_iteration) }
let_it_be(:other_project_issue) { create(:issue, project: project_2, iteration: iteration, assignees: [user], labels: [label1]) }
context 'with license', :js do
before do
stub_licensed_features(iterations: true)
sign_in(user)
visit project_iterations_inherited_path(project, iteration.id)
end
RSpec.shared_examples 'render iteration page' do
context 'view an iteration' do
it 'shows iteration info' do
aggregate_failures 'shows iteration info and dates' do
......@@ -56,10 +50,31 @@ RSpec.describe 'User views iteration' do
end
end
end
end
context 'with license', :js do
let(:url) { project_iteration_path(project, iteration.id) }
before do
stub_licensed_features(iterations: true)
sign_in(user)
visit url
end
it_behaves_like 'render iteration page'
context 'when grouping by label' do
it_behaves_like 'iteration report group by label'
end
context 'with old routes' do
# for backward compatibility we redirect /-/iterations/inherited/ID to /-/iterations/ID and render iteration page
let(:url) { "#{project_path(project)}/-/iterations/inherited/#{iteration.id}" }
it { expect(current_path).to eq("#{project_path(project)}/-/iterations/#{iteration.id}") }
it_behaves_like 'render iteration page'
end
end
context 'without license' do
......@@ -69,7 +84,7 @@ RSpec.describe 'User views iteration' do
end
it 'shows page not found' do
visit project_iterations_inherited_path(project, iteration.id)
visit project_iteration_path(project, iteration.id)
expect(page).to have_title('Not Found')
expect(page).to have_content('Page Not Found')
......
import { GlDropdown, GlDropdownItem, GlEmptyState, GlLoadingIcon, GlTab, GlTabs } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import { shallowMount, createLocalVue } from '@vue/test-utils';
import VueApollo from 'vue-apollo';
import IterationForm from 'ee/iterations/components/iteration_form.vue';
import IterationReport from 'ee/iterations/components/iteration_report.vue';
import IterationReportTabs from 'ee/iterations/components/iteration_report_tabs.vue';
import { Namespace } from 'ee/iterations/constants';
import createMockApollo from 'helpers/mock_apollo_helper';
import query from 'ee/iterations/queries/iteration.query.graphql';
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import waitForPromises from 'helpers/wait_for_promises';
import { mockIterationNode, mockGroupIterations, mockProjectIterations } from '../mock_data';
const localVue = createLocalVue();
describe('Iterations report', () => {
let wrapper;
let mockApollo;
const defaultProps = {
fullPath: 'gitlab-org',
iterationIid: '3',
labelsFetchPath: '/gitlab-org/gitlab-test/-/labels.json?include_ancestor_groups=true',
};
......@@ -22,6 +31,78 @@ describe('Iterations report', () => {
wrapper.find(GlDropdownItem).vm.$emit('click');
};
const mountComponentWithApollo = ({
props = defaultProps,
iterationQueryHandler = jest.fn(),
} = {}) => {
localVue.use(VueApollo);
mockApollo = createMockApollo([[query, iterationQueryHandler]]);
wrapper = shallowMount(IterationReport, {
localVue,
apolloProvider: mockApollo,
propsData: props,
stubs: {
GlLoadingIcon,
GlTab,
GlTabs,
},
});
};
describe('with mock apollo', () => {
describe.each([
[
'group',
{
fullPath: 'group-name',
iterationId: String(getIdFromGraphQLId(mockIterationNode.id)),
},
mockGroupIterations,
{
fullPath: 'group-name',
id: mockIterationNode.id,
isGroup: true,
},
],
[
'project',
{
fullPath: 'group-name/project-name',
iterationId: String(getIdFromGraphQLId(mockIterationNode.id)),
namespaceType: Namespace.Project,
},
mockProjectIterations,
{
fullPath: 'group-name/project-name',
id: mockIterationNode.id,
isGroup: false,
},
],
])('when viewing an iteration in a %s', (_, props, mockIteration, expectedParams) => {
it('calls a query with correct parameters', () => {
const iterationQueryHandler = jest.fn();
mountComponentWithApollo({
props,
iterationQueryHandler,
});
expect(iterationQueryHandler).toHaveBeenNthCalledWith(1, expectedParams);
});
it('renders an iteration title', async () => {
mountComponentWithApollo({
props,
iterationQueryHandler: jest.fn().mockResolvedValue(mockIteration),
});
await waitForPromises();
expect(findTitle().text()).toContain(mockIterationNode.title);
});
});
});
const mountComponent = ({ props = defaultProps, loading = false } = {}) => {
wrapper = shallowMount(IterationReport, {
propsData: props,
......
export const mockIterationNode = {
description: 'some description',
descriptionHtml: '<p></p>',
dueDate: '2021-02-17',
id: 'gid://gitlab/Iteration/4',
iid: '1',
startDate: '2021-02-10',
state: 'upcoming',
title: 'top-level-iteration',
webPath: '/groups/top-level-group/-/iterations/4',
__typename: 'Iteration',
};
export const mockGroupIterations = {
data: {
group: {
iterations: {
nodes: [mockIterationNode],
__typename: 'IterationConnection',
},
__typename: 'Group',
},
},
};
export const mockProjectIterations = {
data: {
project: {
iterations: {
nodes: [mockIterationNode],
__typename: 'IterationConnection',
},
__typename: 'Project',
},
},
};
......@@ -56,6 +56,8 @@ RSpec.describe 'Querying an Iteration' do
nodes {
scopedPath
scopedUrl
webPath
webUrl
}
NODES
......@@ -67,14 +69,24 @@ RSpec.describe 'Querying an Iteration' do
end
specify do
expect(subject).to include('scopedPath' => expected_scope_path, 'scopedUrl' => expected_scope_url)
expect(subject).to include(
'scopedPath' => expected_scope_path,
'scopedUrl' => expected_scope_url,
'webPath' => expected_web_path,
'webUrl' => expected_web_url
)
end
context 'when given a raw model id (backward compatibility)' do
let(:queried_iteration_id) { queried_iteration.id }
specify do
expect(subject).to include('scopedPath' => expected_scope_path, 'scopedUrl' => expected_scope_url)
expect(subject).to include(
'scopedPath' => expected_scope_path,
'scopedUrl' => expected_scope_url,
'webPath' => expected_web_path,
'webUrl' => expected_web_url
)
end
end
end
......@@ -89,16 +101,20 @@ RSpec.describe 'Querying an Iteration' do
describe 'group-owned iteration' do
it_behaves_like 'scoped path' do
let(:queried_iteration) { iteration }
let(:expected_scope_path) { project_iterations_inherited_path(project, iteration.id) }
let(:expected_scope_path) { project_iteration_path(project, iteration.id) }
let(:expected_scope_url) { /#{expected_scope_path}$/ }
let(:expected_web_path) { group_iteration_path(group, iteration.id) }
let(:expected_web_url) { /#{expected_web_path}$/ }
end
end
describe 'project-owned iteration' do
it_behaves_like 'scoped path' do
let(:queried_iteration) { project_iteration }
let(:expected_scope_path) { nil }
let(:expected_scope_url) { nil }
let(:expected_scope_path) { project_iteration_path(project, project_iteration.id) }
let(:expected_scope_url) { /#{expected_scope_path}$/ }
let(:expected_web_path) { project_iteration_path(project, project_iteration.id) }
let(:expected_web_url) { /#{expected_web_path}$/ }
end
end
end
......@@ -113,8 +129,25 @@ RSpec.describe 'Querying an Iteration' do
describe 'group-owned iteration' do
it_behaves_like 'scoped path' do
let(:queried_iteration) { iteration }
let(:expected_scope_path) { nil }
let(:expected_scope_url) { nil }
let(:expected_scope_path) { group_iteration_path(group, iteration.id) }
let(:expected_scope_url) { /#{expected_scope_path}$/ }
let(:expected_web_path) { group_iteration_path(group, iteration.id) }
let(:expected_web_url) { /#{expected_web_path}$/ }
end
end
describe 'group-owned iteration' do
let(:sub_group) { create(:group, :private, parent: group) }
let(:query) do
graphql_query_for('group', { full_path: sub_group.full_path }, iteration_nodes)
end
it_behaves_like 'scoped path' do
let(:queried_iteration) { iteration }
let(:expected_scope_path) { group_iteration_path(sub_group, iteration.id) }
let(:expected_scope_url) { /#{expected_scope_path}$/ }
let(:expected_web_path) { group_iteration_path(group, iteration.id) }
let(:expected_web_url) { /#{expected_web_path}$/ }
end
end
end
......@@ -123,22 +156,26 @@ RSpec.describe 'Querying an Iteration' do
subject { graphql_data['iteration'] }
let(:query) do
graphql_query_for('iteration', { id: iteration.to_global_id.to_s }, [:scoped_path, :scoped_url])
graphql_query_for('iteration', { id: iteration.to_global_id.to_s }, [:scoped_path, :scoped_url, :web_path, :web_url])
end
describe 'group-owned iteration' do
it_behaves_like 'scoped path' do
let(:queried_iteration) { iteration }
let(:expected_scope_path) { nil }
let(:expected_scope_url) { nil }
let(:expected_scope_path) { group_iteration_path(group, iteration.id) }
let(:expected_scope_url) { /#{expected_scope_path}$/ }
let(:expected_web_path) { group_iteration_path(group, iteration.id) }
let(:expected_web_url) { /#{expected_web_path}$/ }
end
end
describe 'project-owned iteration' do
it_behaves_like 'scoped path' do
let(:queried_iteration) { project_iteration }
let(:expected_scope_path) { nil }
let(:expected_scope_url) { nil }
let(:expected_scope_path) { group_iteration_path(group, iteration.id) }
let(:expected_scope_url) { /#{expected_scope_path}$/ }
let(:expected_web_path) { group_iteration_path(group, iteration.id) }
let(:expected_web_url) { /#{expected_web_path}$/ }
end
end
end
......
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