Enforce unique recipe item positions per recipe
CI / ci (push) Successful in 9m17s
CI / ci (pull_request) Successful in 8m10s

Add migration 0018 with unique indexes on recipe_ingredients and
recipe_steps (recipe_id, position), so a recipe can never hold two
ingredients or two steps at one position. The dinner detail read now
orders by position alone and drops its read-time duplicate check; a
database already holding a duplicate fails the migration.

Maestro task 75bb8cc1-fd30-4050-90d2-a1627a865b22

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-11 08:50:11 -04:00
co-authored by Claude Opus 5.5
parent 791b321b71
commit da4f048f85
8 changed files with 119 additions and 54 deletions
+93 -4
View File
@@ -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,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed" {
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,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed", 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,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique" {
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,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique", 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,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed" {
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,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed", 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,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique" {
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,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique", got)
}
if !tableExists(t, s, "dinners") {
t.Error("table from 0001_init was not created")
@@ -922,3 +922,92 @@ func TestMigrateShoppingListChatSendUnconfirmed(t *testing.T) {
t.Errorf("shopping list after 0017 = %s, want %s", got, want)
}
}
// seedRecipeItems inserts two recipes, each with an ingredient and a step
// at position 0, then the extra statements.
func seedRecipeItems(t *testing.T, s *Store, extra ...string) {
t.Helper()
stmts := []string{
`INSERT INTO recipes (id, source_url, title, raw_jsonld, fetched_at, created_at, updated_at)
VALUES ('recipe-1', 'https://example.com/chili', 'Chili', '{}', 'f', 'c', 'u')`,
`INSERT INTO recipes (id, source_url, title, raw_jsonld, fetched_at, created_at, updated_at)
VALUES ('recipe-2', 'https://example.com/curry', 'Curry', '{}', 'f', 'c', 'u')`,
`INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-1', 'recipe-1', 0, '1 onion', 'c', 'u'), ('ing-2', 'recipe-2', 0, '1 lime', 'c', 'u')`,
`INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-1', 'recipe-1', 0, 'Chop.', 'c', 'u'), ('step-2', 'recipe-2', 0, 'Squeeze.', 'c', 'u')`,
}
for _, stmt := range append(stmts, extra...) {
if _, err := s.Writer.Exec(stmt); err != nil {
t.Fatalf("seed: %v", err)
}
}
}
// TestMigrateRecipeItemPositionUnique applies 0018 over two recipes with
// items at the same position: each recipe keeps its items, and a second
// ingredient or step at a position its recipe already uses is refused.
func TestMigrateRecipeItemPositionUnique(t *testing.T) {
ctx := context.Background()
s := openTestStore(t)
migrateBefore(t, s, "0018")
seedRecipeItems(t, s)
if err := Migrate(ctx, s.Writer, migrations.FS); err != nil {
t.Fatal(err)
}
var got string
if err := s.Reader.QueryRowContext(ctx, `SELECT
(SELECT group_concat(id || '@' || recipe_id || ':' || position, ',') FROM (SELECT * FROM recipe_ingredients ORDER BY id)) || ' / ' ||
(SELECT group_concat(id || '@' || recipe_id || ':' || position, ',') FROM (SELECT * FROM recipe_steps ORDER BY id))`).Scan(&got); err != nil {
t.Fatal(err)
}
if want := "ing-1@recipe-1:0,ing-2@recipe-2:0 / step-1@recipe-1:0,step-2@recipe-2:0"; got != want {
t.Errorf("recipe items after 0018 = %s, want %s", got, want)
}
for _, tc := range []struct{ name, stmt string }{
{"second ingredient at a position", `INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-3', 'recipe-1', 0, '2 onions', 'c', 'u')`},
{"second step at a position", `INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-3', 'recipe-1', 0, 'Dice.', 'c', 'u')`},
{"ingredient moved onto another's position", `UPDATE recipe_ingredients SET recipe_id = 'recipe-1' WHERE id = 'ing-2'`},
{"step moved onto another's position", `UPDATE recipe_steps SET recipe_id = 'recipe-1' WHERE id = 'step-2'`},
} {
if _, err := s.Writer.Exec(tc.stmt); err == nil || !strings.Contains(err.Error(), "UNIQUE constraint failed") {
t.Errorf("%s: error %v, want UNIQUE constraint failed", tc.name, err)
}
}
}
// TestMigrateRecipeItemPositionUniqueRefusesDuplicate applies 0018 over a
// recipe already holding two ingredients, or two steps, at one position:
// its order is unknown, so the migration fails and is not recorded.
func TestMigrateRecipeItemPositionUniqueRefusesDuplicate(t *testing.T) {
for _, tc := range []struct{ table, stmt string }{
{"recipe_ingredients", `INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-3', 'recipe-1', 0, '2 onions', 'c', 'u')`},
{"recipe_steps", `INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-3', 'recipe-1', 0, 'Dice.', 'c', 'u')`},
} {
t.Run(tc.table, func(t *testing.T) {
ctx := context.Background()
s := openTestStore(t)
migrateBefore(t, s, "0018")
seedRecipeItems(t, s, tc.stmt)
err := Migrate(ctx, s.Writer, migrations.FS)
if err == nil || !strings.Contains(err.Error(), "UNIQUE constraint failed: "+tc.table+".recipe_id, "+tc.table+".position") {
t.Fatalf("Migrate = %v, want the duplicate position refused", err)
}
var n int
if err := s.Reader.QueryRowContext(ctx, `SELECT count(*) FROM schema_migrations WHERE version = '0018_recipe_item_position_unique'`).Scan(&n); err != nil {
t.Fatal(err)
}
if n != 0 {
t.Error("0018 recorded as applied")
}
})
}
}
+4
View File
@@ -408,6 +408,8 @@ func TestSchemaUniqueConstraintsRejectDuplicates(t *testing.T) {
{"notification_outbox", []string{"event_key", "channel"}},
{"schedule_runs", []string{"schedule", "period_key"}},
{"notification_prefs", []string{"event"}},
{"recipe_ingredients", []string{"recipe_id", "position"}},
{"recipe_steps", []string{"recipe_id", "position"}},
}
s := seededStore(t)
for _, c := range cases {
@@ -445,6 +447,8 @@ func TestSchemaCompositeUniqueAllowsPartialOverlap(t *testing.T) {
{"same plan_date, different dinner", "rotation_reviews", "plan_date", "dinners", map[string]any{"dinner_id": rowID("dinners", 1)}},
{"same event_key, different channel", "notification_outbox", "event_key", "", map[string]any{"channel": "chat"}},
{"same period_key, different schedule", "schedule_runs", "period_key", "", map[string]any{"schedule": "week_rollover"}},
{"same position, different recipe", "recipe_ingredients", "position", "recipes", map[string]any{"recipe_id": rowID("recipes", 1)}},
{"same position, different recipe", "recipe_steps", "position", "recipes", map[string]any{"recipe_id": rowID("recipes", 1)}},
}
s := seededStore(t)
for _, c := range cases {