From da8c3806564d2ee42a8570031ba57e640ffac040 Mon Sep 17 00:00:00 2001
From: Anna Larch <anna@nextcloud.com>
Date: Thu, 7 May 2026 00:28:22 +0200
Subject: [PATCH] refactor: deduplicate getHTMLSubject() via
 HTMLSubjectFormatter

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>
---
 lib/DigestSender.php               | 23 +-----------
 lib/HTMLSubjectFormatter.php       | 37 ++++++++++++++++++++
 lib/MailQueueHandler.php           | 23 +-----------
 tests/HTMLSubjectFormatterTest.php | 56 ++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 44 deletions(-)
 create mode 100644 lib/HTMLSubjectFormatter.php
 create mode 100644 tests/HTMLSubjectFormatterTest.php

diff --git a/lib/DigestSender.php b/lib/DigestSender.php
index 0dfe4d380..963afa996 100644
--- a/lib/DigestSender.php
+++ b/lib/DigestSender.php
@@ -220,27 +220,6 @@ public function sendDigestForUser(IUser $user, int $now, string $timezone, strin
 	 * @return string
 	 */
 	protected function getHTMLSubject(IEvent $event): string {
-		if ($event->getRichSubject() === '') {
-			return htmlspecialchars($event->getParsedSubject());
-		}
-
-		$placeholders = $replacements = [];
-		foreach ($event->getRichSubjectParameters() as $placeholder => $parameter) {
-			$placeholders[] = '{' . $placeholder . '}';
-
-			if ($parameter['type'] === 'file') {
-				$replacement = (string)$parameter['path'];
-			} else {
-				$replacement = (string)$parameter['name'];
-			}
-
-			if (isset($parameter['link'])) {
-				$replacements[] = '<a href="' . $parameter['link'] . '">' . htmlspecialchars($replacement) . '</a>';
-			} else {
-				$replacements[] = '<strong>' . htmlspecialchars($replacement) . '</strong>';
-			}
-		}
-
-		return str_replace($placeholders, $replacements, $event->getRichSubject());
+		return HTMLSubjectFormatter::format($event);
 	}
 }
diff --git a/lib/HTMLSubjectFormatter.php b/lib/HTMLSubjectFormatter.php
new file mode 100644
index 000000000..b9c480b7d
--- /dev/null
+++ b/lib/HTMLSubjectFormatter.php
@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OCA\Activity;
+
+use OCP\Activity\IEvent;
+
+class HTMLSubjectFormatter {
+	public static function format(IEvent $event): string {
+		if ($event->getRichSubject() === '') {
+			return htmlspecialchars($event->getParsedSubject());
+		}
+
+		$placeholders = $replacements = [];
+		foreach ($event->getRichSubjectParameters() as $placeholder => $parameter) {
+			$placeholders[] = '{' . $placeholder . '}';
+
+			$replacement = $parameter['type'] === 'file'
+				? (string)$parameter['path']
+				: (string)$parameter['name'];
+
+			if (isset($parameter['link'])) {
+				$replacements[] = '<a href="' . $parameter['link'] . '">' . htmlspecialchars($replacement) . '</a>';
+			} else {
+				$replacements[] = '<strong>' . htmlspecialchars($replacement) . '</strong>';
+			}
+		}
+
+		return str_replace($placeholders, $replacements, $event->getRichSubject());
+	}
+}
diff --git a/lib/MailQueueHandler.php b/lib/MailQueueHandler.php
index 46be003e4..b089f0c48 100644
--- a/lib/MailQueueHandler.php
+++ b/lib/MailQueueHandler.php
@@ -379,28 +379,7 @@ function ($event) use ($timezone, $l) {
 	}
 
 	protected function getHTMLSubject(IEvent $event): string {
-		if ($event->getRichSubject() === '') {
-			return htmlspecialchars($event->getParsedSubject());
-		}
-
-		$placeholders = $replacements = [];
-		foreach ($event->getRichSubjectParameters() as $placeholder => $parameter) {
-			$placeholders[] = '{' . $placeholder . '}';
-
-			if ($parameter['type'] === 'file') {
-				$replacement = (string)$parameter['path'];
-			} else {
-				$replacement = (string)$parameter['name'];
-			}
-
-			if (isset($parameter['link'])) {
-				$replacements[] = '<a href="' . $parameter['link'] . '">' . htmlspecialchars($replacement) . '</a>';
-			} else {
-				$replacements[] = '<strong>' . htmlspecialchars($replacement) . '</strong>';
-			}
-		}
-
-		return str_replace($placeholders, $replacements, $event->getRichSubject());
+		return HTMLSubjectFormatter::format($event);
 	}
 
 	/**
diff --git a/tests/HTMLSubjectFormatterTest.php b/tests/HTMLSubjectFormatterTest.php
new file mode 100644
index 000000000..321e0c0cc
--- /dev/null
+++ b/tests/HTMLSubjectFormatterTest.php
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OCA\Activity\Tests;
+
+use OCA\Activity\HTMLSubjectFormatter;
+use OCP\Activity\IEvent;
+use PHPUnit\Framework\MockObject\MockObject;
+
+class HTMLSubjectFormatterTest extends TestCase {
+	private function getEvent(string $richSubject, array $richParams, string $parsedSubject): IEvent&MockObject {
+		$event = $this->createMock(IEvent::class);
+		$event->method('getRichSubject')->willReturn($richSubject);
+		$event->method('getRichSubjectParameters')->willReturn($richParams);
+		$event->method('getParsedSubject')->willReturn($parsedSubject);
+		return $event;
+	}
+
+	public function testNoRichSubjectFallsToParsed(): void {
+		$event = $this->getEvent('', [], 'Plain <subject>');
+		$this->assertSame('Plain &lt;subject&gt;', HTMLSubjectFormatter::format($event));
+	}
+
+	public function testFileParameterUsesPath(): void {
+		$event = $this->getEvent(
+			'File {file} was changed',
+			['file' => ['type' => 'file', 'path' => '/my/file.txt', 'name' => 'file.txt']],
+			'',
+		);
+		$this->assertSame('File <strong>/my/file.txt</strong> was changed', HTMLSubjectFormatter::format($event));
+	}
+
+	public function testNonFileParameterUsesName(): void {
+		$event = $this->getEvent(
+			'{user} shared',
+			['user' => ['type' => 'user', 'name' => 'Alice & Bob']],
+			'',
+		);
+		$this->assertSame('<strong>Alice &amp; Bob</strong> shared', HTMLSubjectFormatter::format($event));
+	}
+
+	public function testParameterWithLinkRendersAnchor(): void {
+		$event = $this->getEvent(
+			'See {file}',
+			['file' => ['type' => 'file', 'path' => 'report.pdf', 'name' => 'report.pdf', 'link' => 'https://example.com/report.pdf']],
+			'',
+		);
+		$this->assertSame('See <a href="https://example.com/report.pdf">report.pdf</a>', HTMLSubjectFormatter::format($event));
+	}
+}