Skip to content

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
implementation/73834-stateful-navigation-changes
Apr 16, 2026
Merged

Use displayId in stateful navigation and fix semantic ID state management#22739
akabiru merged 7 commits intoimplementation/73797-use-displayid-in-work-package-urlsfrom
implementation/73834-stateful-navigation-changes

Conversation

@akabiru
Copy link
Copy Markdown
Member

@akabiru akabiru commented Apr 13, 2026

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.workPackageId is 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 takes States and returns displayId ?? 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:

  • Split view focus/selectionupdateFocus() and setRowState() received the raw route param (e.g. "PROJ-7") but the selection system is keyed by numeric PK ("42"). Deferred these calls to init() (after WP loads) so they use the resolved numeric ID.
  • Card view highlight — The card's selected state compared the route param against workPackage.id only. When the route param is semantic, this never matches. Now compares against both id and displayId.
  • Bulk delete split view close — After bulk deletion, the split view checks whether the displayed WP was among those deleted. The check compared numeric deletion IDs against the (potentially semantic) route param. Now resolves the route param to its numeric PK first.
  • Cache dual-entry — Opening a WP via semantic URL (e.g. /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 normalizes this.workPackageId to the numeric PK after first load.

Merge checklist

  • Added/updated tests
  • Tested major browsers (Chrome, Firefox, Edge, ...)

@akabiru akabiru force-pushed the implementation/73834-stateful-navigation-changes branch from 6bc42ae to 370ad29 Compare April 13, 2026 14:03
@akabiru akabiru changed the title Use displayId for stateful navigation with P1 fixes Use displayId in stateful navigation and fix semantic ID state management Apr 13, 2026
@akabiru akabiru self-assigned this Apr 13, 2026
@akabiru akabiru force-pushed the implementation/73797-use-displayid-in-work-package-urls branch from 6aa7c34 to 1801afd Compare April 14, 2026 15:34
@akabiru akabiru force-pushed the implementation/73834-stateful-navigation-changes branch from 370ad29 to 686a5c7 Compare April 14, 2026 15:36
@akabiru akabiru force-pushed the implementation/73797-use-displayid-in-work-package-urls branch from 1801afd to e37e60f Compare April 15, 2026 16:39
@akabiru akabiru force-pushed the implementation/73834-stateful-navigation-changes branch from 686a5c7 to 58a6421 Compare April 15, 2026 16:40
@akabiru akabiru force-pushed the implementation/73797-use-displayid-in-work-package-urls branch from e37e60f to 6b119b4 Compare April 15, 2026 17:03
akabiru added 7 commits April 15, 2026 20:04
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.
@akabiru akabiru force-pushed the implementation/73834-stateful-navigation-changes branch from 58a6421 to 1388c3c Compare April 15, 2026 17:04
@akabiru akabiru merged commit 1ed3783 into implementation/73797-use-displayid-in-work-package-urls Apr 16, 2026
10 of 11 checks passed
@akabiru akabiru deleted the implementation/73834-stateful-navigation-changes branch April 16, 2026 08:45
@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant