Merge branch 'feat/add-toggle-all-discussions-button' into 'master'

Add toggle all discussions button to MRs

Closes #15328

See merge request gitlab-org/gitlab!24670

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

 <script>
-import { mapGetters } from 'vuex';
+import { mapGetters, mapActions } from 'vuex';
 import { GlTooltipDirective } from '@gitlab/ui';
 import Icon from '~/vue_shared/components/icon.vue';
 import discussionNavigation from '../mixins/discussion_navigation';
@@ -18,13 +18,11 @@ export default {
       'getNoteableData',
       'resolvableDiscussionsCount',
       'unresolvedDiscussionsCount',
+      'discussions',
     ]),
     isLoggedIn() {
       return this.getUserData.id;
     },
-    hasNextButton() {
-      return this.isLoggedIn && !this.allResolved;
-    },
     allResolved() {
       return this.unresolvedDiscussionsCount === 0;
     },
@@ -34,6 +32,21 @@ export default {
     resolvedDiscussionsCount() {
       return this.resolvableDiscussionsCount - this.unresolvedDiscussionsCount;
     },
+    toggeableDiscussions() {
+      return this.discussions.filter(discussion => !discussion.individual_note);
+    },
+    allExpanded() {
+      return this.toggeableDiscussions.every(discussion => discussion.expanded);
+    },
+  },
+  methods: {
+    ...mapActions(['setExpandDiscussions']),
+    handleExpandDiscussions() {
+      this.setExpandDiscussions({
+        discussionIds: this.toggeableDiscussions.map(discussion => discussion.id),
+        expanded: !this.allExpanded,
+      });
+    },
   },
 };
 </script>
@@ -44,8 +57,8 @@ export default {
     ref="discussionCounter"
     class="line-resolve-all-container full-width-mobile"
   >
-    <div class="full-width-mobile d-flex d-sm-block">
-      <div :class="{ 'has-next-btn': hasNextButton }" class="line-resolve-all">
+    <div class="full-width-mobile d-flex d-sm-flex">
+      <div class="line-resolve-all">
         <span
           :class="{ 'is-active': allResolved }"
           class="line-resolve-btn is-disabled"
@@ -75,7 +88,7 @@ export default {
       <div v-if="isLoggedIn && !allResolved" class="btn-group btn-group-sm" role="group">
         <button
           v-gl-tooltip
-          title="Jump to next unresolved thread"
+          :title="__('Jump to next unresolved thread')"
           class="btn btn-default discussion-next-btn"
           data-track-event="click_button"
           data-track-label="mr_next_unresolved_thread"
@@ -85,6 +98,16 @@ export default {
           <icon name="comment-next" />
         </button>
       </div>
+      <div v-if="isLoggedIn" class="btn-group btn-group-sm" role="group">
+        <button
+          v-gl-tooltip
+          :title="__('Toggle all threads')"
+          class="btn btn-default toggle-all-discussions-btn"
+          @click="handleExpandDiscussions"
+        >
+          <icon :name="allExpanded ? 'angle-up' : 'angle-down'" />
+        </button>
+      </div>
     </div>
   </div>
 </template>

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

@@ -46,6 +46,10 @@ export const setNotesFetchedState = ({ commit }, state) =>
 
 export const toggleDiscussion = ({ commit }, data) => commit(types.TOGGLE_DISCUSSION, data);
 
+export const setExpandDiscussions = ({ commit }, { discussionIds, expanded }) => {
+  commit(types.SET_EXPAND_DISCUSSIONS, { discussionIds, expanded });
+};
+
 export const fetchDiscussions = ({ commit, dispatch }, { path, filter, persistFilter }) => {
   const config =
     filter !== undefined
@@ -54,6 +58,7 @@ export const fetchDiscussions = ({ commit, dispatch }, { path, filter, persistFi
 
   return axios.get(path, config).then(({ data }) => {
     commit(types.SET_INITIAL_DISCUSSIONS, data);
+
     dispatch('updateResolvableDiscussionsCounts');
   });
 };

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

@@ -24,6 +24,7 @@ export const REMOVE_CONVERTED_DISCUSSION = 'REMOVE_CONVERTED_DISCUSSION';
 export const COLLAPSE_DISCUSSION = 'COLLAPSE_DISCUSSION';
 export const EXPAND_DISCUSSION = 'EXPAND_DISCUSSION';
 export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION';
+export const SET_EXPAND_DISCUSSIONS = 'SET_EXPAND_DISCUSSIONS';
 export const UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS = 'UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS';
 export const SET_CURRENT_DISCUSSION_ID = 'SET_CURRENT_DISCUSSION_ID';
 

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

@@ -190,6 +190,15 @@ export default {
     });
   },
 
+  [types.SET_EXPAND_DISCUSSIONS](state, { discussionIds, expanded }) {
+    if (discussionIds?.length) {
+      discussionIds.forEach(discussionId => {
+        const discussion = utils.findNoteObjectById(state.discussions, discussionId);
+        Object.assign(discussion, { expanded });
+      });
+    }
+  },
+
   [types.UPDATE_NOTE](state, note) {
     const noteObj = utils.findNoteObjectById(state.discussions, note.discussion_id);
 

app/assets/stylesheets/pages/notes.scss

@@ -842,11 +842,11 @@ $note-form-margin-left: 72px;
     white-space: nowrap;
   }
 
-  .btn-group {
-    margin-left: -4px;
+  .discussion-next-btn {
+    border-radius: 0;
   }
 
-  .discussion-next-btn {
+  .toggle-all-discussions-btn {
     border-top-left-radius: 0;
     border-bottom-left-radius: 0;
   }
@@ -859,7 +859,6 @@ $note-form-margin-left: 72px;
     }
 
     &.discussion-create-issue-btn {
-      margin-left: -4px;
       border-radius: 0;
       border-right: 0;
 
@@ -873,6 +872,10 @@ $note-form-margin-left: 72px;
         }
       }
     }
+
+    &.discussion-next-btn {
+      border-right: 0;
+    }
   }
 }
 
@@ -884,12 +887,9 @@ $note-form-margin-left: 72px;
   border: 1px solid $border-color;
   border-radius: $border-radius-default;
   font-size: $gl-btn-small-font-size;
-
-  &.has-next-btn {
-    border-top-right-radius: 0;
-    border-bottom-right-radius: 0;
-    border-right: 0;
-  }
+  border-top-right-radius: 0;
+  border-bottom-right-radius: 0;
+  border-right: 0;
 
   .line-resolve-btn {
     margin-right: 5px;

changelogs/unreleased/feat-add-toggle-all-discussions-button.yml

+---
+title: Add toggle all discussions button to MRs
+merge_request: 24670
+author: Martin Hobert & Diego Louzán
+type: added

locale/gitlab.pot

@@ -21009,6 +21009,9 @@ msgstr ""
 msgid "Toggle Sidebar"
 msgstr ""
 
+msgid "Toggle all threads"
+msgstr ""
+
 msgid "Toggle backtrace"
 msgstr ""
 

spec/frontend/notes/components/discussion_counter_spec.js

@@ -75,17 +75,66 @@ describe('DiscussionCounter component', () => {
     });
 
     it.each`
-      title              | resolved | hasNextBtn | isActive | icon                     | groupLength
-      ${'hasNextButton'} | ${false} | ${true}    | ${false} | ${'check-circle'}        | ${2}
-      ${'allResolved'}   | ${true}  | ${false}   | ${true}  | ${'check-circle-filled'} | ${0}
-    `('renders correctly if $title', ({ resolved, hasNextBtn, isActive, icon, groupLength }) => {
+      title                | resolved | isActive | icon                     | groupLength
+      ${'not allResolved'} | ${false} | ${false} | ${'check-circle'}        | ${3}
+      ${'allResolved'}     | ${true}  | ${true}  | ${'check-circle-filled'} | ${1}
+    `('renders correctly if $title', ({ resolved, isActive, icon, groupLength }) => {
       updateStore({ resolvable: true, resolved });
       wrapper = shallowMount(DiscussionCounter, { store, localVue });
 
-      expect(wrapper.find(`.has-next-btn`).exists()).toBe(hasNextBtn);
       expect(wrapper.find(`.is-active`).exists()).toBe(isActive);
       expect(wrapper.find({ name: icon }).exists()).toBe(true);
       expect(wrapper.findAll('[role="group"').length).toBe(groupLength);
     });
   });
+
+  describe('toggle all threads button', () => {
+    let toggleAllButton;
+    const updateStoreWithExpanded = expanded => {
+      const discussion = { ...discussionMock, expanded };
+      store.commit(types.SET_INITIAL_DISCUSSIONS, [discussion]);
+      store.dispatch('updateResolvableDiscussionsCounts');
+      wrapper = shallowMount(DiscussionCounter, { store, localVue });
+      toggleAllButton = wrapper.find('.toggle-all-discussions-btn');
+    };
+
+    afterEach(() => wrapper.destroy());
+
+    it('calls button handler when clicked', () => {
+      updateStoreWithExpanded(true);
+
+      wrapper.setMethods({ handleExpandDiscussions: jest.fn() });
+      toggleAllButton.trigger('click');
+
+      expect(wrapper.vm.handleExpandDiscussions).toHaveBeenCalledTimes(1);
+    });
+
+    it('collapses all discussions if expanded', () => {
+      updateStoreWithExpanded(true);
+
+      expect(wrapper.vm.allExpanded).toBe(true);
+      expect(toggleAllButton.find({ name: 'angle-up' }).exists()).toBe(true);
+
+      toggleAllButton.trigger('click');
+
+      return wrapper.vm.$nextTick().then(() => {
+        expect(wrapper.vm.allExpanded).toBe(false);
+        expect(toggleAllButton.find({ name: 'angle-down' }).exists()).toBe(true);
+      });
+    });
+
+    it('expands all discussions if collapsed', () => {
+      updateStoreWithExpanded(false);
+
+      expect(wrapper.vm.allExpanded).toBe(false);
+      expect(toggleAllButton.find({ name: 'angle-down' }).exists()).toBe(true);
+
+      toggleAllButton.trigger('click');
+
+      return wrapper.vm.$nextTick().then(() => {
+        expect(wrapper.vm.allExpanded).toBe(true);
+        expect(toggleAllButton.find({ name: 'angle-up' }).exists()).toBe(true);
+      });
+    });
+  });
 });

spec/frontend/notes/stores/mutation_spec.js

@@ -329,6 +329,52 @@ describe('Notes Store mutations', () => {
     });
   });
 
+  describe('SET_EXPAND_DISCUSSIONS', () => {
+    it('should succeed when discussions are null', () => {
+      const state = {};
+
+      mutations.SET_EXPAND_DISCUSSIONS(state, { discussionIds: null, expanded: true });
+
+      expect(state).toEqual({});
+    });
+
+    it('should succeed when discussions are empty', () => {
+      const state = {};
+
+      mutations.SET_EXPAND_DISCUSSIONS(state, { discussionIds: [], expanded: true });
+
+      expect(state).toEqual({});
+    });
+
+    it('should open all closed discussions', () => {
+      const discussion1 = Object.assign({}, discussionMock, { id: 0, expanded: false });
+      const discussion2 = Object.assign({}, discussionMock, { id: 1, expanded: true });
+      const discussionIds = [discussion1.id, discussion2.id];
+
+      const state = { discussions: [discussion1, discussion2] };
+
+      mutations.SET_EXPAND_DISCUSSIONS(state, { discussionIds, expanded: true });
+
+      state.discussions.forEach(discussion => {
+        expect(discussion.expanded).toEqual(true);
+      });
+    });
+
+    it('should close all opened discussions', () => {
+      const discussion1 = Object.assign({}, discussionMock, { id: 0, expanded: false });
+      const discussion2 = Object.assign({}, discussionMock, { id: 1, expanded: true });
+      const discussionIds = [discussion1.id, discussion2.id];
+
+      const state = { discussions: [discussion1, discussion2] };
+
+      mutations.SET_EXPAND_DISCUSSIONS(state, { discussionIds, expanded: false });
+
+      state.discussions.forEach(discussion => {
+        expect(discussion.expanded).toEqual(false);
+      });
+    });
+  });
+
   describe('UPDATE_NOTE', () => {
     it('should update a note', () => {
       const state = {