Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stable29] fix: get rid of denied notification when accept #48752

Merged
merged 3 commits into from
Oct 17, 2024
Merged

[stable29] fix: get rid of denied notification when accept #48752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5051de6
fix: add notification icon for transfer ownership
Antreesy Oct 16, 2024
8f49796
fix: dismiss notification only after transfer bg job created
Antreesy Oct 16, 2024
90573be
fix: do not duplicate existing entity
Antreesy Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/files/img/folder-move.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 4 additions & 11 deletions apps/files/lib/Controller/TransferOwnershipController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,22 +160,15 @@ public function accept(int $id): DataResponse {
return new DataResponse([], Http::STATUS_FORBIDDEN);
}

$this->jobList->add(TransferOwnership::class, [
'id' => $transferOwnership->getId(),
]);

$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());
$newTransferOwnership->setSourceUser($transferOwnership->getSourceUser());
$newTransferOwnership->setTargetUser($transferOwnership->getTargetUser());
$this->mapper->insert($newTransferOwnership);

$this->jobList->add(TransferOwnership::class, [
'id' => $newTransferOwnership->getId(),
]);

return new DataResponse([], Http::STATUS_OK);
}

Expand Down
19 changes: 19 additions & 0 deletions apps/files/lib/Notification/Notifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -88,6 +94,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),
Expand Down Expand Up @@ -277,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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed from UnknownNotificationException in original PR

}

// TODO: This should all be moved to a service that also the transferownershipController uses.
try {
Expand All @@ -285,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')
Expand Down
Loading