Merge branch 'psi-iteration-edit' into 'master'

Update URL when editing iteration

See merge request gitlab-org/gitlab!39296

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

@@ -210,7 +210,7 @@ export default {
         </div>
         <div class="form-group row">
           <div class="col-form-label col-sm-2">
-            <label for="iteration-due-date">{{ __('Due Date') }}</label>
+            <label for="iteration-due-date">{{ __('Due date') }}</label>
           </div>
           <div class="col-sm-10">
             <gl-form-input

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

@@ -23,6 +23,11 @@ const iterationStates = {
   expired: 'expired',
 };
 
+const page = {
+  view: 'viewIteration',
+  edit: 'editIteration',
+};
+
 export default {
   components: {
     GlAlert,
@@ -76,6 +81,11 @@ export default {
       required: false,
       default: false,
     },
+    initiallyEditing: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     namespaceType: {
       type: String,
       required: false,
@@ -90,7 +100,7 @@ export default {
   },
   data() {
     return {
-      isEditing: false,
+      isEditing: this.initiallyEditing,
       error: '',
       iteration: {},
     };
@@ -118,10 +128,36 @@ export default {
       }
     },
   },
+  mounted() {
+    this.boundOnPopState = this.onPopState.bind(this);
+    window.addEventListener('popstate', this.boundOnPopState);
+  },
+  beforeDestroy() {
+    window.removeEventListener('popstate', this.boundOnPopState);
+  },
   methods: {
+    onPopState(e) {
+      if (e.state?.prev === page.view) {
+        this.isEditing = true;
+      } else if (e.state?.prev === page.edit) {
+        this.isEditing = false;
+      } else {
+        this.isEditing = this.initiallyEditing;
+      }
+    },
     formatDate(date) {
       return formatDate(date, 'mmm d, yyyy', true);
     },
+    loadEditPage() {
+      this.isEditing = true;
+      const newUrl = window.location.pathname.replace(/(\/edit)?\/?$/, '/edit');
+      window.history.pushState({ prev: page.view }, null, newUrl);
+    },
+    loadReportPage() {
+      this.isEditing = false;
+      const newUrl = window.location.pathname.replace(/\/edit$/, '');
+      window.history.pushState({ prev: page.edit }, null, newUrl);
+    },
   },
 };
 </script>
@@ -140,11 +176,11 @@ export default {
     <iteration-form
       v-else-if="isEditing"
       :group-path="fullPath"
+      :preview-markdown-path="previewMarkdownPath"
       :is-editing="true"
       :iteration="iteration"
-      :preview-markdown-path="previewMarkdownPath"
-      @updated="isEditing = false"
-      @cancel="isEditing = false"
+      @updated="loadReportPage"
+      @cancel="loadReportPage"
     />
     <template v-else>
       <div
@@ -159,6 +195,7 @@ export default {
         >
         <gl-new-dropdown
           v-if="canEditIteration"
+          data-testid="actions-dropdown"
           variant="default"
           toggle-class="gl-text-decoration-none gl-border-0! gl-shadow-none!"
           class="gl-ml-auto gl-text-secondary"
@@ -168,7 +205,7 @@ export default {
           <template #button-content>
             <gl-icon name="ellipsis_v" /><span class="gl-sr-only">{{ __('Actions') }}</span>
           </template>
-          <gl-new-dropdown-item @click="isEditing = true">{{
+          <gl-new-dropdown-item @click="loadEditPage">{{
             __('Edit iteration')
           }}</gl-new-dropdown-item>
         </gl-new-dropdown>

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

@@ -49,7 +49,7 @@ export function initIterationForm() {
   });
 }
 
-export function initIterationReport(namespaceType) {
+export function initIterationReport({ namespaceType, initiallyEditing } = {}) {
   const el = document.querySelector('.js-iteration');
 
   const {
@@ -74,6 +74,7 @@ export function initIterationReport(namespaceType) {
           editIterationPath,
           namespaceType,
           previewMarkdownPath,
+          initiallyEditing,
         },
       });
     },

ee/app/assets/javascripts/pages/groups/iterations/edit/index.js

+import { initIterationReport } from 'ee/iterations';
+import { Namespace } from 'ee/iterations/constants';
+
+document.addEventListener('DOMContentLoaded', () => {
+  initIterationReport({ namespaceType: Namespace.Group, initiallyEditing: true });
+});

ee/app/assets/javascripts/pages/groups/iterations/show/index.js

@@ -2,5 +2,5 @@ import { initIterationReport } from 'ee/iterations';
 import { Namespace } from 'ee/iterations/constants';
 
 document.addEventListener('DOMContentLoaded', () => {
-  initIterationReport(Namespace.Group);
+  initIterationReport({ namespaceType: Namespace.Group });
 });

ee/app/assets/javascripts/pages/projects/iterations/inherited/show/index.js

@@ -2,5 +2,5 @@ import { initIterationReport } from 'ee/iterations';
 import { Namespace } from 'ee/iterations/constants';
 
 document.addEventListener('DOMContentLoaded', () => {
-  initIterationReport(Namespace.Project);
+  initIterationReport({ namespaceType: Namespace.Project });
 });

ee/app/controllers/groups/iterations_controller.rb

@@ -3,7 +3,7 @@
 class Groups::IterationsController < Groups::ApplicationController
   before_action :check_iterations_available!
   before_action :authorize_show_iteration!, only: [:index, :show]
-  before_action :authorize_create_iteration!, only: :new
+  before_action :authorize_create_iteration!, only: [:new, :edit]
 
   def index; end
 
@@ -11,6 +11,8 @@ class Groups::IterationsController < Groups::ApplicationController
 
   def new; end
 
+  def edit; end
+
   private
 
   def check_iterations_available!

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

+- add_to_breadcrumbs _("Iterations"), group_iterations_path(@group)
+- breadcrumb_title params[:id]
+- page_title _("Edit iteration")
+
+- 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],
+    preview_markdown_path: preview_markdown_path(@group) } }
+

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

@@ -3,4 +3,7 @@
 - page_title _("Iterations")
 
 - if Feature.enabled?(:group_iterations, @group, default_enabled: true)
-  .js-iteration{ data: { full_path: @group.full_path, iteration_iid: params[:id], can_edit: can?(current_user, :admin_iteration, @group).to_s, preview_markdown_path: preview_markdown_path(@group) } }
+  .js-iteration{ data: { full_path: @group.full_path,
+    can_edit: can?(current_user, :admin_iteration, @group).to_s,
+    iteration_iid: params[:id],
+    preview_markdown_path: preview_markdown_path(@group) } }

ee/changelogs/unreleased/psi-iteration-edit.yml

+---
+title: Update URL when editing iteration
+merge_request: 39296
+author:
+type: changed

ee/config/routes/group.rb

@@ -124,7 +124,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
       end
     end
 
-    resources :iterations, only: [:index, :new, :show], constraints: { id: /\d+/ }
+    resources :iterations, only: [:index, :new, :edit, :show], constraints: { id: /\d+/ }
 
     resources :issues, only: [] do
       collection do

ee/spec/controllers/groups/iterations_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::IterationsController do
+  using RSpec::Parameterized::TableSyntax
+
+  let_it_be(:group) { create(:group, :private) }
+  let_it_be(:iteration) { create(:iteration, group: group) }
+  let_it_be(:user) { create(:user) }
+
+  before do
+    stub_licensed_features(iterations: iteration_license_available)
+    group.send("add_#{role}", user) unless role == :none
+    sign_in(user)
+  end
+
+  describe 'index' do
+    subject { get :index, params: { group_id: group } }
+
+    where(:iteration_license_available, :role, :status) do
+      false | :developer | :not_found
+      true  | :none      | :not_found
+      true  | :guest     | :success
+      true  | :developer | :success
+    end
+
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
+    end
+  end
+
+  describe 'show' do
+    subject { get :show, params: { group_id: group, id: iteration } }
+
+    where(:iteration_license_available, :role, :status) do
+      false | :developer | :not_found
+      true  | :none      | :not_found
+      true  | :guest     | :success
+      true  | :developer | :success
+    end
+
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
+    end
+  end
+
+  describe 'new' do
+    subject { get :new, params: { group_id: group } }
+
+    where(:iteration_license_available, :role, :status) do
+      false | :developer | :not_found
+      true  | :none      | :not_found
+      true  | :guest     | :not_found
+      true  | :developer | :success
+    end
+
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
+    end
+  end
+
+  describe 'edit' do
+    subject { get :edit, params: { group_id: group, id: iteration } }
+
+    where(:iteration_license_available, :role, :status) do
+      false | :developer | :not_found
+      true  | :none      | :not_found
+      true  | :guest     | :not_found
+      true  | :developer | :success
+    end
+
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
+    end
+  end
+end

ee/spec/controllers/projects/iterations_controller_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe Projects::IterationsController do
   let_it_be(:project) { create(:project) }
   let_it_be(:user) { create(:user) }
 
-  context 'index' do
+  describe 'index' do
     context 'when iterations license is not available' do
       before do
         stub_licensed_features(iterations: false)

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User views iteration' do
+  let_it_be(:now) { Time.now }
+  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) }
+  dropdown_selector = '[data-testid="actions-dropdown"]'
+
+  context 'with license' do
+    before do
+      stub_licensed_features(iterations: true)
+    end
+
+    context 'as authorized user' do
+      before do
+        sign_in(user)
+      end
+
+      context 'load edit page directly', :js do
+        before do
+          visit edit_group_iteration_path(group, iteration)
+        end
+
+        it 'prefills fields and allows updating all values' do
+          aggregate_failures do
+            expect(title_input.value).to eq(iteration.title)
+            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)
+          end
+
+          updated_title = 'Updated iteration title'
+          updated_desc = 'Updated iteration desc'
+          updated_start_date = now + 4.days
+          updated_due_date = now + 5.days
+
+          fill_in('Title', with: updated_title)
+          fill_in('Description', with: updated_desc)
+          fill_in('Start date', with: updated_start_date.strftime('%Y-%m-%d'))
+          fill_in('Due date', with: updated_due_date.strftime('%Y-%m-%d'))
+          click_button('Update iteration')
+
+          aggregate_failures do
+            expect(page).to have_content(updated_title)
+            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))
+          end
+        end
+      end
+
+      context 'load edit page from report', :js do
+        before do
+          visit group_iteration_path(iteration.group, iteration)
+        end
+
+        it 'prefills fields and updates URL' do
+          find(dropdown_selector).click
+          click_button('Edit iteration')
+
+          aggregate_failures do
+            expect(title_input.value).to eq(iteration.title)
+            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))
+          end
+        end
+      end
+    end
+
+    context 'as guest user' do
+      before do
+        sign_in(guest_user)
+      end
+
+      it 'does not show edit dropdown', :js do
+        visit group_iteration_path(iteration.group, iteration)
+
+        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)
+
+        expect(page).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    def title_input
+      page.find('#iteration-title')
+    end
+
+    def description_input
+      page.find('#iteration-description')
+    end
+
+    def start_date_input
+      page.find('#iteration-start-date')
+    end
+
+    def due_date_input
+      page.find('#iteration-due-date')
+    end
+  end
+end

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

 import { shallowMount } from '@vue/test-utils';
-import { GlNewDropdown, GlEmptyState, GlLoadingIcon, GlTab, GlTabs } from '@gitlab/ui';
+import {
+  GlNewDropdown,
+  GlNewDropdownItem,
+  GlEmptyState,
+  GlLoadingIcon,
+  GlTab,
+  GlTabs,
+} from '@gitlab/ui';
 import IterationReport from 'ee/iterations/components/iteration_report.vue';
+import IterationForm from 'ee/iterations/components/iteration_form.vue';
 import IterationReportSummary from 'ee/iterations/components/iteration_report_summary.vue';
 import IterationReportTabs from 'ee/iterations/components/iteration_report_tabs.vue';
 import { Namespace } from 'ee/iterations/constants';
@@ -15,6 +23,11 @@ describe('Iterations report', () => {
   const findTopbar = () => wrapper.find({ ref: 'topbar' });
   const findTitle = () => wrapper.find({ ref: 'title' });
   const findDescription = () => wrapper.find({ ref: 'description' });
+  const findActionsDropdown = () => wrapper.find('[data-testid="actions-dropdown"]');
+  const clickEditButton = () => {
+    findActionsDropdown().vm.$emit('click');
+    wrapper.find(GlNewDropdownItem).vm.$emit('click');
+  };
 
   const mountComponent = ({ props = defaultProps, loading = false } = {}) => {
     wrapper = shallowMount(IterationReport, {
@@ -55,6 +68,7 @@ describe('Iterations report', () => {
       expect(wrapper.find(GlEmptyState).props('title')).toEqual('Could not find iteration');
       expect(findTitle().exists()).toBe(false);
       expect(findDescription().exists()).toBe(false);
+      expect(findActionsDropdown().exists()).toBe(false);
     });
   });
 
@@ -67,46 +81,127 @@ describe('Iterations report', () => {
       dueDate: '2020-06-08',
     };
 
-    beforeEach(() => {
-      mountComponent({
-        loading: false,
+    describe('user without edit permission', () => {
+      beforeEach(() => {
+        mountComponent({
+          loading: false,
+        });
+
+        wrapper.setData({
+          iteration,
+        });
       });
 
-      wrapper.setData({
-        iteration,
+      it('shows status and date in header', () => {
+        expect(findTopbar().text()).toContain('Open');
+        expect(findTopbar().text()).toContain('Jun 2, 2020');
+        expect(findTopbar().text()).toContain('Jun 8, 2020');
       });
-    });
 
-    it('shows status and date in header', () => {
-      expect(findTopbar().text()).toContain('Open');
-      expect(findTopbar().text()).toContain('Jun 2, 2020');
-      expect(findTopbar().text()).toContain('Jun 8, 2020');
-    });
+      it('hides empty region and loading spinner', () => {
+        expect(wrapper.find(GlLoadingIcon).exists()).toBe(false);
+        expect(wrapper.find(GlEmptyState).exists()).toBe(false);
+      });
 
-    it('hides empty region and loading spinner', () => {
-      expect(wrapper.find(GlLoadingIcon).exists()).toBe(false);
-      expect(wrapper.find(GlEmptyState).exists()).toBe(false);
-    });
+      it('shows title and description', () => {
+        expect(findTitle().text()).toContain(iteration.title);
+        expect(findDescription().text()).toContain(iteration.descriptionHtml);
+      });
 
-    it('shows title and description', () => {
-      expect(findTitle().text()).toContain(iteration.title);
-      expect(findDescription().text()).toContain(iteration.descriptionHtml);
+      it('hides actions dropdown', () => {
+        expect(findActionsDropdown().exists()).toBe(false);
+      });
     });
 
-    it('passes correct props to IterationReportSummary', () => {
-      const iterationReportSummary = wrapper.find(IterationReportSummary);
+    describe('user with edit permission', () => {
+      describe('loading report view', () => {
+        beforeEach(() => {
+          mountComponent({
+            props: {
+              ...defaultProps,
+              canEdit: true,
+            },
+            loading: false,
+          });
 
-      expect(iterationReportSummary.props('fullPath')).toBe(defaultProps.fullPath);
-      expect(iterationReportSummary.props('iterationId')).toBe(iteration.id);
-      expect(iterationReportSummary.props('namespaceType')).toBe(Namespace.Group);
-    });
+          wrapper.setData({
+            iteration,
+          });
+        });
+
+        it('updates URL when loading form', async () => {
+          jest.spyOn(window.history, 'pushState').mockImplementation(() => {});
+
+          clickEditButton();
+
+          await wrapper.vm.$nextTick();
 
-    it('passes correct props to IterationReportTabs', () => {
-      const iterationReportTabs = wrapper.find(IterationReportTabs);
+          expect(window.history.pushState).toHaveBeenCalledWith(
+            { prev: 'viewIteration' },
+            null,
+            '/edit',
+          );
+        });
 
-      expect(iterationReportTabs.props('fullPath')).toBe(defaultProps.fullPath);
-      expect(iterationReportTabs.props('iterationId')).toBe(iteration.id);
-      expect(iterationReportTabs.props('namespaceType')).toBe(Namespace.Group);
+        it('passes correct props to IterationReportSummary', () => {
+          const iterationReportSummary = wrapper.find(IterationReportSummary);
+
+          expect(iterationReportSummary.props('fullPath')).toBe(defaultProps.fullPath);
+          expect(iterationReportSummary.props('iterationId')).toBe(iteration.id);
+          expect(iterationReportSummary.props('namespaceType')).toBe(Namespace.Group);
+        });
+
+        it('passes correct props to IterationReportTabs', () => {
+          const iterationReportTabs = wrapper.find(IterationReportTabs);
+
+          expect(iterationReportTabs.props('fullPath')).toBe(defaultProps.fullPath);
+          expect(iterationReportTabs.props('iterationId')).toBe(iteration.id);
+          expect(iterationReportTabs.props('namespaceType')).toBe(Namespace.Group);
+        });
+      });
+
+      describe('loading edit form directly', () => {
+        beforeEach(() => {
+          mountComponent({
+            props: {
+              ...defaultProps,
+              canEdit: true,
+              initiallyEditing: true,
+            },
+            loading: false,
+          });
+
+          wrapper.setData({
+            iteration,
+          });
+        });
+
+        it('updates URL when cancelling form submit', async () => {
+          jest.spyOn(window.history, 'pushState').mockImplementation(() => {});
+          wrapper.find(IterationForm).vm.$emit('cancel');
+
+          await wrapper.vm.$nextTick();
+
+          expect(window.history.pushState).toHaveBeenCalledWith(
+            { prev: 'editIteration' },
+            null,
+            '/',
+          );
+        });
+
+        it('updates URL after form submitted', async () => {
+          jest.spyOn(window.history, 'pushState').mockImplementation(() => {});
+          wrapper.find(IterationForm).vm.$emit('updated');
+
+          await wrapper.vm.$nextTick();
+
+          expect(window.history.pushState).toHaveBeenCalledWith(
+            { prev: 'editIteration' },
+            null,
+            '/',
+          );
+        });
+      });
     });
 
     describe('actions dropdown to edit iteration', () => {