Addresses Copilot review on PR 22982. The previous spec covered
historical reservation in the two mode contexts but lost the case
where another project's FriendlyId slug history blocks a semantic
identifier when the global mode is classic — the exact scenario
that drives the converter service's :semantic_conversion path.
- Validator moves to app/validators/projects/identifier_validator.rb
as Projects::IdentifierValidator. The path-style symbol on the
validates declaration ('projects/identifier') resolves to it.
Distinguishes domain-specific validators from globally reusable
ones (UrlValidator, JsonValidator, etc).
- Spec moves to spec/validators/projects/identifier_validator_spec.rb.
- Trim comments that explained framework conventions (validator
lookup, declaration order). Keep comments only where the WHY is
non-obvious to a Rails-fluent reader.
Top level no longer biases toward classic. Mode-agnostic behaviors
(blank short-circuit, reserved keyword, historical reservation) move
into shared examples that both 'in classic mode' and 'in semantic
mode' contexts include — so each branch of the validator's mode
dispatch exercises them. Format-specific tests stay inside their
respective mode context.
The :semantic_conversion validation context block stays its own
describe — its purpose is testing the override-from-classic, so
having a global classic setting there is the point, not a default.
Coverage grows from 31 to 48 examples; the additional 17 are
shared-example runs that previously executed only under classic
settings.
Move five private validation methods out of the Projects::Identifier
concern into a top-level ActiveModel::EachValidator. The concern goes
from carrying ~50 lines of validation logic to declaring:
validates :identifier, project_identifier: true,
if: :identifier_changed?
Format rules, reserved-keyword check, and historical-reservation check
all live in ProjectIdentifierValidator. Mode dispatch (classic vs
semantic vs :semantic_conversion context) is internal to the validator
and easy to swap for a registry if a third format ever appears.
Picks up the follow-up suggested in #22931 (comment 4326208630).
Notes:
- Validator is top-level (ProjectIdentifierValidator) not namespaced,
matching the existing convention of UrlValidator / JsonValidator /
SecureContextUriValidator and so Rails' validator lookup for
`validates :identifier, project_identifier: true` resolves directly.
- Format constants (RESERVED_IDENTIFIERS, *_MAX_LENGTH) stay on the
concern since they're shared with acts_as_url's blacklist/limit, the
routing constraint, and suggesters — moving them to the validator
would couple unrelated subsystems to a validator namespace.
- Validator declaration order in the concern is significant: presence
+ uniqueness must run before `project_identifier: true` so the
historical-reservation check can short-circuit when uniqueness has
already flagged :taken.
Pure-validator behaviour (format checks, reserved keyword,
:semantic_conversion context, historical reservation) moves to
spec/validators/project_identifier_validator_spec.rb. Integration
tests (FriendlyId :history wiring, :saving_custom_fields,
.suggest_identifier, .identifier_slugs scopes) stay in the model spec.
Kabiru Mwenja