From 7f5a08b3163a7d2d7b717444df11ae4630803a68 Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 28 Nov 2025 11:06:47 +0100 Subject: [PATCH] fix(tasks): make sure all users get overdue reminder mails (#1901) Fixes a regression introduced in 2a43f9b076f3c84c1c16209c78ffa8b3b5e0f2d2 Resolves https://github.com/go-vikunja/vikunja/issues/1581 --- pkg/models/task_overdue_reminder_test.go | 356 +++++++++++++++++++++++ pkg/models/task_reminder.go | 2 +- 2 files changed, 357 insertions(+), 1 deletion(-) diff --git a/pkg/models/task_overdue_reminder_test.go b/pkg/models/task_overdue_reminder_test.go index 6d2ad3fcb..4a465b03e 100644 --- a/pkg/models/task_overdue_reminder_test.go +++ b/pkg/models/task_overdue_reminder_test.go @@ -21,8 +21,10 @@ import ( "time" "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/user" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "xorm.io/builder" ) func TestGetUndoneOverDueTasks(t *testing.T) { @@ -92,3 +94,357 @@ func TestGetTaskUsersForTasksPermissionFiltering(t *testing.T) { } assert.True(t, hasAssignee) } + +// Tests for issue #1581: Assignees not receiving overdue notifications +func TestOverdueTaskNotificationsForAssignees(t *testing.T) { + t.Run("assignee with direct project share receives overdue notification", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Create an overdue task in project 1 (owned by user 1) + overdueTime, err := time.Parse(time.RFC3339, "2018-11-30T10:00:00Z") + require.NoError(t, err) + + task := &Task{ + Title: "Overdue task assigned to user with direct share", + Done: false, + CreatedByID: 1, // Admin/creator + ProjectID: 1, + DueDate: overdueTime, + } + err = task.Create(s, &user.User{ID: 1}) + require.NoError(t, err) + + // Give user 2 direct read/write access to project 1 + projectUser := &ProjectUser{ + UserID: 2, + ProjectID: 1, + Permission: PermissionWrite, // Read/write access + } + _, err = s.Insert(projectUser) + require.NoError(t, err) + + // Assign task to user 2 + assignee := &TaskAssginee{ + TaskID: task.ID, + UserID: 2, + } + _, err = s.Insert(assignee) + require.NoError(t, err) + + // Subscribe user 2 (simulating auto-subscribe on assignment) + subscription := &Subscription{ + EntityType: SubscriptionEntityTask, + EntityID: task.ID, + UserID: 2, + } + _, err = s.Insert(subscription) + require.NoError(t, err) + + // Get users who should receive overdue notifications + taskUsers, err := getTaskUsersForTasks(s, []int64{task.ID}, builder.Eq{"users.overdue_tasks_reminders_enabled": true}) + require.NoError(t, err) + require.NotEmpty(t, taskUsers, "should have users for overdue task") + + // Verify both creator (user 1) and assignee (user 2) are included + var hasCreator bool + var hasAssignee bool + for _, tu := range taskUsers { + if tu.User.ID == 1 && tu.Task.ID == task.ID { + hasCreator = true + } + if tu.User.ID == 2 && tu.Task.ID == task.ID { + hasAssignee = true + } + } + + assert.True(t, hasCreator, "task creator should receive overdue notification") + assert.True(t, hasAssignee, "assignee with direct project share should receive overdue notification") + }) + + t.Run("assignee with team access receives overdue notification", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Create an overdue task in project 1 (owned by user 1) + overdueTime, err := time.Parse(time.RFC3339, "2018-11-30T10:00:00Z") + require.NoError(t, err) + + task := &Task{ + Title: "Overdue task assigned to user with team access", + Done: false, + CreatedByID: 1, // Admin/creator + ProjectID: 1, + DueDate: overdueTime, + } + err = task.Create(s, &user.User{ID: 1}) + require.NoError(t, err) + + // Create a team and add user 2 to it + team := &Team{ + Name: "Test Team for Issue 1581", + } + _, err = s.Insert(team) + require.NoError(t, err) + + teamMember := &TeamMember{ + TeamID: team.ID, + UserID: 2, + } + _, err = s.Insert(teamMember) + require.NoError(t, err) + + // Share project 1 with the team (read permission) + teamProject := &TeamProject{ + TeamID: team.ID, + ProjectID: 1, + Permission: PermissionRead, + } + _, err = s.Insert(teamProject) + require.NoError(t, err) + + // Assign task to user 2 + assignee := &TaskAssginee{ + TaskID: task.ID, + UserID: 2, + } + _, err = s.Insert(assignee) + require.NoError(t, err) + + // Subscribe user 2 (simulating auto-subscribe on assignment) + subscription := &Subscription{ + EntityType: SubscriptionEntityTask, + EntityID: task.ID, + UserID: 2, + } + _, err = s.Insert(subscription) + require.NoError(t, err) + + // Get users who should receive overdue notifications + taskUsers, err := getTaskUsersForTasks(s, []int64{task.ID}, builder.Eq{"users.overdue_tasks_reminders_enabled": true}) + require.NoError(t, err) + require.NotEmpty(t, taskUsers, "should have users for overdue task") + + // Verify both creator (user 1) and assignee (user 2) are included + var hasCreator bool + var hasAssignee bool + for _, tu := range taskUsers { + if tu.User.ID == 1 && tu.Task.ID == task.ID { + hasCreator = true + } + if tu.User.ID == 2 && tu.Task.ID == task.ID { + hasAssignee = true + } + } + + assert.True(t, hasCreator, "task creator should receive overdue notification") + assert.True(t, hasAssignee, "assignee with team access should receive overdue notification") + }) + + t.Run("unassigned user who lost team access does not receive notification", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Create an overdue task + overdueTime, err := time.Parse(time.RFC3339, "2018-11-30T10:00:00Z") + require.NoError(t, err) + + task := &Task{ + Title: "Overdue task - user lost access", + Done: false, + CreatedByID: 1, + ProjectID: 1, + DueDate: overdueTime, + } + err = task.Create(s, &user.User{ID: 1}) + require.NoError(t, err) + + // User 13 was previously subscribed but no longer has access + subscription := &Subscription{ + EntityType: SubscriptionEntityTask, + EntityID: task.ID, + UserID: 13, // User 13 has no access to project 1 + } + _, err = s.Insert(subscription) + require.NoError(t, err) + + // Get users who should receive overdue notifications + taskUsers, err := getTaskUsersForTasks(s, []int64{task.ID}, builder.Eq{"users.overdue_tasks_reminders_enabled": true}) + require.NoError(t, err) + + // Verify user 13 is NOT included (no access) + var hasUser13 bool + for _, tu := range taskUsers { + if tu.User.ID == 13 && tu.Task.ID == task.ID { + hasUser13 = true + } + } + + assert.False(t, hasUser13, "user without project access should not receive notification") + }) +} + +func TestOverdueTaskNotificationsIncludeSubscribers(t *testing.T) { + t.Run("subscriber with access receives overdue notification", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Create an overdue task in project 1 (owned by user 1) + overdueTime, err := time.Parse(time.RFC3339, "2018-11-30T10:00:00Z") + require.NoError(t, err) + + task := &Task{ + Title: "Overdue task with subscriber", + Done: false, + CreatedByID: 1, + ProjectID: 1, + DueDate: overdueTime, + } + err = task.Create(s, &user.User{ID: 1}) + require.NoError(t, err) + + // Subscribe user 2 to this task + // User 2 needs access to project 1 first - let's give them read access + projectUser := &ProjectUser{ + UserID: 2, + ProjectID: 1, + Permission: PermissionRead, + } + _, err = s.Insert(projectUser) + require.NoError(t, err) + + subscription := &Subscription{ + EntityType: SubscriptionEntityTask, + EntityID: task.ID, + UserID: 2, + } + _, err = s.Insert(subscription) + require.NoError(t, err) + + // Get users who should receive overdue notifications + // Use the same condition as the actual overdue reminder code + taskUsers, err := getTaskUsersForTasks(s, []int64{task.ID}, builder.Eq{"users.overdue_tasks_reminders_enabled": true}) + require.NoError(t, err) + require.NotEmpty(t, taskUsers, "should have users for overdue task") + + // Verify both creator (user 1) and subscriber (user 2) are included + var hasCreator bool + var hasSubscriber bool + for _, tu := range taskUsers { + if tu.User.ID == 1 && tu.Task.ID == task.ID { + hasCreator = true + } + if tu.User.ID == 2 && tu.Task.ID == task.ID { + hasSubscriber = true + } + } + + assert.True(t, hasCreator, "task creator should receive overdue notification") + assert.True(t, hasSubscriber, "task subscriber should receive overdue notification") + }) + + t.Run("subscriber without overdue reminders enabled does not receive notification", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Create an overdue task + overdueTime, err := time.Parse(time.RFC3339, "2018-11-30T10:00:00Z") + require.NoError(t, err) + + task := &Task{ + Title: "Overdue task with subscriber who disabled reminders", + Done: false, + CreatedByID: 1, + ProjectID: 1, + DueDate: overdueTime, + } + err = task.Create(s, &user.User{ID: 1}) + require.NoError(t, err) + + // Give user 2 access to project 1 + projectUser := &ProjectUser{ + UserID: 2, + ProjectID: 1, + Permission: PermissionRead, + } + _, err = s.Insert(projectUser) + require.NoError(t, err) + + // Subscribe user 2 to this task but disable their overdue reminders + subscription := &Subscription{ + EntityType: SubscriptionEntityTask, + EntityID: task.ID, + UserID: 2, + } + _, err = s.Insert(subscription) + require.NoError(t, err) + + // Disable overdue reminders for user 2 + _, err = s.Exec("UPDATE users SET overdue_tasks_reminders_enabled = false WHERE id = ?", 2) + require.NoError(t, err) + + // Get users who should receive overdue notifications + // Use the same condition as the actual overdue reminder code + taskUsers, err := getTaskUsersForTasks(s, []int64{task.ID}, builder.Eq{"users.overdue_tasks_reminders_enabled": true}) + require.NoError(t, err) + + // Verify subscriber (user 2) is NOT included because they disabled reminders + var hasSubscriber bool + for _, tu := range taskUsers { + if tu.User.ID == 2 && tu.Task.ID == task.ID { + hasSubscriber = true + } + } + + assert.False(t, hasSubscriber, "subscriber with overdue reminders disabled should not receive notification") + }) + + t.Run("subscriber without project access does not receive notification", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Create an overdue task + overdueTime, err := time.Parse(time.RFC3339, "2018-11-30T10:00:00Z") + require.NoError(t, err) + + task := &Task{ + Title: "Overdue task with subscriber without access", + Done: false, + CreatedByID: 1, + ProjectID: 1, + DueDate: overdueTime, + } + err = task.Create(s, &user.User{ID: 1}) + require.NoError(t, err) + + // Subscribe user 13 who has NO access to project 1 + subscription := &Subscription{ + EntityType: SubscriptionEntityTask, + EntityID: task.ID, + UserID: 13, + } + _, err = s.Insert(subscription) + require.NoError(t, err) + + // Get users who should receive overdue notifications + // Use the same condition as the actual overdue reminder code + taskUsers, err := getTaskUsersForTasks(s, []int64{task.ID}, builder.Eq{"users.overdue_tasks_reminders_enabled": true}) + require.NoError(t, err) + + // Verify subscriber (user 13) is NOT included because they don't have project access + var hasSubscriber bool + for _, tu := range taskUsers { + if tu.User.ID == 13 && tu.Task.ID == task.ID { + hasSubscriber = true + } + } + + assert.False(t, hasSubscriber, "subscriber without project access should not receive notification") + }) +} diff --git a/pkg/models/task_reminder.go b/pkg/models/task_reminder.go index 9eabefc52..2a44477fe 100644 --- a/pkg/models/task_reminder.go +++ b/pkg/models/task_reminder.go @@ -126,7 +126,7 @@ func getTaskUsersForTasks(s *xorm.Session, taskIDs []int64, cond builder.Cond) ( userProjects[task.ProjectID] = p.isOwner(u) if !p.isOwner(u) { - userProjects[task.ProjectID], _, err = p.checkPermission(s, u, PermissionRead) + userProjects[task.ProjectID], _, err = p.checkPermission(s, u, PermissionRead, PermissionWrite, PermissionAdmin) if err != nil { return err }