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.

app/serializers/group_child_entity.rb

@@ -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

ee/app/assets/javascripts/iterations/components/iteration_report.vue

@@ -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,

ee/app/assets/javascripts/iterations/index.js

@@ -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,

ee/app/assets/javascripts/iterations/queries/iteration.query.graphql

 #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
       }

ee/app/controllers/projects/iterations/inherited_controller.rb

-# 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

ee/app/controllers/projects/iterations_controller.rb

@@ -8,6 +8,8 @@ class Projects::IterationsController < Projects::ApplicationController
   def index; end
+  def show; end
+
   private
   def check_iterations_available!

ee/app/helpers/ee/timeboxes_routing_helper.rb

@@ -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

ee/app/presenters/iteration_presenter.rb

@@ -12,14 +12,22 @@ def iteration_url
   end
   def scoped_iteration_path(parent:)
-    return unless parent[:parent_object]&.is_a?(Project)
-    url_builder.inherited_iteration_path(parent[:parent_object], iteration)
+    parent_object = parent[:parent_object] || iteration.resource_parent
+    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)
-    url_builder.inherited_iteration_url(parent[:parent_object], iteration)
+    parent_object = parent[:parent_object] || iteration.resource_parent
+    if parent_object&.is_a?(Project)
+      project_iteration_url(parent_object, iteration.id)
+    else
+      group_iteration_url(parent_object, iteration.id)
+    end
   end
 end

ee/app/views/groups/iterations/edit.html.haml

@@ -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) } }

ee/app/views/groups/iterations/show.html.haml

@@ -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) } }

ee/changelogs/unreleased/display-iteration-issues-in-subgroups.yml

+---
+title: Scope iteration report to subgroups
+merge_request: 52926
+author:
+type: fixed

ee/config/routes/project.rb

@@ -114,11 +114,12 @@
           end
         end
-        resources :iterations, only: [:index]
-        namespace :iterations do
-          resources :inherited, only: [:show], constraints: { id: /\d+/ }
-        end
+        # Added for backward compatibility with https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39543
+        # TODO: cleanup after a few releases
+        get 'iterations/inherited/:id', to: redirect('%{namespace_id}/%{project_id}/-/iterations/%{id}'),
+            as: :legacy_project_iterations_inherited
+        resources :iterations, only: [:index, :show], constraints: { id: /\d+/ }
         namespace :incident_management, path: '' do
           resources :oncall_schedules, only: [:index], path: 'oncall_schedules'

ee/spec/features/groups/iterations/iterations_list_spec.rb

@@ -55,7 +55,7 @@
           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

ee/spec/features/groups/iterations/user_edits_iteration_spec.rb

@@ -7,7 +7,7 @@
   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 @@
       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 @@
             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 @@
             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 @@
       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

ee/spec/features/groups/iterations/user_views_iteration_spec.rb

@@ -29,7 +29,7 @@
       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 @@
       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 @@
     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')

ee/spec/features/projects/iterations/iterations_list_spec.rb

@@ -17,21 +17,21 @@
     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

ee/spec/features/projects/iterations/user_views_iteration_spec.rb

@@ -17,13 +17,7 @@
   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 @@
         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 @@
     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')

ee/spec/frontend/iterations/components/iteration_report_spec.js

 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,