Conversation
Contributor
|
Automated pre-review (advisory, not a required check) — verdict: HOLD · CI green HOLD — the code-format rule is incomplete for the v2 code-based routes.
|
## Context No Lago model validated code format, so every code-keyed v2 route carries a greedy `code: /.*/` constraint. A product coded `a/b` collides with nested routes (e.g. `/products/a/filters` resolves to the filters index), making it unreachable on member routes — and other slashed codes shadow-collide with current or future nested segments. The catalog tables are still empty, so this is the only window to add a format rule. ## Description Introduce a `CatalogCodeFormat` concern enforcing a slug-safe code (`/\A(?!\.+\z)[a-zA-Z0-9_\-.]+\z/`: letters, digits, underscore, hyphen, dot; the negative lookahead also rejects an all-dot code like `.` or `..`, which proxies normalize and cannot be addressed). It is included in every code-keyed v2 catalog model — Product, ProductCategory, ProductFilter, RateCard, RateCardRate, CatalogPlan, RatePhase. allow_blank leaves the empty case to the existing presence validation, and the failure is a standard `code` field error (value_is_invalid), so REST and GraphQL surface it on the field the caller sent. No data migration — the tables are empty.
Contributor
|
Automated pre-review (advisory, not a required check) — verdict: PASS · CI green PASS — The shared validation covers all code-owning v2 catalog resources, rejects codes that cannot be addressed safely through member routes, and has focused coverage for every affected model. |
endenis
approved these changes
Sep 15, 2026
| it "rejects a code containing a slash" do | ||
| record = build(factory, code: "a/b") | ||
| record.valid? | ||
| expect(record.errors.where(:code, :invalid)).to be_present |
Contributor
There was a problem hiding this comment.
I think this could be more sensitive and validate any error in code. record.errors[:code] it does validate any error and not only the invalid one.
| module CatalogCodeFormat | ||
| extend ActiveSupport::Concern | ||
|
|
||
| CODE_FORMAT = /\A(?!\.+\z)[a-zA-Z0-9_\-.]+\z/ |
Contributor
There was a problem hiding this comment.
these options with many dots and text are allowed as well?
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
3 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.
Context
No Lago model validates code format, so every code-keyed v2 route carries a greedy
code: /.*/constraint. Verified consequence: a product codedsomething/filtersis unreachable on member routes becauseGET /api/v2/products/something/filtersresolves to the filters index of productsomething; other slashed codes shadow-collide with current or future nested segments. The catalog tables are empty in production until v2 ships — the only window to add a format rule for free.Description
CatalogCodeFormatconcern (validates :code, format: {with: CODE_FORMAT}, allow_blank: true) included in every code-keyed v2 catalog model: Product, ProductCategory, ProductFilter, RateCard, RateCardRate, CatalogPlan, RatePhase.allow_blankleaves the empty case to each model's existing presence validation (no double error).codefield error (value_is_invalid), so REST and GraphQL surface it on the field the caller sent.