diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index dcf3f12..59a4538 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -100,7 +100,7 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`). | `suggestion_batches` | trigger, status, ai_run_id. | | `suggestions` | dinner_id, batch_id, name (the name the dish was suggested under, kept whatever its dinner is later renamed, so a Pass stays an immutable record every later batch dedupes against), reason, cited_regular_ids (JSON), status, decided_at. | | `week_plans` | week_start (unique), status (`draft` / `synced` / `changed`), ai_note, ai_run_id, calendar_synced_at, revision (raised by every swap and stored draft). | -| `plan_days` | plan_id, date (unique per plan), dinner_id, is_new, calendar_event_id (set while the chosen calendar shows the day as planned; cleared when its dinner changes or another calendar is chosen). | +| `plan_days` | plan_id, date (unique per plan), dinner_id, is_new, calendar_event_id (set while the chosen calendar shows the day as planned; cleared when its dinner changes, another calendar is chosen or Google Calendar is disconnected). | | `rotation_reviews` | dinner_id + plan_date (unique), status, decision (`keep` / `drop` / `not_made`), name (set only by a drop: the dinner's name when dropped, kept whatever it is later renamed, so a drop stays among the passed names), decided_at, updated_by. | | `shopping_lists` | week_start (unique), plan_id, title, status (`current` / `archived`), sent_to_chat_at, frozen_at. | | `shopping_groups` / `shopping_items` | One group per dinner/day; item name, qty, needed (bool), position. | @@ -640,8 +640,10 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. `connected_at` and redirects to `/settings/calendar`. A reconnect keeps the chosen calendar and clears `last_sync_error`. - `DELETE /api/v1/integrations/google` (CSRF-checked) revokes the token at - Google, then deletes the row; a token Google reports as `invalid_token` - counts as revoked, and any other revoke failure keeps the connection. + Google, then deletes the row and, in the same transaction, forgets every + pushed event (see Push to Calendar); a token Google reports as + `invalid_token` counts as revoked, and any other revoke failure keeps + the connection and its pushed events. - `GET /api/v1/integrations/google/calendars` trades the stored refresh token for an access token and reads every page of the Calendar API's `users/me/calendarList`, answering `[{id, summary, primary, @@ -686,11 +688,15 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. pushed, swapped, or redrafted to another dinner). When none is, it writes every day again, which puts back anything changed in Calendar. - A stored `calendar_event_id` is always an event on the chosen calendar. - Choosing a calendar other than the stored one clears every day's - `calendar_event_id` in the same transaction as the choice, and edits - each plan that had one (revision raised, `synced` → `changed`), so the - next push writes the whole week to the calendar now chosen and a push - that read such a plan before the choice is refused (409 `plan_changed`). + Choosing a calendar other than the stored one, and disconnecting (which + leaves none chosen; connecting again chooses none either), clear every + day's `calendar_event_id` in the same transaction as the choice or the + delete, and edit each plan that had one (revision raised, `synced` → + `changed`), so the next push writes the whole week to the calendar then + chosen and a push that read such a plan before is refused (409 + `plan_changed`). A reconnect while connected keeps the chosen calendar, + so it keeps the event ids. Nothing else clears the choice or the stored + token: a grant Google refuses leaves the connection as it is. - Only a push that wrote all its events is recorded, in one transaction: the plan becomes `synced` with `calendar_synced_at`, and each day written stores its event id. A push that failed part way records diff --git a/internal/db/plans.sql.go b/internal/db/plans.sql.go index 8adddf8..b429e30 100644 --- a/internal/db/plans.sql.go +++ b/internal/db/plans.sql.go @@ -273,9 +273,9 @@ WHERE EXISTS ( ` // Records that Calendar no longer shows the days with a calendar event id -// as planned, since another calendar is chosen: every plan with such a day -// is edited as MarkWeekPlanEdited does, so a synced plan becomes changed -// and a push that read it before is refused. +// as planned, since another calendar or none is chosen: every plan with +// such a day is edited as MarkWeekPlanEdited does, so a synced plan becomes +// changed and a push that read it before is refused. func (q *Queries) MarkCalendarPlansEdited(ctx context.Context, updatedAt string) error { _, err := q.db.ExecContext(ctx, markCalendarPlansEdited, updatedAt) return err diff --git a/internal/db/querier.go b/internal/db/querier.go index e3bc615..bcfc102 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -173,9 +173,9 @@ type Querier interface { // first. ListWeekPlanDinnerIDs(ctx context.Context, weekStart string) ([]sql.NullString, error) // Records that Calendar no longer shows the days with a calendar event id - // as planned, since another calendar is chosen: every plan with such a day - // is edited as MarkWeekPlanEdited does, so a synced plan becomes changed - // and a push that read it before is refused. + // as planned, since another calendar or none is chosen: every plan with + // such a day is edited as MarkWeekPlanEdited does, so a synced plan becomes + // changed and a push that read it before is refused. MarkCalendarPlansEdited(ctx context.Context, updatedAt string) error // Moves a dinner still in the Quest log to tried; a dinner with any other // status is left as it is (0 rows). diff --git a/internal/gcal/calendars.go b/internal/gcal/calendars.go index dce497a..9ce3436 100644 --- a/internal/gcal/calendars.go +++ b/internal/gcal/calendars.go @@ -96,10 +96,11 @@ func (s *Service) SelectCalendar(ctx context.Context, id string) (Selection, err // saveSelection stores sel as the chosen calendar. Choosing another calendar // than the one stored also forgets every plan day's calendar event id, in // the same transaction, since those events are on the calendar no longer -// chosen: each plan with one is edited, so a synced plan becomes changed and -// its next push writes every day to sel. The caller holds s.account, which a -// push also holds from its event writes through recording them, so a push -// never records events written to another calendar than the chosen one. +// chosen (forgetEvents): each plan with one is edited, so a synced plan +// becomes changed and its next push writes every day to sel. The caller +// holds s.account, which a push also holds from its event writes through +// recording them, so a push never records events written to another +// calendar than the chosen one. func (s *Service) saveSelection(ctx context.Context, sel Selection) error { return s.st.WithTx(ctx, func(tx *sql.Tx) error { q := db.New(tx) @@ -123,16 +124,26 @@ func (s *Service) saveSelection(ctx context.Context, sel Selection) error { if row.CalendarID.Valid && row.CalendarID.String == sel.CalendarID { return nil } - if err := q.MarkCalendarPlansEdited(ctx, now); err != nil { - return fmt.Errorf("gcal: mark plans pushed to the earlier calendar edited: %w", err) - } - if err := q.ClearCalendarEvents(ctx, now); err != nil { - return fmt.Errorf("gcal: forget events on the earlier calendar: %w", err) - } - return nil + return forgetEvents(ctx, q, now) }) } +// forgetEvents records, in the caller's transaction q, that no calendar +// event id stored is on the chosen calendar any more: every plan with one is +// edited (a synced plan becomes changed, and a push that read it before is +// refused), then every id is cleared. Every change that ends the chosen +// calendar — choosing another (saveSelection) or disconnecting +// (Disconnect) — runs it in the transaction that makes the change. +func forgetEvents(ctx context.Context, q *db.Queries, now string) error { + if err := q.MarkCalendarPlansEdited(ctx, now); err != nil { + return fmt.Errorf("gcal: mark plans pushed to the earlier calendar edited: %w", err) + } + if err := q.ClearCalendarEvents(ctx, now); err != nil { + return fmt.Errorf("gcal: forget events on the earlier calendar: %w", err) + } + return nil +} + // calendars lists the writable calendars, each once, with the stored refresh // token. The caller holds s.account. func (s *Service) calendars(ctx context.Context) ([]Calendar, error) { diff --git a/internal/gcal/connect.go b/internal/gcal/connect.go index 2e69046..fc37080 100644 --- a/internal/gcal/connect.go +++ b/internal/gcal/connect.go @@ -342,6 +342,11 @@ func (s *Service) store(ctx context.Context, sealed []byte) error { // connection. A token Google no longer knows (the household already removed // MealQuest's access in their Google account) counts as revoked. Any other // revoke failure keeps the connection so the household can retry. +// +// No calendar is chosen once disconnected, so the same transaction that +// deletes the connection forgets every pushed event (forgetEvents): every +// synced plan becomes changed, and the next push after connecting again and +// choosing a calendar writes the whole week. func (s *Service) Disconnect(ctx context.Context) error { s.account.Lock() defer s.account.Unlock() @@ -359,10 +364,13 @@ func (s *Service) Disconnect(ctx context.Context) error { if err := s.revoke(ctx, string(token)); err != nil { return err } - if err := db.New(s.st.Writer).DeleteIntegrationsGoogle(ctx); err != nil { - return fmt.Errorf("gcal: delete Google integration: %w", err) - } - return nil + return s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + if err := q.DeleteIntegrationsGoogle(ctx); err != nil { + return fmt.Errorf("gcal: delete Google integration: %w", err) + } + return forgetEvents(ctx, q, store.Timestamp{Time: s.now().UTC().Truncate(time.Millisecond)}.String()) + }) } // RevokeError is a revoke Google refused or that failed in transit. Its text diff --git a/internal/plan/push.go b/internal/plan/push.go index 770050b..e002397 100644 --- a/internal/plan/push.go +++ b/internal/plan/push.go @@ -64,8 +64,9 @@ type pushDay struct { // calendar_event_id is empty. When every day has one, Calendar shows the // whole plan and every day is written again, which puts back anything // changed in Calendar since: the app is the source of truth. A stored event -// id is always on the chosen calendar: choosing another clears them all -// (gcal.Service.SelectCalendar), so the next push writes the whole week. +// id is always on the chosen calendar: choosing another +// (gcal.Service.SelectCalendar) or disconnecting (gcal.Service.Disconnect) +// clears them all, so the next push writes the whole week. // // Only a push that wrote every one of those events is recorded: the plan // becomes synced with calendar_synced_at now, and each day written stores @@ -74,7 +75,8 @@ type pushDay struct { // updates the events already written rather than adding them twice. The // push is recorded while the calendar written to is still the chosen one. // A plan edited while it was pushed, or whose events were forgotten by -// choosing another calendar, fails with ErrPlanChanged, unrecorded. +// choosing another calendar or disconnecting, fails with ErrPlanChanged, +// unrecorded. // // The week must have a plan (else ErrNotFound). The calendar's errors are // returned as they are: gcal.ErrNotConnected, gcal.ErrNoCalendar, diff --git a/internal/plan/push_test.go b/internal/plan/push_test.go index dfad043..5f4d44d 100644 --- a/internal/plan/push_test.go +++ b/internal/plan/push_test.go @@ -631,3 +631,124 @@ func TestChoosingAnotherCalendarWaitsForAPush(t *testing.T) { } e.assertWeekOn(t, kitchenCal.ID, week, dates, fullWeek...) } + +// pushedWeeks are the weeks the disconnect tests push: the calendar and the +// connection are the household's, so a disconnect concerns every one. +var pushedWeeks = []store.LocalDate{week, mustDate("2026-10-19"), mustDate("2026-10-26")} + +// assertForgotten fails unless the plan of weekStart is changed and no day +// keeps an event id. +func (e *env) assertForgotten(t *testing.T, weekStart store.LocalDate, why string) { + t.Helper() + p, err := e.svc.Get(t.Context(), weekStart) + if err != nil { + t.Fatal(err) + } + if p.Status != plan.StatusChanged { + t.Errorf("plan of %s %s = %s, want changed", weekStart, why, p.Status) + } + for date, id := range e.eventIDs(t, weekStart) { + if id != "" { + t.Errorf("%s kept event id %q %s", date, id, why) + } + } +} + +func TestDisconnectForgetsEveryPushedWeek(t *testing.T) { + e := newEnv(t) + e.connectCalendar(t) + dates := map[store.LocalDate][]string{} + for _, w := range pushedWeeks { + dates[w] = e.storePlan(t, w, fullWeek...) + if _, err := e.svc.Push(t.Context(), w); err != nil { + t.Fatalf("Push %s: %v", w, err) + } + } + draftWeek := mustDate("2026-11-02") + e.storePlan(t, draftWeek, fullWeek...) + + if err := e.gcal.Disconnect(t.Context()); err != nil { + t.Fatalf("Disconnect: %v", err) + } + for _, w := range pushedWeeks { + e.assertForgotten(t, w, "after disconnecting") + } + e.assertUnsynced(t, draftWeek, plan.StatusDraft) + + // Connecting again chooses no calendar, so nothing is pushed until one + // is chosen; then a push writes the whole week to it. + e.google.Connect(t, e.gcal) + if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrNoCalendar) { + t.Fatalf("Push after connecting again = %v, want ErrNoCalendar", err) + } + e.google.SetCalendars(0, dinnersCal, kitchenCal) + if _, err := e.gcal.SelectCalendar(t.Context(), kitchenCal.ID); err != nil { + t.Fatal(err) + } + for _, w := range pushedWeeks { + e.assertForgotten(t, w, "after connecting again") + p, err := e.svc.Push(t.Context(), w) + if err != nil || p.Status != plan.StatusSynced { + t.Fatalf("Push %s = %s, %v; want synced", w, p.Status, err) + } + e.assertWeekOn(t, kitchenCal.ID, w, dates[w], fullWeek...) + } +} + +// TestDisconnectRefusedKeepsPushedWeeks: a revoke Google refuses keeps the +// connection and its calendar, so every pushed week stays synced to it. +func TestDisconnectRefusedKeepsPushedWeeks(t *testing.T) { + e := newEnv(t) + e.connectCalendar(t) + dates := map[store.LocalDate][]string{} + for _, w := range pushedWeeks { + dates[w] = e.storePlan(t, w, fullWeek...) + if _, err := e.svc.Push(t.Context(), w); err != nil { + t.Fatalf("Push %s: %v", w, err) + } + } + e.google.AnswerRevoke(http.StatusServiceUnavailable, "") + var revokeErr *gcal.RevokeError + if err := e.gcal.Disconnect(t.Context()); !errors.As(err, &revokeErr) { + t.Fatalf("Disconnect = %v, want a *RevokeError", err) + } + for _, w := range pushedWeeks { + if p, err := e.svc.Get(t.Context(), w); err != nil || p.Status != plan.StatusSynced { + t.Errorf("plan of %s after a refused disconnect = %s, %v; want synced", w, p.Status, err) + } + e.assertWeekOn(t, dinnersCal.ID, w, dates[w], fullWeek...) + } +} + +// TestDisconnectWaitsForAPush disconnects while a push is at Google: the +// disconnect waits until the push is recorded, then forgets its events. +func TestDisconnectWaitsForAPush(t *testing.T) { + e := newEnv(t) + e.connectCalendar(t) + e.storePlan(t, week, fullWeek...) + arrived, release := e.google.HoldEvents() + defer release() + pushed := make(chan error, 1) + go func() { + _, err := e.svc.Push(t.Context(), week) + pushed <- err + }() + <-arrived + disconnected := make(chan error, 1) + go func() { + disconnected <- e.gcal.Disconnect(t.Context()) + }() + select { + case err := <-disconnected: + t.Fatalf("Disconnect finished (%v) while a push was at Google", err) + case <-time.After(200 * time.Millisecond): + } + release() + if err := <-pushed; err != nil { + t.Fatalf("Push: %v", err) + } + if err := <-disconnected; err != nil { + t.Fatalf("Disconnect: %v", err) + } + e.assertForgotten(t, week, "after disconnecting") +} diff --git a/migrations/queries/plans.sql b/migrations/queries/plans.sql index adece04..369ec03 100644 --- a/migrations/queries/plans.sql +++ b/migrations/queries/plans.sql @@ -60,9 +60,9 @@ WHERE id = ? AND revision = ?; -- name: MarkCalendarPlansEdited :exec -- Records that Calendar no longer shows the days with a calendar event id --- as planned, since another calendar is chosen: every plan with such a day --- is edited as MarkWeekPlanEdited does, so a synced plan becomes changed --- and a push that read it before is refused. +-- as planned, since another calendar or none is chosen: every plan with +-- such a day is edited as MarkWeekPlanEdited does, so a synced plan becomes +-- changed and a push that read it before is refused. UPDATE week_plans SET status = CASE WHEN status = 'synced' THEN 'changed' ELSE status END, revision = revision + 1,