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

[cristim]

Summary

• Shape chosen: A (SQL migration) -- one-shot data cleanup is exactly what migrations are for; a runtime sweep would add code that lives forever for a problem that existed once.
• Migration number: 000057 -- the highest existing migration on origin/feat/multicloud-web-frontend was 000056; no other open PRs carry a migration, so 000057 is uncontested.
• Cleanup semantic: DELETE. The three options from issue ops(plans): clean up existing universal plans (DB rows with no plan_accounts entry) #742 are (a) delete, (b) fan-out to all matching accounts, (c) manual review. A migration can only act uniformly; fan-out silently attaches every account to plans the operator may never have intended to run, and manual review cannot be encoded in SQL. Deletion is the safe default for rows that predate account-scoping enforcement and have no explicit account assignment.

Migration shape

DELETE FROM purchase_plans
WHERE NOT EXISTS (
    SELECT 1 FROM plan_accounts pa WHERE pa.plan_id = purchase_plans.id
);

Referential safety: purchase_executions.plan_id is ON DELETE SET NULL (migration 000033); purchase_history.plan_id is ON DELETE SET NULL (initial schema); plan_accounts.plan_id is ON DELETE CASCADE (migration 000011). No orphaned child rows are created.

Down migration

The .down.sql is an intentional no-op (SELECT 1). Deleted rows cannot be recovered from SQL alone; operators must restore from a backup taken before migration 000057 ran. The file documents this explicitly rather than being silently empty.

Test scope

TestMigration_CleanupUniversalPlans (integration tag) covers:

• 2 universal plans (no plan_accounts rows) -- asserted deleted after migration
• 1 scoped plan (has a plan_accounts row) -- asserted to survive
• FK SET NULL on child rows -- execution and history rows linked to a deleted plan have plan_id NULLed
• Idempotency -- re-running when no universal plans exist is a no-op; scoped plan count stays at 1

Manual verification after merge

-- Should return zero rows once migration runs on the live DB:
SELECT pp.id, pp.name, pp.created_at
FROM purchase_plans pp
LEFT JOIN plan_accounts pa ON pa.plan_id = pp.id
WHERE pa.plan_id IS NULL;

Summary by CodeRabbit

• Chores
  • Added database migration to remove orphaned purchase plan records that lack associated plan accounts
  • Migration properly handles all related record references during cleanup
  • Implemented integration tests validating migration behavior across multiple scenarios, including rollback safety and idempotency verification

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces database migration 000057 that permanently deletes orphaned purchase_plans rows lacking plan_accounts entries. The up migration includes documented assumptions about referential integrity, while the down migration is intentionally a no-op. An integration test validates deletion, referential nulling behavior, and re-run idempotency.

Changes

Universal Plans Cleanup Migration

Layer / File(s)	Summary
Migration implementation internal/database/postgres/migrations/000057_cleanup_universal_plans.up.sql, internal/database/postgres/migrations/000057_cleanup_universal_plans.down.sql	Up migration deletes purchase_plans rows without matching plan_accounts, documents referential behavior expectations (dependent purchase_executions and purchase_history use ON DELETE SET NULL), and confirms deletion is idempotent. Down migration is a documented no-op acknowledging deleted data cannot be restored.
Integration test with fixtures and validation internal/database/postgres/migrations/000057_cleanup_universal_plans_test.go	Test boots Postgres container, establishes pre-migration baseline with two universal and one scoped purchase_plans, plus purchase_executions and purchase_history rows referencing a universal plan. After executing migration 000057, asserts both universal plans are deleted, the scoped plan remains, and dependent rows have plan_id set to NULL. Finally verifies idempotency by re-running migrations from the same baseline.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A cleanup hop through the database lands,
Orphaned plans swept away by careful hands,
Down migrations just rest and smile,
Tests confirm it's idempotent all the while!
✨ Clean data, no mess, mile after mile.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: a migration to clean up universal plans, with a direct reference to the closed issue.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/742-wave3

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[cristim]

@coderabbitai review

[coderabbitai]

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 38 minutes and 41 seconds before sending another message.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@internal/database/postgres/migrations/000057_cleanup_universal_plans_test.go`:
- Around line 152-153: Update the misleading comment in
000057_cleanup_universal_plans_test.go: replace or remove the line claiming
"Re-seed only the scoped plan to simulate a clean DB at version 56" since no
re-seeding occurs; instead state that after the rollback the scoped plan remains
(rollback changes schema version, not data) and that the idempotency test
verifies running migration 000057 is a no-op when universal plans are absent.
Reference the rollback call and the migration ID 000057 in the revised comment
so future maintainers understand intent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 29745840-beac-4b0c-8cf0-93dd073d0320

📥 Commits

Reviewing files that changed from the base of the PR and between 4956d66 and e08d7e4.

📒 Files selected for processing (3)

• internal/database/postgres/migrations/000057_cleanup_universal_plans.down.sql
• internal/database/postgres/migrations/000057_cleanup_universal_plans.up.sql
• internal/database/postgres/migrations/000057_cleanup_universal_plans_test.go

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify or remove misleading comment.

The comment states "Re-seed only the scoped plan to simulate a clean DB at version 56", but no re-seeding occurs. After the rollback on line 151, the scoped plan is already present in the database (rollback changes the schema version, not data). The idempotency test works correctly—it verifies that re-running migration 000057 when no universal plans exist is a no-op—but the comment may confuse future maintainers.

📝 Suggested fix to clarify intent

-	// Re-seed only the scoped plan to simulate a clean DB at version 56.
-	// The universal plans are intentionally absent (already cleaned up).
+	// Data remains unchanged: scoped plan still present, universal plans already deleted.
+	// Re-running 000057 should be a no-op (no universal plans to delete).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Re-seed only the scoped plan to simulate a clean DB at version 56.
	// The universal plans are intentionally absent (already cleaned up).
	// Data remains unchanged: scoped plan still present, universal plans already deleted.
	// Re-running 000057 should be a no-op (no universal plans to delete).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/database/postgres/migrations/000057_cleanup_universal_plans_test.go`
around lines 152 - 153, Update the misleading comment in
000057_cleanup_universal_plans_test.go: replace or remove the line claiming
"Re-seed only the scoped plan to simulate a clean DB at version 56" since no
re-seeding occurs; instead state that after the rollback the scoped plan remains
(rollback changes schema version, not data) and that the idempotency test
verifies running migration 000057 is a no-op when universal plans are absent.
Reference the rollback call and the migration ID 000057 in the revised comment
so future maintainers understand intent.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.