feat: make use of IPreloadableNotifier #2452
Conversation
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
lib/Push.php
Outdated
| $language = $this->l10nFactory->getUserLanguage($user); | ||
| $this->printInfo('Language is set to ' . $language); | ||
|
|
||
| $this->notificationManager->preloadMany([$notification], $language); |
There was a problem hiding this comment.
During pushing it sounds a bit weird, but there apps can parse the notification already before sending it. And that should be done, at least grouped by language, so we don't do the same preparsing multiple times.
We do that in Talk already since some time. Documentation is pending and will be done as perparation for my Lightning Talk that I submitted for the Conf.
- Ref fix(push): Allow apps to provide already parsed notifications #1753
- Ref fix(notifications): Preparse call notifications for improved performance spreed#12303
In case of a push also the information should be there already as the app just created the notification object with the details 🙈
But I guess structurally it makes sense to always have the same calling order.
There was a problem hiding this comment.
Yeah, because I was thinking about consistency. Can implementations/apps expect that the method is always called? Otherwise, they would have to implement the data fetching logic twice, once in preloadMany and once in prepare.
| $language = $this->l10nFactory->getUserLanguage($user); | ||
| $this->printInfo('Language is set to ' . $language); | ||
|
|
||
| $this->notificationManager->preloadDataForParsing([$notification], $language); |
There was a problem hiding this comment.
Tried moving Talk to IPreloadableNotifier I noticed this should be inside the $this->notificationManager->setPreparingPushNotification(true); wrapping, so apps can optimize the push case better.
In most such cases should be loaded/aware already anyway?
There was a problem hiding this comment.
Or should we even add a "preload reason" argument or something?
Because:
- when pushing we will prepare the same notification for a set of different users (N:1)
- when preparing emails we will face different users with different items (N:M)
- and on endpoint controller we are preparing for a single user but different subjects (1:M)
Depending on the case it can can make sense to load data from different directions?
Issue: nextcloud/server#54231
Server PR: nextcloud/server#54232