Work package results on the search page build their link through the
acts_as_event url proc, which passed the numeric primary key instead of
the work package's display id. In semantic mode this rendered
/work_packages/<id> even though the row showed the semantic identifier,
unlike Rails URL helpers that already resolve the object via to_param.
Pass display_id so the link follows the same convention everywhere.
The `mentioned` and `watcher_changed` text-mailer bodies surfaced raw
journal markdown — numeric `#42` references stayed numeric in semantic
mode, and `<mention>` envelopes leaked as HTML source.
Introduces `:plain_text` as a sibling format inside the existing Plain
module. The filter chain mirrors the markdown pipeline (markdown,
sanitization, mention, pattern-matcher) and finishes with a new
`PlainTextOutputFilter` that collapses the DOM to text. The
`WorkPackages` link handler and `MentionFilter` get plain-text branches
keyed off `context[:plain_text]` so identifier resolution stays in one
place across rich and plain channels.
Closes https://community.openproject.org/wp/74762
Splat with a depth-1 flatten lets callers pass scalars, varargs, or a
pre-built array interchangeably. `Array(values)` would have been
misleading here — `Array([[1, "a"], 2])` leaves the inner array
intact and `map(&:to_s)` stringifies it as `"[1, \"a\"]"`, which
then misclassifies through the semantic-id branch. Bounded `flatten(1)`
absorbs the (scalar) and ([scalar, scalar]) shapes that production
code already uses while leaving deeper nesting alone, so the same
pathology produces no match rather than silently working.
Refs https://github.com/opf/openproject/pull/23202#discussion_r3235337043
The route constraint composes the numeric and semantic shapes from
SEMANTIC_ID_PATTERN, which itself composes the project-identifier shape
from `Projects::Identifier::SEMANTIC_FORMAT`. A future change to either
upstream pattern would shift what URLs Rails accepts without touching
routes.rb. Three anchored assertions lock the boundary in place — the
constant is used only as a route constraint, so anchored matching mirrors
the actual call site.
`semantic_id?` already strips surrounding whitespace before its round-trip
check, so `" 123 "` was classified as not-semantic. `numeric_id?` did
not strip, so the same value was also not-numeric — leaving it
"neither" and unreachable through normal routing.
Stripping in `numeric_id?` and expressing the String branch as
`!semantic_id?(value)` removes the duplicated round-trip logic and gives
the routing predicates a single source of truth: any String classified
not-semantic is necessarily numeric. Integer/nil/other types are
unaffected — they fall on their own branches.
Routing-table spec gains a `" 123 "` row to lock the symmetry in.
The `value == value.to_i.to_s` round-trip check that filters leading-
zero ID forms ("0123") was duplicated across the WP link handler, the
PDF export macro, and the cost-query filter.
A new `WorkPackage::SemanticIdentifier.numeric_id?(value)` predicate
captures the canonical-numeric check at one site. It pairs with
`semantic_id?` as the WP-finder shape gate; the two answer different
questions (shape vs routing) and so are kept independent rather than
expressed as one another's negation.
The cost-query filter switches to the predicate in this slice; the
text-formatting and PDF callers convert in a follow-up.
https://community.openproject.org/wp/74366
And also improve rendering of linked work packages for this.
Before, the macros were not expanded, but only the hashes and
ID output directly.
The plural counterpart to find_by_display_id resolves a mix of
numeric and semantic identifiers in a single composite scope so
callers (notably ApplicationController#find_work_packages, used by
bulk WP routes) do not have to branch on the input shape or partition
into numeric/semantic themselves. Mirrors scope_for_semantic_identifier
in using `.or` to union the primary-key, current-identifier, and
historical-alias arms; unknown values resolve to no match rather
than poisoning the rest of the set.
Rails URL helpers — `work_package_path(@wp)`, `polymorphic_path(@wp)`,
`form_for(@wp)` — now produce `/work_packages/PROJ-7` automatically in
semantic mode, without callers having to thread `wp.display_id` through
every path-helper invocation.
In classic mode `display_id` returns the integer primary key, so this is
behaviourally identical to the inherited `id&.to_s`. The override gates
transitively on the existing `semantic_work_package_ids` flag via
`display_id`'s mode check — no new flag is introduced.
API v3 deliberately bypasses this by passing `id:` kwargs explicitly in
the representer, so HAL self-links remain numeric and stable for API
consumers.
Adopts PR 22788's exact method body, comment, and spec text for
WorkPackage#formatted_id so the two PRs produce byte-identical diffs
on the overlapping hunks. Git's 3-way merge auto-resolves when either
PR lands first — no rebase conflict either way.
Functionally equivalent to the previous implementation; the is_a?(String)
guard is a no-op since display_id never returns anything exotic in
practice, but it keeps the two branches textually in sync.
The reporting filter widget pre-populates opce-autocompleter with
plain hashes ({id:, name:}). When the autocompleter template switched
from `#{item.id}` to `{{ item.formattedId }}`, those hashes had no
formattedId key and the ID prefix disappeared from the selected-value
label, breaking work_package_costlog_spec and time_entry_activity_spec.
Extend Widget::Filters::WorkPackage#map_filter_values to emit
formattedId, displayId, and subject alongside id/name so the
autocompleter template has the data it needs.
Centralise the formatting logic in a new WorkPackage#formatted_id
model method, mirroring the frontend formatWorkPackageId helper:
semantic identifiers pass through (PROJ-42); numeric ids are
prefixed with # (#42).
find_by already raises WorkPackage::SemanticIdentifier::UnsupportedLookup
when a semantic identifier is passed through id:/identifier:. Align the
multi-argument find guard so callers can rescue a single specific error
across both entry points, while the ArgumentError parent class keeps
legacy rescuers working.
Document that find_by(subject: x, id: "PROJ-1") raises
UnsupportedLookup — the args.length == 1 check in the guard
distinguishes hash-conditions form from the SQL-string form, not
hash arity.
The shared scope for semantic identifier lookups needs to compose
cleanly with the visible scope (uses join+allowed_to) for both
current identifiers and historical aliases. Pin that behavior so
structural regressions in the OR construction surface early.
find_by(id: [...]) is a legitimate AR pattern for matching the first
record in a set. Previously we only guarded scalar values, so
find_by(id: ["PROJ-1", 1]) silently fell through to a SQL
WHERE id IN ('PROJ-1', 1) that would error or coerce unexpectedly
on Postgres.
Extend the guard to scan array elements and raise UnsupportedLookup
on the first semantic one found, keeping the error message focused
on the offending value.
Use a dedicated error class so callers that need to rescue this
specifically can do so without catching unrelated ArgumentErrors.
It still inherits from ArgumentError, so existing rescue clauses
continue to work.
The convention is that low-level code (queries, filters, services)
uses primary keys and only frontend-facing code uses display-id
resolution. The previous message only pointed at find_by_display_id,
which encouraged the wrong tool for batch lookups.
Also mention both the raising and nil-returning variants so callers
can pick the right semantics.
Internal callers like FilterForWpMixin#value_objects pass arrays of
numeric IDs to find. Blanket-rejecting array/multi-arg calls broke
those paths.
Fall through to standard AR find for all-numeric arrays and multi-arg
calls, but continue raising ArgumentError if any argument is a
semantic identifier — batch resolution across the alias table would
be ambiguous and is out of scope.
WorkPackage.find(['DP-1']) returned a single record instead of an
array, breaking Rails' convention that array-in means array-out.
Rather than fixing that edge case, simplify by disallowing all
multi-argument and array-argument find calls — nobody uses them
and the complexity isn't worth maintaining.
Also extract the hash.assoc chain in reject_semantic_id_in_find_by!
into an explicit pair variable for clearer two-step logic.
find_by(id:) and find_by!(id:) now raise ArgumentError when passed
a semantic identifier string, directing developers to use find() or
the new find_by_display_id() method instead. This avoids silently
altering ActiveRecord's find_by semantics and ensures misuse is
caught in development even when semantic mode is not enabled locally.
Renames find_by_id_or_identifier to find_by_display_id (public API)
and migrates all app callers that receive user-facing strings to use
the new method.
In semantic mode, work packages created before the feature was enabled
have a nil identifier column. Previously display_id returned nil for
these, causing the Ruby representer to serialize displayId as null
while the SQL representer used COALESCE to return the numeric id.
Use identifier.presence to fall back to the numeric id, aligning both
representer paths.
Rather than implementing full multi-argument semantic find support
(which adds significant complexity for a use case with no current
callers), raise ArgumentError with a clear message when semantic
identifiers are passed in multi-arg find calls.
This makes the limitation explicit and discoverable. Full multi-arg
support is being evaluated separately.
Add find_by and find_by! overrides to FinderMethods so that
find_by(id: "PROJ-42") transparently resolves semantic identifiers,
matching the existing behavior of find and exists?.
Only intercepts calls where id: is the sole keyword — all other
find_by usage (e.g. find_by(subject: ...)) passes through unchanged.
Revert ShowComponent, SplitViewComponent, and WorkPackagesController
from the explicit find_by_id_or_identifier back to standard
find_by(id:) now that the override handles semantic resolution.
Make find_by_id_or_identifier and find_by_id_or_identifier! private
since they are now internal implementation details with no external
callers. Move their test coverage into the find_by/find_by! specs.
Replace FriendlyId's Object#friendly_id? monkey-patch with a private
semantic_id? method that owns the full dispatch decision: type check,
whitespace stripping, and numeric detection in one place.
This fixes a bug where numeric strings with whitespace (e.g. " 456 "
from comma-split input) were misclassified as semantic identifiers
because " 456".to_i.to_s != " 456".
Extract FinderMethods module that transparently resolves both numeric and
semantic identifiers (e.g. "PROJ-42") using FriendlyId's Object#friendly_id?
for dispatch. The module is included in both the WorkPackage class and
extended onto every relation, so scoped queries like
WorkPackage.visible(user).find("PROJ-42") work seamlessly.
- Override find to resolve semantic IDs via identifier column + alias table
- Override exists? with the same resolution chain
- Refactor find_by_id_or_identifier to use friendly_id? instead of semantic_id?
- Update API route to accept string IDs (type: Integer → type: String)
- Update controller and ViewComponent finders to use find_by_id_or_identifier
- Pass display_id from Rails views to Angular custom elements
Replace the conditional `semanticId` API field with `displayId` which is
always present in work package responses. In semantic mode it returns the
project-based identifier (e.g. "PROJ-42"), in classic mode it returns the
numeric ID as a string. This gives API consumers (frontend, mobile) a
single field to read without conditional logic.
- Add `WorkPackage#display_id` method that encapsulates the mode check
- Update both representers (JSON and SQL) to render `displayId` unconditionally
- Update OpenAPI schema documentation
The API v3 was allowing reading/writing internal comments when both the
enterprise token doesn't allow it and when the project setting is disabled.
Reading (GET):
- Added EnterpriseToken check to internal_visible scope
Writing (POST):
- Added project.enabled_internal_comments check to CreateNoteContract
Tomas Hykel