Use displayId in stateful navigation and fix semantic ID state management#22739
Merged
akabiru merged 7 commits intoimplementation/73797-use-displayid-in-work-package-urlsfrom Apr 16, 2026
Conversation
6bc42ae to
370ad29
Compare
6aa7c34 to
1801afd
Compare
370ad29 to
686a5c7
Compare
1801afd to
e37e60f
Compare
686a5c7 to
58a6421
Compare
e37e60f to
6b119b4
Compare
Routes already accept semantic identifiers (e.g. PROJ-7) via WP_ID_URL_PATTERN, but all frontend-generated links still used numeric PKs. This wires displayId into every navigation path so the browser URL bar shows the semantic form when available. Key design decision: data-work-package-id attributes on <a> elements stay numeric — the selection/hover system is keyed by PK. Only the href gets the semantic ID via a new optional routingId parameter on UiStateLinkBuilder. Changes span table links (ID column, linked WP fields, details action), navigation handlers (list view, embedded tables, boards, BCF, calendar), breadcrumbs, tabs, hierarchy, single view, context menu, quickinfo macro, and post-creation redirect. API-only paths (time entries, share, hover cards, progress modal) are deliberately left with numeric IDs — they never appear in the address bar.
Replaces 5 identical private resolveRoutingId methods with a shared utility function that accepts States as a parameter.
Defer focus and selection to init() (called after WP loads) so we use this.workPackage.id (always numeric) instead of the route param which may be a semantic identifier like "PROJ-7".
…displayId
The route param may be a semantic identifier ("PROJ-7") which won't
match workPackage.id (numeric "42"). Comparing against displayId
as well handles both classic and semantic modes.
Resolve $state.params.workPackageId to numeric PK via the States cache before comparing against the deleted IDs array.
Normalize this.workPackageId from semantic (e.g. "PROJ-7") to numeric PK after first WP load, ensuring downstream cache lookups use the canonical key.
Document the dual-purpose contract in observeWorkPackage (cache normalization), deferred focus in split view init, card highlight comparison logic, and best-effort fallback in resolveRoutingId.
58a6421 to
1388c3c
Compare
3 tasks
1ed3783
into
implementation/73797-use-displayid-in-work-package-urls
10 of 11 checks passed
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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/wp/73834
Builds on #22733
What are you trying to accomplish?
Wire displayId into all
$state.go()callers and Turbo navigation handlers so that the browser URL bar shows semantic identifiers (e.g.PROJ-7) during stateful navigations — split view opening, board card clicks, calendar event clicks, post-creation redirects, and context menu actions.Also fixes four bugs where passing semantic identifiers through Angular state params broke internal systems that assume numeric PKs.
What approach did you choose and why?
PR #22733 was split into two: href-only changes (safe, landed first) and stateful navigation changes (this PR). The split isolates the riskier
$state.go()changes so they get focused review alongside the bug fixes they require.The key insight is that
$state.params.workPackageIdis consumed by many downstream systems (focus/selection, card highlighting, bulk delete detection, API cache) that assume numeric PKs. Semantic IDs should flow into URLs only — internal plumbing must always use numeric IDs.Changes:
Shared utility — Extracted
resolveRoutingId()to replace 5 identical private methods across components. Pure function that takesStatesand returnsdisplayId ?? workPackageId.Navigation handlers — List view, embedded tables, boards, BCF, calendar, create flow, and context menu now use
resolveRoutingId()for URL generation while keeping numeric IDs for state management.Bug fixes:
updateFocus()andsetRowState()received the raw route param (e.g."PROJ-7") but the selection system is keyed by numeric PK ("42"). Deferred these calls toinit()(after WP loads) so they use the resolved numeric ID.workPackage.idonly. When the route param is semantic, this never matches. Now compares against bothidanddisplayId./work_packages/PROJ-7) created a cache entry under"PROJ-7", while list queries cached the same WP under"42". Updates to one wouldn't propagate to the other. Now normalizesthis.workPackageIdto the numeric PK after first load.Merge checklist