Conversation
nickvergessen
left a comment
nickvergessen
left a comment
There was a problem hiding this comment.
Explanation how to update your Notifier to 17
| 'name' => $l->t('Talk'), | ||
| ]; | ||
| }); | ||
| $manager->registerNotifierService(Notifier::class); |
There was a problem hiding this comment.
Register with the class name, instead of a closure
| * @since 17.0.0 | ||
| */ | ||
| public function getID(): string { | ||
| return 'talk'; |
There was a problem hiding this comment.
New method in 17, replacing the id return of the info closure from the old registration way
| * @since 17.0.0 | ||
| */ | ||
| public function getName(): string { | ||
| return $this->lFactory->get('spreed')->t('Talk'); |
There was a problem hiding this comment.
New method in 17, replacing the name return of the info closure from the old registration way
| * @since 9.0.0 | ||
| */ | ||
| public function prepare(INotification $notification, $languageCode): INotification { | ||
| public function prepare(INotification $notification, string $languageCode): INotification { |
There was a problem hiding this comment.
Type hint for $languageCode is new, and the : INotification return type is enforced now too.
| $participant = $room->getParticipant($userId); | ||
| } catch (ParticipantNotFoundException $e) { | ||
| // Room does not exist | ||
| $this->notificationManager->markProcessed($notification); |
There was a problem hiding this comment.
instead of marking as processed in the prepare method, throw a AlreadyProcessedException which will correctly delete the notification on all devices.
nickvergessen
added
3. to review
bug
enhancement
feature: chat 💬
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
force-pushed
the
bugfix/noid/notification-adjustments
branch
from
9a323b1 to
ebf7781
Compare
|
Rebased to trigger working tests and added some more buttons as requested by jan in #577 |
delete-merged-branch
bot
deleted the
bugfix/noid/notification-adjustments
branch
4 participants
Ref nextcloud/server#15040
Fix #577