Commit aaba5cf7 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Fix discussions filter not working

This bug was introduced by
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69933 when we
changed the mutation to append or update discussions.

When changing the discussions filter, we must clear the existing
discussions before fetching the filtered list.

This also disables the filter when discussions are loading to prevent
race conditions where the response of a previous request could show up
if the filters are switched very quickly

Changelog: fixed
parent 10da9f9f
...@@ -39,7 +39,7 @@ export default { ...@@ -39,7 +39,7 @@ export default {
}; };
}, },
computed: { computed: {
...mapGetters(['getNotesDataByProp', 'timelineEnabled']), ...mapGetters(['getNotesDataByProp', 'timelineEnabled', 'isLoading']),
currentFilter() { currentFilter() {
if (!this.currentValue) return this.filters[0]; if (!this.currentValue) return this.filters[0];
return this.filters.find((filter) => filter.value === this.currentValue); return this.filters.find((filter) => filter.value === this.currentValue);
...@@ -119,6 +119,7 @@ export default { ...@@ -119,6 +119,7 @@ export default {
class="gl-mr-3 full-width-mobile discussion-filter-container js-discussion-filter-container" class="gl-mr-3 full-width-mobile discussion-filter-container js-discussion-filter-container"
data-qa-selector="discussion_filter_dropdown" data-qa-selector="discussion_filter_dropdown"
:text="currentFilter.title" :text="currentFilter.title"
:disabled="isLoading"
> >
<div v-for="filter in filters" :key="filter.value" class="dropdown-item-wrapper"> <div v-for="filter in filters" :key="filter.value" class="dropdown-item-wrapper">
<gl-dropdown-item <gl-dropdown-item
......
...@@ -601,7 +601,8 @@ export const setLoadingState = ({ commit }, data) => { ...@@ -601,7 +601,8 @@ export const setLoadingState = ({ commit }, data) => {
commit(types.SET_NOTES_LOADING_STATE, data); commit(types.SET_NOTES_LOADING_STATE, data);
}; };
export const filterDiscussion = ({ dispatch }, { path, filter, persistFilter }) => { export const filterDiscussion = ({ commit, dispatch }, { path, filter, persistFilter }) => {
commit(types.CLEAR_DISCUSSIONS);
dispatch('setLoadingState', true); dispatch('setLoadingState', true);
dispatch('fetchDiscussions', { path, filter, persistFilter }) dispatch('fetchDiscussions', { path, filter, persistFilter })
.then(() => { .then(() => {
......
export const ADD_NEW_NOTE = 'ADD_NEW_NOTE'; export const ADD_NEW_NOTE = 'ADD_NEW_NOTE';
export const ADD_NEW_REPLY_TO_DISCUSSION = 'ADD_NEW_REPLY_TO_DISCUSSION'; export const ADD_NEW_REPLY_TO_DISCUSSION = 'ADD_NEW_REPLY_TO_DISCUSSION';
export const ADD_OR_UPDATE_DISCUSSIONS = 'ADD_OR_UPDATE_DISCUSSIONS'; export const ADD_OR_UPDATE_DISCUSSIONS = 'ADD_OR_UPDATE_DISCUSSIONS';
export const CLEAR_DISCUSSIONS = 'CLEAR_DISCUSSIONS';
export const DELETE_NOTE = 'DELETE_NOTE'; export const DELETE_NOTE = 'DELETE_NOTE';
export const REMOVE_PLACEHOLDER_NOTES = 'REMOVE_PLACEHOLDER_NOTES'; export const REMOVE_PLACEHOLDER_NOTES = 'REMOVE_PLACEHOLDER_NOTES';
export const SET_NOTES_DATA = 'SET_NOTES_DATA'; export const SET_NOTES_DATA = 'SET_NOTES_DATA';
......
...@@ -129,6 +129,10 @@ export default { ...@@ -129,6 +129,10 @@ export default {
Object.assign(state, { userData: data }); Object.assign(state, { userData: data });
}, },
[types.CLEAR_DISCUSSIONS](state) {
state.discussions = [];
},
[types.ADD_OR_UPDATE_DISCUSSIONS](state, discussionsData) { [types.ADD_OR_UPDATE_DISCUSSIONS](state, discussionsData) {
discussionsData.forEach((d) => { discussionsData.forEach((d) => {
const discussion = { ...d }; const discussion = { ...d };
......
import { GlDropdown } from '@gitlab/ui';
import { createLocalVue, mount } from '@vue/test-utils'; import { createLocalVue, mount } from '@vue/test-utils';
import AxiosMockAdapter from 'axios-mock-adapter'; import AxiosMockAdapter from 'axios-mock-adapter';
import Vuex from 'vuex'; import Vuex from 'vuex';
...@@ -88,6 +89,12 @@ describe('DiscussionFilter component', () => { ...@@ -88,6 +89,12 @@ describe('DiscussionFilter component', () => {
); );
}); });
it('disables the dropdown when discussions are loading', () => {
store.state.isLoading = true;
expect(wrapper.findComponent(GlDropdown).props('disabled')).toBe(true);
});
it('updates to the selected item', () => { it('updates to the selected item', () => {
const filterItem = findFilter(DISCUSSION_FILTER_TYPES.ALL); const filterItem = findFilter(DISCUSSION_FILTER_TYPES.ALL);
......
...@@ -1183,8 +1183,14 @@ describe('Actions Notes Store', () => { ...@@ -1183,8 +1183,14 @@ describe('Actions Notes Store', () => {
dispatch.mockReturnValue(new Promise(() => {})); dispatch.mockReturnValue(new Promise(() => {}));
}); });
it('clears existing discussions', () => {
actions.filterDiscussion({ commit, dispatch }, { path, filter, persistFilter: false });
expect(commit.mock.calls).toEqual([[mutationTypes.CLEAR_DISCUSSIONS]]);
});
it('fetches discussions with filter and persistFilter false', () => { it('fetches discussions with filter and persistFilter false', () => {
actions.filterDiscussion({ dispatch }, { path, filter, persistFilter: false }); actions.filterDiscussion({ commit, dispatch }, { path, filter, persistFilter: false });
expect(dispatch.mock.calls).toEqual([ expect(dispatch.mock.calls).toEqual([
['setLoadingState', true], ['setLoadingState', true],
...@@ -1193,7 +1199,7 @@ describe('Actions Notes Store', () => { ...@@ -1193,7 +1199,7 @@ describe('Actions Notes Store', () => {
}); });
it('fetches discussions with filter and persistFilter true', () => { it('fetches discussions with filter and persistFilter true', () => {
actions.filterDiscussion({ dispatch }, { path, filter, persistFilter: true }); actions.filterDiscussion({ commit, dispatch }, { path, filter, persistFilter: true });
expect(dispatch.mock.calls).toEqual([ expect(dispatch.mock.calls).toEqual([
['setLoadingState', true], ['setLoadingState', true],
......
...@@ -159,6 +159,18 @@ describe('Notes Store mutations', () => { ...@@ -159,6 +159,18 @@ describe('Notes Store mutations', () => {
}); });
}); });
describe('CLEAR_DISCUSSIONS', () => {
it('should set discussions to an empty array', () => {
const state = {
discussions: [discussionMock],
};
mutations.CLEAR_DISCUSSIONS(state);
expect(state.discussions).toEqual([]);
});
});
describe('ADD_OR_UPDATE_DISCUSSIONS', () => { describe('ADD_OR_UPDATE_DISCUSSIONS', () => {
it('should set the initial notes received', () => { it('should set the initial notes received', () => {
const state = { const state = {
......
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