Commit 3a46ab2e authored by Kushal Pandya's avatar Kushal Pandya

Merge branch 'ph/fixDiffsMultipleUniqueUserCalls' into 'master'

Fixes a bug where multiple tracking calls would happen

See merge request gitlab-org/gitlab!66604
parents a4bfc6da 090af754
...@@ -60,14 +60,14 @@ export default { ...@@ -60,14 +60,14 @@ export default {
<gl-button <gl-button
:class="{ selected: !renderTreeList }" :class="{ selected: !renderTreeList }"
class="gl-w-half js-list-view" class="gl-w-half js-list-view"
@click="setRenderTreeList(false)" @click="setRenderTreeList({ renderTreeList: false })"
> >
{{ __('List view') }} {{ __('List view') }}
</gl-button> </gl-button>
<gl-button <gl-button
:class="{ selected: renderTreeList }" :class="{ selected: renderTreeList }"
class="gl-w-half js-tree-view" class="gl-w-half js-tree-view"
@click="setRenderTreeList(true)" @click="setRenderTreeList({ renderTreeList: true })"
> >
{{ __('Tree view') }} {{ __('Tree view') }}
</gl-button> </gl-button>
......
...@@ -93,7 +93,7 @@ export default function initDiffsApp(store) { ...@@ -93,7 +93,7 @@ export default function initDiffsApp(store) {
const treeListStored = localStorage.getItem(TREE_LIST_STORAGE_KEY); const treeListStored = localStorage.getItem(TREE_LIST_STORAGE_KEY);
const renderTreeList = treeListStored !== null ? parseBoolean(treeListStored) : true; const renderTreeList = treeListStored !== null ? parseBoolean(treeListStored) : true;
this.setRenderTreeList(renderTreeList); this.setRenderTreeList({ renderTreeList, trackClick: false });
// NOTE: A "true" or "checked" value for `showWhitespace` is '0' not '1'. // NOTE: A "true" or "checked" value for `showWhitespace` is '0' not '1'.
// Check for cookie and save that setting for future use. // Check for cookie and save that setting for future use.
...@@ -104,6 +104,7 @@ export default function initDiffsApp(store) { ...@@ -104,6 +104,7 @@ export default function initDiffsApp(store) {
this.setShowWhitespace({ this.setShowWhitespace({
url: this.endpointUpdateUser, url: this.endpointUpdateUser,
showWhitespace: hideWhitespace !== '1', showWhitespace: hideWhitespace !== '1',
trackClick: false,
}); });
Cookies.remove(DIFF_WHITESPACE_COOKIE_NAME); Cookies.remove(DIFF_WHITESPACE_COOKIE_NAME);
} else { } else {
...@@ -111,6 +112,7 @@ export default function initDiffsApp(store) { ...@@ -111,6 +112,7 @@ export default function initDiffsApp(store) {
this.setShowWhitespace({ this.setShowWhitespace({
showWhitespace: this.showWhitespaceDefault, showWhitespace: this.showWhitespaceDefault,
updateDatabase: false, updateDatabase: false,
trackClick: false,
}); });
} }
}, },
......
...@@ -560,12 +560,12 @@ export const closeDiffFileCommentForm = ({ commit }, fileHash) => { ...@@ -560,12 +560,12 @@ export const closeDiffFileCommentForm = ({ commit }, fileHash) => {
commit(types.CLOSE_DIFF_FILE_COMMENT_FORM, fileHash); commit(types.CLOSE_DIFF_FILE_COMMENT_FORM, fileHash);
}; };
export const setRenderTreeList = ({ commit }, renderTreeList) => { export const setRenderTreeList = ({ commit }, { renderTreeList, trackClick = true }) => {
commit(types.SET_RENDER_TREE_LIST, renderTreeList); commit(types.SET_RENDER_TREE_LIST, renderTreeList);
localStorage.setItem(TREE_LIST_STORAGE_KEY, renderTreeList); localStorage.setItem(TREE_LIST_STORAGE_KEY, renderTreeList);
if (window.gon?.features?.diffSettingsUsageData) { if (window.gon?.features?.diffSettingsUsageData && trackClick) {
api.trackRedisHllUserEvent(TRACKING_CLICK_FILE_BROWSER_SETTING); api.trackRedisHllUserEvent(TRACKING_CLICK_FILE_BROWSER_SETTING);
if (renderTreeList) { if (renderTreeList) {
...@@ -578,7 +578,7 @@ export const setRenderTreeList = ({ commit }, renderTreeList) => { ...@@ -578,7 +578,7 @@ export const setRenderTreeList = ({ commit }, renderTreeList) => {
export const setShowWhitespace = async ( export const setShowWhitespace = async (
{ state, commit }, { state, commit },
{ url, showWhitespace, updateDatabase = true }, { url, showWhitespace, updateDatabase = true, trackClick = true },
) => { ) => {
if (updateDatabase && Boolean(window.gon?.current_user_id)) { if (updateDatabase && Boolean(window.gon?.current_user_id)) {
await axios.put(url || state.endpointUpdateUser, { show_whitespace_in_diffs: showWhitespace }); await axios.put(url || state.endpointUpdateUser, { show_whitespace_in_diffs: showWhitespace });
...@@ -587,7 +587,7 @@ export const setShowWhitespace = async ( ...@@ -587,7 +587,7 @@ export const setShowWhitespace = async (
commit(types.SET_SHOW_WHITESPACE, showWhitespace); commit(types.SET_SHOW_WHITESPACE, showWhitespace);
notesEventHub.$emit('refetchDiffData'); notesEventHub.$emit('refetchDiffData');
if (window.gon?.features?.diffSettingsUsageData) { if (window.gon?.features?.diffSettingsUsageData && trackClick) {
api.trackRedisHllUserEvent(TRACKING_CLICK_WHITESPACE_SETTING); api.trackRedisHllUserEvent(TRACKING_CLICK_WHITESPACE_SETTING);
if (showWhitespace) { if (showWhitespace) {
......
...@@ -48,13 +48,17 @@ describe('Diff settings dropdown component', () => { ...@@ -48,13 +48,17 @@ describe('Diff settings dropdown component', () => {
it('list view button dispatches setRenderTreeList with false', () => { it('list view button dispatches setRenderTreeList with false', () => {
wrapper.find('.js-list-view').trigger('click'); wrapper.find('.js-list-view').trigger('click');
expect(store.dispatch).toHaveBeenCalledWith('diffs/setRenderTreeList', false); expect(store.dispatch).toHaveBeenCalledWith('diffs/setRenderTreeList', {
renderTreeList: false,
});
}); });
it('tree view button dispatches setRenderTreeList with true', () => { it('tree view button dispatches setRenderTreeList with true', () => {
wrapper.find('.js-tree-view').trigger('click'); wrapper.find('.js-tree-view').trigger('click');
expect(store.dispatch).toHaveBeenCalledWith('diffs/setRenderTreeList', true); expect(store.dispatch).toHaveBeenCalledWith('diffs/setRenderTreeList', {
renderTreeList: true,
});
}); });
it('sets list button as selected when renderTreeList is false', () => { it('sets list button as selected when renderTreeList is false', () => {
......
...@@ -1000,7 +1000,7 @@ describe('DiffsStoreActions', () => { ...@@ -1000,7 +1000,7 @@ describe('DiffsStoreActions', () => {
it('commits SET_RENDER_TREE_LIST', (done) => { it('commits SET_RENDER_TREE_LIST', (done) => {
testAction( testAction(
setRenderTreeList, setRenderTreeList,
true, { renderTreeList: true },
{}, {},
[{ type: types.SET_RENDER_TREE_LIST, payload: true }], [{ type: types.SET_RENDER_TREE_LIST, payload: true }],
[], [],
...@@ -1009,7 +1009,7 @@ describe('DiffsStoreActions', () => { ...@@ -1009,7 +1009,7 @@ describe('DiffsStoreActions', () => {
}); });
it('sets localStorage', () => { it('sets localStorage', () => {
setRenderTreeList({ commit() {} }, true); setRenderTreeList({ commit() {} }, { renderTreeList: true });
expect(localStorage.setItem).toHaveBeenCalledWith('mr_diff_tree_list', true); expect(localStorage.setItem).toHaveBeenCalledWith('mr_diff_tree_list', true);
}); });
......
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