From 80a8dd0edf76805e112403f7c34372b92e1552ab Mon Sep 17 00:00:00 2001 From: dev_agent Date: Fri, 9 Oct 2026 16:26:19 -0400 Subject: [PATCH] Make new_suggestions and Send to Google Chat recoverable MQ-167-01: a scheduled or manual batch now emits new_suggestions:{batchId} in the transaction that first stores its Side quests, so the cards and their notification commit together. A later round that fails, or a failure recording the batch's end, no longer loses the event; a later round's emit is a no-op by key; a notification that cannot be stored fails that store, leaving no unannounced cards. Replenish batches still never notify. finishBatch goes back to recording status only. MQ-167-02: migration 0017 adds shopping_lists.chat_send_unconfirmed_at. SendToChat records the send as begun before posting, clears it with sent_to_chat_at once Google Chat accepts, and clears it alone when Chat refuses. A post Chat does not answer, or one it accepts that cannot be recorded, is ErrSendUnconfirmed (502 chat_unconfirmed, not retryable) and leaves the marker, as does a crash mid-send; a repeat without resend never reposts, and resend posts again. The Supply run screen re-reads the list after a failed send, shows an unconfirmed send, offers no Retry for it, and sends again only with resend. Task: f0cd9d50-9c11-41be-a945-7bae9f5f28a4 Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 31 +++--- internal/db/models.go | 19 ++-- internal/db/querier.go | 10 +- internal/db/shopping.sql.go | 58 +++++++++-- internal/server/shopping.go | 34 ++++--- internal/server/shopping_test.go | 37 ++++--- internal/shopping/chat.go | 50 ++++++++-- internal/shopping/notify_test.go | 89 +++++++++++++++-- internal/shopping/shopping.go | 27 +++-- internal/store/migrate_test.go | 41 +++++++- internal/suggest/batch.go | 90 ++++++++--------- internal/suggest/suggest.go | 36 +++---- internal/suggest/suggest_test.go | 99 ++++++++++++++++++- ...17_shopping_list_chat_send_unconfirmed.sql | 10 ++ migrations/queries/shopping.sql | 22 ++++- web/src/App.test.tsx | 1 + web/src/plan/PlanPage.test.tsx | 1 + web/src/shopping/SupplyArchivePage.test.tsx | 1 + web/src/shopping/SupplyRunPage.module.css | 4 + web/src/shopping/SupplyRunPage.test.tsx | 44 +++++++++ web/src/shopping/SupplyRunPage.tsx | 32 +++++- web/src/shopping/api.ts | 24 +++-- web/src/shopping/offlineQueue.test.tsx | 1 + 23 files changed, 586 insertions(+), 175 deletions(-) create mode 100644 migrations/0017_shopping_list_chat_send_unconfirmed.sql diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7c004aa..7ab3287 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -104,7 +104,7 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`). | `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, 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 (the AI-written post title; NULL until written or when the AI fails, when the week label is shown instead; never empty), status (`current` / `archived`), sent_to_chat_at, frozen_at. | +| `shopping_lists` | week_start (unique), plan_id, title (the AI-written post title; NULL until written or when the AI fails, when the week label is shown instead; never empty), status (`current` / `archived`), sent_to_chat_at, chat_send_unconfirmed_at (a send to Google Chat recorded as begun before posting and cleared once its outcome is known; left set when it may or may not have posted), frozen_at. | | `shopping_groups` / `shopping_items` | One group per plan day with ingredients: dinner_id, date, position (by date). Item name, qty (the display quantity, NULL when the recipe gives none), needed (bool), position. Made by `shopping.Generate` from the plan, with no model call: an item is a grounded recipe ingredient's normalized name with qty and unit joined ("2 cups"), or a manual recipe's line as typed with no qty. Regenerating keeps the id and needed flag of an item whose (dinner_id, name) is unchanged. | | `ai_settings` | One row: provider, base_url, model, temperature, system_prompt, api_key_ct, api_key_last4, key_verified_at. | | `recipe_search_settings` | One row: provider (`searxng` / `brave`), base_url (the SearXNG instance), api_key_ct and api_key_last4 (Brave). | @@ -669,8 +669,11 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. run is first created, by `POST /shopping/{weekStart}/generate` or after Push to Calendar; remaking it does not notify. - `new_suggestions:{batchId}` (URL `/discover`) when a `scheduled` or - `manual` suggestion batch ends done having added Side quests, in the - transaction recording it done. A `replenish` batch never notifies. + `manual` suggestion batch first adds Side quests to the deck, in the + transaction storing them, so they are announced even when a later + round or recording the batch's end fails; it names that round's Side + quests, and a later round's emit stores nothing. A `replenish` batch + never notifies. - `day_of_reminder:{date}` (URL `/dinner/{id}`) through `plan.Service.EmitDayOfReminder(date)`, which names the dinner planned on the date and emits nothing for a week with no plan. The scheduler @@ -889,7 +892,8 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. ## Supply run API (Feature 17) - `GET /api/v1/shopping/{weekStart}` returns `{weekStart, label, title, - status, sent_to_chat_at, needed_count, total_count, groups: [{dinner: + status, sent_to_chat_at, chat_send_unconfirmed_at, needed_count, + total_count, groups: [{dinner: {id, name}, date, day, items: [{id, name, qty, needed}]}]}`: `label` is the week label (shown while `title` is null), `day` the plan day's weekday ("Monday"), `qty` null when the recipe gives none, @@ -905,13 +909,18 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. its title (or week label), each dinner with items still needed and those items, and a card whose "Open" button links to `/plan/shopping?week={weekStart}` under `MEALQUEST_PUBLIC_URL`. It - records `sent_to_chat_at` and answers with the Supply run as `GET` gives - it. A week already sent is answered as it is, with no second post, - unless `resend` is true. No webhook stored is 409 `not_configured`; - Google Chat not answering is 502 `chat_unreachable` (retryable) and - answering with a status other than 2xx 502 `chat_refused` (retryable - only for 429 or 5xx), and neither - records `sent_to_chat_at`. 404 when the week has no Supply run, 409 + records `chat_send_unconfirmed_at` before posting, then + `sent_to_chat_at` (clearing it) once Google Chat accepts, and answers + with the Supply run as `GET` gives it. A week already sent, or with a + send unconfirmed, is answered as it is, with no second post, unless + `resend` is true. No webhook stored is 409 `not_configured`; Google Chat + answering with a status other than 2xx is 502 `chat_refused` (retryable + only for 429 or 5xx), clearing the send begun; Google Chat not + answering, or accepting a post that cannot then be recorded, is 502 + `chat_unconfirmed` (not retryable: the post may have been made), which + leaves `chat_send_unconfirmed_at` set, as does the process stopping + mid-send. The screen shows an unconfirmed send and sends again only on + request. 404 when the week has no Supply run, 409 `archived` when it is in the Supply archive, 400 `invalid_request` for a bad body. `shopping.SendToChat(weekStart, resend)` is what `shopping_chat_autosend` calls (Feature 20). diff --git a/internal/db/models.go b/internal/db/models.go index 464b6b0..d451225 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -276,15 +276,16 @@ type ShoppingItem struct { } type ShoppingList struct { - ID string - WeekStart string - PlanID string - Title sql.NullString - Status string - SentToChatAt sql.NullString - FrozenAt sql.NullString - CreatedAt string - UpdatedAt string + ID string + WeekStart string + PlanID string + Title sql.NullString + Status string + SentToChatAt sql.NullString + FrozenAt sql.NullString + CreatedAt string + UpdatedAt string + ChatSendUnconfirmedAt sql.NullString } type Suggestion struct { diff --git a/internal/db/querier.go b/internal/db/querier.go index fbbab33..c02c42f 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -12,6 +12,10 @@ import ( type Querier interface { // An unknown id returns sql.ErrNoRows. ArchiveDinner(ctx context.Context, arg ArchiveDinnerParams) (Dinner, error) + // Records a send of list id to Google Chat begun at + // chat_send_unconfirmed_at, before it is posted, unless the list is frozen + // into the Supply archive. + BeginShoppingListChatSend(ctx context.Context, arg BeginShoppingListChatSendParams) (int64, error) // Leases the pending notification due longest ago by moving its // next_attempt_at to lease_until, and returns it. A claimed row is not due // again until the lease ends, so no other claim takes it while it is being @@ -26,6 +30,8 @@ type Querier interface { // sets last_sync_error, keeping the chosen calendar. Affects no row when // nothing is connected. ClearIntegrationsGoogleToken(ctx context.Context, arg ClearIntegrationsGoogleTokenParams) (int64, error) + // Clears the send of list id begun, once Google Chat refused it. + ClearShoppingListChatSend(ctx context.Context, arg ClearShoppingListChatSendParams) (int64, error) // Drops references to the runs created before the cutoff, so the purge // can delete them. ClearSuggestionBatchAIRunsBefore(ctx context.Context, cutoff string) (int64, error) @@ -235,8 +241,8 @@ type Querier interface { // Records a delivered notification; updated_at is when it was sent. The // lease must still be the claim's. MarkNotificationSent(ctx context.Context, arg MarkNotificationSentParams) (int64, error) - // Records list id sent to Google Chat at sent_to_chat_at, unless it is - // frozen into the Supply archive. + // Records list id sent to Google Chat at sent_to_chat_at, its send + // confirmed, unless it is frozen into the Supply archive. MarkShoppingListSentToChat(ctx context.Context, arg MarkShoppingListSentToChatParams) (int64, error) // Records an edit of plan id, raising its revision: a synced plan becomes // changed, any other keeps its status. diff --git a/internal/db/shopping.sql.go b/internal/db/shopping.sql.go index eb8e921..91b1b9b 100644 --- a/internal/db/shopping.sql.go +++ b/internal/db/shopping.sql.go @@ -10,6 +10,49 @@ import ( "database/sql" ) +const beginShoppingListChatSend = `-- name: BeginShoppingListChatSend :execrows +UPDATE shopping_lists +SET chat_send_unconfirmed_at = ?, updated_at = ? +WHERE id = ? AND status = 'current' AND frozen_at IS NULL +` + +type BeginShoppingListChatSendParams struct { + ChatSendUnconfirmedAt sql.NullString + UpdatedAt string + ID string +} + +// Records a send of list id to Google Chat begun at +// chat_send_unconfirmed_at, before it is posted, unless the list is frozen +// into the Supply archive. +func (q *Queries) BeginShoppingListChatSend(ctx context.Context, arg BeginShoppingListChatSendParams) (int64, error) { + result, err := q.db.ExecContext(ctx, beginShoppingListChatSend, arg.ChatSendUnconfirmedAt, arg.UpdatedAt, arg.ID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const clearShoppingListChatSend = `-- name: ClearShoppingListChatSend :execrows +UPDATE shopping_lists +SET chat_send_unconfirmed_at = NULL, updated_at = ? +WHERE id = ? +` + +type ClearShoppingListChatSendParams struct { + UpdatedAt string + ID string +} + +// Clears the send of list id begun, once Google Chat refused it. +func (q *Queries) ClearShoppingListChatSend(ctx context.Context, arg ClearShoppingListChatSendParams) (int64, error) { + result, err := q.db.ExecContext(ctx, clearShoppingListChatSend, arg.UpdatedAt, arg.ID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + const createShoppingGroup = `-- name: CreateShoppingGroup :exec INSERT INTO shopping_groups (id, list_id, dinner_id, date, position, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?) @@ -135,7 +178,7 @@ func (q *Queries) FreezeShoppingList(ctx context.Context, arg FreezeShoppingList } const getShoppingItemList = `-- name: GetShoppingItemList :one -SELECT shopping_lists.id, shopping_lists.week_start, shopping_lists.plan_id, shopping_lists.title, shopping_lists.status, shopping_lists.sent_to_chat_at, shopping_lists.frozen_at, shopping_lists.created_at, shopping_lists.updated_at +SELECT shopping_lists.id, shopping_lists.week_start, shopping_lists.plan_id, shopping_lists.title, shopping_lists.status, shopping_lists.sent_to_chat_at, shopping_lists.frozen_at, shopping_lists.created_at, shopping_lists.updated_at, shopping_lists.chat_send_unconfirmed_at FROM shopping_items JOIN shopping_groups ON shopping_groups.id = shopping_items.group_id JOIN shopping_lists ON shopping_lists.id = shopping_groups.list_id @@ -161,12 +204,13 @@ func (q *Queries) GetShoppingItemList(ctx context.Context, id string) (GetShoppi &i.ShoppingList.FrozenAt, &i.ShoppingList.CreatedAt, &i.ShoppingList.UpdatedAt, + &i.ShoppingList.ChatSendUnconfirmedAt, ) return i, err } const getShoppingList = `-- name: GetShoppingList :one -SELECT id, week_start, plan_id, title, status, sent_to_chat_at, frozen_at, created_at, updated_at FROM shopping_lists +SELECT id, week_start, plan_id, title, status, sent_to_chat_at, frozen_at, created_at, updated_at, chat_send_unconfirmed_at FROM shopping_lists WHERE week_start = ? ` @@ -185,6 +229,7 @@ func (q *Queries) GetShoppingList(ctx context.Context, weekStart string) (Shoppi &i.FrozenAt, &i.CreatedAt, &i.UpdatedAt, + &i.ChatSendUnconfirmedAt, ) return i, err } @@ -322,7 +367,7 @@ func (q *Queries) ListShoppingItems(ctx context.Context, listID string) ([]ListS } const listShoppingListsThrough = `-- name: ListShoppingListsThrough :many -SELECT id, week_start, plan_id, title, status, sent_to_chat_at, frozen_at, created_at, updated_at FROM shopping_lists +SELECT id, week_start, plan_id, title, status, sent_to_chat_at, frozen_at, created_at, updated_at, chat_send_unconfirmed_at FROM shopping_lists WHERE week_start <= ? ORDER BY week_start DESC ` @@ -348,6 +393,7 @@ func (q *Queries) ListShoppingListsThrough(ctx context.Context, weekStart string &i.FrozenAt, &i.CreatedAt, &i.UpdatedAt, + &i.ChatSendUnconfirmedAt, ); err != nil { return nil, err } @@ -364,7 +410,7 @@ func (q *Queries) ListShoppingListsThrough(ctx context.Context, weekStart string const markShoppingListSentToChat = `-- name: MarkShoppingListSentToChat :execrows UPDATE shopping_lists -SET sent_to_chat_at = ?, updated_at = ? +SET sent_to_chat_at = ?, chat_send_unconfirmed_at = NULL, updated_at = ? WHERE id = ? AND status = 'current' AND frozen_at IS NULL ` @@ -374,8 +420,8 @@ type MarkShoppingListSentToChatParams struct { ID string } -// Records list id sent to Google Chat at sent_to_chat_at, unless it is -// frozen into the Supply archive. +// Records list id sent to Google Chat at sent_to_chat_at, its send +// confirmed, unless it is frozen into the Supply archive. func (q *Queries) MarkShoppingListSentToChat(ctx context.Context, arg MarkShoppingListSentToChatParams) (int64, error) { result, err := q.db.ExecContext(ctx, markShoppingListSentToChat, arg.SentToChatAt, arg.UpdatedAt, arg.ID) if err != nil { diff --git a/internal/server/shopping.go b/internal/server/shopping.go index 565be55..c5bb173 100644 --- a/internal/server/shopping.go +++ b/internal/server/shopping.go @@ -29,6 +29,9 @@ type supplyRun struct { Title *string `json:"title"` Status string `json:"status"` SentToChatAt *string `json:"sent_to_chat_at"` + // ChatSendUnconfirmedAt is when a send to Google Chat whose outcome is + // not known began. + ChatSendUnconfirmedAt *string `json:"chat_send_unconfirmed_at"` shopping.Counts Groups []supplyRunGroup `json:"groups"` } @@ -132,11 +135,13 @@ func postSupplyRunGenerate(logger *slog.Logger, svc *shopping.Service) http.Hand // postSupplyRunSendChat posts the week's Supply run to Google Chat and // answers with it, sent_to_chat_at recorded. Its body is a JSON object, -// {} or {"resend": true}: a week already sent is answered as it is, -// without a second post, unless resend is true. No webhook stored is 409 -// not_configured; Google Chat not answering is 502 chat_unreachable, -// retryable, and refusing the message 502 chat_refused, retryable only for -// 429 or 5xx. +// {} or {"resend": true}: a week already sent, or with a send +// unconfirmed, is answered as it is, without a second post, unless resend +// is true. No webhook stored is 409 not_configured; a send whose outcome is +// not known (Google Chat did not answer, or its acceptance could not be +// recorded) 502 chat_unconfirmed, not retryable, since retrying could post +// twice; and Google Chat refusing the message 502 chat_refused, retryable +// only for 429 or 5xx. func postSupplyRunSendChat(logger *slog.Logger, svc *shopping.Service) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { week, ok := weekStartOrFail(w, r, logger) @@ -154,9 +159,9 @@ func postSupplyRunSendChat(logger *slog.Logger, svc *shopping.Service) http.Hand case errors.Is(err, chat.ErrNotConfigured): writeError(w, r, logger, http.StatusConflict, "not_configured", "no Google Chat webhook is configured; add one in Settings") return - case errors.Is(err, chat.ErrUnreachable): + case errors.Is(err, shopping.ErrSendUnconfirmed): logger.WarnContext(r.Context(), "send Supply run to Google Chat", "error", err) - writeAPIError(w, r, logger, http.StatusBadGateway, apiError{Code: "chat_unreachable", Message: "Google Chat did not answer", Retryable: true}) + writeAPIError(w, r, logger, http.StatusBadGateway, apiError{Code: "chat_unconfirmed", Message: shopping.ErrSendUnconfirmed.Error()}) return case errors.As(err, &refused): logger.WarnContext(r.Context(), "send Supply run to Google Chat", "error", err) @@ -287,13 +292,14 @@ func supplyRunOf(week store.LocalDate, l shopping.List) (supplyRun, error) { } } return supplyRun{ - WeekStart: l.WeekStart, - Label: label, - Title: l.Title, - Status: l.Status, - SentToChatAt: l.SentToChatAt, - Counts: l.Counts(), - Groups: groups, + WeekStart: l.WeekStart, + Label: label, + Title: l.Title, + Status: l.Status, + SentToChatAt: l.SentToChatAt, + ChatSendUnconfirmedAt: l.ChatSendUnconfirmedAt, + Counts: l.Counts(), + Groups: groups, }, nil } diff --git a/internal/server/shopping_test.go b/internal/server/shopping_test.go index 2f1caf8..7128449 100644 --- a/internal/server/shopping_test.go +++ b/internal/server/shopping_test.go @@ -83,14 +83,15 @@ func (e plansEnv) archive(t *testing.T) { // supplyRunJSON is a Supply run answer decoded with every field the API // promises. type supplyRunJSON struct { - WeekStart string `json:"weekStart"` - Label string `json:"label"` - Title *string `json:"title"` - Status string `json:"status"` - SentToChatAt *string `json:"sent_to_chat_at"` - NeededCount int `json:"needed_count"` - TotalCount int `json:"total_count"` - Groups []struct { + WeekStart string `json:"weekStart"` + Label string `json:"label"` + Title *string `json:"title"` + Status string `json:"status"` + SentToChatAt *string `json:"sent_to_chat_at"` + ChatSendUnconfirmedAt *string `json:"chat_send_unconfirmed_at"` + NeededCount int `json:"needed_count"` + TotalCount int `json:"total_count"` + Groups []struct { Dinner struct { ID string `json:"id"` Name string `json:"name"` @@ -185,7 +186,7 @@ func TestSupplyRunShape(t *testing.T) { if status != http.StatusOK { t.Fatalf("generate = %d %v", status, raw) } - wantTop := []string{"groups", "label", "needed_count", "sent_to_chat_at", "skipped", "status", "title", "total_count", "weekStart"} + wantTop := []string{"chat_send_unconfirmed_at", "groups", "label", "needed_count", "sent_to_chat_at", "skipped", "status", "title", "total_count", "weekStart"} if got := keys(raw); !reflect.DeepEqual(got, wantTop) { t.Errorf("generate fields = %v, want %v", got, wantTop) } @@ -888,15 +889,19 @@ func TestSupplyRunSendChatErrors(t *testing.T) { if status != http.StatusBadGateway || errorCode(body) != "chat_refused" || body["error"].(map[string]any)["retryable"] != false { t.Errorf("send Google Chat answers 404 = %d %v, want 502 chat_refused, not retryable", status, body) } - e.chat.Close() - status, body = callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/send-chat", `{}`, nil) - if status != http.StatusBadGateway || errorCode(body) != "chat_unreachable" { - t.Errorf("send Google Chat does not answer = %d %v, want 502 chat_unreachable", status, body) - } var l supplyRunJSON callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l) - if l.SentToChatAt != nil { - t.Errorf("sent_to_chat_at after failed sends = %s, want null", *l.SentToChatAt) + if l.SentToChatAt != nil || l.ChatSendUnconfirmedAt != nil { + t.Errorf("after refused sends sent_to_chat_at = %v, chat_send_unconfirmed_at = %v, want both null", l.SentToChatAt, l.ChatSendUnconfirmedAt) + } + e.chat.Close() + status, body = callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/send-chat", `{}`, nil) + if status != http.StatusBadGateway || errorCode(body) != "chat_unconfirmed" || body["error"].(map[string]any)["retryable"] != false { + t.Errorf("send Google Chat does not answer = %d %v, want 502 chat_unconfirmed, not retryable", status, body) + } + callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l) + if l.SentToChatAt != nil || l.ChatSendUnconfirmedAt == nil { + t.Errorf("after an unanswered send sent_to_chat_at = %v, chat_send_unconfirmed_at = %v, want null and set", l.SentToChatAt, l.ChatSendUnconfirmedAt) } e.archive(t) diff --git a/internal/shopping/chat.go b/internal/shopping/chat.go index f109926..93c4c84 100644 --- a/internal/shopping/chat.go +++ b/internal/shopping/chat.go @@ -3,6 +3,7 @@ package shopping import ( "context" "database/sql" + "errors" "fmt" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/chat" @@ -16,10 +17,17 @@ import ( // Chat (chat.PostSupplyRun): each dinner with its items still needed, and a // link to the Supply run. It records sent_to_chat_at and returns the list. // -// A week is sent once: a Supply run already sent is returned as it is, -// without posting again, unless resend is true. A week with no Supply run -// is ErrListNotFound and one frozen into the Supply archive ErrFrozen. A -// post that fails returns chat.PostSupplyRun's error and records nothing. +// A week is sent once: a Supply run already sent, or with a send +// unconfirmed, is returned as it is, without posting again, unless resend +// is true. A week with no Supply run is ErrListNotFound and one frozen into +// the Supply archive ErrFrozen. +// +// The send is recorded as begun, in chat_send_unconfirmed_at, before it is +// posted, so a post whose outcome is lost is never taken for one never +// made. A post Google Chat refuses, or that is never made, returns +// chat.PostSupplyRun's error and clears it. A post Google Chat does not +// answer, or one it accepts that cannot then be recorded, is +// ErrSendUnconfirmed and leaves it, as does the process stopping mid-send. func (s *Service) SendToChat(ctx context.Context, weekStart store.LocalDate, resend bool) (List, error) { s.sending.Lock() defer s.sending.Unlock() @@ -30,7 +38,7 @@ func (s *Service) SendToChat(ctx context.Context, weekStart store.LocalDate, res if l.Status != StatusCurrent { return List{}, ErrFrozen } - if l.SentToChatAt != nil && !resend { + if (l.SentToChatAt != nil || l.ChatSendUnconfirmedAt != nil) && !resend { return l, nil } name, err := household.WeekLabel(weekStart) @@ -54,20 +62,42 @@ func (s *Service) SendToChat(ctx context.Context, weekStart store.LocalDate, res groups[i].Items = append(groups[i].Items, item) } } - if err := s.chat.PostSupplyRun(ctx, weekStart, name, groups); err != nil { - return List{}, err + q := db.New(s.st.Writer) + n, err := q.BeginShoppingListChatSend(ctx, db.BeginShoppingListChatSendParams{ + ChatSendUnconfirmedAt: sql.NullString{String: store.Now().String(), Valid: true}, + UpdatedAt: store.Now().String(), + ID: l.ID, + }) + if err != nil { + return List{}, fmt.Errorf("shopping: record send to Google Chat begun: %w", err) } - // The post is made: record it even if the client has gone. + if n != 1 { + return List{}, ErrFrozen + } + postErr := s.chat.PostSupplyRun(ctx, weekStart, name, groups) + // The post's outcome is known: record it even if the client has gone. write, cancel := jobs.FinalWriteContext(ctx) defer cancel() + if errors.Is(postErr, chat.ErrUnreachable) { + return List{}, fmt.Errorf("%w: %w", ErrSendUnconfirmed, postErr) + } + if postErr != nil { + if _, err := q.ClearShoppingListChatSend(write, db.ClearShoppingListChatSendParams{ + UpdatedAt: store.Now().String(), + ID: l.ID, + }); err != nil { + return List{}, errors.Join(postErr, fmt.Errorf("shopping: clear refused send to Google Chat: %w", err)) + } + return List{}, postErr + } now := store.Now().String() - n, err := db.New(s.st.Writer).MarkShoppingListSentToChat(write, db.MarkShoppingListSentToChatParams{ + n, err = q.MarkShoppingListSentToChat(write, db.MarkShoppingListSentToChatParams{ SentToChatAt: sql.NullString{String: now, Valid: true}, UpdatedAt: now, ID: l.ID, }) if err != nil { - return List{}, fmt.Errorf("shopping: record sent to Google Chat: %w", err) + return List{}, fmt.Errorf("%w: record sent to Google Chat: %w", ErrSendUnconfirmed, err) } if n != 1 { return List{}, ErrFrozen diff --git a/internal/shopping/notify_test.go b/internal/shopping/notify_test.go index 0615750..eb23d45 100644 --- a/internal/shopping/notify_test.go +++ b/internal/shopping/notify_test.go @@ -127,8 +127,8 @@ func TestSendToChat(t *testing.T) { } // TestSendToChatFailuresRecordNothing: a send with no webhook, or one -// Google Chat refuses or does not answer, leaves sent_to_chat_at unset; a -// week with no list, or a frozen one, is refused without posting. +// Google Chat refuses, leaves sent_to_chat_at and chat_send_unconfirmed_at +// unset; a week with no list, or a frozen one, is refused without posting. func TestSendToChatFailuresRecordNothing(t *testing.T) { e := newEnv(t) e.dinner(t, "Beef Chili", ing{"ground beef", "1", "lb"}) @@ -148,12 +148,8 @@ func TestSendToChatFailuresRecordNothing(t *testing.T) { if _, err := e.svc.SendToChat(t.Context(), week, false); !errors.Is(err, chat.ErrRefused) { t.Errorf("SendToChat answered 500 = %v, want chat.ErrRefused", err) } - e.chat.Close() - if _, err := e.svc.SendToChat(t.Context(), week, false); !errors.Is(err, chat.ErrUnreachable) { - t.Errorf("SendToChat unanswered = %v, want chat.ErrUnreachable", err) - } - if l, err := e.svc.Get(t.Context(), week); err != nil || l.SentToChatAt != nil { - t.Errorf("sent_to_chat_at after failed sends = %v, %v, want unset", l.SentToChatAt, err) + if l, err := e.svc.Get(t.Context(), week); err != nil || l.SentToChatAt != nil || l.ChatSendUnconfirmedAt != nil { + t.Errorf("after failed sends sent_to_chat_at = %v, chat_send_unconfirmed_at = %v, %v, want both unset", l.SentToChatAt, l.ChatSendUnconfirmedAt, err) } if _, err := e.st.Writer.Exec(`UPDATE shopping_lists SET status = 'archived', frozen_at = '2026-10-19T04:05:00Z'`); err != nil { @@ -167,3 +163,80 @@ func TestSendToChatFailuresRecordNothing(t *testing.T) { t.Errorf("a frozen list was posted") } } + +// TestSendToChatUnconfirmed: a send whose outcome is not known, because +// Google Chat accepted the post but recording it failed, or did not answer, +// is ErrSendUnconfirmed and leaves the send unconfirmed rather than unsent: +// a send without resend does not post again, and one with resend does, +// confirming it. +func TestSendToChatUnconfirmed(t *testing.T) { + e := newEnv(t) + e.dinner(t, "Beef Chili", ing{"ground beef", "1", "lb"}) + e.plan(t, "Beef Chili") + e.generate(t) + if _, err := e.chatSvc.Put(t.Context(), chattest.Webhook, "Family"); err != nil { + t.Fatal(err) + } + if _, err := e.st.Writer.Exec(`CREATE TRIGGER refuse_sent BEFORE UPDATE OF sent_to_chat_at ON shopping_lists BEGIN SELECT RAISE(ABORT, 'sent refused'); END`); err != nil { + t.Fatal(err) + } + + if _, err := e.svc.SendToChat(t.Context(), week, false); !errors.Is(err, shopping.ErrSendUnconfirmed) { + t.Fatalf("SendToChat accepted but not recorded = %v, want ErrSendUnconfirmed", err) + } + if n := len(e.chat.Posts()); n != 1 { + t.Fatalf("Google Chat got %d posts, want 1", n) + } + l, err := e.svc.Get(t.Context(), week) + if err != nil || l.SentToChatAt != nil || l.ChatSendUnconfirmedAt == nil { + t.Fatalf("after an unrecorded send sent_to_chat_at = %v, chat_send_unconfirmed_at = %v, %v, want unset and set", l.SentToChatAt, l.ChatSendUnconfirmedAt, err) + } + if _, err := e.st.Writer.Exec(`DROP TRIGGER refuse_sent`); err != nil { + t.Fatal(err) + } + + again, err := e.svc.SendToChat(t.Context(), week, false) + if err != nil { + t.Fatal(err) + } + if n := len(e.chat.Posts()); n != 1 || again.ChatSendUnconfirmedAt == nil || *again.ChatSendUnconfirmedAt != *l.ChatSendUnconfirmedAt { + t.Errorf("send again without resend: %d posts, chat_send_unconfirmed_at %v, want 1 post and %s", n, again.ChatSendUnconfirmedAt, *l.ChatSendUnconfirmedAt) + } + + resent, err := e.svc.SendToChat(t.Context(), week, true) + if err != nil { + t.Fatal(err) + } + if n := len(e.chat.Posts()); n != 2 || resent.SentToChatAt == nil || resent.ChatSendUnconfirmedAt != nil { + t.Errorf("resend: %d posts, sent_to_chat_at %v, chat_send_unconfirmed_at %v, want 2 posts, set and unset", n, resent.SentToChatAt, resent.ChatSendUnconfirmedAt) + } + + e.chat.Close() + if _, err := e.svc.SendToChat(t.Context(), week, true); !errors.Is(err, shopping.ErrSendUnconfirmed) || !errors.Is(err, chat.ErrUnreachable) { + t.Errorf("SendToChat unanswered = %v, want ErrSendUnconfirmed and chat.ErrUnreachable", err) + } + if l, err := e.svc.Get(t.Context(), week); err != nil || l.SentToChatAt == nil || *l.SentToChatAt != *resent.SentToChatAt || l.ChatSendUnconfirmedAt == nil { + t.Errorf("after an unanswered resend sent_to_chat_at = %v, chat_send_unconfirmed_at = %v, %v, want %s and set", l.SentToChatAt, l.ChatSendUnconfirmedAt, err, *resent.SentToChatAt) + } +} + +// TestSendToChatPostsOnlyOnceBegun: a send that cannot be recorded as +// begun is not posted. +func TestSendToChatPostsOnlyOnceBegun(t *testing.T) { + e := newEnv(t) + e.dinner(t, "Beef Chili", ing{"ground beef", "1", "lb"}) + e.plan(t, "Beef Chili") + e.generate(t) + if _, err := e.chatSvc.Put(t.Context(), chattest.Webhook, "Family"); err != nil { + t.Fatal(err) + } + if _, err := e.st.Writer.Exec(`CREATE TRIGGER refuse_begin BEFORE UPDATE OF chat_send_unconfirmed_at ON shopping_lists WHEN NEW.chat_send_unconfirmed_at IS NOT NULL BEGIN SELECT RAISE(ABORT, 'begin refused'); END`); err != nil { + t.Fatal(err) + } + if _, err := e.svc.SendToChat(t.Context(), week, false); err == nil { + t.Fatal("SendToChat not recorded as begun succeeded, want an error") + } + if n := len(e.chat.Posts()); n != 0 { + t.Errorf("Google Chat got %d posts of a send not recorded as begun, want 0", n) + } +} diff --git a/internal/shopping/shopping.go b/internal/shopping/shopping.go index 3d26211..595d321 100644 --- a/internal/shopping/shopping.go +++ b/internal/shopping/shopping.go @@ -82,6 +82,12 @@ var ErrItemNotFound = errors.New("no Supply run has that item") // the household's time zone. var ErrWeekNotEnded = errors.New("the week has not ended") +// ErrSendUnconfirmed is a send to Google Chat whose outcome is not known: +// Google Chat did not answer, or accepted the post but recording it failed. +// The post may have been made, so the Supply run keeps the send as +// unconfirmed and is posted again only with resend. +var ErrSendUnconfirmed = errors.New("the shopping list may have reached Google Chat, but that could not be confirmed; check the space before sending again") + // ErrInvalidPeriod is an Archive grouping other than ByWeek, ByMonth and // ByYear. var ErrInvalidPeriod = errors.New("by must be week, month or year") @@ -158,7 +164,11 @@ type List struct { // SentToChatAt is when the list was sent to Google Chat; nil until it // is. SentToChatAt *string `json:"sent_to_chat_at"` - Groups []Group `json:"groups"` + // ChatSendUnconfirmedAt is when a send to Google Chat whose outcome is + // not known began: one under way, or one that may or may not have been + // posted. nil when there is none. + ChatSendUnconfirmedAt *string `json:"chat_send_unconfirmed_at"` + Groups []Group `json:"groups"` } // Counts are how many items of a Supply run are still needed and how many @@ -563,13 +573,14 @@ func frozen(list db.ShoppingList) bool { // listOf is list with groups. func listOf(list db.ShoppingList, groups []Group) List { return List{ - ID: list.ID, - WeekStart: list.WeekStart, - PlanID: list.PlanID, - Title: nullString(list.Title), - Status: list.Status, - SentToChatAt: nullString(list.SentToChatAt), - Groups: groups, + ID: list.ID, + WeekStart: list.WeekStart, + PlanID: list.PlanID, + Title: nullString(list.Title), + Status: list.Status, + SentToChatAt: nullString(list.SentToChatAt), + ChatSendUnconfirmedAt: nullString(list.ChatSendUnconfirmedAt), + Groups: groups, } } diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index f674930..bf09f0c 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -176,8 +176,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed", got) } } @@ -197,8 +197,8 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) { if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed", got) } if !tableExists(t, s, "dinners") { t.Error("table from 0001_init was not created") @@ -889,3 +889,36 @@ func TestMigrateShoppingListUntitledRefusesUndatedGroup(t *testing.T) { t.Error("shopping_groups_copy left behind") } } + +// TestMigrateShoppingListChatSendUnconfirmed applies 0017 over a list +// already sent: it keeps every value, and the list has no send +// unconfirmed. +func TestMigrateShoppingListChatSendUnconfirmed(t *testing.T) { + ctx := context.Background() + s := openTestStore(t) + migrateBefore(t, s, "0017") + for _, stmt := range []string{ + `INSERT INTO week_plans (id, week_start, status, created_at, updated_at) + VALUES ('plan-1', '2026-10-12', 'synced', 'c', 'u')`, + `INSERT INTO shopping_lists (id, week_start, plan_id, title, status, sent_to_chat_at, frozen_at, created_at, updated_at) + VALUES ('list-1', '2026-10-12', 'plan-1', 'Noodle week', 'current', 's', NULL, 'c1', 'u1')`, + } { + if _, err := s.Writer.Exec(stmt); err != nil { + t.Fatalf("seed: %v", err) + } + } + + if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + + var got string + if err := s.Reader.QueryRowContext(ctx, `SELECT + id || '|' || title || '|' || status || '|' || sent_to_chat_at || '|' || COALESCE(chat_send_unconfirmed_at, 'NULL') || '|' || updated_at + FROM shopping_lists`).Scan(&got); err != nil { + t.Fatal(err) + } + if want := "list-1|Noodle week|current|s|NULL|u1"; got != want { + t.Errorf("shopping list after 0017 = %s, want %s", got, want) + } +} diff --git a/internal/suggest/batch.go b/internal/suggest/batch.go index 826ae13..00f26b0 100644 --- a/internal/suggest/batch.go +++ b/internal/suggest/batch.go @@ -15,7 +15,6 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" - "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" ) @@ -58,12 +57,12 @@ func (r batchResult) PublicJSON() ([]byte, error) { return json.Marshal(r) } // shutdown, the job is left running and the batch with it, for the next // start to fail. // -// A scheduled or manual batch that ends done having added Side quests to -// the deck emits new_suggestions, keyed by the batch, in the transaction -// that records it done. A replenish batch, which tops up the deck as the -// household swipes, never notifies. A notification that cannot be stored -// fails the done write with it, as any failure recording the batch's end -// does: the job fails, and its Side quests stay in the deck. +// A scheduled or manual batch emits new_suggestions, keyed by the batch, +// in the transaction that first adds Side quests to the deck, so the +// household is told of every Side quest stored, whether or not the batch +// then ends done: a later round, or recording the batch's end, may fail. +// A replenish batch, which tops up the deck as the household swipes, never +// notifies. See store. func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchResult) error) (batchResult, error) { batch, err := db.New(s.st.Reader).GetSuggestionBatch(ctx, p.BatchID) if err != nil { @@ -79,19 +78,14 @@ func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchRe if n == 0 { return batchResult{}, fmt.Errorf("suggest: batch %s is not pending", p.BatchID) } - f, err := s.fill(ctx, p.BatchID, publish) + res, err := s.fill(ctx, p.BatchID, batch.Trigger != TriggerReplenish, publish) if err == nil { - var ev *notify.Event - if batch.Trigger != TriggerReplenish && len(f.names) > 0 { - e := chat.NewSuggestions(p.BatchID, household.WeekStart(f.today), f.names) - ev = &e - } - if err := s.finishBatch(ctx, p.BatchID, BatchDone, ev); err != nil { + if err := s.finishBatch(ctx, p.BatchID, BatchDone); err != nil { return batchResult{}, err } - return f.res, nil + return res, nil } - finishErr := s.finishBatch(ctx, p.BatchID, BatchFailed, nil) + finishErr := s.finishBatch(ctx, p.BatchID, BatchFailed) if ctx.Err() != nil { return batchResult{}, errors.Join(ctx.Err(), finishErr) } @@ -104,60 +98,56 @@ func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchRe return batchResult{}, errors.Join(err, finishErr) } -// filled is what fill did: the batch's result, the names of the Side -// quests it added to the deck, and the household's date it ran on. -type filled struct { - res batchResult - names []string - today store.LocalDate -} - // fill does runBatch's work, returning a *jobs.Failure for a failure the -// household may see. -func (s *Service) fill(ctx context.Context, batchID string, publish func(batchResult) error) (filled, error) { +// household may see. When notifies is true, store emits new_suggestions +// with the Side quests it adds. +func (s *Service) fill(ctx context.Context, batchID string, notifies bool, publish func(batchResult) error) (batchResult, error) { res := batchResult{BatchID: batchID, Stage: StageSuggesting, SuggestionIDs: []string{}} if err := publish(res); err != nil { - return filled{}, err + return batchResult{}, err } in, err := s.request(ctx) if err != nil { - return filled{}, err + return batchResult{}, err } out, err := s.suggest(ctx, batchID, in) if err != nil { if ctx.Err() != nil { - return filled{}, ctx.Err() + return batchResult{}, ctx.Err() } - return filled{}, &jobs.Failure{Code: ReasonAIFailed} + return batchResult{}, &jobs.Failure{Code: ReasonAIFailed} } res.Stage = StageGrounding res.Dropped = len(out.Dropped) if err := publish(res); err != nil { - return filled{}, err + return batchResult{}, err + } + var week *store.LocalDate + if notifies { + w := household.WeekStart(in.Context.Today) + week = &w } - var names []string left := out.Suggestions for len(res.SuggestionIDs) < BatchSize && len(left) > 0 { round := left[:min(BatchSize-len(res.SuggestionIDs), len(left))] left = left[len(round):] grounded, err := s.ground(ctx, round) if err != nil { - return filled{}, err + return batchResult{}, err } res.Dropped += len(round) - len(grounded) - ids, stored, err := s.store(ctx, batchID, grounded) + ids, err := s.store(ctx, batchID, week, grounded) if err != nil { - return filled{}, err + return batchResult{}, err } res.Dropped += len(grounded) - len(ids) res.SuggestionIDs = append(res.SuggestionIDs, ids...) - names = append(names, stored...) if err := publish(res); err != nil { - return filled{}, err + return batchResult{}, err } } res.Stage = StageDone - return filled{res: res, names: names, today: in.Context.Today}, nil + return res, nil } // request reads what suggest_side_quests is given: the household context, @@ -232,9 +222,15 @@ func (s *Service) ground(ctx context.Context, quests []skills.SideQuest) ([]grou // suggest_side_quests ran, a dinner it repeats was added, a suggestion it // repeats passed or a Rotation check dropped, or a Regular it cites renamed // or archived. Passed names are read in the same transaction as the store, -// so a Pass or drop committed before it is never missed. The names stored -// are returned with the ids. -func (s *Service) store(ctx context.Context, batchID string, quests []groundedQuest) ([]string, []string, error) { +// so a Pass or drop committed before it is never missed. +// +// When week is not nil and a Side quest is stored, new_suggestions:{batchID} +// naming those stored is emitted in the same transaction, for the week +// starting week, so the Side quests and their notification commit together +// or not at all: a notification that cannot be stored fails the store, and +// the batch with it. The event's key makes the first round to store Side +// quests the one that notifies; a later round's emit stores nothing. +func (s *Service) store(ctx context.Context, batchID string, week *store.LocalDate, quests []groundedQuest) ([]string, error) { ids := []string{} var stored []string err := s.st.WithTx(ctx, func(tx *sql.Tx) error { @@ -302,12 +298,18 @@ func (s *Service) store(ctx context.Context, batchID string, quests []groundedQu ids = append(ids, id) stored = append(stored, d.Name) } - return nil + if week == nil || len(stored) == 0 { + return nil + } + return s.notifier.EmitTx(ctx, tx, chat.NewSuggestions(batchID, *week, stored)) }) if err != nil { - return nil, nil, err + return nil, err } - return ids, stored, nil + if week != nil && len(stored) > 0 { + s.notifier.Wake() + } + return ids, nil } // citedIDs returns the ids of the Regulars named cited, false when one diff --git a/internal/suggest/suggest.go b/internal/suggest/suggest.go index e4da046..8042a5f 100644 --- a/internal/suggest/suggest.go +++ b/internal/suggest/suggest.go @@ -257,39 +257,27 @@ func (s *Service) startBatch(ctx context.Context, tx *sql.Tx, trigger string) (B return Batch{ID: row.ID, JobID: jobID, Trigger: row.Trigger, Status: row.Status, Created: true}, nil } -// finishBatch records running batch id as status, and emits ev with it -// when ev is not nil, with jobs.FinalWriteContext: the write is made even -// when ctx has been cancelled, but a writer that stays busy cannot hold its -// worker past jobs.FinalWriteTimeout. Its job's handler returns any error, so the job +// finishBatch records running batch id as status, with +// jobs.FinalWriteContext: the write is made even when ctx has been +// cancelled, but a writer that stays busy cannot hold its worker past +// jobs.FinalWriteTimeout. Its job's handler returns any error, so the job // never ends succeeded with the batch still running; the runner keeps only // a code, so the error is logged too. -func (s *Service) finishBatch(ctx context.Context, id, status string, ev *notify.Event) error { +func (s *Service) finishBatch(ctx context.Context, id, status string) error { write, cancel := jobs.FinalWriteContext(ctx) defer cancel() - err := s.st.WithTx(write, func(tx *sql.Tx) error { - n, err := db.New(tx).FinishSuggestionBatch(write, db.FinishSuggestionBatchParams{ - Status: status, - UpdatedAt: store.Now().String(), - ID: id, - }) - if err != nil { - return err - } - if n != 1 { - return errors.New("batch is not running") - } - if ev == nil { - return nil - } - return s.notifier.EmitTx(write, tx, *ev) + n, err := db.New(s.st.Writer).FinishSuggestionBatch(write, db.FinishSuggestionBatchParams{ + Status: status, + UpdatedAt: store.Now().String(), + ID: id, }) + if err == nil && n != 1 { + err = errors.New("batch is not running") + } if err != nil { s.logger.ErrorContext(ctx, "record suggestion batch status", "batch_id", id, "status", status, "error", err) return fmt.Errorf("suggest: record batch %s %s: %w", id, status, err) } - if ev != nil { - s.notifier.Wake() - } return nil } diff --git a/internal/suggest/suggest_test.go b/internal/suggest/suggest_test.go index 78f32a3..4a0eea7 100644 --- a/internal/suggest/suggest_test.go +++ b/internal/suggest/suggest_test.go @@ -454,6 +454,100 @@ func TestBatchEmitsNewSuggestions(t *testing.T) { } } +// TestBatchNotifiesAsSideQuestsAreStored: new_suggestions is stored with +// the first round to add Side quests, so a batch whose later round fails +// still tells the household of those in the deck, once; a later round that +// succeeds adds no second notification; a replenish batch notifies nothing. +func TestBatchNotifiesAsSideQuestsAreStored(t *testing.T) { + for _, tc := range []struct { + trigger string + lastFails bool + notify bool + }{ + {suggest.TriggerScheduled, true, true}, + {suggest.TriggerManual, true, true}, + {suggest.TriggerManual, false, true}, + {suggest.TriggerReplenish, true, false}, + } { + t.Run(fmt.Sprintf("%s last round fails %v", tc.trigger, tc.lastFails), func(t *testing.T) { + e := newEnv(t) + e.start(t) + // The first round grounds BatchSize Side quests but B; the + // second round grounds F. + var quests []string + for i := range suggest.BatchSize + 1 { + name := fmt.Sprintf("Fajita Dish %c", 'A'+i) + e.search.Results[name] = nil + if i != 1 { + e.recipe(t, name) + } + quests = append(quests, sideQuest(name, "Like your Chicken Fajitas.", "Chicken Fajitas")) + } + e.reply(quests...) + if tc.lastFails { + if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_f BEFORE INSERT ON dinners WHEN NEW.name = 'Fajita Dish F' BEGIN SELECT RAISE(ABORT, 'F refused'); END`); err != nil { + t.Fatal(err) + } + } + + b, err := e.svc.StartBatch(t.Context(), tc.trigger) + if err != nil { + t.Fatal(err) + } + job := e.waitJob(t, b.JobID) + want, deck := jobs.StatusSucceeded, suggest.BatchSize + if tc.lastFails { + want, deck = jobs.StatusFailed, suggest.BatchSize-1 + } + if job.Status != want { + t.Fatalf("batch job %s, want %s", job.Status, want) + } + if got := e.deckNames(t); len(got) != deck { + t.Fatalf("deck = %v, want %d Side quests", got, deck) + } + rows := notifytest.Rows(t, e.st, notify.NewSuggestions) + if !tc.notify { + if len(rows) != 0 { + t.Errorf("new_suggestions rows = %+v, want none", rows) + } + return + } + if len(rows) != 1 || rows[0].Key != "new_suggestions:"+b.ID || + rows[0].Payload.Body != "Fajita Dish A, Fajita Dish C, Fajita Dish D + 1 more" { + t.Errorf("new_suggestions rows = %+v, want one keyed by batch %s naming the first round", rows, b.ID) + } + }) + } +} + +// TestBatchFailsWhenItsNotificationCannotBeStored: Side quests are stored +// only with their notification, so a notification that cannot be stored +// fails the batch with an empty deck rather than leaving Side quests the +// household is never told of. +func TestBatchFailsWhenItsNotificationCannotBeStored(t *testing.T) { + e := newEnv(t) + e.start(t) + e.recipe(t, "Chili Verde") + e.reply(sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili")) + if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_outbox BEFORE INSERT ON notification_outbox BEGIN SELECT RAISE(ABORT, 'outbox refused'); END`); err != nil { + t.Fatal(err) + } + + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if job := e.waitJob(t, b.JobID); job.Status != jobs.StatusFailed { + t.Fatalf("batch job %s, want failed", job.Status) + } + if got := e.batchStatus(t, b.ID); got != suggest.BatchFailed { + t.Errorf("batch status = %s, want failed", got) + } + if got := e.deckNames(t); len(got) != 0 { + t.Errorf("deck = %v with no notification stored, want empty", got) + } +} + func TestBatchKeepsAtMostBatchSize(t *testing.T) { e := newEnv(t) e.start(t) @@ -1204,8 +1298,9 @@ func TestBatchJobFailsWhenRecordingItsEndFails(t *testing.T) { if got := e.batchStatus(t, b.ID); got != suggest.BatchFailed { t.Errorf("batch status = %s with its job failed, want failed", got) } - if rows := notifytest.Rows(t, e.st, notify.NewSuggestions); len(rows) != 0 { - t.Errorf("new_suggestions rows of a batch whose end was not recorded = %+v, want none", rows) + // Its Side quests stay in the deck, and the household was told of them. + if rows := notifytest.Rows(t, e.st, notify.NewSuggestions); len(rows) != 1 || rows[0].Key != "new_suggestions:"+b.ID || rows[0].Payload.Body != "Chili Verde" { + t.Errorf("new_suggestions rows of a batch whose end was not recorded = %+v, want one naming Chili Verde", rows) } } diff --git a/migrations/0017_shopping_list_chat_send_unconfirmed.sql b/migrations/0017_shopping_list_chat_send_unconfirmed.sql new file mode 100644 index 0000000..bc63da5 --- /dev/null +++ b/migrations/0017_shopping_list_chat_send_unconfirmed.sql @@ -0,0 +1,10 @@ +-- 0017_shopping_list_chat_send_unconfirmed: a send of a Supply run to +-- Google Chat is recorded as begun, in chat_send_unconfirmed_at, before it +-- is posted, and cleared when Google Chat's answer is known: with +-- sent_to_chat_at when it accepted the post, alone when it refused it. A +-- value left behind (Google Chat did not answer, the process stopped, or +-- recording the answer failed) means the post may or may not have been +-- made, so it is never posted again without resend. Existing rows have no +-- send under way. + +ALTER TABLE shopping_lists ADD COLUMN chat_send_unconfirmed_at TEXT; diff --git a/migrations/queries/shopping.sql b/migrations/queries/shopping.sql index 99c6d68..b4ade22 100644 --- a/migrations/queries/shopping.sql +++ b/migrations/queries/shopping.sql @@ -89,13 +89,27 @@ UPDATE shopping_lists SET title = ?, updated_at = ? WHERE id = ? AND title IS NULL AND status = 'current' AND frozen_at IS NULL; --- name: MarkShoppingListSentToChat :execrows --- Records list id sent to Google Chat at sent_to_chat_at, unless it is --- frozen into the Supply archive. +-- name: BeginShoppingListChatSend :execrows +-- Records a send of list id to Google Chat begun at +-- chat_send_unconfirmed_at, before it is posted, unless the list is frozen +-- into the Supply archive. UPDATE shopping_lists -SET sent_to_chat_at = ?, updated_at = ? +SET chat_send_unconfirmed_at = ?, updated_at = ? WHERE id = ? AND status = 'current' AND frozen_at IS NULL; +-- name: MarkShoppingListSentToChat :execrows +-- Records list id sent to Google Chat at sent_to_chat_at, its send +-- confirmed, unless it is frozen into the Supply archive. +UPDATE shopping_lists +SET sent_to_chat_at = ?, chat_send_unconfirmed_at = NULL, updated_at = ? +WHERE id = ? AND status = 'current' AND frozen_at IS NULL; + +-- name: ClearShoppingListChatSend :execrows +-- Clears the send of list id begun, once Google Chat refused it. +UPDATE shopping_lists +SET chat_send_unconfirmed_at = NULL, updated_at = ? +WHERE id = ?; + -- name: FreezeShoppingList :execrows -- Freezes list id into the Supply archive at frozen_at, unless it is -- frozen already. diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index 35fcc2c..4f395b1 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -120,6 +120,7 @@ describe('App routes', () => { title: null, status: 'current', sent_to_chat_at: null, + chat_send_unconfirmed_at: null, needed_count: 1, total_count: 1, groups: [{ dinner: { id: 'd1', name: 'Beef tacos' }, date: week, day: 'Monday', items: [{ id: 'i1', name: 'Ground beef', qty: '1 lb', needed: true }] }], diff --git a/web/src/plan/PlanPage.test.tsx b/web/src/plan/PlanPage.test.tsx index 1f027a9..fe90d5f 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -215,6 +215,7 @@ function supplyRun(needed: number) { title: null, status: 'current', sent_to_chat_at: null, + chat_send_unconfirmed_at: null, needed_count: needed, total_count: 32, groups: [], diff --git a/web/src/shopping/SupplyArchivePage.test.tsx b/web/src/shopping/SupplyArchivePage.test.tsx index 34fffc5..0ab4000 100644 --- a/web/src/shopping/SupplyArchivePage.test.tsx +++ b/web/src/shopping/SupplyArchivePage.test.tsx @@ -308,6 +308,7 @@ const FROZEN = { title: 'Pizza, pot roast & the great orzo trial', status: 'archived', sent_to_chat_at: null, + chat_send_unconfirmed_at: null, needed_count: 1, total_count: 2, groups: [ diff --git a/web/src/shopping/SupplyRunPage.module.css b/web/src/shopping/SupplyRunPage.module.css index b682c0c..b8aa5a1 100644 --- a/web/src/shopping/SupplyRunPage.module.css +++ b/web/src/shopping/SupplyRunPage.module.css @@ -238,6 +238,10 @@ color: var(--color-accent-2-800); } +.unconfirmed { + color: var(--color-accent-700); +} + .footer { display: flex; flex-direction: column; diff --git a/web/src/shopping/SupplyRunPage.test.tsx b/web/src/shopping/SupplyRunPage.test.tsx index 773cd10..64a0f70 100644 --- a/web/src/shopping/SupplyRunPage.test.tsx +++ b/web/src/shopping/SupplyRunPage.test.tsx @@ -53,6 +53,7 @@ function supplyRun(overrides: Record = {}, weekStart = '2026-10 title: null, status: 'current', sent_to_chat_at: null, + chat_send_unconfirmed_at: null, needed_count: items.filter((item) => item.needed).length, total_count: items.length, groups: GROUPS, @@ -446,6 +447,49 @@ describe('SupplyRunPage', () => { expect(writes(fetchMock)).toEqual([`POST ${SEND_CHAT} {"resend":false}`, `POST ${SEND_CHAT} {"resend":false}`]) }) + it('says a send whose outcome is not known was not confirmed, offers no retry, and sends again only with resend', async () => { + mockViewport(1180) + let run = supplyRun() + const answers = [ + () => { + // Saturday Oct 10, 8:00 am in New York. + run = supplyRun({ chat_send_unconfirmed_at: '2026-10-10T12:00:00Z' }) + return errorResponse(502, 'chat_unconfirmed') + }, + () => jsonResponse(200, supplyRun({ sent_to_chat_at: '2026-10-10T12:05:00Z' })), + ] + const fetchMock = stubServer({ + [`GET ${THIS_WEEK}`]: () => jsonResponse(200, run), + [`POST ${SEND_CHAT}`]: () => answers.shift()!(), + }) + await renderPage() + + fireEvent.click(await screen.findByRole('button', { name: 'Send to Google Chat' })) + + const alert = await screen.findByRole('alert') + expect(alert).toHaveTextContent("Couldn't send the shopping list to Google Chat. chat_unconfirmed message") + expect(within(alert).queryByRole('button')).not.toBeInTheDocument() + // The Supply run is read again, and says the send is unconfirmed. + expect( + await screen.findByText('Send to Google Chat not confirmed · Sat 8:00 am — check the space before sending again'), + ).toBeInTheDocument() + expect(screen.queryByText(/Sent to Google Chat ·/)).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: 'Send again' })) + expect(await screen.findByText('Sent to Google Chat · Sat 8:05 am')).toBeInTheDocument() + expect(screen.queryByText(/not confirmed/)).not.toBeInTheDocument() + expect(writes(fetchMock)).toEqual([`POST ${SEND_CHAT} {"resend":false}`, `POST ${SEND_CHAT} {"resend":true}`]) + }) + + it('shows an unconfirmed send on mobile', async () => { + stubServer({ [`GET ${THIS_WEEK}`]: () => jsonResponse(200, supplyRun({ chat_send_unconfirmed_at: '2026-10-10T12:00:00Z' })) }) + await renderPage() + + expect( + await screen.findByText('Send to Google Chat not confirmed · Sat 8:00 am — check the space before sending again'), + ).toBeInTheDocument() + }) + it('renders an archived list read-only, tagged Archived', async () => { const fetchMock = stubServer({ 'GET /api/v1/shopping/2026-09-28': () => diff --git a/web/src/shopping/SupplyRunPage.tsx b/web/src/shopping/SupplyRunPage.tsx index a657db5..839e8e2 100644 --- a/web/src/shopping/SupplyRunPage.tsx +++ b/web/src/shopping/SupplyRunPage.tsx @@ -2,6 +2,7 @@ import { Archive, Check, CloudUpload, MessageSquare } from 'lucide-react' import { useState } from 'react' import { Link, useSearchParams } from 'react-router' import { defaultWeek, localNow, weekdayKicker, weekLabel, weekStart as weekOf } from '../plan/week.ts' +import { ApiError } from '../api/errors.ts' import { formatClockTime, useHouseholdSettings } from '../settings/api.ts' import { useBreakpoint, type Breakpoint } from '../shell/useBreakpoint.ts' import { Button } from '../ui/Button.tsx' @@ -101,8 +102,20 @@ function SentStatus({ at, timezone }: { at: Date; timezone: string }) { ) } +// UnconfirmedStatus is the status line while a send to Google Chat whose +// outcome is not known is recorded: it may or may not have been posted. +function UnconfirmedStatus({ at, timezone }: { at: Date; timezone: string }) { + return ( +

+ + Send to Google Chat not confirmed · {sentAt(at, timezone)} — check the space before sending again +

+ ) +} + // SendToChat posts the Supply run to Google Chat: "Send to Google Chat" -// until it is sent, then "Send again", which posts it once more. +// until it is sent, or a send is unconfirmed, then "Send again", which +// posts it once more. function SendToChat({ sent, pending, onSend }: { sent: boolean; pending: boolean; onSend: () => void }) { return (