Conversation
Contributor
|
Automated pre-review (advisory, not a required check) — verdict: HOLD · CI green HOLD — duplicate catalog-plan codes bypass model validation and raise
|
Contributor
|
Automated pre-review (advisory, not a required check) — verdict: PASS · CI green PASS — The model, migration, live-row uniqueness, soft deletion, organization association, factory, and focused specs are consistent with the existing catalog patterns. Non-blocking: the existing |
brunomiguelpinto
approved these changes
Sep 4, 2026
endenis
approved these changes
Sep 4, 2026
tiagolupepic
reviewed
Sep 4, 2026
PR 0 of the catalog-plans split (Dive-In 6): the v2 catalog gets its own plan table so a product-catalog plan is no longer a `pricing_type` row on the shared `plans` table carrying nil legacy columns. This is additive and zero-risk — nothing is repointed and the table starts empty. Add the `catalog_plans` table and `CatalogPlan` model: the plan envelope only (name, code, invoice_display_name, description, currency), no legacy pricing columns (interval, amount_cents, pay_in_advance) and no self reference (catalog plans do not use plan overrides). Code is unique per organization among kept rows. Soft-deletable, paper-trailed.
## Context A cold review flagged that CatalogPlan guarded code uniqueness only with the database partial index, so a duplicate code raised a raw RecordNotUnique (a 500) rather than a clean validation failure — and the spec locked in that behaviour. ## Description Add a model-level uniqueness validation on code scoped to the organization and to kept rows, matching the sibling plan models. Update the spec to assert the validation error, that the code is free again once the holder is discarded, and that another organization may reuse it, while keeping a database-level guarantee for the concurrency path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
The v2 catalog gets its own plan table, so a product-catalog plan stops being a
pricing_type: product_catalogrow on the sharedplanstable (carrying nilinterval/amount_cents/pay_in_advance).Changes
catalog_planstable — the plan envelope only:name,code,invoice_display_name,description,currency,organization_id, timestamps,deleted_at. No legacy pricing columns, and no self-reference (catalog plans don't use plan parent/child overrides — the dive-in's open question, resolved).CatalogPlanmodel — paper-trailed, soft-deletable (default_scope { kept }), validates name/code presence and currency inclusion.(organization_id, code) WHERE deleted_at IS NULL— code unique per org among kept rows.Not in scope
No
product_category_id(plans link to categories through products, not a column) and nopending_deletionyet — added later if the catalog destroy flow needs it.