We have a built-in bruteforce protection for built-in users. When users
are being created from LDAP on-the-fly, these limits cannot apply, as we
do not have a user object yet.
Instead, we can provide a more generous throttler to block attempts
Strip a forward-looking aside about future external surfaces in RenderMode;
the invariant is that external surfaces need both absolute URLs and static
rendering. Replace "in practice" with "a coupled set" to drop the soft
hedge.
Drop the lead "wrappers around format_text" sentence on MailFormattingHelper
since the module body already shows the wrapping; the WHY (channel pinning,
extension/helper name parity) is the part worth documenting.
`format_text` accepts `render_mode:` (`:in_app_html`, `:external_html`,
`:external_text`), which resolves the `only_path`, `static_html` and
`plain_text` context flags as a set. External surfaces (mailer HTML
body, future RSS/PDF/webhook) need absolute URLs and static rendering
together; pinning the trio at the public API keeps callers from
forgetting one. Explicit primitive kwargs still override.
`MailFormattingHelper` exposes `format_mail_html` and `format_mail_text`
thin wrappers around `format_text(render_mode:)`. The `_html` / `_text`
suffix matches the `.html.erb` / `.text.erb` template extension so
caller intent stays visible in the view, with no introspection of
`formats`.
The five WorkPackageMailer view sites use the helpers; `_work_package_details`,
`mentioned.html`, `mentioned.text`, `watcher_changed.html`, `watcher_changed.text`
drop the `static_html:`/`only_path:`/`plain_text:` boilerplate.
Same pattern as the static-HTML collapse: the `markdown_as_text` format
symbol was a thin subclass setting a context flag and swapping the filter
list. Replace it with `plain_text: true` on the existing rich formatter,
which now picks between `RICH_FILTERS` and `TEXT_FILTERS` constants based
on the flag. `static_html:` and `plain_text:` now sit as peer options on
one format.
Rename the `as_text` context key to `plain_text` for symmetry with
`static_html`. Update both mailer `.text.erb` views and the two handler
predicates that branch on the flag.
The static-HTML pipeline differs from the rich pipeline only by a
context flag - both share the same filter chain. The dedicated
`Markdown::StaticHtmlFormatter` and `:markdown_as_static_html`
format symbol were pure boilerplate around that one-line override.
Callers now pass `format: :rich, static_html: true` and the matchers
read `context[:static_html]` directly.
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.
Compress the "do we need to load WP records?" condition into
`preload_required?(context)` so the call site reads as intent rather
than a tangle of two unrelated signals. The reasoning (semantic mode
needs row lookup; static-HTML output needs type/subject for the
anchor) moves to a comment on the predicate, where it belongs.
Lift the static-anchor label composition out of LinkHandlers and into
a small Helpers::StaticMacroLabel module so the envelope path
(MentionFilter) and the text-reference path (LinkHandlers) share one
shape — same module called from both, no cross-class reach-through.
Batch the User and Group mention preloads alongside the existing WP
preload so a note with N principals costs one SELECT per type rather
than N. Class lookup now reads from indexed hashes; visibility-gating
stays where it was (at the find for principals, separate from the
label for WPs).
Rename SemanticIdentifier.format → with_hash_prefix; the prior name
was broad enough to invite misuse for arbitrary work-package values.
Override StaticHtmlFormatter#filters explicitly so a future filter
appearing in Formatter#filters is a deliberate decision to apply to
mailer-side rendering, not an automatic one.
Spec coverage: classic-mode quickinfo and inaccessible-WP paths
(symmetric with the existing semantic-mode contexts), a principal
preload N+1 guard, and an anonymous current_user smoke test that
confirms the static-HTML pipeline doesn't raise when invoked without
an authenticated viewer.
Reviewer feedback on cd122f867cc: the `parts << status / type / label`
block was duplicated between the regex-driven (`WorkPackages` link
handler) and envelope-driven (`MentionFilter`) static paths, with no
guard against silent desynchronisation. Centralise the composition on
the link handler and document why the two callers pass different
labels — the regex path preserves the alias-as-matched, the envelope
path normalises to the WP's current formatted_id.
The `##N` and `###N` work-package macros emit JS-hydrated
`<opce-macro-wp-quickinfo>` custom elements, which mail clients
collapse to empty bullets. Introduce a `:markdown_as_static_html`
format that shares the rich filter chain but signals
`context[:as_static_html]` so the matcher and `MentionFilter` emit a
server-rendered anchor — formatted_id, type name, subject, and (for
`###`) status name — closely mirroring the in-app widget once
flattened.
Mailer HTML templates (`mentioned`, `watcher_changed`,
`_work_package_details`) opt into the new format. Invisible WPs still
render as plain-text labels, matching the cross-project visibility
policy.
`ResourceLinksMatcher.build_cache` and
`MentionFilter#preload_work_package_mentions` eager-load `:type` and
`:status` only when `:as_static_html` is set, leaving the default web
path's two-SELECT shape untouched. Classic-mode preload now also runs
under `:as_static_html` so the link handler can resolve type/subject
for `##`/`###`.
Renames the internal flag `context[:plain_text]` to `context[:as_text]`
to restore symmetry with the user-facing `:markdown_as_text` format.
The format runs the full markdown pipeline and then collapses the DOM
to text — it has nothing to do with the existing `:plain` format,
which strips markdown entirely. Moves the formatter under the Markdown
namespace next to the rich-output formatter whose pipeline it mirrors,
and renames the symbol so the relationship is legible from the
formatter_for case clause.
The mention filter previously dropped to the envelope's stored text
when the recipient lacked view permission on the referenced work
package, which left stale identifiers in mailer bodies after a
project rename and diverged from the `#N` text-reference path on
the same render.
Adopts the two-SELECT pattern ResourceLinksMatcher uses for `#N`
references: a single unscoped batched lookup for label resolution
plus a visibility-scoped id pluck for anchor gating. Invisible WPs
render as plain text with the current formatted_id; the per-mention
`WorkPackage.visible.find_by` is gone.
Pairs unscoped label resolution and viewer-scoped link gating in a
WorkPackagePreloadCache instead of two RequestStore keys with a
five-method save/restore protocol. Exposes one `current_cache` reader;
consumers ask the cache directly via `fetch` and `visible?`.
Extracts a `text_only?` predicate in the WP link handler so the
`context[:plain_text]` and invisible-WP guards collapse into a single
call site. `SemanticIdentifier.format` renames its parameter to
reflect that the input may or may not be semantic.
The macro preload was visibility-scoped — references to work packages the
recipient cannot see would fall through to the literal `#43` shape, even
when the same reference rendered as `DCP-1` for an author with full view
permission. Notification recipients saw misleading numeric ids for cross-
project references in journal notes.
Splits label resolution from link gating:
- `ResourceLinksMatcher.build_lookup` now does an unscoped fetch for the
primary identifier and a separate visibility-scoped id pluck. The link
handler reads `visible_to_current_user?` to decide between a navigable
anchor and a plain-text label.
- `UpdateAncestorsService#set_journal_note` writes `#display_id` so new
notes carry the semantic shape at the source; render-time resolution
heals legacy `#N` content for users with view permission.
Tradeoff: a recipient without view permission now sees the WP's semantic
identifier (e.g. `DCP-1`) as plain text rather than `#43`. The reference's
existence was already disclosed by the stored journal text; the project
identifier is the only new piece of information surfaced, and is not
treated as a secret elsewhere in the system (URLs, exports, API).
The `mentioned` and `watcher_changed` text-mailer bodies surfaced raw
journal markdown — numeric `#42` references stayed numeric in semantic
mode, and `<mention>` envelopes leaked as HTML source.
Introduces `:plain_text` as a sibling format inside the existing Plain
module. The filter chain mirrors the markdown pipeline (markdown,
sanitization, mention, pattern-matcher) and finishes with a new
`PlainTextOutputFilter` that collapses the DOM to text. The
`WorkPackages` link handler and `MentionFilter` get plain-text branches
keyed off `context[:plain_text]` so identifier resolution stays in one
place across rich and plain channels.
Closes https://community.openproject.org/wp/74762
* Scope SQL log assertions to target SELECTs in alias-fold-in spec
The N+1 guard was counting the entire QueryRecorder log, which made it
brittle against any incidental Setting/permission query that landed on
the same render path. Switch to scoped greps against the two SELECTs
we actually care about: the work_packages batched query and the
sidecar alias pluck. A regression on either now fails with a clear
message pointing at the offending source.
* Flatten matcher preload wrapping into an iterative fold
The recursive shift-and-recurse shape mutated a duplicated array and
forwarded an anonymous block at every frame, which obscured what the
loop was actually doing: wrap each opt-in matcher's preload hook
around the inner block, first matcher outermost. The iterative form
walks the matcher list once in reverse and rebinds a lambda, so the
nesting order is visible without unwinding a recursion.
* Make the WP preload cache's stringified-key invariant explicit
Both ends of the cache assumed every key was a string, but the
contract lived only in the read site's `identifier.to_s` and an
ambient confidence that the identifier column is text. Normalize at
write time too, swap the safe-navigation lookup for `dig`, and leave
a one-line note at the canonical builder so a future reader doesn't
have to grep the call sites to convince themselves a numeric input
will resolve.
Oliver Günther