bug/74762 Numeric ID in the email notification after adding watchers

app/helpers/mail_formatting_helper.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+#-- 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.
+#++
+
+# Pin the external rendering channel so mailer templates never have to
+# remember the `render_mode:` / `only_path:` / `static_html:` combination.
+# Matching the `.html.erb` / `.text.erb` extension to the helper name keeps
+# caller intent visible.
+module MailFormattingHelper
+  def format_mail_html(*, **)
+    format_text(*, render_mode: :external_html, **)
+  end
+
+  def format_mail_text(*, **)
+    format_text(*, render_mode: :external_text, **)
+  end
+end

app/mailers/application_mailer.rb

@@ -34,6 +34,7 @@ class ApplicationMailer < ActionMailer::Base
   helper :application, # for format_text
          :work_packages, # for css classes
          :custom_fields, # for show_value
+         :mail_formatting, # for format_mail_html / format_mail_text
          :mail_layout # for layouting
 
   include OpenProject::LocaleHelper

app/models/work_package/semantic_identifier.rb

@@ -121,6 +121,13 @@ def self.numeric_id?(value)
     end
   end
 
+  # Returns formatted value for inline UI display.
+  #   * Semantic mode: "PROJ-42" (no prefix — self-describing)
+  #   * Classic mode: "#42" (hash-prefixed)
+  def self.format_display_id(display_id)
+    display_id.is_a?(String) && display_id.match?(/[A-Za-z]/) ? display_id : "##{display_id}"
+  end
+
   # Returns the user-facing identifier for this work package.
   # In semantic mode: the project-based identifier (e.g. "PROJ-42")
   # In classic mode: the numeric database ID
@@ -134,8 +141,7 @@ def display_id
   # Semantic mode: "PROJ-42" (no prefix — self-describing)
   # Classic mode: "#42" (hash-prefixed)
   def formatted_id
-    did = display_id
-    did.is_a?(String) && did.match?(/[A-Za-z]/) ? did : "##{did}"
+    WorkPackage::SemanticIdentifier.format_display_id(display_id)
   end
 
   # Override ActiveRecord's default `to_param` so Rails URL helpers

app/services/work_packages/update_ancestors_service.rb

@@ -123,7 +123,8 @@ def derive_attributes(work_package, loader, attributes)
 
   def set_journal_note(work_packages)
     work_packages.each do |wp|
-      wp.journal_notes = I18n.t("work_package.updated_automatically_by_child_changes", child: "##{initiator_work_package.id}")
+      wp.journal_notes = I18n.t("work_package.updated_automatically_by_child_changes",
+                                child: "##{initiator_work_package.id}")
     end
   end
 

app/views/work_package_mailer/_work_package_details.html.erb

@@ -47,4 +47,4 @@ See COPYRIGHT and LICENSE files for more details.
   <% end %>
 </ul>
 
-<%= format_text(work_package, :description, only_path: false) %>
+<%= format_mail_html(work_package, :description) %>

app/views/work_package_mailer/mentioned.html.erb

@@ -26,7 +26,7 @@
         <table <%= placeholder_table_styles(width: "100%", style: "width:100%;") %>>
           <tr>
             <td style="<%= placeholder_text_styles %>">
-              <%= format_text @journal.notes, only_path: false %>
+              <%= format_mail_html @journal.notes %>
             </td>
           </tr>
         </table>

app/views/work_package_mailer/mentioned.text.erb

@@ -8,6 +8,10 @@
 <%= "=" * ((@work_package.formatted_id + " " + @work_package.subject).length + 4) %>
 
 <%= I18n.t(:label_comment_added) %>:
-<%= strip_tags @journal.notes %>
+<%= format_mail_text(
+      @journal.notes,
+      object: @work_package,
+      project: @work_package.project
+    ) %>
 
 <%= "-" * 100 %>

app/views/work_package_mailer/watcher_changed.html.erb

@@ -30,9 +30,8 @@ See COPYRIGHT and LICENSE files for more details.
 <hr>
 <%= render partial: "work_package_details", locals: { work_package: @work_package } %>
 <p>
-  <%= format_text(
+  <%= format_mail_html(
         t(:text_latest_note, note: last_work_package_note(@work_package)),
-        only_path: false,
         object: @work_package,
         project: @work_package.project
       ) %>

app/views/work_package_mailer/watcher_changed.text.erb

@@ -31,4 +31,8 @@ See COPYRIGHT and LICENSE files for more details.
 
 ----------------------------------------
 <%= render partial: "work_package_details", locals: { work_package: @work_package } %>
-<%= t(:text_latest_note, note: last_work_package_note(@work_package)) %>
+<%= format_mail_text(
+      t(:text_latest_note, note: last_work_package_note(@work_package)),
+      object: @work_package,
+      project: @work_package.project
+    ) %>

lib/open_project/text_formatting.rb

@@ -43,31 +43,36 @@ module TextFormatting
 
     # @!macro format_text_params
     #   @param [Project] project a Project context.
-    #   @param [Boolean] only_path whether to generate links with relative URLs.
+    #   @param [Symbol] render_mode the rendering channel (`:in_app_html`,
+    #     `:external_html`, `:external_text`). Resolves the `only_path`,
+    #     `static_html` and `plain_text` context flags as a set. Prefer this
+    #     over passing the primitives individually. See {RenderMode}.
+    #   @param [Boolean] only_path explicit override for the resolved `only_path`.
     #   @param [User] current_user the current user context.
     #   @param [:plain, :rich] format the text format.
     #     `:plain` will return plain text.
     #     `:rich` will render raw Markdown as HTML.
     #   @param ** [Hash] additional context to pass to the underlying rendering
-    #      pipeline.
+    #      pipeline. Explicit `static_html:` / `plain_text:` here override the
+    #      values resolved from `render_mode:`.
 
     # rubocop:disable Layout/LineLength
 
     ##
     # Formats text according to system settings and provided params.
     #
-    # @overload format_text(text, object: nil, project: @project || object.try(:project), only_path: true, current_user: User.current, format: :rich, **)
+    # @overload format_text(text, object: nil, project: @project || object.try(:project), render_mode: :in_app_html, only_path: nil, current_user: User.current, format: :rich, **)
     #
     #   @param [String] text the raw text to be formatted, typically Markdown.
     #   @param [Object] object an object context.
     #   @macro format_text_params
     #
     #   @example Setting a project context explicitly
     #     format_text("## Hello world", project: current_project)
-    #   @example Generating links with full URLs
-    #     format_text("[Projects](/projects)", only_path: false)
+    #   @example Rendering for an external surface (mailer, RSS, export)
+    #     format_text("see #42", render_mode: :external_html)
     #
-    # @overload format_text(object, attribute, project: @project || object.try(:project), only_path: true, current_user: User.current, format: :rich, **)
+    # @overload format_text(object, attribute, project: @project || object.try(:project), render_mode: :in_app_html, only_path: nil, current_user: User.current, format: :rich, **)
     #
     #   @param [Object] object an object, typically a model
     #     (i.e. `ActiveRecord::Base` descendent).
@@ -79,7 +84,8 @@ module TextFormatting
     #     format_text(issue, :description, options)
     #
     # @return [String] the formatted text as an HTML-safe String.
-    def format_text(*args, object: nil, project: nil, only_path: true, current_user: User.current, format: :rich, **)
+    def format_text(*args, object: nil, project: nil, render_mode: :in_app_html,
+                    only_path: nil, current_user: User.current, format: :rich, **kwargs)
       case args.size
       when 1
         attribute = nil
@@ -94,15 +100,22 @@ def format_text(*args, object: nil, project: nil, only_path: true, current_user:
 
       project ||= @project || object.try(:project)
 
+      resolved = RenderMode.resolve(
+        render_mode,
+        only_path:,
+        static_html: kwargs.delete(:static_html),
+        plain_text: kwargs.delete(:plain_text)
+      )
+
       Renderer.format_text(
         text,
-        **,
+        **kwargs,
         format:,
         object:,
         request: try(:request),
         current_user:,
         attribute:,
-        only_path:,
+        **resolved,
         project:
       )
     end

lib/open_project/text_formatting/filters/mention_filter.rb

@@ -37,6 +37,8 @@ class MentionFilter < HTML::Pipeline::Filter
       include OpenProject::StaticRouting::UrlHelpers
 
       def call
+        preload_mentions
+
         doc.search("mention").each do |mention|
           anchor = mention_anchor(mention)
           mention.replace(anchor) if anchor
@@ -47,6 +49,41 @@ def call
 
       private
 
+      # WP labels resolve regardless of viewer (so an inaccessible WP
+      # still renders its current formatted_id); a separate id pluck
+      # gates anchor-vs-text. Principals collapse the two concerns into
+      # one visibility-scoped fetch — invisible users and groups fall
+      # back to the literal envelope text.
+      def preload_mentions
+        preload_work_package_mentions
+        preload_principal_mentions
+      end
+
+      def preload_work_package_mentions
+        ids = mention_ids_for("work_package")
+        if ids.empty?
+          @mentioned_work_packages = {}
+          @visible_mentioned_ids = Set.new
+          return
+        end
+
+        scope = WorkPackage.where(id: ids)
+        scope = scope.includes(:type, :status) if context[:static_html]
+        @mentioned_work_packages = scope.index_by(&:id)
+        @visible_mentioned_ids = WorkPackage.visible.where(id: ids).pluck(:id).to_set
+      end
+
+      def preload_principal_mentions
+        user_ids = mention_ids_for("user")
+        group_ids = mention_ids_for("group")
+        @mentioned_users = user_ids.empty? ? {} : User.visible.where(id: user_ids).index_by(&:id)
+        @mentioned_groups = group_ids.empty? ? {} : Group.visible.where(id: group_ids).index_by(&:id)
+      end
+
+      def mention_ids_for(type)
+        doc.css(%(mention[data-type="#{type}"])).filter_map { mention_id(it)&.to_i }.uniq
+      end
+
       def mention_anchor(mention)
         mention_instance = class_from_mention(mention)
 
@@ -75,45 +112,61 @@ def group_mention(group)
       end
 
       def work_package_mention(work_package, mention)
-        # Render the mention with the same label and URL convention used for
-        # `#N` text references elsewhere in the markdown pipeline.
-        display_id = work_package.display_id
+        return Nokogiri::XML::Text.new(work_package.formatted_id, mention.document) if text_only?(work_package)
 
         case mention.text.count("#")
-        when 3
-          ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo",
-                                                    "",
-                                                    data: { id: work_package.id, display_id:, detailed: true }
-        when 2
-          ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo",
-                                                    "",
-                                                    data: { id: work_package.id, display_id:, detailed: false }
-        else
-          link_to(work_package.formatted_id,
-                  work_package_path_or_url(id: display_id, only_path: context[:only_path]),
-                  class: "issue work_package",
-                  data: {
-                    hover_card_trigger_target: "trigger",
-                    hover_card_url: hover_card_work_package_path(display_id)
-                  })
+        when 3 then work_package_quickinfo(work_package, detailed: true)
+        when 2 then work_package_quickinfo(work_package, detailed: false)
+        else        work_package_link(work_package)
         end
       end
 
+      # The hover-card endpoint a quickinfo would link to is unreachable
+      # for plain-text recipients and for viewers without view permission.
+      def text_only?(work_package)
+        context[:plain_text] || @visible_mentioned_ids.exclude?(work_package.id)
+      end
+
+      def work_package_quickinfo(work_package, detailed:)
+        return work_package_static_macro(work_package, detailed:) if context[:static_html]
+
+        ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo",
+                                                  "",
+                                                  data: { id: work_package.id,
+                                                          display_id: work_package.display_id,
+                                                          detailed: }
+      end
+
+      # Uses the WP's current `formatted_id` rather than the envelope text,
+      # so a renamed identifier doesn't leave a stale label in the mailer.
+      def work_package_static_macro(work_package, detailed:)
+        label = OpenProject::TextFormatting::Helpers::StaticMacroLabel
+                  .call(work_package, label: work_package.formatted_id, detailed:)
+
+        link_to(label,
+                work_package_path_or_url(id: work_package.display_id, only_path: context[:only_path]),
+                class: "issue work_package")
+      end
+
+      def work_package_link(work_package)
+        display_id = work_package.display_id
+        link_to(work_package.formatted_id,
+                work_package_path_or_url(id: display_id, only_path: context[:only_path]),
+                class: "issue work_package",
+                data: {
+                  hover_card_trigger_target: "trigger",
+                  hover_card_url: hover_card_work_package_path(display_id)
+                })
+      end
+
       def class_from_mention(mention)
-        mention_class = case mention.attributes["data-type"].value
-                        when "user"
-                          User
-                        when "group"
-                          Group
-                        when "work_package"
-                          WorkPackage
-                        else
-                          raise ArgumentError
-                        end
-
-        mention_class
-          .visible
-          .find_by(id: mention_id(mention)) || fallback_text(mention)
+        id = mention_id(mention)&.to_i
+        case mention.attributes["data-type"].value
+        when "user"         then @mentioned_users[id]
+        when "group"        then @mentioned_groups[id]
+        when "work_package" then @mentioned_work_packages[id]
+        else raise ArgumentError
+        end || fallback_text(mention)
       end
 
       ##

lib/open_project/text_formatting/filters/plain_text_output_filter.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+#-- 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.
+#++
+
+module OpenProject::TextFormatting
+  module Filters
+    # Terminal stage when the rich pipeline is rendering for `text/plain`
+    # bodies — collapses the DOM to its visible text so no HTML escapes.
+    class PlainTextOutputFilter < HTML::Pipeline::Filter
+      def call
+        doc.text
+      end
+    end
+  end
+end