-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: [FC-0047] add functionality to send push notifications #282
Changes from 13 commits
8c55b82
cd9238b
0903211
e38cc86
ba5de95
f293a20
0ba2af6
e3a08e1
30486e8
bcf5d8b
7c58b64
557d47d
5f52163
b18c55a
c1ea794
1026e58
2951c57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -78,6 +78,7 @@ class ChannelMap: | |||||||||||
""" | ||||||||||||
A class that represents a channel map, usually as described in Django settings and `setup.py` files. | ||||||||||||
""" | ||||||||||||
|
||||||||||||
def __init__(self, channels_list): | ||||||||||||
""" | ||||||||||||
Initialize a ChannelMap. | ||||||||||||
|
@@ -170,27 +171,29 @@ def get_channel_for_message(channel_type, message): | |||||||||||
Channel: The selected channel object. | ||||||||||||
""" | ||||||||||||
channels_map = channels() | ||||||||||||
channel_names = [] | ||||||||||||
|
||||||||||||
if channel_type == ChannelType.EMAIL: | ||||||||||||
if message.options.get('transactional'): | ||||||||||||
channel_names = [settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL, settings.ACE_CHANNEL_DEFAULT_EMAIL] | ||||||||||||
else: | ||||||||||||
channel_names = [settings.ACE_CHANNEL_DEFAULT_EMAIL] | ||||||||||||
|
||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed. |
||||||||||||
try: | ||||||||||||
possible_channels = [ | ||||||||||||
channels_map.get_channel_by_name(channel_type, channel_name) | ||||||||||||
for channel_name in channel_names | ||||||||||||
] | ||||||||||||
except KeyError: | ||||||||||||
return channels_map.get_default_channel(channel_type) | ||||||||||||
|
||||||||||||
# First see if any channel specifically demands to deliver this message | ||||||||||||
for channel in possible_channels: | ||||||||||||
if channel.overrides_delivery_for_message(message): | ||||||||||||
return channel | ||||||||||||
|
||||||||||||
# Else the normal path: use the preferred channel for this message type | ||||||||||||
return possible_channels[0] | ||||||||||||
|
||||||||||||
return channels_map.get_default_channel(channel_type) | ||||||||||||
if channel_type == ChannelType.PUSH: | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Costmetic changes:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||||||||||||
channel_names = [settings.ACE_CHANNEL_DEFAULT_PUSH] | ||||||||||||
|
||||||||||||
try: | ||||||||||||
possible_channels = [ | ||||||||||||
channels_map.get_channel_by_name(channel_type, channel_name) | ||||||||||||
for channel_name in channel_names | ||||||||||||
] | ||||||||||||
except KeyError: | ||||||||||||
return channels_map.get_default_channel(channel_type) | ||||||||||||
|
||||||||||||
# First see if any channel specifically demands to deliver this message | ||||||||||||
for channel in possible_channels: | ||||||||||||
if channel.overrides_delivery_for_message(message): | ||||||||||||
return channel | ||||||||||||
|
||||||||||||
# Else the normal path: use the preferred channel for this message type | ||||||||||||
return possible_channels[0] if possible_channels else channels_map.get_default_channel(channel_type) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure if this is valid anymore to return a single channel for a message. Would you mind elaborating on why this change was made? Can we send the same message to both Email and Push? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the I reworked this to avoid duplicating code when adding a new channel. This way, messages can be sent via both Email and Push channels at once. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NiedielnitsevIvan I'm having a hard time understanding this function and its role after this change. I suppose we may need to have a I understand that it returns a single channel, but I don't think that makes sense anymore given that we send multi-channel messages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor style changes to improve readability.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for your review, I have corrected the last comment, so the PR is ready to be merged and released for further use in this PR. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
""" | ||
Channel for sending push notifications. | ||
""" | ||
import logging | ||
import re | ||
|
||
from firebase_admin.messaging import APNSConfig, APNSPayload, Aps, ApsAlert | ||
from push_notifications.gcm import dict_to_fcm_message, send_message | ||
from push_notifications.models import GCMDevice | ||
|
||
from django.conf import settings | ||
|
||
from edx_ace.channel import Channel, ChannelType | ||
from edx_ace.errors import FatalChannelDeliveryError | ||
from edx_ace.message import Message | ||
from edx_ace.renderers import RenderedPushNotification | ||
|
||
LOG = logging.getLogger(__name__) | ||
APNS_DEFAULT_PRIORITY = '5' | ||
APNS_DEFAULT_PUSH_TYPE = 'alert' | ||
|
||
|
||
class PushNotificationChannel(Channel): | ||
""" | ||
A channel for sending push notifications. | ||
""" | ||
|
||
channel_type = ChannelType.PUSH | ||
|
||
@classmethod | ||
def enabled(cls): | ||
""" | ||
Returns true if the push notification settings are configured. | ||
""" | ||
return bool(getattr(settings, 'PUSH_NOTIFICATIONS_SETTINGS', None)) | ||
|
||
def deliver(self, message: Message, rendered_message: RenderedPushNotification) -> None: | ||
""" | ||
Transmit a rendered message to a recipient. | ||
|
||
Args: | ||
message: The message to transmit. | ||
rendered_message: The rendered content of the message that has been personalized | ||
for this particular recipient. | ||
""" | ||
device_tokens = self.get_user_device_tokens(message.recipient.lms_user_id) | ||
if not device_tokens: | ||
LOG.info( | ||
'Recipient with ID %s has no push token. Skipping push notification.', | ||
message.recipient.lms_user_id | ||
) | ||
return | ||
|
||
for token in device_tokens: | ||
self.send_message(message, token, rendered_message) | ||
|
||
def send_message(self, message: Message, token: str, rendered_message: RenderedPushNotification) -> None: | ||
""" | ||
Send a push notification to a device by token. | ||
""" | ||
notification_data = { | ||
'title': self.compress_spaces(rendered_message.title), | ||
'body': self.compress_spaces(rendered_message.body), | ||
'notification_key': token, | ||
**message.context.get('push_notification_extra_context', {}), | ||
} | ||
message = dict_to_fcm_message(notification_data) | ||
# Note: By default dict_to_fcm_message does not support APNS configuration, | ||
# only Android configuration, so we need to collect and set it manually. | ||
apns_config = self.collect_apns_config(notification_data) | ||
message.apns = apns_config | ||
try: | ||
send_message(token, message, settings.FCM_APP_NAME) | ||
except Exception as e: | ||
LOG.exception('Failed to send push notification to %s', token) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏼 |
||
raise FatalChannelDeliveryError(f'Failed to send push notification to {token}') from e | ||
|
||
@staticmethod | ||
def collect_apns_config(notification_data: dict) -> APNSConfig: | ||
""" | ||
Collect APNS configuration with payload for the push notification. | ||
|
||
This APNSConfig must be set to notifications for Firebase to send push notifications to iOS devices. | ||
Notification has default priority and visibility settings, described in Apple's documentation. | ||
(https://developer.apple.com/documentation/usernotifications/sending-notification-requests-to-apns) | ||
""" | ||
apns_alert = ApsAlert(title=notification_data['title'], body=notification_data['body']) | ||
aps = Aps(alert=apns_alert, sound='default') | ||
return APNSConfig( | ||
headers={'apns-priority': APNS_DEFAULT_PRIORITY, 'apns-push-type': APNS_DEFAULT_PUSH_TYPE}, | ||
payload=APNSPayload(aps) | ||
) | ||
|
||
@staticmethod | ||
def get_user_device_tokens(user_id: int) -> list: | ||
""" | ||
Get the device tokens for a user. | ||
""" | ||
return list(GCMDevice.objects.filter( | ||
user_id=user_id, | ||
cloud_message_type='FCM', | ||
active=True, | ||
).values_list('registration_id', flat=True)) | ||
|
||
@staticmethod | ||
def compress_spaces(html_str: str) -> str: | ||
""" | ||
Compress spaces and remove newlines to make it easier to author templates. | ||
""" | ||
return re.sub('\\s+', ' ', html_str, re.UNICODE).strip() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
from push_notifications.api.rest_framework import GCMDeviceViewSet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should lower the log in this case? I'm imagining this will spam logs due to the sheer amount of notifications being sent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed.