From fca5e23375efcc96a2c044bc13a3833a977039a6 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Wed, 16 Oct 2024 14:58:41 +0200 Subject: [PATCH 1/3] fix: add notification icon for transfer ownership Signed-off-by: Maksim Sukharev --- apps/files/img/folder-move.svg | 1 + apps/files/lib/Notification/Notifier.php | 4 ++++ 2 files changed, 5 insertions(+) create mode 100644 apps/files/img/folder-move.svg diff --git a/apps/files/img/folder-move.svg b/apps/files/img/folder-move.svg new file mode 100644 index 0000000000000..349106f4cd927 --- /dev/null +++ b/apps/files/img/folder-move.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/apps/files/lib/Notification/Notifier.php b/apps/files/lib/Notification/Notifier.php index 10954ac9c6492..caf707cc6e38e 100644 --- a/apps/files/lib/Notification/Notifier.php +++ b/apps/files/lib/Notification/Notifier.php @@ -88,6 +88,10 @@ public function prepare(INotification $notification, string $languageCode): INot throw new \InvalidArgumentException('Unhandled app'); } + $imagePath = $this->urlGenerator->imagePath('files', 'folder-move.svg'); + $iconUrl = $this->urlGenerator->getAbsoluteURL($imagePath); + $notification->setIcon($iconUrl); + return match($notification->getSubject()) { 'transferownershipRequest' => $this->handleTransferownershipRequest($notification, $languageCode), 'transferownershipRequestDenied' => $this->handleTransferOwnershipRequestDenied($notification, $languageCode), From 443505206ddf1226e3be7ee8ff8c2f736b72f2b3 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Wed, 16 Oct 2024 15:01:00 +0200 Subject: [PATCH 2/3] 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') From dd6668bf98e67d399b585af8fa15bea80d9ee37b Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Wed, 16 Oct 2024 15:03:29 +0200 Subject: [PATCH 3/3] fix: do not duplicate existing entity - this reverts 1e8048abee1745bab648dba5bf96f956c718e4e3 Signed-off-by: Maksim Sukharev --- .../files/lib/Controller/TransferOwnershipController.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/apps/files/lib/Controller/TransferOwnershipController.php b/apps/files/lib/Controller/TransferOwnershipController.php index 43648528b50db..d3256b2c7cc77 100644 --- a/apps/files/lib/Controller/TransferOwnershipController.php +++ b/apps/files/lib/Controller/TransferOwnershipController.php @@ -160,15 +160,8 @@ public function accept(int $id): DataResponse { return new DataResponse([], Http::STATUS_FORBIDDEN); } - $newTransferOwnership = new TransferOwnershipEntity(); - $newTransferOwnership->setNodeName($transferOwnership->getNodeName()); - $newTransferOwnership->setFileId($transferOwnership->getFileId()); - $newTransferOwnership->setSourceUser($transferOwnership->getSourceUser()); - $newTransferOwnership->setTargetUser($transferOwnership->getTargetUser()); - $this->mapper->insert($newTransferOwnership); - $this->jobList->add(TransferOwnership::class, [ - 'id' => $newTransferOwnership->getId(), + 'id' => $transferOwnership->getId(), ]); $notification = $this->notificationManager->createNotification();