Merge branch '34239-raise-warning-when-closing-an-issue-with-open-blockers' into 'master'

Close blocked issue warning - Issue page bottom

See merge request gitlab-org/gitlab!25089

app/assets/javascripts/notes/components/comment_form.vue

@@ -3,6 +3,7 @@ import $ from 'jquery';
 import { mapActions, mapGetters, mapState } from 'vuex';
 import { isEmpty } from 'lodash';
 import Autosize from 'autosize';
+import { GlAlert, GlIntersperse, GlLink, GlSprintf } from '@gitlab/ui';
 import { __, sprintf } from '~/locale';
 import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
 import Flash from '../../flash';
@@ -34,6 +35,10 @@ export default {
     userAvatarLink,
     loadingButton,
     TimelineEntryItem,
+    GlAlert,
+    GlIntersperse,
+    GlLink,
+    GlSprintf,
   },
   mixins: [issuableStateMixin],
   props: {
@@ -57,8 +62,9 @@ export default {
       'getNoteableData',
       'getNotesData',
       'openState',
+      'getBlockedByIssues',
     ]),
-    ...mapState(['isToggleStateButtonLoading']),
+    ...mapState(['isToggleStateButtonLoading', 'isToggleBlockedIssueWarning']),
     noteableDisplayName() {
       return splitCamelCase(this.noteableType).toLowerCase();
     },
@@ -159,6 +165,7 @@ export default {
       'reopenIssue',
       'toggleIssueLocalState',
       'toggleStateButtonLoading',
+      'toggleBlockedIssueWarning',
     ]),
     setIsSubmitButtonDisabled(note, isSubmitting) {
       if (!isEmpty(note) && !isSubmitting) {
@@ -220,22 +227,17 @@ export default {
       this.isSubmitting = false;
     },
     toggleIssueState() {
+      if (
+        this.noteableType.toLowerCase() === constants.ISSUE_NOTEABLE_TYPE &&
+        this.isOpen &&
+        this.getBlockedByIssues &&
+        this.getBlockedByIssues.length > 0
+      ) {
+        this.toggleBlockedIssueWarning(true);
+        return;
+      }
       if (this.isOpen) {
-        this.closeIssue()
-          .then(() => {
-            this.enableButton();
-            refreshUserMergeRequestCounts();
-          })
-          .catch(() => {
-            this.enableButton();
-            this.toggleStateButtonLoading(false);
-            Flash(
-              sprintf(
-                __('Something went wrong while closing the %{issuable}. Please try again later'),
-                { issuable: this.noteableDisplayName },
-              ),
-            );
-          });
+        this.forceCloseIssue();
       } else {
         this.reopenIssue()
           .then(() => {
@@ -258,6 +260,23 @@ export default {
           });
       }
     },
+    forceCloseIssue() {
+      this.closeIssue()
+        .then(() => {
+          this.enableButton();
+          refreshUserMergeRequestCounts();
+        })
+        .catch(() => {
+          this.enableButton();
+          this.toggleStateButtonLoading(false);
+          Flash(
+            sprintf(
+              __('Something went wrong while closing the %{issuable}. Please try again later'),
+              { issuable: this.noteableDisplayName },
+            ),
+          );
+        });
+    },
     discard(shouldClear = true) {
       // `blur` is needed to clear slash commands autocomplete cache if event fired.
       // `focus` is needed to remain cursor in the textarea.
@@ -361,6 +380,36 @@ js-gfm-input js-autosize markdown-area js-vue-textarea qa-comment-input"
               >
               </textarea>
             </markdown-field>
+            <gl-alert
+              v-if="isToggleBlockedIssueWarning"
+              class="prepend-top-16"
+              :title="__('Are you sure you want to close this blocked issue?')"
+              :primary-button-text="__('Yes, close issue')"
+              :secondary-button-text="__('Cancel')"
+              variant="warning"
+              :dismissible="false"
+              @primaryAction="forceCloseIssue"
+              @secondaryAction="toggleBlockedIssueWarning(false) && enableButton()"
+            >
+              <p>
+                <gl-sprintf
+                  :message="
+                    __('This issue is currently blocked by the following issues: %{issues}.')
+                  "
+                >
+                  <template #issues>
+                    <gl-intersperse>
+                      <gl-link
+                        v-for="blockingIssue in getBlockedByIssues"
+                        :key="blockingIssue.web_url"
+                        :href="blockingIssue.web_url"
+                        >#{{ blockingIssue.iid }}</gl-link
+                      >
+                    </gl-intersperse>
+                  </template>
+                </gl-sprintf>
+              </p>
+            </gl-alert>
             <div class="note-form-actions">
               <div
                 class="float-left btn-group
@@ -427,7 +476,7 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown"
               </div>
 
               <loading-button
-                v-if="canToggleIssueState"
+                v-if="canToggleIssueState && !isToggleBlockedIssueWarning"
                 :loading="isToggleStateButtonLoading"
                 :container-class="[
                   actionButtonClassNames,

app/assets/javascripts/notes/stores/actions.js

@@ -185,12 +185,27 @@ export const toggleResolveNote = ({ commit, dispatch }, { endpoint, isResolved,
   });
 };
 
+export const toggleBlockedIssueWarning = ({ commit }, value) => {
+  commit(types.TOGGLE_BLOCKED_ISSUE_WARNING, value);
+  // Hides Close issue button at the top of issue page
+  const closeDropdown = document.querySelector('.js-issuable-close-dropdown');
+  if (closeDropdown) {
+    closeDropdown.classList.toggle('d-none');
+  } else {
+    const closeButton = document.querySelector(
+      '.detail-page-header-actions .btn-close.btn-grouped',
+    );
+    closeButton.classList.toggle('d-md-block');
+  }
+};
+
 export const closeIssue = ({ commit, dispatch, state }) => {
   dispatch('toggleStateButtonLoading', true);
   return axios.put(state.notesData.closePath).then(({ data }) => {
     commit(types.CLOSE_ISSUE);
     dispatch('emitStateChangedEvent', data);
     dispatch('toggleStateButtonLoading', false);
+    dispatch('toggleBlockedIssueWarning', false);
   });
 };
 

app/assets/javascripts/notes/stores/getters.js

@@ -35,6 +35,8 @@ export const getNoteableData = state => state.noteableData;
 
 export const getNoteableDataByProp = state => prop => state.noteableData[prop];
 
+export const getBlockedByIssues = state => state.noteableData.blocked_by_issues;
+
 export const userCanReply = state => Boolean(state.noteableData.current_user.can_create_note);
 
 export const openState = state => state.noteableData.state;

app/assets/javascripts/notes/stores/modules/index.js

@@ -14,6 +14,7 @@ export default () => ({
 
     // View layer
     isToggleStateButtonLoading: false,
+    isToggleBlockedIssueWarning: false,
     isNotesFetched: false,
     isLoading: true,
     isLoadingDescriptionVersion: false,

app/assets/javascripts/notes/stores/mutation_types.js

@@ -33,6 +33,7 @@ export const SET_DISCUSSIONS_SORT = 'SET_DISCUSSIONS_SORT';
 export const CLOSE_ISSUE = 'CLOSE_ISSUE';
 export const REOPEN_ISSUE = 'REOPEN_ISSUE';
 export const TOGGLE_STATE_BUTTON_LOADING = 'TOGGLE_STATE_BUTTON_LOADING';
+export const TOGGLE_BLOCKED_ISSUE_WARNING = 'TOGGLE_BLOCKED_ISSUE_WARNING';
 
 // Description version
 export const REQUEST_DESCRIPTION_VERSION = 'REQUEST_DESCRIPTION_VERSION';

app/assets/javascripts/notes/stores/mutations.js

@@ -249,6 +249,10 @@ export default {
     Object.assign(state, { isToggleStateButtonLoading: value });
   },
 
+  [types.TOGGLE_BLOCKED_ISSUE_WARNING](state, value) {
+    Object.assign(state, { isToggleBlockedIssueWarning: value });
+  },
+
   [types.SET_NOTES_FETCHED_STATE](state, value) {
     Object.assign(state, { isNotesFetched: value });
   },

ee/app/serializers/ee/issue_entity.rb

@@ -11,7 +11,7 @@ module EE
 
         expose :blocked_by_issues do |issue|
           issues = issue.blocked_by_issues(request.current_user)
-          serializer_options = options.merge(only: [:id, :web_url])
+          serializer_options = options.merge(only: [:iid, :web_url])
 
           ::IssueEntity.represent(issues, serializer_options)
         end

ee/changelogs/unreleased/34239-raise-warning-when-closing-an-issue-with-open-blockers.yml

+---
+title: Warn users when they close a blocked issue
+merge_request: 25089
+author:
+type: added

ee/spec/frontend/notes/components/comment_form_spec.js

+import { mount } from '@vue/test-utils';
+import MockAdapter from 'axios-mock-adapter';
+import axios from '~/lib/utils/axios_utils';
+import createStore from '~/notes/stores';
+import CommentForm from '~/notes/components/comment_form.vue';
+import {
+  notesDataMock,
+  userDataMock,
+  noteableDataMock,
+} from '../../../../../spec/frontend/notes/mock_data';
+
+jest.mock('autosize');
+jest.mock('~/commons/nav/user_merge_requests');
+jest.mock('~/gl_form');
+
+describe('issue_comment_form component', () => {
+  let store;
+  let wrapper;
+  let axiosMock;
+
+  const setupStore = (userData, noteableData) => {
+    store.dispatch('setUserData', userData);
+    store.dispatch('setNoteableData', noteableData);
+    store.dispatch('setNotesData', notesDataMock);
+  };
+
+  const mountComponent = (noteableType = 'issue') => {
+    wrapper = mount(CommentForm, {
+      propsData: {
+        noteableType,
+      },
+      store,
+    });
+  };
+
+  const findCloseBtn = () => wrapper.find('.btn-comment-and-close');
+
+  beforeEach(() => {
+    axiosMock = new MockAdapter(axios);
+    store = createStore();
+
+    // This is necessary as we query Close issue button at the top of issue page when clicking bottom button
+    setFixtures(
+      '<div class="detail-page-header-actions"><button class="btn-close btn-grouped"></button></div>',
+    );
+  });
+
+  afterEach(() => {
+    axiosMock.restore();
+    wrapper.destroy();
+  });
+
+  describe('when issue is not blocked by other issues', () => {
+    beforeEach(() => {
+      setupStore(userDataMock, noteableDataMock);
+
+      mountComponent();
+    });
+
+    afterEach(() => {
+      wrapper.destroy();
+    });
+
+    it('should close the issue when clicking close issue button', done => {
+      jest.spyOn(wrapper.vm, 'closeIssue').mockResolvedValue();
+      findCloseBtn().trigger('click');
+
+      wrapper.vm.$nextTick(() => {
+        expect(wrapper.vm.closeIssue).toHaveBeenCalled();
+
+        done();
+      });
+    });
+  });
+
+  describe('when issue is blocked by other issues', () => {
+    let noteableDataMockBlocked;
+    beforeEach(() => {
+      noteableDataMockBlocked = Object.assign(noteableDataMock, {
+        blocked_by_issues: [
+          {
+            iid: 1,
+            web_url: 'path/to/issue',
+          },
+        ],
+      });
+      setupStore(userDataMock, noteableDataMockBlocked);
+
+      mountComponent();
+    });
+
+    afterEach(() => {
+      wrapper.destroy();
+    });
+
+    it('should display alert warning when attempting to close issue, close button is hidden', done => {
+      findCloseBtn().trigger('click');
+
+      wrapper.vm.$nextTick(() => {
+        const warning = wrapper.find('.gl-alert-warning');
+        expect(warning.exists()).toBe(true);
+        expect(warning.text()).toContain('Are you sure you want to close this blocked issue?');
+
+        const linkToBlockingIssue = warning.find('.gl-link');
+        expect(linkToBlockingIssue.text()).toContain(
+          noteableDataMockBlocked.blocked_by_issues[0].iid,
+        );
+        done();
+      });
+    });
+
+    it('should close the issue when clicking close issue button in alert', done => {
+      jest.spyOn(wrapper.vm, 'closeIssue').mockResolvedValue();
+      findCloseBtn().trigger('click');
+
+      wrapper.vm.$nextTick(() => {
+        expect(findCloseBtn().exists()).toBe(false);
+        const warning = wrapper.find('.gl-alert-warning');
+        const primaryButton = warning.find('.gl-alert-actions .gl-button');
+        expect(primaryButton.text()).toEqual('Yes, close issue');
+        primaryButton.trigger('click');
+        setTimeout(() => {
+          expect(wrapper.vm.closeIssue).toHaveBeenCalled();
+
+          done();
+        }, 1000);
+        done();
+      });
+    });
+
+    it('should dismiss alert warning when clicking cancel button in alert', done => {
+      findCloseBtn().trigger('click');
+
+      wrapper.vm.$nextTick(() => {
+        const warning = wrapper.find('.gl-alert-warning');
+        const secondaryButton = warning.find('.gl-alert-actions .btn-secondary');
+        expect(secondaryButton.text()).toEqual('Cancel');
+        secondaryButton.trigger('click');
+        wrapper.vm.$nextTick(() => {
+          expect(warning.exists()).toBe(false);
+
+          done();
+        });
+      });
+    });
+  });
+});

ee/spec/serializers/ee/issue_entity_spec.rb

@@ -32,10 +32,10 @@ describe IssueEntity do
       expect(subject).to include(:blocked_by_issues)
     end
 
-    it 'exposes only id and web_path' do
+    it 'exposes only iid and web_url' do
       response = described_class.new(blocked_issue, request: request, with_blocking_issues: true).as_json
 
-      expect(response[:blocked_by_issues].first.keys).to match_array([:id, :web_url])
+      expect(response[:blocked_by_issues].first.keys).to match_array([:iid, :web_url])
     end
   end
 end

locale/gitlab.pot

@@ -2423,6 +2423,9 @@ msgstr ""
 msgid "Are you sure you want to cancel editing this comment?"
 msgstr ""
 
+msgid "Are you sure you want to close this blocked issue?"
+msgstr ""
+
 msgid "Are you sure you want to delete %{name}?"
 msgstr ""
 
@@ -21074,6 +21077,9 @@ msgstr ""
 msgid "This issue is confidential"
 msgstr ""
 
+msgid "This issue is currently blocked by the following issues: %{issues}."
+msgstr ""
+
 msgid "This issue is locked."
 msgstr ""
 
@@ -23640,6 +23646,9 @@ msgstr ""
 msgid "Yes, add it"
 msgstr ""
 
+msgid "Yes, close issue"
+msgstr ""
+
 msgid "Yes, let me map Google Code users to full names or GitLab users."
 msgstr ""
 

spec/frontend/notes/mock_data.js

@@ -57,6 +57,7 @@ export const noteableDataMock = {
   updated_by_id: 1,
   web_url: '/gitlab-org/gitlab-foss/issues/26',
   noteableType: 'issue',
+  blocked_by_issues: [],
 };
 
 export const lastFetchedAt = '1501862675';

spec/frontend/notes/stores/actions_spec.js

@@ -34,6 +34,11 @@ describe('Actions Notes Store', () => {
     dispatch = jest.fn();
     state = {};
     axiosMock = new AxiosMockAdapter(axios);
+
+    // This is necessary as we query Close issue button at the top of issue page when clicking bottom button
+    setFixtures(
+      '<div class="detail-page-header-actions"><button class="btn-close btn-grouped"></button></div>',
+    );
   });
 
   afterEach(() => {
@@ -242,6 +247,30 @@ describe('Actions Notes Store', () => {
     });
   });
 
+  describe('toggleBlockedIssueWarning', () => {
+    it('should set issue warning as true', done => {
+      testAction(
+        actions.toggleBlockedIssueWarning,
+        true,
+        {},
+        [{ type: 'TOGGLE_BLOCKED_ISSUE_WARNING', payload: true }],
+        [],
+        done,
+      );
+    });
+
+    it('should set issue warning as false', done => {
+      testAction(
+        actions.toggleBlockedIssueWarning,
+        false,
+        {},
+        [{ type: 'TOGGLE_BLOCKED_ISSUE_WARNING', payload: false }],
+        [],
+        done,
+      );
+    });
+  });
+
   describe('poll', () => {
     jest.useFakeTimers();
 

spec/frontend/notes/stores/mutation_spec.js

@@ -664,4 +664,40 @@ describe('Notes Store mutations', () => {
       expect(state.discussionSortOrder).toBe(DESC);
     });
   });
+
+  describe('TOGGLE_BLOCKED_ISSUE_WARNING', () => {
+    it('should set isToggleBlockedIssueWarning as true', () => {
+      const state = {
+        discussions: [],
+        targetNoteHash: null,
+        lastFetchedAt: null,
+        isToggleStateButtonLoading: false,
+        isToggleBlockedIssueWarning: false,
+        notesData: {},
+        userData: {},
+        noteableData: {},
+      };
+
+      mutations.TOGGLE_BLOCKED_ISSUE_WARNING(state, true);
+
+      expect(state.isToggleBlockedIssueWarning).toEqual(true);
+    });
+
+    it('should set isToggleBlockedIssueWarning as false', () => {
+      const state = {
+        discussions: [],
+        targetNoteHash: null,
+        lastFetchedAt: null,
+        isToggleStateButtonLoading: false,
+        isToggleBlockedIssueWarning: true,
+        notesData: {},
+        userData: {},
+        noteableData: {},
+      };
+
+      mutations.TOGGLE_BLOCKED_ISSUE_WARNING(state, false);
+
+      expect(state.isToggleBlockedIssueWarning).toEqual(false);
+    });
+  });
 });