-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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.
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
Decouple remote wipe notifcation channels with events #15775
Conversation
This comment has been minimized.
This comment has been minimized.
f0fb875
to
91fb2ae
Compare
lib/private/Authentication/Listeners/RemoteWipeActivityListener.php
Outdated
Show resolved
Hide resolved
lib/private/Authentication/Listeners/RemoteWipeActivityListener.php
Outdated
Show resolved
Hide resolved
} | ||
|
||
private function sendNotification(string $event, IToken $token): void { | ||
$notification = $this->notificationManager->createNotification(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to send those as normal notifications,
or more like the silent notification when a notification is deleted on another device?
Similar to nextcloud/notifications#318
That would allow us, to send it only to the affected device.
In that case I would simply trigger an event and make the notifications app listen to it and then do it's magic there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me. Can we do this in a follow-up PR? The little refactoring exercise here is needed to also add e-mail notifications 😉
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
91fb2ae
to
aa6622c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Code looks good.
I tested it and it works as advertised! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
For #15646.
Todo