From 6e263b6a9121d1b1d82fe95069a2f175cb7e687c Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Sun, 18 Apr 2021 15:32:02 +0200
Subject: [PATCH] Improve sending overdue task reminders by only sending one
 for all overdue tasks

---
 go.mod                              |  3 ++-
 go.sum                              |  2 ++
 pkg/models/notifications.go         | 34 ++++++++++++++++++++++++
 pkg/models/task_overdue_reminder.go | 41 ++++++++++++++++++++++++-----
 pkg/notifications/mail_render.go    | 40 ++++++++++++++++++++++------
 pkg/notifications/mail_test.go      | 26 +++++++++---------
 6 files changed, 118 insertions(+), 28 deletions(-)

diff --git a/go.mod b/go.mod
index 04d644ef..9c0368b9 100644
--- a/go.mod
+++ b/go.mod
@@ -69,6 +69,7 @@ require (
 	github.com/stretchr/testify v1.7.0
 	github.com/swaggo/swag v1.7.0
 	github.com/ulule/limiter/v3 v3.8.0
+	github.com/yuin/goldmark v1.3.5
 	golang.org/x/crypto v0.0.0-20210415154028-4f45737414dc
 	golang.org/x/image v0.0.0-20210220032944-ac19c3e999fb
 	golang.org/x/oauth2 v0.0.0-20210413134643-5e61552d6c78
@@ -99,4 +100,4 @@ replace (
 	gopkg.in/fsnotify.v1 => github.com/kolaente/fsnotify v1.4.10-0.20200411160148-1bc3c8ff4048 // See https://github.com/fsnotify/fsnotify/issues/328 and https://github.com/golang/go/issues/26904
 )
 
-go 1.13
+go 1.15
diff --git a/go.sum b/go.sum
index aaaf99fc..ccf477cb 100644
--- a/go.sum
+++ b/go.sum
@@ -743,6 +743,8 @@ github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
+github.com/yuin/goldmark v1.3.5 h1:dPmz1Snjq0kmkz159iL7S6WzdahUTHnHB5M56WFVifs=
+github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
 github.com/zenazn/goji v0.9.0/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q=
 github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0=
 go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
diff --git a/pkg/models/notifications.go b/pkg/models/notifications.go
index 1e291a56..77cb8fb6 100644
--- a/pkg/models/notifications.go
+++ b/pkg/models/notifications.go
@@ -214,3 +214,37 @@ func (n *UndoneTaskOverdueNotification) ToDB() interface{} {
 func (n *UndoneTaskOverdueNotification) Name() string {
 	return "task.undone.overdue"
 }
+
+// UndoneTasksOverdueNotification represents a UndoneTasksOverdueNotification notification
+type UndoneTasksOverdueNotification struct {
+	User  *user.User
+	Tasks []*Task
+}
+
+// ToMail returns the mail notification for UndoneTasksOverdueNotification
+func (n *UndoneTasksOverdueNotification) ToMail() *notifications.Mail {
+
+	overdueLine := ""
+	for _, task := range n.Tasks {
+		until := time.Until(task.DueDate).Round(1*time.Hour) * -1
+		overdueLine += `* [` + task.Title + `](` + config.ServiceFrontendurl.GetString() + "tasks/" + strconv.FormatInt(task.ID, 10) + `), overdue since ` + utils.HumanizeDuration(until) + "\n"
+	}
+
+	return notifications.NewMail().
+		Subject(`Your overdue tasks`).
+		Greeting("Hi "+n.User.GetName()+",").
+		Line("You have the following overdue tasks:").
+		Line(overdueLine).
+		Action("Open Vikunja", config.ServiceFrontendurl.GetString()).
+		Line("Have a nice day!")
+}
+
+// ToDB returns the UndoneTasksOverdueNotification notification in a format which can be saved in the db
+func (n *UndoneTasksOverdueNotification) ToDB() interface{} {
+	return nil
+}
+
+// Name returns the name of the notification
+func (n *UndoneTasksOverdueNotification) Name() string {
+	return "task.undone.overdue"
+}
diff --git a/pkg/models/task_overdue_reminder.go b/pkg/models/task_overdue_reminder.go
index 8ab72460..4faa7993 100644
--- a/pkg/models/task_overdue_reminder.go
+++ b/pkg/models/task_overdue_reminder.go
@@ -19,6 +19,8 @@ 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"
@@ -48,6 +50,11 @@ func getUndoneOverdueTasks(s *xorm.Session, now time.Time) (taskIDs []int64, err
 	return
 }
 
+type userWithTasks struct {
+	user  *user.User
+	tasks []*Task
+}
+
 // RegisterOverdueReminderCron registers a function which checks once a day for tasks that are overdue and not done.
 func RegisterOverdueReminderCron() {
 	if !config.ServiceEnableEmailReminders.GetBool() {
@@ -76,21 +83,41 @@ func RegisterOverdueReminderCron() {
 			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))
 
-		for _, u := range users {
-			n := &UndoneTaskOverdueNotification{
-				User: u.User,
-				Task: u.Task,
+		for _, ut := range uts {
+			var n notifications.Notification = &UndoneTasksOverdueNotification{
+				User:  ut.user,
+				Tasks: ut.tasks,
 			}
 
-			err = notifications.Notify(u.User, n)
+			if len(ut.tasks) == 1 {
+				n = &UndoneTaskOverdueNotification{
+					User: ut.user,
+					Task: ut.tasks[0],
+				}
+			}
+
+			err = notifications.Notify(ut.user, n)
 			if err != nil {
-				log.Errorf("[Undone Overdue Tasks Reminder] Could not notify user %d: %s", u.User.ID, err)
+				log.Errorf("[Undone Overdue Tasks Reminder] Could not notify user %d: %s", ut.user.ID, err)
 				return
 			}
 
-			log.Debugf("[Undone Overdue Tasks Reminder] Sent reminder email for task %d to user %d", u.Task.ID, u.User.ID)
+			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/notifications/mail_render.go b/pkg/notifications/mail_render.go
index b45f7b51..0ab72442 100644
--- a/pkg/notifications/mail_render.go
+++ b/pkg/notifications/mail_render.go
@@ -24,6 +24,8 @@ import (
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/api/pkg/mail"
 	"code.vikunja.io/api/pkg/utils"
+
+	"github.com/yuin/goldmark"
 )
 
 const mailTemplatePlain = `
@@ -54,10 +56,8 @@ const mailTemplateHTML = `
 	{{ .Greeting }}
 </p>
 
-{{ range $line := .IntroLines}}
-	<p>
-		{{ $line }}
-	</p>
+{{ range $line := .IntroLinesHTML}}
+	{{ $line }}
 {{ end }}
 
 {{ if .ActionURL }}
@@ -67,10 +67,8 @@ const mailTemplateHTML = `
 	</a>
 {{end}}
 
-{{ range $line := .OutroLines}}
-	<p>
-		{{ $line }}
-	</p>
+{{ range $line := .OutroLinesHTML}}
+	{{ $line }}
 {{ end }}
 
 {{ if .ActionURL }}
@@ -114,6 +112,32 @@ func RenderMail(m *Mail) (mailOpts *mail.Opts, err error) {
 	data["Boundary"] = boundary
 	data["FrontendURL"] = config.ServiceFrontendurl.GetString()
 
+	var introLinesHTML []templatehtml.HTML
+	for _, line := range m.introLines {
+		md := []byte(templatehtml.HTMLEscapeString(line))
+		var buf bytes.Buffer
+		err = goldmark.Convert(md, &buf)
+		if err != nil {
+			return nil, err
+		}
+		//#nosec - the html is escaped few lines before
+		introLinesHTML = append(introLinesHTML, templatehtml.HTML(buf.String()))
+	}
+	data["IntroLinesHTML"] = introLinesHTML
+
+	var outroLinesHTML []templatehtml.HTML
+	for _, line := range m.outroLines {
+		md := []byte(templatehtml.HTMLEscapeString(line))
+		var buf bytes.Buffer
+		err = goldmark.Convert(md, &buf)
+		if err != nil {
+			return nil, err
+		}
+		//#nosec - the html is escaped few lines before
+		outroLinesHTML = append(outroLinesHTML, templatehtml.HTML(buf.String()))
+	}
+	data["OutroLinesHTML"] = outroLinesHTML
+
 	err = plain.Execute(&plainContent, data)
 	if err != nil {
 		return nil, err
diff --git a/pkg/notifications/mail_test.go b/pkg/notifications/mail_test.go
index 57f6e649..a33201d7 100644
--- a/pkg/notifications/mail_test.go
+++ b/pkg/notifications/mail_test.go
@@ -90,6 +90,7 @@ func TestRenderMail(t *testing.T) {
 		Subject("Testmail").
 		Greeting("Hi there,").
 		Line("This is a line").
+		Line("This **line** contains [a link](https://vikunja.io)").
 		Line("And another one").
 		Action("The action", "https://example.com").
 		Line("This should be an outro line").
@@ -105,6 +106,8 @@ Hi there,
 
 This is a line
 
+This **line** contains [a link](https://vikunja.io)
+
 And another one
 
 The action:
@@ -132,13 +135,14 @@ And one more, because why not?
 </p>
 
 
-	<p>
-		This is a line
-	</p>
+	<p>This is a line</p>
+
+
+	<p>This <strong>line</strong> contains <a href="https://vikunja.io">a link</a></p>
+
+
+	<p>And another one</p>
 
-	<p>
-		And another one
-	</p>
 
 
 
@@ -149,13 +153,11 @@ And one more, because why not?
 
 
 
-	<p>
-		This should be an outro line
-	</p>
+	<p>This should be an outro line</p>
+
+
+	<p>And one more, because why not?</p>
 
-	<p>
-		And one more, because why not?
-	</p>