Use database as SSOT for diffs whitespace

This MR removes most of the code for using cookies
and querystrings to set whether or not to show whitespace
diffs in merge requests.

Then saves the changes to the database directly from the
settings dropdown on the diffs page. This keeps the user preferences
in sync and makes them the single source of truth for displaying
whitespace.

Changelog: changed

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

@@ -36,7 +36,7 @@ export default {
       this.setFileByFile({ fileByFile: !this.viewDiffsFileByFile });
     },
     toggleWhitespace(updatedSetting) {
-      this.setShowWhitespace({ showWhitespace: updatedSetting, pushState: true });
+      this.setShowWhitespace({ showWhitespace: updatedSetting });
     },
   },
 };

app/assets/javascripts/diffs/constants.js

-// The backend actually uses "hide_whitespace" while the frontend
-// uses "show whitspace" so these values are opposite what you might expect
-export const NO_SHOW_WHITESPACE = '1';
-export const SHOW_WHITESPACE = '0';
 export const INLINE_DIFF_VIEW_TYPE = 'inline';
 export const PARALLEL_DIFF_VIEW_TYPE = 'parallel';
 export const MATCH_LINE_TYPE = 'match';

app/assets/javascripts/diffs/index.js

@@ -98,10 +98,23 @@ export default function initDiffsApp(store) {
 
       this.setRenderTreeList(renderTreeList);
 
-      // Set whitespace default as per user preferences unless cookie is already set
-      if (!Cookies.get(DIFF_WHITESPACE_COOKIE_NAME)) {
-        const hideWhitespace = this.showWhitespaceDefault ? '0' : '1';
-        this.setShowWhitespace({ showWhitespace: hideWhitespace !== '1' });
+      // NOTE: A "true" or "checked" value for `showWhitespace` is '0' not '1'.
+      // Check for cookie and save that setting for future use.
+      // Then delete the cookie as we are phasing it out and using the database as SSOT.
+      // NOTE: This can/should be removed later
+      if (Cookies.get(DIFF_WHITESPACE_COOKIE_NAME)) {
+        const hideWhitespace = Cookies.get(DIFF_WHITESPACE_COOKIE_NAME);
+        this.setShowWhitespace({
+          url: this.endpointUpdateUser,
+          showWhitespace: hideWhitespace !== '1',
+        });
+        Cookies.remove(DIFF_WHITESPACE_COOKIE_NAME);
+      } else {
+        // This is only to set the the user preference in Vuex for use later
+        this.setShowWhitespace({
+          showWhitespace: this.showWhitespaceDefault,
+          updateDatabase: false,
+        });
       }
     },
     methods: {

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

@@ -26,9 +26,6 @@ import {
   START_RENDERING_INDEX,
   INLINE_DIFF_LINES_KEY,
   DIFFS_PER_PAGE,
-  DIFF_WHITESPACE_COOKIE_NAME,
-  SHOW_WHITESPACE,
-  NO_SHOW_WHITESPACE,
   DIFF_FILE_MANUAL_COLLAPSE,
   DIFF_FILE_AUTOMATIC_COLLAPSE,
   EVT_PERF_MARK_FILE_TREE_START,
@@ -569,16 +566,15 @@ export const setRenderTreeList = ({ commit }, renderTreeList) => {
   }
 };
 
-export const setShowWhitespace = ({ commit }, { showWhitespace, pushState = false }) => {
-  commit(types.SET_SHOW_WHITESPACE, showWhitespace);
-  const w = showWhitespace ? SHOW_WHITESPACE : NO_SHOW_WHITESPACE;
-
-  Cookies.set(DIFF_WHITESPACE_COOKIE_NAME, w);
-
-  if (pushState) {
-    historyPushState(mergeUrlParams({ w }, window.location.href));
+export const setShowWhitespace = async (
+  { state, commit },
+  { url, showWhitespace, updateDatabase = true },
+) => {
+  if (updateDatabase) {
+    await axios.put(url || state.endpointUpdateUser, { show_whitespace_in_diffs: showWhitespace });
   }
 
+  commit(types.SET_SHOW_WHITESPACE, showWhitespace);
   notesEventHub.$emit('refetchDiffData');
 
   if (window.gon?.features?.diffSettingsUsageData) {

app/assets/javascripts/diffs/store/modules/diff_state.js

 import Cookies from 'js-cookie';
 import { getParameterValues } from '~/lib/utils/url_utility';
-import {
-  INLINE_DIFF_VIEW_TYPE,
-  DIFF_VIEW_COOKIE_NAME,
-  DIFF_WHITESPACE_COOKIE_NAME,
-} from '../../constants';
+import { INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME } from '../../constants';
 
 import { fileByFile } from '../../utils/preferences';
-import { getDefaultWhitespace } from '../utils';
 
 const getViewTypeFromQueryString = () => getParameterValues('view')[0];
 
 const viewTypeFromCookie = Cookies.get(DIFF_VIEW_COOKIE_NAME);
 const defaultViewType = INLINE_DIFF_VIEW_TYPE;
-const whiteSpaceFromQueryString = getParameterValues('w')[0];
-const whiteSpaceFromCookie = Cookies.get(DIFF_WHITESPACE_COOKIE_NAME);
 
 export default () => ({
   isLoading: true,
@@ -42,7 +35,7 @@ export default () => ({
   commentForms: [],
   highlightedRow: null,
   renderTreeList: true,
-  showWhitespace: getDefaultWhitespace(whiteSpaceFromQueryString, whiteSpaceFromCookie),
+  showWhitespace: true,
   viewDiffsFileByFile: fileByFile(),
   fileFinderVisible: false,
   dismissEndpoint: '',

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

@@ -11,8 +11,6 @@ import {
   MATCH_LINE_TYPE,
   LINES_TO_BE_RENDERED_DIRECTLY,
   INLINE_DIFF_LINES_KEY,
-  SHOW_WHITESPACE,
-  NO_SHOW_WHITESPACE,
   CONFLICT_OUR,
   CONFLICT_THEIR,
   CONFLICT_MARKER,
@@ -559,10 +557,3 @@ export const allDiscussionWrappersExpanded = (diff) => {
 
   return discussionsExpanded;
 };
-
-export const getDefaultWhitespace = (queryString, cookie) => {
-  // Querystring should override stored cookie value
-  if (queryString) return queryString === SHOW_WHITESPACE;
-  if (cookie === NO_SHOW_WHITESPACE) return false;
-  return true;
-};

doc/user/project/merge_requests/changes.md

@@ -91,10 +91,6 @@ specific commit page.
 
 ![MR diff](img/merge_request_diff.png)
 
-NOTE:
-You can append `?w=1` while on the diffs page of a merge request to ignore any
-whitespace changes.
-
 ## Mark files as viewed
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51513) in GitLab 13.9.

spec/frontend/diffs/components/settings_dropdown_spec.js

@@ -142,7 +142,6 @@ describe('Diff settings dropdown component', () => {
 
       expect(store.dispatch).toHaveBeenCalledWith('diffs/setShowWhitespace', {
         showWhitespace: !checked,
-        pushState: true,
       });
     });
   });

spec/frontend/diffs/store/actions_spec.js

@@ -9,8 +9,6 @@ import {
   INLINE_DIFF_VIEW_TYPE,
   PARALLEL_DIFF_VIEW_TYPE,
   DIFFS_PER_PAGE,
-  DIFF_WHITESPACE_COOKIE_NAME,
-  SHOW_WHITESPACE,
 } from '~/diffs/constants';
 import {
   setBaseConfig,
@@ -1019,14 +1017,26 @@ describe('DiffsStoreActions', () => {
   });
 
   describe('setShowWhitespace', () => {
+    const endpointUpdateUser = 'user/prefs';
+    let putSpy;
+    let mock;
+
     beforeEach(() => {
+      mock = new MockAdapter(axios);
+      putSpy = jest.spyOn(axios, 'put');
+
+      mock.onPut(endpointUpdateUser).reply(200, {});
       jest.spyOn(eventHub, '$emit').mockImplementation();
     });
 
+    afterEach(() => {
+      mock.restore();
+    });
+
     it('commits SET_SHOW_WHITESPACE', (done) => {
       testAction(
         setShowWhitespace,
-        { showWhitespace: true },
+        { showWhitespace: true, updateDatabase: false },
         {},
         [{ type: types.SET_SHOW_WHITESPACE, payload: true }],
         [],
@@ -1034,32 +1044,20 @@ describe('DiffsStoreActions', () => {
       );
     });
 
-    it('sets cookie', () => {
-      setShowWhitespace({ commit() {} }, { showWhitespace: true });
-
-      expect(Cookies.get(DIFF_WHITESPACE_COOKIE_NAME)).toEqual(SHOW_WHITESPACE);
-    });
-
-    it('calls history pushState', () => {
-      setShowWhitespace({ commit() {} }, { showWhitespace: true, pushState: true });
-
-      expect(window.history.pushState).toHaveBeenCalled();
-    });
-
-    it('calls history pushState with merged params', () => {
-      window.history.pushState({}, '', '?test=1');
-
-      setShowWhitespace({ commit() {} }, { showWhitespace: true, pushState: true });
-
-      expect(
-        window.history.pushState.mock.calls[window.history.pushState.mock.calls.length - 1][2],
-      ).toMatch(/(.*)\?test=1&w=0/);
+    it('saves to the database', async () => {
+      await setShowWhitespace(
+        { state: { endpointUpdateUser }, commit() {} },
+        { showWhitespace: true, updateDatabase: true },
+      );
 
-      window.history.pushState({}, '', '?');
+      expect(putSpy).toHaveBeenCalledWith(endpointUpdateUser, { show_whitespace_in_diffs: true });
     });
 
-    it('emits eventHub event', () => {
-      setShowWhitespace({ commit() {} }, { showWhitespace: true, pushState: true });
+    it('emits eventHub event', async () => {
+      await setShowWhitespace(
+        { state: {}, commit() {} },
+        { showWhitespace: true, updateDatabase: false },
+      );
 
       expect(eventHub.$emit).toHaveBeenCalledWith('refetchDiffData');
     });

spec/frontend/diffs/store/utils_spec.js

@@ -752,28 +752,6 @@ describe('DiffsStoreUtils', () => {
     });
   });
 
-  describe('getDefaultWhitespace', () => {
-    it('defaults to true if querystring and cookie are undefined', () => {
-      expect(utils.getDefaultWhitespace()).toBe(true);
-    });
-
-    it('returns false if querystring is `1`', () => {
-      expect(utils.getDefaultWhitespace('1', '0')).toBe(false);
-    });
-
-    it('returns true if querystring is `0`', () => {
-      expect(utils.getDefaultWhitespace('0', undefined)).toBe(true);
-    });
-
-    it('returns false if cookie is `1`', () => {
-      expect(utils.getDefaultWhitespace(undefined, '1')).toBe(false);
-    });
-
-    it('returns true if cookie is `0`', () => {
-      expect(utils.getDefaultWhitespace(undefined, '0')).toBe(true);
-    });
-  });
-
   describe('isAdded', () => {
     it.each`
       type               | expected