Render work-package mentions with semantic identifiers

src/commonmark/commonmarkdataprocessor.js

@@ -22,7 +22,25 @@ import {isPageBreakNode, PAGE_BREAK_MARKDOWN} from "./utils/page-breaks";
 
 export const originalSrcAttribute = 'data-original-src';
 
-const WP_REF_RE = /^(#{1,3})(\d+)(?!\w)/;
+// `#` / `##` / `###` followed by either a numeric id (`6217`) or a
+// semantic identifier (`PROJ-7`, `MY_PROJ-1`, `MACROPROJ-42`). The
+// trailing `(?!\w)` rejects mid-word continuations like `#PROJ-1abc`.
+// Group 1 is the marker, group 2 is the id.
+const WP_REF_RE = /^(#{1,3})(\d+|[A-Z][A-Z0-9_]*-\d+)(?!\w)/;
+
+// Stored `<mention>X</mention>` envelopes round-trip through markdown-it
+// as three independent `html_inline` tokens; `#`-leading text between
+// the open and close must not be re-promoted by this rule.
+function isInsideStoredMention(tokens) {
+	for (let i = tokens.length - 1; i >= 0; i--) {
+		const token = tokens[i];
+		if (token.type !== 'html_inline') continue;
+		const c = token.content;
+		if (c.startsWith('</mention')) return false;
+		if (c.startsWith('<mention')) return true;
+	}
+	return false;
+}
 
 function workPackageRefInlineRule(state, silent) {
 	const start = state.pos;
@@ -38,6 +56,8 @@ function workPackageRefInlineRule(state, silent) {
 	// If we are in markdown-it silent mode, don't do anything and return true.
 	if (silent) return true;
 
+	if (isInsideStoredMention(state.tokens)) return false;
+
 	const hashes = match[1].length;
 	const id = match[2];
 	const ref = match[0];
@@ -46,7 +66,7 @@ function workPackageRefInlineRule(state, silent) {
 	// ##/### results in <opce-macro-wp-quickinfo> custom element
 	const html = hashes === 1
 		? `<mention class="mention" data-id="${id}" data-type="work_package" data-text="${ref}">${ref}</mention>`
-		: `<opce-macro-wp-quickinfo data-id="${id}" data-detailed="${hashes === 3}">${ref}</opce-macro-wp-quickinfo>`;
+		: `<opce-macro-wp-quickinfo data-id="${id}" data-display-id="${id}" data-detailed="${hashes === 3}">${ref}</opce-macro-wp-quickinfo>`;
 
 	const token = state.push('html_inline', '', 0);
 	token.content = html;
@@ -299,7 +319,7 @@ export default class CommonMarkDataProcessor {
 		turndownService.addRule('workPackageQuickinfo', {
 			filter: (node) => node.nodeName === 'OPCE-MACRO-WP-QUICKINFO',
 			replacement: (_content, node) => {
-				const id = node.getAttribute('data-id') || '';
+				const id = node.getAttribute('data-display-id') || node.getAttribute('data-id') || '';
 				if (!id) return '';
 				const detailed = node.getAttribute('data-detailed') === 'true';
 				return detailed ? `###${id}` : `##${id}`;
@@ -323,8 +343,14 @@ export default class CommonMarkDataProcessor {
 				)
 			},
 			replacement: (_content, node) => {
-				// Serialize work package mentions serialize to plain #ID / ##ID / ###ID
 				if (node.getAttribute('data-type') === 'work_package') {
+					// `data-display-id` signals an autocomplete-picked or
+					// round-tripped envelope; preserve those intact.
+					// Parser-emitted single-hash shorthand has no
+					// `data-display-id` and collapses to bare markdown.
+					if (node.getAttribute('data-display-id')) {
+						return node.outerHTML;
+					}
 					return node.getAttribute('data-text') || node.textContent || '';
 				}
 				return node.outerHTML;

src/mentions/mentions-caster.js

@@ -1,5 +1,6 @@
 import {getPluginContext} from "../plugins/op-context/op-context";
 import { ClickObserver } from '@ckeditor/ckeditor5-engine';
+import { isWorkPackageQuickinfoMention } from '../plugins/op-macro-wp-quickinfo/predicate';
 
 export function MentionCaster( editor ) {
 	const pluginContext = getPluginContext(editor);
@@ -32,22 +33,31 @@ export function MentionCaster( editor ) {
 			model: {
 				key: 'mention',
 				value: viewItem => {
-					const idNumber = viewItem.getAttribute( 'data-id' );
+					const dataId = viewItem.getAttribute( 'data-id' );
+					const dataDisplayId = viewItem.getAttribute( 'data-display-id' );
 					const type = viewItem.getAttribute( 'data-type' );
 					const text = viewItem.getAttribute( 'data-text' );
-					const link = getMentionLink(idNumber, type);
-					// The mention feature expects that the mention attribute value
-					// in the model is a plain object with a set of additional attributes.
-					// In order to create a proper object use the toMentionAttribute() helper method:
-					const mentionAttribute = editor.plugins.get( 'Mention' ).toMentionAttribute( viewItem, {
-						// Pass the properties we'll need for the editing and data downcast.
-						idNumber,
+
+					// Multi-hash work-package mentions are routed to the
+					// quickinfo widget model (`op-macro-wp-quickinfo`) by
+					// that plugin's upcast. Returning `null` here keeps the
+					// mention attribute off the text node so the widget
+					// model is the sole representation.
+					if (isWorkPackageQuickinfoMention(viewItem)) {
+						return null;
+					}
+
+					// `link` populates the editor-view `<a href>` only; the
+					// data downcast doesn't persist it.
+					const link = getMentionLink( dataDisplayId || dataId, type );
+
+					return editor.plugins.get( 'Mention' ).toMentionAttribute( viewItem, {
+						dataId,
+						dataDisplayId,
 						link,
 						text,
 						type,
 					} );
-
-					return mentionAttribute;
 				}
 			},
 			converterPriority: 'high'
@@ -124,15 +134,16 @@ export function MentionCaster( editor ) {
 					return writer.createAttributeElement('span');
 				}
 
-				const element = writer.createAttributeElement(
-					'mention',
-					{
-						'class': 'mention',
-						'data-id': modelAttributeValue.idNumber,
-						'data-type': modelAttributeValue.type,
-						'data-text': modelAttributeValue.text,
-					}
-				);
+				const attrs = {
+					'class': 'mention',
+					'data-id': modelAttributeValue.dataId,
+					'data-type': modelAttributeValue.type,
+					'data-text': modelAttributeValue.text,
+				};
+				if (modelAttributeValue.dataDisplayId) {
+					attrs['data-display-id'] = modelAttributeValue.dataDisplayId;
+				}
+				const element = writer.createAttributeElement('mention', attrs);
 
 				return element;
 			}

src/mentions/user-mentions.js

@@ -37,11 +37,10 @@ export function userMentions(queryText) {
 					const type = mention._type.toLowerCase();
 					const text = `@${mention.name}`;
 					const id = `@${mention.id}`;
-					const idNumber = mention.id;
 					const typeSegment = pluginContext.services.apiV3Service[`${type}s`].segment;
-					const link = `${base}/${typeSegment}/${idNumber}`;
+					const link = `${base}/${typeSegment}/${mention.id}`;
 
-					return {type, id, text, link, idNumber, name: mention.name};
+					return {type, id, text, link, dataId: mention.id, name: mention.name};
 				}));
 			})
 			.catch(error => {

src/mentions/work-package-mentions.js

@@ -3,8 +3,8 @@ import { get } from '@rails/request.js';
 export function workPackageMentions(prefix) {
   return function (query) {
     let editor = this;
-    const url = window.OpenProject.urlRoot + `/work_packages/auto_complete.json`;
-    let base = window.OpenProject.urlRoot + `/work_packages/`;
+    const urlRoot = window.OpenProject.urlRoot;
+    const url = `${urlRoot}/work_packages/auto_complete.json`;
 
     if (editor.config.get("disabledMentions").includes("work_package")) {
       return [];
@@ -15,10 +15,20 @@ export function workPackageMentions(prefix) {
         .then(response => response.json)
         .then(collection => {
           resolve(collection.map(wp => {
-            const id = `${prefix}${wp.id}`;
-            const idNumber = wp.id;
+            const displayId = wp.displayId || wp.id;
+            const markerText = `${prefix}${displayId}`;
 
-            return { id, idNumber, type: "work_package", text: id, name: wp.to_s, link: base + wp.id };
+            // CKEditor's mention feed requires `id` to start with the
+            // marker prefix; it's the model attribute and gates insertion.
+            return {
+              id: markerText,
+              dataId: wp.id,
+              dataDisplayId: displayId,
+              type: "work_package",
+              text: markerText,
+              name: wp.to_s,
+              link: `${urlRoot}/work_packages/${displayId}`,
+            };
           }));
         })
         .catch(error => {

src/plugins/op-macro-wp-quickinfo/op-macro-wp-quickinfo-plugin.js

@@ -1,6 +1,9 @@
 import { Plugin } from '@ckeditor/ckeditor5-core';
 import { Widget, toWidget } from '@ckeditor/ckeditor5-widget';
 
+import { isWorkPackageQuickinfoMention } from './predicate';
+
+const QUICKINFO_MODEL = 'op-macro-wp-quickinfo';
 const QUICKINFO_TAG = 'opce-macro-wp-quickinfo';
 
 // Renders OpenProject's ##/### work-package quickinfo references as inline
@@ -21,60 +24,104 @@ export default class OPMacroWpQuickinfoPlugin extends Plugin {
 		const model = editor.model;
 		const conversion = editor.conversion;
 
-		model.schema.register( 'op-macro-wp-quickinfo', {
+		model.schema.register( QUICKINFO_MODEL, {
 			allowWhere: '$text',
 			isInline: true,
 			isObject: true,
-			allowAttributes: [ 'wpId', 'detailed' ],
+			allowAttributes: [ 'wpId', 'wpDisplayId', 'detailed', 'markerText' ],
 		});
 
 		conversion.for( 'upcast' ).elementToElement( {
 			view: { name: QUICKINFO_TAG },
 			model: ( viewElement, { writer } ) => {
-				const wpId = viewElement.getAttribute( 'data-id' ) || '';
+				const dataId = viewElement.getAttribute( 'data-id' ) || '';
+				const dataDisplayId = viewElement.getAttribute( 'data-display-id' ) || '';
+				const wpDisplayId = dataDisplayId || dataId;
 				const detailed = viewElement.getAttribute( 'data-detailed' ) === 'true';
-				return writer.createElement( 'op-macro-wp-quickinfo', { wpId, detailed } );
+				const attrs = { wpDisplayId, detailed };
+				if (dataId && dataId !== wpDisplayId) {
+					attrs.wpId = dataId;
+				}
+				return writer.createElement( QUICKINFO_MODEL, attrs );
 			},
 			converterPriority: 'high',
 		} );
 
+		// Reopened comments preview as widgets instead of plain links by
+		// routing stored work-package `<mention>` envelopes through the
+		// same widget model their autocomplete-picked counterparts use.
+		conversion.for( 'upcast' ).elementToElement( {
+			view: {
+				name: 'mention',
+				classes: 'mention',
+			},
+			model: ( viewElement, { writer } ) => {
+				if (!isWorkPackageQuickinfoMention(viewElement)) return null;
+				const markerText = viewElement.getAttribute( 'data-text' );
+				const detailed = markerText.startsWith('###');
+				const wpId = viewElement.getAttribute( 'data-id' ) || '';
+				const wpDisplayId = viewElement.getAttribute( 'data-display-id' ) || wpId;
+				return writer.createElement( QUICKINFO_MODEL, { wpId, wpDisplayId, detailed, markerText } );
+			},
+			converterPriority: 'highest',
+		} );
+
 		conversion.for( 'editingDowncast' ).elementToElement( {
-			model: 'op-macro-wp-quickinfo',
+			model: QUICKINFO_MODEL,
 			view: ( modelElement, { writer } ) => {
-				const wpId = modelElement.getAttribute( 'wpId' ) || '';
+				const wpDisplayId = modelElement.getAttribute( 'wpDisplayId' ) || '';
 				const detailed = !!modelElement.getAttribute( 'detailed' );
+				const wpId = modelElement.getAttribute( 'wpId' ) || wpDisplayId;
 
-				// toWidget needs a ContainerElement, so we wrap it in a span
 				const wrapper = writer.createContainerElement( 'span', {
 					class: 'op-macro-wp-quickinfo-widget',
 				} );
 				const raw = writer.createRawElement(
 					QUICKINFO_TAG,
 					{
 						'data-id': wpId,
+						'data-display-id': wpDisplayId,
 						'data-detailed': String(detailed),
 					},
 					() => {},
 				);
 				writer.insert( writer.createPositionAt( wrapper, 0 ), raw );
 
-				return toWidget( wrapper, writer, { label: `#${wpId}` } );
+				return toWidget( wrapper, writer, { label: `#${wpDisplayId}` } );
 			},
 		} );
 
-		// Data view: include the literal ##ID / ###ID inside the element so
-		// turndown's isBlank check doesn't skip the content and parent.
 		conversion.for( 'dataDowncast' ).elementToElement( {
-			model: 'op-macro-wp-quickinfo',
+			model: QUICKINFO_MODEL,
 			view: ( modelElement, { writer } ) => {
-				const wpId = modelElement.getAttribute( 'wpId' ) || '';
+				const wpDisplayId = modelElement.getAttribute( 'wpDisplayId' ) || '';
 				const detailed = !!modelElement.getAttribute( 'detailed' );
+				const wpId = modelElement.getAttribute( 'wpId' );
+				const markerText = modelElement.getAttribute( 'markerText' ) || `${detailed ? '###' : '##'}${wpDisplayId}`;
+
+				// Autocomplete picks carry a `wpId`; source-typed widgets
+				// don't. Autocomplete persists as a `<mention>` envelope;
+				// the shorthand path collapses to bare markdown via turndown.
+				if (wpId) {
+					const envelope = writer.createContainerElement('mention', {
+						'class': 'mention',
+						'data-id': wpId,
+						'data-type': 'work_package',
+						'data-text': markerText,
+						'data-display-id': wpDisplayId,
+					});
+					writer.insert(writer.createPositionAt(envelope, 0), writer.createText(markerText));
+					return envelope;
+				}
+
+				// Inline the literal `##ID` / `###ID` so turndown's isBlank
+				// check doesn't skip the empty element.
 				const container = writer.createContainerElement( QUICKINFO_TAG, {
-					'data-id': wpId,
+					'data-id': wpId || wpDisplayId,
+					'data-display-id': wpDisplayId,
 					'data-detailed': String(detailed),
 				} );
-				const ref = (detailed ? '###' : '##') + wpId;
-				writer.insert( writer.createPositionAt( container, 0 ), writer.createText( ref ) );
+				writer.insert( writer.createPositionAt( container, 0 ), writer.createText( markerText ) );
 				return container;
 			},
 		} );
@@ -85,7 +132,9 @@ export default class OPMacroWpQuickinfoPlugin extends Plugin {
 		const mentionCommand = editor.commands.get( 'mention' );
 		if (!mentionCommand) return;
 
-		// Take over ##/### work_package mentions as a widget
+		// Take over ##/### work_package mentions as a widget; the data
+		// downcast chooses bare quickinfo or `<mention>` envelope at save
+		// time based on whether the id matches the displayed identifier.
 		mentionCommand.on( 'execute', ( evt, args ) => {
 			const opts = args && args[0];
 			if (!opts || !opts.mention) return;
@@ -97,14 +146,18 @@ export default class OPMacroWpQuickinfoPlugin extends Plugin {
 			evt.stop();
 
 			const detailed = marker === '###';
-			const wpId = String(opts.mention.idNumber);
+			const wpDisplayId = String(opts.mention.dataDisplayId);
+			const wpId = opts.mention.dataId != null ? String(opts.mention.dataId) : null;
+			const markerText = opts.mention.text || `${marker}${wpDisplayId}`;
 
 			editor.model.change( writer => {
 				const range = opts.range || editor.model.document.selection.getFirstRange();
 				if (range) {
 					writer.remove( range );
 				}
-				const el = writer.createElement( 'op-macro-wp-quickinfo', { wpId, detailed } );
+				const attrs = { wpDisplayId, detailed, markerText };
+				if (wpId) attrs.wpId = wpId;
+				const el = writer.createElement( QUICKINFO_MODEL, attrs );
 				editor.model.insertContent( el, editor.model.document.selection );
 				writer.setSelection( writer.createPositionAfter( el ) );
 			} );

src/plugins/op-macro-wp-quickinfo/predicate.js

@@ -0,0 +1,10 @@
+// Used by the quickinfo widget upcast (to claim the element) and the
+// mention caster (to defer). One source of truth keeps the two in sync.
+
+const QUICKINFO_MARKER_RE = /^#{2,3}/;
+
+export function isWorkPackageQuickinfoMention(viewElement) {
+	if (viewElement.getAttribute('data-type') !== 'work_package') return false;
+	const text = viewElement.getAttribute('data-text');
+	return !!text && QUICKINFO_MARKER_RE.test(text);
+}