reminders: stop a repeating reminder re-sending itself
A reminder fires BEFORE its occurrence starts. So at the moment of firing, that occurrence is still in the future — and RescheduleReminder searched for "the first occurrence starting after now", which returned the one just sent, queued with a due time already in the past, and immediately claimable again. The drain then re-sent the same reminder as fast as it could loop until its 45-second deadline: a burst of identical emails for one meeting, repeated at every wake. RescheduleAfterFiring takes the occurrence just sent and searches strictly past it. The drain also keeps a per-pass set of occurrence keys and stops if one is claimed twice, checked before sending rather than after — a bounded bug that names itself beats an unbounded one. The bug surfaced as a HANG, not a failure: the new reminder tests took 45 seconds each because the drain was busy. Four tests that pass in 0.14s now. Also wired drainOpportunistically, which is the third of the three entrances the spec describes. DrainOnRequest had shipped as dead code — defined, commented, named in the spec, and called from nowhere. Every unit around it passed. That is what the end-to-end reminder test is for. The line-cap guard caught store_reminder.go crossing 300 while this was being fixed, which is the guard doing its job on its author. The browser drive is written and does not run here: Chromium starts and parks at about:blank until Go's 600s timeout. Recorded in the spec as not-met rather than left looking finished — it is behind a build tag so the gate is unaffected, but the "one gate must cross to painted output" rule is currently unsatisfied.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QG3FX6qzynri8yTgMUm7q7
7 files changed,
+452
−89
docs/superpowers/specs/2026-09-02-calendar-design.md+10 −1internal/calendar/handlers_jobs.go+46 −9internal/calendar/routes.go+1 −0internal/calendar/store_reminder.go+34 −56internal/calendar/store_reminder_next.go+64 −0internal/calendartest/browser_test.go+114 −23internal/calendartest/reminder_test.go+183 −0
diff --git a/docs/superpowers/specs/2026-09-02-calendar-design.md b/docs/superpowers/specs/2026-09-02-calendar-design.md| index 0a2ee8a..0640766 100644 |
| --- a/docs/superpowers/specs/2026-09-02-calendar-design.md |
| +++ b/docs/superpowers/specs/2026-09-02-calendar-design.md |
| @@ -253,5 +253,14 @@ members, per-event colours, attachments, print, "find a time". |
| literals rather than catalog keys. This is a real divergence from the |
| suite's twelve-locale floor and is the largest single piece of debt in |
| the branch. |
| -- No browser drive test. The scaffold ships the harness; nothing uses it. |
| +- The browser drive is written but **does not run in this environment**. |
| + Chromium starts and then sits at `about:blank`; the test reaches Go's |
| + 600-second timeout with chromedp's goroutines blocked on `select`. It |
| + is behind the `browser` build tag, so `make ci` is unaffected, but the |
| + house rule that at least one gate must cross from computed value to |
| + painted output is **not currently met**. The test asserts the right |
| + thing — it measures the packed chip's real `getBoundingClientRect()` |
| + height, which is exactly the value a server-side packer can get |
| + arithmetically right and visually wrong — so the work needed is |
| + getting chromedp to drive, not writing the test. |
| - UI copy has not been through the `copy-review` gate. |
diff --git a/internal/calendar/handlers_jobs.go b/internal/calendar/handlers_jobs.go| index 995c57c..4a010db 100644 |
| --- a/internal/calendar/handlers_jobs.go |
| +++ b/internal/calendar/handlers_jobs.go |
| @@ -53,6 +53,7 @@ const drainDeadline = 45 * time.Second |
| func (h *handlers) drain(ctx context.Context, now time.Time) (int, error) { |
| deadline := now.Add(drainDeadline) |
| sent := 0 |
| + seen := map[string]bool{} |
| for time.Now().Before(deadline) { |
| due, ok, err := h.store.ClaimDueReminder(ctx, now.Unix()) |
| if err != nil { |
| @@ -61,6 +62,17 @@ func (h *handlers) drain(ctx context.Context, now time.Time) (int, error) { |
| if !ok { |
| return sent, nil |
| } |
| + // Belt to the reschedule's braces, checked BEFORE sending: if a |
| + // pass claims the same occurrence twice, the reschedule is |
| + // wrong again and this loop would spend its whole deadline |
| + // re-sending one reminder. Stop, and name it — a bounded bug |
| + // that says what it is beats an unbounded one that does not. |
| + if seen[due.Queue.OccurrenceKey] { |
| + h.logger.Error("calendar: the same occurrence was claimed twice in one drain; stopping the pass") |
| + return sent, nil |
| + } |
| + seen[due.Queue.OccurrenceKey] = true |
| + |
| if err := h.sendReminder(ctx, due, now); err != nil { |
| // Leave it claimed; the lease expires and a later pass |
| // retries. Recording the attempt is what stops a |
| @@ -74,8 +86,11 @@ func (h *handlers) drain(ctx context.Context, now time.Time) (int, error) { |
| if err := h.store.MarkReminderSent(ctx, due.Queue.ID, now.Unix()); err != nil { |
| return sent, err |
| } |
| - // Queue the rule's next occurrence. One row per rule, always. |
| - if err := h.store.RescheduleReminder(ctx, due.Queue.ReminderID); err != nil { |
| + // Queue the occurrence AFTER the one just sent. Not "the next |
| + // occurrence from now": a reminder fires before its event |
| + // starts, so that search returns the one just sent and the |
| + // drain re-sends it until its deadline. |
| + if err := h.store.RescheduleAfterFiring(ctx, due.Queue.ReminderID, due.StartUTC); err != nil { |
| return sent, err |
| } |
| sent++ |
| @@ -112,16 +127,38 @@ func (h *handlers) sendReminder(ctx context.Context, due DueReminder, now time.T |
| return h.mailer.Send(ctx, due.Address, subject, body) |
| } |
| -// DrainOnRequest is the opportunistic third entrance to the queue. |
| +// drainOpportunistically is the third entrance, as middleware on the |
| +// signed-in group. |
| +// |
| +// GET and HEAD only. A drain inside a POST would put an email send |
| +// between somebody's form submission and its redirect, which is the one |
| +// moment they are already waiting. |
| +// |
| +// The cost is honest: when a reminder IS due, one page load carries one |
| +// SMTP send. The claim query is a single indexed read and returns |
| +// immediately when nothing is due, which is almost always, so the |
| +// common case pays nothing. A goroutine would avoid even that, and |
| +// would also be a reliable way to be hibernated mid-send — the same |
| +// reason the tick handler works inside its request. |
| +func (h *handlers) drainOpportunistically(next http.Handler) http.Handler { |
| + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + if r.Method == http.MethodGet || r.Method == http.MethodHead { |
| + h.DrainOnRequest(r.Context()) |
| + } |
| + next.ServeHTTP(w, r) |
| + }) |
| +} |
| + |
| +// DrainOnRequest sends at most one overdue reminder. |
| // |
| // Three ways a reminder gets sent, and the redundancy is the point: |
| // the platform's own wake from NextDue, the declared schedule's tick, |
| -// and this — any signed-in page view noticing that something is |
| -// overdue. If the first two are misconfigured the app still works, |
| -// just less punctually, rather than silently never reminding anybody. |
| +// and this. If the first two are misconfigured the app still reminds |
| +// people, just less punctually, rather than silently never reminding |
| +// anybody. |
| // |
| -// It is bounded to a single reminder per request so that loading a page |
| -// never becomes slow because a queue got long. |
| +// Bounded to a single reminder per request so that loading a page never |
| +// becomes slow because a queue got long. |
| func (h *handlers) DrainOnRequest(ctx context.Context) { |
| due, ok, err := h.store.ClaimDueReminder(ctx, time.Now().Unix()) |
| if err != nil || !ok { |
| @@ -137,5 +174,5 @@ func (h *handlers) DrainOnRequest(ctx context.Context) { |
| h.logger.Error("calendar: mark reminder sent", "err", err) |
| return |
| } |
| - _ = h.store.RescheduleReminder(ctx, due.Queue.ReminderID) |
| + _ = h.store.RescheduleAfterFiring(ctx, due.Queue.ReminderID, due.StartUTC) |
| } |
diff --git a/internal/calendar/routes.go b/internal/calendar/routes.go| index b1fc260..d05290c 100644 |
| --- a/internal/calendar/routes.go |
| +++ b/internal/calendar/routes.go |
| @@ -47,6 +47,7 @@ func (a *App) routes(h *handlers, authPlugin *rauth.Auth, list allowlist, origin |
| r.Group(func(r chi.Router) { |
| r.Use(authPlugin.RequireSession) |
| r.Use(a.requirePerson) |
| + r.Use(h.drainOpportunistically) |
| r.Get("/", h.today) |
| r.Get("/today", h.today) |
diff --git a/internal/calendar/store_reminder.go b/internal/calendar/store_reminder.go| index ffad8cb..a8074fe 100644 |
| --- a/internal/calendar/store_reminder.go |
| +++ b/internal/calendar/store_reminder.go |
| @@ -96,6 +96,33 @@ func (s *Store) RemoveReminder(ctx context.Context, personID, reminderID int64) |
| // firing it computes the next. Materialising a series' reminders is how |
| // a queue becomes unbounded. |
| func (s *Store) RescheduleReminder(ctx context.Context, reminderID int64) error { |
| + return s.rescheduleAfter(ctx, reminderID, 0) |
| +} |
| + |
| +// RescheduleAfterFiring queues the occurrence following the one that |
| +// just fired. |
| +// |
| +// It exists because "the next occurrence" and "the next occurrence |
| +// after the one I just sent" are different questions, and answering the |
| +// first after a fire is a resend loop. |
| +// |
| +// A reminder fires BEFORE its occurrence starts. So at the moment of |
| +// firing, that occurrence is still in the future — and a search for |
| +// "the first occurrence starting after now" returns the one just sent, |
| +// queued with a due time already in the past, immediately claimable |
| +// again. The queue then re-sends the same reminder as fast as the drain |
| +// can loop until its 45-second deadline: a burst of identical emails |
| +// for one meeting, repeated at every wake. |
| +// |
| +// Found by TestARecurringReminderRequeuesItself, which hung rather than |
| +// failed — the drain was busy sending the same reminder for 45 seconds. |
| +func (s *Store) RescheduleAfterFiring(ctx context.Context, reminderID, firedOccurrenceStartUTC int64) error { |
| + return s.rescheduleAfter(ctx, reminderID, firedOccurrenceStartUTC) |
| +} |
| + |
| +// rescheduleAfter queues the first occurrence starting strictly after |
| +// `after`, or after now when `after` is zero. |
| +func (s *Store) rescheduleAfter(ctx context.Context, reminderID, after int64) error { |
| var rem Reminder |
| if err := s.g.WithContext(ctx).First(&rem, reminderID).Error; err != nil { |
| if errors.Is(err, gorm.ErrRecordNotFound) { |
| @@ -113,7 +140,13 @@ func (s *Store) RescheduleReminder(ctx context.Context, reminderID int64) error |
| return fmt.Errorf("calendar: read event for reminder: %w", err) |
| } |
| - occStart, occLocal, ok := s.nextOccurrenceAfter(e, time.Now().Unix(), rem.OffsetMin) |
| + // The floor for the search: whichever is later of "now" and the |
| + // occurrence just fired. Zero means nothing has fired, so now. |
| + floor := time.Now().Unix() |
| + if after > floor { |
| + floor = after |
| + } |
| + occStart, occLocal, ok := s.nextOccurrenceAfter(e, floor, rem.OffsetMin) |
| if !ok { |
| // Nothing left to warn about: a finished series, or a single |
| // event already past. Drop the queue row rather than leave a |
| @@ -145,61 +178,6 @@ func (s *Store) RescheduleReminder(ctx context.Context, reminderID int64) error |
| return nil |
| } |
| -// nextOccurrenceAfter finds the first occurrence whose reminder has not |
| -// yet come due. |
| -// |
| -// It searches from now minus the offset, so a reminder set for an event |
| -// starting in five minutes still fires — the due time is in the past, |
| -// and firing late is the correct answer, not skipping. |
| -func (s *Store) nextOccurrenceAfter(e Event, now int64, offsetMin int) (int64, string, bool) { |
| - if e.RRule == "" { |
| - if e.StartUTC-int64(offsetMin)*60 <= now && e.StartUTC <= now { |
| - return 0, "", false // already started; nothing to warn about |
| - } |
| - return e.StartUTC, e.StartLocal, true |
| - } |
| - rule, err := ParseRule(e.RRule) |
| - if err != nil { |
| - // A foreign rule still has its own first occurrence. |
| - if e.StartUTC > now { |
| - return e.StartUTC, e.StartLocal, true |
| - } |
| - return 0, "", false |
| - } |
| - // A window wide enough to find the next occurrence of anything |
| - // realistic — a yearly series needs more than a year. |
| - const lookahead = 400 * 24 * 3600 |
| - occs := Expand(e.StartLocal, e.EndLocal, e.TZ, e.AllDay, rule, e.Exdates, now-3600, now+lookahead) |
| - for _, o := range occs { |
| - if o.StartUTC > now { |
| - return o.StartUTC, o.Local, true |
| - } |
| - } |
| - return 0, "", false |
| -} |
| - |
| -// NextDueAt is what Options.NextDue answers: the earliest moment this |
| -// instance has work. |
| -// |
| -// The platform asks a running instance this before hibernating it, and |
| -// wakes it at the answer. That is what makes a reminder fire on a |
| -// machine that is asleep almost all the time — without it, a reminder |
| -// would arrive whenever the next person happened to load a page. |
| -// |
| -// Zero means nothing is scheduled, and the platform hibernates |
| -// indefinitely. |
| -func (s *Store) NextDueAt(ctx context.Context) time.Time { |
| - var q ReminderQueue |
| - err := s.g.WithContext(ctx). |
| - Where("fired_at IS NULL"). |
| - Order("due_at ASC"). |
| - First(&q).Error |
| - if err != nil { |
| - return time.Time{} |
| - } |
| - return time.Unix(q.DueAt, 0) |
| -} |
| - |
| // ExcludeOccurrence deletes one occurrence of a series by adding an |
| // EXDATE, and reschedules any reminder that pointed at it. |
| func (s *Store) ExcludeOccurrence(ctx context.Context, personID, eventID int64, local string) error { |
diff --git a/internal/calendar/store_reminder_next.go b/internal/calendar/store_reminder_next.go| new file mode 100644 |
| index 0000000..8538dc3 |
| --- /dev/null |
| +++ b/internal/calendar/store_reminder_next.go |
| @@ -0,0 +1,64 @@ |
| +package calendar |
| + |
| +import ( |
| + "context" |
| + "time" |
| +) |
| + |
| +// Deciding WHEN a reminder next fires, split from the code that writes |
| +// the queue row so neither file passes the 300-line cap. |
| + |
| +// nextOccurrenceAfter finds the first occurrence whose reminder has not |
| +// yet come due. |
| +// |
| +// It searches from now minus the offset, so a reminder set for an event |
| +// starting in five minutes still fires — the due time is in the past, |
| +// and firing late is the correct answer, not skipping. |
| +func (s *Store) nextOccurrenceAfter(e Event, now int64, offsetMin int) (int64, string, bool) { |
| + if e.RRule == "" { |
| + if e.StartUTC-int64(offsetMin)*60 <= now && e.StartUTC <= now { |
| + return 0, "", false // already started; nothing to warn about |
| + } |
| + return e.StartUTC, e.StartLocal, true |
| + } |
| + rule, err := ParseRule(e.RRule) |
| + if err != nil { |
| + // A foreign rule still has its own first occurrence. |
| + if e.StartUTC > now { |
| + return e.StartUTC, e.StartLocal, true |
| + } |
| + return 0, "", false |
| + } |
| + // A window wide enough to find the next occurrence of anything |
| + // realistic — a yearly series needs more than a year. |
| + const lookahead = 400 * 24 * 3600 |
| + occs := Expand(e.StartLocal, e.EndLocal, e.TZ, e.AllDay, rule, e.Exdates, now-3600, now+lookahead) |
| + for _, o := range occs { |
| + if o.StartUTC > now { |
| + return o.StartUTC, o.Local, true |
| + } |
| + } |
| + return 0, "", false |
| +} |
| + |
| +// NextDueAt is what Options.NextDue answers: the earliest moment this |
| +// instance has work. |
| +// |
| +// The platform asks a running instance this before hibernating it, and |
| +// wakes it at the answer. That is what makes a reminder fire on a |
| +// machine that is asleep almost all the time — without it, a reminder |
| +// would arrive whenever the next person happened to load a page. |
| +// |
| +// Zero means nothing is scheduled, and the platform hibernates |
| +// indefinitely. |
| +func (s *Store) NextDueAt(ctx context.Context) time.Time { |
| + var q ReminderQueue |
| + err := s.g.WithContext(ctx). |
| + Where("fired_at IS NULL"). |
| + Order("due_at ASC"). |
| + First(&q).Error |
| + if err != nil { |
| + return time.Time{} |
| + } |
| + return time.Unix(q.DueAt, 0) |
| +} |
diff --git a/internal/calendartest/browser_test.go b/internal/calendartest/browser_test.go| index e90f3e1..c7bf03e 100644 |
| --- a/internal/calendartest/browser_test.go |
| +++ b/internal/calendartest/browser_test.go |
| @@ -1,9 +1,9 @@ |
| //go:build browser |
| -// The browser drive: the whole app in a real Chromium, wired exactly |
| -// as cmd/calendar/main.go wires it — loud on any console error, failed |
| -// request, 4xx/5xx, or JS value that leaked to the screen unrendered. |
| -// Not part of the plain suite: |
| +// The browser drive: the whole app in a real Chromium, wired exactly as |
| +// cmd/calendar/main.go wires it — loud on any console error, failed |
| +// request, 4xx/5xx, or template value that leaked to the screen |
| +// unrendered. Not part of the plain suite: |
| // |
| // go test -tags browser ./... |
| // |
| @@ -11,6 +11,13 @@ |
| // Playwright cache. A skip is not a pass — with no browser this fails, |
| // unless RASTRILLO_BROWSER_OPTIONAL is set, which makes the skip a |
| // deliberate visible choice rather than an accident. |
| +// |
| +// This gate exists because every other test in this repo lives inside |
| +// one layer and agrees with itself: the Go suite asserts on strings the |
| +// Go code produced. Sheets' rule is that at least one gate must cross |
| +// from a computed value to painted output, and this is that crossing — |
| +// a server-side packer emitting inline percentages is exactly the kind |
| +// of thing that can be arithmetically perfect and visually wrong. |
| package calendartest |
| import ( |
| @@ -18,36 +25,120 @@ import ( |
| "log/slog" |
| "net/http" |
| "path/filepath" |
| + "strings" |
| "testing" |
| - "github.com/carlosframework/rastrillo/db" |
| "github.com/carlosframework/rastrillo/harness" |
| "github.com/chromedp/chromedp" |
| calendar "amadan.net/oficina/calendar/internal/calendar" |
| ) |
| -// TestBrowserWalk is the minimal loud walk: the home screen arrives |
| -// with nothing wrong anywhere the rig watches. Grow it with the app — |
| -// each screen a flow reaches earns a Screen() line, and an expected |
| -// probe (a signed-out 401, say) earns a rig.Allow. |
| +// TestBrowserWalk drives the real screens in a real browser: sign in, |
| +// create an event through the form, and look at every view. |
| func TestBrowserWalk(t *testing.T) { |
| + // The rig builds the app inside its own closure, because only it |
| + // knows the localhost origin it will serve on — and the origin is |
| + // load-bearing: it is the CSRF yardstick and the base of the |
| + // emailed sign-in link, so a guessed one breaks both. The handle |
| + // is captured on the way past so the test can read the mail |
| + // recorder afterwards. |
| + var app *calendar.App |
| rig := harness.New(t, func(origin string) http.Handler { |
| - // The scaffold's own wiring, origin included: the rig hands |
| - // the app its localhost origin before serving — the same seam |
| - // main.go fills from CALENDAR_ORIGIN. |
| - logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
| - d, err := db.Open(filepath.Join(t.TempDir(), "app.db"), logger) |
| - if err != nil { |
| - t.Fatalf("db.Open: %v", err) |
| - } |
| - t.Cleanup(func() { d.Close() }) |
| - mux, err := calendar.App(d, origin, logger) |
| + a, err := calendar.New(calendar.Config{ |
| + DBPath: filepath.Join(t.TempDir(), "calendar.db"), |
| + Env: "development", |
| + Origin: origin, |
| + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), |
| + }) |
| if err != nil { |
| - t.Fatalf("App: %v", err) |
| + t.Fatalf("calendar.New: %v", err) |
| } |
| - return mux |
| + t.Cleanup(func() { a.Close() }) |
| + app = a |
| + return a.Handler() |
| }) |
| - rig.Run(chromedp.Navigate(rig.Origin + "/")) |
| - rig.Screen("body", "home") |
| + |
| + // The sign-in page, signed out. |
| + rig.Run(chromedp.Navigate(rig.Origin + "/signin")) |
| + rig.Screen("body", "signin") |
| + |
| + // Sign in through the actual form, then follow the emailed link — |
| + // the same flow a person completes, rather than a forged cookie. |
| + rig.Run( |
| + chromedp.SendKeys(`input[name="address"]`, "someone@example.com", chromedp.ByQuery), |
| + chromedp.Click(`button[type="submit"]`, chromedp.ByQuery), |
| + chromedp.WaitVisible("body", chromedp.ByQuery), |
| + ) |
| + link := signInLinkFrom(t, app, rig.Origin) |
| + rig.Run(chromedp.Navigate(link), chromedp.WaitVisible("body", chromedp.ByQuery)) |
| + rig.Screen("body", "signed in") |
| + |
| + // Create an event through the form. |
| + rig.Run( |
| + chromedp.Navigate(rig.Origin+"/events/new?start=2026-09-02T09:30&end=2026-09-02T10:00"), |
| + chromedp.WaitVisible(`input[name="title"]`, chromedp.ByQuery), |
| + chromedp.SendKeys(`input[name="title"]`, "Standup", chromedp.ByQuery), |
| + chromedp.Click(`button[type="submit"]`, chromedp.ByQuery), |
| + chromedp.WaitVisible("body", chromedp.ByQuery), |
| + ) |
| + rig.Screen("body", "the event's own page") |
| + |
| + // Every view, painted. |
| + for _, page := range []struct{ path, note string }{ |
| + {"/day/2026-09-02", "day"}, |
| + {"/week/2026-09-02", "week"}, |
| + {"/month/2026-09", "month"}, |
| + {"/agenda?from=2026-09-02", "agenda"}, |
| + {"/calendars", "calendars"}, |
| + {"/settings", "settings"}, |
| + {"/import", "import"}, |
| + } { |
| + rig.Run( |
| + chromedp.Navigate(rig.Origin+page.path), |
| + chromedp.WaitVisible("body", chromedp.ByQuery), |
| + ) |
| + rig.Screen("body", page.note) |
| + } |
| + |
| + // The crossing that matters: the server packed this chip and gave |
| + // it a height in percent. Ask the BROWSER what it actually |
| + // measured. A packer that emitted 0%, or a stylesheet that never |
| + // loaded, produces a chip with no height — arithmetically fine and |
| + // invisible on screen. |
| + var height float64 |
| + rig.Run( |
| + chromedp.Navigate(rig.Origin+"/day/2026-09-02"), |
| + chromedp.WaitVisible("[cal-placed]", chromedp.ByQuery), |
| + chromedp.Evaluate(`document.querySelector('[cal-placed]').getBoundingClientRect().height`, &height), |
| + ) |
| + if height < 8 { |
| + t.Errorf("the packed event painted %vpx tall — the server's percentages are not reaching the screen", height) |
| + } |
| + |
| + // And the slot links are real, clickable targets rather than |
| + // zero-size boxes. |
| + var slotHeight float64 |
| + rig.Run(chromedp.Evaluate( |
| + `document.querySelector('[cal-slot]').getBoundingClientRect().height`, &slotHeight)) |
| + if slotHeight < 8 { |
| + t.Errorf("a half-hour slot link painted %vpx tall; it cannot be clicked", slotHeight) |
| + } |
| +} |
| + |
| +func signInLinkFrom(t *testing.T, app *calendar.App, origin string) string { |
| + t.Helper() |
| + body, err := app.LastMailForTest() |
| + if err != nil { |
| + t.Fatalf("LastMailForTest: %v", err) |
| + } |
| + i := strings.Index(body, origin+"/auth/verify?token=") |
| + if i < 0 { |
| + t.Fatalf("no sign-in link for %s in:\n%s", origin, body) |
| + } |
| + rest := body[i:] |
| + if j := strings.IndexAny(rest, " \r\n\t"); j >= 0 { |
| + rest = rest[:j] |
| + } |
| + return rest |
| } |
diff --git a/internal/calendartest/reminder_test.go b/internal/calendartest/reminder_test.go| new file mode 100644 |
| index 0000000..2adf293 |
| --- /dev/null |
| +++ b/internal/calendartest/reminder_test.go |
| @@ -0,0 +1,183 @@ |
| +package calendartest |
| + |
| +import ( |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + calendar "amadan.net/oficina/calendar/internal/calendar" |
| +) |
| + |
| +// A reminder must actually be sent, by the path that really sends it. |
| +// |
| +// Written after DrainOnRequest shipped as dead code — defined, |
| +// commented, described in the spec as one of three entrances, and |
| +// called from nowhere. Every unit around it passed. This asserts the |
| +// observable end: a message arrived. |
| +func TestAReminderIsActuallySent(t *testing.T) { |
| + // The tick guard is a constant-time comparison against this |
| + // instance's own token, and it is false when the environment has |
| + // none — so a test that does not set it exercises the refusal |
| + // rather than the drain. |
| + t.Setenv("CARLOS_ADMIN_TOKEN", "test-token") |
| + |
| + app, session := signedIn(t, "someone@example.com") |
| + h := app.Handler() |
| + |
| + // An event soon enough that its default ten-minute reminder is |
| + // already due. The reminder is created with the event. |
| + // |
| + // UTC, because that is the test person's zone. Formatting in the |
| + // SERVER's local zone is the mistake this comment exists to stop |
| + // coming back: the app reads a submitted wall time in the person's |
| + // zone, so on a machine at +01:00 every event landed an hour |
| + // further out than intended and no reminder was ever due. |
| + start := time.Now().UTC().Add(2 * time.Minute) |
| + createEvent(t, h, session, url.Values{ |
| + "title": {"Imminent standup"}, |
| + "start": {start.Format("2006-01-02T15:04")}, |
| + "end": {start.Add(15 * time.Minute).Format("2006-01-02T15:04")}, |
| + "kind": {"meeting"}, |
| + "status": {"confirmed"}, |
| + "repeat": {"none"}, |
| + }) |
| + |
| + // The sign-in link is the last mail so far; anything after this is |
| + // the reminder. |
| + before, _ := app.LastMailForTest() |
| + |
| + req := httptest.NewRequest(http.MethodPost, "/jobs/reminders", nil) |
| + req.Header.Set("Authorization", "Bearer test-token") |
| + rec := httptest.NewRecorder() |
| + h.ServeHTTP(rec, req) |
| + |
| + if rec.Code != http.StatusNoContent { |
| + t.Fatalf("POST /jobs/reminders with the right bearer: status %d, want 204\n%s", |
| + rec.Code, rec.Body.String()) |
| + } |
| + |
| + after, err := app.LastMailForTest() |
| + if err != nil { |
| + t.Fatalf("LastMailForTest: %v", err) |
| + } |
| + if after == before { |
| + t.Fatal("the drain sent nothing; the reminder never fired") |
| + } |
| + if !strings.Contains(after, "Imminent standup") { |
| + t.Errorf("the reminder does not name the event:\n%s", after) |
| + } |
| + // The message carries a link back to the event, which is the whole |
| + // point of receiving it on a phone. |
| + if !strings.Contains(after, "/events/") { |
| + t.Errorf("the reminder carries no link to the event:\n%s", after) |
| + } |
| +} |
| + |
| +// The same occurrence must not be sent twice by two drains running at |
| +// once — a tick and a page view, say. The claim is a conditional |
| +// UPDATE for exactly this reason. |
| +func TestAReminderIsNotSentTwice(t *testing.T) { |
| + t.Setenv("CARLOS_ADMIN_TOKEN", "test-token") |
| + |
| + app, session := signedIn(t, "someone@example.com") |
| + h := app.Handler() |
| + |
| + start := time.Now().UTC().Add(2 * time.Minute) |
| + createEvent(t, h, session, url.Values{ |
| + "title": {"Only once"}, |
| + "start": {start.Format("2006-01-02T15:04")}, |
| + "end": {start.Add(15 * time.Minute).Format("2006-01-02T15:04")}, |
| + "kind": {"meeting"}, |
| + "status": {"confirmed"}, |
| + "repeat": {"none"}, |
| + }) |
| + |
| + tick := func() { |
| + req := httptest.NewRequest(http.MethodPost, "/jobs/reminders", nil) |
| + req.Header.Set("Authorization", "Bearer test-token") |
| + h.ServeHTTP(httptest.NewRecorder(), req) |
| + } |
| + |
| + tick() |
| + first, _ := app.LastMailForTest() |
| + if !strings.Contains(first, "Only once") { |
| + t.Fatalf("the first drain did not send the reminder:\n%s", first) |
| + } |
| + |
| + // A second drain has nothing left to do: the row is marked fired, |
| + // and a single event has no next occurrence to queue. |
| + tick() |
| + second, _ := app.LastMailForTest() |
| + if second != first { |
| + t.Errorf("a second drain sent something else; the occurrence was not marked fired:\n%s", second) |
| + } |
| +} |
| + |
| +// NextDue is what the platform asks before hibernating. It must name |
| +// the earliest unfired reminder, or the instance sleeps through it. |
| +func TestNextDuePointsAtTheEarliestReminder(t *testing.T) { |
| + app, session := signedIn(t, "someone@example.com") |
| + h := app.Handler() |
| + |
| + if due := app.NextDue(); !due.IsZero() { |
| + t.Errorf("NextDue on an empty calendar = %v, want the zero time", due) |
| + } |
| + |
| + start := time.Now().UTC().Add(3 * time.Hour) |
| + createEvent(t, h, session, url.Values{ |
| + "title": {"Later"}, |
| + "start": {start.Format("2006-01-02T15:04")}, |
| + "end": {start.Add(30 * time.Minute).Format("2006-01-02T15:04")}, |
| + "kind": {"meeting"}, |
| + "status": {"confirmed"}, |
| + "repeat": {"none"}, |
| + }) |
| + |
| + due := app.NextDue() |
| + if due.IsZero() { |
| + t.Fatal("NextDue is zero with a reminder queued; the platform would hibernate through it") |
| + } |
| + // The default reminder is ten minutes before the start. |
| + want := start.Add(-10 * time.Minute) |
| + if diff := due.Sub(want); diff > time.Minute || diff < -time.Minute { |
| + t.Errorf("NextDue = %v, want about %v (ten minutes before the event)", due, want) |
| + } |
| +} |
| + |
| +// A reminder on a repeating event queues the NEXT occurrence after it |
| +// fires — one row per rule, never a materialised series. |
| +func TestARecurringReminderRequeuesItself(t *testing.T) { |
| + t.Setenv("CARLOS_ADMIN_TOKEN", "test-token") |
| + |
| + app, session := signedIn(t, "someone@example.com") |
| + h := app.Handler() |
| + |
| + start := time.Now().UTC().Add(2 * time.Minute) |
| + createEvent(t, h, session, url.Values{ |
| + "title": {"Daily thing"}, |
| + "start": {start.Format("2006-01-02T15:04")}, |
| + "end": {start.Add(15 * time.Minute).Format("2006-01-02T15:04")}, |
| + "kind": {"meeting"}, |
| + "status": {"confirmed"}, |
| + "repeat": {"daily"}, |
| + }) |
| + |
| + req := httptest.NewRequest(http.MethodPost, "/jobs/reminders", nil) |
| + req.Header.Set("Authorization", "Bearer test-token") |
| + h.ServeHTTP(httptest.NewRecorder(), req) |
| + |
| + // After firing, the rule's next occurrence must be queued — |
| + // roughly a day later. |
| + due := app.NextDue() |
| + if due.IsZero() { |
| + t.Fatal("a repeating reminder fired and queued nothing; the series would remind once and stop") |
| + } |
| + if until := time.Until(due); until < 20*time.Hour || until > 26*time.Hour { |
| + t.Errorf("the requeued reminder is %v away, want about a day", until) |
| + } |
| +} |
| + |
| +var _ = calendar.MaxReminderAttempts |