Put reminders in an extra table (#75)

This commit is contained in:
konrad 2019-05-25 07:33:57 +00:00 committed by Gitea
parent 802a13cffd
commit 24ce940885
11 changed files with 320 additions and 742 deletions

View File

@ -214,7 +214,7 @@ Sorry for some of them being in German, I'll tranlate them at some point.
* [x] Things like list/task order should use queries and not url params
* [x] Fix lint errors
* [x] Add settings for max open/idle connections and max connection lifetime
* [ ] Reminders should use an extra table so we can make reverse lookups aka "give me all tasks with reminders in this period" which we'll need for things like email reminders notifications
* [x] Reminders should use an extra table so we can make reverse lookups aka "give me all tasks with reminders in this period" which we'll need for things like email reminders notifications
* [ ] Teams and users should also have uuids (for users these can be the username)
* [ ] When giving a team or user access to a list/namespace, they should be reffered to by uuid, not numeric id
* [ ] Adding users to a team should also use uuid

1
go.sum
View File

@ -27,6 +27,7 @@ github.com/cockroachdb/apd v1.1.0/go.mod h1:8Sl8LxpKi29FqWXR16WEFZRNSz3SoPzUzeMe
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/cweill/gotests v1.5.3 h1:k3t4wW/x/YNixWZJhUIn+mivmK5iV1tJVOwVYkx0UcU=
github.com/d4l3k/messagediff v1.2.1 h1:ZcAIMYsUg0EAp9X+tt8/enBE/Q8Yd5kzPynLyKptt9U=
github.com/d4l3k/messagediff v1.2.1/go.mod h1:Oozbb1TVXFac9FtSIxHBMnBCq2qeH/2KkEQxENCrlLo=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

View File

@ -80,33 +80,33 @@ func TestListTask(t *testing.T) {
t.Run("by priority", func(t *testing.T) {
rec, err := testHandler.testReadAll(url.Values{"sort": []string{"priority"}}, nil)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `[{"id":3,"text":"task #3 high prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":100,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":0,"username":"","created":0,"updated":0}},{"id":4,"text":"task #4 low prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":1`)
assert.Contains(t, rec.Body.String(), `[{"id":3,"text":"task #3 high prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":100,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}},{"id":4,"text":"task #4 low prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":1`)
})
t.Run("by priority desc", func(t *testing.T) {
rec, err := testHandler.testReadAll(url.Values{"sort": []string{"prioritydesc"}}, nil)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `[{"id":3,"text":"task #3 high prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":100,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":0,"username":"","created":0,"updated":0}},{"id":4,"text":"task #4 low prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":1`)
assert.Contains(t, rec.Body.String(), `[{"id":3,"text":"task #3 high prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":100,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}},{"id":4,"text":"task #4 low prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":1`)
})
t.Run("by priority asc", func(t *testing.T) {
rec, err := testHandler.testReadAll(url.Values{"sort": []string{"priorityasc"}}, nil)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `{"id":4,"text":"task #4 low prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":1,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":0,"username":"","created":0,"updated":0}},{"id":3,"text":"task #3 high prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":100,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":0,"username":"","created":0,"updated":0}}]`)
assert.Contains(t, rec.Body.String(), `{"id":31,"text":"task #31 with color","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"f0f0f0","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}},{"id":4,"text":"task #4 low prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":1,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}},{"id":3,"text":"task #3 high prio","description":"","done":false,"doneAt":0,"dueDate":0,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":100,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}}]`)
})
// should equal duedate desc
t.Run("by duedate", func(t *testing.T) {
rec, err := testHandler.testReadAll(url.Values{"sort": []string{"dueadate"}}, nil)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `[{"id":5,"text":"task #5 higher due date","description":"","done":false,"doneAt":0,"dueDate":1543636724,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":0,"username":"","created":0,"updated":0}},{"id":6,"text":"task #6 lower due date"`)
assert.Contains(t, rec.Body.String(), `[{"id":5,"text":"task #5 higher due date","description":"","done":false,"doneAt":0,"dueDate":1543636724,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}},{"id":6,"text":"task #6 lower due date"`)
})
t.Run("by duedate desc", func(t *testing.T) {
rec, err := testHandler.testReadAll(url.Values{"sort": []string{"dueadatedesc"}}, nil)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `[{"id":5,"text":"task #5 higher due date","description":"","done":false,"doneAt":0,"dueDate":1543636724,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":0,"username":"","created":0,"updated":0}},{"id":6,"text":"task #6 lower due date"`)
assert.Contains(t, rec.Body.String(), `[{"id":5,"text":"task #5 higher due date","description":"","done":false,"doneAt":0,"dueDate":1543636724,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}},{"id":6,"text":"task #6 lower due date"`)
})
t.Run("by duedate asc", func(t *testing.T) {
rec, err := testHandler.testReadAll(url.Values{"sort": []string{"duedateasc"}}, nil)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"doneAt":0,"dueDate":1543616724,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":0,"username":"","created":0,"updated":0}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"doneAt":0,"dueDate":1543636724,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":0,"username":"","created":0,"updated":0}}]`)
assert.Contains(t, rec.Body.String(), `{"id":6,"text":"task #6 lower due date","description":"","done":false,"doneAt":0,"dueDate":1543616724,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}},{"id":5,"text":"task #5 higher due date","description":"","done":false,"doneAt":0,"dueDate":1543636724,"reminderDates":null,"listID":1,"repeatAfter":0,"parentTaskID":0,"priority":0,"startDate":0,"endDate":0,"assignees":null,"labels":null,"hexColor":"","subtasks":null,"created":1543626724,"updated":1543626724,"createdBy":{"id":1,"username":"user1","created":0,"updated":0}}]`)
})
t.Run("invalid parameter", func(t *testing.T) {
// Invalid parameter should not sort at all

View File

@ -0,0 +1,70 @@
// Vikunja is a todo-list application to facilitate your life.
// Copyright 2019 Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
package migration
import (
"github.com/go-xorm/xorm"
"src.techknowlogick.com/xormigrate"
)
type taskReminder20190524205441 struct {
ID int64 `xorm:"int(11) autoincr not null unique pk"`
TaskID int64 `xorm:"int(11) not null INDEX"`
ReminderUnix int64 `xorm:"int(11) not null INDEX"`
Created int64 `xorm:"created not null"`
}
// TableName returns a pretty table name
func (taskReminder20190524205441) TableName() string {
return "task_reminders"
}
func init() {
migrations = append(migrations, &xormigrate.Migration{
ID: "20190524205441",
Description: "Add extra table for reminders",
Migrate: func(tx *xorm.Engine) error {
err := tx.Sync2(taskReminder20190524205441{})
if err != nil {
return err
}
// get all current reminders and put them into the new table
var allTasks []*listTask20190511202210
err = tx.Find(&allTasks)
if err != nil {
return err
}
reminders := []*taskReminder20190524205441{}
for _, t := range allTasks {
for _, reminder := range t.RemindersUnix {
reminders = append(reminders, &taskReminder20190524205441{TaskID: t.ID, ReminderUnix: reminder})
}
}
_, err = tx.Insert(reminders)
if err != nil {
return err
}
return dropTableColum(tx, "tasks", "reminders_unix")
},
Rollback: func(tx *xorm.Engine) error {
return tx.DropTables(taskReminder20190524205441{})
},
})
}

View File

@ -0,0 +1,8 @@
- id: 1
task_id: 27
reminder_unix: 1543626724
created: 1543626724
- id: 2
task_id: 27
reminder_unix: 1543626824
created: 1543626724

View File

@ -167,7 +167,6 @@
- id: 27
text: 'task #27 with reminders'
created_by_id: 1
reminders_unix: '[1543626724,1543626824]'
list_id: 1
created: 1543626724
updated: 1543626724

View File

@ -8,6 +8,7 @@ package models
import (
"code.vikunja.io/web"
"sort"
"time"
)
@ -66,7 +67,7 @@ func (t *ListTask) ReadAll(search string, a web.Auth, page int) (interface{}, er
}
//GetTasksByUser returns all tasks for a user
func GetTasksByUser(search string, u *User, page int, sortby SortBy, startDate time.Time, endDate time.Time) (tasks []*ListTask, err error) {
func GetTasksByUser(search string, u *User, page int, sortby SortBy, startDate time.Time, endDate time.Time) ([]*ListTask, error) {
// Get all lists
lists, err := getRawListsForUser("", u, page)
if err != nil {
@ -91,6 +92,8 @@ func GetTasksByUser(search string, u *User, page int, sortby SortBy, startDate t
orderby = "due_date_unix asc"
}
taskMap := make(map[int64]*ListTask)
// Then return all tasks for that lists
if startDate.Unix() != 0 || endDate.Unix() != 0 {
@ -111,7 +114,7 @@ func GetTasksByUser(search string, u *User, page int, sortby SortBy, startDate t
"(end_date_unix BETWEEN ? and ?))", startDateUnix, endDateUnix, startDateUnix, endDateUnix, startDateUnix, endDateUnix).
And("(parent_task_id = 0 OR parent_task_id IS NULL)").
OrderBy(orderby).
Find(&tasks); err != nil {
Find(&taskMap); err != nil {
return nil, err
}
} else {
@ -119,10 +122,39 @@ func GetTasksByUser(search string, u *User, page int, sortby SortBy, startDate t
Where("text LIKE ?", "%"+search+"%").
And("(parent_task_id = 0 OR parent_task_id IS NULL)").
OrderBy(orderby).
Find(&tasks); err != nil {
Find(&taskMap); err != nil {
return nil, err
}
}
tasks, err := addMoreInfoToTasks(taskMap)
if err != nil {
return nil, err
}
// Because the list is sorted by id which we don't want (since we're dealing with maps)
// we have to manually sort the tasks again here.
sortTasks(tasks, sortby)
return tasks, err
}
func sortTasks(tasks []*ListTask, by SortBy) {
switch by {
case SortTasksByPriorityDesc:
sort.Slice(tasks, func(i, j int) bool {
return tasks[i].Priority > tasks[j].Priority
})
case SortTasksByPriorityAsc:
sort.Slice(tasks, func(i, j int) bool {
return tasks[i].Priority < tasks[j].Priority
})
case SortTasksByDueDateDesc:
sort.Slice(tasks, func(i, j int) bool {
return tasks[i].DueDateUnix > tasks[j].DueDateUnix
})
case SortTasksByDueDateAsc:
sort.Slice(tasks, func(i, j int) bool {
return tasks[i].DueDateUnix < tasks[j].DueDateUnix
})
}
}

File diff suppressed because it is too large Load Diff

View File

@ -36,7 +36,7 @@ type ListTask struct {
// A unix timestamp when the task is due.
DueDateUnix int64 `xorm:"int(11) INDEX null" json:"dueDate"`
// An array of unix timestamps when the user wants to be reminded of the task.
RemindersUnix []int64 `xorm:"JSON TEXT null" json:"reminderDates"`
RemindersUnix []int64 `xorm:"-" json:"reminderDates"`
CreatedByID int64 `xorm:"int(11) not null" json:"-"` // ID of the user who put that task on the list
// The list this task belongs to.
ListID int64 `xorm:"int(11) INDEX not null" json:"listID" param:"list"`
@ -84,6 +84,19 @@ func (ListTask) TableName() string {
return "tasks"
}
// TaskReminder holds a reminder on a task
type TaskReminder struct {
ID int64 `xorm:"int(11) autoincr not null unique pk"`
TaskID int64 `xorm:"int(11) not null INDEX"`
ReminderUnix int64 `xorm:"int(11) not null INDEX"`
Created int64 `xorm:"created not null"`
}
// TableName returns a pretty table name
func (TaskReminder) TableName() string {
return "task_reminders"
}
// GetTasksByListID gets all todotasks for a list
func GetTasksByListID(listID int64) (tasks []*ListTask, err error) {
// make a map so we can put in a lot of other stuff more easily
@ -236,12 +249,27 @@ func addMoreInfoToTasks(taskMap map[int64]*ListTask) (tasks []*ListTask, err err
return
}
// Get all reminders and put them in a map to have it easier later
reminders := []*TaskReminder{}
err = x.Table("task_reminders").In("task_id", taskIDs).Find(&reminders)
if err != nil {
return
}
taskRemindersUnix := make(map[int64][]int64)
for _, r := range reminders {
taskRemindersUnix[r.TaskID] = append(taskRemindersUnix[r.TaskID], r.ReminderUnix)
}
// Add all user objects to the appropriate tasks
for _, task := range taskMap {
// Make created by user objects
taskMap[task.ID].CreatedBy = *users[task.CreatedByID]
// Add the reminders
taskMap[task.ID].RemindersUnix = taskRemindersUnix[task.ID]
// Reorder all subtasks
if task.ParentTaskID != 0 {
taskMap[task.ParentTaskID].Subtasks = append(taskMap[task.ParentTaskID].Subtasks, task)

View File

@ -78,6 +78,11 @@ func (t *ListTask) Create(a web.Auth) (err error) {
return err
}
// Update the reminders
if err := t.updateReminders(t.RemindersUnix); err != nil {
return err
}
metrics.UpdateCount(1, metrics.TaskCountKey)
err = updateListLastUpdated(&List{ID: t.ListID})
@ -118,6 +123,11 @@ func (t *ListTask) Update() (err error) {
return err
}
// Update the reminders
if err := ot.updateReminders(t.RemindersUnix); err != nil {
return err
}
// Update the labels
//
// Maybe FIXME:
@ -162,10 +172,6 @@ func (t *ListTask) Update() (err error) {
if t.DueDateUnix == 0 {
ot.DueDateUnix = 0
}
// Reminders
if len(t.RemindersUnix) == 0 {
ot.RemindersUnix = nil
}
// Repeat after
if t.RepeatAfter == 0 {
ot.RepeatAfter = 0
@ -192,7 +198,6 @@ func (t *ListTask) Update() (err error) {
"description",
"done",
"due_date_unix",
"reminders_unix",
"repeat_after",
"parent_task_id",
"priority",
@ -232,3 +237,79 @@ func updateDone(oldTask *ListTask, newTask *ListTask) {
oldTask.DoneAtUnix = 0
}
}
// Creates or deletes all necessary remindes without unneded db operations.
// The parameter is a slice with unix dates which holds the new reminders.
func (t *ListTask) updateReminders(reminders []int64) (err error) {
// If we're removing everything, delete all reminders right away
if len(reminders) == 0 && len(t.RemindersUnix) > 0 {
_, err = x.Where("task_id = ?", t.ID).
Delete(TaskReminder{})
t.RemindersUnix = nil
return err
}
// If we didn't change anything (from 0 to zero) don't do anything.
if len(reminders) == 0 && len(t.RemindersUnix) == 0 {
return nil
}
// Make a hashmap of the new reminders for easier comparison
newReminders := make(map[int64]*TaskReminder, len(reminders))
for _, newReminder := range reminders {
newReminders[newReminder] = &TaskReminder{ReminderUnix: newReminder}
}
// Get old reminders to delete
var found bool
var remindersToDelete []int64
oldReminders := make(map[int64]*TaskReminder, len(t.RemindersUnix))
for _, oldReminder := range t.RemindersUnix {
found = false
// If a new reminder is already in the list with old reminders
if newReminders[oldReminder] != nil {
found = true
}
// Put all reminders which are only on the old list to the trash
if !found {
remindersToDelete = append(remindersToDelete, oldReminder)
}
oldReminders[oldReminder] = &TaskReminder{ReminderUnix: oldReminder}
}
// Delete all reminders not passed
if len(remindersToDelete) > 0 {
_, err = x.In("reminder_unix", remindersToDelete).
And("task_id = ?", t.ID).
Delete(ListTaskAssginee{})
if err != nil {
return err
}
}
// Loop through our users and add them
for _, r := range reminders {
// Check if the reminder already exists and only inserts it if not
if oldReminders[r] != nil {
// continue outer loop
continue
}
// Add the new reminder
_, err = x.Insert(TaskReminder{TaskID: t.ID, ReminderUnix: r})
if err != nil {
return err
}
}
t.RemindersUnix = reminders
if len(reminders) == 0 {
t.RemindersUnix = nil
}
err = updateListLastUpdated(&List{ID: t.ListID})
return
}

View File

@ -47,6 +47,7 @@ func GetTables() []interface{} {
&ListTaskAssginee{},
&Label{},
&LabelTask{},
&TaskReminder{},
}
}