Skip to content

[#74314] Use displayId on backlogs view work package click#22905

Merged
myabc merged 6 commits into
devfrom
implementation/74314-use-displayid-on-backlogs-view-work-package-click
May 1, 2026
Merged

[#74314] Use displayId on backlogs view work package click#22905
myabc merged 6 commits into
devfrom
implementation/74314-use-displayid-on-backlogs-view-work-package-click

Conversation

@akabiru
Copy link
Copy Markdown
Member

@akabiru akabiru commented Apr 23, 2026

Ticket

https://community.openproject.org/work_packages/74314

Follow-up from the PR 22733 review: #22733 (review)

Screenshots

Verified manually in semantic mode against the ACSMT project's Backlogs view on the local dev edge.

Story-row Stimulus values carry the semantic identifier in both URLs and a displayIdValue for selection sync:

pr22905-backlogs-story-row

Click → split view URL uses the semantic identifier:

pr22905-backlogs-split-click

Follow-up scope verification

Additional sprint and overflow-menu surfaces verified live against the SEP project (sprint story #20919SEP-7, inbox item #20881SEP-6).

Sprint card story rows (sprint_component) — displayIdValue data-attr and both URLs carry the semantic identifier:

pr22905-sprint-card-displayid

Sprint story overflow menu (story_menu_list_component) — Open details, Open fullscreen, and Copy URL hrefs all use the semantic identifier; the Copy work package ID action intentionally keeps the numeric primary key:

pr22905-story-menu-overflow

Inbox row overflow menu (inbox_menu_component) — same trio of semantic-ID URLs, same numeric Copy work package ID:

pr22905-inbox-menu-overflow

Route constraintbacklog_details now applies WorkPackage::SemanticIdentifier::ID_ROUTE_CONSTRAINT, so navigating to /projects/SEP/backlogs/backlog/details/SEP-7 resolves the correct work package via the display_id finder:

pr22905-route-constraint-resolution

Fix

@akabiru akabiru self-assigned this Apr 23, 2026
@akabiru akabiru force-pushed the implementation/74314-use-displayid-on-backlogs-view-work-package-click branch from 342dab9 to 17e5b18 Compare April 28, 2026 13:31
@akabiru akabiru force-pushed the implementation/74314-use-displayid-on-backlogs-view-work-package-click branch from ed518aa to 0231cd6 Compare April 29, 2026 11:57
@myabc myabc self-requested a review April 29, 2026 15:49
Copy link
Copy Markdown
Contributor

@myabc myabc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds a significant amount of complexity for a concern that is not really Backlogs-related - I'd prefer to see us implement #to_param first - not change the path helper call sites.

akabiru added 5 commits April 30, 2026 11:23
The two Backlogs card components (sprint/bucket and inbox) built the
split-view and full-view URLs by passing the WorkPackage model to Rails
path helpers, which fall back to to_param (numeric PK). Pass
work_package.display_id explicitly so semantic-mode projects get URLs
like /work_packages/PROJ-7 instead of /work_packages/42.

The Stimulus story controller's selection-sync regex assumed numeric
identifiers (/details/(\d+)); widen it via the shared WP_ID_URL_PATTERN
so semantic segments match too, and add a displayId Stimulus value so
the controller can compare the URL capture against either identifier
form.

Component specs cover the semantic-mode URL construction; the existing
classic-mode specs double as the regression guard for the numeric path.
Three more Backlogs view components were still passing the WorkPackage
model to Rails path helpers, which falls back to to_param (numeric PK):

- sprint_component: split-view and full-view URLs in story rows. Also
  threads displayId through as a Stimulus value so backlogs--story can
  match the URL segment against either id form (mirrors the inbox/bucket
  fix from the previous commit).
- inbox_menu_component: open details, open fullscreen, and clipboard
  copy URLs in the overflow menu.
- story_menu_list_component: same trio for sprint stories.

The "copy work package ID" clipboard item keeps the numeric primary key
because that action's contract is explicitly to copy the integer ID.

Each component spec gets a "in semantic mode" context that asserts the
URLs use the displayId and do not include the numeric id, mirroring the
pattern from the bucket/inbox specs in the previous commit. The existing
classic-mode examples remain as the regression guard for the numeric
path; sprint_component additionally gains an explicit classic-mode data-
attribute check since it had none before.
The backlog details route accepted any segment as :work_package_id,
diverging from the equivalent core WP routes which apply
WorkPackage::SemanticIdentifier::ID_ROUTE_CONSTRAINT. Aligning the
constraint keeps the surface uniform and prevents a future sibling
segment from silently matching this route.
- Replace eq("BACKLOG-1") / eq("INBOX-1") with start_with(...) so the
  test pins the URL shape rather than coupling to the allocator's
  "starts at 1" contract.
- Pin data-backlogs--story-display-id-value, the data-attr the
  story Stimulus controller now reads to compare URL segments.
- Add the symmetric not_to include("/details/#{numeric_id}")
  negative assertion alongside the existing full-view one.
WorkPackage#to_param now returns display_id, so URL helpers emit
semantic IDs without per-callsite display_id boilerplate. Drop the
explicit .display_id arguments from split_url, full_url, and the
overflow-menu open/copy links; leave the Stimulus displayId value
pass-through (the JS selection-from-URL comparator still needs both
forms) and the backlog_details route constraint (still needed to
accept either form in the URL) untouched. Component specs already
cover semantic and classic URL output and continue to pass.
@akabiru akabiru force-pushed the implementation/74314-use-displayid-on-backlogs-view-work-package-click branch from 0231cd6 to 9337d66 Compare April 30, 2026 08:30
@akabiru akabiru changed the base branch from dev to implementation/74543-wp-to-param-override April 30, 2026 08:31
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

Deploying openproject with PullPreview

Field Value
Latest commit e78b920
Job deploy
Status 🗑️ Preview destroyed
Preview URL Destroyed

View logs

The previous comment described what the code does (compare two forms)
without explaining why a numeric URL would legitimately appear in
semantic mode. Name the actual reason — bookmarks and external links
predate semantic-mode activation — so the defensive comparison is
self-justifying to a future reader.
Base automatically changed from implementation/74543-wp-to-param-override to dev April 30, 2026 08:52
@akabiru akabiru marked this pull request as ready for review April 30, 2026 09:00
@akabiru akabiru requested a review from myabc April 30, 2026 09:00
@akabiru akabiru added this to the 17.5.x milestone Apr 30, 2026
Comment on lines +163 to +164
clipboard_id = page.find("clipboard-copy##{"work_package_#{work_package.id}_menu_copy_work_package_id"}")
expect(clipboard_id[:value]).to eq(work_package.id.to_s)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the desired behaviour.

Copy link
Copy Markdown
Contributor

@myabc myabc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works. 👍🏻

I'm not sure the backlog menu components need to test URLs in such detail, now that overriding #to_param. However this code will churn considerably in #22936, #22999 and other work planned for this sprint, so I think it makes sure to get this PR in as quickly as possible.

@myabc myabc merged commit 241fd02 into dev May 1, 2026
15 of 16 checks passed
@myabc myabc deleted the implementation/74314-use-displayid-on-backlogs-view-work-package-click branch May 1, 2026 18:07
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

2 participants