From 2af42f8fbee49de4fe332df28f14be0c8e7b5d35 Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 12 Jan 2023 13:32:00 +0100 Subject: [PATCH] fix(quick add magic): make sure assignees which don't exist are not removed from task title Resolves https://kolaente.dev/vikunja/frontend/issues/2927 --- src/helpers/time/parseDate.ts | 22 +++++++++++--------- src/modules/parseTaskText.test.ts | 32 ++++++++++++++++++------------ src/modules/parseTaskText.ts | 5 ++--- src/stores/base.ts | 2 +- src/stores/tasks.ts | 18 +++++++++++++---- src/views/tasks/TaskDetailView.vue | 2 +- 6 files changed, 50 insertions(+), 31 deletions(-) diff --git a/src/helpers/time/parseDate.ts b/src/helpers/time/parseDate.ts index 226cb8848..483d7ac56 100644 --- a/src/helpers/time/parseDate.ts +++ b/src/helpers/time/parseDate.ts @@ -14,31 +14,35 @@ interface dateFoundResult { const monthsRegexGroup = '(jan|feb|mar|apr|may|jun|jul|aug|sep|oct|nov|dec)' +function matchesDateExpr(text: string, dateExpr: string): boolean { + return text.match(new RegExp('(^| )' + dateExpr, 'g')) !== null +} + export const parseDate = (text: string, now: Date = new Date()): dateParseResult => { const lowerText: string = text.toLowerCase() - if (lowerText.includes('today')) { + if (matchesDateExpr(lowerText, 'today')) { return addTimeToDate(text, getDateFromInterval(calculateDayInterval('today')), 'today') } - if (lowerText.includes('tomorrow')) { + if (matchesDateExpr(lowerText, 'tomorrow')) { return addTimeToDate(text, getDateFromInterval(calculateDayInterval('tomorrow')), 'tomorrow') } - if (lowerText.includes('next monday')) { + if (matchesDateExpr(lowerText, 'next monday')) { return addTimeToDate(text, getDateFromInterval(calculateDayInterval('nextMonday')), 'next monday') } - if (lowerText.includes('this weekend')) { + if (matchesDateExpr(lowerText, 'this weekend')) { return addTimeToDate(text, getDateFromInterval(calculateDayInterval('thisWeekend')), 'this weekend') } - if (lowerText.includes('later this week')) { + if (matchesDateExpr(lowerText, 'later this week')) { return addTimeToDate(text, getDateFromInterval(calculateDayInterval('laterThisWeek')), 'later this week') } - if (lowerText.includes('later next week')) { + if (matchesDateExpr(lowerText, 'later next week')) { return addTimeToDate(text, getDateFromInterval(calculateDayInterval('laterNextWeek')), 'later next week') } - if (lowerText.includes('next week')) { + if (matchesDateExpr(lowerText, 'next week')) { return addTimeToDate(text, getDateFromInterval(calculateDayInterval('nextWeek')), 'next week') } - if (lowerText.includes('next month')) { + if (matchesDateExpr(lowerText, 'next month')) { const date: Date = new Date() date.setDate(1) date.setMonth(date.getMonth() + 1) @@ -48,7 +52,7 @@ export const parseDate = (text: string, now: Date = new Date()): dateParseResult return addTimeToDate(text, date, 'next month') } - if (lowerText.includes('end of month')) { + if (matchesDateExpr(lowerText, 'end of month')) { const curDate: Date = new Date() const date: Date = new Date(curDate.getFullYear(), curDate.getMonth() + 1, 0) date.setHours(calculateNearestHours(date)) diff --git a/src/modules/parseTaskText.test.ts b/src/modules/parseTaskText.test.ts index 0fabcb1a6..1622ac1cb 100644 --- a/src/modules/parseTaskText.test.ts +++ b/src/modules/parseTaskText.test.ts @@ -30,7 +30,7 @@ describe('Parse Task Text', () => { it('should parse text in todoist mode when configured', () => { const result = parseTaskText('Lorem Ipsum today @label #list !2 +user', PrefixMode.Todoist) - expect(result.text).toBe('Lorem Ipsum') + expect(result.text).toBe('Lorem Ipsum +user') const now = new Date() expect(result?.date?.getFullYear()).toBe(now.getFullYear()) expect(result?.date?.getMonth()).toBe(now.getMonth()) @@ -633,47 +633,53 @@ describe('Parse Task Text', () => { describe('Assignee', () => { it('should parse an assignee', () => { - const result = parseTaskText('Lorem Ipsum @user') + const text = 'Lorem Ipsum @user' + const result = parseTaskText(text) - expect(result.text).toBe('Lorem Ipsum') + expect(result.text).toBe(text) expect(result.assignees).toHaveLength(1) expect(result.assignees[0]).toBe('user') }) it('should parse multiple assignees', () => { - const result = parseTaskText('Lorem Ipsum @user1 @user2 @user3') + const text = 'Lorem Ipsum @user1 @user2 @user3' + const result = parseTaskText(text) - expect(result.text).toBe('Lorem Ipsum') + expect(result.text).toBe(text) expect(result.assignees).toHaveLength(3) expect(result.assignees[0]).toBe('user1') expect(result.assignees[1]).toBe('user2') expect(result.assignees[2]).toBe('user3') }) it('should parse avoid duplicate assignees', () => { - const result = parseTaskText('Lorem Ipsum @user1 @user1 @user2') + const text = 'Lorem Ipsum @user1 @user1 @user2' + const result = parseTaskText(text) - expect(result.text).toBe('Lorem Ipsum') + expect(result.text).toBe(text) expect(result.assignees).toHaveLength(2) expect(result.assignees[0]).toBe('user1') expect(result.assignees[1]).toBe('user2') }) it('should parse an assignee with a space in it', () => { - const result = parseTaskText(`Lorem Ipsum @'user with long name'`) + const text = `Lorem Ipsum @'user with long name'` + const result = parseTaskText(text) - expect(result.text).toBe('Lorem Ipsum') + expect(result.text).toBe(text) expect(result.assignees).toHaveLength(1) expect(result.assignees[0]).toBe('user with long name') }) it('should parse an assignee with a space in it and "', () => { - const result = parseTaskText(`Lorem Ipsum @"user with long name"`) + const text = `Lorem Ipsum @"user with long name"` + const result = parseTaskText(text) - expect(result.text).toBe('Lorem Ipsum') + expect(result.text).toBe(text) expect(result.assignees).toHaveLength(1) expect(result.assignees[0]).toBe('user with long name') }) it('should parse an assignee who is called like a date as assignee', () => { - const result = parseTaskText(`Lorem Ipsum @today`) + const text = `Lorem Ipsum @today` + const result = parseTaskText(text) - expect(result.text).toBe('Lorem Ipsum') + expect(result.text).toBe(text) expect(result.assignees).toHaveLength(1) expect(result.assignees[0]).toBe('today') }) diff --git a/src/modules/parseTaskText.ts b/src/modules/parseTaskText.ts index 6fab191a8..2ac30942a 100644 --- a/src/modules/parseTaskText.ts +++ b/src/modules/parseTaskText.ts @@ -82,7 +82,6 @@ export const parseTaskText = (text: string, prefixesMode: PrefixMode = PrefixMod result.text = result.priority !== null ? cleanupItemText(result.text, [String(result.priority)], prefixes.priority) : result.text result.assignees = getItemsFromPrefix(result.text, prefixes.assignee) - result.text = cleanupItemText(result.text, result.assignees, prefixes.assignee) const {textWithoutMatched, repeats} = getRepeats(result.text) result.text = textWithoutMatched @@ -277,7 +276,7 @@ const getRepeats = (text: string): repeatParsedResult => { } } -const cleanupItemText = (text: string, items: string[], prefix: string): string => { +export const cleanupItemText = (text: string, items: string[], prefix: string): string => { items.forEach(l => { text = text .replace(`${prefix}'${l}' `, '') @@ -294,7 +293,7 @@ const cleanupResult = (result: ParsedTaskText, prefixes: Prefixes): ParsedTaskTe result.text = cleanupItemText(result.text, result.labels, prefixes.label) result.text = result.list !== null ? cleanupItemText(result.text, [result.list], prefixes.list) : result.text result.text = result.priority !== null ? cleanupItemText(result.text, [String(result.priority)], prefixes.priority) : result.text - result.text = cleanupItemText(result.text, result.assignees, prefixes.assignee) + // Not removing assignees to avoid removing @text where the user does not exist result.text = result.text.trim() return result diff --git a/src/stores/base.ts b/src/stores/base.ts index 258ff2745..7e9c6d71b 100644 --- a/src/stores/base.ts +++ b/src/stores/base.ts @@ -97,7 +97,7 @@ export const useBaseStore = defineStore('base', () => { // The forceUpdate parameter is used only when updating a list background directly because in that case // the current list stays the same, but we want to show the new background right away. - if (list.id !== currentList.value.id || forceUpdate) { + if (list.id !== currentList.value?.id || forceUpdate) { if (list.backgroundInformation) { try { const blurHash = await getBlobFromBlurHash(list.backgroundBlurHash) diff --git a/src/stores/tasks.ts b/src/stores/tasks.ts index 58937307e..f82d26ec9 100644 --- a/src/stores/tasks.ts +++ b/src/stores/tasks.ts @@ -9,7 +9,7 @@ import UserService from '@/services/user' import {playPop} from '@/helpers/playPop' import {getQuickAddMagicMode} from '@/helpers/quickAddMagicMode' -import {parseTaskText} from '@/modules/parseTaskText' +import {cleanupItemText, parseTaskText, PREFIXES} from '@/modules/parseTaskText' import TaskAssigneeModel from '@/models/taskAssignee' import LabelTaskModel from '@/models/labelTask' @@ -63,7 +63,7 @@ async function addLabelToTask(task: ITask, label: ILabel) { return response } -async function findAssignees(parsedTaskAssignees: string[]) { +async function findAssignees(parsedTaskAssignees: string[]): Promise { if (parsedTaskAssignees.length <= 0) { return [] } @@ -376,7 +376,8 @@ export const useTaskStore = defineStore('task', () => { Partial, ) { const cancel = setModuleLoading(setIsLoading) - const parsedTask = parseTaskText(title, getQuickAddMagicMode()) + const quickAddMagicMode = getQuickAddMagicMode() + const parsedTask = parseTaskText(title, quickAddMagicMode) const foundListId = await findListId({ list: parsedTask.list, @@ -390,11 +391,20 @@ export const useTaskStore = defineStore('task', () => { const assignees = await findAssignees(parsedTask.assignees) + // Only clean up those assignees from the task title which actually exist + let cleanedTitle = parsedTask.text + if (assignees.length > 0) { + const assigneePrefix = PREFIXES[quickAddMagicMode]?.assignee + if (assigneePrefix) { + cleanedTitle = cleanupItemText(cleanedTitle, assignees.map(a => a.username), assigneePrefix) + } + } + // I don't know why, but it all goes up in flames when I just pass in the date normally. const dueDate = parsedTask.date !== null ? new Date(parsedTask.date).toISOString() : null const task = new TaskModel({ - title: parsedTask.text, + title: cleanedTitle, listId: foundListId, dueDate, priority: parsedTask.priority, diff --git a/src/views/tasks/TaskDetailView.vue b/src/views/tasks/TaskDetailView.vue index e856cebea..eac5cb90c 100644 --- a/src/views/tasks/TaskDetailView.vue +++ b/src/views/tasks/TaskDetailView.vue @@ -557,7 +557,7 @@ watch( baseStore.handleSetCurrentList({list: parentList}) } }, - {immediate: true }, + {immediate: true}, ) const canWrite = computed(() => (