From 26fde27f5458d656f071e4468b8b7cd8beccbfef Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 22:40:14 -0400 Subject: [PATCH] Forget pushed event ids when another calendar is chosen (Feature 16) Choosing a calendar other than the stored one now clears every plan day's calendar_event_id and edits each plan that had one (revision raised, synced -> changed) in the same transaction as the choice, so a stored event id is always on the chosen calendar and the next push writes the whole week there. PushEvents now runs the push's record under the same account-lock hold as its event writes, so the chosen calendar cannot change between the writes and the sync bookkeeping; a push that read a plan before the choice is refused by its revision check (ErrPlanChanged). Fixes review finding F-01 on task 5885d2b1-cf06-44ff-abf3-242d7763c3c9. Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 17 +++- internal/db/plans.sql.go | 32 +++++++ internal/db/querier.go | 7 ++ internal/gcal/calendars.go | 57 +++++++++--- internal/gcal/connect.go | 4 +- internal/gcal/events.go | 9 +- internal/gcal/events_test.go | 51 ++++++++-- internal/plan/push.go | 80 ++++++++-------- internal/plan/push_test.go | 154 +++++++++++++++++++++++++++++++ internal/suggest/suggest_test.go | 2 +- migrations/queries/plans.sql | 20 ++++ 11 files changed, 364 insertions(+), 69 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ef69fb5..dcf3f12 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 Calendar shows the day as planned; cleared when its dinner changes). | +| `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). | | `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. | @@ -685,19 +685,28 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. - A push writes the days whose `calendar_event_id` is empty (never 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`). - 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 nothing and leaves the status as it was; pushing again updates the - events already written. A plan swapped or redrafted while it was being - pushed (its revision moved) is not recorded: 409 `plan_changed`. + events already written. The record runs under the account lock, in the + same hold as the event writes, so the chosen calendar cannot change + between them. A plan swapped or redrafted while it was being + pushed, or whose events another calendar choice cleared (its revision + moved), is not recorded: 409 `plan_changed`. - Errors: 409 `not_connected`, 409 `no_calendar` (connected, no calendar chosen), 409 `calendar_unavailable` (the chosen calendar was deleted — Google 404 — or is no longer writable — 403 `requiredAccessLevel`; choose another), 502 `reconnect_required` (the grant is refused; the connection is left as it is), 502 `google_failed`, 404 when the week has no plan, 400 `invalid_week`. The push holds the account lock - across its Google requests, like listing and choosing. + across its Google requests and its record, like listing and choosing. - The week's Supply run (Feature 17) is made from a pushed plan: a successful `Push` returns that plan, which is where Feature 17 hooks in. Nothing here generates a shopping list. diff --git a/internal/db/plans.sql.go b/internal/db/plans.sql.go index 60acaa8..8adddf8 100644 --- a/internal/db/plans.sql.go +++ b/internal/db/plans.sql.go @@ -10,6 +10,18 @@ import ( "database/sql" ) +const clearCalendarEvents = `-- name: ClearCalendarEvents :exec +UPDATE plan_days +SET calendar_event_id = NULL, updated_at = ? +WHERE calendar_event_id IS NOT NULL +` + +// Clears every day's calendar event id: none is on the chosen calendar. +func (q *Queries) ClearCalendarEvents(ctx context.Context, updatedAt string) error { + _, err := q.db.ExecContext(ctx, clearCalendarEvents, updatedAt) + return err +} + const createWeekPlan = `-- name: CreateWeekPlan :exec INSERT INTO week_plans (id, week_start, status, ai_note, created_at, updated_at) VALUES (?, ?, 'draft', ?, ?, ?) @@ -249,6 +261,26 @@ func (q *Queries) ListWeekPlanDinnerIDs(ctx context.Context, weekStart string) ( return items, nil } +const markCalendarPlansEdited = `-- name: MarkCalendarPlansEdited :exec +UPDATE week_plans +SET status = CASE WHEN status = 'synced' THEN 'changed' ELSE status END, + revision = revision + 1, + updated_at = ? +WHERE EXISTS ( + SELECT 1 FROM plan_days + WHERE plan_days.plan_id = week_plans.id AND plan_days.calendar_event_id IS NOT NULL +) +` + +// 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. +func (q *Queries) MarkCalendarPlansEdited(ctx context.Context, updatedAt string) error { + _, err := q.db.ExecContext(ctx, markCalendarPlansEdited, updatedAt) + return err +} + const markWeekPlanEdited = `-- name: MarkWeekPlanEdited :exec UPDATE week_plans SET status = CASE WHEN status = 'synced' THEN 'changed' ELSE status END, diff --git a/internal/db/querier.go b/internal/db/querier.go index c8bc416..e3bc615 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -15,6 +15,8 @@ type Querier interface { // Marks the oldest queued job running and returns it. Returns sql.ErrNoRows // when no job is queued. ClaimNextJob(ctx context.Context, now sql.NullString) (Job, error) + // Clears every day's calendar event id: none is on the chosen calendar. + ClearCalendarEvents(ctx context.Context, updatedAt string) error // Drops references to the runs created before the cutoff, so the purge // can delete them. ClearSuggestionBatchAIRunsBefore(ctx context.Context, cutoff string) (int64, error) @@ -170,6 +172,11 @@ type Querier interface { // The ids of the dinners planned in the week starting week_start, Monday // 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. + 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). MarkDinnerTried(ctx context.Context, arg MarkDinnerTriedParams) (int64, error) diff --git a/internal/gcal/calendars.go b/internal/gcal/calendars.go index 0d416cd..dce497a 100644 --- a/internal/gcal/calendars.go +++ b/internal/gcal/calendars.go @@ -71,7 +71,8 @@ func (s *Service) Calendars(ctx context.Context) ([]Calendar, error) { // and name. id must be among Calendars at the time of the call; anything // else, including a calendar chosen earlier that has since been deleted or // is no longer writable, is ErrUnknownCalendar and leaves the stored -// choice as it was. +// choice as it was. Choosing another calendar than the one chosen returns +// every plan pushed to that one to not synced; see saveSelection. func (s *Service) SelectCalendar(ctx context.Context, id string) (Selection, error) { s.account.Lock() defer s.account.Unlock() @@ -84,24 +85,54 @@ func (s *Service) SelectCalendar(ctx context.Context, id string) (Selection, err continue } sel := Selection{CalendarID: c.ID, CalendarName: c.Summary} - n, err := db.New(s.st.Writer).SetIntegrationsGoogleCalendar(ctx, db.SetIntegrationsGoogleCalendarParams{ - CalendarID: sql.NullString{String: sel.CalendarID, Valid: true}, - CalendarName: sql.NullString{String: sel.CalendarName, Valid: true}, - UpdatedAt: store.Timestamp{Time: s.now().UTC().Truncate(time.Millisecond)}.String(), - }) - if err != nil { - return Selection{}, fmt.Errorf("gcal: save calendar: %w", err) - } - if n != 1 { - // The lock is held since the connection was read, so only a - // change outside this service gets here. - return Selection{}, ErrNotConnected + if err := s.saveSelection(ctx, sel); err != nil { + return Selection{}, err } return sel, nil } return Selection{}, ErrUnknownCalendar } +// 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. +func (s *Service) saveSelection(ctx context.Context, sel Selection) error { + return s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + row, err := q.GetIntegrationsGoogle(ctx) + if errors.Is(err, sql.ErrNoRows) { + // The lock is held since the connection was read, so only a + // change outside this service gets here. + return ErrNotConnected + } + if err != nil { + return fmt.Errorf("gcal: read Google integration: %w", err) + } + now := store.Timestamp{Time: s.now().UTC().Truncate(time.Millisecond)}.String() + if _, err := q.SetIntegrationsGoogleCalendar(ctx, db.SetIntegrationsGoogleCalendarParams{ + CalendarID: sql.NullString{String: sel.CalendarID, Valid: true}, + CalendarName: sql.NullString{String: sel.CalendarName, Valid: true}, + UpdatedAt: now, + }); err != nil { + return fmt.Errorf("gcal: save calendar: %w", err) + } + 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 + }) +} + // 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 1311560..2e69046 100644 --- a/internal/gcal/connect.go +++ b/internal/gcal/connect.go @@ -128,8 +128,8 @@ type Service struct { // account serializes every use of and change to the connected account — // a connect's exchange and store, a disconnect's read, revoke and delete, // a calendar listing's read and Calendar API calls, a calendar choice's - // listing and store, an event push's read and event writes — across the - // requests to Google and the database write. Google revokes the whole grant, so a revoke overlapping a + // listing and store, an event push's read, event writes and record — + // across the requests to Google and the database write. Google revokes the whole grant, so a revoke overlapping a // connect would also kill the token the connect stores. account sync.Mutex diff --git a/internal/gcal/events.go b/internal/gcal/events.go index e021ce4..fa63590 100644 --- a/internal/gcal/events.go +++ b/internal/gcal/events.go @@ -85,7 +85,12 @@ type eventTime struct { // ErrNoCalendar, a chosen calendar that was deleted or is no longer // writable is ErrCalendarUnavailable, a grant Google no longer accepts is // ErrReconnect, and any other Google failure is a *CalendarError. -func (s *Service) PushEvents(ctx context.Context, events []Event) error { +// +// Once every event is written, PushEvents returns record(), which the +// caller uses to store what it pushed. record runs under the same hold of +// the account as the writes, so the chosen calendar cannot change between +// them; it is not run when a write fails. +func (s *Service) PushEvents(ctx context.Context, events []Event, record func() error) error { if len(events) == 0 { return errors.New("gcal: no events to push") } @@ -113,7 +118,7 @@ func (s *Service) PushEvents(ctx context.Context, events []Event) error { return err } } - return nil + return record() } // writeEvent inserts e at base, the chosen calendar's events collection, diff --git a/internal/gcal/events_test.go b/internal/gcal/events_test.go index fb238d2..b754419 100644 --- a/internal/gcal/events_test.go +++ b/internal/gcal/events_test.go @@ -22,6 +22,9 @@ func chosen(t *testing.T) *harness { return h } +// noRecord is a PushEvents record that stores nothing. +func noRecord() error { return nil } + // event is a valid event with id. func event(id string) gcal.Event { start := time.Date(2026, 10, 12, 21, 30, 0, 0, time.UTC) @@ -32,12 +35,12 @@ func event(id string) gcal.Event { func TestPushEventsInsertsThenUpdates(t *testing.T) { t.Parallel() h := chosen(t) - if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde0"), event("abcde1")}); err != nil { + if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde0"), event("abcde1")}, noRecord); err != nil { t.Fatalf("PushEvents: %v", err) } renamed := event("abcde0") renamed.Summary = "Dinner: Pot Roast" - if err := h.svc.PushEvents(context.Background(), []gcal.Event{renamed}); err != nil { + if err := h.svc.PushEvents(context.Background(), []gcal.Event{renamed}, noRecord); err != nil { t.Fatalf("PushEvents again: %v", err) } if inserts, updates := h.google.EventWrites(); inserts != 2 || updates != 1 { @@ -67,14 +70,14 @@ func TestPushEventsRefusesInvalidEvents(t *testing.T) { } { ev := event("abcde") edit(&ev) - if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde0"), ev}); err == nil { + if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde0"), ev}, noRecord); err == nil { t.Errorf("%s: PushEvents succeeded", name) } } - if err := h.svc.PushEvents(context.Background(), nil); err == nil { + if err := h.svc.PushEvents(context.Background(), nil, noRecord); err == nil { t.Error("PushEvents of no events succeeded") } - if err := h.svc.PushEvents(context.Background(), []gcal.Event{event(strings.Repeat("v", 1024))}); err != nil { + if err := h.svc.PushEvents(context.Background(), []gcal.Event{event(strings.Repeat("v", 1024))}, noRecord); err != nil { t.Errorf("PushEvents of a 1024-character id: %v", err) } if inserts, _ := h.google.EventWrites(); inserts != 1 { @@ -89,7 +92,10 @@ func TestPushEventsGrantNoLongerAccepted(t *testing.T) { t.Parallel() h := chosen(t) h.google.FailEventWrite(1, http.StatusUnauthorized) - err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}) + err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, func() error { + t.Error("record ran after a failed write") + return nil + }) if !errors.Is(err, gcal.ErrReconnect) { t.Fatalf("PushEvents = %v, want ErrReconnect", err) } @@ -106,13 +112,13 @@ func TestPushEventsGoogleFailureIsRedacted(t *testing.T) { t.Parallel() h := chosen(t) if name == "update" { - if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}); err != nil { + if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, noRecord); err != nil { t.Fatal(err) } } // The fake's failure body quotes the access token. h.google.FailEventWrite(n, http.StatusInternalServerError) - err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}) + err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, noRecord) var calendar *gcal.CalendarError if !errors.As(err, &calendar) { t.Fatalf("PushEvents = %v, want *CalendarError", err) @@ -134,7 +140,7 @@ func TestPushEventsHoldsTheAccount(t *testing.T) { arrived, release := h.google.HoldEvents() defer release() pushed := make(chan error, 1) - go func() { pushed <- h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}) }() + go func() { pushed <- h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, noRecord) }() <-arrived disconnected := make(chan error, 1) go func() { disconnected <- h.svc.Disconnect(context.Background()) }() @@ -154,3 +160,30 @@ func TestPushEventsHoldsTheAccount(t *testing.T) { t.Errorf("Disconnect: %v", err) } } + +// TestPushEventsRecordsWhileTheCalendarIsChosen chooses another calendar +// while a push records: the choice waits for the record, so a push never +// records events on a calendar no longer chosen. +func TestPushEventsRecordsWhileTheCalendarIsChosen(t *testing.T) { + t.Parallel() + h := chosen(t) + selected := make(chan error, 1) + err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, func() error { + go func() { + _, err := h.svc.SelectCalendar(context.Background(), family.ID) + selected <- err + }() + select { + case err := <-selected: + t.Errorf("SelectCalendar finished (%v) while a push was recording", err) + case <-time.After(200 * time.Millisecond): + } + return nil + }) + if err != nil { + t.Fatalf("PushEvents: %v", err) + } + if err := <-selected; err != nil { + t.Errorf("SelectCalendar: %v", err) + } +} diff --git a/internal/plan/push.go b/internal/plan/push.go index 06f05b8..770050b 100644 --- a/internal/plan/push.go +++ b/internal/plan/push.go @@ -25,10 +25,11 @@ const eventLength = 60 * time.Minute // date. var ErrPlanChanged = errors.New("the plan changed while it was being pushed to Calendar; push again") -// Calendar writes events to the household's chosen calendar. In -// production it is *gcal.Service. +// Calendar writes events to the household's chosen calendar, then runs +// record while that calendar stays chosen. In production it is +// *gcal.Service. type Calendar interface { - PushEvents(ctx context.Context, events []gcal.Event) error + PushEvents(ctx context.Context, events []gcal.Event, record func() error) error } // eventIDEncoding is base32hex, lowercase and unpadded: Google's alphabet @@ -62,14 +63,18 @@ type pushDay struct { // pushed, swapped, or redrafted to another dinner since, whose // 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. +// 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. // // 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 // its event id. A push that failed part way records nothing and leaves the // plan's status as it was; since event ids are fixed, pushing again -// updates the events already written rather than adding them twice. A -// plan edited while it was pushed fails with ErrPlanChanged, unrecorded. +// 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. // // The week must have a plan (else ErrNotFound). The calendar's errors are // returned as they are: gcal.ErrNotConnected, gcal.ErrNoCalendar, @@ -90,44 +95,43 @@ func (s *Service) Push(ctx context.Context, weekStart store.LocalDate) (Plan, er for i, d := range days { events[i] = d.event } - if err := s.cal.PushEvents(ctx, events); err != nil { - return Plan{}, fmt.Errorf("plan: push %s to Calendar: %w", weekStart, err) - } var p Plan - err = s.st.WithTx(ctx, func(tx *sql.Tx) error { - q := db.New(tx) - now := store.Now().String() - n, err := q.MarkWeekPlanSynced(ctx, db.MarkWeekPlanSyncedParams{ - CalendarSyncedAt: sql.NullString{String: now, Valid: true}, - UpdatedAt: now, - ID: planID, - Revision: revision, - }) - if err != nil { - return fmt.Errorf("plan: mark %s synced: %w", planID, err) - } - if n != 1 { - return ErrPlanChanged - } - for _, d := range days { - n, err := q.SetPlanDayCalendarEvent(ctx, db.SetPlanDayCalendarEventParams{ - CalendarEventID: sql.NullString{String: d.event.ID, Valid: true}, - UpdatedAt: now, - PlanID: planID, - Date: d.date, + record := func() error { + return s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + now := store.Now().String() + n, err := q.MarkWeekPlanSynced(ctx, db.MarkWeekPlanSyncedParams{ + CalendarSyncedAt: sql.NullString{String: now, Valid: true}, + UpdatedAt: now, + ID: planID, + Revision: revision, }) if err != nil { - return fmt.Errorf("plan: record event of %s of %s: %w", d.date, planID, err) + return fmt.Errorf("plan: mark %s synced: %w", planID, err) } if n != 1 { - return fmt.Errorf("plan: %s has no day %s", planID, d.date) + return ErrPlanChanged } - } - p, err = readPlan(ctx, q, weekStart) - return err - }) - if err != nil { - return Plan{}, err + for _, d := range days { + n, err := q.SetPlanDayCalendarEvent(ctx, db.SetPlanDayCalendarEventParams{ + CalendarEventID: sql.NullString{String: d.event.ID, Valid: true}, + UpdatedAt: now, + PlanID: planID, + Date: d.date, + }) + if err != nil { + return fmt.Errorf("plan: record event of %s of %s: %w", d.date, planID, err) + } + if n != 1 { + return fmt.Errorf("plan: %s has no day %s", planID, d.date) + } + } + p, err = readPlan(ctx, q, weekStart) + return err + }) + } + if err := s.cal.PushEvents(ctx, events, record); err != nil { + return Plan{}, fmt.Errorf("plan: push %s to Calendar: %w", weekStart, err) } return p, nil } diff --git a/internal/plan/push_test.go b/internal/plan/push_test.go index e3557e9..dfad043 100644 --- a/internal/plan/push_test.go +++ b/internal/plan/push_test.go @@ -8,6 +8,7 @@ import ( "maps" "net/http" "regexp" + "slices" "strings" "testing" "time" @@ -477,3 +478,156 @@ func TestPushFailsWhenPlanSwappedWhilePushing(t *testing.T) { } e.assertUnsynced(t, week, plan.StatusDraft) } + +// kitchenCal is the calendar the tests choose after pushing to dinnersCal. +var kitchenCal = gcaltest.Calendar{ID: "kitchen@group.calendar.google.com", Summary: "Kitchen", AccessRole: "owner"} + +// assertWeekOn fails unless calendar shows each of names, Monday first, on +// its day of the week starting weekStart, and each day stores that event's +// id. +func (e *env) assertWeekOn(t *testing.T, calendar string, weekStart store.LocalDate, dates []string, names ...string) { + t.Helper() + events := e.google.Events(calendar) + stored := e.eventIDs(t, weekStart) + for i, date := range dates { + id := plan.EventID(weekStart, mustDate(date)) + if ev := events[id]; ev.Summary != "Dinner: "+names[i] || ev.Status != "confirmed" { + t.Errorf("%s on %s = %q %s, want \"Dinner: %s\" confirmed", date, calendar, ev.Summary, ev.Status, names[i]) + } + if stored[date] != id { + t.Errorf("%s stored event id %q, want %s", date, stored[date], id) + } + } +} + +func TestPushAfterChoosingAnotherCalendarWritesTheWholeWeek(t *testing.T) { + for _, swap := range []bool{false, true} { + name := "push again" + if swap { + name = "swap then push" + } + t.Run(name, func(t *testing.T) { + e := newEnv(t) + e.connectCalendar(t) + dates := e.storePlan(t, week, fullWeek...) + first, err := e.svc.Push(t.Context(), week) + if err != nil { + t.Fatal(err) + } + onDinners := e.google.Events(dinnersCal.ID) + + // Choosing the same calendar again keeps the plan synced. + e.google.SetCalendars(0, dinnersCal, kitchenCal) + if _, err := e.gcal.SelectCalendar(t.Context(), dinnersCal.ID); err != nil { + t.Fatal(err) + } + if p, err := e.svc.Get(t.Context(), week); err != nil || p.Status != plan.StatusSynced { + t.Fatalf("plan after choosing the same calendar = %s, %v; want synced", p.Status, err) + } + + if _, err := e.gcal.SelectCalendar(t.Context(), kitchenCal.ID); err != nil { + t.Fatal(err) + } + // The plan's events are on a calendar no longer chosen. + p, err := e.svc.Get(t.Context(), week) + if err != nil { + t.Fatal(err) + } + if p.Status != plan.StatusChanged || *p.CalendarSyncedAt != *first.CalendarSyncedAt { + t.Errorf("plan after choosing another calendar = %s synced at %v, want changed", p.Status, *p.CalendarSyncedAt) + } + for date, id := range e.eventIDs(t, week) { + if id != "" { + t.Errorf("%s kept event id %q of the calendar no longer chosen", date, id) + } + } + + names := slices.Clone(fullWeek) + if swap { + fishTacos := e.dinner(t, "Fish Tacos", "regular", "stovetop", 25) + if _, err := e.svc.Swap(t.Context(), week, "2026-10-14", fishTacos); err != nil { + t.Fatal(err) + } + names[2] = "Fish Tacos" + } + + // A push that fails part way leaves the plan changed. + e.google.ResetEventWrites() + e.google.FailEventWrite(4, http.StatusServiceUnavailable) + if _, err := e.svc.Push(t.Context(), week); err == nil { + t.Fatal("Push succeeded, want the fourth event to fail") + } + if p, err := e.svc.Get(t.Context(), week); err != nil || p.Status != plan.StatusChanged { + t.Errorf("plan after a failed push = %s, %v; want changed", p.Status, err) + } + + p, err = e.svc.Push(t.Context(), week) + if err != nil { + t.Fatalf("Push: %v", err) + } + if p.Status != plan.StatusSynced { + t.Errorf("status = %s, want synced", p.Status) + } + if n := len(e.google.Events(kitchenCal.ID)); n != 7 { + t.Errorf("%s has %d events, want 7", kitchenCal.ID, n) + } + e.assertWeekOn(t, kitchenCal.ID, week, dates, names...) + if after := e.google.Events(dinnersCal.ID); !maps.Equal(onDinners, after) { + t.Errorf("%s changed:\nbefore %+v\nafter %+v", dinnersCal.ID, onDinners, after) + } + }) + } +} + +// TestChoosingAnotherCalendarWaitsForAPush chooses another calendar while a +// push is at Google: the choice waits until the push is recorded, then +// forgets its events, so the next push writes the whole week to the +// calendar now chosen. +func TestChoosingAnotherCalendarWaitsForAPush(t *testing.T) { + e := newEnv(t) + e.connectCalendar(t) + dates := e.storePlan(t, week, fullWeek...) + e.google.SetCalendars(0, dinnersCal, kitchenCal) + arrived, release := e.google.HoldEvents() + defer release() + pushed := make(chan error, 1) + go func() { + _, err := e.svc.Push(t.Context(), week) + pushed <- err + }() + <-arrived + selected := make(chan error, 1) + go func() { + _, err := e.gcal.SelectCalendar(t.Context(), kitchenCal.ID) + selected <- err + }() + select { + case err := <-selected: + t.Fatalf("SelectCalendar 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 := <-selected; err != nil { + t.Fatalf("SelectCalendar: %v", err) + } + p, err := e.svc.Get(t.Context(), week) + if err != nil { + t.Fatal(err) + } + if p.Status != plan.StatusChanged { + t.Errorf("plan = %s, want changed: its events are on %s", p.Status, dinnersCal.ID) + } + for date, id := range e.eventIDs(t, week) { + if id != "" { + t.Errorf("%s kept event id %q of the calendar no longer chosen", date, id) + } + } + + if p, err = e.svc.Push(t.Context(), week); err != nil || p.Status != plan.StatusSynced { + t.Fatalf("Push = %s, %v; want synced", p.Status, err) + } + e.assertWeekOn(t, kitchenCal.ID, week, dates, fullWeek...) +} diff --git a/internal/suggest/suggest_test.go b/internal/suggest/suggest_test.go index eae7111..cac527e 100644 --- a/internal/suggest/suggest_test.go +++ b/internal/suggest/suggest_test.go @@ -1326,6 +1326,6 @@ func TestStartBatchRefusesUnknownTrigger(t *testing.T) { // noCalendar is a plan.Calendar for tests that never push. type noCalendar struct{} -func (noCalendar) PushEvents(context.Context, []gcal.Event) error { +func (noCalendar) PushEvents(context.Context, []gcal.Event, func() error) error { return errors.New("no calendar in this test") } diff --git a/migrations/queries/plans.sql b/migrations/queries/plans.sql index 4e61d17..adece04 100644 --- a/migrations/queries/plans.sql +++ b/migrations/queries/plans.sql @@ -58,6 +58,26 @@ UPDATE week_plans SET status = 'synced', calendar_synced_at = ?, updated_at = ? 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. +UPDATE week_plans +SET status = CASE WHEN status = 'synced' THEN 'changed' ELSE status END, + revision = revision + 1, + updated_at = ? +WHERE EXISTS ( + SELECT 1 FROM plan_days + WHERE plan_days.plan_id = week_plans.id AND plan_days.calendar_event_id IS NOT NULL +); + +-- name: ClearCalendarEvents :exec +-- Clears every day's calendar event id: none is on the chosen calendar. +UPDATE plan_days +SET calendar_event_id = NULL, updated_at = ? +WHERE calendar_event_id IS NOT NULL; + -- name: MarkWeekPlanEdited :exec -- Records an edit of plan id, raising its revision: a synced plan becomes -- changed, any other keeps its status.