feat: automatically create subtask relations based on indention #2443

Merged
konrad merged 6 commits from feature/subtask-via-indention into main 2022-09-30 11:47:22 +00:00
3 changed files with 199 additions and 9 deletions

View File

@ -41,18 +41,19 @@
</template>
<script setup lang="ts">
import {ref, watch, unref, computed} from 'vue'
import {computed, ref, unref, watch} from 'vue'
import {useI18n} from 'vue-i18n'
import {tryOnMounted, debouncedWatch, useWindowSize, type MaybeRef} from '@vueuse/core'
import {debouncedWatch, type MaybeRef, tryOnMounted, useWindowSize} from '@vueuse/core'
import QuickAddMagic from '@/components/tasks/partials/quick-add-magic.vue'
import type {ITask} from '@/modelTypes/ITask'
import {parseSubtasksViaIndention} from '@/helpers/parseSubtasksViaIndention'
import TaskRelationService from '@/services/taskRelation'
import TaskRelationModel from '@/models/taskRelation'
import {RELATION_KIND} from '@/types/IRelationKind'
import {useAuthStore} from '@/stores/auth'
import {useTaskStore} from '@/stores/tasks'
function cleanupTitle(title: string) {
return title.replace(/^((\* |\+ |- )(\[ \] )?)/g, '')
}
function useAutoHeightTextarea(value: MaybeRef<string>) {
const textarea = ref<HTMLInputElement>()
const minHeight = ref(0)
@ -161,8 +162,9 @@ async function addTask() {
}
const taskTitleBackup = newTaskTitle.value
const newTasks = newTaskTitle.value.split(/[\r\n]+/).map(async uncleanedTitle => {
const title = cleanupTitle(uncleanedTitle)
const createdTasks: ITask[] = []
const tasksToCreate = parseSubtasksViaIndention(newTaskTitle.value)
konrad marked this conversation as resolved Outdated

spread title: ({title}) => {

spread title: `({title}) => {`
const newTasks = tasksToCreate.map(async ({title}) => {
if (title === '') {
return
}
@ -172,13 +174,44 @@ async function addTask() {
listId: authStore.settings.defaultListId,
position: props.defaultPosition,
})
emit('taskAdded', task)
createdTasks.push(task)
return task
})
try {
newTaskTitle.value = ''
await Promise.all(newTasks)
const taskRelationService = new TaskRelationService()
const relations = tasksToCreate.map(async t => {
const createdTask = createdTasks.find(ct => ct.title === t.title)
if (typeof createdTask === 'undefined') {
konrad marked this conversation as resolved Outdated

In line 194 we return if typeof createdTask === 'undefined'.
This implies that createdTask could be undefined here aswell. Is that intended?

In line 194 we return if `typeof createdTask === 'undefined'`. This implies that createdTask could be undefined here aswell. Is that intended?

mhh I didn't think this through. It should never be null.

I've adjusted this so that there's a check before emitting it.

mhh I didn't think this through. It should never be null. I've adjusted this so that there's a check before emitting it.
return
}
if (t.parent === null) {
emit('taskAdded', createdTask)
return
}
const createdParentTask = createdTasks.find(ct => ct.title === t.parent)
if (typeof createdTask === 'undefined' || typeof createdParentTask === 'undefined') {
return
}
const rel = await taskRelationService.create(new TaskRelationModel({
taskId: createdTask.id,
otherTaskId: createdParentTask.id,
relationKind: RELATION_KIND.PARENTTASK,
}))
createdTask.relatedTasks[RELATION_KIND.PARENTTASK] = [createdParentTask]
// we're only emitting here so that the relation shows up in the task list
emit('taskAdded', createdTask)
return rel
})
await Promise.all(relations)
} catch (e: any) {
newTaskTitle.value = taskTitleBackup
if (e?.message === 'NO_LIST') {

View File

@ -0,0 +1,109 @@
import {describe, it, expect} from 'vitest'
import {parseSubtasksViaIndention} from '@/helpers/parseSubtasksViaIndention'
describe('Parse Subtasks via Relation', () => {
it('Should not return a parent for a single task', () => {
const tasks = parseSubtasksViaIndention('single task')
expect(tasks).to.have.length(1)
expect(tasks[0].parent).toBeNull()
})
it('Should not return a parent for multiple tasks without indention', () => {
const tasks = parseSubtasksViaIndention(`task one
task two`)
expect(tasks).to.have.length(2)
expect(tasks[0].parent).toBeNull()
expect(tasks[1].parent).toBeNull()
})
it('Should return a parent for two tasks with indention', () => {
const tasks = parseSubtasksViaIndention(`parent task
sub task`)
expect(tasks).to.have.length(2)
expect(tasks[0].parent).toBeNull()
expect(tasks[0].title).to.eq('parent task')
expect(tasks[1].parent).to.eq('parent task')
expect(tasks[1].title).to.eq('sub task')
})
it('Should return a parent for multiple subtasks', () => {
const tasks = parseSubtasksViaIndention(`parent task
sub task one
sub task two`)
expect(tasks).to.have.length(3)
expect(tasks[0].parent).toBeNull()
expect(tasks[0].title).to.eq('parent task')
expect(tasks[1].title).to.eq('sub task one')
expect(tasks[1].parent).to.eq('parent task')
expect(tasks[2].title).to.eq('sub task two')
expect(tasks[2].parent).to.eq('parent task')
})
it('Should work with multiple indention levels', () => {
const tasks = parseSubtasksViaIndention(`parent task
sub task
sub sub task`)
expect(tasks).to.have.length(3)
expect(tasks[0].parent).toBeNull()
expect(tasks[0].title).to.eq('parent task')
expect(tasks[1].title).to.eq('sub task')
expect(tasks[1].parent).to.eq('parent task')
expect(tasks[2].title).to.eq('sub sub task')
expect(tasks[2].parent).to.eq('sub task')
})
it('Should work with multiple indention levels and multiple tasks', () => {
const tasks = parseSubtasksViaIndention(`parent task
sub task
sub sub task one
sub sub task two`)
expect(tasks).to.have.length(4)
expect(tasks[0].parent).toBeNull()
expect(tasks[0].title).to.eq('parent task')
expect(tasks[1].title).to.eq('sub task')
expect(tasks[1].parent).to.eq('parent task')
expect(tasks[2].title).to.eq('sub sub task one')
expect(tasks[2].parent).to.eq('sub task')
expect(tasks[3].title).to.eq('sub sub task two')
expect(tasks[3].parent).to.eq('sub task')
})
it('Should work with multiple indention levels and multiple tasks', () => {
const tasks = parseSubtasksViaIndention(`parent task
sub task
sub sub task one
sub sub sub task
sub sub task two`)
expect(tasks).to.have.length(5)
expect(tasks[0].parent).toBeNull()
expect(tasks[0].title).to.eq('parent task')
expect(tasks[1].title).to.eq('sub task')
expect(tasks[1].parent).to.eq('parent task')
expect(tasks[2].title).to.eq('sub sub task one')
expect(tasks[2].parent).to.eq('sub task')
expect(tasks[3].title).to.eq('sub sub sub task')
expect(tasks[3].parent).to.eq('sub sub task one')
expect(tasks[4].title).to.eq('sub sub task two')
expect(tasks[4].parent).to.eq('sub task')
})
it('Should return a parent for multiple subtasks with special stuff', () => {
const tasks = parseSubtasksViaIndention(`* parent task
* sub task one
sub task two`)
expect(tasks).to.have.length(3)
expect(tasks[0].parent).toBeNull()
expect(tasks[0].title).to.eq('parent task')
expect(tasks[1].title).to.eq('sub task one')
expect(tasks[1].parent).to.eq('parent task')
expect(tasks[2].title).to.eq('sub task two')
expect(tasks[2].parent).to.eq('parent task')
})
it('Should not break when the first line is indented', () => {
const tasks = parseSubtasksViaIndention(' single task')
expect(tasks).to.have.length(1)
expect(tasks[0].parent).toBeNull()
})
})

View File

@ -0,0 +1,48 @@
export interface TaskWithParent {
dpschen marked this conversation as resolved Outdated

Why do we create a new interface for this and don't use ITask for this?

Why do we create a new interface for this and don't use ITask for this?

Because this is only used to hold the title and parent title. Using ITask would probably work but I felt it would introduce too much overhead for a simple relation.

Because this is only used to hold the title and parent title. Using `ITask` would probably work but I felt it would introduce too much overhead for a simple relation.

Unsure: would it make sense to create the new type still on ITask with the help of Pick, like:

export interface TaskWithPartent = Pick<ITask, 'id' | 'parent'>
Unsure: would it make sense to create the new type still on `ITask` with the help of `Pick`, like: ```ts export interface TaskWithPartent = Pick<ITask, 'id' | 'parent'> ```

I think explicitely declaring it is more readable.

I think explicitely declaring it is more readable.

Good with me. I still think we should reuse the types from the task interface. Meaning:

export interface TaskWithParent {
	title: string,
	parent: ITask['id'] | null,
}

The reason beeing here: In the medium term we should change the id in the frontend to a string to make indexing easier. Because objects don't support number types as keys. The alternative would beto use Maps everywhere which is unrealistic (because they do support numbers as keys). By using zod this shouldn't be too complex.

Good with me. I still think we should reuse the types from the task interface. Meaning: ```t export interface TaskWithParent { title: string, parent: ITask['id'] | null, } ``` The reason beeing here: In the medium term we should change the id in the frontend to a string to make indexing easier. Because objects don't support number types as keys. The alternative would beto use Maps everywhere which is unrealistic (because they do support numbers as keys). By using zod this shouldn't be too complex.

But we're not even using IDs here, only string titles.

But we're not even using IDs here, only string titles.

Arrrrgh… makes sense. Sry i missed that part :D

Arrrrgh… makes sense. Sry i missed that part :D
title: string,
parent: string | null,
}
function cleanupTitle(title: string) {
return title.replace(/^((\* |\+ |- )(\[ \] )?)/g, '')
}
const spaceRegex = /^ */
/**
dpschen marked this conversation as resolved Outdated
Idea: use [JSDoc to explain param](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#param-and-returns)
* @param taskTitles should be multiple lines of task tiles with indention to declare their parent/subtask
* relation between each other.
*/
export function parseSubtasksViaIndention(taskTitles: string): TaskWithParent[] {
const titles = taskTitles.split(/[\r\n]+/)
konrad marked this conversation as resolved Outdated

Picky: use clear variable name, e.g. title, index.

Makes reading easier since you don't have to remember and jump to the var definition.

Picky: use clear variable name, e.g. `title`, `index`. Makes reading easier since you don't have to remember and jump to the var definition.

Makes sense. Done!

Makes sense. Done!
return titles.map((title, index) => {
const task: TaskWithParent = {
title: cleanupTitle(title),
parent: null,
}
konrad marked this conversation as resolved Outdated

Move matched and matchedSpaces in condition aswell since it's not used for index === 0

Move `matched` and `matchedSpaces` in condition aswell since it's not used for `index === 0`

Added an earlier break for index === 0.

Added an earlier break for `index === 0`.
if (index === 0) {
return task
}
const matched = spaceRegex.exec(title)
const matchedSpaces = matched ? matched[0].length : 0
if (matchedSpaces > 0) {
// Go up the tree to find the first task with less indention than the current one
let pi = 1
let parentSpaces = 0
do {
task.parent = cleanupTitle(titles[index - pi])
pi++
const parentMatched = spaceRegex.exec(task.parent)
parentSpaces = parentMatched ? parentMatched[0].length : 0
} while (parentSpaces >= matchedSpaces)
task.title = cleanupTitle(title.replace(spaceRegex, ''))
task.parent = task.parent.replace(spaceRegex, '')
}
return task
})
}