ops(plans): migration to clean up universal plans (closes #742)

internal/database/postgres/migrations/000057_cleanup_universal_plans.down.sql

@@ -0,0 +1,13 @@
+-- 000057 down: intentional no-op.
+--
+-- The up migration deletes purchase_plans rows that had no plan_accounts
+-- entry (universal plans).  Deleted rows cannot be recovered by a SQL
+-- migration because the data is gone.  To restore a deleted plan, operators
+-- must replay from a database backup taken before the up migration ran.
+--
+-- Rolling back the migration version without restoring the data would leave
+-- the schema_migrations table claiming version 56 while the deleted rows are
+-- still absent -- the rollback cannot make the DB consistent with pre-57
+-- state anyway.  A no-op .down.sql makes this explicit rather than hiding it
+-- behind a silent empty file.
+SELECT 1; -- no-op: data cannot be reconstructed from SQL alone

internal/database/postgres/migrations/000057_cleanup_universal_plans.up.sql

@@ -0,0 +1,40 @@
+-- 000057: delete purchase_plans rows that have no plan_accounts entry.
+--
+-- Background: before PR "fix(plans): eliminate universal plans" (closes #743),
+-- the API accepted a POST /api/plans body with an empty target_accounts list
+-- and wrote a purchase_plans row with no corresponding plan_accounts rows.
+-- Those "universal plans" were never correctly scoped to any account;
+-- migration 000057 removes them permanently.
+--
+-- Cleanup semantic: DELETE.
+-- The three options discussed in issue #742 are (a) delete, (b) fan-out to
+-- all matching accounts, and (c) manual review per plan.  A SQL migration
+-- can only act uniformly across all rows; fan-out (b) would silently attach
+-- every account to plans the operator may never have intended to run, and
+-- manual review (c) cannot be encoded in a migration.  Deletion is the only
+-- safe default: the rows predate account-scoping enforcement, have no
+-- explicit account assignment, and leaving them in place could trigger
+-- unintended purchases once the scheduler resumes.
+--
+-- Referential safety:
+--   purchase_executions.plan_id  FK is ON DELETE SET NULL (migration 000033).
+--   purchase_history.plan_id     FK is ON DELETE SET NULL (initial schema).
+--   plan_accounts.plan_id        FK is ON DELETE CASCADE  (migration 000011).
+-- Deleting from purchase_plans therefore nullifies the plan_id in any
+-- linked execution/history rows and removes the (already-absent) plan_accounts
+-- rows -- no orphaned child rows remain.
+--
+-- Idempotency: re-running after all universal plans are already gone is a
+-- no-op (zero rows match the WHERE NOT EXISTS predicate).
+--
+-- Down migration note: the .down.sql is intentionally a no-op.  Deleted rows
+-- cannot be restored by a migration because the original data is gone.
+-- Operators who need to recover a specific plan must restore from a backup
+-- taken before this migration ran.
+
+DELETE FROM purchase_plans
+WHERE NOT EXISTS (
+    SELECT 1
+    FROM plan_accounts pa
+    WHERE pa.plan_id = purchase_plans.id
+);

internal/database/postgres/migrations/000057_cleanup_universal_plans_test.go

@@ -0,0 +1,160 @@
+//go:build integration
+// +build integration
+
+package migrations_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/LeanerCloud/CUDly/internal/database/postgres/migrations"
+	"github.com/LeanerCloud/CUDly/internal/database/postgres/testhelpers"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestMigration_CleanupUniversalPlans asserts that migration 000057 deletes
+// every purchase_plans row that has no plan_accounts entry (universal plan)
+// while leaving correctly scoped plans intact.
+//
+// Fixture layout:
+//
+//	planUniversal1, planUniversal2  -- no plan_accounts rows (should be deleted)
+//	planScoped                      -- has one plan_accounts row (must survive)
+//
+// The test also verifies that:
+//   - purchase_executions rows linked to a deleted universal plan have their
+//     plan_id NULLed (ON DELETE SET NULL, migration 000033).
+//   - purchase_history rows linked to a deleted universal plan have their
+//     plan_id NULLed (ON DELETE SET NULL, initial schema).
+//   - Re-running the migration (idempotency) is a no-op when no universal
+//     plans remain.
+//
+// The down migration is a no-op by design (deleted rows cannot be recovered
+// from SQL alone), so no rollback assertion is made.
+func TestMigration_CleanupUniversalPlans(t *testing.T) {
+	ctx := context.Background()
+	migrationsPath := getMigrationsPath()
+
+	container, err := testhelpers.SetupPostgresContainer(ctx, t)
+	require.NoError(t, err)
+	defer container.Cleanup(ctx)
+	pool := container.DB.Pool()
+
+	// Run all migrations to head (includes 000057), then roll back one so we
+	// sit at 000056, seed fixtures, and re-run to apply 000057 in isolation.
+	require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", ""))
+	require.NoError(t, migrations.RollbackMigrations(ctx, pool, migrationsPath, 1))
+
+	// Seed: two cloud accounts (needed for plan_accounts FK).
+	_, err = pool.Exec(ctx, `
+		INSERT INTO cloud_accounts (id, name, provider, external_id)
+		VALUES
+		  ('aaaaaaaa-aaaa-aaaa-aaaa-000000000001', 'acct-1', 'aws', '111111111111'),
+		  ('aaaaaaaa-aaaa-aaaa-aaaa-000000000002', 'acct-2', 'aws', '222222222222')
+	`)
+	require.NoError(t, err)
+
+	// Two universal plans (no plan_accounts rows).
+	_, err = pool.Exec(ctx, `
+		INSERT INTO purchase_plans (id, name, services)
+		VALUES
+		  ('11111111-1111-1111-1111-000000000001', 'universal-plan-1', '{"aws:rds": {}}'::jsonb),
+		  ('11111111-1111-1111-1111-000000000002', 'universal-plan-2', '{"aws:ec2": {}}'::jsonb)
+	`)
+	require.NoError(t, err)
+
+	// One correctly scoped plan (has a plan_accounts entry).
+	_, err = pool.Exec(ctx, `
+		INSERT INTO purchase_plans (id, name, services)
+		VALUES ('22222222-2222-2222-2222-000000000001', 'scoped-plan', '{"aws:rds": {}}'::jsonb)
+	`)
+	require.NoError(t, err)
+
+	_, err = pool.Exec(ctx, `
+		INSERT INTO plan_accounts (plan_id, account_id)
+		VALUES ('22222222-2222-2222-2222-000000000001', 'aaaaaaaa-aaaa-aaaa-aaaa-000000000001')
+	`)
+	require.NoError(t, err)
+
+	// Execution linked to universal-plan-1: plan_id must be NULLed after delete.
+	_, err = pool.Exec(ctx, `
+		INSERT INTO purchase_executions (id, plan_id, execution_id, status, step_number, scheduled_date)
+		VALUES (
+		  'eeeeeeee-eeee-eeee-eeee-000000000001',
+		  '11111111-1111-1111-1111-000000000001',
+		  'eeeeeeee-eeee-eeee-eeee-000000000002',
+		  'completed', 1, NOW()
+		)
+	`)
+	require.NoError(t, err)
+
+	// History row linked to universal-plan-1: plan_id must be NULLed after delete.
+	_, err = pool.Exec(ctx, `
+		INSERT INTO purchase_history
+		    (id, account_id, purchase_id, timestamp, provider, service, region,
+		     resource_type, term, payment, plan_id, plan_name)
+		VALUES (
+		  'hhhhhhhh-hhhh-hhhh-hhhh-000000000001',
+		  '111111111111',
+		  'ri-abc123',
+		  NOW(), 'aws', 'rds', 'us-east-1', 'm5.large', 36, 'all-upfront',
+		  '11111111-1111-1111-1111-000000000001',
+		  'universal-plan-1'
+		)
+	`)
+	require.NoError(t, err)
+
+	// Apply migration 000057.
+	require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", ""))
+
+	// Universal plans must be gone.
+	var universalCount int
+	err = pool.QueryRow(ctx, `
+		SELECT COUNT(*) FROM purchase_plans
+		WHERE id IN (
+		  '11111111-1111-1111-1111-000000000001',
+		  '11111111-1111-1111-1111-000000000002'
+		)
+	`).Scan(&universalCount)
+	require.NoError(t, err)
+	assert.Equal(t, 0, universalCount, "both universal plans must be deleted by migration 000057")
+
+	// Scoped plan must survive.
+	var scopedCount int
+	err = pool.QueryRow(ctx, `
+		SELECT COUNT(*) FROM purchase_plans
+		WHERE id = '22222222-2222-2222-2222-000000000001'
+	`).Scan(&scopedCount)
+	require.NoError(t, err)
+	assert.Equal(t, 1, scopedCount, "scoped plan (with plan_accounts) must survive migration 000057")
+
+	// Execution linked to deleted universal plan must have plan_id NULLed.
+	var execPlanID *string
+	err = pool.QueryRow(ctx, `
+		SELECT plan_id::TEXT FROM purchase_executions
+		WHERE id = 'eeeeeeee-eeee-eeee-eeee-000000000001'
+	`).Scan(&execPlanID)
+	require.NoError(t, err)
+	assert.Nil(t, execPlanID, "execution.plan_id must be NULLed when universal plan is deleted (ON DELETE SET NULL)")
+
+	// History row linked to deleted universal plan must have plan_id NULLed.
+	var histPlanID *string
+	err = pool.QueryRow(ctx, `
+		SELECT plan_id::TEXT FROM purchase_history
+		WHERE id = 'hhhhhhhh-hhhh-hhhh-hhhh-000000000001'
+	`).Scan(&histPlanID)
+	require.NoError(t, err)
+	assert.Nil(t, histPlanID, "history.plan_id must be NULLed when universal plan is deleted (ON DELETE SET NULL)")
+
+	// Idempotency: re-running the migration path must be a no-op.
+	require.NoError(t, migrations.RollbackMigrations(ctx, pool, migrationsPath, 1))
+	// Re-seed only the scoped plan to simulate a clean DB at version 56.
+	// The universal plans are intentionally absent (already cleaned up).
+	require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", ""))
+
+	var postIdempotentCount int
+	err = pool.QueryRow(ctx, `SELECT COUNT(*) FROM purchase_plans`).Scan(&postIdempotentCount)
+	require.NoError(t, err)
+	assert.Equal(t, 1, postIdempotentCount, "idempotent re-run must leave the one scoped plan intact")
+}