Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
f2a2ab0
Use formatted_id and displayId for #N text macros
akabiru Apr 29, 2026
2e92308
Address review feedback on PR 22976
akabiru Apr 29, 2026
1666ccd
Extract PreformattedBlocks; share parse_match; trim WP preload columns
akabiru May 6, 2026
06a5d61
Promote WorkPackage::SemanticIdentifier.semantic_id? to public
akabiru May 6, 2026
74c12f9
Accept semantic work-package identifiers in text macros
akabiru May 6, 2026
1e7a649
Pin PDF export macros to numeric-only references
akabiru May 6, 2026
085ed19
Expose displayId and formattedId on work-package auto_complete API
akabiru May 6, 2026
dd42232
Rebuild vendored CKEditor bundle with semantic-id support
akabiru May 6, 2026
f83cbf9
Rename single-letter parameter to satisfy Naming/MethodParameterName
akabiru May 6, 2026
53986c0
Use named captures in ResourceLinksMatcher.regexp
akabiru May 6, 2026
4566337
Replace history/jargon framing in code comments
akabiru May 6, 2026
06d7e64
Use ActiveRecord::QueryRecorder and shared context in WP macro spec
akabiru May 6, 2026
013b6e1
Use formatted_id in WorkPackage#to_s so autocomplete labels speak sem…
akabiru May 6, 2026
90a3c5b
Use formatted_id in journal cause formatter and link_to_work_package
akabiru May 6, 2026
4090ea2
Rebuild vendored CKEditor bundle for user-mention idNumber fix
akabiru May 6, 2026
d1b87a6
Rebuild CKEditor bundle with patch-package mention-marker fix re-applied
akabiru May 7, 2026
1e59f69
Run WP mention-trigger feature spec in both modes
akabiru May 7, 2026
c066978
Pass id and label as lets to mention-trigger shared example
akabiru May 7, 2026
b58cbfb
Drop unused formattedId from auto_complete JSON response
akabiru May 7, 2026
6aa3699
Resolve semantic identifiers in wiki reverse-link parser
akabiru May 7, 2026
6b31bf6
Stringify displayId in auto_complete payload for APIv3 parity
akabiru May 7, 2026
b8dfb39
Encapsulate canonical-numeric guard as numeric_id?
akabiru May 7, 2026
7647174
Compose semantic-identifier patterns from a single source
akabiru May 7, 2026
8728c0e
Reduce verbose comments
akabiru May 7, 2026
9ef7627
Reject semantic-shaped ids in HashSeparator#applicable?
akabiru May 8, 2026
3de326f
Cap WP identifier preload IN-list at 500
akabiru May 8, 2026
d5a8c5d
Make numeric_id? the exact complement of semantic_id? for Strings
akabiru May 8, 2026
5fda19b
Pin ID_ROUTE_CONSTRAINT against silent widening
akabiru May 8, 2026
fe9bfe3
Cover displayId JSON contract in both modes
akabiru May 8, 2026
6302027
Lock numeric trailing-text behaviour in wiki reverse-link parser
akabiru May 8, 2026
366588c
Document singular String input on where_display_id_in
akabiru May 8, 2026
443cc7c
Specialise WP link handler with hash_trigger? predicate
akabiru May 8, 2026
e51ccd1
Trim verbose contextual comments on WP identifier branch
akabiru May 8, 2026
26975ee
Emit data-id (record id) and data-display-id on work-package mentions
akabiru May 12, 2026
9339d84
Lock wysiwyg autocomplete round-trip across modes
akabiru May 12, 2026
5df1456
Rebuild vendor CKEditor bundle for the mention-contract changes
akabiru May 12, 2026
9001986
Merge pull request #23150 from opf/implementation/mention-filter-wp-d…
akabiru May 12, 2026
0fd9f44
Merge branch 'dev' into implementation/74315-use-formatted-id-and-dis…
akabiru May 12, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/controllers/work_packages/auto_completes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ 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)
work_package.attributes.merge(
"to_s" => work_package.to_s,
"displayId" => work_package.display_id.to_s
)
end
end
end
2 changes: 1 addition & 1 deletion app/helpers/work_packages_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def link_to_work_package(work_package, display_project: false, link_subject: fal
class: link_to_work_package_css_classes(work_package)) do
link_parts = []
link_parts << work_package.type.to_s if work_package.type_id
link_parts << "##{work_package.id}:"
link_parts << "#{work_package.formatted_id}:"
link_parts << content_tag(:span, I18n.t(:label_closed_work_packages), class: "sr-only") if work_package.closed?
link_parts << work_package.subject if link_subject

Expand Down
4 changes: 4 additions & 0 deletions app/models/projects/identifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ module Projects::Identifier
# Classic identifier format: lowercase letters, digits, hyphens, underscores — but not all-numeric.
CLASSIC_IDENTIFIER_FORMAT = /\A(?!\d+\z)[a-z0-9\-_]+\z/

# Unanchored shape of a semantic project identifier ("PROJ", "MY_PROJECT_1").
Comment on lines 38 to +39
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akabiru Would you be open to split these parts into a separate refactoring PR? That way, I can already make use of the regexes here. 🙏

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing, I'll take a stab at that 👍🏾

# Composed into `WorkPackage::SemanticIdentifier::SEMANTIC_ID_PATTERN`.
SEMANTIC_FORMAT = /[A-Z][A-Z0-9_]*/

RESERVED_IDENTIFIERS = %w[new menu queries filters identifier_update_dialog identifier_suggestion].freeze

included do
Expand Down
2 changes: 1 addition & 1 deletion app/models/work_package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def assignable_versions(only_open: true)
end

def to_s
"#{type.name unless type.is_standard} ##{id}: #{subject}"
"#{type.name unless type.is_standard} #{formatted_id}: #{subject}"
end

def infoline(show_standard_type: true)
Expand Down
15 changes: 14 additions & 1 deletion app/models/work_package/exports/macros/links.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,21 @@
module WorkPackage::Exports
module Macros
class WorkPackagesLinkHandler < OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages
# PDF export only handles canonical numeric `#N` references. Semantic and
# leading-zero shapes fall through to literal text to avoid emitting a
# `<mention data-id="0">` (since `"PROJ-1".to_i == 0`). Semantic-id
# support in PDF export is tracked in https://community.openproject.org/wp/74766.
def applicable?
Comment on lines +34 to 38
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be handled in #23138

%w(# ## ###).include?(matcher.sep) && matcher.prefix.blank?
hash_trigger? &&
matcher.prefix.blank? &&
WorkPackage::SemanticIdentifier.numeric_id?(matcher.identifier)
end

# PDF rendering walks Markly nodes directly and bypasses the
# `PatternMatcherFilter` preload pipeline, so the parent's cache-driven
# `call` would miss every reference.
def call
render_link(matcher.identifier.to_i, matcher)
end

def render_link(wp_id, matcher)
Expand Down
27 changes: 26 additions & 1 deletion app/models/work_package/semantic_identifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,16 @@
module WorkPackage::SemanticIdentifier
extend ActiveSupport::Concern

# Semantic-identifier shape ("PROJ-42"). Use this when the numeric branch
# lives on a separate alternation (e.g. text-macro parsers that need
# different trailing-boundary rules per branch); use `ID_ROUTE_CONSTRAINT`
# when both branches share one regex.
SEMANTIC_ID_PATTERN = /#{Projects::Identifier::SEMANTIC_FORMAT.source}-\d+/

# Matches either a numeric ID ("12345") or a semantic identifier ("PROJ-42").
# Used in Rails route constraints so both forms are accepted in URLs.
# The frontend equivalent lives in WP_ID_URL_PATTERN (work-package-id-pattern.ts).
ID_ROUTE_CONSTRAINT = /\d+|[A-Z][A-Z0-9_]*-\d+/
ID_ROUTE_CONSTRAINT = /\d+|#{SEMANTIC_ID_PATTERN.source}/

# Raised when a finder is invoked in a way that cannot resolve a semantic
# identifier — e.g. find_by(id: "PROJ-42") which reduces to a raw SQL
Expand Down Expand Up @@ -79,6 +85,25 @@ def relation
end
end

# True for strings that look like a semantic identifier ("PROJ-42").
# Returns false for non-strings and numeric strings ("123") so they fall
# through to standard PK lookup. The round-trip `to_i.to_s` check (rather
# than a regex) is intentional for performance — don't tighten it.
def self.semantic_id?(value)
value.is_a?(String) && value.strip.to_i.to_s != value.strip
end

# True for canonical numeric IDs: any Integer, or a String that round-trips
# through `to_i.to_s` ("0", "123" — not "0123"). For Strings the exact
# complement of `semantic_id?` so PK-vs-alias routing has a single source.
def self.numeric_id?(value)
case value
when Integer then true
when String then !semantic_id?(value)
else false
end
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
Expand Down
12 changes: 5 additions & 7 deletions app/models/work_package/semantic_identifier/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ def find_by_display_id!(identifier)
# historical alias matches one of the supplied display ids. Numeric and
# semantic strings may be freely mixed; unknown values produce no match
# rather than poisoning the rest of the set.
#
# @param values [Array<String, Integer>, String, Integer] one or many
# display ids. A bare String/Integer is wrapped via Array() so callers
# can pass `where_display_id_in("PROJ-1")` and get a one-element relation.
def where_display_id_in(values)
values = Array(values).map(&:to_s)
return none if values.empty?
Expand Down Expand Up @@ -152,14 +156,8 @@ def first_semantic_value(value)
end
end

# Returns true when value looks like a semantic work package identifier (e.g. "PROJ-42").
# Non-string values (Integer, Hash, nil, Array) and numeric strings ("123", " 456 ")
# return false — these fall through to standard ActiveRecord lookup.
def semantic_id?(value)
return false unless value.is_a?(String)

stripped = value.strip
stripped.to_i.to_s != stripped
WorkPackage::SemanticIdentifier.semantic_id?(value)
end

def find_by_semantic_identifier(identifier)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/vendor/ckeditor/ckeditor.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion frontend/src/vendor/ckeditor/ckeditor.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lib/open_project/journal_formatter/cause.rb
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def related_work_package_changed_message
if related_work_package
I18n.t(
"journals.cause_descriptions.#{cause['type']}",
link: html? ? link_to_work_package(related_work_package, link_subject: true) : "##{related_work_package.id}"
link: html? ? link_to_work_package(related_work_package, link_subject: true) : related_work_package.formatted_id
)

else
Expand Down
23 changes: 13 additions & 10 deletions lib/open_project/text_formatting/filters/mention_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
href_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: href_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: href_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: href_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(href_id)
})
end
end
Expand Down Expand Up @@ -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
Expand Down
29 changes: 23 additions & 6 deletions lib/open_project/text_formatting/filters/pattern_matcher_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@
module OpenProject::TextFormatting
module Filters
class PatternMatcherFilter < HTML::Pipeline::Filter
# Skip text nodes that are within preformatted blocks
PREFORMATTED_BLOCKS = %w(pre code).to_set

class << self
def append_matcher(matcher)
matchers << matcher
Expand All @@ -45,15 +42,35 @@ def matchers
end

def call
with_matcher_preloads(self.class.matchers.dup) { process_text_nodes }
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See -> Text formatting cache flow interactive walkthrough

How PatternMatcherFilter#with_matcher_preloads wraps ResourceLinksMatcher.with_preloaded_resources to build the per-render WP cache, then tears it down.

Image

doc
end

private

# Wraps the per-node loop in each matcher's `with_preloaded_resources`
# hook so matchers can warm per-render caches and save/restore them
# around nested `format_text` calls. Opt-in: matchers without the hook
# are skipped.
def with_matcher_preloads(matchers, &)
matcher = matchers.shift
return yield if matcher.nil?

if matcher.respond_to?(:with_preloaded_resources)
matcher.with_preloaded_resources(doc, context) { with_matcher_preloads(matchers, &) }
else
with_matcher_preloads(matchers, &)
end
end

def process_text_nodes
doc.search(".//text()").each do |node|
next if has_ancestor?(node, PREFORMATTED_BLOCKS)
next if has_ancestor?(node, OpenProject::TextFormatting::PreformattedBlocks::BLOCKS)

self.class.matchers.each do |matcher|
matcher.call(node, doc:, context:)
end
end

doc
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,10 @@ def self.allowed_prefixes
%w(version message project user group document meeting view)
end

##
# Hash-separated object links
# Condition: Separator is '#'
# Condition: Prefix is present, checked to be one of the allowed values
# Prefixed resources (`version#3`, `message#12`) address rows by primary
# key — `numeric_id?` rejects semantic shapes that would `to_i` to 0.
def applicable?
matcher.sep == "#" && valid_prefix? && oid.present?
matcher.sep == "#" && valid_prefix? && WorkPackage::SemanticIdentifier.numeric_id?(matcher.identifier)
end

# Examples:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,51 +31,96 @@
module OpenProject::TextFormatting::Matchers
module LinkHandlers
class WorkPackages < Base
##
# Match work package links.
# Condition: Separator is #|##|###
# Condition: Prefix is nil
# CKEditor `#`-based mention triggers for WP references: `#N` plain link,
# `##N` compact quickinfo, `###N` detailed quickinfo. Distinct from the
# matcher's generic `sep` vocabulary (where `#` *separates* prefix from
# id in `version#3`); here it's a sigil that triggers mention recognition.
# Shared with the PDF-export subclass in `app/models/work_package/exports/macros/links.rb`.
HASH_TRIGGERS = %w[# ## ###].freeze

def applicable?
%w(# ## ###).include?(matcher.sep) && matcher.prefix.nil?
hash_trigger? && matcher.prefix.nil?
end

#
# Examples:
#
# #1234, ##1234, ###1234
# Examples: #1234, ##1234, ###1234, #PROJ-7, ##PROJ-7, ###PROJ-7
def call
wp_id = matcher.identifier.to_i
identifier = matcher.identifier

if WorkPackage::SemanticIdentifier.semantic_id?(identifier)
# Semantic shapes are only meaningful in semantic mode; classic
# instances render the literal text fallback.
return nil unless Setting::WorkPackageIdentifier.semantic_mode_active?

render_for_semantic(identifier)
else
# 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)
end
end

# Ensure that the element was entered numeric,
# prohibits links to things like #0123
return if wp_id.to_s != matcher.identifier
private

def hash_trigger?
HASH_TRIGGERS.include?(matcher.sep)
end

render_link(wp_id, matcher)
def quickinfo?
matcher.sep.length > 1
end

def render_link(wp_id, matcher)
if ["##", "###"].include?(matcher.sep)
render_work_package_macro(wp_id, detailed: (matcher.sep === "###"))
def detailed?
matcher.sep == "###"
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(id: wp.id, display_id: wp.display_id, detailed: detailed?)
else
render_work_package_link(wp_id)
render_work_package_link(wp, fallback_id: display_id)
end
end

private
def render_for_numeric(wp_id)
wp = OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.work_package_for(wp_id)

def render_work_package_macro(wp_id, detailed: false)
if quickinfo?
# 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(id:, display_id:, detailed: false)
ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo",
"",
data: { id: wp_id, detailed: }
data: { id:, display_id:, detailed: }
end

def render_work_package_link(wp_id)
link_to("##{wp_id}",
work_package_path_or_url(id: wp_id, only_path: context[:only_path]),
def render_work_package_link(work_package, fallback_id:)
# Fall back to the bare `#N` shape when no WP is provided (classic mode,
# render path bypassing `PatternMatcherFilter`) rather than running a
# per-link query inside the renderer.
label = work_package&.formatted_id || "##{fallback_id}"
href_id = work_package&.display_id || fallback_id

link_to(label,
work_package_path_or_url(id: href_id, only_path: context[:only_path]),
class: "issue work_package",
data: {
hover_card_trigger_target: "trigger",
hover_card_url: hover_card_work_package_path(wp_id)
hover_card_url: hover_card_work_package_path(href_id)
})
end
end
Expand Down
Loading
Loading