#1416: Add relative Reminders #1427

Merged
konrad merged 21 commits from ce72/api:1416_reminders into main 2023-03-27 20:07:08 +00:00
2 changed files with 40 additions and 2 deletions
Showing only changes of commit e089976d95 - Show all commits

View File

@ -875,6 +875,33 @@ func (err ErrUserAlreadyAssigned) HTTPError() web.HTTPError {
}
}
// ErrReminderRelativeToMissing represents an error where a task has a relative reminder without reference date
type ErrReminderRelativeToMissing struct {
konrad marked this conversation as resolved
Review

Please add this error to the error.md file in the docs.

Please add this error to the error.md file in the docs.
Review

ok

ok
TaskID int64
}
// IsErrReminderRelativeToMissing checks if an error is ErrReminderRelativeToMissing.
func IsErrReminderRelativeToMissing(err error) bool {
_, ok := err.(ErrReminderRelativeToMissing)
return ok
}
func (err ErrReminderRelativeToMissing) Error() string {
return fmt.Sprintf("Task [TaskID: %v] has a relative reminder without relative_to", err.TaskID)
}
// ErrCodeRelationDoesNotExist holds the unique world-error code of this error
const ErrCodeReminderRelativeToMissing = 4022
// HTTPError holds the http error description
func (err ErrReminderRelativeToMissing) HTTPError() web.HTTPError {
return web.HTTPError{
HTTPCode: http.StatusBadRequest,
Code: ErrCodeReminderRelativeToMissing,
Message: "Relative reminder without relative_to",
konrad marked this conversation as resolved Outdated

Since this is the message that gets shown to the user, please change this to something like `Please provide what the reminder date is relative to

Since this is the message that gets shown to the user, please change this to something like `Please provide what the reminder date is relative to
Outdated
Review

ok

ok
}
}
// =================
// Namespace errors
// =================

View File

@ -1510,7 +1510,7 @@ func (t *Task) overwriteRemindersWithReminderDates(reminderDates []time.Time) {
}
// Set the absolute trigger dates for Reminders with relative period
func updateRelativeReminderDates(task *Task) {
func updateRelativeReminderDates(task *Task) (err error) {
for _, reminder := range task.Reminders {
relativeDuration := time.Duration(reminder.RelativePeriod) * time.Second
switch reminder.RelativeTo {
@ -1526,8 +1526,16 @@ func updateRelativeReminderDates(task *Task) {
if !task.EndDate.IsZero() {
ce72 marked this conversation as resolved Outdated

We should add a validation so that it's not possible to create a relative reminder without telling it what it's relative to (probably not here).

We should add a validation so that it's not possible to create a relative reminder without telling it what it's relative to (probably not here).
Outdated
Review

That would not break anything. There would just be a useless half-of-a-reminder (which maybe could be edited later). Where should we add a validation, if?

That would not break anything. There would just be a useless half-of-a-reminder (which maybe could be edited later). Where should we add a validation, if?

Since there's no dedicated endpoint just for reminders, I think right here is a good place for a validation. I think updateRelativeReminderDates is called when creating or updating a task?

Since there's no dedicated endpoint just for reminders, I think right here is a good place for a validation. I think `updateRelativeReminderDates` is called when creating or updating a task?
Outdated
Review

ok

ok
reminder.Reminder = task.EndDate.Add(relativeDuration)
}
default:
if reminder.RelativePeriod != 0 {
err = ErrReminderRelativeToMissing{
TaskID: task.ID,
}
}
return err
}
}
return nil
}
// Removes all old reminders and adds the new ones. This is a lot easier and less buggy than
@ -1548,7 +1556,10 @@ func (t *Task) updateReminders(s *xorm.Session, task *Task) (err error) {
return
}
updateRelativeReminderDates(task)
err = updateRelativeReminderDates(task)
konrad marked this conversation as resolved Outdated

Please use a pointer to TaskReminder.

Please use a pointer to `TaskReminder`.
Outdated
Review

ok

ok
if err != nil {
return
}
// Resolve duplicates and sort them
reminderMap := make(map[int64]*TaskReminder, len(task.Reminders))