Merge branch 'defect/collapsed-diff-files/expand-all-banner' into 'master'

Use an event hub to coordinate all possible ways to expand all diff files

See merge request gitlab-org/gitlab!46379

app/assets/javascripts/diffs/components/collapsed_files_warning.vue

 <script>
-import { mapActions } from 'vuex';
-
 import { GlAlert, GlButton } from '@gitlab/ui';
 
 import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants';
+import eventHub from '../event_hub';
 
 export default {
   components: {
@@ -36,13 +35,12 @@ export default {
   },
 
   methods: {
-    ...mapActions('diffs', ['expandAllFiles']),
     dismiss() {
       this.isDismissed = true;
       this.$emit('dismiss');
     },
     expand() {
-      this.expandAllFiles();
+      eventHub.$emit('mr:diffs:expandAllFiles');
       this.dismiss();
     },
   },

app/assets/javascripts/diffs/components/compare_versions.vue

@@ -7,6 +7,7 @@ import CompareDropdownLayout from './compare_dropdown_layout.vue';
 import SettingsDropdown from './settings_dropdown.vue';
 import DiffStats from './diff_stats.vue';
 import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants';
+import eventHub from '../event_hub';
 
 export default {
   components: {
@@ -67,9 +68,11 @@ export default {
     ...mapActions('diffs', [
       'setInlineDiffViewType',
       'setParallelDiffViewType',
-      'expandAllFiles',
       'toggleShowTreeList',
     ]),
+    expandAllFiles() {
+      eventHub.$emit('mr:diffs:expandAllFiles');
+    },
   },
 };
 </script>

app/assets/javascripts/diffs/components/diff_file.vue

@@ -6,13 +6,14 @@ import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { sprintf } from '~/locale';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
 import { hasDiff } from '~/helpers/diffs_helper';
-import eventHub from '../../notes/event_hub';
+import notesEventHub from '../../notes/event_hub';
 import DiffFileHeader from './diff_file_header.vue';
 import DiffContent from './diff_content.vue';
 import { diffViewerErrors } from '~/ide/constants';
 import { collapsedType, isCollapsed } from '../diff_file';
 import { DIFF_FILE_AUTOMATIC_COLLAPSE, DIFF_FILE_MANUAL_COLLAPSE } from '../constants';
 import { DIFF_FILE, GENERIC_ERROR } from '../i18n';
+import eventHub from '../event_hub';
 
 export default {
   components: {
@@ -151,7 +152,11 @@ export default {
     },
   },
   created() {
-    eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiff);
+    notesEventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiff);
+    eventHub.$on('mr:diffs:expandAllFiles', this.expandAllListener);
+  },
+  beforeDestroy() {
+    eventHub.$off('mr:diffs:expandAllFiles', this.expandAllListener);
   },
   methods: {
     ...mapActions('diffs', [
@@ -160,6 +165,11 @@ export default {
       'setRenderIt',
       'setFileCollapsedByUser',
     ]),
+    expandAllListener() {
+      if (this.isCollapsed) {
+        this.handleToggle();
+      }
+    },
     handleToggle() {
       const currentCollapsedFlag = this.isCollapsed;
 

app/assets/javascripts/diffs/event_hub.js

+import eventHubFactory from '~/helpers/event_hub_factory';
+
+export default eventHubFactory();

app/assets/javascripts/diffs/store/actions.js

@@ -364,10 +364,6 @@ export const loadCollapsedDiff = ({ commit, getters, state }, file) =>
       });
     });
 
-export const expandAllFiles = ({ commit }) => {
-  commit(types.EXPAND_ALL_FILES);
-};
-
 /**
  * Toggles the file discussions after user clicked on the toggle discussions button.
  *

app/assets/javascripts/diffs/store/mutation_types.js

@@ -13,7 +13,6 @@ export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS';
 export const TOGGLE_LINE_HAS_FORM = 'TOGGLE_LINE_HAS_FORM';
 export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES';
 export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS';
-export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES';
 export const RENDER_FILE = 'RENDER_FILE';
 export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE';
 export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE';

app/assets/javascripts/diffs/store/mutations.js

@@ -176,17 +176,6 @@ export default {
     Object.assign(selectedFile, { ...newFileData });
   },
 
-  [types.EXPAND_ALL_FILES](state) {
-    state.diffFiles.forEach(file => {
-      Object.assign(file, {
-        viewer: Object.assign(file.viewer, {
-          automaticallyCollapsed: false,
-          manuallyCollapsed: false,
-        }),
-      });
-    });
-  },
-
   [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, hash }) {
     const { latestDiff } = state;
 

spec/frontend/diffs/components/collapsed_files_warning_spec.js

@@ -3,6 +3,7 @@ import { shallowMount, mount, createLocalVue } from '@vue/test-utils';
 import createStore from '~/diffs/store/modules';
 import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue';
 import { CENTERED_LIMITED_CONTAINER_CLASSES } from '~/diffs/constants';
+import eventHub from '~/diffs/event_hub';
 
 const propsData = {
   limited: true,
@@ -76,13 +77,13 @@ describe('CollapsedFilesWarning', () => {
     expect(wrapper.find('[data-testid="root"]').exists()).toBe(false);
   });
 
-  it('triggers the expandAllFiles action when the alert action button is clicked', () => {
+  it('emits the `mr:diffs:expandAllFiles` event when the alert action button is clicked', () => {
     createComponent({}, { full: true });
 
-    jest.spyOn(wrapper.vm.$store, 'dispatch').mockReturnValue(undefined);
+    jest.spyOn(eventHub, '$emit');
 
     getAlertActionButton().vm.$emit('click');
 
-    expect(wrapper.vm.$store.dispatch).toHaveBeenCalledWith('diffs/expandAllFiles', undefined);
+    expect(eventHub.$emit).toHaveBeenCalledWith('mr:diffs:expandAllFiles');
   });
 });

spec/frontend/diffs/components/diff_file_spec.js

@@ -9,6 +9,8 @@ import DiffFileComponent from '~/diffs/components/diff_file.vue';
 import DiffFileHeaderComponent from '~/diffs/components/diff_file_header.vue';
 import DiffContentComponent from '~/diffs/components/diff_content.vue';
 
+import eventHub from '~/diffs/event_hub';
+
 import { diffViewerModes, diffViewerErrors } from '~/ide/constants';
 
 function changeViewer(store, index, { automaticallyCollapsed, manuallyCollapsed, name }) {
@@ -138,6 +140,30 @@ describe('DiffFile', () => {
   });
 
   describe('collapsing', () => {
+    describe('`mr:diffs:expandAllFiles` event', () => {
+      beforeEach(() => {
+        jest.spyOn(wrapper.vm, 'handleToggle').mockImplementation(() => {});
+      });
+
+      it('performs the normal file toggle when the file is collapsed', async () => {
+        makeFileAutomaticallyCollapsed(store);
+
+        await wrapper.vm.$nextTick();
+
+        eventHub.$emit('mr:diffs:expandAllFiles');
+
+        expect(wrapper.vm.handleToggle).toHaveBeenCalledTimes(1);
+      });
+
+      it('does nothing when the file is not collapsed', async () => {
+        eventHub.$emit('mr:diffs:expandAllFiles');
+
+        await wrapper.vm.$nextTick();
+
+        expect(wrapper.vm.handleToggle).not.toHaveBeenCalled();
+      });
+    });
+
     describe('user collapsed', () => {
       beforeEach(() => {
         makeFileManuallyCollapsed(store);

spec/frontend/diffs/store/actions_spec.js

@@ -27,7 +27,6 @@ import {
   scrollToLineIfNeededInline,
   scrollToLineIfNeededParallel,
   loadCollapsedDiff,
-  expandAllFiles,
   toggleFileDiscussions,
   saveDiffDiscussion,
   setHighlightedRow,
@@ -658,23 +657,6 @@ describe('DiffsStoreActions', () => {
     });
   });
 
-  describe('expandAllFiles', () => {
-    it('should change the collapsed prop from the diffFiles', done => {
-      testAction(
-        expandAllFiles,
-        null,
-        {},
-        [
-          {
-            type: types.EXPAND_ALL_FILES,
-          },
-        ],
-        [],
-        done,
-      );
-    });
-  });
-
   describe('toggleFileDiscussions', () => {
     it('should dispatch collapseDiscussion when all discussions are expanded', () => {
       const getters = {

spec/frontend/diffs/store/mutations_spec.js

@@ -126,21 +126,6 @@ describe('DiffsStoreMutations', () => {
     });
   });
 
-  describe('EXPAND_ALL_FILES', () => {
-    it('should change the collapsed prop from diffFiles', () => {
-      const diffFile = {
-        viewer: {
-          automaticallyCollapsed: true,
-        },
-      };
-      const state = { expandAllFiles: true, diffFiles: [diffFile] };
-
-      mutations[types.EXPAND_ALL_FILES](state);
-
-      expect(state.diffFiles[0].viewer.automaticallyCollapsed).toEqual(false);
-    });
-  });
-
   describe('ADD_CONTEXT_LINES', () => {
     it('should call utils.addContextLines with proper params', () => {
       const options = {