From e5cec4bd163548b0596311c2a6e710a3667c4d14 Mon Sep 17 00:00:00 2001 From: ce72 Date: Tue, 7 Mar 2023 21:24:47 +0100 Subject: [PATCH] #1416: Step 4 Update reminder time in relative reminders --- pkg/models/task_reminder.go | 6 +- pkg/models/tasks.go | 30 +++++- pkg/models/tasks_test.go | 119 +++++++++++++++------ pkg/modules/migration/ticktick/ticktick.go | 2 +- pkg/swagger/docs.go | 6 +- pkg/swagger/swagger.json | 6 +- pkg/swagger/swagger.yaml | 6 +- 7 files changed, 125 insertions(+), 50 deletions(-) diff --git a/pkg/models/task_reminder.go b/pkg/models/task_reminder.go index dba9c3cb4..0f0e70c28 100644 --- a/pkg/models/task_reminder.go +++ b/pkg/models/task_reminder.go @@ -38,9 +38,9 @@ type ReminderRelation string // All valid ReminderRelations const ( - DueDate ReminderRelation = `due_date` - StartDate ReminderRelation = `start_date` - EndDate ReminderRelation = `end_date` + ReminderDueDate ReminderRelation = `due_date` + ReminderStartDate ReminderRelation = `start_date` + ReminderEndDate ReminderRelation = `end_date` ) // TaskReminder holds a reminder on a task diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index 83a413037..6f43d5e2c 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -979,7 +979,7 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err } // Update the reminders - if err := t.updateReminders(s, t.Reminders); err != nil { + if err := t.updateReminders(s, t.Reminders, t.DueDate, t.StartDate, t.EndDate); err != nil { return err } @@ -1072,7 +1072,7 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) { } // Update the reminders - if err := ot.updateReminders(s, t.Reminders); err != nil { + if err := ot.updateReminders(s, t.Reminders, t.DueDate, t.StartDate, t.EndDate); err != nil { return err } @@ -1525,8 +1525,8 @@ func (t *Task) updateRemindersFromReminderDates(reminderDates []time.Time) { // Removes all old reminders and adds the new ones. This is a lot easier and less buggy than // 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 with unix dates which holds the new reminders. -func (t *Task) updateReminders(s *xorm.Session, reminders []*TaskReminder) (err error) { +// 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) { _, err = s. Where("task_id = ?", t.ID). @@ -1535,6 +1535,8 @@ func (t *Task) updateReminders(s *xorm.Session, reminders []*TaskReminder) (err return } + setRelativeReminderDates(reminders, dueDate, startDate, endDate) + // Resolve duplicates and sort them reminderMap := make(map[int64]TaskReminder, len(reminders)) for _, reminder := range reminders { @@ -1564,6 +1566,26 @@ func (t *Task) updateReminders(s *xorm.Session, reminders []*TaskReminder) (err return } +func setRelativeReminderDates(reminders []*TaskReminder, dueDate time.Time, startDate time.Time, endDate time.Time) { + for _, reminder := range reminders { + relativeDuration := time.Duration(reminder.RelativePeriod) * time.Second + switch reminder.RelativeTo { + case ReminderDueDate: + if !dueDate.IsZero() { + reminder.Reminder = dueDate.Add(relativeDuration) + } + case ReminderStartDate: + if !startDate.IsZero() { + reminder.Reminder = startDate.Add(relativeDuration) + } + case ReminderEndDate: + if !endDate.IsZero() { + reminder.Reminder = endDate.Add(relativeDuration) + } + } + } +} + func updateTaskLastUpdated(s *xorm.Session, task *Task) error { _, err := s.ID(task.ID).Cols("updated").Update(task) return err diff --git a/pkg/models/tasks_test.go b/pkg/models/tasks_test.go index 65c0eb3eb..06318197f 100644 --- a/pkg/models/tasks_test.go +++ b/pkg/models/tasks_test.go @@ -70,6 +70,44 @@ func TestTask_Create(t *testing.T) { events.AssertDispatched(t, &TaskCreatedEvent{}) }) + t.Run("with reminders", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + task := &Task{ + Title: "Lorem", + Description: "Lorem Ipsum Dolor", + ListID: 1, + DueDate: time.Unix(1550000000, 0), + StartDate: time.Unix(1550000010, 0), + EndDate: time.Unix(1550000020, 0), + Reminders: []*TaskReminder{ + { + RelativeTo: "due_date", + RelativePeriod: 1, + }, + { + RelativeTo: "start_date", + RelativePeriod: -2, + }, + { + RelativeTo: "end_date", + RelativePeriod: -1, + }, + { + Reminder: time.Unix(1550000020, 0), + }, + }} + err := task.Create(s, usr) + assert.NoError(t, err) + assert.Equal(t, task.Reminders[0].Reminder, time.Unix(1550000001, 0)) + assert.Equal(t, task.Reminders[1].Reminder, time.Unix(1550000008, 0)) + assert.Equal(t, task.Reminders[2].Reminder, time.Unix(1550000019, 0)) + assert.Equal(t, task.Reminders[3].Reminder, time.Unix(1550000020, 0)) + err = s.Commit() + assert.NoError(t, err) + }) t.Run("empty title", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() @@ -369,8 +407,47 @@ func TestTask_Update(t *testing.T) { assert.Equal(t, int64(3), task.Index) }) - // Deprecated: see next test - t.Run("the same reminder date multiple times should be saved once", func(t *testing.T) { + t.Run("reminders will be updated", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + task := &Task{ + ID: 1, + ListID: 1, + Title: "test", + DueDate: time.Unix(1550000000, 0), + StartDate: time.Unix(1550000010, 0), + EndDate: time.Unix(1550000020, 0), + Reminders: []*TaskReminder{ + { + RelativeTo: "due_date", + RelativePeriod: 1, + }, + { + RelativeTo: "start_date", + RelativePeriod: -2, + }, + { + RelativeTo: "end_date", + RelativePeriod: -1, + }, + { + Reminder: time.Unix(1550000020, 0), + }, + }, + } + err := task.Update(s, u) + assert.NoError(t, err) + assert.Equal(t, task.Reminders[0].Reminder, time.Unix(1550000001, 0)) + assert.Equal(t, task.Reminders[1].Reminder, time.Unix(1550000008, 0)) + assert.Equal(t, task.Reminders[2].Reminder, time.Unix(1550000019, 0)) + assert.Equal(t, task.Reminders[3].Reminder, time.Unix(1550000020, 0)) + err = s.Commit() + assert.NoError(t, err) + db.AssertCount(t, "task_reminders", builder.Eq{"task_id": 1}, 4) + }) + t.Run("the same reminder multiple times should be saved once", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() defer s.Close() @@ -378,9 +455,13 @@ func TestTask_Update(t *testing.T) { task := &Task{ ID: 1, Title: "test", - ReminderDates: []time.Time{ - time.Unix(1674745156, 0), - time.Unix(1674745156, 223), + Reminders: []*TaskReminder{ + { + Reminder: time.Unix(1674745156, 0), + }, + { + Reminder: time.Unix(1674745156, 223), + }, }, ListID: 1, } @@ -388,34 +469,6 @@ func TestTask_Update(t *testing.T) { assert.NoError(t, err) err = s.Commit() assert.NoError(t, err) - - db.AssertCount(t, "task_reminders", builder.Eq{"task_id": 1}, 1) - }) - - t.Run("the same reminder multiple times should be saved once", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - taskReminders := []*TaskReminder{ - { - Reminder: time.Unix(1674745156, 0), - }, - { - Reminder: time.Unix(1674745156, 223), - }, - } - - task := &Task{ - ID: 1, - Title: "test", - Reminders: taskReminders, - ListID: 1, - } - err := task.Update(s, u) - assert.NoError(t, err) - err = s.Commit() - assert.NoError(t, err) db.AssertCount(t, "task_reminders", builder.Eq{"task_id": 1}, 1) }) } diff --git a/pkg/modules/migration/ticktick/ticktick.go b/pkg/modules/migration/ticktick/ticktick.go index 91bbf9972..8036ffa88 100644 --- a/pkg/modules/migration/ticktick/ticktick.go +++ b/pkg/modules/migration/ticktick/ticktick.go @@ -149,7 +149,7 @@ func convertTickTickToVikunja(tasks []*tickTickTask) (result []*models.Namespace if !t.DueDate.IsZero() && t.Reminder > 0 { task.Task.Reminders = []*models.TaskReminder{ { - RelativeTo: models.DueDate, + RelativeTo: models.ReminderDueDate, RelativePeriod: int64((t.Reminder * -1).Seconds()), }, } diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index 7d70fee0c..728221ea7 100644 --- a/pkg/swagger/docs.go +++ b/pkg/swagger/docs.go @@ -8317,9 +8317,9 @@ const docTemplate = `{ "end_date" ], "x-enum-varnames": [ - "DueDate", - "StartDate", - "EndDate" + "ReminderDueDate", + "ReminderStartDate", + "ReminderEndDate" ] }, "models.Right": { diff --git a/pkg/swagger/swagger.json b/pkg/swagger/swagger.json index 0e09adecc..773189696 100644 --- a/pkg/swagger/swagger.json +++ b/pkg/swagger/swagger.json @@ -8309,9 +8309,9 @@ "end_date" ], "x-enum-varnames": [ - "DueDate", - "StartDate", - "EndDate" + "ReminderDueDate", + "ReminderStartDate", + "ReminderEndDate" ] }, "models.Right": { diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index 8dbd01881..a7b1a80e5 100644 --- a/pkg/swagger/swagger.yaml +++ b/pkg/swagger/swagger.yaml @@ -609,9 +609,9 @@ definitions: - end_date type: string x-enum-varnames: - - DueDate - - StartDate - - EndDate + - ReminderDueDate + - ReminderStartDate + - ReminderEndDate models.Right: enum: - 0