Rubocop fixes
1. Layout/HeredocIndentation: Use 2 spaces for indentation in a heredoc.
[Layout/HeredocIndentation]
2. Use 2 spaces for indentation in a heredoc.
[Layout/HeredocIndentation]
The bulk SQL UPDATE persists identifier and sequence_number directly,
but callers holding live records have no cheap way to learn what was
written without reloading. Returning the {wp_id => "PROJ-N"} mapping
lets callers (next commit: WorkPackages::UpdateService) refresh
in-memory state without N round trips. Empty input returns an empty
hash for symmetric Hash-typed callers.
- 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.
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.
Address review feedback: `historical_slugs` was misleading because
FriendlyId's :history module records a row on every save, so the
relation contains current identifiers as well as old ones. The
follow-up `truly_historical` filter read as a tautology, which was
the tell-tale sign of the misnaming.
Renames:
- `historical_slugs` -> `identifier_slugs`
- `HistoricalSlugScopes` -> `IdentifierSlugScopes`
- `truly_historical` -> `historically_reserved`
- `matching(value)` -> `for_identifier(value)`
`for_identifier` is preferred over `reserving` / `with_reservation_of`
because the slug record itself doesn't reserve anything; the slug
value is what blocks reuse, and `for_identifier` plainly names the
case-insensitive lookup without importing a "reservation" concept
that isn't otherwise modelled.
FriendlyId's `:history` module already adds a `project.slugs` association.
Splitting the class-level name to `historical_identifiers` while the
instance-level stays `slugs` creates a vocabulary inconsistency that's
worse than the original "slugs vs identifiers" friction.
The reviewer's concern about newcomers confusing the two is addressed
with an inline comment near the method, not by partially renaming.
The scope module is renamed back from `HistoricalIdentifierScopes` to
`HistoricalSlugScopes` for the same consistency reason. The
`ProblematicIdentifiers#historical_identifiers` instance method
(unrelated, pre-existing) is unchanged.
Replaces the conditional `.then { |q| project ? q.where.not(...) : q }`
in `ClassicIdentifierSuggestionGenerator#taken_identifiers` with a
named scope. The scope handles the nil case so the call site stays
unconditional and reads as the verb it is: "historical identifiers,
excluding this project's own history."
Completes the symmetric trio: upcased_values / downcased_values / raw_values.
Named `raw_values` rather than `values` because the latter collides with
`ActiveRecord::Relation#values` (an internal method Rails depends on).
`slug` is FriendlyId terminology; `identifier` is the project domain
term. Reviewers unfamiliar with FriendlyId may not connect the two.
Renaming at the call site removes that translation step.
Updated four call sites and the corresponding spec.
Add `Projects::Identifier::HistoricalIdentifierScopes`, extended onto the
relation returned by `Project.historical_slugs`. Replace four inlined
SQL fragments with domain-named scopes:
* `truly_historical` — slugs no longer used as any active project's identifier
* `matching(value)` — case-insensitive equality
* `upcased_values` / `downcased_values` — SQL-side case-folded value lists
Call sites updated:
* `Projects::Identifier#identifier_used_by_other_project_in_past?`
* `ProblematicIdentifiers.reserved_identifiers` and `#historical_identifiers`
* `ClassicIdentifierSuggestionGenerator#taken_identifiers`
After this change, `FriendlyId::Slug` is mentioned in exactly one line —
the body of `Project.historical_slugs`.
Note: `#taken_identifiers` shifts from Ruby-side `String#downcase` to
SQL-side `LOWER()`. For ASCII-only project identifiers the two are
equivalent.
The friendly_id_slugs table already contains both current and historical
project identifiers (backfilled by InitializeHistoricIdentifiers migration,
maintained by FriendlyId history module on create/update). Query it
directly via ProblematicIdentifiers.reserved_identifiers instead of
merging Project.pluck with a filtered slug query.
Remove SemanticIdentifierGenerator wrapper class and move collision-aware
logic directly into Projects::Identifier.suggest_identifier. This gives
both the model callback and the frontend suggestion controller automatic
collision detection.
Promote reserved_identifiers to a class method on ProblematicIdentifiers
since it has no instance state dependency.
https://community.openproject.org/work_packages/72809
- Extract the error message to I18n.
- Change the message to have only one option to resolve the issue(Change identifier in Jira).
- In case it is a historic identifier then provide this value in the message.
Documents created with zero-width Unicode characters (e.g. U+200B)
in their titles become unclickable on the index page, making them
hard to manage or delete.
Introduce RemoveInvisibleCharacters normalizer, replacing the former
RemoveAsciiControlCharacters. It strips both ASCII control characters
and Unicode zero-width characters, with each category defined as a
named constant for clarity. Apply it to Document#title and update
existing callers (Project#identifier, CustomField#name).
Add a shared RSpec example "strips invisible characters" to verify
normalization consistently across all three models.
The setting previously used "numeric" and "alphanumeric" as its allowed
values. Rename them to "classic" and "semantic" to better align with the
product terminology for the work package identifier modes.
Includes a migration to update any stored setting values in the database,
updated constants and helper methods on Setting::WorkPackageIdentifier,
and all corresponding references across models, components, forms,
frontend controllers, locales, and specs.
Remove the early return in identifier_alphanumeric_format so users see
all validation errors at once (e.g. "123!" reports both
must_start_with_letter and no_special_characters). Simplify the special
characters regex since the leading-letter check is already separate.
Update API specs to use test identifiers that trigger only one
validation error, matching the new non-short-circuiting behavior.
The numeric (legacy) format validation was inline as a `validates :format`
declaration, while the alphanumeric validator was already a dedicated method.
Extract it into `identifier_numeric_format` so both mode-specific validators
follow the same pattern, and fix a misleading comment on the shared validators.
These were built on the normalizes premise which has been removed.
In alphanumeric mode, SuggestionGenerator handles identifier generation
rather than acts_as_url. The API PR (22417) will handle the proper
wiring between modes.
Per code review: the model should not auto-transform identifier casing
on every read/write. Format validators already enforce correct casing
(lowercase for numeric, uppercase for alphanumeric), and the background
job will handle migrating existing identifiers.
Restores LOWER() in identifier_not_historically_reserved since without
normalizes, slugs and identifiers may differ in case.
LOWER(slug) = LOWER(?) is unnecessary because normalizes :identifier
ensures consistent casing before FriendlyId records the slug. Plain
equality uses the existing composite index on (slug, sluggable_type).
Introduce case-insensitive handling for project identifiers:
- Add `normalizes :identifier` to automatically upcase (alphanumeric
mode) or downcase (numeric mode) identifiers on assignment
- Add `parse_friendly_id` to normalize FriendlyId lookups for
case-insensitive finder queries
- Switch uniqueness validation to `case_sensitive: false`
- Replace inline `exclusion:` validator with explicit
`identifier_not_reserved` that checks case-insensitively
- Consolidate alphanumeric format validators into a single
`identifier_alphanumeric_format` method with early return to
prevent cascading error messages
- Use case-insensitive LOWER() comparison in historical identifier
reservation check
- Add `post_process` support to the OpActiveRecord acts_as_url
adapter with an allowlist of safe transforms (upcase/downcase)
- Add migration to replace the unique index on projects.identifier
with a case-insensitive LOWER(identifier) index
- Update table definition to match the new index
Includes corresponding test additions for normalization, case-
insensitive uniqueness, reserved identifier rejection, and the
create_spec fixture fix for alphanumeric mode.
Kabiru Mwenja