Make find/exists? resolve semantic work package identifiers#22718
Open
Make find/exists? resolve semantic work package identifiers#22718
Conversation
3 tasks
akabiru
force-pushed
the
feature/73735-expose-project-based-semantic-work-package-identifier-on-the-api
branch
from
2a99fe8 to
2839dc2
Compare
akabiru
force-pushed
the
feature/73756-adapt-routes-for-project-based-semantic-work-package-identifiers
branch
from
7c38287 to
a3b5e32
Compare
akabiru
force-pushed
the
feature/73735-expose-project-based-semantic-work-package-identifier-on-the-api
branch
from
2839dc2 to
47c4b8a
Compare
akabiru
force-pushed
the
feature/73756-adapt-routes-for-project-based-semantic-work-package-identifiers
branch
2 times, most recently
from
bb04400 to
c466ac6
Compare
akabiru
force-pushed
the
feature/73735-expose-project-based-semantic-work-package-identifier-on-the-api
branch
from
47c4b8a to
5bbc4e7
Compare
akabiru
force-pushed
the
feature/73756-adapt-routes-for-project-based-semantic-work-package-identifiers
branch
from
128f33b to
be79dc3
Compare
thykel
reviewed
Apr 13, 2026
| @id = id | ||
| @tab = tab | ||
| @work_package = WorkPackage.visible.find_by(id:) | ||
| @work_package = WorkPackage.visible.find_by_id_or_identifier(id) |
Contributor
There was a problem hiding this comment.
Quick one: Don't we already want to call a simple find here? Same for the other occurrences.
Member
Author
There was a problem hiding this comment.
Since we were using find_by(id:) before- I suppose we don't want RecordNotFound errors that would be raised by find?
Base automatically changed from
feature/73735-expose-project-based-semantic-work-package-identifier-on-the-api
to
dev
April 14, 2026 14:17
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 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".
Replace stubbed work package and mock permission chain with real database records and role membership. This eliminates rubocop violations (ExpectInHook, StubbedMock, MessageChain) while also testing the actual finder path through WorkPackage.visible.
The route constraint for work_packages#show used /\d+/ which rejected semantic identifiers like "KSTP-7". Widen to also match the PROJECTID-SEQ pattern so both numeric and semantic IDs route correctly.
Verifies that GET /work_packages/PROJ-42 routes to work_packages#show, matching the widened route constraint.
Centralise the regex that matches both numeric IDs and semantic identifiers into a named constant so route definitions reference a single source of truth instead of duplicating the pattern.
Pass model, primary_key, and id to ActiveRecord::RecordNotFound so error reporters and rescue handlers get the same structured data that Rails' own find provides.
"eeek" now matches the semantic identifier pattern, so find_by_id_or_identifier! raises RecordNotFound with model: "WorkPackage", producing the WP-specific error message instead of the generic 404.
akabiru
force-pushed
the
feature/73756-adapt-routes-for-project-based-semantic-work-package-identifiers
branch
from
73bae1e to
48946fe
Compare
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
2 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.
Ticket
https://community.openproject.org/work_packages/73756
What are you trying to accomplish?
Make
WorkPackage.findand.exists?transparently resolve semantic identifiers (e.g."PROJ-42") in addition to numeric IDs. Also widen Rails route constraints to accept semantic IDs, and update controller/ViewComponent finders to usefind_by_id_or_identifier.This is the second part of the API layer work — PR #22710 introduced the
displayIdfield, and this PR makes the routes and finders work end-to-end with semantic identifiers.What approach did you choose and why?
Extracted a
FinderMethodsmodule inWorkPackage::SemanticIdentifierthat is shared between the class-levelWorkPackagemethods and ActiveRecord relation extensions (viasuper.extending(FinderMethods)). This ensures scoped queries likeWorkPackage.visible(user).find("PROJ-42")work correctly.The dispatch uses a
semantic_id?predicate — it strips whitespace, rejects non-strings and numeric strings, and returns true only for identifiers like"PROJ-42". Numeric lookups fall through to standard ActiveRecordfind.The
exists?override mirrors the same resolution chain: check theidentifiercolumn, then fall back to thework_package_semantic_aliasestable for historical identifiers.Also extracts
ID_ROUTE_CONSTRAINTintoWorkPackage::SemanticIdentifierso the regex for matching work package identifiers in URLs is defined once and referenced from both route definitions inconfig/routes.rb.Merge checklist