fix(tasks): make sure all users get overdue reminder mails (#1901)

Fixes a regression introduced in 2a43f9b076 Resolves https://github.com/go-vikunja/vikunja/issues/1581
2025-12-05 19:16:51 -06:00 · 2025-11-28 11:06:47 +01:00
parent d1add5c621
commit 7f5a08b316
2 changed files with 357 additions and 1 deletions
--- 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")
+	})
+}
--- 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
 				}