Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Jul 16, 2019

Copy link
Member Author

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Explanation how to update your Notifier to 17

'name' => $l->t('Talk'),
];
});
$manager->registerNotifierService(Notifier::class);
Copy link
Member Author

Choose a reason for hiding this comment

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

Register with the class name, instead of a closure

* @since 17.0.0
*/
public function getID(): string {
return 'talk';
Copy link
Member Author

Choose a reason for hiding this comment

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

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');
Copy link
Member Author

Choose a reason for hiding this comment

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

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 {
Copy link
Member Author

Choose a reason for hiding this comment

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

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);
Copy link
Member Author

Choose a reason for hiding this comment

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

instead of marking as processed in the prepare method, throw a AlreadyProcessedException which will correctly delete the notification on all devices.

@nickvergessen nickvergessen force-pushed the bugfix/noid/notification-adjustments branch from 9a323b1 to ebf7781 Compare July 18, 2019 10:31
@nickvergessen
Copy link
Member Author

Rebased to trigger working tests and added some more buttons as requested by jan in #577

@Ivansss Ivansss merged commit 28410f8 into master Jul 18, 2019
@delete-merged-branch delete-merged-branch bot deleted the bugfix/noid/notification-adjustments branch July 18, 2019 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Call to action missing in the notifications

5 participants