fix: vuex mutation violation from draggable (#674)

Co-authored-by: Dominik Pschenitschni <mail@celement.de>
Reviewed-on: https://kolaente.dev/vikunja/frontend/pulls/674
Reviewed-by: konrad <k@knt.li>
Co-authored-by: dpschen <dpschen@noreply.kolaente.de>
Co-committed-by: dpschen <dpschen@noreply.kolaente.de>

src/components/home/navigation.vue

@@ -78,8 +78,13 @@
 					class="more-container"
 					v-if="typeof listsVisible[n.id] !== 'undefined' ? listsVisible[n.id] : true"
 				>
+					<!--
+						NOTE: a v-model / computed setter is not possible, since the updateActiveLists function
+						triggered by the change needs to have access to the current namespace
+					--> 
 					<draggable
-						v-model="n.lists"
+						:value="activeLists[nk]"
+						@input="(lists) => updateActiveLists(n, lists)"
 						:group="`namespace-${n.id}-lists`"
 						@start="() => drag = true"
 						@end="e => saveListPosition(e, nk)"
@@ -94,11 +99,9 @@
 							tag="ul"
 							class="menu-list can-be-hidden"
 						>
-							<!-- eslint-disable vue/no-use-v-if-with-v-for,vue/no-confusing-v-for-v-if -->
 							<li
-								v-for="l in n.lists"
+								v-for="l in activeLists[nk]"
 								:key="l.id"
-								v-if="!l.isArchived"
 								class="loader-container"
 								:class="{'is-loading': listUpdating[l.id]}"
 							>
@@ -167,13 +170,18 @@ export default {
 		NamespaceSettingsDropdown,
 		draggable,
 	},
-	computed: mapState({
+	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 listsActive = this.activeLists[namespaceIndex]
-			const list = listsFiltered[e.newIndex]
+			const list = listsActive[e.newIndex]
-			const listBefore = listsFiltered[e.newIndex - 1] ?? null
+			const listBefore = listsActive[e.newIndex - 1] ?? null
-			const listAfter = listsFiltered[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)
 				})

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('')

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,

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 => {
+				.then(() => {
-					ctx.commit('setList', r)
+					ctx.commit('setList', list)
-					ctx.commit('namespaces/setListInNamespaceById', r, {root: true})
+					ctx.commit('namespaces/setListInNamespaceById', list, {root: true})
-					if (r.isFavorite) {
+
-						r.namespaceId = FavoriteListsNamespace
+					// the returned list from listService.update is the same!
-						ctx.commit('namespaces/addListToNamespace', r, {root: true})
+					// 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', newList, {root: true})
-						ctx.commit('namespaces/removeListFromNamespaceById', r, {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', {
-					ctx.commit('setList', list)
+						...list,
+						isFavorite: !list.isFavorite,
+					})
 					return Promise.reject(e)
 				})
 				.finally(() => cancel())

src/store/modules/namespaces.js

@@ -13,13 +13,13 @@ export default {
 			state.namespaces = namespaces
 		},
 		setNamespaceById(state, namespace) {
-			for (const n in state.namespaces) {
+			const namespaceIndex = state.namespaces.findIndex(n => n.id === namespace.id)
-				if (state.namespaces[n].id === namespace.id) {
+			
-					namespace.lists = state.namespaces[n].lists
+			if (namespaceIndex === -1) {
-					Vue.set(state.namespaces, n, namespace)
 				return
 			}
-			}
+			
+			Vue.set(state.namespaces, namespaceIndex, namespace)
 		},
 		setListInNamespaceById(state, list) {
 			for (const n in state.namespaces) {