From 443505206ddf1226e3be7ee8ff8c2f736b72f2b3 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Wed, 16 Oct 2024 15:01:00 +0200 Subject: [PATCH] fix: dismiss notification only after transfer bg job created - do not create 'denied' notification if bg job exists Signed-off-by: Maksim Sukharev --- .../Controller/TransferOwnershipController.php | 10 +++++----- apps/files/lib/Notification/Notifier.php | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/apps/files/lib/Controller/TransferOwnershipController.php b/apps/files/lib/Controller/TransferOwnershipController.php index eec654bd678d1..43648528b50db 100644 --- a/apps/files/lib/Controller/TransferOwnershipController.php +++ b/apps/files/lib/Controller/TransferOwnershipController.php @@ -160,11 +160,6 @@ public function accept(int $id): DataResponse { return new DataResponse([], Http::STATUS_FORBIDDEN); } - $notification = $this->notificationManager->createNotification(); - $notification->setApp('files') - ->setObject('transfer', (string)$id); - $this->notificationManager->markProcessed($notification); - $newTransferOwnership = new TransferOwnershipEntity(); $newTransferOwnership->setNodeName($transferOwnership->getNodeName()); $newTransferOwnership->setFileId($transferOwnership->getFileId()); @@ -176,6 +171,11 @@ public function accept(int $id): DataResponse { 'id' => $newTransferOwnership->getId(), ]); + $notification = $this->notificationManager->createNotification(); + $notification->setApp('files') + ->setObject('transfer', (string)$id); + $this->notificationManager->markProcessed($notification); + return new DataResponse([], Http::STATUS_OK); } diff --git a/apps/files/lib/Notification/Notifier.php b/apps/files/lib/Notification/Notifier.php index caf707cc6e38e..d73ab2d69b076 100644 --- a/apps/files/lib/Notification/Notifier.php +++ b/apps/files/lib/Notification/Notifier.php @@ -27,9 +27,11 @@ */ namespace OCA\Files\Notification; +use OCA\Files\BackgroundJob\TransferOwnership; use OCA\Files\Db\TransferOwnershipMapper; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; @@ -54,15 +56,19 @@ class Notifier implements INotifier, IDismissableNotifier { private $userManager; /** @var ITimeFactory */ private $timeFactory; + /** @var IJobList */ + private $jobList; public function __construct(IFactory $l10nFactory, IURLGenerator $urlGenerator, TransferOwnershipMapper $mapper, IManager $notificationManager, IUserManager $userManager, + IJobList $jobList, ITimeFactory $timeFactory) { $this->l10nFactory = $l10nFactory; $this->urlGenerator = $urlGenerator; + $this->jobList = $jobList; $this->mapper = $mapper; $this->notificationManager = $notificationManager; $this->userManager = $userManager; @@ -281,6 +287,9 @@ public function dismissNotification(INotification $notification): void { if ($notification->getApp() !== 'files') { throw new \InvalidArgumentException('Unhandled app'); } + if ($notification->getSubject() !== 'transferownershipRequest') { + throw new \InvalidArgumentException('Unhandled notification type'); + } // TODO: This should all be moved to a service that also the transferownershipController uses. try { @@ -289,6 +298,12 @@ public function dismissNotification(INotification $notification): void { return; } + if ($this->jobList->has(TransferOwnership::class, [ + 'id' => $transferOwnership->getId(), + ])) { + return; + } + $notification = $this->notificationManager->createNotification(); $notification->setUser($transferOwnership->getSourceUser()) ->setApp('files')