event_queue: Call build_offline_notification unconditionally.

Previously, maybe_enqueue_notifications had this very subtle logic,
where it set the notice variable only inside the block for push
notifications, but then also used it inside the block for email
notifications.

This "worked", because previously the conditions for push
notifications were always true if the conditions for email
notifications were, but the code was unnecessarily confusing.  The
only good reason to write it this way is if build_offline_notification
was expensive; in fact, the most expensive thing it does is calling
time.time(), so that reason does not apply here.

This was further confusing, in that in the original logic, we relied
on the fact that push notification code path edited the "notice"
dictionary for further processing.

Instead, we just call it separately and setup the data separately in
each code path.
This commit is contained in:
Tim Abbott
2018-07-14 11:48:25 +05:30
parent a09ebf0551
commit 58a7a390c8

View File

@@ -702,12 +702,14 @@ def maybe_enqueue_notifications(user_profile_id: int, message_id: int, private_m
# notifications to match the model of push notifications
# above.
if idle and (private_message or mentioned):
notice = build_offline_notification(user_profile_id, message_id)
if private_message:
notice['trigger'] = 'private_message'
elif mentioned:
notice['trigger'] = 'mentioned'
else:
raise AssertionError("Unknown notification trigger!")
notice['stream_name'] = stream_name
if not already_notified.get("email_notified"):
queue_json_publish("missedmessage_emails", notice, lambda notice: None)
notified['email_notified'] = True