From e269fddffce9977edbfd2290960365c9c480aa68 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 26 May 2026 12:54:40 +0300 Subject: [PATCH] Extract shared static-macro label composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer feedback on cd122f867cc: the `parts << status / type / label` block was duplicated between the regex-driven (`WorkPackages` link handler) and envelope-driven (`MentionFilter`) static paths, with no guard against silent desynchronisation. Centralise the composition on the link handler and document why the two callers pass different labels — the regex path preserves the alias-as-matched, the envelope path normalises to the WP's current formatted_id. --- .../text_formatting/filters/mention_filter.rb | 13 ++++----- .../matchers/link_handlers/work_packages.rb | 29 ++++++++++++------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/lib/open_project/text_formatting/filters/mention_filter.rb b/lib/open_project/text_formatting/filters/mention_filter.rb index 6730542395e..7f11e8bfd45 100644 --- a/lib/open_project/text_formatting/filters/mention_filter.rb +++ b/lib/open_project/text_formatting/filters/mention_filter.rb @@ -134,15 +134,14 @@ module OpenProject::TextFormatting # Static fallback shared with the PatternMatcherFilter's `##`/`###` # path so envelope-driven and text-driven references render the same - # shape in channels that cannot hydrate the custom element. + # shape in channels that cannot hydrate the custom element. Always + # uses the WP's current `formatted_id`, normalising any historical + # alias the envelope may have been authored with. def work_package_static_macro(work_package, detailed:) - parts = [] - parts << work_package.status&.name if detailed - parts << work_package.type&.name - parts << work_package.formatted_id - link_text = "#{parts.compact.join(' ')}: #{work_package.subject}" + label = OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages + .compose_static_macro_label(work_package, label: work_package.formatted_id, detailed:) - link_to(link_text, + link_to(label, work_package_path_or_url(id: work_package.display_id, only_path: context[:only_path]), class: "issue work_package") end diff --git a/lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb b/lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb index b1d7f706516..09c41d04bdf 100644 --- a/lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb +++ b/lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb @@ -38,6 +38,17 @@ module OpenProject::TextFormatting::Matchers # Shared with the PDF-export subclass in `app/models/work_package/exports/macros/links.rb`. HASH_TRIGGERS = %w[# ## ###].freeze + # Builds the user-facing label for the static-anchor variant of `##`/ + # `###` macros. Centralised so the PatternMatcherFilter path here and + # the ``-envelope path in `MentionFilter` stay in lockstep. + def self.compose_static_macro_label(work_package, label:, detailed:) + parts = [] + parts << work_package.status&.name if detailed + parts << work_package.type&.name + parts << label + "#{parts.compact.join(' ')}: #{work_package.subject}" + end + def applicable? hash_trigger? && matcher.prefix.nil? end @@ -112,20 +123,16 @@ module OpenProject::TextFormatting::Matchers end # Static fallback for channels that cannot hydrate the quickinfo - # custom element (HTML mailers, exports). Mirrors the in-app widget's - # text composition — type, optional status, formatted_id, subject — - # so the anchor reads the same as the rich rendering once flattened. - # Unresolved references collapse to the bare label. + # custom element. Composes type, optional status, label, and subject + # into the anchor; unresolved references collapse to the bare label. + # The label is the matched identifier (potentially a historical alias) + # to preserve what the author wrote — the `` envelope path + # in `MentionFilter` instead normalises to the WP's current + # `formatted_id`. def render_static_work_package_macro(work_package, label, detailed:) return label unless work_package - parts = [] - parts << work_package.status&.name if detailed - parts << work_package.type&.name - parts << label - link_text = "#{parts.compact.join(' ')}: #{work_package.subject}" - - link_to(link_text, + link_to(self.class.compose_static_macro_label(work_package, label:, detailed:), work_package_path_or_url(id: work_package.display_id, only_path: context[:only_path]), class: "issue work_package") end