diff --git a/src/commonmark/commonmarkdataprocessor.js b/src/commonmark/commonmarkdataprocessor.js index 3ed3388..a4498b5 100644 --- a/src/commonmark/commonmarkdataprocessor.js +++ b/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 `X` 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(' custom element const html = hashes === 1 ? `${ref}` - : `${ref}`; + : `${ref}`; 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; diff --git a/src/mentions/mentions-caster.js b/src/mentions/mentions-caster.js index 4ce6fc2..f5e9df4 100644 --- a/src/mentions/mentions-caster.js +++ b/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 `` 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; } diff --git a/src/mentions/user-mentions.js b/src/mentions/user-mentions.js index cc4f329..ad7d330 100644 --- a/src/mentions/user-mentions.js +++ b/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 => { diff --git a/src/mentions/work-package-mentions.js b/src/mentions/work-package-mentions.js index a26e7a0..ba7e714 100644 --- a/src/mentions/work-package-mentions.js +++ b/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 => { diff --git a/src/plugins/op-macro-wp-quickinfo/op-macro-wp-quickinfo-plugin.js b/src/plugins/op-macro-wp-quickinfo/op-macro-wp-quickinfo-plugin.js index 0379939..9f3725d 100644 --- a/src/plugins/op-macro-wp-quickinfo/op-macro-wp-quickinfo-plugin.js +++ b/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,30 +24,55 @@ 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 `` 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', } ); @@ -52,29 +80,48 @@ export default class OPMacroWpQuickinfoPlugin extends Plugin { 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 `` 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 `` 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 ) ); } ); diff --git a/src/plugins/op-macro-wp-quickinfo/predicate.js b/src/plugins/op-macro-wp-quickinfo/predicate.js new file mode 100644 index 0000000..49f5d1b --- /dev/null +++ b/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); +} diff --git a/tests/commonmark/work-package-refs.test.js b/tests/commonmark/work-package-refs.test.js index 81ea4e8..46a4190 100644 --- a/tests/commonmark/work-package-refs.test.js +++ b/tests/commonmark/work-package-refs.test.js @@ -22,14 +22,14 @@ describe('CommonMarkProcessor', () => { it('upcasts ##6217 to ', () => { testDataProcessor( '##6217', - '

##6217

' + '

##6217

' ); }); it('round-trips inside surrounding text', () => { testDataProcessor( 'see ##6217 here', - '

see ##6217 here

' + '

see ##6217 here

' ); }); }); @@ -38,7 +38,7 @@ describe('CommonMarkProcessor', () => { it('upcasts ###6217 to ', () => { testDataProcessor( '###6217', - '

###6217

' + '

###6217

' ); }); }); @@ -72,5 +72,116 @@ describe('CommonMarkProcessor', () => { ); }); }); + + describe('semantic identifier shorthand', () => { + it('upcasts #PROJ-7 to a ', () => { + testDataProcessor( + '#PROJ-7', + '

#PROJ-7

' + ); + }); + + it('upcasts ##PROJ-7 to ', () => { + testDataProcessor( + '##PROJ-7', + '

##PROJ-7

' + ); + }); + + it('upcasts ###PROJ-7 to ', () => { + testDataProcessor( + '###PROJ-7', + '

###PROJ-7

' + ); + }); + + it('matches longer project identifiers like #MACROPROJ-42', () => { + testDataProcessor( + '#MACROPROJ-42', + '

#MACROPROJ-42

' + ); + }); + + it('matches identifiers with underscores (e.g. #MY_PROJ-1)', () => { + testDataProcessor( + '#MY_PROJ-1', + '

#MY_PROJ-1

' + ); + }); + + describe('boundary handling for semantic shape', () => { + it('does not match mid-word: foo#PROJ-1 stays as text', () => { + testDataProcessor( + 'foo#PROJ-1', + '

foo#PROJ-1

' + ); + }); + + it('does not match if followed by a word char: #PROJ-1abc stays as text', () => { + testDataProcessor( + '#PROJ-1abc', + '

#PROJ-1abc

' + ); + }); + + it('does not match a trailing-dash-only shape: #PROJ- stays as text', () => { + testDataProcessor( + '#PROJ-', + '

#PROJ-

' + ); + }); + + it('does not match lowercase project identifiers: #proj-1 stays as text', () => { + testDataProcessor( + '#proj-1', + '

#proj-1

' + ); + }); + }); + }); + + describe('stored envelope round-trip', () => { + // The two fixtures differ only in attribute order — view + // stringify sorts alphabetically, DOM serialization preserves + // source order. + const inputFor = (text) => + `${text}`; + const viewFor = (text) => + `${text}`; + + it('preserves single-hash envelope', () => { + testDataProcessor( + inputFor('#KSTP-2'), + `

${viewFor('#KSTP-2')}

`, + inputFor('#KSTP-2') + ); + }); + + it('preserves double-hash envelope', () => { + testDataProcessor( + inputFor('##KSTP-2'), + `

${viewFor('##KSTP-2')}

`, + inputFor('##KSTP-2') + ); + }); + + it('preserves triple-hash envelope', () => { + testDataProcessor( + inputFor('###KSTP-2'), + `

${viewFor('###KSTP-2')}

`, + inputFor('###KSTP-2') + ); + }); + + it('collapses a legacy single-attribute envelope to bare data-text', () => { + const legacy = '#6217'; + testDataProcessor( + legacy, + `

${legacy}

`, + '#6217' + ); + }); + }); + }); });