From 3ede0fb42287cb0dff30f4313ce735999a3853f9 Mon Sep 17 00:00:00 2001 From: Harley Trung Date: Thu, 8 Oct 2026 21:07:47 +0700 Subject: [PATCH 1/2] fix: suppress reminders for cancelled Google events --- docs/ARCHITECTURE.md | 18 ++++ internal/calendar/calendar.go | 23 +++++ internal/calendar/provider_event_test.go | 45 ++++++++ internal/gcal/events.go | 43 ++++++++ internal/gcal/events_test.go | 45 ++++++++ internal/handler/reminder_check.go | 36 +++++++ internal/handler/reminder_check_test.go | 124 +++++++++++++++++++++++ internal/server/server.go | 2 +- internal/worker/worker.go | 42 ++++++-- 9 files changed, 370 insertions(+), 8 deletions(-) create mode 100644 internal/handler/reminder_check.go create mode 100644 internal/handler/reminder_check_test.go diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8baf33cc..0a4d494c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -761,6 +761,24 @@ as the desired state: --- +### Reminders after external Google Calendar cancellation + +Before sending a reminder, the worker checks the primary host's recorded calendar +and event ID. Google `status=cancelled` suppresses that job even when the local +booking remains confirmed. An active event permits delivery; lookup failures +(including unavailable credentials, 404/410, rate limits and malformed responses) +use the job's normal retry policy and never authorize sending. After retries are +exhausted the job is failed. The lookup has a 10-second deadline, and local booking +status is checked again afterwards to catch cancellation during the request. + +This is a reminder guard, not inbound booking synchronization: it does not change +booking status, free the slot, refund a payment or send cancellation notifications. +Secondary hosts' calendar copies do not decide cancellation. Bookings without a +recorded primary-host event and providers without cancellation-read support retain +the existing reminder behavior. Only Google currently implements this check. +The recorded calendar rescues destination changes within an account; changing to +another account can still make the event inaccessible and fail its reminder job. + ## 13. Webhooks & background worker - `internal/worker`: polls the `jobs` table **every 5s** (batch ≤10). Job types: diff --git a/internal/calendar/calendar.go b/internal/calendar/calendar.go index 7d4f5bc8..7fb657cb 100644 --- a/internal/calendar/calendar.go +++ b/internal/calendar/calendar.go @@ -8,6 +8,7 @@ import ( "context" "database/sql" "errors" + "fmt" "sort" "time" @@ -434,3 +435,25 @@ func (s *Service) CancelEvent(ctx context.Context, userID, calendarID, eventID, } return nil } + +// EventCancellationReader is an optional provider capability. Only a positively +// identified cancellation returns true; access and lookup failures are errors. +type EventCancellationReader interface { + EventCancelled(context.Context, string, string, string) (bool, error) +} + +// EventCancelled checks the provider recorded at creation, using the same routing +// as updates and cancellations. Providers without this capability keep existing behavior. +func (s *Service) EventCancelled(ctx context.Context, userID, calendarID, eventID, provider string) (bool, error) { + if provider != "" && s.providers[provider] == nil { + return false, fmt.Errorf("calendar: recorded event provider unavailable") + } + pr := s.providerForEvent(ctx, userID, eventID, provider) + if pr == nil { + return false, fmt.Errorf("calendar: event provider unavailable") + } + if reader, ok := pr.(EventCancellationReader); ok { + return reader.EventCancelled(ctx, userID, calendarID, eventID) + } + return false, nil +} diff --git a/internal/calendar/provider_event_test.go b/internal/calendar/provider_event_test.go index 6043a208..915ef57c 100644 --- a/internal/calendar/provider_event_test.go +++ b/internal/calendar/provider_event_test.go @@ -118,3 +118,48 @@ func TestProviderForEvent_prefersStampedProvider(t *testing.T) { } } } + +type cancellationProvider struct { + recordProvider + cancelled bool + calls int +} + +func (p *cancellationProvider) EventCancelled(context.Context, string, string, string) (bool, error) { + p.calls++ + return p.cancelled, nil +} + +func TestEventCancelledUsesStoredProvider(t *testing.T) { + database, err := db.Open("sqlite://" + t.TempDir() + "/test.db") + if err != nil { + t.Fatal(err) + } + defer database.Close() + s := NewService(database) + google := &cancellationProvider{recordProvider: recordProvider{name: "google"}, cancelled: true} + microsoft := &cancellationProvider{recordProvider: recordProvider{name: "microsoft"}} + s.Register(google) + s.Register(microsoft) + cancelled, err := s.EventCancelled(context.Background(), "host", "original", "event", "google") + if err != nil || !cancelled || google.calls != 1 || microsoft.calls != 0 { + t.Fatalf("cancelled=%v err=%v calls=%d/%d", cancelled, err, google.calls, microsoft.calls) + } + if _, err := s.EventCancelled(context.Background(), "host", "original", "event", "missing-provider"); err == nil { + t.Fatal("missing stamped provider must not fall through") + } +} + +func TestEventCancelledUnsupportedProvider(t *testing.T) { + database, err := db.Open("sqlite://" + t.TempDir() + "/test.db") + if err != nil { + t.Fatal(err) + } + defer database.Close() + s := NewService(database) + s.Register(&recordProvider{name: "microsoft"}) + cancelled, err := s.EventCancelled(context.Background(), "host", "calendar", "event", "microsoft") + if err != nil || cancelled { + t.Fatalf("unsupported provider got %v, %v", cancelled, err) + } +} diff --git a/internal/gcal/events.go b/internal/gcal/events.go index fa3c89d0..57044c15 100644 --- a/internal/gcal/events.go +++ b/internal/gcal/events.go @@ -231,3 +231,46 @@ func (c *Client) CancelEvent(ctx context.Context, userID, calendarID, eventID st } return nil } + +// EventCancelled reads the stored event before reminder delivery. A missing or +// inaccessible event is ambiguous (e.g. the destination account changed), so only +// an explicit cancelled status suppresses delivery; all other failures retry. +func (c *Client) EventCancelled(ctx context.Context, userID, calendarID, eventID string) (bool, error) { + hc, calID, err := c.DestinationClient(ctx, userID) + if err != nil { + return false, err + } + if hc == nil { + return false, fmt.Errorf("gcal: event status: no destination connection") + } + if calendarID != "" { + calID = calendarID + } + apiURL := c.apiBase + "/calendars/" + url.PathEscape(calID) + "/events/" + url.PathEscape(eventID) + "?fields=status" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return false, fmt.Errorf("gcal: event status request: %w", err) + } + resp, err := hc.Do(req) + if err != nil { + return false, fmt.Errorf("gcal: event status: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return false, fmt.Errorf("gcal: event status HTTP %d", resp.StatusCode) + } + var event struct { + Status string `json:"status"` + } + if err := json.NewDecoder(resp.Body).Decode(&event); err != nil { + return false, fmt.Errorf("gcal: decode event status: %w", err) + } + switch event.Status { + case "cancelled": + return true, nil + case "confirmed", "tentative": + return false, nil + default: + return false, fmt.Errorf("gcal: unknown event status %q", event.Status) + } +} diff --git a/internal/gcal/events_test.go b/internal/gcal/events_test.go index b9a75a62..fb5f1ac6 100644 --- a/internal/gcal/events_test.go +++ b/internal/gcal/events_test.go @@ -328,3 +328,48 @@ func TestUpdateEvent_emptyEventID_noOp(t *testing.T) { t.Errorf("UpdateEvent(\"\") = %v; want nil", err) } } + +func TestEventCancelled(t *testing.T) { + for _, tc := range []struct { + name, body string + code int + cancelled, wantErr bool + }{ + {"cancelled", `{"status":"cancelled"}`, 200, true, false}, + {"confirmed", `{"status":"confirmed"}`, 200, false, false}, + {"tentative", `{"status":"tentative"}`, 200, false, false}, + {"missing", `{}`, 404, false, true}, + {"gone", `{}`, 410, false, true}, + {"forbidden", `{}`, 403, false, true}, + {"rate_limit", `{}`, 429, false, true}, + {"server_error", `{}`, 500, false, true}, + {"invalid_json", `{`, 200, false, true}, + {"empty_status", `{}`, 200, false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" || r.URL.Path != "/calendars/original@example.com/events/event-id" || r.URL.Query().Get("fields") != "status" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL) + } + w.WriteHeader(tc.code) + _, _ = w.Write([]byte(tc.body)) + })) + defer srv.Close() + c := newTestClient(t) + c.apiBase = srv.URL + saveDestinationConnection(t, c, "user-1", "new-destination") + got, err := c.EventCancelled(context.Background(), "user-1", "original@example.com", "event-id") + if got != tc.cancelled || (err != nil) != tc.wantErr { + t.Fatalf("got (%v, %v), want cancelled=%v error=%v", got, err, tc.cancelled, tc.wantErr) + } + }) + } +} + +func TestEventCancelledDisconnected(t *testing.T) { + c := newTestClient(t) + got, err := c.EventCancelled(context.Background(), "unknown", "primary", "event-id") + if got || err == nil { + t.Fatalf("got (%v, %v); want lookup error", got, err) + } +} diff --git a/internal/handler/reminder_check.go b/internal/handler/reminder_check.go new file mode 100644 index 00000000..9e2843a3 --- /dev/null +++ b/internal/handler/reminder_check.go @@ -0,0 +1,36 @@ +package handler + +import ( + "context" + "database/sql" + "fmt" +) + +// ReminderAllowed checks the primary host's calendar event without changing the +// booking lifecycle. Deleting a secondary host's copy is not a booking cancellation. +func (h *Handler) ReminderAllowed(ctx context.Context, bookingID string) (bool, error) { + var hostID, eventID, calendarID, provider string + err := h.db.QueryRowContext(ctx, ` + SELECT bh.user_id, bh.external_event_id, COALESCE(bh.external_calendar_id, ''), COALESCE(bh.external_provider, '') + FROM booking_hosts bh JOIN bookings b ON b.id = bh.booking_id + WHERE b.id = ? AND bh.user_id = b.host_id AND COALESCE(bh.external_event_id, '') != ''`, bookingID). + Scan(&hostID, &eventID, &calendarID, &provider) + if err == sql.ErrNoRows { + return true, nil + } + if err != nil { + return false, fmt.Errorf("reminder: load calendar event: %w", err) + } + gc := h.getCal() + if gc == nil { + return false, fmt.Errorf("reminder: calendar service unavailable") + } + cancelled, err := gc.EventCancelled(ctx, hostID, calendarID, eventID, provider) + if err != nil { + return false, err + } + if cancelled { + h.logger.Info("reminder: skipped externally cancelled event", "booking_id", bookingID) + } + return !cancelled, nil +} diff --git a/internal/handler/reminder_check_test.go b/internal/handler/reminder_check_test.go new file mode 100644 index 00000000..41565048 --- /dev/null +++ b/internal/handler/reminder_check_test.go @@ -0,0 +1,124 @@ +package handler_test + +import ( + "context" + "errors" + "log/slog" + "strings" + "testing" + "time" + + "github.com/calnode/calnode/internal/calendar" + "github.com/calnode/calnode/internal/mailer" + "github.com/calnode/calnode/internal/webhook" + "github.com/calnode/calnode/internal/worker" +) + +type reminderCalendar struct { + calendar.Provider + cancelled bool + err error + calls int +} + +func (p *reminderCalendar) Name() string { return "google" } +func (p *reminderCalendar) EventCancelled(_ context.Context, userID, calendarID, eventID string) (bool, error) { + p.calls++ + if userID == "" || calendarID != "original-calendar" || eventID != "original-event" { + return false, errors.New("wrong stored event identity") + } + return p.cancelled, p.err +} + +type reminderMailer struct{ sent int } + +func (m *reminderMailer) Send(context.Context, mailer.Message) error { m.sent++; return nil } + +// Run the actual worker and handler guard against a booking that is still locally +// confirmed, exactly the state left by a Google-origin cancellation. +func TestReminderChecksExternalCancellation(t *testing.T) { + for _, tc := range []struct { + name string + cancelled, lookupErr, noEvent, noService, cancelDuringCheck bool + wantSent int + wantJob string + }{ + {name: "external_cancel", cancelled: true, wantJob: "done"}, + {name: "active", wantSent: 1, wantJob: "done"}, + {name: "lookup_error", lookupErr: true, wantJob: "pending"}, + {name: "no_calendar_event", noEvent: true, wantSent: 1, wantJob: "done"}, + {name: "unconfigured_service", noService: true, wantJob: "pending"}, + {name: "cancel_during_lookup", cancelDuringCheck: true, wantJob: "done"}, + } { + t.Run(tc.name, func(t *testing.T) { + h, database, key, hostID := setupWorkspaceWithDB(t) + _, etID := seedEventTypeHTTP(t, h, key) + start := time.Now().UTC().Add(time.Hour).Format(time.RFC3339) + calendarCheckExec(t, database, `INSERT INTO bookings (id,event_type_id,host_id,start_at,end_at,status) VALUES ('rem-booking',?,?,?,?,'confirmed')`, etID, hostID, start, start) + calendarCheckExec(t, database, `INSERT INTO booking_attendees (id,booking_id,name,email,iana_timezone,is_organizer) VALUES ('rem-att','rem-booking','Test','test@example.com','UTC',1)`) + if !tc.noEvent { + calendarCheckExec(t, database, `INSERT INTO booking_hosts (booking_id,user_id,external_event_id,external_calendar_id,external_provider) VALUES ('rem-booking',?,'original-event','original-calendar','google')`, hostID) + } + calendarCheckExec(t, database, `INSERT INTO jobs (id,type,payload,run_at,status,attempts,max_attempts) VALUES ('rem-job','reminder.send','{"booking_id":"rem-booking"}',?,'pending',0,3)`, time.Now().UTC().Add(-time.Second).Format(time.RFC3339)) + p := &reminderCalendar{cancelled: tc.cancelled} + if tc.lookupErr { + p.err = errors.New("Google unavailable") + } + if !tc.noService { + svc := calendar.NewService(database) + svc.Register(p) + h.SetCalendar(svc) + } + check := h.ReminderAllowed + if tc.cancelDuringCheck { + check = func(ctx context.Context, id string) (bool, error) { + allowed, err := h.ReminderAllowed(ctx, id) + _, updateErr := database.ExecContext(ctx, `UPDATE bookings SET status='cancelled' WHERE id=?`, id) + if updateErr != nil { + return false, updateErr + } + return allowed, err + } + } + m := &reminderMailer{} + whs, err := webhook.New(database, strings.Repeat("01", 32)) + if err != nil { + t.Fatal(err) + } + w := worker.New(database, whs, slog.Default(), worker.WithMailer(m), worker.WithReminderCheck(check)) + w.Poll(context.Background()) + if m.sent != tc.wantSent { + t.Fatalf("sent %d emails, want %d", m.sent, tc.wantSent) + } + var status string + if err := database.QueryRow(`SELECT status FROM jobs WHERE id='rem-job'`).Scan(&status); err != nil { + t.Fatal(err) + } + if status != tc.wantJob { + t.Fatalf("job status %q, want %q", status, tc.wantJob) + } + if tc.lookupErr { + p.err = nil + calendarCheckExec(t, database, `UPDATE jobs SET run_at=? WHERE id='rem-job'`, time.Now().UTC().Add(-time.Second).Format(time.RFC3339)) + w.Poll(context.Background()) + if m.sent != 1 { + t.Fatalf("recovered lookup sent %d reminders, want 1", m.sent) + } + if err := database.QueryRow(`SELECT status FROM jobs WHERE id='rem-job'`).Scan(&status); err != nil { + t.Fatal(err) + } + if status != "done" { + t.Fatalf("recovered job = %s", status) + } + } + if tc.cancelled { + if err := database.QueryRow(`SELECT status FROM bookings WHERE id='rem-booking'`).Scan(&status); err != nil { + t.Fatal(err) + } + if status != "confirmed" { + t.Fatal("reminder check changed booking state") + } + } + }) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 6c0c07ca..b74cf834 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -125,7 +125,7 @@ func BuildHandler(ctx context.Context, cfg *config.Config, db *sql.DB, logger *s } else { h.SetWebhookSvc(whs) // Pass live so the worker picks up SMTP changes automatically. - wrk := worker.New(db, whs, logger, worker.WithMailer(live)) + wrk := worker.New(db, whs, logger, worker.WithMailer(live), worker.WithReminderCheck(h.ReminderAllowed)) // Notetaker jobs live in the handler package (they need LLM/S3/encKey). wrk.RegisterHandler("notetaker.transcribe", h.JobNotetakerTranscribe) wrk.RegisterHandler("notetaker.summarize", h.JobNotetakerSummarize) diff --git a/internal/worker/worker.go b/internal/worker/worker.go index 72bc8cb7..6cfba723 100644 --- a/internal/worker/worker.go +++ b/internal/worker/worker.go @@ -25,13 +25,14 @@ const webhookDeliveryRetention = 30 * 24 * time.Hour // Worker polls the jobs table and processes pending jobs (webhooks, reminders). type Worker struct { - db *sql.DB - svc *webhook.Service - mailer mailer.Mailer - logger *slog.Logger - httpClient *http.Client - handlers map[string]func(context.Context, string) error // custom job types (e.g. notetaker) - done chan struct{} + db *sql.DB + svc *webhook.Service + mailer mailer.Mailer + logger *slog.Logger + httpClient *http.Client + handlers map[string]func(context.Context, string) error // custom job types (e.g. notetaker) + reminderCheck func(context.Context, string) (bool, error) + done chan struct{} } // RegisterHandler registers a processor for a custom job type whose logic lives outside this @@ -51,6 +52,12 @@ func WithMailer(m mailer.Mailer) func(*Worker) { return func(w *Worker) { w.mailer = m } } +// WithReminderCheck adds a live calendar check before sending a reminder. +// False suppresses delivery; errors use the job's normal retry policy. +func WithReminderCheck(check func(context.Context, string) (bool, error)) func(*Worker) { + return func(w *Worker) { w.reminderCheck = check } +} + func New(db *sql.DB, svc *webhook.Service, logger *slog.Logger, opts ...func(*Worker)) *Worker { w := &Worker{ db: db, @@ -353,6 +360,27 @@ func (w *Worker) sendReminder(ctx context.Context, payload string) error { SELECT COALESCE(business_name,''), COALESCE(logo_url,'') FROM server_settings WHERE id = 1`).Scan(&d.BrandName, &d.LogoURL) + if w.reminderCheck != nil { + checkCtx, cancel := context.WithTimeout(ctx, 10*time.Second) + allowed, err := w.reminderCheck(checkCtx, p.BookingID) + cancel() + if err != nil { + return fmt.Errorf("worker: reminder: calendar check: %w", err) + } + if !allowed { + return nil + } + } + // Cancellation may have completed while the external lookup was in flight. + if err := w.db.QueryRowContext(ctx, `SELECT status FROM bookings WHERE id = ?`, p.BookingID).Scan(&status); err == sql.ErrNoRows { + return nil + } else if err != nil { + return fmt.Errorf("worker: reminder: recheck booking: %w", err) + } + if status != "confirmed" { + return nil + } + if err := mailer.SendReminder(ctx, w.mailer, d); err != nil { return fmt.Errorf("worker: reminder: send: %w", err) } From fc5d5b5f13c04162c4f721564386f5b1648dd8ea Mon Sep 17 00:00:00 2001 From: Harley Trung Date: Thu, 8 Oct 2026 21:39:34 +0700 Subject: [PATCH 2/2] fix: preserve calendar providers during Google reload (#133) --- docs/ARCHITECTURE.md | 7 ++ internal/handler/calendar_reload.go | 31 ++++++ internal/handler/calendar_reload_test.go | 67 ++++++++++++ .../handler/google_calendar_reload_test.go | 102 ++++++++++++++++++ internal/handler/google_settings.go | 7 +- 5 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 internal/handler/calendar_reload.go create mode 100644 internal/handler/calendar_reload_test.go create mode 100644 internal/handler/google_calendar_reload_test.go diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0a4d494c..498b8da4 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -776,6 +776,13 @@ booking status, free the slot, refund a payment or send cancellation notificatio Secondary hosts' calendar copies do not decide cancellation. Bookings without a recorded primary-host event and providers without cancellation-read support retain the existing reminder behavior. Only Google currently implements this check. +Saving or clearing Google credentials replaces only the Google provider using a +fresh calendar-service snapshot; Microsoft/CalDAV registrations and an existing +non-Google primary provider survive without a restart. In-flight operations keep +the previous snapshot. Clearing Google credentials still leaves Google reminders +unverifiable, so they follow the same retry/failure policy rather than sending +without a cancellation check. + The recorded calendar rescues destination changes within an account; changing to another account can still make the event inaccessible and fail its reminder job. diff --git a/internal/handler/calendar_reload.go b/internal/handler/calendar_reload.go new file mode 100644 index 00000000..0d33ccff --- /dev/null +++ b/internal/handler/calendar_reload.go @@ -0,0 +1,31 @@ +package handler + +import "github.com/calnode/calnode/internal/calendar" + +// setGoogleCalendar replaces only Google's provider. Publish a fresh service so +// in-flight operations can finish on the previous snapshot without map races. +func (h *Handler) setGoogleCalendar(google calendar.Provider) { + h.calMu.Lock() + defer h.calMu.Unlock() + next := calendar.NewService(h.db) + if h.cal != nil { + if primary := h.cal.Primary(); primary != nil && primary.Name() != "google" { + next.Register(primary) + } else if google != nil { + next.Register(google) + } + for _, name := range h.cal.ProviderNames() { + if name != "google" && next.Provider(name) == nil { + next.Register(h.cal.Provider(name)) + } + } + } + if google != nil && next.Provider("google") == nil { + next.Register(google) + } + if next.Any() { + h.cal = next + } else { + h.cal = nil + } +} diff --git a/internal/handler/calendar_reload_test.go b/internal/handler/calendar_reload_test.go new file mode 100644 index 00000000..465860e8 --- /dev/null +++ b/internal/handler/calendar_reload_test.go @@ -0,0 +1,67 @@ +package handler + +import ( + "github.com/calnode/calnode/internal/calendar" + "testing" +) + +type reloadProvider struct { + calendar.Provider + name string +} + +func (p *reloadProvider) Name() string { return p.name } + +func TestSetGoogleCalendarPreservesPrimaryAndSnapshot(t *testing.T) { + for _, primary := range []string{"google", "microsoft"} { + for _, clear := range []bool{false, true} { + t.Run(primary+map[bool]string{false: "/replace", true: "/clear"}[clear], func(t *testing.T) { + h := &Handler{} + old := calendar.NewService(nil) + old.Register(&reloadProvider{name: primary}) + other := "google" + if primary == "google" { + other = "microsoft" + } + old.Register(&reloadProvider{name: other}) + h.SetCalendar(old) + replacement := &reloadProvider{name: "google"} + var provider calendar.Provider = replacement + if clear { + provider = nil + } + h.setGoogleCalendar(provider) + current := h.getCal() + wantPrimary := primary + if clear && primary == "google" { + wantPrimary = "microsoft" + } + if current.Primary().Name() != wantPrimary { + t.Fatalf("primary=%s, want %s", current.Primary().Name(), wantPrimary) + } + if current.Provider("microsoft") != old.Provider("microsoft") { + t.Fatal("non-Google provider replaced") + } + if clear { + if current.Provider("google") != nil { + t.Fatal("Google not removed") + } + } else if current.Provider("google") != replacement { + t.Fatal("Google not replaced") + } + if old.Provider("google") == nil || len(old.ProviderNames()) != 2 { + t.Fatal("old snapshot mutated") + } + }) + } + } + h := &Handler{} + h.setGoogleCalendar(&reloadProvider{name: "google"}) + if h.getCal() == nil { + t.Fatal("first Google configuration not installed") + } + h.setGoogleCalendar(nil) + if h.getCal() != nil { + t.Fatal("Google-only removal did not clear service") + } +} diff --git a/internal/handler/google_calendar_reload_test.go b/internal/handler/google_calendar_reload_test.go new file mode 100644 index 00000000..8912cc78 --- /dev/null +++ b/internal/handler/google_calendar_reload_test.go @@ -0,0 +1,102 @@ +package handler_test + +import ( + "context" + "encoding/json" + "log/slog" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + "time" + + "github.com/calnode/calnode/internal/calendar" + "github.com/calnode/calnode/internal/webhook" + "github.com/calnode/calnode/internal/worker" +) + +type reloadCalendar struct { + calendar.Provider + name string +} + +func (p *reloadCalendar) Name() string { return p.name } + +// Exercise the real settings handler followed by the real reminder worker. +// Neither saving nor clearing Google credentials may remove another provider. +func TestGoogleSettingsReloadPreservesOtherReminders(t *testing.T) { + for _, provider := range []string{"microsoft", "caldav"} { + for _, action := range []string{"save", "clear"} { + t.Run(provider+"/"+action, func(t *testing.T) { + h, database, key, hostID := setupWorkspaceWithDB(t) + h.SetEncKey(testGCalKeyHex) + h.SetBaseURL("http://localhost:3000") + _, etID := seedEventTypeHTTP(t, h, key) + original := calendar.NewService(database) + original.Register(&reloadCalendar{name: "google"}) + original.Register(&reloadCalendar{name: "microsoft"}) + original.Register(&reloadCalendar{name: "caldav"}) + h.SetCalendar(original) + body := `{"client_id":"new-id","client_secret":"new-secret"}` + if action == "clear" { + body = `{"client_id":""}` + } + rec := patchGoogleSettings(t, h, body, key) + if rec.Code != http.StatusOK { + t.Fatalf("settings: %d %s", rec.Code, rec.Body.String()) + } + req := authReq(http.MethodGet, "/v1/calendar/status", "", key) + statusRec := httptest.NewRecorder() + h.RequireAuth(h.CalendarStatus)(statusRec, req) + if statusRec.Code != http.StatusOK { + t.Fatalf("calendar status: %d %s", statusRec.Code, statusRec.Body.String()) + } + var status struct { + Providers []string `json:"providers"` + } + if err := json.Unmarshal(statusRec.Body.Bytes(), &status); err != nil { + t.Fatal(err) + } + wantProviders := []string{"caldav", "microsoft"} + if action == "save" { + wantProviders = []string{"caldav", "google", "microsoft"} + } + if !slices.Equal(status.Providers, wantProviders) { + t.Fatalf("providers=%v, want %v", status.Providers, wantProviders) + } + if original.Provider("google") == nil || len(original.ProviderNames()) != 3 { + t.Fatal("reload mutated in-flight service") + } + start := time.Now().UTC().Add(time.Hour).Format(time.RFC3339) + calendarCheckExec(t, database, `INSERT INTO bookings (id,event_type_id,host_id,start_at,end_at,status) VALUES ('reload-booking',?,?,?,?,'confirmed')`, etID, hostID, start, start) + calendarCheckExec(t, database, `INSERT INTO booking_hosts (booking_id,user_id,external_event_id,external_calendar_id,external_provider) VALUES ('reload-booking',?,'original-event','original-calendar',?)`, hostID, provider) + calendarCheckExec(t, database, `INSERT INTO booking_attendees (id,booking_id,name,email,iana_timezone,is_organizer) VALUES ('reload-att','reload-booking','Test','test@example.com','UTC',1)`) + calendarCheckExec(t, database, `INSERT INTO jobs (id,type,payload,run_at,status,attempts,max_attempts) VALUES ('reload-job','reminder.send','{"booking_id":"reload-booking"}',?,'pending',0,3)`, time.Now().UTC().Add(-time.Second).Format(time.RFC3339)) + whs, err := webhook.New(database, strings.Repeat("01", 32)) + if err != nil { + t.Fatal(err) + } + m := &reminderMailer{} + w := worker.New(database, whs, slog.Default(), worker.WithMailer(m), worker.WithReminderCheck(h.ReminderAllowed)) + w.Poll(context.Background()) + if m.sent != 1 { + t.Fatalf("sent=%d, want 1 after Google %s", m.sent, action) + } + var jobStatus string + if err := database.QueryRow(`SELECT status FROM jobs WHERE id='reload-job'`).Scan(&jobStatus); err != nil { + t.Fatal(err) + } + if jobStatus != "done" { + t.Fatalf("job=%s, want done", jobStatus) + } + // Missing Google credentials still cannot authorize an unchecked Google reminder. + calendarCheckExec(t, database, `UPDATE booking_hosts SET external_provider='google' WHERE booking_id='reload-booking'`) + allowed, err := h.ReminderAllowed(context.Background(), "reload-booking") + if allowed || err == nil { + t.Fatalf("unchecked Google reminder allowed=%v err=%v", allowed, err) + } + }) + } + } +} diff --git a/internal/handler/google_settings.go b/internal/handler/google_settings.go index 965aa5e1..4793a40d 100644 --- a/internal/handler/google_settings.go +++ b/internal/handler/google_settings.go @@ -7,7 +7,6 @@ import ( "fmt" "net/http" - "github.com/calnode/calnode/internal/calendar" "github.com/calnode/calnode/internal/gcal" "github.com/calnode/calnode/internal/secret" ) @@ -103,7 +102,7 @@ func (h *Handler) PatchGoogleSettings(w http.ResponseWriter, r *http.Request) { h.writeError(w, http.StatusInternalServerError, "internal error") return } - h.SetCalendar(nil) + h.setGoogleCalendar(nil) h.authMu.Lock() h.googleAuth = nil h.authMu.Unlock() @@ -170,9 +169,7 @@ func (h *Handler) PatchGoogleSettings(w http.ResponseWriter, r *http.Request) { h.writeError(w, http.StatusInternalServerError, "failed to initialize calendar client") return } - svc := calendar.NewService(h.db) - svc.Register(gc) - h.SetCalendar(svc) + h.setGoogleCalendar(gc) h.SetGoogleAuth(req.ClientID, resolvedSecret, h.baseURL+"/v1/auth/callback", h.secureCookie) h.logger.Info("google settings: credentials updated and gcal hot-reloaded") }