Skip to content

Commit 2ca379f

Browse files
committed
Trim verbose comments
Strip explanatory paragraphs and cross-reference jargon ("Mirrors X", "Shared with Y", etc.) from comments introduced on this branch. Keep only the WHY — the parts of the design intent that aren't already evident from method names and short bodies. Pre-existing comments on methods this branch didn't author stay as-is.
1 parent 23d52fc commit 2ca379f

7 files changed

Lines changed: 38 additions & 102 deletions

File tree

lib/open_project/text_formatting/filters/mention_filter.rb

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,11 @@ def call
4949

5050
private
5151

52-
# Doc-level preload so a note with N user/group/WP mentions costs
53-
# a bounded number of SELECTs rather than one per mention. WP
54-
# mentions need the unscoped record (label) plus a separate
55-
# visibility id pluck (anchor-vs-text gating); principals fold the
56-
# two concerns into a single visibility-scoped fetch since
57-
# invisible principals have no plain-text render path — they fall
58-
# back to the literal envelope text instead.
52+
# WP labels resolve regardless of viewer (so an inaccessible WP
53+
# still renders its current formatted_id); a separate id pluck
54+
# gates anchor-vs-text. Principals collapse the two concerns into
55+
# one visibility-scoped fetch — invisible users and groups fall
56+
# back to the literal envelope text.
5957
def preload_mentions
6058
preload_work_package_mentions
6159
preload_principal_mentions
@@ -70,8 +68,6 @@ def preload_work_package_mentions
7068
end
7169

7270
scope = WorkPackage.where(id: ids)
73-
# Static-HTML channels need `type` and `status` to render
74-
# quickinfo envelopes as anchors instead of `<opce-*>` widgets.
7571
scope = scope.includes(:type, :status) if context[:as_static_html]
7672
@mentioned_work_packages = scope.index_by(&:id)
7773
@visible_mentioned_ids = WorkPackage.visible.where(id: ids).pluck(:id).to_set
@@ -118,21 +114,15 @@ def group_mention(group)
118114
def work_package_mention(work_package, mention)
119115
return Nokogiri::XML::Text.new(work_package.formatted_id, mention.document) if text_only?(work_package)
120116

121-
# Match the label and URL convention used for `#N` text references
122-
# elsewhere in the markdown pipeline.
123117
case mention.text.count("#")
124118
when 3 then work_package_quickinfo(work_package, detailed: true)
125119
when 2 then work_package_quickinfo(work_package, detailed: false)
126120
else work_package_link(work_package)
127121
end
128122
end
129123

130-
# Plain-text channels and inaccessible WPs both render the
131-
# `formatted_id` without an anchor or quickinfo widget — the
132-
# latter would resolve to a hover-card endpoint the recipient
133-
# can't reach. Mirrors `LinkHandlers::WorkPackages#text_only?`,
134-
# which gates the matching decision for `#N` text references; keep
135-
# the two in sync.
124+
# The hover-card endpoint a quickinfo would link to is unreachable
125+
# for plain-text recipients and for viewers without view permission.
136126
def text_only?(work_package)
137127
context[:as_text] || @visible_mentioned_ids.exclude?(work_package.id)
138128
end
@@ -147,11 +137,8 @@ def work_package_quickinfo(work_package, detailed:)
147137
detailed: }
148138
end
149139

150-
# Static fallback shared with the PatternMatcherFilter's `##`/`###`
151-
# path so envelope-driven and text-driven references render the same
152-
# shape in channels that cannot hydrate the custom element. Always
153-
# uses the WP's current `formatted_id`, normalising any historical
154-
# alias the envelope may have been authored with.
140+
# Uses the WP's current `formatted_id` rather than the envelope text,
141+
# so a renamed identifier doesn't leave a stale label in the mailer.
155142
def work_package_static_macro(work_package, detailed:)
156143
label = OpenProject::TextFormatting::Helpers::StaticMacroLabel
157144
.call(work_package, label: work_package.formatted_id, detailed:)
@@ -172,12 +159,6 @@ def work_package_link(work_package)
172159
})
173160
end
174161

175-
# WP label resolution is unscoped (preloaded above); visibility is
176-
# gated separately in `text_only?` so an inaccessible WP renders
177-
# its current formatted_id rather than the envelope text the
178-
# author originally typed. Principals stay visibility-gated at the
179-
# preload — there's no equivalent text-only render path for users
180-
# or groups, so invisible principals fall back to the literal text.
181162
def class_from_mention(mention)
182163
id = mention_id(mention)&.to_i
183164
case mention.attributes["data-type"].value

lib/open_project/text_formatting/filters/plain_text_output_filter.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@
3030

3131
module OpenProject::TextFormatting
3232
module Filters
33-
# Final stage of the plain-text pipeline. Earlier filters resolve
34-
# mentions and macros to their text-mode shapes (driven by
35-
# `context[:as_text]`); this stage collapses any remaining markup
36-
# so the pipeline output is suitable for `text/plain` bodies.
33+
# Final stage of the `markdown_as_text` pipeline — strips remaining
34+
# markup so the output is safe for `text/plain` bodies.
3735
class PlainTextOutputFilter < HTML::Pipeline::Filter
3836
def call
3937
doc.text

lib/open_project/text_formatting/formats/markdown/static_html_formatter.rb

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,10 @@
3030

3131
module OpenProject::TextFormatting::Formats
3232
module Markdown
33-
# Static-HTML sibling of `Markdown::Formatter`. Shares the same filter
34-
# chain so identifier resolution, mention handling, and link rendering
35-
# stay consistent, but signals `context[:as_static_html]` so matchers
36-
# and filters emit server-rendered anchors in place of JS-hydrated
37-
# custom elements. Intended for channels that cannot run JS — HTML
38-
# mailers, server-side previews, archival exports — where dynamic
39-
# widgets would collapse to empty placeholders.
33+
# Inherits `Markdown::Formatter`'s filter chain but signals
34+
# `context[:as_static_html]` so matchers emit server-rendered
35+
# anchors instead of JS-hydrated custom elements. For channels
36+
# that can't run JS — HTML mailers, server-side previews.
4037
class StaticHtmlFormatter < Formatter
4138
def initialize(context)
4239
super(context.merge(as_static_html: true))

lib/open_project/text_formatting/formats/markdown/text_formatter.rb

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,9 @@
3030

3131
module OpenProject::TextFormatting::Formats
3232
module Markdown
33-
# Text-output sibling of `Markdown::Formatter`. Shares the matcher and
34-
# mention pipeline with the rich renderer so identifier resolution
35-
# stays consistent across channels, then collapses the final DOM to
36-
# text via `PlainTextOutputFilter`. Intended for plain/text mailers
37-
# and other channels where HTML would be a foreign body.
33+
# Runs the matcher and mention pipeline, then collapses the DOM to
34+
# text via `PlainTextOutputFilter`. For `text/plain` mailers and
35+
# other channels where HTML would be a foreign body.
3836
class TextFormatter < OpenProject::TextFormatting::Formats::BaseFormatter
3937
def initialize(context)
4038
super(context.merge(as_text: true))

lib/open_project/text_formatting/helpers/static_macro_label.rb

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,9 @@
3030

3131
module OpenProject::TextFormatting
3232
module Helpers
33-
# Composes the anchor text for a work-package quickinfo macro rendered
34-
# in a static-HTML channel (mailers, server-side previews) — channels
35-
# that cannot hydrate the JS-driven `<opce-macro-wp-quickinfo>` widget
36-
# and so flatten the macro to an `<a>` whose text must carry enough
37-
# context for a reader to recognise the reference: type, optional
38-
# status, the identifier label, and the subject.
39-
#
40-
# Shared between the text-reference path (`LinkHandlers::WorkPackages`)
41-
# and the envelope path (`Filters::MentionFilter`) so both render the
42-
# same shape for the same WP.
33+
# Anchor text for the static-HTML form of a WP quickinfo macro:
34+
# `[status ]type label: subject`. Used in channels (HTML mailers,
35+
# server-side previews) that can't hydrate the `<opce-*>` widget.
4336
module StaticMacroLabel
4437
def self.call(work_package, label:, detailed:)
4538
parts = []

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,8 @@ def render_work_package_macro(work_package:, fallback_id:, detailed: false)
111111
data: { id:, display_id:, detailed: }
112112
end
113113

114-
# Static fallback for channels that cannot hydrate the quickinfo
115-
# custom element. Composes type, optional status, label, and subject
116-
# into the anchor; unresolved references collapse to the bare label.
117-
# The label is the matched identifier (potentially a historical alias)
118-
# to preserve what the author wrote — the `<mention>` envelope path
119-
# in `MentionFilter` instead normalises to the WP's current
120-
# `formatted_id`.
114+
# The label keeps what the author wrote (possibly a historical
115+
# alias) so the rendered text matches the source markdown.
121116
def render_static_work_package_macro(work_package, label, detailed:)
122117
return label unless work_package
123118

@@ -144,12 +139,8 @@ def render_work_package_link(work_package, fallback_id:)
144139
})
145140
end
146141

147-
# Plain-text channels and inaccessible WPs both render the label
148-
# without an anchor or quickinfo. Visibility is checked only when a
149-
# WP was preloaded — a nil work_package means a classic-mode render
150-
# or an unresolved reference, neither of which needs gating. Mirrors
151-
# `MentionFilter#text_only?`, which gates the same decision for
152-
# `<mention>` envelopes; keep the two in sync.
142+
# A nil WP means classic mode skipped the preload, or the reference
143+
# didn't resolve — neither case needs visibility gating.
153144
def text_only?(work_package)
154145
context[:as_text] || (work_package && !preload_cache.visible?(work_package.id))
155146
end

lib/open_project/text_formatting/matchers/resource_links_matcher.rb

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,9 @@ module Matchers
6969
# identifier:version:1.0.0
7070
# identifier:source:some/file
7171
class ResourceLinksMatcher < RegexMatcher
72-
# Per-render preload state shared between the matcher and the link
73-
# handler. Pairs unscoped label resolution (`lookup`) with viewer-scoped
74-
# link gating (`visible_ids`) so a single render computes both with a
75-
# bounded number of round-trips and renders consistent labels for
76-
# invisible WPs.
72+
# Unscoped label resolution (`lookup`) paired with viewer-scoped
73+
# link gating (`visible_ids`), so the link handler renders the
74+
# same label for everyone and decides anchor-vs-text per viewer.
7775
class WorkPackagePreloadCache
7876
attr_reader :lookup, :visible_ids
7977

@@ -90,8 +88,7 @@ def visible?(work_package_id)
9088
visible_ids.include?(work_package_id)
9189
end
9290

93-
# Per-process frozen singleton returned when no preload is in
94-
# scope — not a factory; callers must not mutate it.
91+
# Frozen singleton, not a factory — callers must not mutate it.
9592
EMPTY = new(lookup: {}.freeze, visible_ids: Set.new.freeze).freeze
9693
end
9794

@@ -156,16 +153,13 @@ def self.link_handlers
156153
]
157154
end
158155

159-
# The active preload cache for the current render, or an empty
160-
# singleton when no preload is in scope (classic mode, no `#N`
161-
# references, or rendering outside `with_preloaded_resources`).
162156
def self.current_cache
163157
RequestStore.store[CACHE_KEY] || WorkPackagePreloadCache::EMPTY
164158
end
165159

166-
# Doc-level preload called by `PatternMatcherFilter`. Save/restores
167-
# the cache so a nested `format_text` (e.g. custom-field formatter
168-
# re-entering the pipeline) doesn't clobber the outer render.
160+
# Save/restore so a nested `format_text` (e.g. a custom-field
161+
# formatter re-entering the pipeline) doesn't clobber the outer
162+
# render's cache.
169163
def self.with_preloaded_resources(doc, context)
170164
previous = RequestStore.store[CACHE_KEY]
171165
return yield unless preload_required?(context)
@@ -179,12 +173,8 @@ def self.with_preloaded_resources(doc, context)
179173
RequestStore.store[CACHE_KEY] = previous
180174
end
181175

182-
# Two channels need the WP record at render time: semantic mode (to
183-
# resolve `PROJ-7` to a row) and static-HTML output (to compose the
184-
# type/subject anchor of a quickinfo macro). Classic-mode rich HTML
185-
# and the as-text channel both render from the matched id alone —
186-
# the latter short-circuits on `text_only?` before consulting the
187-
# cache.
176+
# Semantic mode needs the row to map `PROJ-7` to an id; static-HTML
177+
# output needs `type`/`subject` to compose the quickinfo anchor.
188178
def self.preload_required?(context)
189179
Setting::WorkPackageIdentifier.semantic? || context[:as_static_html]
190180
end
@@ -215,22 +205,10 @@ def self.extract_work_package_identifier(match)
215205
identifier
216206
end
217207

218-
# Label resolution is unscoped: a reference to an inaccessible
219-
# work package still renders as its semantic identifier (e.g.
220-
# `DCP-1`), just without a navigable anchor. Visibility is
221-
# captured separately so the link handler can pick anchor-vs-text
222-
# per WP without re-querying.
223-
#
224-
# Two SELECTs in the common case: one unscoped fetch by identifier,
225-
# one visibility-scoped id pluck. A third targeted SELECT fires
226-
# for historical aliases — the loaded row carries only the current
227-
# identifier, so unmapped inputs are filled in from
228-
# `WorkPackageSemanticAlias`. Static-HTML channels also eager-load
229-
# `:type` and `:status` so the link handler can render the
230-
# static-anchor variant of `##`/`###` macros without N+1 queries —
231-
# those associations are the metadata a reader needs to recognise a
232-
# WP reference flattened to text. Anything beyond that (project,
233-
# versions, custom fields) stays out of this preload.
208+
# Two SELECTs in the common case (unscoped fetch + visibility id
209+
# pluck), a third when historical aliases need resolving. Static-
210+
# HTML output additionally needs `:type` and `:status` to compose
211+
# the anchor for `##`/`###` macros.
234212
def self.build_cache(identifiers, context = {})
235213
scope = WorkPackage.where_display_id_in(*identifiers)
236214
scope = if context[:as_static_html]

0 commit comments

Comments
 (0)