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 <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user