From 1b9c4204a8e3f72fa5403b015d7acd5fbcfacaa0 Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 24 Aug 2023 10:47:17 +0200 Subject: [PATCH] fix(reminders): make sure reminders are only sent once per user Previously, when a user was creator and assigned to a task, they would get two reminder notifications for the same task. This was caused by Vikunja first fetching all creators and then all assignees and not removing duplicates from that list. Related: https://community.vikunja.io/t/duplicate-email-reminders/1505/3 --- pkg/models/task_reminder.go | 13 +++++++++++++ pkg/models/task_reminder_test.go | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/models/task_reminder.go b/pkg/models/task_reminder.go index 917e37c6e..a6ec05c84 100644 --- a/pkg/models/task_reminder.go +++ b/pkg/models/task_reminder.go @@ -171,6 +171,8 @@ func getTasksWithRemindersDueAndTheirUsers(s *xorm.Session, now time.Time) (remi usersPerTask[ur.Task.ID] = append(usersPerTask[ur.Task.ID], ur) } + seen := make(map[int64]map[int64]bool) + // Time zone cache per time zone string to avoid parsing the same time zone over and over again tzs := make(map[string]*time.Location) // Figure out which reminders are actually due in the time zone of the users @@ -178,6 +180,17 @@ func getTasksWithRemindersDueAndTheirUsers(s *xorm.Session, now time.Time) (remi for _, u := range usersPerTask[r.TaskID] { + // This ensures we send each reminder only once to each user + if seen[r.TaskID] == nil { + seen[r.Ta2skID] = make(map[int64]bool) + } + + if _, exists := seen[r.TaskID][u.User.ID]; exists { + continue + } + + seen[r.TaskID][u.User.ID] = true + if u.User.Timezone == "" { u.User.Timezone = config.GetTimeZone().String() } diff --git a/pkg/models/task_reminder_test.go b/pkg/models/task_reminder_test.go index 12eefe2c2..68190f637 100644 --- a/pkg/models/task_reminder_test.go +++ b/pkg/models/task_reminder_test.go @@ -30,7 +30,7 @@ func TestReminderGetTasksInTheNextMinute(t *testing.T) { s := db.NewSession() defer s.Close() - now, err := time.Parse(time.RFC3339Nano, "2018-12-01T01:13:00Z") + now, err := time.Parse(time.RFC3339Nano, "2018-12-01T01:12:00Z") assert.NoError(t, err) notifications, err := getTasksWithRemindersDueAndTheirUsers(s, now) assert.NoError(t, err)