From f97b4c7b8c8e3c0ac393b070f2e36d10fff608f4 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 07:14:01 -0400 Subject: [PATCH 1/2] Add the Rotation check: MarkTried and keep/drop decisions Per D-021, plan.MarkTried(date) moves every Quest log dinner planned on a household-local date (any plan status) to tried and opens a pending rotation_reviews row; a dinner already checked for the date is left as it is, so it is idempotent and never re-marks a dinner returned to the Quest log. GET /api/v1/rotation-reviews lists the pending checks as {dinner {id, name, photo}, plan_date}. POST /api/v1/dinners/{id}/rotation-review {decision} applies keep (Regular, origin kept), drop ("Not for us", archived) or not_made ("We didn't make it", back to the Quest log), closing the check with decided_at and the session's updated_by; a second decision is 409 no_pending_review. A dropped dinner's name joins the passed names: ListDeclinedSuggestionNames and ListRecentDeclinedNames become ListPassedNames and ListRecentPassedNames, which also read Rotation checks decided drop, so the Side quest dedupe and household context never suggest it again. A static test guards that keep is the only path to Regular: every query writing dinners.status and every use of the regular status is reviewed. Maestro: a1887d1c-c405-478c-ac7d-8cb8b40d280d Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 20 ++ internal/ai/harness/context.go | 10 +- internal/ai/skills/suggest_side_quests.go | 5 +- internal/db/household_context.sql.go | 23 +- internal/db/querier.go | 35 +- internal/db/rotation_reviews.sql.go | 240 ++++++++++++++ internal/db/suggestions.sql.go | 25 +- internal/plan/plan.go | 4 + internal/plan/rotation.go | 209 ++++++++++++ internal/plan/rotation_test.go | 373 ++++++++++++++++++++++ internal/server/plans_test.go | 3 +- internal/server/rotation.go | 85 +++++ internal/server/rotation_test.go | 129 ++++++++ internal/server/server.go | 7 +- internal/suggest/batch.go | 18 +- internal/suggest/suggest_test.go | 2 +- migrations/queries/household_context.sql | 19 +- migrations/queries/rotation_reviews.sql | 60 ++++ migrations/queries/suggestions.sql | 21 +- 19 files changed, 1234 insertions(+), 54 deletions(-) create mode 100644 internal/db/rotation_reviews.sql.go create mode 100644 internal/plan/rotation.go create mode 100644 internal/plan/rotation_test.go create mode 100644 internal/server/rotation.go create mode 100644 internal/server/rotation_test.go create mode 100644 migrations/queries/rotation_reviews.sql diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f2cb34d..fb0927d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -364,6 +364,26 @@ automatic** (D-021). `is_new`, moves a `synced` plan to `changed`, and answers 200 with the plan. A date outside the week is 400 `invalid_date`, an unknown dinner or a week with no plan 404, and any other dinner 409 `dinner_not_plannable`. +- Rotation check (`plan.Service`, D-021). `MarkTried(date)`, which the + `rotation_check` schedule calls for the household-local day just past, + moves every `quest_log` dinner planned on that date, on a plan of any + status, to `tried` and opens a `pending` `rotation_reviews` row for it; + a dinner already checked for that date is left as it is, so it is + idempotent. `GET /api/v1/rotation-reviews` returns `{reviews}`, the + pending checks whose dinner is still tried, oldest `plan_date` first, + each `{dinner: {id, name, photo}, plan_date}`. `POST + /api/v1/dinners/{id}/rotation-review` takes `{decision}`: `keep` makes + the dinner a Regular, `drop` ("Not for us") archives it and `not_made` + ("We didn't make it") returns it to the Quest log; its origin is kept. + The check is closed with `decided_at` and the session's `updated_by`, + and the answer is `{dinner, plan_date, decision, dinner_status, + decided_at, updated_by}`. Any other decision is 400 `invalid_decision`, + an unknown dinner 404, a dinner with no pending check (including a + second decision) 409 `no_pending_review`, and one no longer tried 409 + `dinner_not_tried`. Keep is the only path from tried to Regular. A + dropped dinner's name joins the passed names (`ListPassedNames`) the + Side quest dedupe and household context use, so it is never suggested + again. - `/health` — liveness. - Everything else serves the embedded PWA with SPA fallback. diff --git a/internal/ai/harness/context.go b/internal/ai/harness/context.go index eae19d6..259b7e7 100644 --- a/internal/ai/harness/context.go +++ b/internal/ai/harness/context.go @@ -15,8 +15,8 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" ) -// PassedLimit is how many of the most recently passed Side quests the -// household context holds. +// PassedLimit is how many of the most recently passed names (Side quests +// passed and dinners a Rotation check dropped) the household context holds. const PassedLimit = 60 // PlanWeeks is how many ISO weeks of plans, ending with the current week, @@ -34,8 +34,8 @@ type HouseholdContext struct { Regulars []RegularDinner // QuestLog holds the Quest log dinner names, in name order. QuestLog []string - // Passed holds the names of the most recently passed Side quests, - // oldest first. + // Passed holds the most recently passed names, oldest first: Side + // quests passed and dinners a Rotation check dropped ("Not for us"). Passed []string // Plans are the planned dinners of the last PlanWeeks weeks, oldest // first. @@ -137,7 +137,7 @@ func readContext(ctx context.Context, st *store.Store, now time.Time) (h Househo h.QuestLog = append(h.QuestLog, d.Name) } - passed, err := q.ListRecentDeclinedNames(ctx, PassedLimit) + passed, err := q.ListRecentPassedNames(ctx, PassedLimit) if err != nil { return h, fmt.Errorf("read passed Side quests: %w", err) } diff --git a/internal/ai/skills/suggest_side_quests.go b/internal/ai/skills/suggest_side_quests.go index ac10af7..e35d0a2 100644 --- a/internal/ai/skills/suggest_side_quests.go +++ b/internal/ai/skills/suggest_side_quests.go @@ -30,8 +30,9 @@ type SideQuestsRequest struct { Needed int // Pending holds the names of the Side quests not yet tried or passed. Pending []string - // Declined holds the name of every passed Side quest, not only the - // recent ones Context holds. + // Declined holds every passed name — each passed Side quest and each + // dinner a Rotation check dropped — not only the recent ones Context + // holds. Declined []string } diff --git a/internal/db/household_context.sql.go b/internal/db/household_context.sql.go index e26740a..a84a4dd 100644 --- a/internal/db/household_context.sql.go +++ b/internal/db/household_context.sql.go @@ -52,20 +52,27 @@ func (q *Queries) ListPlannedDinners(ctx context.Context, arg ListPlannedDinners return items, nil } -const listRecentDeclinedNames = `-- name: ListRecentDeclinedNames :many +const listRecentPassedNames = `-- name: ListRecentPassedNames :many -SELECT name -FROM suggestions -WHERE status = 'declined' +SELECT name FROM ( + SELECT name, decided_at, id + FROM suggestions + WHERE status = 'declined' + UNION ALL + SELECT dinners.name, rotation_reviews.decided_at, rotation_reviews.id + FROM rotation_reviews + JOIN dinners ON dinners.id = rotation_reviews.dinner_id + WHERE rotation_reviews.decision = 'drop' +) ORDER BY decided_at DESC, id DESC LIMIT ? ` // Reads for the AI household context block (internal/ai/harness). -// The names the most recently declined suggestions were suggested under, -// newest first; renaming their dinners does not change them. -func (q *Queries) ListRecentDeclinedNames(ctx context.Context, limit int64) ([]string, error) { - rows, err := q.db.QueryContext(ctx, listRecentDeclinedNames, limit) +// The most recent names the household turned down, newest first, as +// ListPassedNames has them. +func (q *Queries) ListRecentPassedNames(ctx context.Context, limit int64) ([]string, error) { + rows, err := q.db.QueryContext(ctx, listRecentPassedNames, limit) if err != nil { return nil, err } diff --git a/internal/db/querier.go b/internal/db/querier.go index 55b84c4..55be29f 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -34,11 +34,15 @@ type Querier interface { CreatePhoto(ctx context.Context, arg CreatePhotoParams) (Photo, error) CreateRecipeIngredient(ctx context.Context, arg CreateRecipeIngredientParams) error CreateRecipeStep(ctx context.Context, arg CreateRecipeStepParams) error + CreateRotationReview(ctx context.Context, arg CreateRotationReviewParams) error // name is the name the dish is suggested under, kept whatever its dinner is // later renamed. CreateSuggestion(ctx context.Context, arg CreateSuggestionParams) error CreateSuggestionBatch(ctx context.Context, arg CreateSuggestionBatchParams) (SuggestionBatch, error) CreateWeekPlan(ctx context.Context, arg CreateWeekPlanParams) error + // Records the household's decision on a pending Rotation check; one + // already decided is left as it is (0 rows). + DecideRotationReview(ctx context.Context, arg DecideRotationReviewParams) (int64, error) // Records the household's decision on a pending suggestion; one already // decided is left as it is (0 rows). DecideSuggestion(ctx context.Context, arg DecideSuggestionParams) (int64, error) @@ -87,6 +91,10 @@ type Querier interface { // The newest batch, whatever its status. Returns sql.ErrNoRows when there // is none. GetLatestSuggestionBatch(ctx context.Context) (SuggestionBatch, error) + // The pending Rotation check of dinner_id. MarkTried makes one only for a + // dinner in the Quest log, which it moves to tried, so a dinner has at + // most one. Returns sql.ErrNoRows when there is none. + GetPendingRotationReview(ctx context.Context, dinnerID string) (RotationReview, error) GetPhoto(ctx context.Context, id string) (Photo, error) // Finds stored image bytes by content hash. The schema does not make sha256 // unique, so the earliest-created (lowest id) match is returned. @@ -121,14 +129,19 @@ type Querier interface { // The pending suggestions whose dinner is still suggested, oldest first, // with their dinners. ListDeck(ctx context.Context) ([]ListDeckRow, error) - // The name every declined suggestion was suggested under, oldest decision - // first; renaming its dinner does not change it. - ListDeclinedSuggestionNames(ctx context.Context) ([]string, error) ListDinnersByStatus(ctx context.Context, status string) ([]Dinner, error) // The dinners a draft of the week starting week_start may plan: every // Regular, and every Quest log dinner no other plan for the week starting // from_week_start or later has on any day. ListDraftPool(ctx context.Context, arg ListDraftPoolParams) ([]Dinner, error) + // Every name the household turned down, oldest decision first: the name + // each declined suggestion was suggested under, which renaming its dinner + // does not change, and the name of each dinner a Rotation check dropped + // ("Not for us"), which archiving keeps. + ListPassedNames(ctx context.Context) ([]string, error) + // The pending Rotation checks whose dinner is still tried, oldest plan + // date first, with their dinners. + ListPendingRotationReviews(ctx context.Context) ([]ListPendingRotationReviewsRow, error) // The days of plan plan_id with their dinners, Monday first. A day with // no dinner is left out. ListPlanDays(ctx context.Context, planID string) ([]ListPlanDaysRow, error) @@ -136,11 +149,14 @@ type Querier interface { // to_date, oldest first. Days with no dinner are left out. ListPlannedDinners(ctx context.Context, arg ListPlannedDinnersParams) ([]ListPlannedDinnersRow, error) // Reads for the AI household context block (internal/ai/harness). - // The names the most recently declined suggestions were suggested under, - // newest first; renaming their dinners does not change them. - ListRecentDeclinedNames(ctx context.Context, limit int64) ([]string, error) + // The most recent names the household turned down, newest first, as + // ListPassedNames has them. + ListRecentPassedNames(ctx context.Context, limit int64) ([]string, error) ListRecipeIngredients(ctx context.Context, recipeID string) ([]RecipeIngredient, error) ListRecipeSteps(ctx context.Context, recipeID string) ([]RecipeStep, error) + // The ids of the dinners with status quest_log planned on date, on a plan + // of any status, that have no Rotation check for date yet. + ListTriedCandidates(ctx context.Context, date string) ([]string, error) // Every dinner a new or renamed dinner must not repeat, with the // source_url of the recipe it is grounded in (NULL while it has none). ListUnarchivedDinners(ctx context.Context) ([]ListUnarchivedDinnersRow, error) @@ -150,6 +166,9 @@ type Querier interface { // The ids of the dinners planned in the week starting week_start, Monday // first. ListWeekPlanDinnerIDs(ctx context.Context, weekStart string) ([]sql.NullString, error) + // Moves a dinner still in the Quest log to tried; a dinner with any other + // status is left as it is (0 rows). + MarkDinnerTried(ctx context.Context, arg MarkDinnerTriedParams) (int64, error) // Records an edit of plan id, raising its revision: a synced plan becomes // changed, any other keeps its status. MarkWeekPlanEdited(ctx context.Context, arg MarkWeekPlanEditedParams) error @@ -157,6 +176,10 @@ type Querier interface { // recording updated_by; a dinner no longer suggested is left as it is // (0 rows). MoveSuggestedDinner(ctx context.Context, arg MoveSuggestedDinnerParams) (int64, error) + // Moves a dinner still tried to status (regular, archived or quest_log) + // by a Rotation check decision, recording updated_by; its origin is kept. + // A dinner no longer tried is left as it is (0 rows). + MoveTriedDinner(ctx context.Context, arg MoveTriedDinnerParams) (int64, error) // Stores a partial result of a running job. PublishJobResult(ctx context.Context, arg PublishJobResultParams) (int64, error) // Plans dinner_id on date of plan_id, keeping the day's calendar event id diff --git a/internal/db/rotation_reviews.sql.go b/internal/db/rotation_reviews.sql.go new file mode 100644 index 0000000..cb613cd --- /dev/null +++ b/internal/db/rotation_reviews.sql.go @@ -0,0 +1,240 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: rotation_reviews.sql + +package db + +import ( + "context" + "database/sql" +) + +const createRotationReview = `-- name: CreateRotationReview :exec +INSERT INTO rotation_reviews (id, dinner_id, plan_date, status, created_at, updated_at) +VALUES (?, ?, ?, 'pending', ?, ?) +` + +type CreateRotationReviewParams struct { + ID string + DinnerID string + PlanDate string + CreatedAt string + UpdatedAt string +} + +func (q *Queries) CreateRotationReview(ctx context.Context, arg CreateRotationReviewParams) error { + _, err := q.db.ExecContext(ctx, createRotationReview, + arg.ID, + arg.DinnerID, + arg.PlanDate, + arg.CreatedAt, + arg.UpdatedAt, + ) + return err +} + +const decideRotationReview = `-- name: DecideRotationReview :execrows +UPDATE rotation_reviews +SET status = 'decided', + decision = ?1, + decided_at = ?2, + updated_by = ?3, + updated_at = ?2 +WHERE id = ?4 AND status = 'pending' +` + +type DecideRotationReviewParams struct { + Decision sql.NullString + Now sql.NullString + UpdatedBy sql.NullString + ID string +} + +// Records the household's decision on a pending Rotation check; one +// already decided is left as it is (0 rows). +func (q *Queries) DecideRotationReview(ctx context.Context, arg DecideRotationReviewParams) (int64, error) { + result, err := q.db.ExecContext(ctx, decideRotationReview, + arg.Decision, + arg.Now, + arg.UpdatedBy, + arg.ID, + ) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const getPendingRotationReview = `-- name: GetPendingRotationReview :one +SELECT id, dinner_id, plan_date, status, decision, decided_at, updated_by, created_at, updated_at FROM rotation_reviews +WHERE dinner_id = ? AND status = 'pending' +` + +// The pending Rotation check of dinner_id. MarkTried makes one only for a +// dinner in the Quest log, which it moves to tried, so a dinner has at +// most one. Returns sql.ErrNoRows when there is none. +func (q *Queries) GetPendingRotationReview(ctx context.Context, dinnerID string) (RotationReview, error) { + row := q.db.QueryRowContext(ctx, getPendingRotationReview, dinnerID) + var i RotationReview + err := row.Scan( + &i.ID, + &i.DinnerID, + &i.PlanDate, + &i.Status, + &i.Decision, + &i.DecidedAt, + &i.UpdatedBy, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const listPendingRotationReviews = `-- name: ListPendingRotationReviews :many +SELECT rotation_reviews.plan_date, dinners.id, dinners.name, dinners.normalized_name, dinners.status, dinners.origin, dinners.method, dinners.minutes_total, dinners.difficulty, dinners.cuisine, dinners.is_favorite, dinners.recipe_id, dinners.photo_id, dinners.grounding_status, dinners.updated_by, dinners.created_at, dinners.updated_at, dinners.recipe_url, dinners.grounding_error, dinners.grounding_revision +FROM rotation_reviews +JOIN dinners ON dinners.id = rotation_reviews.dinner_id +WHERE rotation_reviews.status = 'pending' AND dinners.status = 'tried' +ORDER BY rotation_reviews.plan_date, rotation_reviews.id +` + +type ListPendingRotationReviewsRow struct { + PlanDate string + Dinner Dinner +} + +// The pending Rotation checks whose dinner is still tried, oldest plan +// date first, with their dinners. +func (q *Queries) ListPendingRotationReviews(ctx context.Context) ([]ListPendingRotationReviewsRow, error) { + rows, err := q.db.QueryContext(ctx, listPendingRotationReviews) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListPendingRotationReviewsRow + for rows.Next() { + var i ListPendingRotationReviewsRow + if err := rows.Scan( + &i.PlanDate, + &i.Dinner.ID, + &i.Dinner.Name, + &i.Dinner.NormalizedName, + &i.Dinner.Status, + &i.Dinner.Origin, + &i.Dinner.Method, + &i.Dinner.MinutesTotal, + &i.Dinner.Difficulty, + &i.Dinner.Cuisine, + &i.Dinner.IsFavorite, + &i.Dinner.RecipeID, + &i.Dinner.PhotoID, + &i.Dinner.GroundingStatus, + &i.Dinner.UpdatedBy, + &i.Dinner.CreatedAt, + &i.Dinner.UpdatedAt, + &i.Dinner.RecipeUrl, + &i.Dinner.GroundingError, + &i.Dinner.GroundingRevision, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listTriedCandidates = `-- name: ListTriedCandidates :many +SELECT DISTINCT dinners.id +FROM plan_days +JOIN dinners ON dinners.id = plan_days.dinner_id +WHERE plan_days.date = ?1 + AND dinners.status = 'quest_log' + AND NOT EXISTS ( + SELECT 1 FROM rotation_reviews + WHERE rotation_reviews.dinner_id = dinners.id + AND rotation_reviews.plan_date = plan_days.date + ) +ORDER BY dinners.id +` + +// The ids of the dinners with status quest_log planned on date, on a plan +// of any status, that have no Rotation check for date yet. +func (q *Queries) ListTriedCandidates(ctx context.Context, date string) ([]string, error) { + rows, err := q.db.QueryContext(ctx, listTriedCandidates, date) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, err + } + items = append(items, id) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const markDinnerTried = `-- name: MarkDinnerTried :execrows +UPDATE dinners +SET status = 'tried', updated_at = ? +WHERE id = ? AND status = 'quest_log' +` + +type MarkDinnerTriedParams struct { + UpdatedAt string + ID string +} + +// Moves a dinner still in the Quest log to tried; a dinner with any other +// status is left as it is (0 rows). +func (q *Queries) MarkDinnerTried(ctx context.Context, arg MarkDinnerTriedParams) (int64, error) { + result, err := q.db.ExecContext(ctx, markDinnerTried, arg.UpdatedAt, arg.ID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const moveTriedDinner = `-- name: MoveTriedDinner :execrows +UPDATE dinners +SET status = ?1, updated_by = ?2, updated_at = ?3 +WHERE id = ?4 AND status = 'tried' +` + +type MoveTriedDinnerParams struct { + Status string + UpdatedBy sql.NullString + UpdatedAt string + ID string +} + +// Moves a dinner still tried to status (regular, archived or quest_log) +// by a Rotation check decision, recording updated_by; its origin is kept. +// A dinner no longer tried is left as it is (0 rows). +func (q *Queries) MoveTriedDinner(ctx context.Context, arg MoveTriedDinnerParams) (int64, error) { + result, err := q.db.ExecContext(ctx, moveTriedDinner, + arg.Status, + arg.UpdatedBy, + arg.UpdatedAt, + arg.ID, + ) + if err != nil { + return 0, err + } + return result.RowsAffected() +} diff --git a/internal/db/suggestions.sql.go b/internal/db/suggestions.sql.go index 3829c5a..1588fa2 100644 --- a/internal/db/suggestions.sql.go +++ b/internal/db/suggestions.sql.go @@ -348,17 +348,26 @@ func (q *Queries) ListDeck(ctx context.Context) ([]ListDeckRow, error) { return items, nil } -const listDeclinedSuggestionNames = `-- name: ListDeclinedSuggestionNames :many -SELECT name -FROM suggestions -WHERE status = 'declined' +const listPassedNames = `-- name: ListPassedNames :many +SELECT name FROM ( + SELECT name, decided_at, id + FROM suggestions + WHERE status = 'declined' + UNION ALL + SELECT dinners.name, rotation_reviews.decided_at, rotation_reviews.id + FROM rotation_reviews + JOIN dinners ON dinners.id = rotation_reviews.dinner_id + WHERE rotation_reviews.decision = 'drop' +) ORDER BY decided_at, id ` -// The name every declined suggestion was suggested under, oldest decision -// first; renaming its dinner does not change it. -func (q *Queries) ListDeclinedSuggestionNames(ctx context.Context) ([]string, error) { - rows, err := q.db.QueryContext(ctx, listDeclinedSuggestionNames) +// Every name the household turned down, oldest decision first: the name +// each declined suggestion was suggested under, which renaming its dinner +// does not change, and the name of each dinner a Rotation check dropped +// ("Not for us"), which archiving keeps. +func (q *Queries) ListPassedNames(ctx context.Context) ([]string, error) { + rows, err := q.db.QueryContext(ctx, listPassedNames) if err != nil { return nil, err } diff --git a/internal/plan/plan.go b/internal/plan/plan.go index e88d74c..ec9647f 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -5,6 +5,10 @@ // replacing any draft before it unless the household edited the plan // while draft_week ran. A plan is draft until Push to Calendar // syncs it (Feature 16); an edit of a synced plan makes it changed. +// +// The Rotation check follows a plan day: MarkTried moves the Quest log +// dinners planned on it to tried, and the household answers "Keep it in +// rotation?" for each (D-021). Only their keep makes a dinner a Regular. package plan import ( diff --git a/internal/plan/rotation.go b/internal/plan/rotation.go new file mode 100644 index 0000000..6b4aa8d --- /dev/null +++ b/internal/plan/rotation.go @@ -0,0 +1,209 @@ +package plan + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// Rotation check decisions (the rotation_reviews.decision column), the +// household's answer to "Keep it in rotation?" (D-021). +const ( + // DecisionKeep makes the tried dinner a Regular. + DecisionKeep = "keep" + // DecisionDrop ("Not for us") archives it; its name stays among the + // passed names, so it is never suggested again. + DecisionDrop = "drop" + // DecisionNotMade ("We didn't make it") returns it to the Quest log. + DecisionNotMade = "not_made" +) + +// decisionStatus is the status each decision moves a tried dinner to. Keep +// is the only way a tried dinner becomes a Regular: nothing automatic +// promotes one (D-021). +var decisionStatus = map[string]string{ + DecisionKeep: dinners.StatusRegular, + DecisionDrop: dinners.StatusArchived, + DecisionNotMade: dinners.StatusQuestLog, +} + +// ErrNoPendingReview is a decision on a dinner with no pending Rotation +// check: it was never tried, or its check is already decided. +var ErrNoPendingReview = errors.New("the dinner has no pending Rotation check") + +// ErrDinnerNotTried is a decision on a pending Rotation check whose dinner +// is no longer tried, such as one archived meanwhile. +var ErrDinnerNotTried = errors.New("the dinner is no longer tried") + +// InvalidDecisionError is a decision that is not keep, drop or not_made. +type InvalidDecisionError struct { + Value string +} + +func (e *InvalidDecisionError) Error() string { + return fmt.Sprintf("decision %q is not one of keep, drop, not_made", e.Value) +} + +// ReviewDinner is the dinner of a Rotation check. +type ReviewDinner struct { + ID string `json:"id"` + Name string `json:"name"` + Photo *string `json:"photo"` +} + +// Review is a pending Rotation check: the dinner tried and the plan date +// it was planned on. +type Review struct { + Dinner ReviewDinner `json:"dinner"` + // PlanDate is household-local, YYYY-MM-DD. + PlanDate string `json:"plan_date"` +} + +// Decided is a Rotation check as decided. +type Decided struct { + Dinner ReviewDinner `json:"dinner"` + PlanDate string `json:"plan_date"` + Decision string `json:"decision"` + // DinnerStatus is the status the decision moved the dinner to. + DinnerStatus string `json:"dinner_status"` + DecidedAt string `json:"decided_at"` + UpdatedBy string `json:"updated_by"` +} + +// MarkTried moves every dinner in the Quest log planned on date, the +// household-local day just past, on a plan of any status, to tried and +// opens a pending Rotation check of it for date. It is idempotent: a +// dinner already checked for date is left as it is, so a later run never +// moves one returned to the Quest log ("We didn't make it") again. It +// returns how many dinners it moved. +func (s *Service) MarkTried(ctx context.Context, date store.LocalDate) (int, error) { + var n int + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + ids, err := q.ListTriedCandidates(ctx, date.String()) + if err != nil { + return fmt.Errorf("plan: list Quest log dinners planned on %s: %w", date, err) + } + now := store.Now().String() + for _, id := range ids { + moved, err := q.MarkDinnerTried(ctx, db.MarkDinnerTriedParams{UpdatedAt: now, ID: id}) + if err != nil { + return fmt.Errorf("plan: mark %s tried: %w", id, err) + } + if moved != 1 { + return fmt.Errorf("plan: dinner %s left the Quest log while marking it tried", id) + } + if err := q.CreateRotationReview(ctx, db.CreateRotationReviewParams{ + ID: store.NewID(), DinnerID: id, PlanDate: date.String(), CreatedAt: now, UpdatedAt: now, + }); err != nil { + return fmt.Errorf("plan: open Rotation check of %s for %s: %w", id, date, err) + } + } + n = len(ids) + return nil + }) + if err != nil { + return 0, err + } + return n, nil +} + +// PendingReviews returns the pending Rotation checks, oldest plan date +// first. +func (s *Service) PendingReviews(ctx context.Context) ([]Review, error) { + rows, err := db.New(s.st.Reader).ListPendingRotationReviews(ctx) + if err != nil { + return nil, fmt.Errorf("plan: list pending Rotation checks: %w", err) + } + out := make([]Review, len(rows)) + for i, r := range rows { + out[i] = Review{Dinner: reviewDinner(r.Dinner), PlanDate: r.PlanDate} + } + return out, nil +} + +// DecideReview applies decision to the pending Rotation check of dinnerID +// and closes it, recording updatedBy: keep makes the dinner a Regular, +// drop archives it and not_made returns it to the Quest log; its origin is +// kept. An unknown dinner is ErrDinnerNotFound, a dinner with no pending +// check (including a second decision) ErrNoPendingReview, and one no +// longer tried ErrDinnerNotTried; none of them changes anything. +func (s *Service) DecideReview(ctx context.Context, dinnerID, decision, updatedBy string) (Decided, error) { + status, ok := decisionStatus[decision] + if !ok { + return Decided{}, &InvalidDecisionError{Value: decision} + } + if updatedBy == "" { + return Decided{}, errors.New("plan: decide Rotation check: no member to record as updated_by") + } + var out Decided + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + d, err := q.GetDinner(ctx, dinnerID) + if errors.Is(err, sql.ErrNoRows) { + return ErrDinnerNotFound + } + if err != nil { + return fmt.Errorf("plan: read dinner %s: %w", dinnerID, err) + } + review, err := q.GetPendingRotationReview(ctx, d.ID) + if errors.Is(err, sql.ErrNoRows) { + return ErrNoPendingReview + } + if err != nil { + return fmt.Errorf("plan: read Rotation check of %s: %w", d.ID, err) + } + now := store.Now().String() + moved, err := q.MoveTriedDinner(ctx, db.MoveTriedDinnerParams{ + Status: status, + UpdatedBy: sql.NullString{String: updatedBy, Valid: true}, + UpdatedAt: now, + ID: d.ID, + }) + if err != nil { + return fmt.Errorf("plan: move %s to %s: %w", d.ID, status, err) + } + if moved != 1 { + return ErrDinnerNotTried + } + closed, err := q.DecideRotationReview(ctx, db.DecideRotationReviewParams{ + Decision: sql.NullString{String: decision, Valid: true}, + Now: sql.NullString{String: now, Valid: true}, + UpdatedBy: sql.NullString{String: updatedBy, Valid: true}, + ID: review.ID, + }) + if err != nil { + return fmt.Errorf("plan: close Rotation check %s: %w", review.ID, err) + } + if closed != 1 { + return fmt.Errorf("plan: Rotation check %s was decided while deciding it", review.ID) + } + out = Decided{ + Dinner: reviewDinner(d), + PlanDate: review.PlanDate, + Decision: decision, + DinnerStatus: status, + DecidedAt: now, + UpdatedBy: updatedBy, + } + return nil + }) + if err != nil { + return Decided{}, err + } + return out, nil +} + +func reviewDinner(d db.Dinner) ReviewDinner { + out := ReviewDinner{ID: d.ID, Name: d.Name} + if d.PhotoID.Valid { + u := dinners.PhotoURL(d.PhotoID.String) + out.Photo = &u + } + return out +} diff --git a/internal/plan/rotation_test.go b/internal/plan/rotation_test.go new file mode 100644 index 0000000..092cde7 --- /dev/null +++ b/internal/plan/rotation_test.go @@ -0,0 +1,373 @@ +package plan_test + +import ( + "database/sql" + "errors" + "io/fs" + "os" + "path/filepath" + "regexp" + "slices" + "strings" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/plan" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +const reviewer = "member-1" + +// Pad Thai and Shawarma Bowls, the seeded Quest log dinners, are planned +// on these days of fullWeek. +var ( + padThaiDay = mustDate("2026-10-14") + shawarmaDay = mustDate("2026-10-16") +) + +// putWeek stores fullWeek as the plan of week with status, as a draft and +// Push to Calendar would. +func (e *env) putWeek(t *testing.T, status string) { + t.Helper() + q := db.New(e.st.Writer) + now := store.Now().String() + id := store.NewID() + if err := q.CreateWeekPlan(t.Context(), db.CreateWeekPlanParams{ID: id, WeekStart: week.String(), CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatal(err) + } + for i, name := range fullWeek { + if err := q.PutPlanDay(t.Context(), db.PutPlanDayParams{ID: store.NewID(), PlanID: id, Date: weekDates[i], + DinnerID: sql.NullString{String: e.ids[name], Valid: true}, CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatal(err) + } + } + e.setStatus(t, status) +} + +func (e *env) dinnerRow(t *testing.T, name string) db.Dinner { + t.Helper() + d, err := db.New(e.st.Reader).GetDinner(t.Context(), e.ids[name]) + if err != nil { + t.Fatal(err) + } + return d +} + +// review is a rotation_reviews row as stored. +type review struct { + planDate, status string + decision, decidedAt, updatedBy sql.NullString +} + +func (e *env) reviews(t *testing.T, name string) []review { + t.Helper() + rows, err := e.st.Reader.QueryContext(t.Context(), + `SELECT plan_date, status, decision, decided_at, updated_by FROM rotation_reviews WHERE dinner_id = ? ORDER BY plan_date`, e.ids[name]) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + var out []review + for rows.Next() { + var r review + if err := rows.Scan(&r.planDate, &r.status, &r.decision, &r.decidedAt, &r.updatedBy); err != nil { + t.Fatal(err) + } + out = append(out, r) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + return out +} + +func (e *env) markTried(t *testing.T, date store.LocalDate, want int) { + t.Helper() + n, err := e.svc.MarkTried(t.Context(), date) + if err != nil { + t.Fatal(err) + } + if n != want { + t.Fatalf("MarkTried(%s) = %d, want %d", date, n, want) + } +} + +func TestMarkTried(t *testing.T) { + e := newEnv(t) + // Any plan status: a plan pushed to Calendar and edited still counts. + e.putWeek(t, plan.StatusChanged) + + e.markTried(t, padThaiDay, 1) + if got := e.dinnerRow(t, "Pad Thai").Status; got != "tried" { + t.Errorf("Pad Thai status = %s, want tried", got) + } + got := e.reviews(t, "Pad Thai") + if len(got) != 1 || got[0].planDate != padThaiDay.String() || got[0].status != "pending" || + got[0].decision.Valid || got[0].decidedAt.Valid || got[0].updatedBy.Valid { + t.Fatalf("Pad Thai reviews = %+v, want one pending for %s", got, padThaiDay) + } + if got := e.dinnerRow(t, "Shawarma Bowls").Status; got != "quest_log" { + t.Errorf("Shawarma Bowls, planned on another day, status = %s, want quest_log", got) + } + + // Idempotent: a second run moves nothing and opens no second check. + e.markTried(t, padThaiDay, 0) + if got := e.reviews(t, "Pad Thai"); len(got) != 1 || got[0].status != "pending" { + t.Errorf("Pad Thai reviews after a second run = %+v, want the one pending", got) + } + + // A day planning a Regular, or with no plan, moves nothing. + e.markTried(t, week, 0) + e.markTried(t, mustDate("2026-10-20"), 0) + if got := e.dinnerRow(t, "Beef Chili").Status; got != "regular" { + t.Errorf("Beef Chili status = %s, want regular", got) + } + if got := e.reviews(t, "Beef Chili"); len(got) != 0 { + t.Errorf("Beef Chili reviews = %+v, want none", got) + } +} + +// A dinner returned to the Quest log ("We didn't make it") is not marked +// tried again for the same day. +func TestMarkTriedAfterNotMade(t *testing.T) { + e := newEnv(t) + e.putWeek(t, plan.StatusDraft) + e.markTried(t, padThaiDay, 1) + if _, err := e.svc.DecideReview(t.Context(), e.ids["Pad Thai"], plan.DecisionNotMade, reviewer); err != nil { + t.Fatal(err) + } + e.markTried(t, padThaiDay, 0) + if got := e.dinnerRow(t, "Pad Thai").Status; got != "quest_log" { + t.Errorf("Pad Thai status = %s, want quest_log", got) + } + if got := e.reviews(t, "Pad Thai"); len(got) != 1 || got[0].status != "decided" { + t.Errorf("Pad Thai reviews = %+v, want the one decided", got) + } +} + +func TestPendingReviews(t *testing.T) { + e := newEnv(t) + e.putWeek(t, plan.StatusDraft) + got, err := e.svc.PendingReviews(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(got) != 0 { + t.Fatalf("pending before MarkTried = %+v, want none", got) + } + + e.markTried(t, shawarmaDay, 1) + e.markTried(t, padThaiDay, 1) + got, err = e.svc.PendingReviews(t.Context()) + if err != nil { + t.Fatal(err) + } + want := []plan.Review{ + {Dinner: plan.ReviewDinner{ID: e.ids["Pad Thai"], Name: "Pad Thai"}, PlanDate: padThaiDay.String()}, + {Dinner: plan.ReviewDinner{ID: e.ids["Shawarma Bowls"], Name: "Shawarma Bowls"}, PlanDate: shawarmaDay.String()}, + } + if !slices.Equal(got, want) { + t.Errorf("pending = %+v, want %+v", got, want) + } +} + +func TestDecideReview(t *testing.T) { + for _, tc := range []struct { + decision, status string + }{ + {plan.DecisionKeep, "regular"}, + {plan.DecisionDrop, "archived"}, + {plan.DecisionNotMade, "quest_log"}, + } { + t.Run(tc.decision, func(t *testing.T) { + e := newEnv(t) + e.putWeek(t, plan.StatusDraft) + e.markTried(t, padThaiDay, 1) + + d, err := e.svc.DecideReview(t.Context(), e.ids["Pad Thai"], tc.decision, reviewer) + if err != nil { + t.Fatal(err) + } + if d.Dinner.ID != e.ids["Pad Thai"] || d.Dinner.Name != "Pad Thai" || d.PlanDate != padThaiDay.String() || + d.Decision != tc.decision || d.DinnerStatus != tc.status || d.DecidedAt == "" || d.UpdatedBy != reviewer { + t.Errorf("decided = %+v", d) + } + row := e.dinnerRow(t, "Pad Thai") + if row.Status != tc.status || row.Origin != "side_quest" || row.Name != "Pad Thai" || row.UpdatedBy.String != reviewer { + t.Errorf("Pad Thai = status %s origin %s name %s updated_by %v, want %s side_quest Pad Thai %s", + row.Status, row.Origin, row.Name, row.UpdatedBy, tc.status, reviewer) + } + got := e.reviews(t, "Pad Thai") + if len(got) != 1 || got[0].status != "decided" || got[0].decision.String != tc.decision || + got[0].decidedAt.String != d.DecidedAt || got[0].updatedBy.String != reviewer { + t.Errorf("reviews = %+v, want one decided %s by %s at %s", got, tc.decision, reviewer, d.DecidedAt) + } + if pending, err := e.svc.PendingReviews(t.Context()); err != nil || len(pending) != 0 { + t.Errorf("pending after deciding = %+v, %v, want none", pending, err) + } + + // A second decision, of any kind, changes nothing. + for _, again := range []string{plan.DecisionKeep, plan.DecisionDrop, plan.DecisionNotMade} { + if _, err := e.svc.DecideReview(t.Context(), e.ids["Pad Thai"], again, reviewer); !errors.Is(err, plan.ErrNoPendingReview) { + t.Errorf("second decision %s: %v, want ErrNoPendingReview", again, err) + } + } + if got := e.dinnerRow(t, "Pad Thai").Status; got != tc.status { + t.Errorf("status after a second decision = %s, want %s", got, tc.status) + } + + passed, err := db.New(e.st.Reader).ListPassedNames(t.Context()) + if err != nil { + t.Fatal(err) + } + recent, err := db.New(e.st.Reader).ListRecentPassedNames(t.Context(), 10) + if err != nil { + t.Fatal(err) + } + wantPassed := tc.decision == plan.DecisionDrop + if slices.Contains(passed, "Pad Thai") != wantPassed || slices.Contains(recent, "Pad Thai") != wantPassed { + t.Errorf("passed names = %v, recent %v; want Pad Thai among them: %t", passed, recent, wantPassed) + } + }) + } +} + +func TestDecideReviewRefused(t *testing.T) { + e := newEnv(t) + e.putWeek(t, plan.StatusDraft) + e.markTried(t, padThaiDay, 1) + ctx := t.Context() + + var invalid *plan.InvalidDecisionError + if _, err := e.svc.DecideReview(ctx, e.ids["Pad Thai"], "promote", reviewer); !errors.As(err, &invalid) { + t.Errorf("decision promote: %v, want *InvalidDecisionError", err) + } + if _, err := e.svc.DecideReview(ctx, "nope", plan.DecisionKeep, reviewer); !errors.Is(err, plan.ErrDinnerNotFound) { + t.Errorf("unknown dinner: %v, want ErrDinnerNotFound", err) + } + // Never tried: Shawarma Bowls is in the Quest log with no check. + if _, err := e.svc.DecideReview(ctx, e.ids["Shawarma Bowls"], plan.DecisionKeep, reviewer); !errors.Is(err, plan.ErrNoPendingReview) { + t.Errorf("dinner never tried: %v, want ErrNoPendingReview", err) + } + if got := e.dinnerRow(t, "Shawarma Bowls").Status; got != "quest_log" { + t.Errorf("Shawarma Bowls status = %s, want quest_log", got) + } + if _, err := e.svc.DecideReview(ctx, e.ids["Pad Thai"], plan.DecisionKeep, ""); err == nil { + t.Error("decision with no member succeeded") + } + + // A tried dinner archived meanwhile is neither kept nor listed. + if _, err := e.st.Writer.ExecContext(ctx, `UPDATE dinners SET status = 'archived' WHERE id = ?`, e.ids["Pad Thai"]); err != nil { + t.Fatal(err) + } + if _, err := e.svc.DecideReview(ctx, e.ids["Pad Thai"], plan.DecisionKeep, reviewer); !errors.Is(err, plan.ErrDinnerNotTried) { + t.Errorf("archived dinner: %v, want ErrDinnerNotTried", err) + } + if got := e.dinnerRow(t, "Pad Thai").Status; got != "archived" { + t.Errorf("Pad Thai status = %s, want archived", got) + } + if got := e.reviews(t, "Pad Thai"); len(got) != 1 || got[0].status != "pending" { + t.Errorf("Pad Thai reviews = %+v, want the one still pending", got) + } + if pending, err := e.svc.PendingReviews(ctx); err != nil || len(pending) != 0 { + t.Errorf("pending = %+v, %v, want none for an archived dinner", pending, err) + } +} + +// TestOnlyKeepPromotesToRegular guards D-021: no code path but the Rotation +// check's keep may make an existing dinner a Regular. Every query writing +// dinners.status and every non-test use of the regular status must be one +// of those reviewed below; a new one fails here until it is reviewed. +func TestOnlyKeepPromotesToRegular(t *testing.T) { + // Each query writing dinners.status, and why it cannot promote. + statusWriters := map[string]string{ + "CreateDinner": "inserts a new dinner; only CreateRegular inserts one as a Regular", + "UpdateDinner": "dinners.Service.Update writes the status it read back", + "ArchiveDinner": "sets archived", + "MoveSuggestedDinner": "Try it / Pass move a suggested dinner to quest_log or archived", + "MarkDinnerTried": "sets tried", + "MoveTriedDinner": "DecideReview's decisionStatus: only keep maps to regular", + } + files, err := filepath.Glob("../../migrations/queries/*.sql") + if err != nil || len(files) == 0 { + t.Fatalf("no query files: %v", err) + } + writes := regexp.MustCompile(`(?is)^\s*(INSERT\s+INTO\s+dinners\b|UPDATE\s+dinners\s+SET\b.*\bstatus\s*=)`) + nameRe := regexp.MustCompile(`^\s*(\w+)`) + found := map[string]bool{} + for _, f := range files { + src, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + for _, stmt := range strings.Split(string(src), "-- name:")[1:] { + name := nameRe.FindStringSubmatch(stmt)[1] + var body []string + for _, line := range strings.Split(stmt, "\n")[1:] { + if !strings.HasPrefix(strings.TrimSpace(line), "--") { + body = append(body, line) + } + } + sqlText := strings.Join(body, "\n") + if !writes.MatchString(sqlText) { + continue + } + found[name] = true + if _, ok := statusWriters[name]; !ok { + t.Errorf("%s: query %s writes dinners.status; review it against D-021 and list it here", f, name) + } + if regexp.MustCompile(`(?i)\bstatus\s*=\s*'regular'`).MatchString(sqlText) { + t.Errorf("%s: query %s sets status regular outright", f, name) + } + } + } + for name := range statusWriters { + if !found[name] { + t.Errorf("reviewed query %s no longer writes dinners.status; drop it from the list", name) + } + } + + // Each non-test Go file naming the regular status, and how often. + uses := map[string]int{ + "dinners/dinners.go": 5, // the constant (2), listable, CreateRegular's status and origin + "plan/plan.go": 1, // plannable + "plan/rotation.go": 1, // decisionStatus[DecisionKeep] + "suggest/batch.go": 1, // reads which dinners are Regulars to cite + "ai/harness/context.go": 1, // reads the Regulars + "ai/skills/draft_week.go": 1, // KindRegular, a planned day's label + } + regular := regexp.MustCompile(`StatusRegular\b|"regular"`) + got := map[string]int{} + err = filepath.WalkDir("..", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() && d.Name() == "db" { + return filepath.SkipDir // generated; the queries are checked above + } + if d.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + src, err := os.ReadFile(path) + if err != nil { + return err + } + if n := len(regular.FindAllIndex(src, -1)); n > 0 { + got[filepath.ToSlash(strings.TrimPrefix(path, ".."+string(filepath.Separator)))] = n + } + return nil + }) + if err != nil { + t.Fatal(err) + } + for path, n := range got { + if uses[path] != n { + t.Errorf("%s names the regular status %d times, reviewed %d; review it against D-021", path, n, uses[path]) + } + } + for path, n := range uses { + if got[path] == 0 { + t.Errorf("%s no longer names the regular status (reviewed %d); update the list", path, n) + } + } +} diff --git a/internal/server/plans_test.go b/internal/server/plans_test.go index 86c1a18..00b2f2f 100644 --- a/internal/server/plans_test.go +++ b/internal/server/plans_test.go @@ -32,6 +32,7 @@ const plansPath = "/api/v1/plans/" type plansEnv struct { h http.Handler st *store.Store + svc *plan.Service questLog string } @@ -89,7 +90,7 @@ func newPlansRouter(t *testing.T) plansEnv { t.Fatal(err) } } - return plansEnv{h: h, st: st, questLog: dinner("Pad Thai", "quest_log", "stovetop", 35)} + return plansEnv{h: h, st: st, svc: svc, questLog: dinner("Pad Thai", "quest_log", "stovetop", 35)} } func (e plansEnv) setStatus(t *testing.T, status string) { diff --git a/internal/server/rotation.go b/internal/server/rotation.go new file mode 100644 index 0000000..f59b2bd --- /dev/null +++ b/internal/server/rotation.go @@ -0,0 +1,85 @@ +package server + +import ( + "errors" + "log/slog" + "net/http" + + "github.com/go-chi/chi/v5" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/plan" +) + +// maxRotationReviewBody bounds a POST /dinners/{id}/rotation-review body. +const maxRotationReviewBody = 1 << 10 + +// rotationDecision is the POST /dinners/{id}/rotation-review body: +// keep, drop ("Not for us") or not_made ("We didn't make it"). +type rotationDecision struct { + Decision *string `json:"decision"` +} + +type rotationReviewList struct { + Reviews []plan.Review `json:"reviews"` +} + +// getRotationReviews answers GET /rotation-reviews with the pending +// Rotation checks, oldest plan date first. +func getRotationReviews(logger *slog.Logger, svc *plan.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + list, err := svc.PendingReviews(r.Context()) + if err != nil { + writeRotationError(w, r, logger, "list Rotation checks", err) + return + } + writeJSON(w, r, logger, http.StatusOK, rotationReviewList{Reviews: list}) + } +} + +// postRotationReview applies the household's answer to "Keep it in +// rotation?" for a tried dinner, recording the session's actor as +// updated_by, and answers with the Rotation check as decided. +func postRotationReview(logger *slog.Logger, svc *plan.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + actor, ok := actorOrFail(w, r, logger) + if !ok { + return + } + body, err := decodePatch[rotationDecision](http.MaxBytesReader(w, r.Body, maxRotationReviewBody)) + if err != nil { + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", err.Error()) + return + } + if body.Decision == nil { + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", "decision: required") + return + } + d, err := svc.DecideReview(r.Context(), chi.URLParam(r, "id"), *body.Decision, actor.Subject) + if err != nil { + writeRotationError(w, r, logger, "decide Rotation check", err) + return + } + writeJSON(w, r, logger, http.StatusOK, d) + } +} + +// writeRotationError maps a Rotation check error to its API error: a +// decision other than keep, drop or not_made is 400, an unknown dinner +// 404, and a dinner with no pending check (such as a second decision) or +// no longer tried 409. +func writeRotationError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, action string, err error) { + var invalid *plan.InvalidDecisionError + switch { + case errors.As(err, &invalid): + writeError(w, r, logger, http.StatusBadRequest, "invalid_decision", invalid.Error()) + case errors.Is(err, plan.ErrDinnerNotFound): + writeError(w, r, logger, http.StatusNotFound, "not_found", "dinner not found") + case errors.Is(err, plan.ErrNoPendingReview): + writeError(w, r, logger, http.StatusConflict, "no_pending_review", err.Error()) + case errors.Is(err, plan.ErrDinnerNotTried): + writeError(w, r, logger, http.StatusConflict, "dinner_not_tried", err.Error()) + default: + logger.ErrorContext(r.Context(), action, "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + } +} diff --git a/internal/server/rotation_test.go b/internal/server/rotation_test.go new file mode 100644 index 0000000..503f1b8 --- /dev/null +++ b/internal/server/rotation_test.go @@ -0,0 +1,129 @@ +package server + +import ( + "net/http" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +const rotationReviewsPath = "/api/v1/rotation-reviews" + +// triedPadThai plans the Quest log dinner Pad Thai on Wednesday 2026-10-14 +// and marks the day tried, so Pad Thai has a pending Rotation check. +func triedPadThai(t *testing.T, e plansEnv) { + t.Helper() + rec, _ := callDinners(t, e.h, http.MethodPut, plansPath+"2026-10-12/days/2026-10-14", `{"dinner_id":"`+e.questLog+`"}`) + if rec.Code != http.StatusOK { + t.Fatalf("swap Pad Thai in = %d: %s", rec.Code, rec.Body) + } + day, err := store.ParseLocalDate("2026-10-14") + if err != nil { + t.Fatal(err) + } + if n, err := e.svc.MarkTried(t.Context(), day); err != nil || n != 1 { + t.Fatalf("MarkTried = %d, %v, want 1", n, err) + } +} + +func TestGetRotationReviews(t *testing.T) { + t.Parallel() + e := newPlansRouter(t) + + rec, body := callDinners(t, e.h, http.MethodGet, rotationReviewsPath, "") + if rec.Code != http.StatusOK { + t.Fatalf("GET = %d: %s", rec.Code, rec.Body) + } + if reviews, ok := body["reviews"].([]any); !ok || len(reviews) != 0 { + t.Errorf("GET before MarkTried = %v, want an empty reviews list", body) + } + + triedPadThai(t, e) + rec, body = callDinners(t, e.h, http.MethodGet, rotationReviewsPath, "") + if rec.Code != http.StatusOK { + t.Fatalf("GET = %d: %s", rec.Code, rec.Body) + } + reviews, _ := body["reviews"].([]any) + if len(reviews) != 1 { + t.Fatalf("reviews = %v, want one", body) + } + r := reviews[0].(map[string]any) + d, _ := r["dinner"].(map[string]any) + if len(r) != 2 || r["plan_date"] != "2026-10-14" || len(d) != 3 || + d["id"] != e.questLog || d["name"] != "Pad Thai" || d["photo"] != nil { + t.Errorf("review = %v, want {dinner {id, name, photo}, plan_date}", r) + } +} + +func TestPostRotationReview(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ decision, status string }{ + {"keep", "regular"}, + {"drop", "archived"}, + {"not_made", "quest_log"}, + } { + t.Run(tc.decision, func(t *testing.T) { + t.Parallel() + e := newPlansRouter(t) + triedPadThai(t, e) + target := dinnersPath + "/" + e.questLog + "/rotation-review" + + rec, body := callDinners(t, e.h, http.MethodPost, target, `{"decision":"`+tc.decision+`"}`) + if rec.Code != http.StatusOK { + t.Fatalf("POST = %d: %s", rec.Code, rec.Body) + } + if body["decision"] != tc.decision || body["dinner_status"] != tc.status || body["plan_date"] != "2026-10-14" || + body["updated_by"] != testSession.Subject || body["decided_at"] == "" { + t.Errorf("decided = %v", body) + } + var status, origin, updatedBy string + if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT status, origin, updated_by FROM dinners WHERE id = ?`, e.questLog). + Scan(&status, &origin, &updatedBy); err != nil { + t.Fatal(err) + } + if status != tc.status || origin != "regular" || updatedBy != testSession.Subject { + t.Errorf("dinner = %s %s %s, want %s regular %s", status, origin, updatedBy, tc.status, testSession.Subject) + } + + rec, body = callDinners(t, e.h, http.MethodPost, target, `{"decision":"keep"}`) + if rec.Code != http.StatusConflict || errorCode(body) != "no_pending_review" { + t.Errorf("second decision = %d %v, want 409 no_pending_review", rec.Code, body) + } + rec, body = callDinners(t, e.h, http.MethodGet, rotationReviewsPath, "") + if reviews, _ := body["reviews"].([]any); rec.Code != http.StatusOK || len(reviews) != 0 { + t.Errorf("GET after deciding = %d %v, want no reviews", rec.Code, body) + } + }) + } +} + +func TestPostRotationReviewRefused(t *testing.T) { + t.Parallel() + e := newPlansRouter(t) + triedPadThai(t, e) + target := dinnersPath + "/" + e.questLog + "/rotation-review" + + for _, tc := range []struct { + name, target, body string + code int + errCode string + }{ + {"no decision", target, `{}`, http.StatusBadRequest, "invalid_request"}, + {"null decision", target, `{"decision":null}`, http.StatusBadRequest, "invalid_request"}, + {"unknown field", target, `{"decision":"keep","status":"regular"}`, http.StatusBadRequest, "invalid_request"}, + {"unknown decision", target, `{"decision":"promote"}`, http.StatusBadRequest, "invalid_decision"}, + {"unknown dinner", dinnersPath + "/nope/rotation-review", `{"decision":"keep"}`, http.StatusNotFound, "not_found"}, + } { + rec, body := callDinners(t, e.h, http.MethodPost, tc.target, tc.body) + if rec.Code != tc.code || errorCode(body) != tc.errCode { + t.Errorf("%s = %d %v, want %d %s", tc.name, rec.Code, body, tc.code, tc.errCode) + } + } + var status string + if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT status FROM dinners WHERE id = ?`, e.questLog).Scan(&status); err != nil { + t.Fatal(err) + } + if status != "tried" { + t.Errorf("status after refused decisions = %s, want tried", status) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 966f21d..5fbceb6 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -43,8 +43,9 @@ type RouteMounter interface { // suggestSvc serves POST /api/v1/suggestions/batches, GET // /api/v1/suggestions/deck and POST /api/v1/suggestions/{id}/accept and // /decline; planSvc serves GET /api/v1/plans/{weekStart}, POST -// /api/v1/plans/{weekStart}/draft and PUT -// /api/v1/plans/{weekStart}/days/{date}; runner serves +// /api/v1/plans/{weekStart}/draft, PUT +// /api/v1/plans/{weekStart}/days/{date}, GET /api/v1/rotation-reviews and +// POST /api/v1/dinners/{id}/rotation-review; runner serves // POST /api/v1/jobs and GET /api/v1/jobs/{id}; photos serves // GET /media/photos/{id}-{size}.jpg; assets is the web build // served for every other GET. /health, /auth and the web build are public. @@ -125,6 +126,8 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a apiV1.Get("/plans/{weekStart}", getPlan(logger, planSvc)) apiV1.Post("/plans/{weekStart}/draft", postPlanDraft(logger, planSvc)) apiV1.Put("/plans/{weekStart}/days/{date}", putPlanDay(logger, planSvc)) + apiV1.Get("/rotation-reviews", getRotationReviews(logger, planSvc)) + apiV1.Post("/dinners/{id}/rotation-review", postRotationReview(logger, planSvc)) apiV1.Post("/jobs", postJob(logger, runner)) apiV1.Get("/jobs/{id}", getJob(logger, runner)) media.Get("/photos/{name}", getPhoto(logger, photos)) diff --git a/internal/suggest/batch.go b/internal/suggest/batch.go index 00aeccb..51ee427 100644 --- a/internal/suggest/batch.go +++ b/internal/suggest/batch.go @@ -43,7 +43,7 @@ type batchResult struct { func (r batchResult) PublicJSON() ([]byte, error) { return json.Marshal(r) } // runBatch runs batch p.BatchID: it asks suggest_side_quests for BatchSize -// Side quests given the household context, the deck and every declined +// Side quests given the household context, the deck and every passed // name, then grounds them in the model's order, at most the number still // wanted at a time, until BatchSize are kept or none are left. A Side quest // no recipe grounds is dropped; any other grounding failure fails the @@ -132,7 +132,7 @@ func (s *Service) fill(ctx context.Context, batchID string, publish func(batchRe } // request reads what suggest_side_quests is given: the household context, -// the deck's names and every declined name. A household with no Regulars +// the deck's names and every passed name (ListPassedNames). A household with no Regulars // fails with ReasonNoRegulars. func (s *Service) request(ctx context.Context) (skills.SideQuestsRequest, error) { hctx, err := harness.BuildContext(ctx, s.st, time.Now(), ContextBudget) @@ -151,9 +151,9 @@ func (s *Service) request(ctx context.Context) (skills.SideQuestsRequest, error) for _, row := range deck { pending = append(pending, row.Dinner.Name) } - declined, err := q.ListDeclinedSuggestionNames(ctx) + declined, err := q.ListPassedNames(ctx) if err != nil { - return skills.SideQuestsRequest{}, fmt.Errorf("suggest: list declined names: %w", err) + return skills.SideQuestsRequest{}, fmt.Errorf("suggest: list passed names: %w", err) } return skills.SideQuestsRequest{Context: hctx, Needed: BatchSize, Pending: pending, Declined: declined}, nil } @@ -201,9 +201,9 @@ func (s *Service) ground(ctx context.Context, quests []skills.SideQuest) ([]grou // suggestion citing its Regulars by id, all in one transaction, and // returns the suggestion ids. A Side quest is left out when, since // suggest_side_quests ran, a dinner it repeats was added, a suggestion it -// repeats passed, or a Regular it cites renamed or archived. Declined -// names are read in the same transaction as the store, so a Pass committed -// before it is never missed. +// repeats passed or a Rotation check dropped, or a Regular it cites renamed +// or archived. Passed names are read in the same transaction as the store, +// so a Pass or drop committed before it is never missed. func (s *Service) store(ctx context.Context, batchID string, quests []groundedQuest) ([]string, error) { ids := []string{} err := s.st.WithTx(ctx, func(tx *sql.Tx) error { @@ -212,9 +212,9 @@ func (s *Service) store(ctx context.Context, batchID string, quests []groundedQu if err != nil { return fmt.Errorf("suggest: list dinners for dedupe: %w", err) } - names, err := q.ListDeclinedSuggestionNames(ctx) + names, err := q.ListPassedNames(ctx) if err != nil { - return fmt.Errorf("suggest: list declined names for dedupe: %w", err) + return fmt.Errorf("suggest: list passed names for dedupe: %w", err) } regulars := map[string][]string{} for _, row := range existing { diff --git a/internal/suggest/suggest_test.go b/internal/suggest/suggest_test.go index 915b789..7bc5b24 100644 --- a/internal/suggest/suggest_test.go +++ b/internal/suggest/suggest_test.go @@ -646,7 +646,7 @@ func TestDeclineIsIdempotent(t *testing.T) { if got := e.dinnerStatus(t, s.DinnerID); got != "archived" { t.Errorf("dinner status after refused accept = %s, want archived", got) } - names, err := db.New(e.st.Reader).ListDeclinedSuggestionNames(t.Context()) + names, err := db.New(e.st.Reader).ListPassedNames(t.Context()) if err != nil { t.Fatal(err) } diff --git a/migrations/queries/household_context.sql b/migrations/queries/household_context.sql index fbea66d..78f5b20 100644 --- a/migrations/queries/household_context.sql +++ b/migrations/queries/household_context.sql @@ -1,11 +1,18 @@ -- Reads for the AI household context block (internal/ai/harness). --- name: ListRecentDeclinedNames :many --- The names the most recently declined suggestions were suggested under, --- newest first; renaming their dinners does not change them. -SELECT name -FROM suggestions -WHERE status = 'declined' +-- name: ListRecentPassedNames :many +-- The most recent names the household turned down, newest first, as +-- ListPassedNames has them. +SELECT name FROM ( + SELECT name, decided_at, id + FROM suggestions + WHERE status = 'declined' + UNION ALL + SELECT dinners.name, rotation_reviews.decided_at, rotation_reviews.id + FROM rotation_reviews + JOIN dinners ON dinners.id = rotation_reviews.dinner_id + WHERE rotation_reviews.decision = 'drop' +) ORDER BY decided_at DESC, id DESC LIMIT ?; diff --git a/migrations/queries/rotation_reviews.sql b/migrations/queries/rotation_reviews.sql new file mode 100644 index 0000000..073bd4d --- /dev/null +++ b/migrations/queries/rotation_reviews.sql @@ -0,0 +1,60 @@ +-- name: ListTriedCandidates :many +-- The ids of the dinners with status quest_log planned on date, on a plan +-- of any status, that have no Rotation check for date yet. +SELECT DISTINCT dinners.id +FROM plan_days +JOIN dinners ON dinners.id = plan_days.dinner_id +WHERE plan_days.date = sqlc.arg(date) + AND dinners.status = 'quest_log' + AND NOT EXISTS ( + SELECT 1 FROM rotation_reviews + WHERE rotation_reviews.dinner_id = dinners.id + AND rotation_reviews.plan_date = plan_days.date + ) +ORDER BY dinners.id; + +-- name: MarkDinnerTried :execrows +-- Moves a dinner still in the Quest log to tried; a dinner with any other +-- status is left as it is (0 rows). +UPDATE dinners +SET status = 'tried', updated_at = ? +WHERE id = ? AND status = 'quest_log'; + +-- name: CreateRotationReview :exec +INSERT INTO rotation_reviews (id, dinner_id, plan_date, status, created_at, updated_at) +VALUES (?, ?, ?, 'pending', ?, ?); + +-- name: ListPendingRotationReviews :many +-- The pending Rotation checks whose dinner is still tried, oldest plan +-- date first, with their dinners. +SELECT rotation_reviews.plan_date, sqlc.embed(dinners) +FROM rotation_reviews +JOIN dinners ON dinners.id = rotation_reviews.dinner_id +WHERE rotation_reviews.status = 'pending' AND dinners.status = 'tried' +ORDER BY rotation_reviews.plan_date, rotation_reviews.id; + +-- name: GetPendingRotationReview :one +-- The pending Rotation check of dinner_id. MarkTried makes one only for a +-- dinner in the Quest log, which it moves to tried, so a dinner has at +-- most one. Returns sql.ErrNoRows when there is none. +SELECT * FROM rotation_reviews +WHERE dinner_id = ? AND status = 'pending'; + +-- name: DecideRotationReview :execrows +-- Records the household's decision on a pending Rotation check; one +-- already decided is left as it is (0 rows). +UPDATE rotation_reviews +SET status = 'decided', + decision = sqlc.arg(decision), + decided_at = sqlc.arg(now), + updated_by = sqlc.arg(updated_by), + updated_at = sqlc.arg(now) +WHERE id = sqlc.arg(id) AND status = 'pending'; + +-- name: MoveTriedDinner :execrows +-- Moves a dinner still tried to status (regular, archived or quest_log) +-- by a Rotation check decision, recording updated_by; its origin is kept. +-- A dinner no longer tried is left as it is (0 rows). +UPDATE dinners +SET status = sqlc.arg(status), updated_by = sqlc.arg(updated_by), updated_at = sqlc.arg(updated_at) +WHERE id = sqlc.arg(id) AND status = 'tried'; diff --git a/migrations/queries/suggestions.sql b/migrations/queries/suggestions.sql index 2d81299..8f23acd 100644 --- a/migrations/queries/suggestions.sql +++ b/migrations/queries/suggestions.sql @@ -86,12 +86,21 @@ FROM suggestions JOIN dinners ON dinners.id = suggestions.dinner_id WHERE suggestions.status = 'pending' AND dinners.status = 'suggested'; --- name: ListDeclinedSuggestionNames :many --- The name every declined suggestion was suggested under, oldest decision --- first; renaming its dinner does not change it. -SELECT name -FROM suggestions -WHERE status = 'declined' +-- name: ListPassedNames :many +-- Every name the household turned down, oldest decision first: the name +-- each declined suggestion was suggested under, which renaming its dinner +-- does not change, and the name of each dinner a Rotation check dropped +-- ("Not for us"), which archiving keeps. +SELECT name FROM ( + SELECT name, decided_at, id + FROM suggestions + WHERE status = 'declined' + UNION ALL + SELECT dinners.name, rotation_reviews.decided_at, rotation_reviews.id + FROM rotation_reviews + JOIN dinners ON dinners.id = rotation_reviews.dinner_id + WHERE rotation_reviews.decision = 'drop' +) ORDER BY decided_at, id; -- name: DecideSuggestion :execrows -- 2.55.0 From cca6a8281009e46dff917a423a5afd2bde4f84c2 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 07:32:05 -0400 Subject: [PATCH 2/2] Keep a dropped dinner's name immutable and 404 a never-checked dinner R-01: a Rotation check decided drop now records the dinner's name at the decision in rotation_reviews.name (migration 0014, a table rebuild as 0012 did for suggestions; checks dropped before it take their dinner's name). ListPassedNames and ListRecentPassedNames read that name instead of the mutable dinners.name, so renaming the archived dinner no longer lets the dropped dish back into a Side quest batch. A CHECK keeps a name on drops only. R-02: deciding on a dinner that has never had a Rotation check is now ErrReviewNotFound, 404 not_found; a dinner whose checks are all decided, such as a second decision, stays 409 no_pending_review. Tests: TestLaterBatchNeverResuggestsRenamedDrop (drop, rename, next batch still refuses the original name), TestMigrateRotationReviewName (0013 to 0014 upgrade and constraints), TestPostRotationReviewNeverChecked (404), and TestDecideReview/TestDecideReviewRefused updated. Maestro: a1887d1c-c405-478c-ac7d-8cb8b40d280d Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 16 ++-- internal/db/household_context.sql.go | 5 +- internal/db/models.go | 1 + internal/db/querier.go | 11 ++- internal/db/rotation_reviews.sql.go | 31 ++++++-- internal/db/suggestions.sql.go | 9 +-- internal/plan/rotation.go | 24 ++++-- internal/plan/rotation_test.go | 26 ++++--- internal/server/rotation.go | 8 +- internal/server/rotation_test.go | 18 +++++ internal/store/migrate_test.go | 93 +++++++++++++++++++++++- internal/suggest/suggest_test.go | 69 ++++++++++++++++++ migrations/0014_rotation_review_name.sql | 43 +++++++++++ migrations/queries/household_context.sql | 5 +- migrations/queries/rotation_reviews.sql | 10 ++- migrations/queries/suggestions.sql | 9 +-- 16 files changed, 320 insertions(+), 58 deletions(-) create mode 100644 migrations/0014_rotation_review_name.sql diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index fb0927d..0b266bb 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -101,7 +101,7 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`). | `suggestions` | dinner_id, batch_id, name (the name the dish was suggested under, kept whatever its dinner is later renamed, so a Pass stays an immutable record every later batch dedupes against), reason, cited_regular_ids (JSON), status, decided_at. | | `week_plans` | week_start (unique), status (`draft` / `synced` / `changed`), ai_note, ai_run_id, calendar_synced_at, revision (raised by every swap and stored draft). | | `plan_days` | plan_id, date (unique per plan), dinner_id, is_new, calendar_event_id. | -| `rotation_reviews` | dinner_id + plan_date (unique), status, decision (`keep` / `drop` / `not_made`), decided_at, updated_by. | +| `rotation_reviews` | dinner_id + plan_date (unique), status, decision (`keep` / `drop` / `not_made`), name (set only by a drop: the dinner's name when dropped, kept whatever it is later renamed, so a drop stays among the passed names), decided_at, updated_by. | | `shopping_lists` | week_start (unique), plan_id, title, status (`current` / `archived`), sent_to_chat_at, frozen_at. | | `shopping_groups` / `shopping_items` | One group per dinner/day; item name, qty, needed (bool), position. | | `ai_settings` | One row: provider, base_url, model, temperature, system_prompt, api_key_ct, api_key_last4, key_verified_at. | @@ -378,12 +378,14 @@ automatic** (D-021). The check is closed with `decided_at` and the session's `updated_by`, and the answer is `{dinner, plan_date, decision, dinner_status, decided_at, updated_by}`. Any other decision is 400 `invalid_decision`, - an unknown dinner 404, a dinner with no pending check (including a - second decision) 409 `no_pending_review`, and one no longer tried 409 - `dinner_not_tried`. Keep is the only path from tried to Regular. A - dropped dinner's name joins the passed names (`ListPassedNames`) the - Side quest dedupe and household context use, so it is never suggested - again. + an unknown dinner or a dinner never checked 404 `not_found`, a dinner + whose checks are all decided (including a second decision) 409 + `no_pending_review`, and one no longer tried 409 `dinner_not_tried`. + Keep is the only path from tried to Regular. A drop records the + dinner's name on the check (`rotation_reviews.name`), and that name + joins the passed names (`ListPassedNames`) the Side quest dedupe and + household context use, so it is never suggested again, even after the + dinner is renamed. - `/health` — liveness. - Everything else serves the embedded PWA with SPA fallback. diff --git a/internal/db/household_context.sql.go b/internal/db/household_context.sql.go index a84a4dd..e03efdf 100644 --- a/internal/db/household_context.sql.go +++ b/internal/db/household_context.sql.go @@ -59,10 +59,9 @@ SELECT name FROM ( FROM suggestions WHERE status = 'declined' UNION ALL - SELECT dinners.name, rotation_reviews.decided_at, rotation_reviews.id + SELECT name, decided_at, id FROM rotation_reviews - JOIN dinners ON dinners.id = rotation_reviews.dinner_id - WHERE rotation_reviews.decision = 'drop' + WHERE decision = 'drop' ) ORDER BY decided_at DESC, id DESC LIMIT ? diff --git a/internal/db/models.go b/internal/db/models.go index 6a3998f..9cdf5d0 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -228,6 +228,7 @@ type RotationReview struct { PlanDate string Status string Decision sql.NullString + Name sql.NullString DecidedAt sql.NullString UpdatedBy sql.NullString CreatedAt string diff --git a/internal/db/querier.go b/internal/db/querier.go index 55be29f..88053ef 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -25,6 +25,8 @@ type Querier interface { CountDeck(ctx context.Context) (int64, error) // How many Regulars the household has; archived dinners are not counted. CountRegularDinners(ctx context.Context) (int64, error) + // How many Rotation checks dinner_id has had, pending or decided. + CountRotationReviews(ctx context.Context, dinnerID string) (int64, error) CreateAIRun(ctx context.Context, arg CreateAIRunParams) error CreateDinner(ctx context.Context, arg CreateDinnerParams) (Dinner, error) CreateJob(ctx context.Context, arg CreateJobParams) error @@ -40,8 +42,9 @@ type Querier interface { CreateSuggestion(ctx context.Context, arg CreateSuggestionParams) error CreateSuggestionBatch(ctx context.Context, arg CreateSuggestionBatchParams) (SuggestionBatch, error) CreateWeekPlan(ctx context.Context, arg CreateWeekPlanParams) error - // Records the household's decision on a pending Rotation check; one - // already decided is left as it is (0 rows). + // Records the household's decision on a pending Rotation check, with the + // dinner's name for a drop and NULL otherwise; one already decided is left + // as it is (0 rows). DecideRotationReview(ctx context.Context, arg DecideRotationReviewParams) (int64, error) // Records the household's decision on a pending suggestion; one already // decided is left as it is (0 rows). @@ -136,8 +139,8 @@ type Querier interface { ListDraftPool(ctx context.Context, arg ListDraftPoolParams) ([]Dinner, error) // Every name the household turned down, oldest decision first: the name // each declined suggestion was suggested under, which renaming its dinner - // does not change, and the name of each dinner a Rotation check dropped - // ("Not for us"), which archiving keeps. + // does not change, and the name each dinner a Rotation check dropped + // ("Not for us") had when dropped, which renaming it does not change. ListPassedNames(ctx context.Context) ([]string, error) // The pending Rotation checks whose dinner is still tried, oldest plan // date first, with their dinners. diff --git a/internal/db/rotation_reviews.sql.go b/internal/db/rotation_reviews.sql.go index cb613cd..bd819f0 100644 --- a/internal/db/rotation_reviews.sql.go +++ b/internal/db/rotation_reviews.sql.go @@ -10,6 +10,18 @@ import ( "database/sql" ) +const countRotationReviews = `-- name: CountRotationReviews :one +SELECT COUNT(*) FROM rotation_reviews WHERE dinner_id = ? +` + +// How many Rotation checks dinner_id has had, pending or decided. +func (q *Queries) CountRotationReviews(ctx context.Context, dinnerID string) (int64, error) { + row := q.db.QueryRowContext(ctx, countRotationReviews, dinnerID) + var count int64 + err := row.Scan(&count) + return count, err +} + const createRotationReview = `-- name: CreateRotationReview :exec INSERT INTO rotation_reviews (id, dinner_id, plan_date, status, created_at, updated_at) VALUES (?, ?, ?, 'pending', ?, ?) @@ -38,24 +50,28 @@ const decideRotationReview = `-- name: DecideRotationReview :execrows UPDATE rotation_reviews SET status = 'decided', decision = ?1, - decided_at = ?2, - updated_by = ?3, - updated_at = ?2 -WHERE id = ?4 AND status = 'pending' + name = ?2, + decided_at = ?3, + updated_by = ?4, + updated_at = ?3 +WHERE id = ?5 AND status = 'pending' ` type DecideRotationReviewParams struct { Decision sql.NullString + Name sql.NullString Now sql.NullString UpdatedBy sql.NullString ID string } -// Records the household's decision on a pending Rotation check; one -// already decided is left as it is (0 rows). +// Records the household's decision on a pending Rotation check, with the +// dinner's name for a drop and NULL otherwise; one already decided is left +// as it is (0 rows). func (q *Queries) DecideRotationReview(ctx context.Context, arg DecideRotationReviewParams) (int64, error) { result, err := q.db.ExecContext(ctx, decideRotationReview, arg.Decision, + arg.Name, arg.Now, arg.UpdatedBy, arg.ID, @@ -67,7 +83,7 @@ func (q *Queries) DecideRotationReview(ctx context.Context, arg DecideRotationRe } const getPendingRotationReview = `-- name: GetPendingRotationReview :one -SELECT id, dinner_id, plan_date, status, decision, decided_at, updated_by, created_at, updated_at FROM rotation_reviews +SELECT id, dinner_id, plan_date, status, decision, name, decided_at, updated_by, created_at, updated_at FROM rotation_reviews WHERE dinner_id = ? AND status = 'pending' ` @@ -83,6 +99,7 @@ func (q *Queries) GetPendingRotationReview(ctx context.Context, dinnerID string) &i.PlanDate, &i.Status, &i.Decision, + &i.Name, &i.DecidedAt, &i.UpdatedBy, &i.CreatedAt, diff --git a/internal/db/suggestions.sql.go b/internal/db/suggestions.sql.go index 1588fa2..e7f12d7 100644 --- a/internal/db/suggestions.sql.go +++ b/internal/db/suggestions.sql.go @@ -354,18 +354,17 @@ SELECT name FROM ( FROM suggestions WHERE status = 'declined' UNION ALL - SELECT dinners.name, rotation_reviews.decided_at, rotation_reviews.id + SELECT name, decided_at, id FROM rotation_reviews - JOIN dinners ON dinners.id = rotation_reviews.dinner_id - WHERE rotation_reviews.decision = 'drop' + WHERE decision = 'drop' ) ORDER BY decided_at, id ` // Every name the household turned down, oldest decision first: the name // each declined suggestion was suggested under, which renaming its dinner -// does not change, and the name of each dinner a Rotation check dropped -// ("Not for us"), which archiving keeps. +// does not change, and the name each dinner a Rotation check dropped +// ("Not for us") had when dropped, which renaming it does not change. func (q *Queries) ListPassedNames(ctx context.Context) ([]string, error) { rows, err := q.db.QueryContext(ctx, listPassedNames) if err != nil { diff --git a/internal/plan/rotation.go b/internal/plan/rotation.go index 6b4aa8d..129987e 100644 --- a/internal/plan/rotation.go +++ b/internal/plan/rotation.go @@ -32,8 +32,12 @@ var decisionStatus = map[string]string{ DecisionNotMade: dinners.StatusQuestLog, } -// ErrNoPendingReview is a decision on a dinner with no pending Rotation -// check: it was never tried, or its check is already decided. +// ErrReviewNotFound is a decision on a dinner that has never had a +// Rotation check: it was never tried. +var ErrReviewNotFound = errors.New("the dinner has no Rotation check") + +// ErrNoPendingReview is a decision on a dinner whose Rotation checks are +// all decided, such as a second decision. var ErrNoPendingReview = errors.New("the dinner has no pending Rotation check") // ErrDinnerNotTried is a decision on a pending Rotation check whose dinner @@ -130,9 +134,11 @@ func (s *Service) PendingReviews(ctx context.Context) ([]Review, error) { // DecideReview applies decision to the pending Rotation check of dinnerID // and closes it, recording updatedBy: keep makes the dinner a Regular, // drop archives it and not_made returns it to the Quest log; its origin is -// kept. An unknown dinner is ErrDinnerNotFound, a dinner with no pending -// check (including a second decision) ErrNoPendingReview, and one no -// longer tried ErrDinnerNotTried; none of them changes anything. +// kept; a drop records the dinner's name on the check, so renaming the +// dinner later never takes it off the passed names. An unknown dinner is +// ErrDinnerNotFound, a dinner never checked ErrReviewNotFound, one whose +// checks are all decided (such as a second decision) ErrNoPendingReview, +// and one no longer tried ErrDinnerNotTried; none of them changes anything. func (s *Service) DecideReview(ctx context.Context, dinnerID, decision, updatedBy string) (Decided, error) { status, ok := decisionStatus[decision] if !ok { @@ -153,6 +159,13 @@ func (s *Service) DecideReview(ctx context.Context, dinnerID, decision, updatedB } review, err := q.GetPendingRotationReview(ctx, d.ID) if errors.Is(err, sql.ErrNoRows) { + checks, err := q.CountRotationReviews(ctx, d.ID) + if err != nil { + return fmt.Errorf("plan: count Rotation checks of %s: %w", d.ID, err) + } + if checks == 0 { + return ErrReviewNotFound + } return ErrNoPendingReview } if err != nil { @@ -173,6 +186,7 @@ func (s *Service) DecideReview(ctx context.Context, dinnerID, decision, updatedB } closed, err := q.DecideRotationReview(ctx, db.DecideRotationReviewParams{ Decision: sql.NullString{String: decision, Valid: true}, + Name: sql.NullString{String: d.Name, Valid: decision == DecisionDrop}, Now: sql.NullString{String: now, Valid: true}, UpdatedBy: sql.NullString{String: updatedBy, Valid: true}, ID: review.ID, diff --git a/internal/plan/rotation_test.go b/internal/plan/rotation_test.go index 092cde7..f5b1f03 100644 --- a/internal/plan/rotation_test.go +++ b/internal/plan/rotation_test.go @@ -55,14 +55,14 @@ func (e *env) dinnerRow(t *testing.T, name string) db.Dinner { // review is a rotation_reviews row as stored. type review struct { - planDate, status string - decision, decidedAt, updatedBy sql.NullString + planDate, status string + decision, name, decidedAt, updatedBy sql.NullString } func (e *env) reviews(t *testing.T, name string) []review { t.Helper() rows, err := e.st.Reader.QueryContext(t.Context(), - `SELECT plan_date, status, decision, decided_at, updated_by FROM rotation_reviews WHERE dinner_id = ? ORDER BY plan_date`, e.ids[name]) + `SELECT plan_date, status, decision, name, decided_at, updated_by FROM rotation_reviews WHERE dinner_id = ? ORDER BY plan_date`, e.ids[name]) if err != nil { t.Fatal(err) } @@ -70,7 +70,7 @@ func (e *env) reviews(t *testing.T, name string) []review { var out []review for rows.Next() { var r review - if err := rows.Scan(&r.planDate, &r.status, &r.decision, &r.decidedAt, &r.updatedBy); err != nil { + if err := rows.Scan(&r.planDate, &r.status, &r.decision, &r.name, &r.decidedAt, &r.updatedBy); err != nil { t.Fatal(err) } out = append(out, r) @@ -103,7 +103,7 @@ func TestMarkTried(t *testing.T) { } got := e.reviews(t, "Pad Thai") if len(got) != 1 || got[0].planDate != padThaiDay.String() || got[0].status != "pending" || - got[0].decision.Valid || got[0].decidedAt.Valid || got[0].updatedBy.Valid { + got[0].decision.Valid || got[0].name.Valid || got[0].decidedAt.Valid || got[0].updatedBy.Valid { t.Fatalf("Pad Thai reviews = %+v, want one pending for %s", got, padThaiDay) } if got := e.dinnerRow(t, "Shawarma Bowls").Status; got != "quest_log" { @@ -197,10 +197,17 @@ func TestDecideReview(t *testing.T) { t.Errorf("Pad Thai = status %s origin %s name %s updated_by %v, want %s side_quest Pad Thai %s", row.Status, row.Origin, row.Name, row.UpdatedBy, tc.status, reviewer) } + // Only a drop records the dinner's name. + wantPassed := tc.decision == plan.DecisionDrop + wantName := sql.NullString{} + if wantPassed { + wantName = sql.NullString{String: "Pad Thai", Valid: true} + } got := e.reviews(t, "Pad Thai") if len(got) != 1 || got[0].status != "decided" || got[0].decision.String != tc.decision || - got[0].decidedAt.String != d.DecidedAt || got[0].updatedBy.String != reviewer { - t.Errorf("reviews = %+v, want one decided %s by %s at %s", got, tc.decision, reviewer, d.DecidedAt) + got[0].decidedAt.String != d.DecidedAt || got[0].updatedBy.String != reviewer || + got[0].name != wantName { + t.Errorf("reviews = %+v, want one decided %s by %s at %s, named only by a drop", got, tc.decision, reviewer, d.DecidedAt) } if pending, err := e.svc.PendingReviews(t.Context()); err != nil || len(pending) != 0 { t.Errorf("pending after deciding = %+v, %v, want none", pending, err) @@ -224,7 +231,6 @@ func TestDecideReview(t *testing.T) { if err != nil { t.Fatal(err) } - wantPassed := tc.decision == plan.DecisionDrop if slices.Contains(passed, "Pad Thai") != wantPassed || slices.Contains(recent, "Pad Thai") != wantPassed { t.Errorf("passed names = %v, recent %v; want Pad Thai among them: %t", passed, recent, wantPassed) } @@ -246,8 +252,8 @@ func TestDecideReviewRefused(t *testing.T) { t.Errorf("unknown dinner: %v, want ErrDinnerNotFound", err) } // Never tried: Shawarma Bowls is in the Quest log with no check. - if _, err := e.svc.DecideReview(ctx, e.ids["Shawarma Bowls"], plan.DecisionKeep, reviewer); !errors.Is(err, plan.ErrNoPendingReview) { - t.Errorf("dinner never tried: %v, want ErrNoPendingReview", err) + if _, err := e.svc.DecideReview(ctx, e.ids["Shawarma Bowls"], plan.DecisionKeep, reviewer); !errors.Is(err, plan.ErrReviewNotFound) { + t.Errorf("dinner never tried: %v, want ErrReviewNotFound", err) } if got := e.dinnerRow(t, "Shawarma Bowls").Status; got != "quest_log" { t.Errorf("Shawarma Bowls status = %s, want quest_log", got) diff --git a/internal/server/rotation.go b/internal/server/rotation.go index f59b2bd..a9f6232 100644 --- a/internal/server/rotation.go +++ b/internal/server/rotation.go @@ -64,9 +64,9 @@ func postRotationReview(logger *slog.Logger, svc *plan.Service) http.HandlerFunc } // writeRotationError maps a Rotation check error to its API error: a -// decision other than keep, drop or not_made is 400, an unknown dinner -// 404, and a dinner with no pending check (such as a second decision) or -// no longer tried 409. +// decision other than keep, drop or not_made is 400, an unknown dinner or +// a dinner never checked 404, and a dinner whose checks are all decided +// (such as a second decision) or no longer tried 409. func writeRotationError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, action string, err error) { var invalid *plan.InvalidDecisionError switch { @@ -74,6 +74,8 @@ func writeRotationError(w http.ResponseWriter, r *http.Request, logger *slog.Log writeError(w, r, logger, http.StatusBadRequest, "invalid_decision", invalid.Error()) case errors.Is(err, plan.ErrDinnerNotFound): writeError(w, r, logger, http.StatusNotFound, "not_found", "dinner not found") + case errors.Is(err, plan.ErrReviewNotFound): + writeError(w, r, logger, http.StatusNotFound, "not_found", "rotation check not found") case errors.Is(err, plan.ErrNoPendingReview): writeError(w, r, logger, http.StatusConflict, "no_pending_review", err.Error()) case errors.Is(err, plan.ErrDinnerNotTried): diff --git a/internal/server/rotation_test.go b/internal/server/rotation_test.go index 503f1b8..05e4c15 100644 --- a/internal/server/rotation_test.go +++ b/internal/server/rotation_test.go @@ -97,6 +97,24 @@ func TestPostRotationReview(t *testing.T) { } } +// A dinner that has never had a Rotation check is 404; a second decision +// on one already decided is 409 (TestPostRotationReview). +func TestPostRotationReviewNeverChecked(t *testing.T) { + t.Parallel() + e := newPlansRouter(t) + rec, body := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+e.questLog+"/rotation-review", `{"decision":"keep"}`) + if rec.Code != http.StatusNotFound || errorCode(body) != "not_found" { + t.Errorf("never checked = %d %v, want 404 not_found", rec.Code, body) + } + var status string + if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT status FROM dinners WHERE id = ?`, e.questLog).Scan(&status); err != nil { + t.Fatal(err) + } + if status != "quest_log" { + t.Errorf("status after a refused decision = %s, want quest_log", status) + } +} + func TestPostRotationReviewRefused(t *testing.T) { t.Parallel() e := newPlansRouter(t) diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index 9662138..5533fdc 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -176,8 +176,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name", got) } } @@ -197,8 +197,8 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) { if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name", got) } if !tableExists(t, s, "dinners") { t.Error("table from 0001_init was not created") @@ -633,3 +633,88 @@ func TestMigrateSuggestionName(t *testing.T) { t.Error("suggestions_copy left behind") } } + +// TestMigrateRotationReviewName upgrades a database at 0013 holding a +// pending, a kept and a dropped Rotation check and checks 0014 keeps all +// three, gives only the drop its dinner's name, and refuses a drop with no +// name or a name on any other check. +func TestMigrateRotationReviewName(t *testing.T) { + ctx := context.Background() + s := openTestStore(t) + before := fstest.MapFS{} + for _, name := range []string{ + "0000_bootstrap.sql", "0001_init.sql", "0002_household_settings_updated_by.sql", "0003_jobs_status.sql", + "0004_ai_provider_presets.sql", "0005_ai_provider_xai.sql", "0006_ai_runs_status.sql", "0007_recipe_grounding.sql", + "0008_dinner_recipe_url.sql", "0009_dinner_grounding_error.sql", "0010_dinner_grounding_revision.sql", + "0011_ground_dinner_job_revision.sql", "0012_suggestion_name.sql", "0013_week_plan_revision.sql", + } { + body, err := fs.ReadFile(migrations.FS, name) + if err != nil { + t.Fatal(err) + } + before[name] = &fstest.MapFile{Data: body} + } + if err := Migrate(ctx, s.Writer, before); err != nil { + t.Fatal(err) + } + for _, stmt := range []string{ + `INSERT INTO dinners (id, name, normalized_name, status, origin, is_favorite, grounding_status, created_at, updated_at) + VALUES ('dinner-1', 'Pho', 'pho', 'tried', 'side_quest', 0, 'grounded', 'c', 'u'), + ('dinner-2', 'Ramen', 'ramen', 'regular', 'side_quest', 0, 'grounded', 'c', 'u'), + ('dinner-3', 'Laksa', 'laksa', 'archived', 'side_quest', 0, 'grounded', 'c', 'u')`, + `INSERT INTO rotation_reviews (id, dinner_id, plan_date, status, decision, decided_at, updated_by, created_at, updated_at) + VALUES ('review-1', 'dinner-1', '2026-10-12', 'pending', NULL, NULL, NULL, 'c1', 'u1'), + ('review-2', 'dinner-2', '2026-10-13', 'decided', 'keep', 'd2', 'member-1', 'c2', 'u2'), + ('review-3', 'dinner-3', '2026-10-14', 'decided', 'drop', 'd3', 'member-1', 'c3', 'u3')`, + } { + if _, err := s.Writer.Exec(stmt); err != nil { + t.Fatalf("seed: %v", err) + } + } + + if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + + rows, err := s.Reader.Query(`SELECT id, dinner_id, plan_date, status, COALESCE(decision, ''), COALESCE(name, ''), + COALESCE(decided_at, ''), COALESCE(updated_by, ''), created_at, updated_at + FROM rotation_reviews ORDER BY id`) + if err != nil { + t.Fatal(err) + } + var got [][]string + for rows.Next() { + r := make([]string, 10) + if err := rows.Scan(&r[0], &r[1], &r[2], &r[3], &r[4], &r[5], &r[6], &r[7], &r[8], &r[9]); err != nil { + t.Fatal(err) + } + got = append(got, r) + } + if err := rows.Close(); err != nil { + t.Fatal(err) + } + want := [][]string{ + {"review-1", "dinner-1", "2026-10-12", "pending", "", "", "", "", "c1", "u1"}, + {"review-2", "dinner-2", "2026-10-13", "decided", "keep", "", "d2", "member-1", "c2", "u2"}, + {"review-3", "dinner-3", "2026-10-14", "decided", "drop", "Laksa", "d3", "member-1", "c3", "u3"}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("rotation_reviews after 0014 =\n %v\nwant\n %v", got, want) + } + for _, tc := range []struct{ name, stmt, want string }{ + {"drop with no name", `UPDATE rotation_reviews SET name = NULL WHERE id = 'review-3'`, "CHECK constraint failed"}, + {"empty name", `UPDATE rotation_reviews SET name = '' WHERE id = 'review-3'`, "CHECK constraint failed"}, + {"name on a keep", `UPDATE rotation_reviews SET name = 'Ramen' WHERE id = 'review-2'`, "CHECK constraint failed"}, + {"name on a pending check", `UPDATE rotation_reviews SET name = 'Pho' WHERE id = 'review-1'`, "CHECK constraint failed"}, + {"unknown dinner", `UPDATE rotation_reviews SET dinner_id = 'nope' WHERE id = 'review-1'`, "FOREIGN KEY constraint failed"}, + {"second check of a dinner and date", `INSERT INTO rotation_reviews (id, dinner_id, plan_date, status, created_at, updated_at) + VALUES ('review-4', 'dinner-1', '2026-10-12', 'pending', 'c', 'u')`, "UNIQUE constraint failed"}, + } { + if _, err := s.Writer.Exec(tc.stmt); err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: error %v, want %s", tc.name, err, tc.want) + } + } + if tableExists(t, s, "rotation_reviews_copy") { + t.Error("rotation_reviews_copy left behind") + } +} diff --git a/internal/suggest/suggest_test.go b/internal/suggest/suggest_test.go index 7bc5b24..469579a 100644 --- a/internal/suggest/suggest_test.go +++ b/internal/suggest/suggest_test.go @@ -21,6 +21,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/plan" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" @@ -912,6 +913,74 @@ func TestLaterBatchNeverResuggestsRenamedPass(t *testing.T) { } } +// TestLaterBatchNeverResuggestsRenamedDrop drops a tried dinner at its +// Rotation check ("Not for us"), renames its archived dinner, and checks +// the next batch still refuses the name it was dropped under. +func TestLaterBatchNeverResuggestsRenamedDrop(t *testing.T) { + e := newEnv(t) + ctx := t.Context() + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + dinnerSvc, err := dinners.NewService(e.st, e.grounder, e.runner, logger) + if err != nil { + t.Fatal(err) + } + fail := func(context.Context, string, skills.WeekRequest) (skills.Week, error) { + return skills.Week{}, errors.New("no draft in this test") + } + planSvc, err := plan.NewService(e.st, fail, e.runner, logger) + if err != nil { + t.Fatal(err) + } + id := e.dinner(t, "Pad Thai", "quest_log") + q := db.New(e.st.Writer) + now := store.Now().String() + planID := store.NewID() + if err := q.CreateWeekPlan(ctx, db.CreateWeekPlanParams{ID: planID, WeekStart: "2026-10-12", CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatal(err) + } + if err := q.PutPlanDay(ctx, db.PutPlanDayParams{ID: store.NewID(), PlanID: planID, Date: "2026-10-14", + DinnerID: sql.NullString{String: id, Valid: true}, CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatal(err) + } + day, err := store.ParseLocalDate("2026-10-14") + if err != nil { + t.Fatal(err) + } + if n, err := planSvc.MarkTried(ctx, day); err != nil || n != 1 { + t.Fatalf("MarkTried = %d, %v, want 1", n, err) + } + if _, err := planSvc.DecideReview(ctx, id, plan.DecisionDrop, member); err != nil { + t.Fatal(err) + } + renamed := "Peanut Noodles" + if _, err := dinnerSvc.Update(ctx, id, dinners.Patch{Name: &renamed}, member); err != nil { + t.Fatal(err) + } + + // The respelled dropped dish would ground, so only the passed names + // keep it out of the deck. + e.recipe(t, "pad thai!") + e.recipe(t, "Beef Ragu") + e.reply( + sideQuest("pad thai!", "Noodles, like your Chicken Fajitas.", "Chicken Fajitas"), + sideQuest("Beef Ragu", "Slow beef like your Beef Chili.", "Beef Chili"), + ) + e.start(t) + res := e.runBatch(t) + if len(res.SuggestionIDs) != 1 || res.Dropped != 1 { + t.Errorf("batch result = %+v, want 1 kept and the dropped dish dropped", res) + } + if got := e.deckNames(t); !slices.Equal(got, []string{"Beef Ragu"}) { + t.Errorf("deck = %v, want [Beef Ragu]", got) + } + e.mu.Lock() + in := e.inputs[0] + e.mu.Unlock() + if !slices.Equal(in.Declined, []string{"Pad Thai"}) || !slices.Equal(in.Context.Passed, []string{"Pad Thai"}) { + t.Errorf("batch declined %v, passed %v; want the name it was dropped under in both", in.Declined, in.Context.Passed) + } +} + // TestDecisionNeverCommitsWithoutItsReplenish makes the replenish batch's // job fail to store: the decision must not be recorded either, so the // retry decides again and starts the replenish. diff --git a/migrations/0014_rotation_review_name.sql b/migrations/0014_rotation_review_name.sql new file mode 100644 index 0000000..b1fd14c --- /dev/null +++ b/migrations/0014_rotation_review_name.sql @@ -0,0 +1,43 @@ +-- 0014_rotation_review_name: a Rotation check decided drop ("Not for us") +-- keeps the name its dinner had when the household dropped it. The dinner +-- can be renamed, even once archived, so the passed names every later +-- batch refuses are read from here, never from the dinner, as 0012 does +-- for a Pass. Only a drop has a name. A check dropped before this takes +-- its dinner's name now. +-- SQLite cannot add a column with a CHECK that existing drops would fail +-- before they are filled in, so the table is rebuilt as 0012 does: copy +-- the rows aside, drop the table, create it again, copy the rows back and +-- drop the copy. Nothing references rotation_reviews. + +CREATE TABLE rotation_reviews_copy AS SELECT * FROM rotation_reviews; + +DROP TABLE rotation_reviews; + +CREATE TABLE rotation_reviews ( + id TEXT NOT NULL PRIMARY KEY, + dinner_id TEXT NOT NULL REFERENCES dinners (id), + plan_date TEXT NOT NULL, + status TEXT NOT NULL CHECK (status IN ('pending', 'decided')), + decision TEXT CHECK (decision IN ('keep', 'drop', 'not_made')), + name TEXT CHECK (name <> ''), -- the dinner's name when dropped + decided_at TEXT, + updated_by TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + UNIQUE (dinner_id, plan_date), + CHECK ((decision IS 'drop') = (name IS NOT NULL)) +); + +INSERT INTO rotation_reviews ( + id, dinner_id, plan_date, status, decision, name, decided_at, updated_by, created_at, updated_at +) +SELECT + rotation_reviews_copy.id, rotation_reviews_copy.dinner_id, rotation_reviews_copy.plan_date, + rotation_reviews_copy.status, rotation_reviews_copy.decision, + CASE WHEN rotation_reviews_copy.decision = 'drop' THEN dinners.name END, + rotation_reviews_copy.decided_at, rotation_reviews_copy.updated_by, + rotation_reviews_copy.created_at, rotation_reviews_copy.updated_at +FROM rotation_reviews_copy +JOIN dinners ON dinners.id = rotation_reviews_copy.dinner_id; + +DROP TABLE rotation_reviews_copy; diff --git a/migrations/queries/household_context.sql b/migrations/queries/household_context.sql index 78f5b20..38f5c3c 100644 --- a/migrations/queries/household_context.sql +++ b/migrations/queries/household_context.sql @@ -8,10 +8,9 @@ SELECT name FROM ( FROM suggestions WHERE status = 'declined' UNION ALL - SELECT dinners.name, rotation_reviews.decided_at, rotation_reviews.id + SELECT name, decided_at, id FROM rotation_reviews - JOIN dinners ON dinners.id = rotation_reviews.dinner_id - WHERE rotation_reviews.decision = 'drop' + WHERE decision = 'drop' ) ORDER BY decided_at DESC, id DESC LIMIT ?; diff --git a/migrations/queries/rotation_reviews.sql b/migrations/queries/rotation_reviews.sql index 073bd4d..2d857d2 100644 --- a/migrations/queries/rotation_reviews.sql +++ b/migrations/queries/rotation_reviews.sql @@ -40,12 +40,18 @@ ORDER BY rotation_reviews.plan_date, rotation_reviews.id; SELECT * FROM rotation_reviews WHERE dinner_id = ? AND status = 'pending'; +-- name: CountRotationReviews :one +-- How many Rotation checks dinner_id has had, pending or decided. +SELECT COUNT(*) FROM rotation_reviews WHERE dinner_id = ?; + -- name: DecideRotationReview :execrows --- Records the household's decision on a pending Rotation check; one --- already decided is left as it is (0 rows). +-- Records the household's decision on a pending Rotation check, with the +-- dinner's name for a drop and NULL otherwise; one already decided is left +-- as it is (0 rows). UPDATE rotation_reviews SET status = 'decided', decision = sqlc.arg(decision), + name = sqlc.narg(name), decided_at = sqlc.arg(now), updated_by = sqlc.arg(updated_by), updated_at = sqlc.arg(now) diff --git a/migrations/queries/suggestions.sql b/migrations/queries/suggestions.sql index 8f23acd..1068622 100644 --- a/migrations/queries/suggestions.sql +++ b/migrations/queries/suggestions.sql @@ -89,17 +89,16 @@ WHERE suggestions.status = 'pending' AND dinners.status = 'suggested'; -- name: ListPassedNames :many -- Every name the household turned down, oldest decision first: the name -- each declined suggestion was suggested under, which renaming its dinner --- does not change, and the name of each dinner a Rotation check dropped --- ("Not for us"), which archiving keeps. +-- does not change, and the name each dinner a Rotation check dropped +-- ("Not for us") had when dropped, which renaming it does not change. SELECT name FROM ( SELECT name, decided_at, id FROM suggestions WHERE status = 'declined' UNION ALL - SELECT dinners.name, rotation_reviews.decided_at, rotation_reviews.id + SELECT name, decided_at, id FROM rotation_reviews - JOIN dinners ON dinners.id = rotation_reviews.dinner_id - WHERE rotation_reviews.decision = 'drop' + WHERE decision = 'drop' ) ORDER BY decided_at, id; -- 2.55.0