From 7eb3b96a4465ca6648572b07c506c06f2c28c375 Mon Sep 17 00:00:00 2001 From: kolaente Date: Sun, 12 Jun 2022 21:24:16 +0200 Subject: [PATCH] feat: send overdue tasks email notification at 9:00 in the user's time zone --- pkg/models/task_overdue_reminder.go | 85 ++++++++++++++++-------- pkg/models/task_overdue_reminder_test.go | 29 +++++--- 2 files changed, 78 insertions(+), 36 deletions(-) diff --git a/pkg/models/task_overdue_reminder.go b/pkg/models/task_overdue_reminder.go index 4faa79930..6d1b7b445 100644 --- a/pkg/models/task_overdue_reminder.go +++ b/pkg/models/task_overdue_reminder.go @@ -19,35 +19,83 @@ package models import ( "time" - "code.vikunja.io/api/pkg/user" - "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/cron" "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/notifications" + "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/utils" + "xorm.io/builder" "xorm.io/xorm" ) -func getUndoneOverdueTasks(s *xorm.Session, now time.Time) (taskIDs []int64, err error) { +func getUndoneOverdueTasks(s *xorm.Session, now time.Time) (usersWithTasks map[int64]*userWithTasks, err error) { now = utils.GetTimeWithoutNanoSeconds(now) + nextMinute := now.Add(1 * time.Minute) var tasks []*Task err = s. - Where("due_date is not null and due_date < ?", now.Format(dbTimeFormat)). + Where("due_date is not null and due_date < ?", nextMinute.Add(time.Hour*14).Format(dbTimeFormat)). And("done = false"). Find(&tasks) if err != nil { return } + if len(tasks) == 0 { + return + } + + var taskIDs []int64 for _, task := range tasks { taskIDs = append(taskIDs, task.ID) } - return + users, err := getTaskUsersForTasks(s, taskIDs, builder.Eq{"users.overdue_tasks_reminders_enabled": true}) + if err != nil { + return + } + + if len(users) == 0 { + return + } + + uts := make(map[int64]*userWithTasks) + tzs := make(map[string]*time.Location) + for _, t := range users { + if t.User.Timezone == "" { + t.User.Timezone = config.GetTimeZone().String() + } + + tz, exists := tzs[t.User.Timezone] + if !exists { + tz, err = time.LoadLocation(t.User.Timezone) + if err != nil { + return + } + tzs[t.User.Timezone] = tz + } + + // If it is 9:00 for that current user, add the task to their list of overdue tasks + overdueMailTime := time.Date(now.Year(), now.Month(), now.Day(), 9, 0, 0, 0, tz) + isTimeForReminder := overdueMailTime.After(now) || overdueMailTime.Equal(now.In(tz)) + wasTimeForReminder := overdueMailTime.Before(now.Add(time.Minute)) + taskIsOverdueInUserTimezone := overdueMailTime.After(t.Task.DueDate.In(tz)) + if isTimeForReminder && wasTimeForReminder && taskIsOverdueInUserTimezone { + _, exists := uts[t.User.ID] + if !exists { + uts[t.User.ID] = &userWithTasks{ + user: t.User, + tasks: []*Task{}, + } + } + uts[t.User.ID].tasks = append(uts[t.User.ID].tasks, t.Task) + } + } + + return uts, nil } type userWithTasks struct { @@ -66,36 +114,18 @@ func RegisterOverdueReminderCron() { return } - err := cron.Schedule("0 8 * * *", func() { + err := cron.Schedule("* * * * *", func() { s := db.NewSession() defer s.Close() now := time.Now() - taskIDs, err := getUndoneOverdueTasks(s, now) + uts, err := getUndoneOverdueTasks(s, now) if err != nil { - log.Errorf("[Undone Overdue Tasks Reminder] Could not get tasks with reminders in the next minute: %s", err) + log.Errorf("[Undone Overdue Tasks Reminder] Could not get undone overdue tasks in the next minute: %s", err) return } - users, err := getTaskUsersForTasks(s, taskIDs, builder.Eq{"users.overdue_tasks_reminders_enabled": true}) - if err != nil { - log.Errorf("[Undone Overdue Tasks Reminder] Could not get task users to send them reminders: %s", err) - return - } - - uts := make(map[int64]*userWithTasks) - for _, t := range users { - _, exists := uts[t.User.ID] - if !exists { - uts[t.User.ID] = &userWithTasks{ - user: t.User, - tasks: []*Task{}, - } - } - uts[t.User.ID].tasks = append(uts[t.User.ID].tasks, t.Task) - } - - log.Debugf("[Undone Overdue Tasks Reminder] Sending reminders to %d users", len(users)) + log.Debugf("[Undone Overdue Tasks Reminder] Sending reminders to %d users", len(uts)) for _, ut := range uts { var n notifications.Notification = &UndoneTasksOverdueNotification{ @@ -117,7 +147,6 @@ func RegisterOverdueReminderCron() { } log.Debugf("[Undone Overdue Tasks Reminder] Sent reminder email for %d tasks to user %d", len(ut.tasks), ut.user.ID) - continue } }) if err != nil { diff --git a/pkg/models/task_overdue_reminder_test.go b/pkg/models/task_overdue_reminder_test.go index 9845ed6fa..b1ca24ede 100644 --- a/pkg/models/task_overdue_reminder_test.go +++ b/pkg/models/task_overdue_reminder_test.go @@ -32,21 +32,34 @@ func TestGetUndoneOverDueTasks(t *testing.T) { now, err := time.Parse(time.RFC3339Nano, "2018-01-01T01:13:00Z") assert.NoError(t, err) - taskIDs, err := getUndoneOverdueTasks(s, now) + tasks, err := getUndoneOverdueTasks(s, now) assert.NoError(t, err) - assert.Len(t, taskIDs, 0) + assert.Len(t, tasks, 0) }) t.Run("undone overdue", func(t *testing.T) { db.LoadAndAssertFixtures(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-01T09:00:00Z") assert.NoError(t, err) - taskIDs, err := getUndoneOverdueTasks(s, now) + uts, err := getUndoneOverdueTasks(s, now) assert.NoError(t, err) - assert.Len(t, taskIDs, 1) - assert.Equal(t, int64(6), taskIDs[0]) + assert.Len(t, uts, 1) + assert.Len(t, uts[1].tasks, 2) + // The tasks don't always have the same order, so we only check their presence, not their position. + var task5Present bool + var task6Present bool + for _, t := range uts[1].tasks { + if t.ID == 5 { + task5Present = true + } + if t.ID == 6 { + task6Present = true + } + } + assert.Truef(t, task5Present, "expected task 5 to be present but was not") + assert.Truef(t, task6Present, "expected task 6 to be present but was not") }) t.Run("done overdue", func(t *testing.T) { db.LoadAndAssertFixtures(t) @@ -55,8 +68,8 @@ func TestGetUndoneOverDueTasks(t *testing.T) { now, err := time.Parse(time.RFC3339Nano, "2018-11-01T01:13:00Z") assert.NoError(t, err) - taskIDs, err := getUndoneOverdueTasks(s, now) + tasks, err := getUndoneOverdueTasks(s, now) assert.NoError(t, err) - assert.Len(t, taskIDs, 0) + assert.Len(t, tasks, 0) }) }