From f1e729ce16b697ccb513d5c32e4e0873e25c7c4c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 6 May 2026 23:35:40 +0300 Subject: [PATCH 1/4] Use formatted_id in WorkPackage#to_s so autocomplete labels speak semantic ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The autocomplete dropdown in CKEditor renders each result with `item.name = wp.to_s`, and `to_s` hardcoded `##{id}` — so a semantic-mode search for `#KSTP-2` resolved correctly but the dropdown row showed `Task #10244: Subject` instead of `Task KSTP-2: Subject`. Switch to `formatted_id`, which already produces the user-facing form in both modes (`PROJ-7` semantic, `#42` classic). The other caller — the delete-dialog component — benefits from the same mode awareness. --- app/models/work_package.rb | 2 +- spec/models/work_package_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/app/models/work_package.rb b/app/models/work_package.rb index bb8057560ed9..d4a110bbdd43 100644 --- a/app/models/work_package.rb +++ b/app/models/work_package.rb @@ -276,7 +276,7 @@ def assignable_versions(only_open: true) end def to_s - "#{type.name unless type.is_standard} ##{id}: #{subject}" + "#{type.name unless type.is_standard} #{formatted_id}: #{subject}" end def infoline(show_standard_type: true) diff --git a/spec/models/work_package_spec.rb b/spec/models/work_package_spec.rb index e797d5075e05..afdedae635e5 100644 --- a/spec/models/work_package_spec.rb +++ b/spec/models/work_package_spec.rb @@ -943,4 +943,31 @@ def stub_shared_versions(version = nil) end end end + + describe "#to_s" do + let(:task_type) { create(:type_task) } + + context "in classic mode", + with_flag: { semantic_work_package_ids: false }, + with_settings: { work_packages_identifier: "classic" } do + let(:work_package) { create(:work_package, project:, type: task_type, subject: "Hello world") } + + it "renders the numeric id with a `#` prefix" do + expect(work_package.to_s).to eq("Task ##{work_package.id}: Hello world") + end + end + + context "in semantic mode", + with_flag: { semantic_work_package_ids: true }, + with_settings: { work_packages_identifier: "semantic" } do + let(:project) { create(:project, identifier: "MACROPROJ", types: [task_type]) } + let(:work_package) { create(:work_package, project:, type: task_type, subject: "Hello world") } + + it "renders the semantic identifier without a `#` prefix" do + wp = work_package.reload + expect(wp.to_s).to eq("Task #{wp.display_id}: Hello world") + expect(wp.to_s).not_to include("##{wp.id}") + end + end + end end From 392ba04d1d972e626c08c006f183de22e7bd9f13 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 7 May 2026 00:24:28 +0300 Subject: [PATCH 2/4] Use formatted_id in journal cause formatter and link_to_work_package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The activity feed renders an automatic entry whenever a parent / child / predecessor / related work package change cascades dates onto the current WP. Both render paths spoke numeric ids: - The plain-text branch (used for the API JSON) hardcoded `##{id}`. - The HTML branch went through `link_to_work_package`, which built the visible link label as `Type ##{id}: subject`. Both now use `formatted_id`, which produces `PROJ-7` in semantic mode and `#42` in classic — same shape, mode-aware. The link href was already mode-correct via `to_param`; only the visible label needed the swap. --- app/helpers/work_packages_helper.rb | 2 +- lib/open_project/journal_formatter/cause.rb | 2 +- spec/helpers/work_packages_helper_spec.rb | 19 +++++++++++++ .../journal_formatter/cause_spec.rb | 27 +++++++++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/app/helpers/work_packages_helper.rb b/app/helpers/work_packages_helper.rb index 9a1d0ddd6633..ec88a5995c6b 100644 --- a/app/helpers/work_packages_helper.rb +++ b/app/helpers/work_packages_helper.rb @@ -47,7 +47,7 @@ def link_to_work_package(work_package, display_project: false, link_subject: fal class: link_to_work_package_css_classes(work_package)) do link_parts = [] link_parts << work_package.type.to_s if work_package.type_id - link_parts << "##{work_package.id}:" + link_parts << "#{work_package.formatted_id}:" link_parts << content_tag(:span, I18n.t(:label_closed_work_packages), class: "sr-only") if work_package.closed? link_parts << work_package.subject if link_subject diff --git a/lib/open_project/journal_formatter/cause.rb b/lib/open_project/journal_formatter/cause.rb index e4ba6e017b38..9df00bdaa939 100644 --- a/lib/open_project/journal_formatter/cause.rb +++ b/lib/open_project/journal_formatter/cause.rb @@ -243,7 +243,7 @@ def related_work_package_changed_message if related_work_package I18n.t( "journals.cause_descriptions.#{cause['type']}", - link: html? ? link_to_work_package(related_work_package, link_subject: true) : "##{related_work_package.id}" + link: html? ? link_to_work_package(related_work_package, link_subject: true) : related_work_package.formatted_id ) else diff --git a/spec/helpers/work_packages_helper_spec.rb b/spec/helpers/work_packages_helper_spec.rb index 20c06a8cf70e..7c242d284b53 100644 --- a/spec/helpers/work_packages_helper_spec.rb +++ b/spec/helpers/work_packages_helper_spec.rb @@ -90,6 +90,25 @@ expect(helper.link_to_work_package(stub_work_package)).to have_no_text(text) end end + + describe "in semantic mode", + with_flag: { semantic_work_package_ids: true }, + with_settings: { work_packages_identifier: "semantic" } do + let(:stub_work_package) { build_stubbed(:work_package, type: stub_type, identifier: "MACROPROJ-42") } + + it "uses the semantic identifier in the visible link label" do + link_text = Regexp.new("^#{stub_type.name} #{stub_work_package.formatted_id}:$") + expect(helper.link_to_work_package(stub_work_package)) + .to have_css("a[href='#{work_package_path(stub_work_package)}']", text: link_text) + end + + it "does not embed the bare numeric `#N` form in the link label" do + # The href independently uses display_id (via to_param) and is fine; + # the visible label is what this assertion guards. + result = helper.link_to_work_package(stub_work_package) + expect(result).to have_no_css("a", text: /##{stub_work_package.id}:/) + end + end end describe "#work_packages_columns_options" do diff --git a/spec/lib/open_project/journal_formatter/cause_spec.rb b/spec/lib/open_project/journal_formatter/cause_spec.rb index 6ded9787da43..2c6befd4b7bb 100644 --- a/spec/lib/open_project/journal_formatter/cause_spec.rb +++ b/spec/lib/open_project/journal_formatter/cause_spec.rb @@ -154,6 +154,33 @@ def render(cause, html:) ) end end + + context "in semantic mode", + with_flag: { semantic_work_package_ids: true }, + with_settings: { work_packages_identifier: "semantic" } do + let(:semantic_project) { create(:project, identifier: "MACROPROJ") } + let(:work_package) { create(:work_package, project: semantic_project) } + + before do + work_package.allocate_and_register_semantic_id + allow(WorkPackage).to receive(:visible).with(User.current).and_return(WorkPackage.where(id: work_package.id)) + end + + it "renders the related work package's formatted_id in raw text mode" do + wp = work_package.reload + expect(cause).to render_raw_variant(a_string_including(wp.formatted_id)) + expect(cause).not_to render_raw_variant(a_string_including("##{wp.id}")) + end + + it "renders the related work package's formatted_id in the link label in HTML mode" do + wp = work_package.reload + # The visible link label should be `… formatted_id: subject`, not + # `… #1234: subject`. The href is mode-agnostic and uses display_id + # via `to_param` regardless. + expect(cause).to render_html_variant(a_string_including("#{wp.formatted_id}:")) + expect(cause).not_to render_html_variant(a_string_including(" ##{wp.id}:")) + end + end end context "when the change was caused by a change to a predecessor" do From d1fc18b7f2375b1ec7c675f69f47ddd7b5f2fda3 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 13 May 2026 17:49:52 +0300 Subject: [PATCH 3/4] Apply S1 review feedback - Pin literal "MACROPROJ-42" in helper spec so a regression where the helper accidentally returns `#N` shape still trips the assertion. Previously the expected string was built from the same accessor under test. - Drop framework-mechanics narration from the cause spec comment. The assertion already conveys intent. --- spec/helpers/work_packages_helper_spec.rb | 4 +--- spec/lib/open_project/journal_formatter/cause_spec.rb | 3 --- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/spec/helpers/work_packages_helper_spec.rb b/spec/helpers/work_packages_helper_spec.rb index 7c242d284b53..70397371df16 100644 --- a/spec/helpers/work_packages_helper_spec.rb +++ b/spec/helpers/work_packages_helper_spec.rb @@ -97,14 +97,12 @@ let(:stub_work_package) { build_stubbed(:work_package, type: stub_type, identifier: "MACROPROJ-42") } it "uses the semantic identifier in the visible link label" do - link_text = Regexp.new("^#{stub_type.name} #{stub_work_package.formatted_id}:$") + link_text = Regexp.new("^#{stub_type.name} MACROPROJ-42:$") expect(helper.link_to_work_package(stub_work_package)) .to have_css("a[href='#{work_package_path(stub_work_package)}']", text: link_text) end it "does not embed the bare numeric `#N` form in the link label" do - # The href independently uses display_id (via to_param) and is fine; - # the visible label is what this assertion guards. result = helper.link_to_work_package(stub_work_package) expect(result).to have_no_css("a", text: /##{stub_work_package.id}:/) end diff --git a/spec/lib/open_project/journal_formatter/cause_spec.rb b/spec/lib/open_project/journal_formatter/cause_spec.rb index 2c6befd4b7bb..58e4138570a6 100644 --- a/spec/lib/open_project/journal_formatter/cause_spec.rb +++ b/spec/lib/open_project/journal_formatter/cause_spec.rb @@ -174,9 +174,6 @@ def render(cause, html:) it "renders the related work package's formatted_id in the link label in HTML mode" do wp = work_package.reload - # The visible link label should be `… formatted_id: subject`, not - # `… #1234: subject`. The href is mode-agnostic and uses display_id - # via `to_param` regardless. expect(cause).to render_html_variant(a_string_including("#{wp.formatted_id}:")) expect(cause).not_to render_html_variant(a_string_including(" ##{wp.id}:")) end From c9c9ea782ca853e585fc666370c9229d5af8ccbe Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 13 May 2026 21:20:35 +0300 Subject: [PATCH 4/4] Drop mode-specific id from link_to_work_package doc examples The hardcoded `#6` in the examples was misleading once the rendered label became mode-dependent. Use `` placeholder so the docstring stays accurate across both modes without re-describing the modes. --- app/helpers/work_packages_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/work_packages_helper.rb b/app/helpers/work_packages_helper.rb index ec88a5995c6b..4011e03baacc 100644 --- a/app/helpers/work_packages_helper.rb +++ b/app/helpers/work_packages_helper.rb @@ -35,9 +35,9 @@ module WorkPackagesHelper # Displays a link to +work_package+ with its subject. # Examples: # - # link_to_work_package(package) # => Defect #6: This is the subject - # link_to_work_package(package, link_subject: true) # => Defect #6: This is the subject (everything within the link) - # link_to_work_package(package, display_project: true) # => Foo - Defect #6: This is the subject + # link_to_work_package(package) # => Defect : This is the subject + # link_to_work_package(package, link_subject: true) # => Defect : This is the subject (everything within the link) + # link_to_work_package(package, display_project: true) # => Foo - Defect : This is the subject def link_to_work_package(work_package, display_project: false, link_subject: false) # rubocop:disable Metrics/AbcSize output = ActiveSupport::SafeBuffer.new output << "#{work_package.project} - " if display_project && work_package.project_id