Skip to content

Commit

Permalink
docs: [FC-0047] extend Decision description
Browse files Browse the repository at this point in the history
  • Loading branch information
NiedielnitsevIvan committed May 29, 2024
1 parent 4f7d1a4 commit d84e2d2
Showing 1 changed file with 62 additions and 0 deletions.
62 changes: 62 additions & 0 deletions docs/decisions/0002-push-notifications.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ with users through their mobile devices, enhancing user engagement and
ensuring timely delivery of important information.
Flexibility and seamless integration with the existing framework are priorities for this new notification channel.

`ace.send` already supports sending via multiple channels.
It turns out that in order to send both push and email notifications,
you need to add a new channel `push_notifications` to the `ACE_ENABLED_CHANNELS`.
In this case, `ace.send` will send both push and email notifications,
but it is worth noting that the necessary templates must also be present,
otherwise we will get the `TemplateDoesNotExist` exception at the rendering stage.

Decision
--------

Expand All @@ -30,10 +37,55 @@ Use the existing `GCMDevice` model and `GCMDeviceViewSet` view from
django-push-notifications_ to manage user device tokens.
Provide secure and authenticated communication with Firebase Cloud Messaging (FCM).

It is proposed to add a new optional argument `limit_to_channels=None` to the
`ace.send` method, which can specify the list of channels through which
messages should be sent. This will allow more flexibility,
for example in cases with discussion notifications, where we need to send
a push for each new reply, while an email is sent only for the first reply.

In this case, the code will look like this:

.. code-block:: python
message_context = _build_message_context(context, notification_type='forum_response')
message = ResponseNotification().personalize(
Recipient(thread_author.id, thread_author.email),
_get_course_language(context['course_id']),
message_context
)
log.info('Sending forum comment notification with context %s', message_context)
if _is_first_comment(context['comment_id'], context['thread_id']):
limit_to_channels = None
else:
limit_to_channels = [ChannelType.PUSH]
ace.send(message, limit_to_channels=limit_to_channels)
For the first response (as it is currently implemented), both notifications
will be sent, and for all other replies, only push notifications will be sent.

The `_build_message_context` method should also be extended:

.. code-block:: python
push_notification_extra_context': {
'notification_type': 'notification_type',
'thread_id': context['thread_id'],
'comment_id': context['comment_id'],
}
The presence of `push_notification_extra_context` will be checked in the
`CoursePushNotificationOptout`, and based on this, it will be determined
whether the `PushNotificationChannel` is allowed.

It should also be noted that django-push-notifications_ does not currently
implement sending notifications to IOS devices using the FCM channel,
although the FCM service itself supports it.
This means that support for IOS devices should be added on the edx-ace side.
At the moment, we didn't plan to add this directly to the
django-push-notifications_ package, but to limit it to an additional method
in `PushNotificationChannel` that implements the collection of the necessary
context for iOS devices to send notifications to them as well.

This will involve:
- PushNotificationRenderer: Responsible for formatting and structuring the content
Expand Down Expand Up @@ -62,6 +114,16 @@ To create a new push notification, on edx-platform side the following steps are
- Add `PushNotificationChannel` to the enabled channels in the setting.
- Call the `ace.send` method to send the push notification.


To configure push notification from platform side, you will need to add/update
the following settings in edx-platform (most likely in
`edx-platform/openedx/core/djangoapps/ace_common/settings/`):
- FIREBASE_CREDENTIALS - add the Firebase credentials for the FCM service;
- ACE_ENABLED_CHANNELS - add the 'push_notification' channel to the list;
- ACE_ENABLED_POLICIES - add policies for 'push_notification' to the list;
- PUSH_NOTIFICATIONS_SETTINGS - settings required for the `django-push-notifications` module;
- add `push_notifications` to the INSTALLED_APPS.

Consequences
------------

Expand Down

0 comments on commit d84e2d2

Please sign in to comment.