fix(ui): Numeric ID in URL of the wp link when opened from search results (WP #74834)#23342
Open
thykel wants to merge 6 commits into
Open
fix(ui): Numeric ID in URL of the wp link when opened from search results (WP #74834)#23342thykel wants to merge 6 commits into
thykel wants to merge 6 commits into
Conversation
The autocomplete dropdown's visible result link and its click handler navigated to the numeric work package id, so opening a suggestion landed on /work_packages/<id> while keyboard selection already used the semantic identifier. Route both through displayId so the link target matches the identifier shown in the result row. Rework the followItem unit spec to drive the method with plain typed stubs instead of jasmine.createSpy, and have the wpPath stub return a fragment so the window.location assignment does not navigate the runner.
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.
akabiru
force-pushed
the
fix/74834-numeric-id-in-url-of-the-wp-link-when-op
branch
from
d8a7286 to
14ef6f9
Compare
Build the work package fixture from WorkPackageResource with a HAL $source instead of stubbing the displayId getter, so followItem is verified through the production getter. Adds a classic-mode case proving the numeric fallback drives the link, and an instanceof assertion confirming the fixture takes the HAL branch.
Member
|
Hey @thykel , want to have a look? Can't request you directly as you're the author :) |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes work package search links so semantic work package identifiers are used in URLs instead of numeric database IDs when semantic IDs are enabled.
Changes:
- Uses
display_id/displayIdfor backend and frontend work package link generation. - Adds backend feature/model coverage for semantic work package URLs in search/event paths.
- Adds frontend unit coverage for keyboard selection in global search.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
app/models/work_package/journalized.rb |
Uses display_id for work package event URLs. |
spec/models/work_package/work_package_acts_as_event_spec.rb |
Adds classic and semantic mode expectations for event_url. |
spec/features/search/search_spec.rb |
Verifies search result links use semantic identifiers. |
frontend/src/app/core/global_search/input/global-search-input.component.ts |
Uses displayId when following a selected work package. |
frontend/src/app/core/global_search/input/global-search-input.component.html |
Uses displayId for visible global search result links and click handling. |
frontend/src/app/core/global_search/input/global-search-input.component.spec.ts |
Adds unit tests for followItem URL behavior. |
Deploying openproject with ⚡ PullPreview
|
The header comment claimed search did not rely on acts_as_event; the server-rendered search results page builds its work package links through WorkPackage#event_url, so the note now reflects that search and atom feeds both depend on it while the Activities subsystem uses its own providers. Type the followItem spec helper from the method signature instead of unknown, so the test states the argument contract explicitly.
followItem navigates only work package results, and displayId is defined on WorkPackageResource rather than the HalResource base. Guarding on the concrete type guarantees the displayId access at runtime instead of leaning on union narrowing to reach it.
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
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/74834
What are you trying to accomplish?
On instances with semantic work package IDs enabled, opening a work package from global search navigated to a URL built from the numeric database ID instead of the semantic identifier — e.g.
/work_packages/21456rather than/work_packages/SGS-349.The numeric ID leaked in through two independent link-generation paths:
id, both on the visible result link and when a suggestion was opened directly with the keyboard./searchbuild their link through the activity-event URL definition, which passed the numeric primary key. The activity feed itself already resolves the identifier correctly; the results page is a separate path that did not.Screenshots
Before — numeric ID in the search result link
After — semantic identifier in the search result link
What approach did you choose and why?
The identifier a user should see in a URL already has a single source of truth:
WorkPackage#display_idreturns the semantic identifier in semantic mode and the numeric ID in classic mode, andWorkPackage#to_paramalready routes the standard Rails URL helpers through it. The fix makes the two remaining stragglers follow the same convention.On the frontend, the global search component routes both the visible dropdown link and the keyboard-open path through
displayId, which the API already serializes — so the link target matches the identifier shown in the result row. Inside theHalResourcebranch the value is aWorkPackageResourceanddisplayIdis a non-null string, which also removes a non-null assertion the numericidrequired.On the backend, the activity-event URL definition passes
display_idinstead of the primary key. In classic modedisplay_idis the numeric ID, so behaviour there is unchanged.Normalising numeric-ID bookmarks (and other redirect entry points) to semantic URLs is a broader concern that affects API consumers and external integrations, so it is left out of scope for its own change.
Merge checklist