From 0a8505f53c5c64d675b9130fb3847966119dc20b Mon Sep 17 00:00:00 2001 From: dpschen Date: Mon, 23 Aug 2021 19:24:52 +0000 Subject: [PATCH] fix: vuex mutation violation from draggable (#674) Co-authored-by: Dominik Pschenitschni Reviewed-on: https://kolaente.dev/vikunja/frontend/pulls/674 Reviewed-by: konrad Co-authored-by: dpschen Co-committed-by: dpschen --- src/components/home/navigation.vue | 64 ++++++++++++++++++++++-------- src/services/list.js | 5 +++ src/store/index.js | 1 + src/store/modules/lists.js | 36 ++++++++++------- src/store/modules/namespaces.js | 12 +++--- 5 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/components/home/navigation.vue b/src/components/home/navigation.vue index 82c4cdae..af734f0b 100644 --- a/src/components/home/navigation.vue +++ b/src/components/home/navigation.vue @@ -78,8 +78,13 @@ class="more-container" v-if="typeof listsVisible[n.id] !== 'undefined' ? listsVisible[n.id] : true" > + -
  • @@ -167,13 +170,18 @@ export default { NamespaceSettingsDropdown, draggable, }, - computed: mapState({ - namespaces: state => state.namespaces.namespaces.filter(n => !n.isArchived), - currentList: CURRENT_LIST, - background: 'background', - menuActive: MENU_ACTIVE, - loading: state => state[LOADING] && state[LOADING_MODULE] === 'namespaces', - }), + computed: { + ...mapState({ + namespaces: state => state.namespaces.namespaces.filter(n => !n.isArchived), + currentList: CURRENT_LIST, + background: 'background', + menuActive: MENU_ACTIVE, + loading: state => state[LOADING] && state[LOADING_MODULE] === 'namespaces', + }), + activeLists() { + return this.namespaces.map(({lists}) => lists.filter(item => !item.isArchived)) + }, + }, beforeCreate() { this.$store.dispatch('namespaces/loadNamespaces') .then(namespaces => { @@ -211,16 +219,38 @@ export default { toggleLists(namespaceId) { this.$set(this.listsVisible, namespaceId, !this.listsVisible[namespaceId] ?? false) }, + updateActiveLists(namespace, activeLists) { + // this is a bit hacky: since we do have to filter out the archived items from the list + // for vue draggable updating it is not as simple as replacing it. + // instead we iterate over the non archived items in the old list and replace them with the ones in their new order + const lists = namespace.lists.map((item) => { + if (item.isArchived) { + return item + } + return activeLists.shift() + }) + + const newNamespace = { + ...namespace, + lists, + } + + this.$store.commit('namespaces/setNamespaceById', newNamespace) + }, saveListPosition(e, namespaceIndex) { - const listsFiltered = this.namespaces[namespaceIndex].lists.filter(l => !l.isArchived) - const list = listsFiltered[e.newIndex] - const listBefore = listsFiltered[e.newIndex - 1] ?? null - const listAfter = listsFiltered[e.newIndex + 1] ?? null + const listsActive = this.activeLists[namespaceIndex] + const list = listsActive[e.newIndex] + const listBefore = listsActive[e.newIndex - 1] ?? null + const listAfter = listsActive[e.newIndex + 1] ?? null this.$set(this.listUpdating, list.id, true) - list.position = calculateItemPosition(listBefore !== null ? listBefore.position : null, listAfter !== null ? listAfter.position : null) + const position = calculateItemPosition(listBefore !== null ? listBefore.position : null, listAfter !== null ? listAfter.position : null) - this.$store.dispatch('lists/updateList', list) + // create a copy of the list in order to not violate vuex mutations + this.$store.dispatch('lists/updateList', { + ...list, + position, + }) .catch(e => { this.error(e) }) diff --git a/src/services/list.js b/src/services/list.js index 22d73088..5fd2fc3d 100644 --- a/src/services/list.js +++ b/src/services/list.js @@ -39,6 +39,11 @@ export default class ListService extends AbstractService { return list } + update(model) { + const newModel = { ... model } + return super.update(newModel) + } + background(list) { if (list.background === null) { return Promise.resolve('') diff --git a/src/store/index.js b/src/store/index.js index df24ccd0..921ff864 100644 --- a/src/store/index.js +++ b/src/store/index.js @@ -24,6 +24,7 @@ import ListService from '../services/list' Vue.use(Vuex) export const store = new Vuex.Store({ + strict: import.meta.env.DEV, modules: { config, auth, diff --git a/src/store/modules/lists.js b/src/store/modules/lists.js index fe66d76b..21400801 100644 --- a/src/store/modules/lists.js +++ b/src/store/modules/lists.js @@ -38,9 +38,10 @@ export default { }, actions: { toggleListFavorite(ctx, list) { - list.isFavorite = !list.isFavorite - - return ctx.dispatch('updateList', list) + return ctx.dispatch('updateList', { + ...list, + isFavorite: !list.isFavorite, + }) }, createList(ctx, list) { const cancel = setLoading(ctx, 'lists') @@ -61,24 +62,31 @@ export default { const listService = new ListService() return listService.update(list) - .then(r => { - ctx.commit('setList', r) - ctx.commit('namespaces/setListInNamespaceById', r, {root: true}) - if (r.isFavorite) { - r.namespaceId = FavoriteListsNamespace - ctx.commit('namespaces/addListToNamespace', r, {root: true}) + .then(() => { + ctx.commit('setList', list) + ctx.commit('namespaces/setListInNamespaceById', list, {root: true}) + + // the returned list from listService.update is the same! + // in order to not validate vuex mutations we have to create a new copy + const newList = { + ...list, + namespaceId: FavoriteListsNamespace, + } + if (list.isFavorite) { + ctx.commit('namespaces/addListToNamespace', newList, {root: true}) } else { - r.namespaceId = FavoriteListsNamespace - ctx.commit('namespaces/removeListFromNamespaceById', r, {root: true}) + ctx.commit('namespaces/removeListFromNamespaceById', newList, {root: true}) } ctx.dispatch('namespaces/loadNamespacesIfFavoritesDontExist', null, {root: true}) ctx.dispatch('namespaces/removeFavoritesNamespaceIfEmpty', null, {root: true}) - return Promise.resolve(r) + return Promise.resolve(newList) }) .catch(e => { // Reset the list state to the initial one to avoid confusion for the user - list.isFavorite = !list.isFavorite - ctx.commit('setList', list) + ctx.commit('setList', { + ...list, + isFavorite: !list.isFavorite, + }) return Promise.reject(e) }) .finally(() => cancel()) diff --git a/src/store/modules/namespaces.js b/src/store/modules/namespaces.js index 4dadd347..65505db9 100644 --- a/src/store/modules/namespaces.js +++ b/src/store/modules/namespaces.js @@ -13,13 +13,13 @@ export default { state.namespaces = namespaces }, setNamespaceById(state, namespace) { - for (const n in state.namespaces) { - if (state.namespaces[n].id === namespace.id) { - namespace.lists = state.namespaces[n].lists - Vue.set(state.namespaces, n, namespace) - return - } + const namespaceIndex = state.namespaces.findIndex(n => n.id === namespace.id) + + if (namespaceIndex === -1) { + return } + + Vue.set(state.namespaces, namespaceIndex, namespace) }, setListInNamespaceById(state, list) { for (const n in state.namespaces) {