From a9d3446ce30dff5024f1bd3afd0e7098a574f947 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Thu, 3 Jun 2021 22:23:04 +0200
Subject: [PATCH] Preload labels and use locally stored in vuex

---
 src/components/home/contentAuth.vue          |   7 ++
 src/components/list/partials/filters.vue     |  62 +++++------
 src/components/tasks/partials/editLabels.vue |  58 ++++------
 src/store/index.js                           |   2 +
 src/store/modules/labels.js                  | 100 +++++++++++++++++
 src/views/labels/ListLabels.vue              |  50 ++-------
 src/views/labels/NewLabel.vue                |  27 ++---
 src/views/list/views/List.vue                | 110 ++++++++++++-------
 8 files changed, 246 insertions(+), 170 deletions(-)
 create mode 100644 src/store/modules/labels.js

diff --git a/src/components/home/contentAuth.vue b/src/components/home/contentAuth.vue
index cee4fadbd..576d77bf0 100644
--- a/src/components/home/contentAuth.vue
+++ b/src/components/home/contentAuth.vue
@@ -55,6 +55,7 @@ export default {
 	},
 	created() {
 		this.renewTokenOnFocus()
+		this.loadLabels()
 	},
 	computed: mapState({
 		namespaces(state) {
@@ -126,6 +127,12 @@ export default {
 		showKeyboardShortcuts() {
 			this.$store.commit(KEYBOARD_SHORTCUTS_ACTIVE, true)
 		},
+		loadLabels() {
+			this.$store.dispatch('labels/loadAllLabels')
+				.catch(e => {
+					this.error(e, this)
+				})
+		},
 	},
 }
 </script>
diff --git a/src/components/list/partials/filters.vue b/src/components/list/partials/filters.vue
index 478c4f0b2..a28f339e6 100644
--- a/src/components/list/partials/filters.vue
+++ b/src/components/list/partials/filters.vue
@@ -131,7 +131,6 @@
 			<label class="label">Labels</label>
 			<div class="control">
 				<multiselect
-					:loading="labelService.loading"
 					placeholder="Type to search for a label..."
 					@search="findLabels"
 					:search-results="foundLabels"
@@ -202,7 +201,6 @@ import PercentDoneSelect from '@/components/tasks/partials/percentDoneSelect'
 import Multiselect from '@/components/input/multiselect'
 
 import UserService from '@/services/user'
-import LabelService from '@/services/label'
 import ListService from '@/services/list'
 import NamespaceService from '@/services/namespace'
 import {mapState} from 'vuex'
@@ -249,8 +247,7 @@ export default {
 			foundusers: [],
 			users: [],
 
-			labelService: LabelService,
-			foundLabels: [],
+			labelQuery: '',
 			labels: [],
 
 			listsService: ListService,
@@ -264,7 +261,6 @@ export default {
 	},
 	created() {
 		this.usersService = new UserService()
-		this.labelService = new LabelService()
 		this.listsService = new ListService()
 		this.namespaceService = new NamespaceService()
 	},
@@ -284,19 +280,30 @@ export default {
 			this.prepareFilters()
 		},
 	},
-	computed: mapState({
-		flatPickerConfig: state => ({
-			altFormat: 'j M Y H:i',
-			altInput: true,
-			dateFormat: 'Y-m-d H:i',
-			enableTime: true,
-			time_24hr: true,
-			mode: 'range',
-			locale: {
-				firstDayOfWeek: state.auth.settings.weekStart,
-			},
-		})
-	}),
+	computed: {
+		foundLabels() {
+			const labels = (Object.values(this.$store.state.labels.labels).filter(l => {
+				return l.title.toLowerCase().includes(this.labelQuery.toLowerCase())
+			}) ?? [])
+
+			return differenceWith(labels, this.labels, (first, second) => {
+				return first.id === second.id
+			})
+		},
+		...mapState({
+			flatPickerConfig: state => ({
+				altFormat: 'j M Y H:i',
+				altInput: true,
+				dateFormat: 'Y-m-d H:i',
+				enableTime: true,
+				time_24hr: true,
+				mode: 'range',
+				locale: {
+					firstDayOfWeek: state.auth.settings.weekStart,
+				},
+			}),
+		}),
+	},
 	methods: {
 		change() {
 			this.$emit('input', this.params)
@@ -566,25 +573,8 @@ export default {
 			this.$set(this.filters, filterName, ids.join(','))
 			this.setSingleValueFilter(filterName, filterName, '', 'in')
 		},
-		clearLabels() {
-			this.$set(this, 'foundLabels', [])
-		},
 		findLabels(query) {
-
-			if (query === '') {
-				this.clearLabels()
-			}
-
-			this.labelService.getAll({}, {s: query})
-				.then(response => {
-					// Filter the results to not include labels already selected
-					this.$set(this, 'foundLabels', differenceWith(response, this.labels, (first, second) => {
-						return first.id === second.id
-					}))
-				})
-				.catch(e => {
-					this.error(e, this)
-				})
+			this.labelQuery = query
 		},
 		addLabel() {
 			this.$nextTick(() => {
diff --git a/src/components/tasks/partials/editLabels.vue b/src/components/tasks/partials/editLabels.vue
index daf3271ba..a40f2113e 100644
--- a/src/components/tasks/partials/editLabels.vue
+++ b/src/components/tasks/partials/editLabels.vue
@@ -1,6 +1,6 @@
 <template>
 	<multiselect
-		:loading="labelService.loading || labelTaskService.loading"
+		:loading="loading"
 		placeholder="Type to add a new label..."
 		:multiple="true"
 		@search="findLabel"
@@ -39,11 +39,11 @@
 <script>
 import differenceWith from 'lodash/differenceWith'
 
-import LabelService from '../../../services/label'
 import LabelModel from '../../../models/label'
 import LabelTaskService from '../../../services/labelTask'
 
 import Multiselect from '@/components/input/multiselect'
+import {LOADING, LOADING_MODULE} from '@/store/mutation-types'
 
 export default {
 	name: 'edit-labels',
@@ -62,12 +62,10 @@ export default {
 	},
 	data() {
 		return {
-			labelService: LabelService,
 			labelTaskService: LabelTaskService,
-			foundLabels: [],
 			labelTimeout: null,
 			labels: [],
-			searchQuery: '',
+			query: '',
 		}
 	},
 	components: {
@@ -79,38 +77,26 @@ export default {
 		},
 	},
 	created() {
-		this.labelService = new LabelService()
 		this.labelTaskService = new LabelTaskService()
 		this.labels = this.value
 	},
+	computed: {
+		foundLabels() {
+			const labels = (Object.values(this.$store.state.labels.labels).filter(l => {
+				return l.title.toLowerCase().includes(this.query.toLowerCase())
+			}) ?? [])
+
+			return differenceWith(labels, this.labels, (first, second) => {
+				return first.id === second.id
+			})
+		},
+		loading() {
+			return this.labelTaskService.loading || (this.$store.state[LOADING] && this.$store.state[LOADING_MODULE] === 'labels')
+		},
+	},
 	methods: {
 		findLabel(query) {
-			this.searchQuery = query
-			if (query === '') {
-				this.clearAllLabels()
-				return
-			}
-
-			if (this.labelTimeout !== null) {
-				clearTimeout(this.labelTimeout)
-			}
-
-			// Delay the search 300ms to not send a request on every keystroke
-			this.labelTimeout = setTimeout(() => {
-				this.labelService.getAll({}, {s: query})
-					.then(response => {
-						this.$set(this, 'foundLabels', differenceWith(response, this.labels, (first, second) => {
-							return first.id === second.id
-						}))
-						this.labelTimeout = null
-					})
-					.catch(e => {
-						this.error(e, this)
-					})
-			}, 300)
-		},
-		clearAllLabels() {
-			this.$set(this, 'foundLabels', [])
+			this.query = query
 		},
 		addLabel(label, showNotification = true) {
 			this.$store.dispatch('tasks/addLabel', {label: label, taskId: this.taskId})
@@ -141,8 +127,8 @@ export default {
 				})
 		},
 		createAndAddLabel(title) {
-			let newLabel = new LabelModel({title: title})
-			this.labelService.create(newLabel)
+			const newLabel = new LabelModel({title: title})
+			this.$store.dispatch('labels/createLabel', newLabel)
 				.then(r => {
 					this.addLabel(r, false)
 					this.labels.push(r)
@@ -156,7 +142,3 @@ export default {
 	},
 }
 </script>
-
-<style scoped>
-
-</style>
\ No newline at end of file
diff --git a/src/store/index.js b/src/store/index.js
index 260d02705..dbe860673 100644
--- a/src/store/index.js
+++ b/src/store/index.js
@@ -17,6 +17,7 @@ import kanban from './modules/kanban'
 import tasks from './modules/tasks'
 import lists from './modules/lists'
 import attachments from './modules/attachments'
+import labels from './modules/labels'
 
 import ListService from '../services/list'
 import {setTitle} from '@/helpers/setTitle'
@@ -32,6 +33,7 @@ export const store = new Vuex.Store({
 		tasks,
 		lists,
 		attachments,
+		labels,
 	},
 	state: {
 		loading: false,
diff --git a/src/store/modules/labels.js b/src/store/modules/labels.js
new file mode 100644
index 000000000..f8dde6563
--- /dev/null
+++ b/src/store/modules/labels.js
@@ -0,0 +1,100 @@
+import LabelService from '@/services/label'
+import Vue from 'vue'
+import {setLoading} from '@/store/helper'
+
+export default {
+	namespaced: true,
+	// The state is an object which has the label ids as keys.
+	state: () => ({
+		labels: {},
+		loaded: false,
+	}),
+	mutations: {
+		setLabels(state, labels) {
+			labels.forEach(l => {
+				Vue.set(state.labels, l.id, l)
+			})
+		},
+		setLabel(state, label) {
+			Vue.set(state.labels, label.id, label)
+		},
+		removeLabelById(state, label) {
+			Vue.delete(state.labels, label.id)
+		},
+		setLoaded(state, loaded) {
+			state.loaded = loaded
+		},
+	},
+	actions: {
+		loadAllLabels(ctx, {forceLoad} = {}) {
+			if (ctx.state.loaded && !forceLoad) {
+				return Promise.resolve()
+			}
+
+			const cancel = setLoading(ctx, 'labels')
+			const labelService = new LabelService()
+
+			const getAllLabels = (page = 1) => {
+				return labelService.getAll({}, {}, page)
+					.then(labels => {
+						if (page < labelService.totalPages) {
+							return getAllLabels(page + 1)
+								.then(nextLabels => {
+									return labels.concat(nextLabels)
+								})
+						} else {
+							return labels
+						}
+					})
+					.catch(e => {
+						return Promise.reject(e)
+					})
+			}
+
+			return getAllLabels()
+				.then(r => {
+					ctx.commit('setLabels', r)
+					ctx.commit('setLoaded', true)
+					return Promise.resolve(r)
+				})
+				.catch(e => Promise.reject(e))
+				.finally(() => cancel())
+		},
+		deleteLabel(ctx, label) {
+			const cancel = setLoading(ctx, 'labels')
+			const labelService = new LabelService()
+
+			return labelService.delete(label)
+				.then(r => {
+					ctx.commit('removeLabelById', label)
+					return Promise.resolve(r)
+				})
+				.catch(e => Promise.reject(e))
+				.finally(() => cancel())
+		},
+		updateLabel(ctx, label) {
+			const cancel = setLoading(ctx, 'labels')
+			const labelService = new LabelService()
+
+			return labelService.update(label)
+				.then(r => {
+					ctx.commit('setLabel', r)
+					return Promise.resolve(r)
+				})
+				.catch(e => Promise.reject(e))
+				.finally(() => cancel())
+		},
+		createLabel(ctx, label) {
+			const cancel = setLoading(ctx, 'labels')
+			const labelService = new LabelService()
+
+			return labelService.create(label)
+				.then(r => {
+					ctx.commit('setLabel', r)
+					return Promise.resolve(r)
+				})
+				.catch(e => Promise.reject(e))
+				.finally(() => cancel())
+		},
+	},
+}
diff --git a/src/views/labels/ListLabels.vue b/src/views/labels/ListLabels.vue
index 21a5f504a..ddeeca4a8 100644
--- a/src/views/labels/ListLabels.vue
+++ b/src/views/labels/ListLabels.vue
@@ -1,5 +1,5 @@
 <template>
-	<div :class="{ 'is-loading': labelService.loading}" class="loader-container">
+	<div :class="{ 'is-loading': loading}" class="loader-container">
 		<x-button
 			:to="{name:'labels.create'}"
 			class="is-pulled-right"
@@ -76,7 +76,7 @@
 						<div class="field has-addons">
 							<div class="control is-expanded">
 								<x-button
-									:loading="labelService.loading"
+									:loading="loading"
 									class="is-fullwidth"
 									@click="editLabelSubmit()"
 								>
@@ -101,11 +101,11 @@
 <script>
 import {mapState} from 'vuex'
 
-import LabelService from '../../services/label'
 import LabelModel from '../../models/label'
 import ColorPicker from '../../components/input/colorPicker'
 import LoadingComponent from '../../components/misc/loading'
 import ErrorComponent from '../../components/misc/error'
+import {LOADING, LOADING_MODULE} from '@/store/mutation-types'
 
 export default {
 	name: 'ListLabels',
@@ -120,15 +120,12 @@ export default {
 	},
 	data() {
 		return {
-			labelService: LabelService,
-			labels: [],
 			labelEditLabel: LabelModel,
 			isLabelEdit: false,
 			editorActive: false,
 		}
 	},
 	created() {
-		this.labelService = new LabelService()
 		this.labelEditLabel = new LabelModel()
 		this.loadLabels()
 	},
@@ -137,43 +134,19 @@ export default {
 	},
 	computed: mapState({
 		userInfo: state => state.auth.info,
+		labels: state => state.labels.labels,
+		loading: state => state[LOADING] && state[LOADING_MODULE] === 'labels',
 	}),
 	methods: {
 		loadLabels() {
-			const getAllLabels = (page = 1) => {
-				return this.labelService.getAll({}, {}, page)
-					.then(labels => {
-						if (page < this.labelService.totalPages) {
-							return getAllLabels(page + 1)
-								.then(nextLabels => {
-									return labels.concat(nextLabels)
-								})
-						} else {
-							return labels
-						}
-					})
-					.catch(e => {
-						return Promise.reject(e)
-					})
-			}
-
-			getAllLabels()
-				.then(r => {
-					this.$set(this, 'labels', r)
-				})
+			this.$store.dispatch('labels/loadAllLabels')
 				.catch(e => {
 					this.error(e, this)
 				})
 		},
 		deleteLabel(label) {
-			this.labelService.delete(label)
+			this.$store.dispatch('labels/deleteLabel', label)
 				.then(() => {
-					// Remove the label from the list
-					for (const l in this.labels) {
-						if (this.labels[l].id === label.id) {
-							this.labels.splice(l, 1)
-						}
-					}
 					this.success({message: 'The label was successfully deleted.'}, this)
 				})
 				.catch(e => {
@@ -181,13 +154,8 @@ export default {
 				})
 		},
 		editLabelSubmit() {
-			this.labelService.update(this.labelEditLabel)
-				.then(r => {
-					for (const l in this.labels) {
-						if (this.labels[l].id === r.id) {
-							this.$set(this.labels, l, r)
-						}
-					}
+			this.$store.dispatch('labels/updateLabel', this.labelEditLabel)
+				.then(() => {
 					this.success({message: 'The label was successfully updated.'}, this)
 				})
 				.catch(e => {
diff --git a/src/views/labels/NewLabel.vue b/src/views/labels/NewLabel.vue
index 84009183e..a0925d1bb 100644
--- a/src/views/labels/NewLabel.vue
+++ b/src/views/labels/NewLabel.vue
@@ -8,10 +8,10 @@
 			<label class="label" for="labelTitle">Label Title</label>
 			<div
 				class="control is-expanded"
-				:class="{ 'is-loading': labelService.loading }"
+				:class="{ 'is-loading': loading }"
 			>
 				<input
-					:class="{ disabled: labelService.loading }"
+					:class="{ disabled: loading }"
 					class="input"
 					placeholder="The label title goes here..."
 					type="text"
@@ -28,7 +28,7 @@
 		<div class="field">
 			<label class="label">Color</label>
 			<div class="control">
-				<color-picker v-model="label.hexColor" />
+				<color-picker v-model="label.hexColor"/>
 			</div>
 		</div>
 	</create-edit>
@@ -36,17 +36,16 @@
 
 <script>
 import labelModel from '../../models/label'
-import labelService from '../../services/label'
 import LabelModel from '../../models/label'
-import LabelService from '../../services/label'
 import CreateEdit from '@/components/misc/create-edit'
 import ColorPicker from '../../components/input/colorPicker'
+import {mapState} from 'vuex'
+import {LOADING, LOADING_MODULE} from '@/store/mutation-types'
 
 export default {
 	name: 'NewLabel',
 	data() {
 		return {
-			labelService: labelService,
 			label: labelModel,
 			showError: false,
 		}
@@ -56,12 +55,14 @@ export default {
 		ColorPicker,
 	},
 	created() {
-		this.labelService = new LabelService()
 		this.label = new LabelModel()
 	},
 	mounted() {
 		this.setTitle('Create a new label')
 	},
+	computed: mapState({
+		loading: state => state[LOADING] && state[LOADING_MODULE] === 'labels',
+	}),
 	methods: {
 		newLabel() {
 			if (this.label.title === '') {
@@ -70,17 +71,13 @@ export default {
 			}
 			this.showError = false
 
-			this.labelService
-				.create(this.label)
-				.then((response) => {
+			this.$store.dispatch('labels/createLabel', this.label)
+				.then(r => {
 					this.$router.push({
 						name: 'labels.index',
-						params: { id: response.id },
+						params: {id: r.id},
 					})
-					this.success(
-						{ message: 'The label was successfully created.' },
-						this
-					)
+					this.success({message: 'The label was successfully created.'}, this)
 				})
 				.catch((e) => {
 					this.error(e, this)
diff --git a/src/views/list/views/List.vue b/src/views/list/views/List.vue
index 36c77e94c..46b5bd7e4 100644
--- a/src/views/list/views/List.vue
+++ b/src/views/list/views/List.vue
@@ -161,7 +161,6 @@
 import TaskService from '../../../services/task'
 import TaskModel from '../../../models/task'
 import LabelTaskService from '../../../services/labelTask'
-import LabelService from '../../../services/label'
 import LabelTask from '../../../models/labelTask'
 import LabelModel from '../../../models/label'
 
@@ -186,7 +185,6 @@ export default {
 
 			showError: false,
 			labelTaskService: LabelTaskService,
-			labelService: LabelService,
 
 			ctaVisible: false,
 		}
@@ -202,7 +200,6 @@ export default {
 	},
 	created() {
 		this.taskService = new TaskService()
-		this.labelService = new LabelService()
 		this.labelTaskService = new LabelTaskService()
 
 		// Save the current list view to local storage
@@ -237,6 +234,40 @@ export default {
 					this.sortTasks()
 					this.newTaskText = ''
 
+					// Unlike a proper programming language, Javascript only knows references to objects and does not
+					// allow you to control what is a reference and what isnt. Because of this we can't just add
+					// all labels to the task they belong to right after we found and added them to the task since
+					// the task update method also ensures all data the api sees has the right format. That means
+					// it processes labels. That processing changes the date format and the label color and makes
+					// the label pretty much unusable for everything else. Normally, this is not a big deal, because
+					// the labels on a task get thrown away anyway and replaced with the new models from the api
+					// when we get the updated answer back. However, in this specific case because we're passing a
+					// label we obtained from vuex that reference is kept and not thrown away. The task itself gets
+					// a new label object - you won't notice the bad reference until you want to add the same label
+					// again and notice it doesn't have a color anymore.
+					// I think this is what happens: (or rather would happen without the hack I've put in)
+					//   1. Query the store for a label which matches the name
+					//   2. Find one - remember, we get only a *reference* to the label from the store, not a new label object.
+					//      (Now there's *two* places with a reference to the same label object: in the store and in the
+					//      variable which holds the label from the search in the store)
+					//   3. .push the label to the task
+					//   4. Update the task to remove the labels from the name
+					//   4.1. The task update processes all labels belonging to that task, changing attributes of our
+					//        label in the process. Because this is a reference, it is also "updated" in the store.
+					//   5. Get an api response back. The service handler now creates a new label object for all labels
+					//      returned from the api. It will throw away all references to the old label in the process.
+					//   6. Now we have two objects with the same label data: The old one we originally obtained from
+					//      the store and the one that was created when parsing the api response. The old one was
+					//      modified before sending the api request and thus, our store which still holds a reference
+					//      to the old label now contains old data.
+					//      I guess this is the point where normally the GC would come in and collect the old label
+					//      object if the store wouldn't still hold a reference to it.
+					//
+					// Now, as a workaround, I'm putting all new labels added to that task in this separate variable to
+					// add them only after the task was updated to circumvent the task update service processing the
+					// label before sending it. Feels more hacky than it probably is.
+					const newLabels = []
+
 					// Check if the task has words starting with ~ in the title and make them to labels
 					const parts = task.title.split(' ~')
 					// The first element will always contain the title, even if there is no occurrence of ~
@@ -271,17 +302,41 @@ export default {
 							}
 
 							// Check if the label exists
-							this.labelService.getAll({}, {s: labelTitle})
-								.then(res => {
-									// Label found, use it
-									if (res.length > 0 && res[0].title === labelTitle) {
+							const label = Object.values(this.$store.state.labels.labels).find(l => {
+								return l.title.toLowerCase() === labelTitle.toLowerCase()
+							})
+
+							// Label found, use it
+							if (typeof label !== 'undefined') {
+								const labelTask = new LabelTask({
+									taskId: task.id,
+									labelId: label.id,
+								})
+								this.labelTaskService.create(labelTask)
+									.then(result => {
+										newLabels.push(label)
+
+										// Remove the label text from the task title
+										task.title = task.title.replace(` ~${labelTitle}`, '')
+
+										// Make the promise done (the one with the index 0 does not exist)
+										labelAddings[index - 1].resolve(result)
+									})
+									.catch(e => {
+										this.error(e, this)
+									})
+							} else {
+								// label not found, create it
+								const label = new LabelModel({title: labelTitle})
+								this.$store.dispatch('labels/createLabel', label)
+									.then(res => {
 										const labelTask = new LabelTask({
 											taskId: task.id,
-											labelId: res[0].id,
+											labelId: res.id,
 										})
 										this.labelTaskService.create(labelTask)
 											.then(result => {
-												task.labels.push(res[0])
+												newLabels.push(res)
 
 												// Remove the label text from the task title
 												task.title = task.title.replace(` ~${labelTitle}`, '')
@@ -292,37 +347,11 @@ export default {
 											.catch(e => {
 												this.error(e, this)
 											})
-									} else {
-										// label not found, create it
-										const label = new LabelModel({title: labelTitle})
-										this.labelService.create(label)
-											.then(res => {
-												const labelTask = new LabelTask({
-													taskId: task.id,
-													labelId: res.id,
-												})
-												this.labelTaskService.create(labelTask)
-													.then(result => {
-														task.labels.push(res)
-
-														// Remove the label text from the task title
-														task.title = task.title.replace(` ~${labelTitle}`, '')
-
-														// Make the promise done (the one with the index 0 does not exist)
-														labelAddings[index - 1].resolve(result)
-													})
-													.catch(e => {
-														this.error(e, this)
-													})
-											})
-											.catch(e => {
-												this.error(e, this)
-											})
-									}
-								})
-								.catch(e => {
-									this.error(e, this)
-								})
+									})
+									.catch(e => {
+										this.error(e, this)
+									})
+							}
 						})
 
 						// This waits to update the task until all labels have been added and the title has
@@ -331,6 +360,7 @@ export default {
 							.then(() => {
 								this.taskService.update(task)
 									.then(updatedTask => {
+										updatedTask.labels = newLabels
 										this.updateTasks(updatedTask)
 										this.$store.commit(HAS_TASKS, true)
 									})