diff --git a/cmd/verdande/main.go b/cmd/verdande/main.go index 874a342..edbdea6 100644 --- a/cmd/verdande/main.go +++ b/cmd/verdande/main.go @@ -75,6 +75,7 @@ func run() error { runner := jobs.New(cfg, db, log, api.Mail(), api.Hub()) runner.SyncGmail = api.SyncGmail runner.SyncMailbox = api.SyncMailbox + runner.Push = api.PushToUser runner.Start(jobCtx) srv := &http.Server{ diff --git a/internal/httpapi/notifications_handlers.go b/internal/httpapi/notifications_handlers.go index 891b2a4..299fbb6 100644 --- a/internal/httpapi/notifications_handlers.go +++ b/internal/httpapi/notifications_handlers.go @@ -98,6 +98,12 @@ func (s *Server) notifyProject(r *http.Request, projectID, taskID, actorID, kind } } +// PushToUser is pushToUser for callers outside this package — the reminder job, +// which has the schedule but not the keys. +func (s *Server) PushToUser(userID, title, body, projectID string) { + s.pushToUser(userID, title, body, projectID) +} + func (s *Server) pushToUser(userID, title, body, projectID string) { ctx, cancel := contextWithBackgroundTimeout(30 * time.Second) defer cancel() diff --git a/internal/jobs/jobs.go b/internal/jobs/jobs.go index e5b5c7d..9cc9470 100644 --- a/internal/jobs/jobs.go +++ b/internal/jobs/jobs.go @@ -41,6 +41,10 @@ type Runner struct { // reason as SyncGmail: it owns turning a message into a task. SyncMailbox func(ctx context.Context, user *store.User, m *store.Mailbox) (int, error) + // Push delivers a notification to somebody's devices. Supplied by the HTTP + // layer, which owns the VAPID keys and the subscription list. + Push func(userID, title, body, projectID string) + wg sync.WaitGroup } @@ -126,6 +130,15 @@ func (r *Runner) sendDueReminders(ctx context.Context) error { "due": rem.RemindAt.Format(time.RFC3339), }) + // And to the devices that asked for it. This was missing: reminders went out + // in-app and by mail, so somebody with the tab closed and no mail configured + // got a notification that never arrived — which is the case a reminder is + // for. Notifications from other people's actions did push; only the timed + // ones did not, and the interface said "on for this device" either way. + if r.Push != nil { + r.Push(rem.UserID, rem.TaskContent, reminderBody(rem.RemindAt), rem.ProjectID) + } + if r.mail.Configured() && rem.UserEmail != "" { link := r.cfg.BaseURL + "/projekt/" + rem.ProjectID if err := r.mail.SendReminder(ctx, rem.UserEmail, rem.UserName, rem.TaskContent, link); err != nil { @@ -139,6 +152,12 @@ func (r *Runner) sendDueReminders(ctx context.Context) error { return nil } +// reminderBody is the line under the task's title. The time rather than the date: +// a reminder arrives when it is due, so the day is today and saying so is noise. +func reminderBody(at time.Time) string { + return at.Format("15:04") +} + // --- backups --------------------------------------------------------------------- const ( diff --git a/internal/jobs/jobs_test.go b/internal/jobs/jobs_test.go index 175c941..b9bc830 100644 --- a/internal/jobs/jobs_test.go +++ b/internal/jobs/jobs_test.go @@ -276,3 +276,59 @@ func backupFiles(t *testing.T, cfg *config.Config) []string { } return out } + +// A reminder that nobody receives is not a reminder. +// +// The job sent them in-app and by mail. Somebody with the tab closed and no mail +// server configured — which is the setup the docker one-liner produces — got +// nothing, while the settings page said notifications were on for the device. +// The push half was simply never wired: notifications from other people's actions +// went out, only the timed ones did not. +func TestADueReminderReachesTheDevices(t *testing.T) { + runner, db, _ := newRunner(t) + user := seedUser(t, db) + + type sent struct{ userID, title, projectID string } + var got []sent + runner.Push = func(userID, title, body, projectID string) { + got = append(got, sent{userID, title, projectID}) + } + + inbox, err := db.InboxID(t.Context(), user.ID) + if err != nil { + t.Fatal(err) + } + task := &store.Task{ProjectID: inbox, Content: "ring til Anders", CreatedBy: user.ID} + if err := db.CreateTask(t.Context(), task, nil); err != nil { + t.Fatal(err) + } + // A minute ago, so it is due rather than upcoming. + at := time.Now().Add(-time.Minute) + if _, err := db.CreateReminder(t.Context(), task.ID, user.ID, &at, nil); err != nil { + t.Fatal(err) + } + + if err := runner.sendDueReminders(t.Context()); err != nil { + t.Fatal(err) + } + + if len(got) != 1 { + t.Fatalf("%d reminders reached the devices, want 1", len(got)) + } + if got[0].userID != user.ID { + t.Errorf("it went to %q", got[0].userID) + } + if got[0].title != "ring til Anders" { + t.Errorf("the title is %q, not the task", got[0].title) + } + + // And once. A reminder that arrives on every sweep is worse than one that does + // not arrive at all. + got = nil + if err := runner.sendDueReminders(t.Context()); err != nil { + t.Fatal(err) + } + if len(got) != 0 { + t.Errorf("the same reminder went out %d more times", len(got)) + } +}