Skip to content

fix: suppress reminders for cancelled Google events - #133

Open
harley wants to merge 2 commits into
Calnode:mainfrom
harley:fix/google-cancelled-reminders-upstream
Open

harley wants to merge 2 commits into
Calnode:mainfrom
harley:fix/google-cancelled-reminders-upstream

Conversation

@harley

@harley harley commented Oct 8, 2026 •

Copy link
Copy Markdown

Intent

Cancelling a meeting directly in Google Calendar can leave its Calnode booking confirmed, so queued reminders still tell the attendee to attend. This change suppresses the reminder when Google explicitly reports the primary host's stored event as cancelled.

The check uses the recorded provider, calendar and event ID, applies a 10-second request context, and rechecks local booking status after the lookup. Lookup errors follow the existing job retry policy instead of authorizing delivery. Saving or clearing Google credentials replaces only Google in a fresh service snapshot, preserving Microsoft/CalDAV registrations, the existing non-Google default provider, and in-flight operations.

Risks and limits

This fixes reminder delivery only. It does not change booking status, release availability, trigger refunds, or send cancellation messages. Only Google supports the new read check; other registered providers retain their current behavior. A missing connection, changed Google account, 404/410, or persistent API outage can exhaust retries and fail a reminder for an otherwise active booking. Secondary hosts' calendar copies do not decide cancellation.

The same fix is prepared for the maintained CoderPush fork in harley#6. This upstream PR contains only the reminder fix and no fork-specific configuration or migrations.

Validation

  • Full Go test suite and go vet ./... passed.
  • Frontend production build and backend build passed.
  • Regression coverage: explicit external cancellation with local status still confirmed, active event, lookup failure and recovery, no recorded event, unconfigured service, and local cancellation during lookup.
  • Settings reload regressions exercise the real settings handler and reminder worker: Microsoft/CalDAV reminders send after saving and clearing Google credentials; default-provider choice and in-flight snapshots are preserved; unchecked Google reminders remain blocked.
  • Provider tests cover cancelled/confirmed/tentative status, malformed payloads, 403/404/410/429/500, disconnected credentials, stored-provider routing and unsupported providers.

No live booking or test email was created. The affected historical booking's current database row was not inspected.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA. ✅
Posted by the CLA Assistant Lite bot.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one rough edge worth a look.

Reviewed changes

  • Worker seam — worker.WithReminderCheck adds a callback run before sendReminder sends; false suppresses, error follows the job's retry policy. sendReminder runs it under a 10s deadline and re-reads bookings.status afterwards to catch a cancellation during the lookup.
  • Handler check — new Handler.ReminderAllowed loads the row where booking_hosts.user_id = bookings.host_id with a non-empty external_event_id (primary host only) and returns deliver when no such row exists.
  • Provider capability — calendar.EventCancellationReader plus Service.EventCancelled, routed through the same providerForEvent path as updates/cancels. Providers without the capability (Microsoft, CalDAV) return (false, nil), preserving delivery.
  • Google read — gcal.Client.EventCancelled GETs .../events/{id}?fields=status via DestinationClient, using the recorded calendar id when present. Only status:"cancelled" suppresses; confirmed/tentative deliver; everything else errors.
  • Tests + docs — gcal status matrix, calendar routing, worker+handler integration, and an ARCHITECTURE §12 note.

Verified the load-bearing Google contract independently: a retained cancelled event returns HTTP 200 with status:"cancelled" from events.get, so suppression works in the normal window (Google purges cancelled events after ~30 days, after which the lookup degrades to the documented fail-closed error).

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash | 𝕏

Comment thread internal/calendar/calendar.go
@harley

harley commented Oct 8, 2026

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Oct 8, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (3ede0fb), which addresses the hot-reload concern raised there.

  • Google-only service reload — new Handler.setGoogleCalendar builds a fresh calendar.Service, preserving the old non-Google primary plus every other registered provider and either replacing or removing only google. PatchGoogleSettings now routes both save and clear through it instead of SetCalendar(nil) / a gcal-only rebuild.
  • Snapshot safety — the replacement is published under calMu while reading the old snapshot, so the previous provider map is never mutated and in-flight operations keep resolving on it.
  • Regression tests — calendar_reload_test.go covers primary/snapshot preservation; google_calendar_reload_test.go drives the real settings handler then the real reminder worker and asserts Microsoft/CalDAV reminders still send after both credential save and clear, plus continued refusal to send an unverifiable Google reminder.

Verified the prior concern is closed: clearing or saving Google credentials no longer drops Microsoft/CalDAV registrations, so those bookings' reminders still deliver. go test for the touched packages and go vet pass at fc5d5b5.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant