1
0

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
This commit is contained in:
kolaente 2024-07-18 12:39:00 +02:00
parent 34691b8edf
commit 12474e5be6
No known key found for this signature in database
GPG Key ID: F40E70337AB24C9B
6 changed files with 134 additions and 28 deletions

View File

@ -119,14 +119,14 @@ watch(
loadedProjectId.value = 0 loadedProjectId.value = 0
const projectFromStore = projectStore.projects[projectData.id] const projectFromStore = projectStore.projects[projectData.id]
if (projectFromStore) { 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. // 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) const project = new ProjectModel(projectData)
try { try {
const loadedProject = await projectService.value.get(project) const loadedProject = await projectService.value.get(project)
baseStore.handleSetCurrentProject({project: loadedProject}) baseStore.handleSetCurrentProject({project: loadedProject, currentProjectViewId: props.viewId})
} finally { } finally {
loadedProjectId.value = projectIdToLoad loadedProjectId.value = projectIdToLoad
} }
@ -134,6 +134,14 @@ watch(
{immediate: true}, {immediate: true},
) )
watch(
() => props.viewId,
() => {
baseStore.setCurrentProjectViewId(props.viewId)
},
{immediate: true},
)
function getViewTitle(view: IProjectView) { function getViewTitle(view: IProjectView) {
switch (view.title) { switch (view.title) {
case 'List': case 'List':

View File

@ -12,6 +12,7 @@ import {useMenuActive} from '@/composables/useMenuActive'
import {useAuthStore} from '@/stores/auth' import {useAuthStore} from '@/stores/auth'
import type {IProject} from '@/modelTypes/IProject' import type {IProject} from '@/modelTypes/IProject'
import type {Right} from '@/constants/rights' import type {Right} from '@/constants/rights'
import type {IProjectView} from '@/modelTypes/IProjectView'
export const useBaseStore = defineStore('base', () => { export const useBaseStore = defineStore('base', () => {
const loading = ref(false) const loading = ref(false)
@ -22,6 +23,7 @@ export const useBaseStore = defineStore('base', () => {
id: 0, id: 0,
isArchived: false, isArchived: false,
})) }))
const currentProjectViewId = ref<IProjectView['id'] | undefined>(undefined)
const background = ref('') const background = ref('')
const blurHash = ref('') const blurHash = ref('')
@ -35,7 +37,7 @@ export const useBaseStore = defineStore('base', () => {
loading.value = newLoading 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 // 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 // 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. // when updating the project in global state.
@ -58,6 +60,11 @@ export const useBaseStore = defineStore('base', () => {
...newCurrentProject, ...newCurrentProject,
maxRight, maxRight,
} }
setCurrentProjectViewId(currentViewId)
}
function setCurrentProjectViewId(viewId?: IProjectView['id']) {
currentProjectViewId.value = viewId
} }
function setHasTasks(newHasTasks: boolean) { function setHasTasks(newHasTasks: boolean) {
@ -93,7 +100,7 @@ export const useBaseStore = defineStore('base', () => {
} }
async function handleSetCurrentProject( 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') { if (project === null || typeof project === 'undefined') {
setCurrentProject({}) setCurrentProject({})
@ -129,7 +136,7 @@ export const useBaseStore = defineStore('base', () => {
setBlurHash('') setBlurHash('')
} }
setCurrentProject(project) setCurrentProject(project, currentProjectViewId)
} }
const authStore = useAuthStore() const authStore = useAuthStore()
@ -143,6 +150,7 @@ export const useBaseStore = defineStore('base', () => {
loading: readonly(loading), loading: readonly(loading),
ready: readonly(ready), ready: readonly(ready),
currentProject: readonly(currentProject), currentProject: readonly(currentProject),
currentProjectViewId: readonly(currentProjectViewId),
background: readonly(background), background: readonly(background),
blurHash: readonly(blurHash), blurHash: readonly(blurHash),
hasTasks: readonly(hasTasks), hasTasks: readonly(hasTasks),
@ -154,6 +162,7 @@ export const useBaseStore = defineStore('base', () => {
setLoading, setLoading,
setReady, setReady,
setCurrentProject, setCurrentProject,
setCurrentProjectViewId,
setHasTasks, setHasTasks,
setKeyboardShortcutsActive, setKeyboardShortcutsActive,
setQuickActionsActive, setQuickActionsActive,

View File

@ -14,6 +14,7 @@ import type {IProject} from '@/modelTypes/IProject'
import type {IBucket} from '@/modelTypes/IBucket' import type {IBucket} from '@/modelTypes/IBucket'
import {useAuthStore} from '@/stores/auth' import {useAuthStore} from '@/stores/auth'
import type {IProjectView} from '@/modelTypes/IProjectView' import type {IProjectView} from '@/modelTypes/IProjectView'
import {useBaseStore} from '@/stores/base'
const TASKS_PER_BUCKET = 25 const TASKS_PER_BUCKET = 25
@ -36,6 +37,7 @@ function getTaskIndicesById(buckets: IBucket[], taskId: ITask['id']) {
*/ */
export const useKanbanStore = defineStore('kanban', () => { export const useKanbanStore = defineStore('kanban', () => {
const authStore = useAuthStore() const authStore = useAuthStore()
const baseStore = useBaseStore()
const buckets = ref<IBucket[]>([]) const buckets = ref<IBucket[]>([])
const projectId = ref<IProject['id']>(0) const projectId = ref<IProject['id']>(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']) { 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) removeTaskInBucket(task)
task.bucketId = bucketId task.bucketId = bucketId
addTaskToBucket(task) addTaskToBucket(task)
@ -149,7 +192,7 @@ export const useKanbanStore = defineStore('kanban', () => {
const oldBucket = buckets.value[bucketIndex] const oldBucket = buckets.value[bucketIndex]
const newBucket = { const newBucket = {
...oldBucket, ...oldBucket,
count: oldBucket.count + 1, count: (oldBucket?.count || 0) + 1,
tasks: [ tasks: [
...oldBucket.tasks, ...oldBucket.tasks,
task, task,
@ -342,6 +385,7 @@ export const useKanbanStore = defineStore('kanban', () => {
createBucket, createBucket,
deleteBucket, deleteBucket,
updateBucket, updateBucket,
ensureTaskIsInCorrectBucket,
} }
}) })

View File

@ -156,7 +156,7 @@ export const useTaskStore = defineStore('task', () => {
const taskService = new TaskService() const taskService = new TaskService()
try { try {
const updatedTask = await taskService.update(task) const updatedTask = await taskService.update(task)
kanbanStore.setTaskInBucket(updatedTask) kanbanStore.ensureTaskIsInCorrectBucket(updatedTask)
return updatedTask return updatedTask
} finally { } finally {
cancel() cancel()

View File

@ -625,7 +625,6 @@ import {scrollIntoView} from '@/helpers/scrollIntoView'
import {useAttachmentStore} from '@/stores/attachments' import {useAttachmentStore} from '@/stores/attachments'
import {useTaskStore} from '@/stores/tasks' import {useTaskStore} from '@/stores/tasks'
import {useKanbanStore} from '@/stores/kanban' import {useKanbanStore} from '@/stores/kanban'
import {useBaseStore} from '@/stores/base'
import {useTitle} from '@/composables/useTitle' import {useTitle} from '@/composables/useTitle'
@ -651,7 +650,6 @@ const router = useRouter()
const {t} = useI18n({useScope: 'global'}) const {t} = useI18n({useScope: 'global'})
const projectStore = useProjectStore() const projectStore = useProjectStore()
const baseStore = useBaseStore()
const attachmentStore = useAttachmentStore() const attachmentStore = useAttachmentStore()
const taskStore = useTaskStore() const taskStore = useTaskStore()
const kanbanStore = useKanbanStore() const kanbanStore = useKanbanStore()
@ -888,14 +886,6 @@ async function toggleTaskDone() {
newTask, newTask,
toggleTaskDone, 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) { async function changeProject(project: IProject) {

View File

@ -917,31 +917,86 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) {
colsToUpdate = append(colsToUpdate, "index") 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 { 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 { if err != nil {
return err return
} }
}
for _, view := range views { // When a task was moved between projects, ensure it is in the correct bucket
if view.ViewKind != ProjectViewKindKanban && view.BucketConfigurationMode != BucketConfigurationModeManual { if t.ProjectID != ot.ProjectID {
continue for _, view := range viewsWithDoneBucket {
}
var bucketID = view.DoneBucketID var bucketID = view.DoneBucketID
if bucketID == 0 || t.ProjectID != ot.ProjectID { if bucketID == 0 || !t.Done {
bucketID, err = getDefaultBucketID(s, view) bucketID, err = getDefaultBucketID(s, view)
if err != nil { if err != nil {
return err return err
} }
} }
// Task is done and was moved between projects, should go into the done bucket of the new project tb := &TaskBucket{
if t.Done && t.ProjectID != ot.ProjectID { 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 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{ tb := &TaskBucket{
BucketID: bucketID, BucketID: bucketID,
TaskID: t.ID, TaskID: t.ID,