From 171c9c2d11df3a5a9089b4429f8c26b9640b1fe0 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Tue, 6 Oct 2026 18:47:41 -0400 Subject: [PATCH] Fix stale Regular grounding and enforce three Regulars for onboarding A background grounding result now applies only while the dinner is still pending under the normalized name the job searched: GroundPendingDinner and FailPendingDinnerGrounding match normalized_name, so a rename makes the old job's success or failure a no-op. Renaming a dinner still pending enqueues a fresh grounding job for the new name; if that cannot be enqueued, the dinner's grounding is failed so it is not left pending with no job. Completing onboarding (PATCH /api/v1/settings/household with onboarding_completed true) is refused with 400 invalid_settings until the household has at least three Regulars (status regular; archived and Quest log dinners do not count), and onboarding_completed_at is set only then. Co-Authored-By: Claude Opus 5.5 --- internal/db/dinners.sql.go | 51 +++++++++---- internal/db/querier.go | 12 ++- internal/dinners/dinners.go | 64 +++++++++++----- internal/dinners/grounding.go | 22 +++--- internal/dinners/grounding_test.go | 114 ++++++++++++++++++++++++++++ internal/household/settings.go | 20 ++++- internal/household/settings_test.go | 60 ++++++++++++++- internal/server/settings_test.go | 48 +++++++++++- migrations/queries/dinners.sql | 19 +++-- 9 files changed, 355 insertions(+), 55 deletions(-) diff --git a/internal/db/dinners.sql.go b/internal/db/dinners.sql.go index 9e867ce..049f318 100644 --- a/internal/db/dinners.sql.go +++ b/internal/db/dinners.sql.go @@ -52,6 +52,19 @@ func (q *Queries) ArchiveDinner(ctx context.Context, arg ArchiveDinnerParams) (D return i, err } +const countRegularDinners = `-- name: CountRegularDinners :one +SELECT COUNT(*) FROM dinners +WHERE status = 'regular' +` + +// How many Regulars the household has; archived dinners are not counted. +func (q *Queries) CountRegularDinners(ctx context.Context) (int64, error) { + row := q.db.QueryRowContext(ctx, countRegularDinners) + var count int64 + err := row.Scan(&count) + return count, err +} + const createDinner = `-- name: CreateDinner :one INSERT INTO dinners ( id, name, normalized_name, status, origin, method, minutes_total, @@ -166,19 +179,26 @@ UPDATE dinners SET grounding_status = 'failed', grounding_error = ?, updated_at = ? -WHERE id = ? AND grounding_status = 'pending' +WHERE id = ? AND grounding_status = 'pending' AND normalized_name = ? ` type FailPendingDinnerGroundingParams struct { GroundingError sql.NullString UpdatedAt string ID string + NormalizedName string } -// Records why the background grounding of a dinner still pending failed; a -// dinner grounded some other way meanwhile is left as it is (0 rows). +// Records why the background grounding of a dinner still pending under the +// normalized name it was grounded for failed; a dinner grounded some other +// way or renamed meanwhile is left as it is (0 rows). func (q *Queries) FailPendingDinnerGrounding(ctx context.Context, arg FailPendingDinnerGroundingParams) (int64, error) { - result, err := q.db.ExecContext(ctx, failPendingDinnerGrounding, arg.GroundingError, arg.UpdatedAt, arg.ID) + result, err := q.db.ExecContext(ctx, failPendingDinnerGrounding, + arg.GroundingError, + arg.UpdatedAt, + arg.ID, + arg.NormalizedName, + ) if err != nil { return 0, err } @@ -291,21 +311,23 @@ SET recipe_id = ?, grounding_status = 'grounded', grounding_error = NULL, updated_at = ? -WHERE id = ? AND grounding_status = 'pending' +WHERE id = ? AND grounding_status = 'pending' AND normalized_name = ? ` type GroundPendingDinnerParams struct { - RecipeID sql.NullString - PhotoID sql.NullString - Method sql.NullString - MinutesTotal sql.NullInt64 - Difficulty sql.NullString - UpdatedAt string - ID string + RecipeID sql.NullString + PhotoID sql.NullString + Method sql.NullString + MinutesTotal sql.NullInt64 + Difficulty sql.NullString + UpdatedAt string + ID string + NormalizedName string } -// Records the background grounding of a dinner still pending; a dinner -// grounded some other way meanwhile is left as it is (0 rows). +// Records the background grounding of a dinner still pending under the +// normalized name it was grounded for; a dinner grounded some other way or +// renamed meanwhile is left as it is (0 rows). func (q *Queries) GroundPendingDinner(ctx context.Context, arg GroundPendingDinnerParams) (int64, error) { result, err := q.db.ExecContext(ctx, groundPendingDinner, arg.RecipeID, @@ -315,6 +337,7 @@ func (q *Queries) GroundPendingDinner(ctx context.Context, arg GroundPendingDinn arg.Difficulty, arg.UpdatedAt, arg.ID, + arg.NormalizedName, ) if err != nil { return 0, err diff --git a/internal/db/querier.go b/internal/db/querier.go index 3c804d8..2c6e15f 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -21,6 +21,8 @@ type Querier interface { // Drops references to the runs created before the cutoff, so the purge // can delete them. ClearWeekPlanAIRunsBefore(ctx context.Context, cutoff string) (int64, error) + // How many Regulars the household has; archived dinners are not counted. + CountRegularDinners(ctx context.Context) (int64, error) CreateAIRun(ctx context.Context, arg CreateAIRunParams) error CreateDinner(ctx context.Context, arg CreateDinnerParams) (Dinner, error) CreateJob(ctx context.Context, arg CreateJobParams) error @@ -39,8 +41,9 @@ type Querier interface { // recorded a result. Run at boot, after the job runner fails the jobs a // previous process left running. FailOrphanedPendingDinnerGroundings(ctx context.Context, arg FailOrphanedPendingDinnerGroundingsParams) (int64, error) - // Records why the background grounding of a dinner still pending failed; a - // dinner grounded some other way meanwhile is left as it is (0 rows). + // Records why the background grounding of a dinner still pending under the + // normalized name it was grounded for failed; a dinner grounded some other + // way or renamed meanwhile is left as it is (0 rows). FailPendingDinnerGrounding(ctx context.Context, arg FailPendingDinnerGroundingParams) (int64, error) // Fails every job left running, at boot before any worker starts. FailRunningJobs(ctx context.Context, arg FailRunningJobsParams) (int64, error) @@ -65,8 +68,9 @@ type Querier interface { // Records a dinner's grounding (grounded or manual) and clears any // grounding_error. An unknown id returns sql.ErrNoRows. GroundDinner(ctx context.Context, arg GroundDinnerParams) (Dinner, error) - // Records the background grounding of a dinner still pending; a dinner - // grounded some other way meanwhile is left as it is (0 rows). + // Records the background grounding of a dinner still pending under the + // normalized name it was grounded for; a dinner grounded some other way or + // renamed meanwhile is left as it is (0 rows). GroundPendingDinner(ctx context.Context, arg GroundPendingDinnerParams) (int64, error) ListDinnersByStatus(ctx context.Context, status string) ([]Dinner, error) // The planned dinners dated from from_date up to but not including diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index 2af0e2f..731136a 100644 --- a/internal/dinners/dinners.go +++ b/internal/dinners/dinners.go @@ -176,27 +176,41 @@ func (s *Service) CreateRegular(ctx context.Context, r NewRegular, updatedBy str if err != nil { return Created{}, err } - jobID, err := s.groundJob.Enqueue(ctx, GroundJob{DinnerID: d.ID}) - if err == nil { - return Created{Dinner: d, GroundingJobID: &jobID}, nil + jobID, err := s.enqueueGrounding(ctx, d.ID, harness.Normalize(d.Name)) + if err != nil { + return Created{}, err } - // The dinner is stored; record that it will not be grounded so it is - // not left pending with no job. The error is a database error, never a - // dependency's text. - s.logger.ErrorContext(ctx, "dinner grounding not enqueued", "dinner_id", d.ID, "error", err) - write := context.WithoutCancel(ctx) - if _, err := db.New(s.st.Writer).FailPendingDinnerGrounding(write, db.FailPendingDinnerGroundingParams{ + if jobID == nil { + row, err := db.New(s.st.Reader).GetDinner(context.WithoutCancel(ctx), d.ID) + if err != nil { + return Created{}, fmt.Errorf("dinners: read %s: %w", d.ID, err) + } + return Created{Dinner: fromRow(row)}, nil + } + return Created{Dinner: d, GroundingJobID: jobID}, nil +} + +// enqueueGrounding enqueues the KindGroundDinner job for pending dinner id, +// whose normalized name is normalizedName, and returns its id. When the job +// cannot be enqueued, the dinner's grounding is failed with +// ReasonGroundingError, so it is not left pending with no job, and the id +// is nil. +func (s *Service) enqueueGrounding(ctx context.Context, id, normalizedName string) (*string, error) { + jobID, err := s.groundJob.Enqueue(ctx, GroundJob{DinnerID: id}) + if err == nil { + return &jobID, nil + } + // The error is a database error, never a dependency's text. + s.logger.ErrorContext(ctx, "dinner grounding not enqueued", "dinner_id", id, "error", err) + if _, err := db.New(s.st.Writer).FailPendingDinnerGrounding(context.WithoutCancel(ctx), db.FailPendingDinnerGroundingParams{ GroundingError: sql.NullString{String: ReasonGroundingError, Valid: true}, UpdatedAt: store.Now().String(), - ID: d.ID, + ID: id, + NormalizedName: normalizedName, }); err != nil { - return Created{}, fmt.Errorf("dinners: record failed grounding of %s: %w", d.ID, err) + return nil, fmt.Errorf("dinners: record failed grounding of %s: %w", id, err) } - row, err := db.New(s.st.Reader).GetDinner(write, d.ID) - if err != nil { - return Created{}, fmt.Errorf("dinners: read %s: %w", d.ID, err) - } - return Created{Dinner: fromRow(row)}, nil + return nil, nil } func (s *Service) createRegular(ctx context.Context, r NewRegular, updatedBy string) (Dinner, error) { @@ -279,7 +293,9 @@ func (s *Service) List(ctx context.Context, status string) ([]Dinner, error) { // Update applies p to dinner id, recording updatedBy. An invalid patch is a // *ValidationError and a new name repeating another unarchived dinner a -// *DuplicateError; either changes nothing. +// *DuplicateError; either changes nothing. Renaming a dinner still pending +// enqueues a fresh KindGroundDinner job for the new name: the job already +// running was for the old name, and its result no longer applies. func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy string) (Dinner, error) { if updatedBy == "" { return Dinner{}, errors.New("dinners: update: no member to record as updated_by") @@ -290,6 +306,7 @@ func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy stri } var row db.Dinner + var regrounding bool err = s.st.WithTx(ctx, func(tx *sql.Tx) error { q := db.New(tx) cur, err := q.GetDinner(ctx, id) @@ -335,11 +352,24 @@ func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy stri if err != nil { return fmt.Errorf("dinners: update %s: %w", id, err) } + regrounding = row.GroundingStatus == GroundingPending && row.NormalizedName != cur.NormalizedName return nil }) if err != nil { return Dinner{}, err } + if regrounding { + jobID, err := s.enqueueGrounding(ctx, row.ID, row.NormalizedName) + if err != nil { + return Dinner{}, err + } + if jobID == nil { + row, err = db.New(s.st.Reader).GetDinner(context.WithoutCancel(ctx), row.ID) + if err != nil { + return Dinner{}, fmt.Errorf("dinners: read %s: %w", id, err) + } + } + } return fromRow(row), nil } diff --git a/internal/dinners/grounding.go b/internal/dinners/grounding.go index 9f8335f..b3ff4f7 100644 --- a/internal/dinners/grounding.go +++ b/internal/dinners/grounding.go @@ -95,8 +95,10 @@ func (r groundResult) PublicJSON() ([]byte, error) { return json.Marshal(r) } // recipe_url with GroundURL, else its name with Ground. Success records // the recipe, photo, method, minutes and difficulty with grounding_status // grounded; failure records grounding_status failed with its Reason, which -// is also the job's failure code. A dinner no longer pending (grounded by -// hand meanwhile) is left as it is. +// is also the job's failure code. Either is recorded only while the dinner +// is still pending under the normalized name read here: a dinner grounded +// by hand meanwhile is left as it is, and one renamed meanwhile is left +// pending for the job its rename enqueued. func (s *Service) runGroundJob(ctx context.Context, p GroundJob, _ func(groundResult) error) (groundResult, error) { row, err := db.New(s.st.Reader).GetDinner(ctx, p.DinnerID) if err != nil { @@ -125,6 +127,7 @@ func (s *Service) runGroundJob(ctx context.Context, p GroundJob, _ func(groundRe GroundingError: sql.NullString{String: reason, Valid: true}, UpdatedAt: store.Now().String(), ID: row.ID, + NormalizedName: row.NormalizedName, }); failErr != nil { return groundResult{}, fmt.Errorf("dinners: ground: record failure of %s: %w", row.ID, failErr) } @@ -135,13 +138,14 @@ func (s *Service) runGroundJob(ctx context.Context, p GroundJob, _ func(groundRe } n, err := db.New(s.st.Writer).GroundPendingDinner(ctx, db.GroundPendingDinnerParams{ - RecipeID: sql.NullString{String: gr.RecipeID, Valid: true}, - PhotoID: sql.NullString{String: gr.PhotoID, Valid: true}, - Method: sql.NullString{String: gr.Method, Valid: true}, - MinutesTotal: nullInt(gr.MinutesTotal), - Difficulty: sql.NullString{String: gr.Difficulty, Valid: true}, - UpdatedAt: store.Now().String(), - ID: row.ID, + RecipeID: sql.NullString{String: gr.RecipeID, Valid: true}, + PhotoID: sql.NullString{String: gr.PhotoID, Valid: true}, + Method: sql.NullString{String: gr.Method, Valid: true}, + MinutesTotal: nullInt(gr.MinutesTotal), + Difficulty: sql.NullString{String: gr.Difficulty, Valid: true}, + UpdatedAt: store.Now().String(), + ID: row.ID, + NormalizedName: row.NormalizedName, }) if err != nil { return groundResult{}, fmt.Errorf("dinners: ground: record %s: %w", row.ID, err) diff --git a/internal/dinners/grounding_test.go b/internal/dinners/grounding_test.go index 1d7e70e..325082d 100644 --- a/internal/dinners/grounding_test.go +++ b/internal/dinners/grounding_test.go @@ -286,6 +286,120 @@ func TestManualFixWhilePendingIsKept(t *testing.T) { } } +// TestRenameWhilePendingGroundsNewName: a Regular renamed while its job is +// still grounding the old name never gets the old name's result, whether +// that job succeeds or fails; the job the rename enqueues grounds the new +// name. +func TestRenameWhilePendingGroundsNewName(t *testing.T) { + t.Parallel() + tests := []struct { + name string + oldName string + replies []llm.GenerateResult + }{ + {"old name found", "Chicken curry", []llm.GenerateResult{ + recipestest.NormalizedReply("stovetop", 40), recipestest.NormalizedReply("oven", 150), + }}, + {"old name not found", "Unknown dish", []llm.GenerateResult{recipestest.NormalizedReply("oven", 150)}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + e := newEnv(t) + e.search.Results["Beef stew"] = []recipes.Candidate{{URL: stew}} + e.search.Gate = make(chan struct{}) + for _, r := range tt.replies { + e.model.QueueGenerate(r) + } + + c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr(tt.oldName)}, member) + if err != nil { + t.Fatal(err) + } + // Rename only once the job has read the old name and is searching it. + e.waitQuery(t, tt.oldName) + d, err := e.svc.Update(t.Context(), c.ID, dinners.Patch{Name: ptr("Beef stew")}, member) + if err != nil { + t.Fatal(err) + } + if d.GroundingStatus != dinners.GroundingPending { + t.Fatalf("renamed = %+v, want still pending", d) + } + close(e.search.Gate) + + first := e.waitJob(t, *c.GroundingJobID) + if string(first.Result) == `{"dinner_id":"`+c.ID+`","grounding_status":"grounded"}` { + t.Fatalf("old name's job reported grounded: %+v (result %s)", first, first.Result) + } + row := e.waitGrounding(t, c.ID) + if row.GroundingStatus != dinners.GroundingGrounded { + t.Fatalf("dinner = %+v, want grounded for the new name", row) + } + rec, err := db.New(e.st.Reader).GetRecipe(t.Context(), row.RecipeID.String) + if err != nil { + t.Fatal(err) + } + if rec.SourceUrl != stew || rec.PhotoID != row.PhotoID || row.Method.String != "oven" || row.MinutesTotal.Int64 != 150 { + t.Errorf("dinner = %+v recipe %s, want grounded in %s", row, rec.SourceUrl, stew) + } + if got := e.search.Queries(); !slices.Equal(got, []string{tt.oldName, "Beef stew"}) { + t.Errorf("searched %q, want the old name then the new", got) + } + }) + } +} + +// TestRenameWhilePendingEnqueueFailure: a pending Regular renamed when the +// job for its new name cannot be enqueued is failed, not left pending. +func TestRenameWhilePendingEnqueueFailure(t *testing.T) { + t.Parallel() + e := newUnstartedEnv(t) + c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Chicken curry")}, member) + if err != nil { + t.Fatal(err) + } + if _, err := e.st.Writer.ExecContext(t.Context(), + `CREATE TRIGGER refuse_jobs BEFORE INSERT ON jobs BEGIN SELECT RAISE(ABORT, 'no jobs'); END`); err != nil { + t.Fatal(err) + } + d, err := e.svc.Update(t.Context(), c.ID, dinners.Patch{Name: ptr("Beef stew")}, member) + if err != nil { + t.Fatal(err) + } + if d.Name != "Beef stew" || d.GroundingStatus != dinners.GroundingFailed || + d.GroundingError == nil || *d.GroundingError != dinners.ReasonGroundingError { + t.Fatalf("renamed = %+v, want Beef stew failed (grounding_error)", d) + } +} + +// waitQuery waits until the fake search has been asked query. +func (e *env) waitQuery(t *testing.T, query string) { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for !slices.Contains(e.search.Queries(), query) { + if time.Now().After(deadline) { + t.Fatalf("search never asked %q; asked %q", query, e.search.Queries()) + } + time.Sleep(5 * time.Millisecond) + } +} + +// waitGrounding waits for dinner id to be no longer pending and returns it. +func (e *env) waitGrounding(t *testing.T, id string) db.Dinner { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for { + row := e.dinner(t, id) + if row.GroundingStatus != dinners.GroundingPending { + return row + } + if time.Now().After(deadline) { + t.Fatalf("dinner %s still pending", id) + } + time.Sleep(5 * time.Millisecond) + } +} + // failedDinner creates a Regular whose background grounding fails. func failedDinner(t *testing.T, e *env, name string) string { t.Helper() diff --git a/internal/household/settings.go b/internal/household/settings.go index 8e6342c..345e87a 100644 --- a/internal/household/settings.go +++ b/internal/household/settings.go @@ -71,10 +71,15 @@ type SettingsPatch struct { // OnboardingCompleted, when true, records that the household finished // onboarding: onboarding_completed_at becomes the time of the update, // unless it is already set, when the first time is kept. Onboarding is - // never undone, so false is invalid. + // never undone, so false is invalid, and it cannot first be completed + // with fewer than MinRegulars Regulars. OnboardingCompleted *bool } +// MinRegulars is how many Regulars the household must have to complete +// onboarding. +const MinRegulars = 3 + // Problem is one invalid field of a SettingsPatch, named by its JSON key. type Problem struct { Field string @@ -167,7 +172,8 @@ func (s *SettingsService) Get(ctx context.Context) (Settings, error) { // Update validates patch, applies it, records updatedBy (the signed-in // member's subject, which must not be empty) and publishes the new settings // to every subscriber. An invalid patch is a *ValidationError and changes -// nothing. +// nothing, as does completing onboarding with fewer than MinRegulars +// Regulars. func (s *SettingsService) Update(ctx context.Context, patch SettingsPatch, updatedBy string) (Settings, error) { if updatedBy == "" { return Settings{}, errors.New("household: update settings: no member to record as updated_by") @@ -214,6 +220,16 @@ func (s *SettingsService) Update(ctx context.Context, patch SettingsPatch, updat setDow(&p.ChatAutosendDow, patch.ChatAutosendDow) setString(&p.ChatAutosendTime, patch.ChatAutosendTime) if patch.OnboardingCompleted != nil && !p.OnboardingCompletedAt.Valid { + n, err := q.CountRegularDinners(ctx) + if err != nil { + return fmt.Errorf("household: count Regulars: %w", err) + } + if n < MinRegulars { + return &ValidationError{Problems: []Problem{{ + Field: "onboarding_completed", + Reason: fmt.Sprintf("add at least %d Regulars first (have %d)", MinRegulars, n), + }}} + } p.OnboardingCompletedAt = sql.NullString{String: p.UpdatedAt, Valid: true} } row, err = q.UpsertHouseholdSettings(ctx, p) diff --git a/internal/household/settings_test.go b/internal/household/settings_test.go index 9fca588..b277895 100644 --- a/internal/household/settings_test.go +++ b/internal/household/settings_test.go @@ -3,10 +3,13 @@ package household import ( "context" "errors" + "fmt" "slices" + "strings" "sync" "testing" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" ) @@ -34,12 +37,34 @@ func migratedStore(t *testing.T, dir string) *store.Store { // seededService returns a settings service over a migrated temp database // seeded with zone America/New_York. func seededService(t *testing.T) *SettingsService { + t.Helper() + svc, _ := seededServiceStore(t) + return svc +} + +// seededServiceStore is seededService with its store. +func seededServiceStore(t *testing.T) (*SettingsService, *store.Store) { t.Helper() st := migratedStore(t, t.TempDir()) if err := SeedSettings(context.Background(), st, "America/New_York"); err != nil { t.Fatal(err) } - return NewSettingsService(st) + return NewSettingsService(st), st +} + +// addDinners stores n dinners with status. +func addDinners(t *testing.T, st *store.Store, status string, n int) { + t.Helper() + for range n { + id := store.NewID() + now := store.Now().String() + if _, err := db.New(st.Writer).CreateDinner(context.Background(), db.CreateDinnerParams{ + ID: id, Name: "Dinner " + id, NormalizedName: "dinner " + strings.ToLower(id), + Status: status, Origin: "regular", GroundingStatus: "pending", CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatal(err) + } + } } func ptr[T any](v T) *T { return &v } @@ -220,7 +245,8 @@ func TestUpdateAppliesPatchAndRecordsUpdatedBy(t *testing.T) { func TestUpdateCompletesOnboardingOnce(t *testing.T) { t.Parallel() ctx := context.Background() - svc := seededService(t) + svc, st := seededServiceStore(t) + addDinners(t, st, "regular", MinRegulars) got, err := svc.Update(ctx, SettingsPatch{OnboardingCompleted: ptr(true)}, "sam") if err != nil { @@ -250,6 +276,36 @@ func TestUpdateCompletesOnboardingOnce(t *testing.T) { } } +// TestUpdateOnboardingNeedsMinRegulars: onboarding cannot be completed +// with fewer than MinRegulars Regulars; archived and Quest log dinners do +// not count. +func TestUpdateOnboardingNeedsMinRegulars(t *testing.T) { + t.Parallel() + for regulars := range MinRegulars { + t.Run(fmt.Sprintf("%d Regulars", regulars), func(t *testing.T) { + t.Parallel() + ctx := context.Background() + svc, st := seededServiceStore(t) + addDinners(t, st, "regular", regulars) + addDinners(t, st, "archived", MinRegulars) + addDinners(t, st, "quest_log", MinRegulars) + + _, err := svc.Update(ctx, SettingsPatch{OnboardingCompleted: ptr(true)}, "sam") + var verr *ValidationError + if !errors.As(err, &verr) || len(verr.Problems) != 1 || verr.Problems[0].Field != "onboarding_completed" { + t.Fatalf("Update = %v, want a ValidationError on onboarding_completed", err) + } + got, err := svc.Get(ctx) + if err != nil { + t.Fatal(err) + } + if got.OnboardingCompletedAt != nil { + t.Errorf("onboarding_completed_at = %s after a refused completion, want null", *got.OnboardingCompletedAt) + } + }) + } +} + func TestUpdateRequiresMember(t *testing.T) { t.Parallel() ctx := context.Background() diff --git a/internal/server/settings_test.go b/internal/server/settings_test.go index 08b7736..9570fbc 100644 --- a/internal/server/settings_test.go +++ b/internal/server/settings_test.go @@ -3,6 +3,7 @@ package server import ( "context" "encoding/json" + "fmt" "io" "log/slog" "net/http" @@ -13,6 +14,7 @@ import ( "time" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" @@ -115,10 +117,30 @@ func TestPatchHouseholdSettings(t *testing.T) { } } +// newOnboardingRouter returns a router whose settings store holds regulars +// Regulars. +func newOnboardingRouter(t *testing.T, regulars int) http.Handler { + t.Helper() + st := newTestStore(t) + if err := household.SeedSettings(context.Background(), st, "America/New_York"); err != nil { + t.Fatal(err) + } + for i := range regulars { + now := store.Now().String() + if _, err := db.New(st.Writer).CreateDinner(context.Background(), db.CreateDinnerParams{ + ID: store.NewID(), Name: fmt.Sprintf("Dinner %d", i), NormalizedName: fmt.Sprintf("dinner %d", i), + Status: "regular", Origin: "regular", GroundingStatus: "pending", CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatal(err) + } + } + return newTestRouterWith(t, "dev", household.NewSettingsService(st), newTestRunner(t)) +} + func TestPatchHouseholdSettingsCompletesOnboarding(t *testing.T) { t.Parallel() - rec, body := serve(t, newTestRouter(t, "dev"), signedIn(http.MethodPatch, settingsPath, + rec, body := serve(t, newOnboardingRouter(t, household.MinRegulars), signedIn(http.MethodPatch, settingsPath, strings.NewReader(`{"onboarding_completed":true}`))) if rec.Code != http.StatusOK { t.Fatalf("status = %d, want 200: %s", rec.Code, rec.Body) @@ -128,6 +150,30 @@ func TestPatchHouseholdSettingsCompletesOnboarding(t *testing.T) { } } +// TestPatchHouseholdSettingsOnboardingNeedsRegulars: completing onboarding +// with fewer than three Regulars is refused and leaves it incomplete. +func TestPatchHouseholdSettingsOnboardingNeedsRegulars(t *testing.T) { + t.Parallel() + + for regulars := range household.MinRegulars { + t.Run(fmt.Sprintf("%d Regulars", regulars), func(t *testing.T) { + t.Parallel() + router := newOnboardingRouter(t, regulars) + rec, _ := serve(t, router, signedIn(http.MethodPatch, settingsPath, + strings.NewReader(`{"onboarding_completed":true}`))) + if rec.Code != http.StatusBadRequest || + !strings.Contains(rec.Body.String(), `"code":"invalid_settings"`) || + !strings.Contains(rec.Body.String(), "onboarding_completed: add at least 3 Regulars first") { + t.Fatalf("PATCH = %d %s, want 400 invalid_settings on onboarding_completed", rec.Code, rec.Body) + } + _, body := serve(t, router, signedIn(http.MethodGet, settingsPath, nil)) + if body["onboarding_completed_at"] != nil { + t.Errorf("onboarding_completed_at = %#v after a refused completion, want null", body["onboarding_completed_at"]) + } + }) + } +} + func TestPatchHouseholdSettingsRejected(t *testing.T) { t.Parallel() diff --git a/migrations/queries/dinners.sql b/migrations/queries/dinners.sql index 54eaa3d..118f439 100644 --- a/migrations/queries/dinners.sql +++ b/migrations/queries/dinners.sql @@ -19,6 +19,11 @@ SELECT * FROM dinners WHERE status = ? ORDER BY normalized_name, id; +-- name: CountRegularDinners :one +-- How many Regulars the household has; archived dinners are not counted. +SELECT COUNT(*) FROM dinners +WHERE status = 'regular'; + -- name: ListUnarchivedDinners :many -- Every dinner a new or renamed dinner must not repeat. SELECT * FROM dinners @@ -88,8 +93,9 @@ WHERE id = ? RETURNING *; -- name: GroundPendingDinner :execrows --- Records the background grounding of a dinner still pending; a dinner --- grounded some other way meanwhile is left as it is (0 rows). +-- Records the background grounding of a dinner still pending under the +-- normalized name it was grounded for; a dinner grounded some other way or +-- renamed meanwhile is left as it is (0 rows). UPDATE dinners SET recipe_id = ?, photo_id = ?, @@ -99,16 +105,17 @@ SET recipe_id = ?, grounding_status = 'grounded', grounding_error = NULL, updated_at = ? -WHERE id = ? AND grounding_status = 'pending'; +WHERE id = ? AND grounding_status = 'pending' AND normalized_name = ?; -- name: FailPendingDinnerGrounding :execrows --- Records why the background grounding of a dinner still pending failed; a --- dinner grounded some other way meanwhile is left as it is (0 rows). +-- Records why the background grounding of a dinner still pending under the +-- normalized name it was grounded for failed; a dinner grounded some other +-- way or renamed meanwhile is left as it is (0 rows). UPDATE dinners SET grounding_status = 'failed', grounding_error = ?, updated_at = ? -WHERE id = ? AND grounding_status = 'pending'; +WHERE id = ? AND grounding_status = 'pending' AND normalized_name = ?; -- name: FailOrphanedPendingDinnerGroundings :execrows -- Fails every dinner still pending whose grounding job is no longer queued