From 7f9e70cc1f64c3683d17f330419e7aa0e3557140 Mon Sep 17 00:00:00 2001
From: ce72 <christoph.ernst72@googlemail.com>
Date: Wed, 8 Mar 2023 15:55:11 +0100
Subject: [PATCH] fix: Review findings

---
 pkg/models/tasks.go | 47 ++++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go
index 9762fb2a8..9f343c8f4 100644
--- a/pkg/models/tasks.go
+++ b/pkg/models/tasks.go
@@ -973,13 +973,8 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err
 		}
 	}
 
-	// Deprecated: the if clause can be removed when ReminderDates will be removed
-	if t.ReminderDates != nil {
-		t.overwriteRemindersWithReminderDates(t.ReminderDates)
-	}
-
 	// Update the reminders
-	if err := t.updateReminders(s, t.Reminders, t.DueDate, t.StartDate, t.EndDate); err != nil {
+	if err := t.updateReminders(s, t); err != nil {
 		return err
 	}
 
@@ -1031,7 +1026,7 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) {
 		t.ListID = ot.ListID
 	}
 
-	// Get the reminders
+	// Get the stored reminders
 	reminders, err := getRemindersForTasks(s, []int64{t.ID})
 	if err != nil {
 		return
@@ -1066,13 +1061,8 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) {
 		return err
 	}
 
-	// Deprecated: This statement can be removed when ReminderDates will be removed
-	if t.ReminderDates != nil {
-		t.overwriteRemindersWithReminderDates(t.ReminderDates)
-	}
-
 	// Update the reminders
-	if err := ot.updateReminders(s, t.Reminders, t.DueDate, t.StartDate, t.EndDate); err != nil {
+	if err := ot.updateReminders(s, t); err != nil {
 		return err
 	}
 
@@ -1520,21 +1510,21 @@ func (t *Task) overwriteRemindersWithReminderDates(reminderDates []time.Time) {
 }
 
 // Set the absolute trigger dates for Reminders with relative period
-func updateRelativeReminderDates(reminders []*TaskReminder, dueDate time.Time, startDate time.Time, endDate time.Time) {
-	for _, reminder := range reminders {
+func updateRelativeReminderDates(task *Task) {
+	for _, reminder := range task.Reminders {
 		relativeDuration := time.Duration(reminder.RelativePeriod) * time.Second
 		switch reminder.RelativeTo {
 		case ReminderRelationDueDate:
-			if !dueDate.IsZero() {
-				reminder.Reminder = dueDate.Add(relativeDuration)
+			if !task.DueDate.IsZero() {
+				reminder.Reminder = task.DueDate.Add(relativeDuration)
 			}
 		case ReminderRelationStartDate:
-			if !startDate.IsZero() {
-				reminder.Reminder = startDate.Add(relativeDuration)
+			if !task.StartDate.IsZero() {
+				reminder.Reminder = task.StartDate.Add(relativeDuration)
 			}
 		case ReminderRelationEndDate:
-			if !endDate.IsZero() {
-				reminder.Reminder = endDate.Add(relativeDuration)
+			if !task.EndDate.IsZero() {
+				reminder.Reminder = task.EndDate.Add(relativeDuration)
 			}
 		}
 	}
@@ -1544,7 +1534,12 @@ func updateRelativeReminderDates(reminders []*TaskReminder, dueDate time.Time, s
 // trying to figure out which reminders changed and then only re-add those needed. And since it does
 // not make a performance difference we'll just do that.
 // The parameter is a slice which holds the new reminders.
-func (t *Task) updateReminders(s *xorm.Session, reminders []*TaskReminder, dueDate time.Time, startDate time.Time, endDate time.Time) (err error) {
+func (t *Task) updateReminders(s *xorm.Session, task *Task) (err error) {
+
+	// Deprecated: This statement must be removed when ReminderDates will be removed
+	if t.ReminderDates != nil {
+		t.overwriteRemindersWithReminderDates(t.ReminderDates)
+	}
 
 	_, err = s.
 		Where("task_id = ?", t.ID).
@@ -1553,12 +1548,12 @@ func (t *Task) updateReminders(s *xorm.Session, reminders []*TaskReminder, dueDa
 		return
 	}
 
-	updateRelativeReminderDates(reminders, dueDate, startDate, endDate)
+	updateRelativeReminderDates(task)
 
 	// Resolve duplicates and sort them
-	reminderMap := make(map[int64]TaskReminder, len(reminders))
-	for _, reminder := range reminders {
-		reminderMap[reminder.Reminder.UTC().Unix()] = *reminder
+	reminderMap := make(map[int64]*TaskReminder, len(task.Reminders))
+	for _, reminder := range task.Reminders {
+		reminderMap[reminder.Reminder.UTC().Unix()] = reminder
 	}
 
 	t.Reminders = make([]*TaskReminder, 0, len(reminderMap))