Commit 40eb7f72 authored by Phil Hughes's avatar Phil Hughes

Fixes issues with show whitespace button in diffs

Correctly updates the URL without overwriting parameters
Reloads the diff file content without reloading the page

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/58852, https://gitlab.com/gitlab-org/gitlab-ce/issues/42597
parent 712dccd4
...@@ -157,10 +157,12 @@ export default { ...@@ -157,10 +157,12 @@ export default {
this.adjustView(); this.adjustView();
eventHub.$once('fetchedNotesData', this.setDiscussions); eventHub.$once('fetchedNotesData', this.setDiscussions);
eventHub.$once('fetchDiffData', this.fetchData); eventHub.$once('fetchDiffData', this.fetchData);
eventHub.$on('refetchDiffData', this.refetchDiffData);
this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES; this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES;
}, },
beforeDestroy() { beforeDestroy() {
eventHub.$off('fetchDiffData', this.fetchData); eventHub.$off('fetchDiffData', this.fetchData);
eventHub.$off('refetchDiffData', this.refetchDiffData);
this.removeEventListeners(); this.removeEventListeners();
}, },
methods: { methods: {
...@@ -175,10 +177,16 @@ export default { ...@@ -175,10 +177,16 @@ export default {
'scrollToFile', 'scrollToFile',
'toggleShowTreeList', 'toggleShowTreeList',
]), ]),
fetchData() { refetchDiffData() {
this.assignedDiscussions = false;
this.fetchData(false);
},
fetchData(toggleTree = true) {
this.fetchDiffFiles() this.fetchDiffFiles()
.then(() => { .then(() => {
this.hideTreeListIfJustOneFile(); if (toggleTree) {
this.hideTreeListIfJustOneFile();
}
requestIdleCallback( requestIdleCallback(
() => { () => {
......
...@@ -52,7 +52,7 @@ export const fetchDiffFiles = ({ state, commit }) => { ...@@ -52,7 +52,7 @@ export const fetchDiffFiles = ({ state, commit }) => {
}); });
return axios return axios
.get(state.endpoint, { params: { w: state.showWhitespace ? null : '1' } }) .get(mergeUrlParams({ w: state.showWhitespace ? '0' : '1' }, state.endpoint))
.then(res => { .then(res => {
commit(types.SET_LOADING, false); commit(types.SET_LOADING, false);
commit(types.SET_MERGE_REQUEST_DIFFS, res.data.merge_request_diffs || []); commit(types.SET_MERGE_REQUEST_DIFFS, res.data.merge_request_diffs || []);
...@@ -125,7 +125,8 @@ export const startRenderDiffsQueue = ({ state, commit }) => { ...@@ -125,7 +125,8 @@ export const startRenderDiffsQueue = ({ state, commit }) => {
new Promise(resolve => { new Promise(resolve => {
const nextFile = state.diffFiles.find( const nextFile = state.diffFiles.find(
file => 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) { if (nextFile) {
...@@ -315,8 +316,10 @@ export const setShowWhitespace = ({ commit }, { showWhitespace, pushState = fals ...@@ -315,8 +316,10 @@ export const setShowWhitespace = ({ commit }, { showWhitespace, pushState = fals
localStorage.setItem(WHITESPACE_STORAGE_KEY, showWhitespace); localStorage.setItem(WHITESPACE_STORAGE_KEY, showWhitespace);
if (pushState) { 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) => { export const toggleFileFinder = ({ commit }, visible) => {
......
---
title: Fixed show whitespace button not refetching diff content
merge_request:
author:
type: fixed
...@@ -82,7 +82,7 @@ describe('DiffsStoreActions', () => { ...@@ -82,7 +82,7 @@ describe('DiffsStoreActions', () => {
describe('fetchDiffFiles', () => { describe('fetchDiffFiles', () => {
it('should fetch diff files', done => { it('should fetch diff files', done => {
const endpoint = '/fetch/diff/files'; const endpoint = '/fetch/diff/files?w=1';
const mock = new MockAdapter(axios); const mock = new MockAdapter(axios);
const res = { diff_files: 1, merge_request_diffs: [] }; const res = { diff_files: 1, merge_request_diffs: [] };
mock.onGet(endpoint).reply(200, res); mock.onGet(endpoint).reply(200, res);
...@@ -828,6 +828,10 @@ describe('DiffsStoreActions', () => { ...@@ -828,6 +828,10 @@ describe('DiffsStoreActions', () => {
}); });
describe('setShowWhitespace', () => { describe('setShowWhitespace', () => {
beforeEach(() => {
spyOn(eventHub, '$emit').and.stub();
});
it('commits SET_SHOW_WHITESPACE', done => { it('commits SET_SHOW_WHITESPACE', done => {
testAction( testAction(
setShowWhitespace, setShowWhitespace,
...@@ -855,6 +859,30 @@ describe('DiffsStoreActions', () => { ...@@ -855,6 +859,30 @@ describe('DiffsStoreActions', () => {
expect(window.history.pushState).toHaveBeenCalled(); 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', () => { describe('setRenderIt', () => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment