Commit 090af754 authored by Phil Hughes's avatar Phil Hughes

Fixes a bug where multiple tracking calls would happen

This happens in the diffs app because of when the Vuex actions
we increment clicks, but the actions also get called on page load.
parent b67a72df
...@@ -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