Skip to content

refactor: deduplicate getHTMLSubject() via HTMLSubjectFormatter#2571

Open
miaulalala wants to merge 1 commit into
masterfrom
refactor/html-subject-trait
Open

refactor: deduplicate getHTMLSubject() via HTMLSubjectFormatter#2571
miaulalala wants to merge 1 commit into
masterfrom
refactor/html-subject-trait

Conversation

@miaulalala

Copy link
Copy Markdown
Collaborator

Summary

getHTMLSubject() was copy-pasted identically in MailQueueHandler and DigestSender — any bug fix or change had to be applied twice.

Extracted to a static HTMLSubjectFormatter::format(IEvent): string class. Both callers now delegate to the single implementation; their getHTMLSubject() methods become one-liners.

Also adds four focused unit tests that were previously missing: no-rich-subject fallback (HTML-escaping), file-type parameter (uses path), non-file parameter (uses name), and parameter with a link (renders <a>).

Test plan

  • 4 new HTMLSubjectFormatterTest cases — green
  • Full test suite (332 tests) — green

🤖 Generated with Claude Code

The method was copy-pasted identically in MailQueueHandler and
DigestSender. Extract it to a static HTMLSubjectFormatter::format()
class, add four unit tests covering the no-rich-subject fallback, file
parameters, name parameters, and linked parameters. Both callers now
delegate to the single implementation.

Signed-off-by: Anna Larch <anna@nextcloud.com>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cypress

cypress Bot commented May 6, 2026

Copy link
Copy Markdown

Activity    Run #3709

Run Properties:  status check passed Passed #3709  •  git commit 79d6ad6927: refactor: deduplicate getHTMLSubject() via HTMLSubjectFormatter
Project Activity
Branch Review refactor/html-subject-trait
Run status status check passed Passed #3709
Run duration 01m 57s
Commit git commit 79d6ad6927: refactor: deduplicate getHTMLSubject() via HTMLSubjectFormatter
Committer Anna
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 1
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 9
View all changes introduced in this branch ↗︎
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment