From 1550af87cd341dfd81c8338b82464123e5cb0510 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Fri, 14 Jun 2024 17:35:59 +0200
Subject: [PATCH] fix(files): Properly handle denied ownership transfers

When the receiver denies the transfer the notification handler was missing,
so no notification was created for the transfer owner.

But also the internal notification was created two times:
1. When rejecting the transfer
2. By the reject function when dismissing the notification

This is fixed by only relying on the dismiss function.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 .../TransferOwnershipController.php           | 14 +-----
 apps/files/lib/Notification/Notifier.php      | 49 ++++++++++++-------
 2 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/apps/files/lib/Controller/TransferOwnershipController.php b/apps/files/lib/Controller/TransferOwnershipController.php
index e5bce7946e330..924c200f37555 100644
--- a/apps/files/lib/Controller/TransferOwnershipController.php
+++ b/apps/files/lib/Controller/TransferOwnershipController.php
@@ -189,20 +189,10 @@ public function reject(int $id): DataResponse {
 			->setObject('transfer', (string)$id);
 		$this->notificationManager->markProcessed($notification);
 
-		$notification = $this->notificationManager->createNotification();
-		$notification->setUser($transferOwnership->getSourceUser())
-			->setApp($this->appName)
-			->setDateTime($this->timeFactory->getDateTime())
-			->setSubject('transferownershipRequestDenied', [
-				'sourceUser' => $transferOwnership->getSourceUser(),
-				'targetUser' => $transferOwnership->getTargetUser(),
-				'nodeName' => $transferOwnership->getNodeName()
-			])
-			->setObject('transfer', (string)$transferOwnership->getId());
-		$this->notificationManager->notify($notification);
-
 		$this->mapper->delete($transferOwnership);
 
+		// A "request denied" notification will be created by Notifier::dismissNotification
+
 		return new DataResponse([], Http::STATUS_OK);
 	}
 }
diff --git a/apps/files/lib/Notification/Notifier.php b/apps/files/lib/Notification/Notifier.php
index 92a0c4f909097..b9cda6c907abf 100644
--- a/apps/files/lib/Notification/Notifier.php
+++ b/apps/files/lib/Notification/Notifier.php
@@ -69,23 +69,15 @@ public function prepare(INotification $notification, string $languageCode): INot
 			throw new \InvalidArgumentException('Unhandled app');
 		}
 
-		if ($notification->getSubject() === 'transferownershipRequest') {
-			return $this->handleTransferownershipRequest($notification, $languageCode);
-		}
-		if ($notification->getSubject() === 'transferOwnershipFailedSource') {
-			return $this->handleTransferOwnershipFailedSource($notification, $languageCode);
-		}
-		if ($notification->getSubject() === 'transferOwnershipFailedTarget') {
-			return $this->handleTransferOwnershipFailedTarget($notification, $languageCode);
-		}
-		if ($notification->getSubject() === 'transferOwnershipDoneSource') {
-			return $this->handleTransferOwnershipDoneSource($notification, $languageCode);
-		}
-		if ($notification->getSubject() === 'transferOwnershipDoneTarget') {
-			return $this->handleTransferOwnershipDoneTarget($notification, $languageCode);
-		}
-
-		throw new \InvalidArgumentException('Unhandled subject');
+		return match($notification->getSubject()) {
+			'transferownershipRequest' => $this->handleTransferownershipRequest($notification, $languageCode),
+			'transferownershipRequestDenied' => $this->handleTransferOwnershipRequestDenied($notification, $languageCode),
+			'transferOwnershipFailedSource' => $this->handleTransferOwnershipFailedSource($notification, $languageCode),
+			'transferOwnershipFailedTarget' => $this->handleTransferOwnershipFailedTarget($notification, $languageCode),
+			'transferOwnershipDoneSource' => $this->handleTransferOwnershipDoneSource($notification, $languageCode),
+			'transferOwnershipDoneTarget' => $this->handleTransferOwnershipDoneTarget($notification, $languageCode),
+			default => throw new \InvalidArgumentException('Unhandled subject')
+		};
 	}
 
 	public function handleTransferownershipRequest(INotification $notification, string $languageCode): INotification {
@@ -144,6 +136,29 @@ public function handleTransferownershipRequest(INotification $notification, stri
 		return $notification;
 	}
 
+	public function handleTransferOwnershipRequestDenied(INotification $notification, string $languageCode): INotification {
+		$l = $this->l10nFactory->get('files', $languageCode);
+		$param = $notification->getSubjectParameters();
+
+		$targetUser = $this->getUser($param['targetUser']);
+		$notification->setRichSubject($l->t('Ownership transfer denied'))
+			->setRichMessage(
+				$l->t('Your ownership transfer of {path} was denied by {user}.'),
+				[
+					'path' => [
+						'type' => 'highlight',
+						'id' => $param['targetUser'] . '::' . $param['nodeName'],
+						'name' => $param['nodeName'],
+					],
+					'user' => [
+						'type' => 'user',
+						'id' => $targetUser->getUID(),
+						'name' => $targetUser->getDisplayName(),
+					],
+				]);
+		return $notification;
+	}
+
 	public function handleTransferOwnershipFailedSource(INotification $notification, string $languageCode): INotification {
 		$l = $this->l10nFactory->get('files', $languageCode);
 		$param = $notification->getSubjectParameters();