fix(events): only trigger task.updated once when marking task done

Resolves https://github.com/go-vikunja/vikunja/issues/1724

pkg/events/testing.go

@@ -61,3 +61,20 @@ func TestListener(t *testing.T, event Event, listener Listener) {
 	err = listener.Handle(msg)
 	require.NoError(t, err)
 }
+
+// ClearDispatchedEvents clears the list of dispatched test events. This is useful when you want to
+// test event dispatch counts in a specific section of code without events from previous test operations.
+func ClearDispatchedEvents() {
+	dispatchedTestEvents = nil
+}
+
+// CountDispatchedEvents counts how many events of a specific type have been dispatched.
+func CountDispatchedEvents(eventName string) int {
+	count := 0
+	for _, testEvent := range dispatchedTestEvents {
+		if testEvent.Name() == eventName {
+			count++
+		}
+	}
+	return count
+}

pkg/models/kanban_task_bucket.go

@@ -83,23 +83,8 @@ func (b *TaskBucket) upsert(s *xorm.Session) (err error) {
 	return
 }
 
-// Update is the handler to update a task bucket
-// @Summary Update a task bucket
-// @Description Updates a task in a bucket
-// @tags task
-// @Accept json
-// @Produce json
-// @Security JWTKeyAuth
-// @Param project path int true "Project ID"
-// @Param view path int true "Project View ID"
-// @Param bucket path int true "Bucket ID"
-// @Param taskBucket body models.TaskBucket true "The id of the task you want to move into the bucket."
-// @Success 200 {object} models.TaskBucket "The updated task bucket."
-// @Failure 400 {object} web.HTTPError "Invalid task bucket object provided."
-// @Failure 500 {object} models.Message "Internal error"
-// @Router /projects/{project}/views/{view}/buckets/{bucket}/tasks [post]
-func (b *TaskBucket) Update(s *xorm.Session, a web.Auth) (err error) {
-
+// updateTaskBucket is internally used to actually do the update.
+func updateTaskBucket(s *xorm.Session, a web.Auth, b *TaskBucket) (err error) {
 	oldTaskBucket := &TaskBucket{}
 	_, err = s.
 		Where("task_id = ? AND project_view_id = ?", b.TaskID, b.ProjectViewID).
@@ -192,7 +177,7 @@ func (b *TaskBucket) Update(s *xorm.Session, a web.Auth) (err error) {
 
 		err = task.updateReminders(s, task)
 		if err != nil {
-			return err
+			return
 		}
 
 		// Since the done state of the task was changed, we need to move the task into all done buckets everywhere
@@ -230,9 +215,33 @@ func (b *TaskBucket) Update(s *xorm.Session, a web.Auth) (err error) {
 	b.Task = task
 	b.Bucket = bucket
 
+	return
+}
+
+// Update is the handler to update a task bucket
+// @Summary Update a task bucket
+// @Description Updates a task in a bucket
+// @tags task
+// @Accept json
+// @Produce json
+// @Security JWTKeyAuth
+// @Param project path int true "Project ID"
+// @Param view path int true "Project View ID"
+// @Param bucket path int true "Bucket ID"
+// @Param taskBucket body models.TaskBucket true "The id of the task you want to move into the bucket."
+// @Success 200 {object} models.TaskBucket "The updated task bucket."
+// @Failure 400 {object} web.HTTPError "Invalid task bucket object provided."
+// @Failure 500 {object} models.Message "Internal error"
+// @Router /projects/{project}/views/{view}/buckets/{bucket}/tasks [post]
+func (b *TaskBucket) Update(s *xorm.Session, a web.Auth) (err error) {
+	err = updateTaskBucket(s, a, b)
+	if err != nil {
+		return err
+	}
+
 	doer, _ := user.GetFromAuth(a)
 	return events.Dispatch(&TaskUpdatedEvent{
-		Task: task,
+		Task: b.Task,
 		Doer: doer,
 	})
 }

pkg/models/task_position.go

@@ -54,21 +54,9 @@ func (tp *TaskPosition) CanUpdate(s *xorm.Session, a web.Auth) (bool, error) {
 	return t.CanUpdate(s, a)
 }
 
-// Update is the handler to update a task position
-// @Summary Updates a task position
-// @Description Updates a task position.
-// @tags task
-// @Accept json
-// @Produce json
-// @Security JWTKeyAuth
-// @Param id path int true "Task ID"
-// @Param view body models.TaskPosition true "The task position with updated values you want to change."
-// @Success 200 {object} models.TaskPosition "The updated task position."
-// @Failure 400 {object} web.HTTPError "Invalid task position object provided."
-// @Failure 500 {object} models.Message "Internal error"
-// @Router /tasks/{id}/position [post]
-func (tp *TaskPosition) Update(s *xorm.Session, a web.Auth) (err error) {
-
+// updateTaskPosition is the internal function that performs the task position update logic
+// without dispatching events. This is used by moveTaskToDoneBuckets to avoid duplicate events.
+func updateTaskPosition(s *xorm.Session, a web.Auth, tp *TaskPosition) (err error) {
 	// Update all positions if the newly saved position is < 0.1
 	var shouldRecalculate bool
 	var view *ProjectView
@@ -110,6 +98,28 @@ func (tp *TaskPosition) Update(s *xorm.Session, a web.Auth) (err error) {
 		return RecalculateTaskPositions(s, view, a)
 	}
 
+	return nil
+}
+
+// Update is the handler to update a task position
+// @Summary Updates a task position
+// @Description Updates a task position.
+// @tags task
+// @Accept json
+// @Produce json
+// @Security JWTKeyAuth
+// @Param id path int true "Task ID"
+// @Param view body models.TaskPosition true "The task position with updated values you want to change."
+// @Success 200 {object} models.TaskPosition "The updated task position."
+// @Failure 400 {object} web.HTTPError "Invalid task position object provided."
+// @Failure 500 {object} models.Message "Internal error"
+// @Router /tasks/{id}/position [post]
+func (tp *TaskPosition) Update(s *xorm.Session, a web.Auth) (err error) {
+	err = updateTaskPosition(s, a, tp)
+	if err != nil {
+		return err
+	}
+
 	return triggerTaskUpdatedEventForTaskID(s, a, tp.TaskID)
 }
 

pkg/models/tasks.go

@@ -1173,7 +1173,7 @@ func (t *Task) updateSingleTask(s *xorm.Session, a web.Auth, fields []string) (e
 				ProjectViewID: view.ID,
 				ProjectID:     t.ProjectID,
 			}
-			err = tb.Update(s, a)
+			err = updateTaskBucket(s, a, tb)
 			if err != nil {
 				return err
 			}
@@ -1183,7 +1183,7 @@ func (t *Task) updateSingleTask(s *xorm.Session, a web.Auth, fields []string) (e
 				return err
 			}
 
-			err = tp.Update(s, a)
+			err = updateTaskPosition(s, a, tp)
 			if err != nil {
 				return err
 			}
@@ -1404,7 +1404,7 @@ func (t *Task) moveTaskToDoneBuckets(s *xorm.Session, a web.Auth, views []*Proje
 			ProjectViewID: view.ID,
 			ProjectID:     t.ProjectID,
 		}
-		err = tb.Update(s, a)
+		err = updateTaskBucket(s, a, tb)
 		if err != nil {
 			return err
 		}
@@ -1414,7 +1414,7 @@ func (t *Task) moveTaskToDoneBuckets(s *xorm.Session, a web.Auth, views []*Proje
 			ProjectViewID: view.ID,
 			Position:      calculateDefaultPosition(t.Index, t.Position),
 		}
-		err = tp.Update(s, a)
+		err = updateTaskPosition(s, a, &tp)
 		if err != nil {
 			return err
 		}

pkg/models/tasks_test.go

@@ -261,6 +261,27 @@ func TestTask_Update(t *testing.T) {
 			"bucket_id": 3,
 		}, false)
 	})
+	t.Run("marking a task as done should fire exactly ONE task.updated event", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		// Clear any events from previous operations
+		events.ClearDispatchedEvents()
+
+		task := &Task{
+			ID:   1,
+			Done: true,
+		}
+		err := task.Update(s, u)
+		require.NoError(t, err)
+		err = s.Commit()
+		require.NoError(t, err)
+
+		// Verify exactly ONE task.updated event was dispatched
+		count := events.CountDispatchedEvents("task.updated")
+		assert.Equal(t, 1, count, "Expected exactly 1 task.updated event, got %d", count)
+	})
 	t.Run("move task to another project should use the default bucket", func(t *testing.T) {
 		db.LoadAndAssertFixtures(t)
 		s := db.NewSession()