Forum: add instant push notifications

[iszmais]

https://docu.ilias.de/go/wiki/wpage_9000_1357

[mjansenDatabay]

Can we please move this step to the bottom of the method, after the notification delivery?

Update: Ah okay, you are filtering the recipients, and therefore cannot change the order.

[mjansenDatabay]

Hi @iszmais

Thank you for wiring Forum into the new push provider mechanism.

Please answer the following questions:

• How should PushQueueResult::UNKNOWN be treated? NotificationsPushProvider::push() only returns true for PushQueueResult::SUCCEEDED. Unknown is treated as failure, so mail is sent as well. That can duplicate a notification that the endpoint already accepted.

Please consider the following suggestions. You do not need to follow those, but please indicate shortly why you prefer to do otherwise:

• Cache user objects / languages or load users in bulk. ilObjectFactory::getInstanceByObjId() plus ilLanguageFactory::_getLanguageOfUser() per recipient, then a synchronous Web Push request, will sit on the poster’s HTTP request. On forums with many subscribers this can make posting and instant notification slow enough that installations will leave the feature off.
• Log push failures the same way you log success. Failures currently disappear into the mail fallback.
• The new language key forums_forum_push_notification_desc follows the old forums_* pattern. Current language rules prefer module prefix frm_ and _info for setting bylines. Of course, existing Forum keys are inconsistent, so this is optional.
• If forum_notification is disabled, filterMailRecipients() returns []. That is redundant with the mail sender, but it is also easy to misread as "suppress all mail". Please prefer returning the original $recipients and skipping push.

Please implement the following changes:

• Rename $pushProvider to $push_provider in ilForumAppEventListener.
• Fix the action title on ilNotificationLink. ilNotificationLink stores ilNotificationParameter::getName() as title, and ilNotificationPushHandler::buildContent() puts that string into the payload actions[].title. You pass forums_notification_show_post with no parameters. The English translation is "Show post: %s" (the %s is the URL in mail).
• If somehow possible, please do not construct NotificationsPushProvider (ilNotificationPushHandler) unless push can actually run. getPushProvider() always does new NotificationsPushProvider($this), and the handler constructor does file_get_contents($settings->get('private_key_path')). ilSetting::get() declares ?string, so file_get_contents(null) results in a type error if no private key path can be found. That runs for every immediate Forum notification with recipients, even when enable_push is off or keys were never configured. The handler returns early in notify() when disabled, but the constructor already ran. A failed constructor would abort sendNotification() before mail is sent. Please guard on enable_push (and a usable key path), or make the handler construction null-safe, before this is merged.

Best regards,
Michael