Merge branch 'diff-whitespace-setting-changes' into 'master'

Fixes issues with show whitespace button in diffs

Closes #42597 and #58852

See merge request gitlab-org/gitlab-ce!28242

4 files changed, 50 insertions, 6 deletions

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 0ed4dcdcd81..11d6672cacf 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -157,10 +157,12 @@ export default {
     this.adjustView();
     eventHub.$once('fetchedNotesData', this.setDiscussions);
     eventHub.$once('fetchDiffData', this.fetchData);
+    eventHub.$on('refetchDiffData', this.refetchDiffData);
     this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES;
   },
   beforeDestroy() {
     eventHub.$off('fetchDiffData', this.fetchData);
+    eventHub.$off('refetchDiffData', this.refetchDiffData);
     this.removeEventListeners();
   },
   methods: {
@@ -175,10 +177,16 @@ export default {
       'scrollToFile',
       'toggleShowTreeList',
     ]),
-    fetchData() {
+    refetchDiffData() {
+      this.assignedDiscussions = false;
+      this.fetchData(false);
+    },
+    fetchData(toggleTree = true) {
       this.fetchDiffFiles()
         .then(() => {
-          this.hideTreeListIfJustOneFile();
+          if (toggleTree) {
+            this.hideTreeListIfJustOneFile();
+          }
 
           requestIdleCallback(
             () => {
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 386d08aed2b..35297b7c264 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -52,7 +52,7 @@ export const fetchDiffFiles = ({ state, commit }) => {
   });
 
   return axios
-    .get(state.endpoint, { params: { w: state.showWhitespace ? null : '1' } })
+    .get(mergeUrlParams({ w: state.showWhitespace ? '0' : '1' }, state.endpoint))
     .then(res => {
       commit(types.SET_LOADING, false);
       commit(types.SET_MERGE_REQUEST_DIFFS, res.data.merge_request_diffs || []);
@@ -125,7 +125,8 @@ export const startRenderDiffsQueue = ({ state, commit }) => {
     new Promise(resolve => {
       const nextFile = state.diffFiles.find(
         file =>
-          !file.renderIt && (!file.viewer.collapsed || !file.viewer.name === diffViewerModes.text),
+          !file.renderIt &&
+          (file.viewer && (!file.viewer.collapsed || !file.viewer.name === diffViewerModes.text)),
       );
 
       if (nextFile) {
@@ -315,8 +316,10 @@ export const setShowWhitespace = ({ commit }, { showWhitespace, pushState = fals
   localStorage.setItem(WHITESPACE_STORAGE_KEY, showWhitespace);
 
   if (pushState) {
-    historyPushState(showWhitespace ? '?w=0' : '?w=1');
+    historyPushState(mergeUrlParams({ w: showWhitespace ? '0' : '1' }, window.location.href));
   }
+
+  eventHub.$emit('refetchDiffData');
 };
 
 export const toggleFileFinder = ({ commit }, visible) => {
diff --git a/changelogs/unreleased/diff-whitespace-setting-changes.yml b/changelogs/unreleased/diff-whitespace-setting-changes.yml
new file mode 100644
index 00000000000..640e9e589df
--- /dev/null
+++ b/changelogs/unreleased/diff-whitespace-setting-changes.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed show whitespace button not refetching diff content
+merge_request:
+author:
+type: fixed
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index c82dcadd2f1..6309a8823d7 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -82,7 +82,7 @@ describe('DiffsStoreActions', () => {
 
   describe('fetchDiffFiles', () => {
     it('should fetch diff files', done => {
-      const endpoint = '/fetch/diff/files';
+      const endpoint = '/fetch/diff/files?w=1';
       const mock = new MockAdapter(axios);
       const res = { diff_files: 1, merge_request_diffs: [] };
       mock.onGet(endpoint).reply(200, res);
@@ -828,6 +828,10 @@ describe('DiffsStoreActions', () => {
   });
 
   describe('setShowWhitespace', () => {
+    beforeEach(() => {
+      spyOn(eventHub, '$emit').and.stub();
+    });
+
     it('commits SET_SHOW_WHITESPACE', done => {
       testAction(
         setShowWhitespace,
@@ -855,6 +859,30 @@ describe('DiffsStoreActions', () => {
 
       expect(window.history.pushState).toHaveBeenCalled();
     });
+
+    it('calls history pushState with merged params', () => {
+      const originalPushState = window.history;
+
+      originalPushState.pushState({}, '', '?test=1');
+
+      spyOn(localStorage, 'setItem').and.stub();
+      spyOn(window.history, 'pushState').and.stub();
+
+      setShowWhitespace({ commit() {} }, { showWhitespace: true, pushState: true });
+
+      expect(window.history.pushState.calls.mostRecent().args[2]).toMatch(/(.*)\?test=1&w=0/);
+
+      originalPushState.pushState({}, '', '?');
+    });
+
+    it('emits eventHub event', () => {
+      spyOn(localStorage, 'setItem').and.stub();
+      spyOn(window.history, 'pushState').and.stub();
+
+      setShowWhitespace({ commit() {} }, { showWhitespace: true, pushState: true });
+
+      expect(eventHub.$emit).toHaveBeenCalledWith('refetchDiffData');
+    });
   });
 
   describe('setRenderIt', () => {