Make the Supply run from the plan revision Push to Calendar pushed
CI / ci (push) Successful in 5m6s
CI / ci (pull_request) Successful in 5m40s

A plan edit landing between a successful Push and the Generate that
followed it made the Supply run from the newer plan while the push answer
reported supply_run generated, so the list could hold a dinner Calendar
never showed (MQ-133-01).

Push now returns the revision it pushed, and the push hook calls
shopping.GeneratePushed, which checks inside its transaction that the
week's plan is still at that revision (week_plans.revision, as the draft
path does) and otherwise fails with ErrPlanChanged, leaving the Supply run
as it was. The push answer then reports supply_run failed with error
plan_changed (retryable: push again), logged as a warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-08 04:59:16 -04:00
co-authored by Claude Opus 5.5
parent 6aa5113488
commit a31bbce1dc
7 changed files with 168 additions and 53 deletions
+12 -8
View File
@@ -739,14 +739,18 @@ Runs are recorded in `schedule_runs` and executed through `jobs`.
has no plan, 400 `invalid_week`. The push holds the account lock has no plan, 400 `invalid_week`. The push holds the account lock
across its Google requests and its record, 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: after - The week's Supply run (Feature 17) is made from a pushed plan: after
a successful `Push`, the handler runs `shopping.Generate` for the week a successful `Push`, the handler runs `shopping.GeneratePushed` for
and the 200 answer carries `supply_run: {status, error}`. `status` is the week at the plan revision `Push` pushed, and the 200 answer
`generated`; `archived` when the week's Supply run is in the Supply carries `supply_run: {status, error}`. `status` is `generated`;
archive and was left as it was; or `failed` (error `archived` when the week's Supply run is in the Supply archive and was
`supply_run_failed`, retryable, logged as an error) when it could not left as it was; or `failed` — error `plan_changed` (retryable, logged
be made. Generation never fails or undoes the push — Calendar already as a warning) when the plan was edited between the push and
shows the plan — and `POST /api/v1/shopping/{weekStart}/generate` generation, so no Supply run is made from a plan Calendar does not
makes it again. show and pushing again makes it; or error `supply_run_failed`
(retryable, logged as an error) when it could not be made, and
`POST /api/v1/shopping/{weekStart}/generate` makes it again.
Generation checks the revision inside its own transaction, and never
fails or undoes the push — Calendar already shows the plan.
## Supply run API (Feature 17) ## Supply run API (Feature 17)
+7 -7
View File
@@ -84,20 +84,20 @@ type pushDay struct {
// //
// A successful Push is the household confirming the week's plan, which is // A successful Push is the household confirming the week's plan, which is
// what the week's Supply run (Feature 17) is made from; the returned plan // what the week's Supply run (Feature 17) is made from; the returned plan
// is the one pushed. // is the one pushed, and revision is its revision as pushed, so the Supply
func (s *Service) Push(ctx context.Context, weekStart store.LocalDate) (Plan, error) { // run can be made from that plan and no later edit of it.
func (s *Service) Push(ctx context.Context, weekStart store.LocalDate) (p Plan, revision int64, err error) {
if _, err := household.WeekDays(weekStart); err != nil { if _, err := household.WeekDays(weekStart); err != nil {
return Plan{}, &InvalidWeekError{Value: weekStart.String()} return Plan{}, 0, &InvalidWeekError{Value: weekStart.String()}
} }
planID, revision, days, err := s.pushDays(ctx, weekStart) planID, revision, days, err := s.pushDays(ctx, weekStart)
if err != nil { if err != nil {
return Plan{}, err return Plan{}, 0, err
} }
events := make([]gcal.Event, len(days)) events := make([]gcal.Event, len(days))
for i, d := range days { for i, d := range days {
events[i] = d.event events[i] = d.event
} }
var p Plan
record := func() error { record := func() error {
return s.st.WithTx(ctx, func(tx *sql.Tx) error { return s.st.WithTx(ctx, func(tx *sql.Tx) error {
q := db.New(tx) q := db.New(tx)
@@ -133,9 +133,9 @@ func (s *Service) Push(ctx context.Context, weekStart store.LocalDate) (Plan, er
}) })
} }
if err := s.cal.PushEvents(ctx, events, record); err != nil { if err := s.cal.PushEvents(ctx, events, record); err != nil {
return Plan{}, fmt.Errorf("plan: push %s to Calendar: %w", weekStart, err) return Plan{}, 0, fmt.Errorf("plan: push %s to Calendar: %w", weekStart, err)
} }
return p, nil return p, revision, nil
} }
// pushDays reads, in one read transaction, the plan of the week starting // pushDays reads, in one read transaction, the plan of the week starting
+31 -31
View File
@@ -135,7 +135,7 @@ func TestPushCreatesOneEventPerDay(t *testing.T) {
e.connectCalendar(t) e.connectCalendar(t)
dates := e.storePlan(t, week, fullWeek...) dates := e.storePlan(t, week, fullWeek...)
p, err := e.svc.Push(t.Context(), week) p, _, err := e.svc.Push(t.Context(), week)
if err != nil { if err != nil {
t.Fatalf("Push: %v", err) t.Fatalf("Push: %v", err)
} }
@@ -197,7 +197,7 @@ func TestPushAgainUpdatesOnly(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.connectCalendar(t) e.connectCalendar(t)
e.storePlan(t, week, fullWeek...) e.storePlan(t, week, fullWeek...)
if _, err := e.svc.Push(t.Context(), week); err != nil { if _, _, err := e.svc.Push(t.Context(), week); err != nil {
t.Fatal(err) t.Fatal(err)
} }
// The household deletes Monday's event in Google Calendar; Google keeps // The household deletes Monday's event in Google Calendar; Google keeps
@@ -206,7 +206,7 @@ func TestPushAgainUpdatesOnly(t *testing.T) {
e.google.CancelEvent(dinnersCal.ID, monday) e.google.CancelEvent(dinnersCal.ID, monday)
e.google.ResetEventWrites() e.google.ResetEventWrites()
p, err := e.svc.Push(t.Context(), week) p, _, err := e.svc.Push(t.Context(), week)
if err != nil { if err != nil {
t.Fatalf("second Push: %v", err) t.Fatalf("second Push: %v", err)
} }
@@ -229,7 +229,7 @@ func TestSwapThenPushUpdatesThatDay(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.connectCalendar(t) e.connectCalendar(t)
e.storePlan(t, week, fullWeek...) e.storePlan(t, week, fullWeek...)
if _, err := e.svc.Push(t.Context(), week); err != nil { if _, _, err := e.svc.Push(t.Context(), week); err != nil {
t.Fatal(err) t.Fatal(err)
} }
before := e.google.Events(dinnersCal.ID) before := e.google.Events(dinnersCal.ID)
@@ -247,7 +247,7 @@ func TestSwapThenPushUpdatesThatDay(t *testing.T) {
t.Errorf("swapped day kept event id %q", id) t.Errorf("swapped day kept event id %q", id)
} }
p, err := e.svc.Push(t.Context(), week) p, _, err := e.svc.Push(t.Context(), week)
if err != nil { if err != nil {
t.Fatalf("Push: %v", err) t.Fatalf("Push: %v", err)
} }
@@ -296,7 +296,7 @@ func TestPushAcrossDaylightSavingTime(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.connectCalendar(t) e.connectCalendar(t)
dates := e.storePlan(t, tc.week, fullWeek...) dates := e.storePlan(t, tc.week, fullWeek...)
if _, err := e.svc.Push(t.Context(), tc.week); err != nil { if _, _, err := e.svc.Push(t.Context(), tc.week); err != nil {
t.Fatalf("Push: %v", err) t.Fatalf("Push: %v", err)
} }
events := e.google.Events(dinnersCal.ID) events := e.google.Events(dinnersCal.ID)
@@ -330,12 +330,12 @@ func TestPushAcrossDaylightSavingTime(t *testing.T) {
func TestPushNeedsAChosenCalendar(t *testing.T) { func TestPushNeedsAChosenCalendar(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.storePlan(t, week, fullWeek...) e.storePlan(t, week, fullWeek...)
if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrNotConnected) { if _, _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrNotConnected) {
t.Errorf("Push with nothing connected = %v, want ErrNotConnected", err) t.Errorf("Push with nothing connected = %v, want ErrNotConnected", err)
} }
e.google.SetCalendars(0, dinnersCal) e.google.SetCalendars(0, dinnersCal)
e.google.Connect(t, e.gcal) e.google.Connect(t, e.gcal)
if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrNoCalendar) { if _, _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrNoCalendar) {
t.Errorf("Push with no calendar chosen = %v, want ErrNoCalendar", err) t.Errorf("Push with no calendar chosen = %v, want ErrNoCalendar", err)
} }
if inserts, updates := e.google.EventWrites(); inserts+updates != 0 { if inserts, updates := e.google.EventWrites(); inserts+updates != 0 {
@@ -348,7 +348,7 @@ func TestPushWithoutAPlan(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.connectCalendar(t) e.connectCalendar(t)
before := e.google.TokenCalls() before := e.google.TokenCalls()
if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, plan.ErrNotFound) { if _, _, err := e.svc.Push(t.Context(), week); !errors.Is(err, plan.ErrNotFound) {
t.Errorf("Push = %v, want ErrNotFound", err) t.Errorf("Push = %v, want ErrNotFound", err)
} }
if n := e.google.TokenCalls() - before; n != 0 { if n := e.google.TokenCalls() - before; n != 0 {
@@ -363,7 +363,7 @@ func TestPushPartialFailureLeavesPlanUnsynced(t *testing.T) {
// Google fails the fourth event on every try. // Google fails the fourth event on every try.
e.google.FailEventWrites(4, 3, http.StatusServiceUnavailable) e.google.FailEventWrites(4, 3, http.StatusServiceUnavailable)
_, err := e.svc.Push(t.Context(), week) _, _, err := e.svc.Push(t.Context(), week)
var calendar *gcal.CalendarError var calendar *gcal.CalendarError
if !errors.As(err, &calendar) || !calendar.Retryable() { if !errors.As(err, &calendar) || !calendar.Retryable() {
t.Fatalf("Push = %v, want a retryable *gcal.CalendarError", err) t.Fatalf("Push = %v, want a retryable *gcal.CalendarError", err)
@@ -377,7 +377,7 @@ func TestPushPartialFailureLeavesPlanUnsynced(t *testing.T) {
// Pushing again writes every day: the three already there are updated, // Pushing again writes every day: the three already there are updated,
// not added twice. // not added twice.
e.google.ResetEventWrites() e.google.ResetEventWrites()
p, err := e.svc.Push(t.Context(), week) p, _, err := e.svc.Push(t.Context(), week)
if err != nil { if err != nil {
t.Fatalf("retried Push: %v", err) t.Fatalf("retried Push: %v", err)
} }
@@ -393,7 +393,7 @@ func TestPushPartialFailureKeepsAChangedPlanChanged(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.connectCalendar(t) e.connectCalendar(t)
e.storePlan(t, week, fullWeek...) e.storePlan(t, week, fullWeek...)
first, err := e.svc.Push(t.Context(), week) first, _, err := e.svc.Push(t.Context(), week)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -403,7 +403,7 @@ func TestPushPartialFailureKeepsAChangedPlanChanged(t *testing.T) {
} }
} }
e.google.FailEventWrite(2, http.StatusBadRequest) e.google.FailEventWrite(2, http.StatusBadRequest)
if _, err := e.svc.Push(t.Context(), week); err == nil { if _, _, err := e.svc.Push(t.Context(), week); err == nil {
t.Fatal("Push succeeded, want the second event to fail") t.Fatal("Push succeeded, want the second event to fail")
} }
p, err := e.svc.Get(t.Context(), week) p, err := e.svc.Get(t.Context(), week)
@@ -431,7 +431,7 @@ func TestPushToStaleCalendar(t *testing.T) {
e.connectCalendar(t) e.connectCalendar(t)
e.storePlan(t, week, fullWeek...) e.storePlan(t, week, fullWeek...)
edit(e.google) edit(e.google)
if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrCalendarUnavailable) { if _, _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrCalendarUnavailable) {
t.Errorf("Push = %v, want ErrCalendarUnavailable", err) t.Errorf("Push = %v, want ErrCalendarUnavailable", err)
} }
e.assertUnsynced(t, week, plan.StatusDraft) e.assertUnsynced(t, week, plan.StatusDraft)
@@ -444,7 +444,7 @@ func TestPushGrantNoLongerAccepted(t *testing.T) {
e.connectCalendar(t) e.connectCalendar(t)
e.storePlan(t, week, fullWeek...) e.storePlan(t, week, fullWeek...)
e.google.RevokeGrant() e.google.RevokeGrant()
_, err := e.svc.Push(t.Context(), week) _, _, err := e.svc.Push(t.Context(), week)
if !errors.Is(err, gcal.ErrReconnect) { if !errors.Is(err, gcal.ErrReconnect) {
t.Fatalf("Push = %v, want ErrReconnect", err) t.Fatalf("Push = %v, want ErrReconnect", err)
} }
@@ -479,12 +479,12 @@ func TestRefusedGrantKeepsPushedWeeks(t *testing.T) {
dates := map[store.LocalDate][]string{} dates := map[store.LocalDate][]string{}
for _, w := range pushedWeeks { for _, w := range pushedWeeks {
dates[w] = e.storePlan(t, w, fullWeek...) dates[w] = e.storePlan(t, w, fullWeek...)
if _, err := e.svc.Push(t.Context(), w); err != nil { if _, _, err := e.svc.Push(t.Context(), w); err != nil {
t.Fatalf("Push %s: %v", w, err) t.Fatalf("Push %s: %v", w, err)
} }
} }
e.google.RevokeGrant() e.google.RevokeGrant()
if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrReconnect) { if _, _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrReconnect) {
t.Fatalf("Push = %v, want ErrReconnect", err) t.Fatalf("Push = %v, want ErrReconnect", err)
} }
e.assertRefused(t) e.assertRefused(t)
@@ -497,7 +497,7 @@ func TestRefusedGrantKeepsPushedWeeks(t *testing.T) {
// The refused token is not offered to Google again. // The refused token is not offered to Google again.
before := e.google.TokenCalls() before := e.google.TokenCalls()
if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrReconnect) { if _, _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrReconnect) {
t.Fatalf("Push again = %v, want ErrReconnect", err) t.Fatalf("Push again = %v, want ErrReconnect", err)
} }
if n := e.google.TokenCalls() - before; n != 0 { if n := e.google.TokenCalls() - before; n != 0 {
@@ -506,7 +506,7 @@ func TestRefusedGrantKeepsPushedWeeks(t *testing.T) {
e.google.Connect(t, e.gcal) e.google.Connect(t, e.gcal)
e.google.ResetEventWrites() e.google.ResetEventWrites()
p, err := e.svc.Push(t.Context(), week) p, _, err := e.svc.Push(t.Context(), week)
if err != nil || p.Status != plan.StatusSynced { if err != nil || p.Status != plan.StatusSynced {
t.Fatalf("Push after connecting again = %s, %v; want synced", p.Status, err) t.Fatalf("Push after connecting again = %s, %v; want synced", p.Status, err)
} }
@@ -523,7 +523,7 @@ func TestDisconnectAfterRefusedGrant(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.connectCalendar(t) e.connectCalendar(t)
e.storePlan(t, week, fullWeek...) e.storePlan(t, week, fullWeek...)
if _, err := e.svc.Push(t.Context(), week); err != nil { if _, _, err := e.svc.Push(t.Context(), week); err != nil {
t.Fatal(err) t.Fatal(err)
} }
e.google.RevokeGrant() e.google.RevokeGrant()
@@ -551,7 +551,7 @@ func TestPushFailsWhenPlanSwappedWhilePushing(t *testing.T) {
defer release() defer release()
pushed := make(chan error, 1) pushed := make(chan error, 1)
go func() { go func() {
_, err := e.svc.Push(t.Context(), week) _, _, err := e.svc.Push(t.Context(), week)
pushed <- err pushed <- err
}() }()
<-arrived <-arrived
@@ -596,7 +596,7 @@ func TestPushAfterChoosingAnotherCalendarWritesTheWholeWeek(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.connectCalendar(t) e.connectCalendar(t)
dates := e.storePlan(t, week, fullWeek...) dates := e.storePlan(t, week, fullWeek...)
first, err := e.svc.Push(t.Context(), week) first, _, err := e.svc.Push(t.Context(), week)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -640,14 +640,14 @@ func TestPushAfterChoosingAnotherCalendarWritesTheWholeWeek(t *testing.T) {
// A push that fails part way leaves the plan changed. // A push that fails part way leaves the plan changed.
e.google.ResetEventWrites() e.google.ResetEventWrites()
e.google.FailEventWrite(4, http.StatusBadRequest) e.google.FailEventWrite(4, http.StatusBadRequest)
if _, err := e.svc.Push(t.Context(), week); err == nil { if _, _, err := e.svc.Push(t.Context(), week); err == nil {
t.Fatal("Push succeeded, want the fourth event to fail") 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 { 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) t.Errorf("plan after a failed push = %s, %v; want changed", p.Status, err)
} }
p, err = e.svc.Push(t.Context(), week) p, _, err = e.svc.Push(t.Context(), week)
if err != nil { if err != nil {
t.Fatalf("Push: %v", err) t.Fatalf("Push: %v", err)
} }
@@ -678,7 +678,7 @@ func TestChoosingAnotherCalendarWaitsForAPush(t *testing.T) {
defer release() defer release()
pushed := make(chan error, 1) pushed := make(chan error, 1)
go func() { go func() {
_, err := e.svc.Push(t.Context(), week) _, _, err := e.svc.Push(t.Context(), week)
pushed <- err pushed <- err
}() }()
<-arrived <-arrived
@@ -712,7 +712,7 @@ func TestChoosingAnotherCalendarWaitsForAPush(t *testing.T) {
} }
} }
if p, err = e.svc.Push(t.Context(), week); err != nil || p.Status != plan.StatusSynced { 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) t.Fatalf("Push = %s, %v; want synced", p.Status, err)
} }
e.assertWeekOn(t, kitchenCal.ID, week, dates, fullWeek...) e.assertWeekOn(t, kitchenCal.ID, week, dates, fullWeek...)
@@ -746,7 +746,7 @@ func TestDisconnectForgetsEveryPushedWeek(t *testing.T) {
dates := map[store.LocalDate][]string{} dates := map[store.LocalDate][]string{}
for _, w := range pushedWeeks { for _, w := range pushedWeeks {
dates[w] = e.storePlan(t, w, fullWeek...) dates[w] = e.storePlan(t, w, fullWeek...)
if _, err := e.svc.Push(t.Context(), w); err != nil { if _, _, err := e.svc.Push(t.Context(), w); err != nil {
t.Fatalf("Push %s: %v", w, err) t.Fatalf("Push %s: %v", w, err)
} }
} }
@@ -764,7 +764,7 @@ func TestDisconnectForgetsEveryPushedWeek(t *testing.T) {
// Connecting again chooses no calendar, so nothing is pushed until one // Connecting again chooses no calendar, so nothing is pushed until one
// is chosen; then a push writes the whole week to it. // is chosen; then a push writes the whole week to it.
e.google.Connect(t, e.gcal) e.google.Connect(t, e.gcal)
if _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrNoCalendar) { if _, _, err := e.svc.Push(t.Context(), week); !errors.Is(err, gcal.ErrNoCalendar) {
t.Fatalf("Push after connecting again = %v, want ErrNoCalendar", err) t.Fatalf("Push after connecting again = %v, want ErrNoCalendar", err)
} }
e.google.SetCalendars(0, dinnersCal, kitchenCal) e.google.SetCalendars(0, dinnersCal, kitchenCal)
@@ -773,7 +773,7 @@ func TestDisconnectForgetsEveryPushedWeek(t *testing.T) {
} }
for _, w := range pushedWeeks { for _, w := range pushedWeeks {
e.assertForgotten(t, w, "after connecting again") e.assertForgotten(t, w, "after connecting again")
p, err := e.svc.Push(t.Context(), w) p, _, err := e.svc.Push(t.Context(), w)
if err != nil || p.Status != plan.StatusSynced { if err != nil || p.Status != plan.StatusSynced {
t.Fatalf("Push %s = %s, %v; want synced", w, p.Status, err) t.Fatalf("Push %s = %s, %v; want synced", w, p.Status, err)
} }
@@ -789,7 +789,7 @@ func TestDisconnectRefusedKeepsPushedWeeks(t *testing.T) {
dates := map[store.LocalDate][]string{} dates := map[store.LocalDate][]string{}
for _, w := range pushedWeeks { for _, w := range pushedWeeks {
dates[w] = e.storePlan(t, w, fullWeek...) dates[w] = e.storePlan(t, w, fullWeek...)
if _, err := e.svc.Push(t.Context(), w); err != nil { if _, _, err := e.svc.Push(t.Context(), w); err != nil {
t.Fatalf("Push %s: %v", w, err) t.Fatalf("Push %s: %v", w, err)
} }
} }
@@ -816,7 +816,7 @@ func TestDisconnectWaitsForAPush(t *testing.T) {
defer release() defer release()
pushed := make(chan error, 1) pushed := make(chan error, 1)
go func() { go func() {
_, err := e.svc.Push(t.Context(), week) _, _, err := e.svc.Push(t.Context(), week)
pushed <- err pushed <- err
}() }()
<-arrived <-arrived
+18 -7
View File
@@ -106,9 +106,12 @@ type pushedPlan struct {
// pushedSupplyRun says whether a Push to Calendar made the week's Supply // pushedSupplyRun says whether a Push to Calendar made the week's Supply
// run. status is generated when it did; archived when the week's Supply // run. status is generated when it did; archived when the week's Supply
// run is in the Supply archive and was left as it was; or failed when it // run is in the Supply archive and was left as it was; or failed when it
// could not be made, when error says so and POST // was not made. A failed Supply run's error is plan_changed when the plan
// /shopping/{weekStart}/generate makes it again. error is null when // was edited after it was pushed, so the Supply run was not made from a
// status is generated. // plan Calendar does not show and pushing again makes it; or
// supply_run_failed when it could not be made, and POST
// /shopping/{weekStart}/generate makes it again. error is null when status
// is generated.
type pushedSupplyRun struct { type pushedSupplyRun struct {
Status string `json:"status"` Status string `json:"status"`
Error *apiError `json:"error"` Error *apiError `json:"error"`
@@ -122,15 +125,16 @@ type pushedSupplyRun struct {
// The Supply run is made after the push, never as part of it: Calendar // The Supply run is made after the push, never as part of it: Calendar
// already shows the plan, so a Supply run that cannot be made does not // already shows the plan, so a Supply run that cannot be made does not
// undo or fail the push. The answer is still 200 with supply_run failed, // undo or fail the push. The answer is still 200 with supply_run failed,
// and the failure is logged as an error, so nothing claims a Supply run // and the failure is logged, so nothing claims a Supply run that was not
// that was not made. // made. It is made only from the plan as pushed: one edited between the
// push and the Supply run is not made into a Supply run.
func postPlanPush(logger *slog.Logger, svc *plan.Service, shop *shopping.Service) http.HandlerFunc { func postPlanPush(logger *slog.Logger, svc *plan.Service, shop *shopping.Service) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
week, ok := weekStartOrFail(w, r, logger) week, ok := weekStartOrFail(w, r, logger)
if !ok { if !ok {
return return
} }
p, err := svc.Push(r.Context(), week) p, revision, err := svc.Push(r.Context(), week)
if err != nil { if err != nil {
writePlanPushError(w, r, logger, err) writePlanPushError(w, r, logger, err)
return return
@@ -139,9 +143,16 @@ func postPlanPush(logger *slog.Logger, svc *plan.Service, shop *shopping.Service
// gone. // gone.
ctx := context.WithoutCancel(r.Context()) ctx := context.WithoutCancel(r.Context())
run := pushedSupplyRun{Status: "generated"} run := pushedSupplyRun{Status: "generated"}
if _, err := shop.Generate(ctx, week); errors.Is(err, shopping.ErrFrozen) { if _, err := shop.GeneratePushed(ctx, week, revision); errors.Is(err, shopping.ErrFrozen) {
logger.InfoContext(ctx, "Supply run in the Supply archive left as it was after Push to Calendar", "week", week.String()) logger.InfoContext(ctx, "Supply run in the Supply archive left as it was after Push to Calendar", "week", week.String())
run = pushedSupplyRun{Status: "archived", Error: &apiError{Code: "archived", Message: err.Error()}} run = pushedSupplyRun{Status: "archived", Error: &apiError{Code: "archived", Message: err.Error()}}
} else if errors.Is(err, shopping.ErrPlanChanged) {
logger.WarnContext(ctx, "plan edited after Push to Calendar; Supply run not made", "week", week.String())
run = pushedSupplyRun{Status: "failed", Error: &apiError{
Code: "plan_changed",
Message: "the plan is on Calendar as pushed, but it was edited before its Supply run was made, so no Supply run was made from it; push again",
Retryable: true,
}}
} else if err != nil { } else if err != nil {
logger.ErrorContext(ctx, "generate Supply run after Push to Calendar", "week", week.String(), "error", err) logger.ErrorContext(ctx, "generate Supply run after Push to Calendar", "week", week.String(), "error", err)
run = pushedSupplyRun{Status: "failed", Error: &apiError{ run = pushedSupplyRun{Status: "failed", Error: &apiError{
+45
View File
@@ -437,6 +437,51 @@ func TestPushPlanSupplyRunFails(t *testing.T) {
} }
} }
// TestPushPlanEditedBeforeSupplyRun: a plan edit landing after the push
// is recorded but before the Supply run is made does not make the Supply
// run from the edited plan, which Calendar does not show. The push stands
// (200, on Calendar) and the answer says the Supply run was not made
// because the plan changed.
func TestPushPlanEditedBeforeSupplyRun(t *testing.T) {
t.Parallel()
e := newPlansRouter(t)
e.groundWeek(t)
e.connectCalendar(t, true)
// Once the push records Sunday's event, the household swaps Tuesday to
// Pad Thai, as a swap does: committed before the Supply run is made.
if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER edit_after_push
AFTER UPDATE OF calendar_event_id ON plan_days
WHEN NEW.date = '2026-10-18' AND NEW.calendar_event_id IS NOT NULL
BEGIN
UPDATE plan_days SET dinner_id = '`+e.questLog+`', is_new = 1, calendar_event_id = NULL WHERE date = '2026-10-13';
UPDATE week_plans SET status = CASE WHEN status = 'synced' THEN 'changed' ELSE status END, revision = revision + 1;
END`); err != nil {
t.Fatal(err)
}
rec, body := e.push(t, "2026-10-12")
if rec.Code != http.StatusOK {
t.Fatalf("push = %d %s, want 200", rec.Code, rec.Body)
}
want := map[string]any{"status": "failed", "error": map[string]any{
"code": "plan_changed",
"message": "the plan is on Calendar as pushed, but it was edited before its Supply run was made, so no Supply run was made from it; push again",
"retryable": true,
}}
if !reflect.DeepEqual(body["supply_run"], want) {
t.Errorf("supply_run = %v, want %v", body["supply_run"], want)
}
if n := len(e.google.Events(pushCalendar.ID)); n != 7 {
t.Errorf("calendar has %d events, want 7", n)
}
if status, got := callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", nil); status != http.StatusNotFound {
t.Errorf("GET after the edit = %d %v, want 404: no Supply run from the edited plan", status, got)
}
if !strings.Contains(e.logs.String(), `"level":"WARN","msg":"plan edited after Push to Calendar; Supply run not made"`) {
t.Errorf("logs = %s, want the edit logged as a warning", e.logs)
}
}
// TestPushPlanLeavesArchivedSupplyRun: pushing a week whose Supply run is // TestPushPlanLeavesArchivedSupplyRun: pushing a week whose Supply run is
// in the Supply archive leaves it as it was and says so. // in the Supply archive leaves it as it was and says so.
func TestPushPlanLeavesArchivedSupplyRun(t *testing.T) { func TestPushPlanLeavesArchivedSupplyRun(t *testing.T) {
+28
View File
@@ -39,6 +39,11 @@ const (
// ErrPlanNotFound is a generation for a week with no plan. // ErrPlanNotFound is a generation for a week with no plan.
var ErrPlanNotFound = errors.New("the week has no plan") var ErrPlanNotFound = errors.New("the week has no plan")
// ErrPlanChanged is a generation from a pushed plan the household has
// edited since it was pushed: the Supply run would not match Calendar, so
// none is made, and pushing the plan again makes it.
var ErrPlanChanged = errors.New("the plan changed after it was pushed to Calendar; push again")
// ErrFrozen is a generation of, or a change to an item of, a Supply run // ErrFrozen is a generation of, or a change to an item of, a Supply run
// frozen into the Supply archive, which is read-only. // frozen into the Supply archive, which is read-only.
var ErrFrozen = errors.New("the week's Supply run is in the Supply archive and can no longer change") var ErrFrozen = errors.New("the week's Supply run is in the Supply archive and can no longer change")
@@ -165,6 +170,26 @@ type Generated struct {
// archive is ErrFrozen. A fetched recipe's ingredient without a name is an // archive is ErrFrozen. A fetched recipe's ingredient without a name is an
// error: grounding always names one. // error: grounding always names one.
func (s *Service) Generate(ctx context.Context, weekStart store.LocalDate) (Generated, error) { func (s *Service) Generate(ctx context.Context, weekStart store.LocalDate) (Generated, error) {
return s.generate(ctx, weekStart, func(db.WeekPlan) error { return nil })
}
// GeneratePushed makes the Supply run of the week starting weekStart as
// Generate does, from the plan a Push to Calendar pushed at revision. A
// plan edited since (another revision) is ErrPlanChanged, and the Supply
// run is left as it was.
func (s *Service) GeneratePushed(ctx context.Context, weekStart store.LocalDate, revision int64) (Generated, error) {
return s.generate(ctx, weekStart, func(plan db.WeekPlan) error {
if plan.Revision != revision {
return ErrPlanChanged
}
return nil
})
}
// generate makes the Supply run of the week starting weekStart from the
// week's plan, as Generate says, once check accepts that plan, in the same
// transaction.
func (s *Service) generate(ctx context.Context, weekStart store.LocalDate, check func(db.WeekPlan) error) (Generated, error) {
if _, err := household.WeekDays(weekStart); err != nil { if _, err := household.WeekDays(weekStart); err != nil {
return Generated{}, fmt.Errorf("shopping: generate: %w", err) return Generated{}, fmt.Errorf("shopping: generate: %w", err)
} }
@@ -178,6 +203,9 @@ func (s *Service) Generate(ctx context.Context, weekStart store.LocalDate) (Gene
if err != nil { if err != nil {
return fmt.Errorf("shopping: read plan: %w", err) return fmt.Errorf("shopping: read plan: %w", err)
} }
if err := check(plan); err != nil {
return err
}
want, skipped, err := wanted(ctx, q, plan.ID) want, skipped, err := wanted(ctx, q, plan.ID)
if err != nil { if err != nil {
return err return err
+27
View File
@@ -561,6 +561,33 @@ func TestGenerateRefusesFrozenList(t *testing.T) {
wantLines(t, "stored list", e.stored(t), []string{"0 2026-10-12 Beef Chili: 0 ground beef [1 lb] needed"}) wantLines(t, "stored list", e.stored(t), []string{"0 2026-10-12 Beef Chili: 0 ground beef [1 lb] needed"})
} }
// TestGeneratePushedRefusesEditedPlan makes the Supply run of a pushed
// plan at the revision pushed, then edits the plan: the pushed revision no
// longer makes one, and the Supply run is left as it was.
func TestGeneratePushedRefusesEditedPlan(t *testing.T) {
e := newEnv(t)
e.dinner(t, "Beef Chili", ing{"ground beef", "1", "lb"})
e.dinner(t, "Pad Thai", ing{"rice noodles", "8", "oz"})
e.plan(t, "Beef Chili")
q := db.New(e.st.Writer)
pushed, err := q.GetWeekPlan(t.Context(), week.String())
if err != nil {
t.Fatal(err)
}
if _, err := e.svc.GeneratePushed(t.Context(), week, pushed.Revision); err != nil {
t.Fatalf("GeneratePushed at the pushed revision: %v", err)
}
e.swap(t, "2026-10-12", "Pad Thai")
if err := q.MarkWeekPlanEdited(t.Context(), db.MarkWeekPlanEditedParams{UpdatedAt: store.Now().String(), ID: e.planID}); err != nil {
t.Fatal(err)
}
if _, err := e.svc.GeneratePushed(t.Context(), week, pushed.Revision); !errors.Is(err, shopping.ErrPlanChanged) {
t.Fatalf("GeneratePushed after an edit = %v, want ErrPlanChanged", err)
}
wantLines(t, "stored list", e.stored(t), []string{"0 2026-10-12 Beef Chili: 0 ground beef [1 lb] needed"})
}
// TestGenerateRefusesUnnamedIngredient: grounding always names a fetched // TestGenerateRefusesUnnamedIngredient: grounding always names a fetched
// recipe's ingredient, so one without a name is an error, and nothing is // recipe's ingredient, so one without a name is an error, and nothing is
// stored. // stored.