From b005d023c9ab4662022040914a250445dc560ff9 Mon Sep 17 00:00:00 2001 From: Sytone Date: Mon, 7 Jun 2021 09:07:42 -0700 Subject: [PATCH] Update label logic on add-task component Lint updates --- src/components/input/datepicker.vue | 2 +- src/components/tasks/add-task.vue | 137 +++++++++++-------- src/components/tasks/partials/defer-task.vue | 2 +- src/views/list/views/Gantt.vue | 2 +- src/views/tasks/ShowTasks.vue | 2 +- 5 files changed, 86 insertions(+), 59 deletions(-) diff --git a/src/components/input/datepicker.vue b/src/components/input/datepicker.vue index 91ed9f617..5062bfe48 100644 --- a/src/components/input/datepicker.vue +++ b/src/components/input/datepicker.vue @@ -176,7 +176,7 @@ export default { locale: { firstDayOfWeek: state.auth.settings.weekStart, }, - }) + }), }), methods: { setDateValue(newVal) { diff --git a/src/components/tasks/add-task.vue b/src/components/tasks/add-task.vue index b6c9f4f51..5a4db5504 100644 --- a/src/components/tasks/add-task.vue +++ b/src/components/tasks/add-task.vue @@ -97,6 +97,40 @@ export default { .then(task => { 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 ~ @@ -132,19 +166,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) + 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}`, '') @@ -155,56 +211,27 @@ 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 // been modified to remove each label text - Promise.all(labelAddsToWaitFor).then(() => { - this.taskService - .update(task) - .then(() => { - this.$store.commit(HAS_TASKS, true) - }) - .catch(e => { - this.error(e, this) - }) - }) + Promise.all(labelAddsToWaitFor) + .then(() => { + this.taskService.update(task) + .then(updatedTask => { + updatedTask.labels = newLabels + this.$store.commit(HAS_TASKS, true) + }) + .catch(e => { + this.error(e, this) + }) + }) + } this.$emit('taskAdded', task) }) diff --git a/src/components/tasks/partials/defer-task.vue b/src/components/tasks/partials/defer-task.vue index 0f6c0c474..62d3541e9 100644 --- a/src/components/tasks/partials/defer-task.vue +++ b/src/components/tasks/partials/defer-task.vue @@ -105,7 +105,7 @@ export default { locale: { firstDayOfWeek: state.auth.settings.weekStart, }, - }) + }), }), methods: { deferDays(days) { diff --git a/src/views/list/views/Gantt.vue b/src/views/list/views/Gantt.vue index 395f0c43f..102de1296 100644 --- a/src/views/list/views/Gantt.vue +++ b/src/views/list/views/Gantt.vue @@ -97,7 +97,7 @@ export default { locale: { firstDayOfWeek: state.auth.settings.weekStart, }, - }) + }), }), beforeMount() { this.dateFrom = new Date((new Date()).setDate((new Date()).getDate() - 15)) diff --git a/src/views/tasks/ShowTasks.vue b/src/views/tasks/ShowTasks.vue index bf0b976f0..ee0e08c5f 100644 --- a/src/views/tasks/ShowTasks.vue +++ b/src/views/tasks/ShowTasks.vue @@ -117,7 +117,7 @@ export default { locale: { firstDayOfWeek: state.auth.settings.weekStart, }, - }) + }), }), methods: { setDate() {