Editor round-trip - semantic-id-aware mention contract

app/controllers/work_packages/auto_completes_controller.rb

@@ -57,7 +57,15 @@ def work_packages_matching_query_prop
 
   def wp_hashes_with_string(work_packages)
     work_packages.map do |work_package|
-      work_package.attributes.merge("to_s" => work_package.to_s)
+      # `displayId` collapses to the numeric id in classic mode and to the
+      # semantic identifier in semantic mode. The CKEditor mention plugin
+      # reads it to insert `#PROJ-7` (or `#1234`) into the markdown source.
+      # Stringified for parity with APIv3 (`WorkPackageRepresenter` serialises
+      # `displayId` as a string).
+      work_package.attributes.merge(
+        "to_s" => work_package.to_s,
+        "displayId" => work_package.display_id.to_s
+      )
     end
   end
 end

frontend/src/app/shared/components/fields/macros/work-package-quickinfo-macro.component.ts

@@ -91,7 +91,9 @@ export class WorkPackageQuickinfoMacroComponent implements OnInit {
 
   ngOnInit() {
     const element = this.elementRef.nativeElement as HTMLElement;
-    const id:string = element.dataset.id!;
+    // Prefer `data-display-id`; fall back to `data-id` for legacy
+    // stored markdown emitted before the attribute split.
+    const id:string = element.dataset.displayId ?? element.dataset.id!;
     this.detailed = element.dataset.detailed === 'true';
     this.workPackageHoverCardUrl = this.pathHelper.workPackageHoverCardPath(id);
 

lib/open_project/text_formatting/filters/mention_filter.rb

@@ -75,22 +75,26 @@ 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
+
         case mention.text.count("#")
         when 3
           ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo",
                                                     "",
-                                                    data: { id: work_package.id, detailed: true }
+                                                    data: { id: work_package.id, display_id:, detailed: true }
         when 2
           ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo",
                                                     "",
-                                                    data: { id: work_package.id, detailed: false }
+                                                    data: { id: work_package.id, display_id:, detailed: false }
         else
-          link_to("##{work_package.id}",
-                  work_package_path_or_url(id: work_package.id, only_path: context[:only_path]),
+          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(work_package.id)
+                    hover_card_url: hover_card_work_package_path(display_id)
                   })
         end
       end
@@ -123,11 +127,10 @@ def fallback_text(mention)
       def controller; end
 
       def mention_id(mention)
-        attribute_value = mention.attributes["data-id"]&.value
-
-        id_match = attribute_value&.match(/\d+/)
-
-        id_match ? id_match[0] : nil
+        value = mention.attributes["data-id"]&.value
+        # Reject semantic-shaped data-ids: `PROJ-42` must not silently
+        # resolve to WP id 42 via embedded-digit extraction.
+        value if value&.match?(/\A\d+\z/)
       end
     end
   end

lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb

@@ -53,7 +53,7 @@ def call
 
           render_for_semantic(identifier)
         else
-          # Reject leading-zero shapes like `#0123` that aren't canonical PK strings.
+          # Reject leading-zero shapes like `#0123` that aren't canonical id strings.
           return nil unless WorkPackage::SemanticIdentifier.numeric_id?(identifier)
 
           render_for_numeric(identifier.to_i)
@@ -75,14 +75,15 @@ def detailed?
       end
 
       def render_for_semantic(display_id)
+        # Both quickinfo and plain link need the WP record so the rendered
+        # HTML can carry the record id in `data-id`. Unresolved WP →
+        # literal text rather than a broken reference.
+        wp = OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.work_package_for(display_id)
+        return nil unless wp
+
         if quickinfo?
-          render_work_package_macro(display_id, detailed: detailed?)
+          render_work_package_macro(id: wp.id, display_id: wp.display_id, detailed: detailed?)
         else
-          # Plain link needs the WP record for the `formatted_id` label and
-          # hover-card URL. Cache miss → literal text rather than a broken URL.
-          wp = OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.work_package_for(display_id)
-          return nil unless wp
-
           render_work_package_link(wp, fallback_id: display_id)
         end
       end
@@ -91,19 +92,20 @@ def render_for_numeric(wp_id)
         wp = OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.work_package_for(wp_id)
 
         if quickinfo?
-          # Prefer the resolved WP's display_id so `##1234` typed in semantic
-          # mode renders the user-facing identifier in the editor model.
-          data_id = wp&.display_id || wp_id
-          render_work_package_macro(data_id, detailed: detailed?)
+          # Prefer the resolved WP's identifiers; fall back to the matched
+          # id when no preload is available (classic mode).
+          record_id = wp&.id || wp_id
+          display_id = wp&.display_id || wp_id
+          render_work_package_macro(id: record_id, display_id:, detailed: detailed?)
         else
           render_work_package_link(wp, fallback_id: wp_id)
         end
       end
 
-      def render_work_package_macro(data_id, detailed: false)
+      def render_work_package_macro(id:, display_id:, detailed: false)
         ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo",
                                                   "",
-                                                  data: { id: data_id, detailed: }
+                                                  data: { id:, display_id:, detailed: }
       end
 
       def render_work_package_link(work_package, fallback_id:)

spec/controllers/work_packages/auto_completes_controller_spec.rb

@@ -197,6 +197,45 @@
       end
     end
 
+    describe "displayId JSON contract" do
+      # The CKEditor mention plugin reads `displayId` (camelCase, stringified)
+      # off each entry to insert `#PROJ-7` or `#1234` into the editor and to
+      # build the mention's link URL. The contract must hold in both modes
+      # so the frontend doesn't have to branch on identifier shape.
+      render_views
+
+      let(:expected_values) { work_package1 }
+      let(:json) { response.parsed_body }
+      let(:entry) { json.find { |e| e["id"] == work_package1.id } }
+
+      before do
+        get :index,
+            params: { project_id: project.id, q: work_package1.id },
+            format: :json
+      end
+
+      context "in classic mode",
+              with_flag: { semantic_work_package_ids: false },
+              with_settings: { work_packages_identifier: "classic" } do
+        it "exposes displayId as the stringified numeric id" do
+          expect(entry).to include("displayId" => work_package1.id.to_s)
+        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: "MENTPROJ") }
+
+        before { work_package1.allocate_and_register_semantic_id }
+
+        it "exposes displayId as the semantic identifier string" do
+          expect(entry["displayId"]).to start_with("MENTPROJ-")
+          expect(entry["displayId"]).to be_a(String)
+        end
+      end
+    end
+
     describe "in different projects" do
       let(:project2) do
         create(:project,

spec/features/wysiwyg/mentions_spec.rb

@@ -73,7 +73,7 @@
     end
   end
 
-  let!(:other_work_package) do
+  let!(:mentioned_work_package) do
     create(:work_package, subject: "Other work package", status:, author: user, project:)
   end
 
@@ -182,33 +182,72 @@
     expect(page).to have_css("span", text: "👍")
   end
 
-  it "can autocomplete work packages with different triggers" do
-    # Test # trigger
-    activity_tab.type_comment("##{other_work_package.id}")
-    page.find(".mention-list-item", text: other_work_package.subject, wait: 10).click
-    expect(page).to have_css("a.mention", text: "##{other_work_package.id}")
-    activity_tab.submit_comment
-    activity_tab.expect_journal_notes text: "##{other_work_package.id}"
-
-    # Test ## trigger
-    activity_tab.type_comment("###{other_work_package.id}")
-    page.find(".mention-list-item", text: other_work_package.subject, wait: 10).click
-    expect(page).to have_css(".op-macro-wp-quickinfo-widget")
-    expect(page).to have_css(
-      "opce-macro-wp-quickinfo[data-id='#{other_work_package.id}'][data-detailed='false']"
-    )
-    activity_tab.submit_comment
-    activity_tab.expect_journal_notes text: "NONE ##{other_work_package.id}: #{other_work_package.subject}"
-
-    # Test ### trigger
-    activity_tab.type_comment("####{other_work_package.id}")
-    page.find(".mention-list-item", text: other_work_package.subject, wait: 10).click
-    expect(page).to have_css(".op-macro-wp-quickinfo-widget")
-    expect(page).to have_css(
-      "opce-macro-wp-quickinfo[data-id='#{other_work_package.id}'][data-detailed='true']"
-    )
-    activity_tab.submit_comment
+  # Each consuming context defines `wp_display_id` — the value the user
+  # types after the marker, which appears on the rendered widget as
+  # `data-display-id` — and `wp_label`, the visible link label after the
+  # macro pipeline renders the comment.
+  shared_examples "work package mention with all triggers" do
+    it "can autocomplete work packages with different triggers" do
+      # Test # trigger
+      activity_tab.type_comment("##{wp_display_id}")
+      page.find(".mention-list-item", text: mentioned_work_package.subject, wait: 10).click
+      expect(page).to have_css("a.mention", text: "##{wp_display_id}")
+      activity_tab.submit_comment
+      activity_tab.expect_journal_notes text: wp_label
+
+      # Test ## trigger
+      activity_tab.type_comment("###{wp_display_id}")
+      page.find(".mention-list-item", text: mentioned_work_package.subject, wait: 10).click
+      expect(page).to have_css(".op-macro-wp-quickinfo-widget")
+      expect(page).to have_css(
+        "opce-macro-wp-quickinfo" \
+        "[data-id='#{mentioned_work_package.id}']" \
+        "[data-display-id='#{wp_display_id}']" \
+        "[data-detailed='false']"
+      )
+      activity_tab.submit_comment
+      activity_tab.expect_journal_notes text: "NONE #{wp_label}: #{mentioned_work_package.subject}"
+
+      # Test ### trigger
+      activity_tab.type_comment("####{wp_display_id}")
+      page.find(".mention-list-item", text: mentioned_work_package.subject, wait: 10).click
+      expect(page).to have_css(".op-macro-wp-quickinfo-widget")
+      expect(page).to have_css(
+        "opce-macro-wp-quickinfo" \
+        "[data-id='#{mentioned_work_package.id}']" \
+        "[data-display-id='#{wp_display_id}']" \
+        "[data-detailed='true']"
+      )
+      activity_tab.submit_comment
+
+      activity_tab.expect_journal_notes text: "Some statusNONE #{wp_label}: #{mentioned_work_package.subject}"
+    end
+  end
+
+  context "in classic mode",
+          with_flag: { semantic_work_package_ids: false },
+          with_settings: { work_packages_identifier: "classic" } do
+    let(:wp_display_id) { mentioned_work_package.id }
+    let(:wp_label) { "##{mentioned_work_package.id}" }
+
+    include_examples "work package mention with all triggers"
+  end
+
+  context "in semantic mode",
+          with_flag: { semantic_work_package_ids: true },
+          with_settings: { work_packages_identifier: "semantic" } do
+    let!(:project) do
+      create(:project, :semantic, enabled_module_names: %w[work_package_tracking])
+    end
+    let!(:mentioned_work_package) do
+      create(:work_package, subject: "Other work package", status:, author: user, project:).tap do |wp|
+        wp.allocate_and_register_semantic_id
+        wp.reload
+      end
+    end
+    let(:wp_display_id) { mentioned_work_package.identifier }
+    let(:wp_label) { mentioned_work_package.identifier }
 
-    activity_tab.expect_journal_notes text: "Some statusNONE ##{other_work_package.id}: #{other_work_package.subject}"
+    include_examples "work package mention with all triggers"
   end
 end