Expose the Supply run title job and cover the replacement race (#194)
CI / ci (push) Successful in 8m9s
CI / ci (push) Successful in 8m9s
Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>
This commit was merged in pull request #194.
This commit is contained in:
+10
-3
@@ -1120,7 +1120,10 @@ Fake-clock tests of a schedule live in `internal/scheduler`, where
|
||||
- The week's Supply run (Feature 17) is made from a pushed plan: after
|
||||
a successful `Push`, the handler runs `shopping.GeneratePushed` for
|
||||
the week at the plan revision `Push` pushed, and the 200 answer
|
||||
carries `supply_run: {status, error}`. `status` is `generated`;
|
||||
carries `supply_run: {status, error, title_job_id}`. `title_job_id` is
|
||||
the `title_supply_run` job titling a generated list, as for `POST
|
||||
/shopping/{weekStart}/generate`; null when the list already has its
|
||||
title or was not generated. `status` is `generated`;
|
||||
`archived` when the week's Supply run is in the Supply archive and was
|
||||
left as it was; or `failed` — error `plan_changed` (retryable, logged
|
||||
as a warning) when the plan was edited between the push and
|
||||
@@ -1143,8 +1146,12 @@ Fake-clock tests of a schedule live in `internal/scheduler`, where
|
||||
when the week has no Supply run, 400 `invalid_week`.
|
||||
- `POST /api/v1/shopping/{weekStart}/generate` makes or remakes the list
|
||||
through `shopping.Generate` and answers with the same shape plus
|
||||
`skipped: [{date, dinner_id, dinner_name, reason}]`. 404 when the week
|
||||
has no plan, 409 `archived` when its list is in the Supply archive.
|
||||
`skipped: [{date, dinner_id, dinner_name, reason}]` and
|
||||
`title_job_id`, the `title_supply_run` job titling the list, which
|
||||
`GET /api/v1/jobs/{id}` reports on (failed `title_failed` when the title
|
||||
could not be written); null when the list already has its title. 404
|
||||
when the week has no plan, 409 `archived` when its list is in the Supply
|
||||
archive.
|
||||
- `POST /api/v1/shopping/{weekStart}/send-chat` (CSRF-checked) takes
|
||||
`{}` or `{"resend": true}` and posts the Supply run to Google Chat
|
||||
through `chat.PostSupplyRun`, in the week's thread: "Supply run · " and
|
||||
|
||||
@@ -111,10 +111,13 @@ type pushedPlan struct {
|
||||
// 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.
|
||||
// is generated. title_job_id is the job titling a generated Supply run,
|
||||
// which GET /jobs/{id} reports on; null when it already has its title or
|
||||
// was not generated.
|
||||
type pushedSupplyRun struct {
|
||||
Status string `json:"status"`
|
||||
Error *apiError `json:"error"`
|
||||
Status string `json:"status"`
|
||||
Error *apiError `json:"error"`
|
||||
TitleJobID *string `json:"title_job_id"`
|
||||
}
|
||||
|
||||
// postPlanPush pushes the week's plan to the chosen Google calendar
|
||||
@@ -143,7 +146,9 @@ func postPlanPush(logger *slog.Logger, svc *plan.Service, shop *shopping.Service
|
||||
// gone.
|
||||
ctx := context.WithoutCancel(r.Context())
|
||||
run := pushedSupplyRun{Status: "generated"}
|
||||
if _, err := shop.GeneratePushed(ctx, week, revision); errors.Is(err, shopping.ErrFrozen) {
|
||||
if g, err := shop.GeneratePushed(ctx, week, revision); err == nil {
|
||||
run.TitleJobID = g.TitleJobID
|
||||
} else if 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())
|
||||
run = pushedSupplyRun{Status: "archived", Error: &apiError{Code: "archived", Message: err.Error()}}
|
||||
} else if errors.Is(err, shopping.ErrPlanChanged) {
|
||||
@@ -153,7 +158,7 @@ func postPlanPush(logger *slog.Logger, svc *plan.Service, shop *shopping.Service
|
||||
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 {
|
||||
logger.ErrorContext(ctx, "generate Supply run after Push to Calendar", "week", week.String(), "error", err)
|
||||
run = pushedSupplyRun{Status: "failed", Error: &apiError{
|
||||
Code: "supply_run_failed",
|
||||
|
||||
@@ -60,10 +60,13 @@ type supplyRunItem struct {
|
||||
}
|
||||
|
||||
// generatedSupplyRun is the POST /shopping/{weekStart}/generate answer:
|
||||
// the Supply run as made, and the plan days with nothing to shop for.
|
||||
// the Supply run as made, the plan days with nothing to shop for, and the
|
||||
// title_job_id of the job titling it, which GET /jobs/{id} reports on;
|
||||
// null when the Supply run already has its title.
|
||||
type generatedSupplyRun struct {
|
||||
supplyRun
|
||||
Skipped []shopping.SkippedDay `json:"skipped"`
|
||||
Skipped []shopping.SkippedDay `json:"skipped"`
|
||||
TitleJobID *string `json:"title_job_id"`
|
||||
}
|
||||
|
||||
// sendChatRequest is the POST /shopping/{weekStart}/send-chat body:
|
||||
@@ -108,7 +111,7 @@ func getSupplyRun(logger *slog.Logger, svc *shopping.Service) http.HandlerFunc {
|
||||
}
|
||||
|
||||
// postSupplyRunGenerate makes, or remakes, the week's Supply run from its
|
||||
// plan and answers with it and the plan days it skipped.
|
||||
// plan and answers with it, the plan days it skipped and its title job.
|
||||
func postSupplyRunGenerate(logger *slog.Logger, svc *shopping.Service) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
week, ok := weekStartOrFail(w, r, logger)
|
||||
@@ -129,7 +132,7 @@ func postSupplyRunGenerate(logger *slog.Logger, svc *shopping.Service) http.Hand
|
||||
if skipped == nil {
|
||||
skipped = []shopping.SkippedDay{}
|
||||
}
|
||||
writeJSON(w, r, logger, http.StatusOK, generatedSupplyRun{supplyRun: out, Skipped: skipped})
|
||||
writeJSON(w, r, logger, http.StatusOK, generatedSupplyRun{supplyRun: out, Skipped: skipped, TitleJobID: g.TitleJobID})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -186,7 +186,7 @@ func TestSupplyRunShape(t *testing.T) {
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("generate = %d %v", status, raw)
|
||||
}
|
||||
wantTop := []string{"chat_send_unconfirmed_at", "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", "title_job_id", "total_count", "weekStart"}
|
||||
if got := keys(raw); !reflect.DeepEqual(got, wantTop) {
|
||||
t.Errorf("generate fields = %v, want %v", got, wantTop)
|
||||
}
|
||||
@@ -199,7 +199,7 @@ func TestSupplyRunShape(t *testing.T) {
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("GET = %d %v", status, raw)
|
||||
}
|
||||
if k := keys(raw); !reflect.DeepEqual(k, slices.DeleteFunc(slices.Clone(wantTop), func(s string) bool { return s == "skipped" })) {
|
||||
if k := keys(raw); !reflect.DeepEqual(k, slices.DeleteFunc(slices.Clone(wantTop), func(s string) bool { return s == "skipped" || s == "title_job_id" })) {
|
||||
t.Errorf("GET fields = %v", k)
|
||||
}
|
||||
group := raw["groups"].([]any)[0].(map[string]any)
|
||||
@@ -376,8 +376,12 @@ func TestPushPlanMakesSupplyRun(t *testing.T) {
|
||||
if rec.Code != http.StatusOK || body["status"] != "synced" {
|
||||
t.Fatalf("push = %d %s", rec.Code, rec.Body)
|
||||
}
|
||||
if want := map[string]any{"status": "generated", "error": nil}; !reflect.DeepEqual(body["supply_run"], want) {
|
||||
t.Errorf("supply_run = %v, want %v", body["supply_run"], want)
|
||||
run, _ := body["supply_run"].(map[string]any)
|
||||
if k := keys(run); run["status"] != "generated" || run["error"] != nil || !reflect.DeepEqual(k, []string{"error", "status", "title_job_id"}) {
|
||||
t.Errorf("supply_run = %v, want generated with no error", run)
|
||||
}
|
||||
if job := titleJob(t, e, run["title_job_id"]); job["status"] != "queued" {
|
||||
t.Errorf("title job = %v, want queued: no runner in this test", job)
|
||||
}
|
||||
var l supplyRunJSON
|
||||
if status, got := callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l); status != http.StatusOK {
|
||||
@@ -467,9 +471,11 @@ func TestPushPlanDoesNotWaitForTitle(t *testing.T) {
|
||||
if rec.Code != http.StatusOK || body["status"] != "synced" {
|
||||
t.Fatalf("push = %d %s", rec.Code, rec.Body)
|
||||
}
|
||||
if want := map[string]any{"status": "generated", "error": nil}; !reflect.DeepEqual(body["supply_run"], want) {
|
||||
t.Errorf("supply_run = %v, want %v", body["supply_run"], want)
|
||||
run, _ := body["supply_run"].(map[string]any)
|
||||
if run["status"] != "generated" || run["error"] != nil {
|
||||
t.Errorf("supply_run = %v, want generated with no error", run)
|
||||
}
|
||||
titleJobID, _ := run["title_job_id"].(string)
|
||||
held.waitEntered(t)
|
||||
var l supplyRunJSON
|
||||
callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l)
|
||||
@@ -478,6 +484,7 @@ func TestPushPlanDoesNotWaitForTitle(t *testing.T) {
|
||||
}
|
||||
|
||||
close(held.release)
|
||||
wantTitleJobSucceeded(t, e, titleJobID)
|
||||
if got := e.waitTitled(t); got != "Chili and Friends" {
|
||||
t.Errorf("title = %q, want the title job's", got)
|
||||
}
|
||||
@@ -493,7 +500,8 @@ func TestSupplyRunGenerateDoesNotWaitForTitle(t *testing.T) {
|
||||
e.startRunner(t)
|
||||
|
||||
var l supplyRunJSON
|
||||
if status, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", &l); status != http.StatusOK {
|
||||
status, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", &l)
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("generate = %d %v", status, body)
|
||||
}
|
||||
if l.Title != nil {
|
||||
@@ -502,12 +510,66 @@ func TestSupplyRunGenerateDoesNotWaitForTitle(t *testing.T) {
|
||||
if !reflect.DeepEqual(l.render(), weekLines) {
|
||||
t.Errorf("generated = %v, want %v", l.render(), weekLines)
|
||||
}
|
||||
titleJobID, _ := body["title_job_id"].(string)
|
||||
held.waitEntered(t)
|
||||
|
||||
close(held.release)
|
||||
wantTitleJobSucceeded(t, e, titleJobID)
|
||||
if got := e.waitTitled(t); got != "Chili and Friends" {
|
||||
t.Errorf("title = %q, want the title job's", got)
|
||||
}
|
||||
if _, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", nil); body["title_job_id"] != nil {
|
||||
t.Errorf("remake title_job_id = %v, want null: the Supply run has its title", body["title_job_id"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestSupplyRunGenerateTitleJobFails: a title job that cannot title the
|
||||
// Supply run is reported failed with title_failed by GET /jobs/{id}, and
|
||||
// the week's next generation answers with a new title job.
|
||||
func TestSupplyRunGenerateTitleJobFails(t *testing.T) {
|
||||
t.Parallel()
|
||||
e := newPlansRouter(t)
|
||||
e.groundWeek(t)
|
||||
e.startRunner(t)
|
||||
|
||||
_, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", nil)
|
||||
first := titleJob(t, e, body["title_job_id"])
|
||||
job := waitJob(t, e.h, first["id"].(string))
|
||||
if job["status"] != "failed" || job["error"] != shopping.ReasonTitleFailed {
|
||||
t.Errorf("title job = %v, want failed %s", job, shopping.ReasonTitleFailed)
|
||||
}
|
||||
|
||||
_, body = callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", nil)
|
||||
if second := titleJob(t, e, body["title_job_id"]); second["id"] == first["id"] {
|
||||
t.Errorf("retry title_job_id = %v, want a new job", second["id"])
|
||||
}
|
||||
}
|
||||
|
||||
// titleJob returns the title_supply_run job id, as GET /jobs/{id} reports
|
||||
// it; id must be a job id string.
|
||||
func titleJob(t *testing.T, e plansEnv, id any) map[string]any {
|
||||
t.Helper()
|
||||
s, ok := id.(string)
|
||||
if !ok || s == "" {
|
||||
t.Fatalf("title_job_id = %v, want a job id", id)
|
||||
}
|
||||
rec, job := callDinners(t, e.h, http.MethodGet, "/api/v1/jobs/"+s, "")
|
||||
if rec.Code != http.StatusOK || job["kind"] != shopping.KindTitleSupplyRun {
|
||||
t.Fatalf("GET title job = %d %s, want a %s job", rec.Code, rec.Body, shopping.KindTitleSupplyRun)
|
||||
}
|
||||
return job
|
||||
}
|
||||
|
||||
// wantTitleJobSucceeded waits for title job id to finish and fails t
|
||||
// unless GET /jobs/{id} reports it succeeded with the title written.
|
||||
func wantTitleJobSucceeded(t *testing.T, e plansEnv, id string) {
|
||||
t.Helper()
|
||||
titleJob(t, e, id)
|
||||
job := waitJob(t, e.h, id)
|
||||
result, _ := job["result"].(map[string]any)
|
||||
if job["status"] != "succeeded" || result["title"] != "Chili and Friends" {
|
||||
t.Errorf("title job = %v, want succeeded with the title", job)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFailedPushMakesNoSupplyRun: a push Calendar refused makes no Supply
|
||||
@@ -546,7 +608,7 @@ func TestPushPlanSupplyRunFails(t *testing.T) {
|
||||
"code": "supply_run_failed",
|
||||
"message": "the plan is on Calendar, but its Supply run could not be made; make it again from the Supply run",
|
||||
"retryable": true,
|
||||
}}
|
||||
}, "title_job_id": nil}
|
||||
if !reflect.DeepEqual(body["supply_run"], want) {
|
||||
t.Errorf("supply_run = %v, want %v", body["supply_run"], want)
|
||||
}
|
||||
@@ -599,7 +661,7 @@ func TestPushPlanEditedBeforeSupplyRun(t *testing.T) {
|
||||
"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,
|
||||
}}
|
||||
}, "title_job_id": nil}
|
||||
if !reflect.DeepEqual(body["supply_run"], want) {
|
||||
t.Errorf("supply_run = %v, want %v", body["supply_run"], want)
|
||||
}
|
||||
@@ -632,7 +694,7 @@ func TestPushPlanLeavesArchivedSupplyRun(t *testing.T) {
|
||||
t.Fatalf("push = %d %s", rec.Code, rec.Body)
|
||||
}
|
||||
run, _ := body["supply_run"].(map[string]any)
|
||||
if apiErr, _ := run["error"].(map[string]any); run["status"] != "archived" || apiErr["code"] != "archived" {
|
||||
if apiErr, _ := run["error"].(map[string]any); run["status"] != "archived" || apiErr["code"] != "archived" || run["title_job_id"] != nil {
|
||||
t.Errorf("supply_run = %v, want archived", body["supply_run"])
|
||||
}
|
||||
var l supplyRunJSON
|
||||
|
||||
@@ -311,3 +311,38 @@ func TestGenerateTitleOverlapAcrossRename(t *testing.T) {
|
||||
t.Errorf("stored title = %v, want one of the two titles", deref(got))
|
||||
}
|
||||
}
|
||||
|
||||
// TestGenerateTitleNotStoredOnReplacedList: a list replaced by a new
|
||||
// untitled list of the same week once archive_title has answered, before
|
||||
// its title is stored, keeps the replacement untitled; the old list's
|
||||
// title job fails, and the failure is logged.
|
||||
func TestGenerateTitleNotStoredOnReplacedList(t *testing.T) {
|
||||
e := titleWeek(t)
|
||||
e.model.QueueGenerate(titled("Noodles, Tacos and Chili"))
|
||||
e.afterTitle = func() {
|
||||
for _, stmt := range []string{
|
||||
`DELETE FROM shopping_items WHERE group_id IN (SELECT id FROM shopping_groups WHERE list_id IN (SELECT id FROM shopping_lists WHERE week_start = ?1))`,
|
||||
`DELETE FROM shopping_groups WHERE list_id IN (SELECT id FROM shopping_lists WHERE week_start = ?1)`,
|
||||
`UPDATE shopping_lists SET id = 'replacement' WHERE week_start = ?1`,
|
||||
} {
|
||||
if _, err := e.st.Writer.Exec(stmt, week.String()); err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
g := e.generate(t)
|
||||
wantJob(t, e.waitTitle(t, g), jobs.StatusFailed, shopping.ReasonTitleFailed)
|
||||
|
||||
var id string
|
||||
if err := e.st.Reader.QueryRow(`SELECT id FROM shopping_lists WHERE week_start = ?`, week.String()).Scan(&id); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if id != "replacement" || id == g.List.ID {
|
||||
t.Fatalf("week's list = %s, want the replacement of %s", id, g.List.ID)
|
||||
}
|
||||
wantUntitled(t, "replacement's title", e.storedTitle(t))
|
||||
if !strings.Contains(e.log.String(), "the Supply run was frozen or replaced before its title was stored") {
|
||||
t.Errorf("log lacks the failure:\n%s", e.log.String())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user