From 12474e5be6feb8b8fcb18c4670ab4893b945990b Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 18 Jul 2024 12:39:00 +0200 Subject: [PATCH] fix(kanban): make sure tasks which changed their done status are moved around in buckets This fixes a bug where tasks which had their done status changed were not moved in the correct bucket. This affected both frontend and api. The move of the task between buckets is now correctly done in the api and frontend - with a bit of duplicated logic between the two. This could be optimized further in the future. Resolves https://kolaente.dev/vikunja/vikunja/issues/2610 --- .../src/components/project/ProjectWrapper.vue | 12 ++- frontend/src/stores/base.ts | 15 +++- frontend/src/stores/kanban.ts | 46 ++++++++++- frontend/src/stores/tasks.ts | 2 +- frontend/src/views/tasks/TaskDetailView.vue | 10 --- pkg/models/tasks.go | 77 ++++++++++++++++--- 6 files changed, 134 insertions(+), 28 deletions(-) diff --git a/frontend/src/components/project/ProjectWrapper.vue b/frontend/src/components/project/ProjectWrapper.vue index 72640d2a1..1a92ca1aa 100644 --- a/frontend/src/components/project/ProjectWrapper.vue +++ b/frontend/src/components/project/ProjectWrapper.vue @@ -119,14 +119,14 @@ watch( loadedProjectId.value = 0 const projectFromStore = projectStore.projects[projectData.id] if (projectFromStore) { - baseStore.handleSetCurrentProject({project: projectFromStore}) + baseStore.handleSetCurrentProject({project: projectFromStore, currentProjectViewId: props.viewId}) } // We create an extra project object instead of creating it in project.value because that would trigger a ui update which would result in bad ux. const project = new ProjectModel(projectData) try { const loadedProject = await projectService.value.get(project) - baseStore.handleSetCurrentProject({project: loadedProject}) + baseStore.handleSetCurrentProject({project: loadedProject, currentProjectViewId: props.viewId}) } finally { loadedProjectId.value = projectIdToLoad } @@ -134,6 +134,14 @@ watch( {immediate: true}, ) +watch( + () => props.viewId, + () => { + baseStore.setCurrentProjectViewId(props.viewId) + }, + {immediate: true}, +) + function getViewTitle(view: IProjectView) { switch (view.title) { case 'List': diff --git a/frontend/src/stores/base.ts b/frontend/src/stores/base.ts index f861376cc..76642c7cf 100644 --- a/frontend/src/stores/base.ts +++ b/frontend/src/stores/base.ts @@ -12,6 +12,7 @@ import {useMenuActive} from '@/composables/useMenuActive' import {useAuthStore} from '@/stores/auth' import type {IProject} from '@/modelTypes/IProject' import type {Right} from '@/constants/rights' +import type {IProjectView} from '@/modelTypes/IProjectView' export const useBaseStore = defineStore('base', () => { const loading = ref(false) @@ -22,6 +23,7 @@ export const useBaseStore = defineStore('base', () => { id: 0, isArchived: false, })) + const currentProjectViewId = ref(undefined) const background = ref('') const blurHash = ref('') @@ -35,7 +37,7 @@ export const useBaseStore = defineStore('base', () => { loading.value = newLoading } - function setCurrentProject(newCurrentProject: IProject | null) { + function setCurrentProject(newCurrentProject: IProject | null, currentViewId?: IProjectView['id']) { // Server updates don't return the right. Therefore, the right is reset after updating the project which is // confusing because all the buttons will disappear in that case. To prevent this, we're keeping the right // when updating the project in global state. @@ -58,6 +60,11 @@ export const useBaseStore = defineStore('base', () => { ...newCurrentProject, maxRight, } + setCurrentProjectViewId(currentViewId) + } + + function setCurrentProjectViewId(viewId?: IProjectView['id']) { + currentProjectViewId.value = viewId } function setHasTasks(newHasTasks: boolean) { @@ -93,7 +100,7 @@ export const useBaseStore = defineStore('base', () => { } async function handleSetCurrentProject( - {project, forceUpdate = false}: {project: IProject | null, forceUpdate?: boolean}, + {project, forceUpdate = false, currentProjectViewId = undefined}: {project: IProject | null, forceUpdate?: boolean, currentProjectViewId?: IProjectView['id']}, ) { if (project === null || typeof project === 'undefined') { setCurrentProject({}) @@ -129,7 +136,7 @@ export const useBaseStore = defineStore('base', () => { setBlurHash('') } - setCurrentProject(project) + setCurrentProject(project, currentProjectViewId) } const authStore = useAuthStore() @@ -143,6 +150,7 @@ export const useBaseStore = defineStore('base', () => { loading: readonly(loading), ready: readonly(ready), currentProject: readonly(currentProject), + currentProjectViewId: readonly(currentProjectViewId), background: readonly(background), blurHash: readonly(blurHash), hasTasks: readonly(hasTasks), @@ -154,6 +162,7 @@ export const useBaseStore = defineStore('base', () => { setLoading, setReady, setCurrentProject, + setCurrentProjectViewId, setHasTasks, setKeyboardShortcutsActive, setQuickActionsActive, diff --git a/frontend/src/stores/kanban.ts b/frontend/src/stores/kanban.ts index a69fc84b7..4b9bca38b 100644 --- a/frontend/src/stores/kanban.ts +++ b/frontend/src/stores/kanban.ts @@ -14,6 +14,7 @@ import type {IProject} from '@/modelTypes/IProject' import type {IBucket} from '@/modelTypes/IBucket' import {useAuthStore} from '@/stores/auth' import type {IProjectView} from '@/modelTypes/IProjectView' +import {useBaseStore} from '@/stores/base' const TASKS_PER_BUCKET = 25 @@ -36,6 +37,7 @@ function getTaskIndicesById(buckets: IBucket[], taskId: ITask['id']) { */ export const useKanbanStore = defineStore('kanban', () => { const authStore = useAuthStore() + const baseStore = useBaseStore() const buckets = ref([]) const projectId = ref(0) @@ -138,7 +140,48 @@ export const useKanbanStore = defineStore('kanban', () => { } } + // This function is an exact clone of the logic in the api + function getDefaultBucketId(view: IProjectView): IBucket['id'] { + if (view.defaultBucketId) { + return view.defaultBucketId + } + + return buckets.value[0]?.id + } + + function ensureTaskIsInCorrectBucket(task: ITask) { + if (buckets.value.length === 0) { + return + } + + const {bucketIndex} = getTaskIndicesById(buckets.value, task.id) + if (bucketIndex === null) return + const currentTaskBucket = buckets.value[bucketIndex] + + const currentView: IProjectView = baseStore.currentProject?.views.find(v => v.id === baseStore.currentProjectViewId) + if(typeof currentView === 'undefined') return + + // If the task is done, make sure it is in the done bucket + if (task.done && currentView.doneBucketId !== 0 && currentTaskBucket.id !== currentView.doneBucketId) { + moveTaskToBucket(task, currentView.doneBucketId) + } + + // If the task is not done but was in the done bucket before, move it to the default bucket + if(!task.done && currentView.doneBucketId !== 0 && currentTaskBucket.id === currentView.doneBucketId) { + const defaultBucketId = getDefaultBucketId(currentView) + moveTaskToBucket(task, defaultBucketId) + } + + setTaskInBucket(task) + } + function moveTaskToBucket(task: ITask, bucketId: IBucket['id']) { + const {bucketIndex} = getTaskIndicesById(buckets.value, task.id) + if (bucketIndex === null) return + const currentTaskBucket = buckets.value[bucketIndex] + if (typeof currentTaskBucket === 'undefined' || currentTaskBucket.id === bucketId) { + return + } removeTaskInBucket(task) task.bucketId = bucketId addTaskToBucket(task) @@ -149,7 +192,7 @@ export const useKanbanStore = defineStore('kanban', () => { const oldBucket = buckets.value[bucketIndex] const newBucket = { ...oldBucket, - count: oldBucket.count + 1, + count: (oldBucket?.count || 0) + 1, tasks: [ ...oldBucket.tasks, task, @@ -342,6 +385,7 @@ export const useKanbanStore = defineStore('kanban', () => { createBucket, deleteBucket, updateBucket, + ensureTaskIsInCorrectBucket, } }) diff --git a/frontend/src/stores/tasks.ts b/frontend/src/stores/tasks.ts index 7aec0b3e7..02e5b74c2 100644 --- a/frontend/src/stores/tasks.ts +++ b/frontend/src/stores/tasks.ts @@ -156,7 +156,7 @@ export const useTaskStore = defineStore('task', () => { const taskService = new TaskService() try { const updatedTask = await taskService.update(task) - kanbanStore.setTaskInBucket(updatedTask) + kanbanStore.ensureTaskIsInCorrectBucket(updatedTask) return updatedTask } finally { cancel() diff --git a/frontend/src/views/tasks/TaskDetailView.vue b/frontend/src/views/tasks/TaskDetailView.vue index f7c625aea..22ea88a83 100644 --- a/frontend/src/views/tasks/TaskDetailView.vue +++ b/frontend/src/views/tasks/TaskDetailView.vue @@ -625,7 +625,6 @@ import {scrollIntoView} from '@/helpers/scrollIntoView' import {useAttachmentStore} from '@/stores/attachments' import {useTaskStore} from '@/stores/tasks' import {useKanbanStore} from '@/stores/kanban' -import {useBaseStore} from '@/stores/base' import {useTitle} from '@/composables/useTitle' @@ -651,7 +650,6 @@ const router = useRouter() const {t} = useI18n({useScope: 'global'}) const projectStore = useProjectStore() -const baseStore = useBaseStore() const attachmentStore = useAttachmentStore() const taskStore = useTaskStore() const kanbanStore = useKanbanStore() @@ -888,14 +886,6 @@ async function toggleTaskDone() { newTask, toggleTaskDone, ) - - if(task.value.done) { - baseStore.currentProject?.views.forEach(v => { - if (v.doneBucketId !== 0) { - kanbanStore.moveTaskToBucket(task.value, v.doneBucketId) - } - }) - } } async function changeProject(project: IProject) { diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index 43a119b9e..d3cc7279a 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -917,31 +917,86 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) { colsToUpdate = append(colsToUpdate, "index") } - // When a task was marked done or moved between projects, make sure it is in the correct bucket + viewsWithDoneBucket := []*ProjectView{} if (!t.isRepeating() && t.Done != ot.Done) || t.ProjectID != ot.ProjectID { - views, err := getViewsForProject(s, t.ProjectID) + err = s. + Where("project_id = ? AND view_kind = ? AND bucket_configuration_mode = ? AND done_bucket_id != 0", + t.ProjectID, ProjectViewKindKanban, BucketConfigurationModeManual). + Find(&viewsWithDoneBucket) if err != nil { - return err + return } + } - for _, view := range views { - if view.ViewKind != ProjectViewKindKanban && view.BucketConfigurationMode != BucketConfigurationModeManual { - continue - } - + // When a task was moved between projects, ensure it is in the correct bucket + if t.ProjectID != ot.ProjectID { + for _, view := range viewsWithDoneBucket { var bucketID = view.DoneBucketID - if bucketID == 0 || t.ProjectID != ot.ProjectID { + if bucketID == 0 || !t.Done { bucketID, err = getDefaultBucketID(s, view) if err != nil { return err } } - // Task is done and was moved between projects, should go into the done bucket of the new project - if t.Done && t.ProjectID != ot.ProjectID { + tb := &TaskBucket{ + BucketID: bucketID, + TaskID: t.ID, + ProjectViewID: view.ID, + ProjectID: t.ProjectID, + } + err = tb.Update(s, a) + if err != nil { + return err + } + + tp := TaskPosition{ + TaskID: t.ID, + ProjectViewID: view.ID, + Position: calculateDefaultPosition(t.Index, t.Position), + } + err = tp.Update(s, a) + if err != nil { + return err + } + } + } + + // When a task changed its done status, make sure it is in the correct bucket + if t.ProjectID == ot.ProjectID && !t.isRepeating() && t.Done != ot.Done { + for _, view := range viewsWithDoneBucket { + currentTaskBucket := &TaskBucket{} + _, err := s.Where("task_id = ? AND project_view_id = ?", t.ID, view.ID). + Get(currentTaskBucket) + if err != nil { + return err + } + + var bucketID = currentTaskBucket.BucketID + + // Task done, but no done bucket? Do nothing + if t.Done && view.DoneBucketID == 0 { + continue + } + + // Task not done, currently not in done bucket? Do nothing + if !t.Done && bucketID != view.DoneBucketID { + continue + } + + // Task done? Done bucket + if t.Done && view.DoneBucketID != 0 { bucketID = view.DoneBucketID } + // Task not done, currently in done bucket? Move to default + if !t.Done && bucketID == view.DoneBucketID { + bucketID, err = getDefaultBucketID(s, view) + if err != nil { + return err + } + } + tb := &TaskBucket{ BucketID: bucketID, TaskID: t.ID,