From 3415da148f8cd13f1ce9ed6db60c423787d09057 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Mon, 25 May 2026 22:48:37 +0200 Subject: [PATCH 1/6] fix(ui): Numeric ID in URL of the wp link when opened from search results (WP #74834) --- .../global-search-input.component.spec.ts | 91 +++++++++++++++++++ .../input/global-search-input.component.ts | 2 +- spec/features/search/search_spec.rb | 22 +++++ 3 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 frontend/src/app/core/global_search/input/global-search-input.component.spec.ts diff --git a/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts b/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts new file mode 100644 index 000000000000..30a508332dc2 --- /dev/null +++ b/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts @@ -0,0 +1,91 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { HalResource } from 'core-app/features/hal/resources/hal-resource'; +import { GlobalSearchInputComponent } from './global-search-input.component'; + +describe('GlobalSearchInputComponent#followItem', () => { + let wpPathSpy:jasmine.Spy<(id:string) => string>; + let searchInScopeSpy:jasmine.Spy; + let context:Pick & { searchInScope:jasmine.Spy }; + + function callFollowItem(item:unknown):void { + GlobalSearchInputComponent.prototype.followItem.call(context, item); + } + + beforeEach(() => { + wpPathSpy = jasmine.createSpy('wpPath').and.returnValue('/work_packages/PROJ-42'); + searchInScopeSpy = jasmine.createSpy('searchInScope'); + context = { + wpPath: wpPathSpy, + selectedItem: undefined, + searchInScope: searchInScopeSpy, + }; + }); + + describe('when item is a HalResource', () => { + let item:HalResource; + + beforeEach(() => { + item = Object.create(HalResource.prototype) as HalResource; + Object.defineProperty(item, 'id', { get: () => '42', configurable: true }); + Object.defineProperty(item, 'displayId', { get: () => 'PROJ-42', configurable: true }); + }); + + it('calls wpPath with displayId', () => { + callFollowItem(item); + expect(wpPathSpy).toHaveBeenCalledWith('PROJ-42'); + }); + + it('does not call wpPath with the numeric id', () => { + callFollowItem(item); + expect(wpPathSpy).not.toHaveBeenCalledWith('42'); + }); + + it('sets selectedItem to the item', () => { + callFollowItem(item); + expect(context.selectedItem).toBe(item as any); + }); + }); + + describe('when item is a scope option (not a HalResource)', () => { + it('delegates to searchInScope and does not call wpPath', () => { + callFollowItem({ projectScope: 'current_project', text: 'In this project ↵' }); + expect(searchInScopeSpy).toHaveBeenCalledWith('current_project'); + expect(wpPathSpy).not.toHaveBeenCalled(); + }); + }); + + describe('when item is undefined', () => { + it('does nothing', () => { + callFollowItem(undefined); + expect(wpPathSpy).not.toHaveBeenCalled(); + expect(searchInScopeSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/frontend/src/app/core/global_search/input/global-search-input.component.ts b/frontend/src/app/core/global_search/input/global-search-input.component.ts index 5f693bd17280..7c067bdd4d72 100644 --- a/frontend/src/app/core/global_search/input/global-search-input.component.ts +++ b/frontend/src/app/core/global_search/input/global-search-input.component.ts @@ -263,7 +263,7 @@ export class GlobalSearchInputComponent implements AfterViewInit, OnDestroy { public followItem(item:WorkPackageResource|SearchOptionItem|undefined):void { this.selectedItem = item; if (item instanceof HalResource) { - window.location.href = this.wpPath(item.id!); + window.location.href = this.wpPath(item.displayId); } else if (item) { this.searchInScope(item.projectScope); } diff --git a/spec/features/search/search_spec.rb b/spec/features/search/search_spec.rb index 453566c40645..e939a6bc0714 100644 --- a/spec/features/search/search_spec.rb +++ b/spec/features/search/search_spec.rb @@ -461,6 +461,28 @@ def expect_range(param_a, param_b) end end + describe "when semantic work package IDs are active" do + let(:run_visit) { false } + let(:semantic_identifier) { "#{project.identifier}-99" } + let!(:semantic_wp) do + create(:work_package, subject: "SemanticIdentifierTest WP", project:).tap do |wp| + wp.update_column(:identifier, semantic_identifier) + end + end + + before do + allow(Setting::WorkPackageIdentifier).to receive(:semantic_mode_active?).and_return(true) + visit search_path(scope: "all", q: "SemanticIdentifierTest") + end + + it "classic results link to the semantic identifier URL, not the numeric ID" do + within("dt.work_package-edit") do + expect(page).to have_link(href: %r{/work_packages/#{Regexp.escape(semantic_identifier)}}) + expect(page).not_to have_link(href: %r{/work_packages/#{semantic_wp.id}[^-]}) + end + end + end + describe "search for notes" do let(:work_package) { work_packages[0] } let!(:note_one) do From 78025601fe09c5fd9ab73a617235cd10008a3804 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 29 May 2026 19:12:16 +0300 Subject: [PATCH 2/6] Use semantic identifier for search dropdown result links 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/ 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. --- .../input/global-search-input.component.html | 4 +- .../global-search-input.component.spec.ts | 41 +++++++++++-------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/frontend/src/app/core/global_search/input/global-search-input.component.html b/frontend/src/app/core/global_search/input/global-search-input.component.html index 0a5d28b4fc42..a1568f29b670 100644 --- a/frontend/src/app/core/global_search/input/global-search-input.component.html +++ b/frontend/src/app/core/global_search/input/global-search-input.component.html @@ -79,8 +79,8 @@ } @else {
diff --git a/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts b/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts index 30a508332dc2..40631f6cd586 100644 --- a/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts +++ b/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts @@ -27,65 +27,74 @@ //++ import { HalResource } from 'core-app/features/hal/resources/hal-resource'; +import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; import { GlobalSearchInputComponent } from './global-search-input.component'; +// followItem is verified through the prototype against a stand-in context, avoiding +// a real component instance whose many injected dependencies this branch never uses. describe('GlobalSearchInputComponent#followItem', () => { - let wpPathSpy:jasmine.Spy<(id:string) => string>; - let searchInScopeSpy:jasmine.Spy; - let context:Pick & { searchInScope:jasmine.Spy }; + let wpPathArgs:string[]; + let searchInScopeArgs:string[]; + let context:Pick & { searchInScope:(scope:string) => void }; function callFollowItem(item:unknown):void { GlobalSearchInputComponent.prototype.followItem.call(context, item); } beforeEach(() => { - wpPathSpy = jasmine.createSpy('wpPath').and.returnValue('/work_packages/PROJ-42'); - searchInScopeSpy = jasmine.createSpy('searchInScope'); + wpPathArgs = []; + searchInScopeArgs = []; context = { - wpPath: wpPathSpy, + wpPath: (id:string):string => { + wpPathArgs.push(id); + // A fragment keeps followItem's window.location assignment from navigating the runner. + return '#stub'; + }, selectedItem: undefined, - searchInScope: searchInScopeSpy, + searchInScope: (scope:string):void => { + searchInScopeArgs.push(scope); + }, }; }); describe('when item is a HalResource', () => { - let item:HalResource; + let item:WorkPackageResource; beforeEach(() => { - item = Object.create(HalResource.prototype) as HalResource; + item = Object.create(HalResource.prototype) as WorkPackageResource; Object.defineProperty(item, 'id', { get: () => '42', configurable: true }); Object.defineProperty(item, 'displayId', { get: () => 'PROJ-42', configurable: true }); }); it('calls wpPath with displayId', () => { callFollowItem(item); - expect(wpPathSpy).toHaveBeenCalledWith('PROJ-42'); + expect(wpPathArgs).toEqual(['PROJ-42']); }); it('does not call wpPath with the numeric id', () => { callFollowItem(item); - expect(wpPathSpy).not.toHaveBeenCalledWith('42'); + expect(wpPathArgs).not.toContain('42'); }); it('sets selectedItem to the item', () => { callFollowItem(item); - expect(context.selectedItem).toBe(item as any); + expect(context.selectedItem).toBe(item); }); }); describe('when item is a scope option (not a HalResource)', () => { it('delegates to searchInScope and does not call wpPath', () => { callFollowItem({ projectScope: 'current_project', text: 'In this project ↵' }); - expect(searchInScopeSpy).toHaveBeenCalledWith('current_project'); - expect(wpPathSpy).not.toHaveBeenCalled(); + expect(searchInScopeArgs).toEqual(['current_project']); + expect(wpPathArgs).toEqual([]); }); }); describe('when item is undefined', () => { it('does nothing', () => { callFollowItem(undefined); - expect(wpPathSpy).not.toHaveBeenCalled(); - expect(searchInScopeSpy).not.toHaveBeenCalled(); + expect(wpPathArgs).toEqual([]); + expect(searchInScopeArgs).toEqual([]); }); }); }); From 14ef6f901de7d728dfc0014f69148a04009c9d62 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 29 May 2026 19:12:27 +0300 Subject: [PATCH 3/6] Use semantic identifier for server-rendered search result links 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/ 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. --- app/models/work_package/journalized.rb | 2 +- spec/features/search/search_spec.rb | 21 ++++++++++--------- .../work_package_acts_as_event_spec.rb | 16 ++++++++++++-- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/app/models/work_package/journalized.rb b/app/models/work_package/journalized.rb index 01347465f5ec..90be69c56289 100644 --- a/app/models/work_package/journalized.rb +++ b/app/models/work_package/journalized.rb @@ -79,7 +79,7 @@ def self.event_type def self.event_url Proc.new do |o| - { controller: :work_packages, action: :show, id: o.id } + { controller: :work_packages, action: :show, id: o.display_id } end end end diff --git a/spec/features/search/search_spec.rb b/spec/features/search/search_spec.rb index e939a6bc0714..b7a8227e8d1c 100644 --- a/spec/features/search/search_spec.rb +++ b/spec/features/search/search_spec.rb @@ -461,24 +461,25 @@ def expect_range(param_a, param_b) end end - describe "when semantic work package IDs are active" do + describe "when semantic work package IDs are active", + with_settings: { work_packages_identifier: "semantic" } do let(:run_visit) { false } - let(:semantic_identifier) { "#{project.identifier}-99" } - let!(:semantic_wp) do - create(:work_package, subject: "SemanticIdentifierTest WP", project:).tap do |wp| - wp.update_column(:identifier, semantic_identifier) - end + let(:semantic_project) { create(:project, :semantic) } + let(:semantic_wp) do + create(:work_package, subject: "SemanticIdentifierTest WP", project: semantic_project) end before do - allow(Setting::WorkPackageIdentifier).to receive(:semantic_mode_active?).and_return(true) + semantic_wp visit search_path(scope: "all", q: "SemanticIdentifierTest") end - it "classic results link to the semantic identifier URL, not the numeric ID" do + it "links results to the semantic identifier URL, not the numeric ID" do + identifier = semantic_wp.reload.identifier + within("dt.work_package-edit") do - expect(page).to have_link(href: %r{/work_packages/#{Regexp.escape(semantic_identifier)}}) - expect(page).not_to have_link(href: %r{/work_packages/#{semantic_wp.id}[^-]}) + expect(page).to have_link(href: %r{/work_packages/#{Regexp.escape(identifier)}(?:$|[#?])}) + expect(page).to have_no_link(href: %r{/work_packages/#{semantic_wp.id}(?:$|[#?])}) end end end diff --git a/spec/models/work_package/work_package_acts_as_event_spec.rb b/spec/models/work_package/work_package_acts_as_event_spec.rb index 21a23ba4ffef..ccd38692587a 100644 --- a/spec/models/work_package/work_package_acts_as_event_spec.rb +++ b/spec/models/work_package/work_package_acts_as_event_spec.rb @@ -35,9 +35,21 @@ let(:stub_work_package) { build_stubbed(:work_package) } describe "#event_url" do - let(:expected_url) { { controller: :work_packages, action: :show, id: stub_work_package.id } } + context "in classic mode" do + let(:expected_url) { { controller: :work_packages, action: :show, id: stub_work_package.id } } - it { expect(stub_work_package.event_url).to eq(expected_url) } + it { expect(stub_work_package.event_url).to eq(expected_url) } + end + + context "in semantic mode", with_settings: { work_packages_identifier: "semantic" } do + let(:project) { create(:project, :semantic) } + let(:work_package) { create(:work_package, project:) } + + it "links to the semantic identifier rather than the numeric id" do + expect(work_package.event_url) + .to eq(controller: :work_packages, action: :show, id: work_package.reload.identifier) + end + end end end end From 784756091fcada959dc7e11adab2de907a04bd9e Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 29 May 2026 19:41:07 +0300 Subject: [PATCH 4/6] Strengthen global search spec to exercise real displayId getter 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. --- .../global-search-input.component.spec.ts | 47 ++++++++++++------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts b/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts index 40631f6cd586..6ee4354bb3c2 100644 --- a/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts +++ b/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts @@ -57,28 +57,43 @@ describe('GlobalSearchInputComponent#followItem', () => { }; }); - describe('when item is a HalResource', () => { - let item:WorkPackageResource; + describe('when item is a work package resource', () => { + // Build a real WorkPackageResource off its prototype and feed it a HAL $source, + // so followItem exercises the production displayId getter rather than a stub. + function buildWorkPackage(source:{ id:number, displayId?:string }):WorkPackageResource { + const item = Object.create(WorkPackageResource.prototype) as WorkPackageResource; + item.$source = source; + return item; + } - beforeEach(() => { - item = Object.create(HalResource.prototype) as WorkPackageResource; - Object.defineProperty(item, 'id', { get: () => '42', configurable: true }); - Object.defineProperty(item, 'displayId', { get: () => 'PROJ-42', configurable: true }); + it('is recognised as a HalResource', () => { + expect(buildWorkPackage({ id: 42 }) instanceof HalResource).toBe(true); }); - it('calls wpPath with displayId', () => { - callFollowItem(item); - expect(wpPathArgs).toEqual(['PROJ-42']); - }); + describe('in semantic mode (source carries a semantic displayId)', () => { + let item:WorkPackageResource; + + beforeEach(() => { + item = buildWorkPackage({ id: 42, displayId: 'PROJ-42' }); + }); + + it('navigates via the semantic displayId, not the numeric id', () => { + callFollowItem(item); + expect(wpPathArgs).toEqual(['PROJ-42']); + expect(wpPathArgs).not.toContain('42'); + }); - it('does not call wpPath with the numeric id', () => { - callFollowItem(item); - expect(wpPathArgs).not.toContain('42'); + it('sets selectedItem to the item', () => { + callFollowItem(item); + expect(context.selectedItem).toBe(item); + }); }); - it('sets selectedItem to the item', () => { - callFollowItem(item); - expect(context.selectedItem).toBe(item); + describe('in classic mode (source has only the numeric id)', () => { + it('falls back to the numeric id through displayId', () => { + callFollowItem(buildWorkPackage({ id: 42 })); + expect(wpPathArgs).toEqual(['42']); + }); }); }); From b29cf5a6bb2baaa9a15d9e4f9f768ac2c0dc1d6e Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 29 May 2026 20:10:34 +0300 Subject: [PATCH 5/6] Correct acts_as_event usage note and tighten followItem spec arg type 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. --- .../input/global-search-input.component.spec.ts | 2 +- lib_static/plugins/acts_as_event/lib/acts_as_event.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts b/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts index 6ee4354bb3c2..0549b10ecf46 100644 --- a/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts +++ b/frontend/src/app/core/global_search/input/global-search-input.component.spec.ts @@ -37,7 +37,7 @@ describe('GlobalSearchInputComponent#followItem', () => { let searchInScopeArgs:string[]; let context:Pick & { searchInScope:(scope:string) => void }; - function callFollowItem(item:unknown):void { + function callFollowItem(item:Parameters[0]):void { GlobalSearchInputComponent.prototype.followItem.call(context, item); } diff --git a/lib_static/plugins/acts_as_event/lib/acts_as_event.rb b/lib_static/plugins/acts_as_event/lib/acts_as_event.rb index 1d3e6ea90404..c8199c1d54a9 100644 --- a/lib_static/plugins/acts_as_event/lib/acts_as_event.rb +++ b/lib_static/plugins/acts_as_event/lib/acts_as_event.rb @@ -28,9 +28,9 @@ # See COPYRIGHT and LICENSE files for more details. #++ -# This now only seems to be used when rendering atom responses. -# Search as well as activities do not rely on it. -# Thus, whenever an atom link is removed for a resource, acts_as_event within that model can also be removed. +# The event_* methods defined here power the server-rendered search results page +# and atom feeds. The Activities subsystem renders through its own providers, so +# a model can drop acts_as_event only once neither search nor atom references it. module Redmine module Acts From 8d53064d34600cee821b109ef3ba85e239c28173 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Sat, 30 May 2026 09:18:24 +0300 Subject: [PATCH 6/6] Narrow followItem guard to WorkPackageResource 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. --- .../core/global_search/input/global-search-input.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/app/core/global_search/input/global-search-input.component.ts b/frontend/src/app/core/global_search/input/global-search-input.component.ts index 7c067bdd4d72..a6af6a09f856 100644 --- a/frontend/src/app/core/global_search/input/global-search-input.component.ts +++ b/frontend/src/app/core/global_search/input/global-search-input.component.ts @@ -262,7 +262,7 @@ export class GlobalSearchInputComponent implements AfterViewInit, OnDestroy { public followItem(item:WorkPackageResource|SearchOptionItem|undefined):void { this.selectedItem = item; - if (item instanceof HalResource) { + if (item instanceof WorkPackageResource) { window.location.href = this.wpPath(item.displayId); } else if (item) { this.searchInScope(item.projectScope);