Skip to content
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

Allow multiple recipients within single PushMessage #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions exponent_server_sdk/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,13 @@ def _publish_internal(self, push_messages):
'badge': 1,
'data': {'any': 'json object'},
}
or
{
'to': ['ExponentPushToken[xxx]', 'ExponentPushToken[yyy]'],
'body': 'This text gets display in the notification',
'badge': 1,
'data': {'any': 'json object'},
}

Args:
push_messages: An array of PushMessage objects.
Expand Down Expand Up @@ -368,11 +375,15 @@ def _publish_internal(self, push_messages):
response.raise_for_status()

# Sanity check the response
if len(push_messages) != len(response_data['data']):
expected_data_length = 0
for push_message in push_messages:
expected_data_length += len(push_message.to) if isinstance(push_message.to, list) else 1
# Note : expected_data_length may exceed max_message_count
if expected_data_length != len(response_data['data']):
raise PushServerError(
('Mismatched response length. Expected %d %s but only '
'received %d' %
(len(push_messages), 'receipt' if len(push_messages) == 1 else
(expected_data_length, 'receipt' if expected_data_length == 1 else
'receipts', len(response_data['data']))),
response,
response_data=response_data)
Expand Down Expand Up @@ -401,6 +412,9 @@ def publish(self, push_message):
Returns:
A PushTicket object which contains the results.
"""
if isinstance(push_message.to, list) and len(push_message.to) > 1:
raise ValueError("Sending notification to multiple recipients is not allowed \
with publish method. Use publish_multiple method instead.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

you opted to just not allow it?

Is it easy enough to implement?

return self.publish_multiple([push_message])[0]

def publish_multiple(self, push_messages):
Expand All @@ -414,6 +428,9 @@ def publish_multiple(self, push_messages):
"""
push_tickets = []
for start in itertools.count(0, self.max_message_count):
# Todo : Check if len(push_message.to) check is required here as well
Copy link
Collaborator

Choose a reason for hiding this comment

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

From reading the docs it looks like it's 100 total (including the different ones, or same one with multiple "to's"

# If yes : We will divide the push_message with len(to) > max_message_count
# into multiple push messages.
chunk = list(
itertools.islice(push_messages, start,
start + self.max_message_count))
Expand Down