Enforce unique recipe item positions per recipe (Maestro 75bb8cc1) (#198)

Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>

internal/store/migrate_test.go

@@ -177,8 +177,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,0020_site_name_from_page" {
-		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,0020_site_name_from_page", 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,0020_site_name_from_page" {
+		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,0020_site_name_from_page", got)
 	}
 }
 
@@ -198,8 +198,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,0020_site_name_from_page" {
-		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,0020_site_name_from_page", 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,0020_site_name_from_page" {
+		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,0020_site_name_from_page", got)
 	}
 	if !tableExists(t, s, "dinners") {
 		t.Error("table from 0001_init was not created")
@@ -924,6 +924,95 @@ func TestMigrateShoppingListChatSendUnconfirmed(t *testing.T) {
 	}
 }
 
+// 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")
+			}
+		})
+	}
+}
+
 // TestMigrateSiteNameFromPage applies 0020 over recipes and idea job
 // results stored when a page naming no site was stored under its host: a
 // stored name that could be a host — one with no space that is dotted, an

internal/store/schema_test.go

@@ -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 {