Merge pull request #23337 from opf/bug/74762-numeric-id-in-the-email-notification-after-adding-watchers

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

Author: akabiru

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