Skip to content

refactor: deduplicate getHTMLSubject() via HTMLSubjectFormatter#2571

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

refactor: deduplicate getHTMLSubject() via HTMLSubjectFormatter#2571
miaulalala wants to merge 1 commit intomasterfrom
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
Copy link
Copy Markdown

cypress Bot commented May 6, 2026

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant