Make the Supply run from the plan revision Push to Calendar pushed
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:
+12
-8
@@ -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)
|
||||||
|
|
||||||
|
|||||||
@@ -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
@@ -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
|
||||||
|
|||||||
@@ -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{
|
||||||
|
|||||||
@@ -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) {
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -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.
|
||||||
|
|||||||
Reference in New Issue
Block a user