fix: populate complete entity data in deletion event webhooks (#2135)

Fixes webhook payloads for deletion events that were previously
containing incomplete or empty entity data. This occurred because
entities were being deleted from the database before the webhook event
was dispatched.

## Changes

This PR implements four targeted fixes to ensure complete entity data in
deletion event webhooks:

### 1. TaskAssignee Deletion (`pkg/models/listeners.go`)
- Extended `reloadEventData()` to fetch full assignee user data by ID
- Webhook payload now includes complete user object (username, email,
timestamps, etc.)

### 2. TaskComment Deletion (`pkg/models/task_comments.go`)
- Modified `Delete()` to call `ReadOne()` before deletion
- Ensures comment text, author, and timestamps are included in webhook
payload
- Follows the same pattern used by `Task.Delete()` and
`TaskAttachment.Delete()`

### 3. TaskAttachment Deletion (`pkg/models/task_attachment.go`)
- Extended `ReadOne()` to fetch the `CreatedBy` user
- Webhook payload now includes file creator information

### 4. TaskRelation Deletion (`pkg/models/task_relation.go`)
- Modified `Delete()` to fetch complete relation including `CreatedBy`
user before deletion
- Webhook payload now includes relation timestamps and creator
information

Fixes #2125

pkg/models/listeners.go

@@ -896,65 +896,152 @@ func getProjectIDFromAnyEvent(eventPayload map[string]interface{}) int64 {
 	return 0
 }
 
+func reloadDoerInEvent(s *xorm.Session, event map[string]interface{}) (doerID int64, err error) {
+	doer, has := event["doer"]
+	if !has || doer == nil {
+		return 0, nil
+	}
+
+	// doer can be null in incoming payloads, so guard the type assertion
+	d, ok := doer.(map[string]interface{})
+	if !ok {
+		return 0, nil
+	}
+
+	rawDoerID, has := d["id"]
+	if !has || rawDoerID == nil {
+		return 0, nil
+	}
+
+	doerID = getIDAsInt64(rawDoerID)
+	if doerID <= 0 {
+		return 0, nil
+	}
+
+	fullDoer, err := user.GetUserByID(s, doerID)
+	if err != nil && !user.IsErrUserDoesNotExist(err) {
+		return 0, err
+	}
+	if err == nil {
+		event["doer"] = fullDoer
+	}
+
+	return doerID, nil
+}
+
+func reloadTaskInEvent(s *xorm.Session, event map[string]interface{}, doerID int64) error {
+	task, has := event["task"]
+	if !has || task == nil || doerID == 0 {
+		return nil
+	}
+
+	// guard the type assertion for task as well
+	t, ok := task.(map[string]interface{})
+	if !ok {
+		return nil
+	}
+
+	taskID, has := t["id"]
+	if !has || taskID == nil {
+		return nil
+	}
+
+	id := getIDAsInt64(taskID)
+	if id <= 0 {
+		return nil
+	}
+
+	fullTask := Task{
+		ID: id,
+		Expand: []TaskCollectionExpandable{
+			TaskCollectionExpandBuckets,
+		},
+	}
+	err := fullTask.ReadOne(s, &user.User{ID: doerID})
+	if err != nil && !IsErrTaskDoesNotExist(err) {
+		return err
+	}
+	if err == nil {
+		event["task"] = fullTask
+	}
+
+	return nil
+}
+
+func reloadProjectInEvent(s *xorm.Session, event map[string]interface{}, projectID, doerID int64) error {
+	_, has := event["project"]
+	if !has || doerID == 0 {
+		return nil
+	}
+
+	project, err := GetProjectSimpleByID(s, projectID)
+	if err != nil {
+		if IsErrProjectDoesNotExist(err) {
+			return nil
+		}
+		return err
+	}
+
+	err = project.ReadOne(s, &user.User{ID: doerID})
+	if err != nil && !IsErrProjectDoesNotExist(err) {
+		return err
+	}
+
+	if err == nil {
+		event["project"] = project
+	}
+
+	return nil
+}
+
+func reloadAssigneeInEvent(s *xorm.Session, event map[string]interface{}) error {
+	assignee, has := event["assignee"]
+	if !has || assignee == nil {
+		return nil
+	}
+
+	a, ok := assignee.(map[string]interface{})
+	if !ok {
+		return nil
+	}
+
+	assigneeID := getIDAsInt64(a["id"])
+	if assigneeID <= 0 {
+		return nil
+	}
+
+	fullAssignee, err := user.GetUserByID(s, assigneeID)
+	if err != nil && !user.IsErrUserDoesNotExist(err) {
+		return err
+	}
+	if err == nil {
+		event["assignee"] = fullAssignee
+	}
+
+	return nil
+}
+
 func reloadEventData(s *xorm.Session, event map[string]interface{}, projectID int64) (eventWithData map[string]interface{}, doerID int64, err error) {
 	// Load event data again so that it is always populated in the webhook payload
-	if doer, has := event["doer"]; has && doer != nil {
-		// doer can be null in incoming payloads, so guard the type assertion
-		d, ok := doer.(map[string]interface{})
-		if ok {
-			if rawDoerID, has := d["id"]; has && rawDoerID != nil {
-				doerID = getIDAsInt64(rawDoerID)
-				if doerID > 0 {
-					fullDoer, err := user.GetUserByID(s, doerID)
-					if err != nil && !user.IsErrUserDoesNotExist(err) {
-						return nil, 0, err
-					}
-					if err == nil {
-						event["doer"] = fullDoer
-					}
-				}
-			}
-		}
+
+	doerID, err = reloadDoerInEvent(s, event)
+	if err != nil {
+		return nil, 0, err
 	}
 
-	if task, has := event["task"]; has && task != nil && doerID != 0 {
-		// guard the type assertion for task as well
-		t, ok := task.(map[string]interface{})
-		if ok {
-			if taskID, has := t["id"]; has && taskID != nil {
-				id := getIDAsInt64(taskID)
-				if id > 0 {
-					fullTask := Task{
-						ID: id,
-						Expand: []TaskCollectionExpandable{
-							TaskCollectionExpandBuckets,
-						},
-					}
-					err = fullTask.ReadOne(s, &user.User{ID: doerID})
-					if err != nil && !IsErrTaskDoesNotExist(err) {
-						return
-					}
-					if err == nil {
-						event["task"] = fullTask
-					}
-				}
-			}
-		}
+	err = reloadTaskInEvent(s, event, doerID)
+	if err != nil {
+		return nil, doerID, err
 	}
 
-	if _, has := event["project"]; has && doerID != 0 {
-		var project *Project
-		project, err = GetProjectSimpleByID(s, projectID)
-		if err != nil && !IsErrProjectDoesNotExist(err) {
-			return
-		}
-		err = project.ReadOne(s, &user.User{ID: doerID})
-		if err != nil && !IsErrProjectDoesNotExist(err) {
-			return
-		}
-		if err == nil {
-			event["project"] = project
-		}
+	err = reloadProjectInEvent(s, event, projectID, doerID)
+	if err != nil {
+		return nil, doerID, err
+	}
+
+	err = reloadAssigneeInEvent(s, event)
+	if err != nil {
+		return nil, doerID, err
 	}
 
 	return event, doerID, nil

pkg/models/task_attachment.go

@@ -121,7 +121,17 @@ func (ta *TaskAttachment) ReadOne(s *xorm.Session, _ web.Auth) (err error) {
 	// Get the file
 	ta.File = &files.File{ID: ta.FileID}
 	err = ta.File.LoadFileMetaByID()
-	return
+	if err != nil {
+		return
+	}
+
+	// Get the creator (non-fatal if user doesn't exist)
+	ta.CreatedBy, err = user.GetUserByID(s, ta.CreatedByID)
+	if err != nil && !user.IsErrUserDoesNotExist(err) {
+		return err
+	}
+
+	return nil
 }
 
 // ReadAll returns a project with all attachments

pkg/models/task_comments.go

@@ -118,7 +118,12 @@ func (tc *TaskComment) CreateWithTimestamps(s *xorm.Session, a web.Auth) (err er
 // @Failure 404 {object} web.HTTPError "The task comment was not found."
 // @Failure 500 {object} models.Message "Internal error"
 // @Router /tasks/{taskID}/comments/{commentID} [delete]
-func (tc *TaskComment) Delete(s *xorm.Session, _ web.Auth) error {
+func (tc *TaskComment) Delete(s *xorm.Session, a web.Auth) error {
+	err := tc.ReadOne(s, a)
+	if err != nil {
+		return err
+	}
+
 	deleted, err := s.
 		ID(tc.ID).
 		NoAutoCondition().
@@ -131,6 +136,7 @@ func (tc *TaskComment) Delete(s *xorm.Session, _ web.Auth) error {
 		return err
 	}
 
+	doer, _ := user.GetFromAuth(a)
 	task, err := GetTaskByIDSimple(s, tc.TaskID)
 	if err != nil {
 		return err
@@ -139,7 +145,7 @@ func (tc *TaskComment) Delete(s *xorm.Session, _ web.Auth) error {
 	return events.Dispatch(&TaskCommentDeletedEvent{
 		Task:    &task,
 		Comment: tc,
-		Doer:    tc.Author,
+		Doer:    doer,
 	})
 }
 

pkg/models/task_relation.go

@@ -20,12 +20,11 @@ import (
 	"time"
 
 	"code.vikunja.io/api/pkg/events"
+	"code.vikunja.io/api/pkg/user"
+	"code.vikunja.io/api/pkg/web"
 
 	"xorm.io/builder"
 	"xorm.io/xorm"
-
-	"code.vikunja.io/api/pkg/user"
-	"code.vikunja.io/api/pkg/web"
 )
 
 // RelationKind represents a kind of relation between to tasks
@@ -270,6 +269,31 @@ func (rel *TaskRelation) Create(s *xorm.Session, a web.Auth) error {
 	})
 }
 
+// ReadOne returns a task relation
+func (rel *TaskRelation) ReadOne(s *xorm.Session, _ web.Auth) (err error) {
+	exists, err := s.
+		Where("task_id = ? AND other_task_id = ? AND relation_kind = ?", rel.TaskID, rel.OtherTaskID, rel.RelationKind).
+		Get(rel)
+	if err != nil {
+		return err
+	}
+	if !exists {
+		return ErrRelationDoesNotExist{
+			TaskID:      rel.TaskID,
+			OtherTaskID: rel.OtherTaskID,
+			Kind:        rel.RelationKind,
+		}
+	}
+
+	// Get the creator
+	rel.CreatedBy, err = user.GetUserByID(s, rel.CreatedByID)
+	if err != nil && !user.IsErrUserDoesNotExist(err) {
+		return err
+	}
+
+	return nil
+}
+
 // Delete removes a task relation
 // @Summary Remove a task relation
 // @tags task
@@ -286,7 +310,13 @@ func (rel *TaskRelation) Create(s *xorm.Session, a web.Auth) error {
 // @Failure 500 {object} models.Message "Internal error"
 // @Router /tasks/{taskID}/relations/{relationKind}/{otherTaskID} [delete]
 func (rel *TaskRelation) Delete(s *xorm.Session, a web.Auth) error {
+	// Load the full relation first (populates CreatedBy)
+	err := rel.ReadOne(s, a)
+	if err != nil {
+		return err
+	}
 
+	// Build condition for deleting both the forward and inverse relations
 	cond := builder.Or(
 		builder.And(
 			builder.Eq{"task_id": rel.TaskID},
@@ -300,21 +330,6 @@ func (rel *TaskRelation) Delete(s *xorm.Session, a web.Auth) error {
 		),
 	)
 
-	// Check if the relation exists
-	exists, err := s.
-		Where(cond).
-		Exist(&TaskRelation{})
-	if err != nil {
-		return err
-	}
-	if !exists {
-		return ErrRelationDoesNotExist{
-			TaskID:      rel.TaskID,
-			OtherTaskID: rel.OtherTaskID,
-			Kind:        rel.RelationKind,
-		}
-	}
-
 	_, err = s.
 		Where(cond).
 		Delete(&TaskRelation{})