Merge pull request #799 from matrix-org/matthew/quieter-email-notifs

Tune email notifs to make them quieter:
This commit is contained in:
David Baker 2016-05-24 14:30:51 +01:00
commit cbf8d146ac
5 changed files with 79 additions and 25 deletions

View File

@ -145,6 +145,11 @@ pre, code {
text-decoration: none; text-decoration: none;
} }
.debug {
font-size: 10px;
color: #888;
}
.footer { .footer {
margin-top: 20px; margin-top: 20px;
text-align: center; text-align: center;

View File

@ -30,7 +30,7 @@
{% include 'room.html' with context %} {% include 'room.html' with context %}
{% endfor %} {% endfor %}
<div class="footer"> <div class="footer">
<small> <div class="debug">
Sending email at {{ reason.now|format_ts("%c") }} due to activity in room '{{ reason.room_name }}' because:<br/> Sending email at {{ reason.now|format_ts("%c") }} due to activity in room '{{ reason.room_name }}' because:<br/>
1. An event was received at {{ reason.received_at|format_ts("%c") }} 1. An event was received at {{ reason.received_at|format_ts("%c") }}
which is more than {{ "%.1f"|format(reason.delay_before_mail_ms / (60*1000)) }} (delay_before_mail_ms) mins ago.<br/> which is more than {{ "%.1f"|format(reason.delay_before_mail_ms / (60*1000)) }} (delay_before_mail_ms) mins ago.<br/>
@ -40,7 +40,7 @@
{% else %} {% else %}
2. We can't remember the last time we sent a mail for this room. 2. We can't remember the last time we sent a mail for this room.
{% endif %} {% endif %}
</small> </div>
<a href="{{ unsubscribe_link }}">Unsubscribe</a> <a href="{{ unsubscribe_link }}">Unsubscribe</a>
</div> </div>
</td> </td>

View File

@ -32,12 +32,19 @@ DELAY_BEFORE_MAIL_MS = 10 * 60 * 1000
# Each room maintains its own throttle counter, but each new mail notification # Each room maintains its own throttle counter, but each new mail notification
# sends the pending notifications for all rooms. # sends the pending notifications for all rooms.
THROTTLE_START_MS = 10 * 60 * 1000 THROTTLE_START_MS = 10 * 60 * 1000
THROTTLE_MAX_MS = 24 * 60 * 60 * 1000 # (2 * 60 * 1000) * (2 ** 11) # ~3 days THROTTLE_MAX_MS = 24 * 60 * 60 * 1000 # 24h
THROTTLE_MULTIPLIER = 6 # 10 mins, 1 hour, 6 hours, 24 hours # THROTTLE_MULTIPLIER = 6 # 10 mins, 1 hour, 6 hours, 24 hours
THROTTLE_MULTIPLIER = 144 # 10 mins, 24 hours - i.e. jump straight to 1 day
# If no event triggers a notification for this long after the previous, # If no event triggers a notification for this long after the previous,
# the throttle is released. # the throttle is released.
THROTTLE_RESET_AFTER_MS = (2 * 60 * 1000) * (2 ** 11) # ~3 days # 12 hours - a gap of 12 hours in conversation is surely enough to merit a new
# notification when things get going again...
THROTTLE_RESET_AFTER_MS = (12 * 60 * 60 * 1000)
# does each email include all unread notifs, or just the ones which have happened
# since the last mail?
INCLUDE_ALL_UNREAD_NOTIFS = True
class EmailPusher(object): class EmailPusher(object):
@ -126,8 +133,9 @@ class EmailPusher(object):
up logging, measures and guards against multiple instances of it up logging, measures and guards against multiple instances of it
being run. being run.
""" """
start = 0 if INCLUDE_ALL_UNREAD_NOTIFS else self.last_stream_ordering
unprocessed = yield self.store.get_unread_push_actions_for_user_in_range( unprocessed = yield self.store.get_unread_push_actions_for_user_in_range(
self.user_id, self.last_stream_ordering, self.max_stream_ordering self.user_id, start, self.max_stream_ordering
) )
soonest_due_at = None soonest_due_at = None
@ -150,7 +158,6 @@ class EmailPusher(object):
# we then consider all previously outstanding notifications # we then consider all previously outstanding notifications
# to be delivered. # to be delivered.
# debugging:
reason = { reason = {
'room_id': push_action['room_id'], 'room_id': push_action['room_id'],
'now': self.clock.time_msec(), 'now': self.clock.time_msec(),
@ -165,8 +172,11 @@ class EmailPusher(object):
yield self.save_last_stream_ordering_and_success(max([ yield self.save_last_stream_ordering_and_success(max([
ea['stream_ordering'] for ea in unprocessed ea['stream_ordering'] for ea in unprocessed
])) ]))
# we update the throttle on all the possible unprocessed push actions
for ea in unprocessed:
yield self.sent_notif_update_throttle( yield self.sent_notif_update_throttle(
push_action['room_id'], push_action ea['room_id'], ea
) )
break break
else: else:

View File

@ -44,8 +44,11 @@ MESSAGE_FROM_PERSON_IN_ROOM = "You have a message on %(app)s from %(person)s " \
"in the %s room..." "in the %s room..."
MESSAGE_FROM_PERSON = "You have a message on %(app)s from %(person)s..." MESSAGE_FROM_PERSON = "You have a message on %(app)s from %(person)s..."
MESSAGES_FROM_PERSON = "You have messages on %(app)s from %(person)s..." MESSAGES_FROM_PERSON = "You have messages on %(app)s from %(person)s..."
MESSAGES_IN_ROOM = "There are some messages on %(app)s for you in the %(room)s room..." MESSAGES_IN_ROOM = "You have messages on %(app)s in the %(room)s room..."
MESSAGES_IN_ROOMS = "Here are some messages on %(app)s you may have missed..." MESSAGES_IN_ROOM_AND_OTHERS = \
"You have messages on %(app)s in the %(room)s room and others..."
MESSAGES_FROM_PERSON_AND_OTHERS = \
"You have messages on %(app)s from %(person)s and others..."
INVITE_FROM_PERSON_TO_ROOM = "%(person)s has invited you to join the " \ INVITE_FROM_PERSON_TO_ROOM = "%(person)s has invited you to join the " \
"%(room)s room on %(app)s..." "%(room)s room on %(app)s..."
INVITE_FROM_PERSON = "%(person)s has invited you to chat on %(app)s..." INVITE_FROM_PERSON = "%(person)s has invited you to chat on %(app)s..."
@ -128,9 +131,14 @@ class Mailer(object):
state_by_room[room_id] = room_state state_by_room[room_id] = room_state
# Run at most 3 of these at once: sync does 10 at a time but email # Run at most 3 of these at once: sync does 10 at a time but email
# notifs are much realtime than sync so we can afford to wait a bit. # notifs are much less realtime than sync so we can afford to wait a bit.
yield concurrently_execute(_fetch_room_state, rooms_in_order, 3) yield concurrently_execute(_fetch_room_state, rooms_in_order, 3)
# actually sort our so-called rooms_in_order list, most recent room first
rooms_in_order.sort(
key=lambda r: -(notifs_by_room[r][-1]['received_ts'] or 0)
)
rooms = [] rooms = []
for r in rooms_in_order: for r in rooms_in_order:
@ -139,12 +147,12 @@ class Mailer(object):
) )
rooms.append(roomvars) rooms.append(roomvars)
summary_text = self.make_summary_text( reason['room_name'] = calculate_room_name(
notifs_by_room, state_by_room, notif_events, user_id state_by_room[reason['room_id']], user_id, fallback_to_members=True
) )
reason['room_name'] = calculate_room_name( summary_text = self.make_summary_text(
state_by_room[reason['room_id']], user_id, fallback_to_members=False notifs_by_room, state_by_room, notif_events, user_id, reason
) )
template_vars = { template_vars = {
@ -296,7 +304,8 @@ class Mailer(object):
return messagevars return messagevars
def make_summary_text(self, notifs_by_room, state_by_room, notif_events, user_id): def make_summary_text(self, notifs_by_room, state_by_room,
notif_events, user_id, reason):
if len(notifs_by_room) == 1: if len(notifs_by_room) == 1:
# Only one room has new stuff # Only one room has new stuff
room_id = notifs_by_room.keys()[0] room_id = notifs_by_room.keys()[0]
@ -371,7 +380,26 @@ class Mailer(object):
} }
else: else:
# Stuff's happened in multiple different rooms # Stuff's happened in multiple different rooms
return MESSAGES_IN_ROOMS % {
# ...but we still refer to the 'reason' room which triggered the mail
if reason['room_name'] is not None:
return MESSAGES_IN_ROOM_AND_OTHERS % {
"room": reason['room_name'],
"app": self.app_name,
}
else:
# If the reason room doesn't have a name, say who the messages
# are from explicitly to avoid, "messages in the Bob room"
sender_ids = list(set([
notif_events[n['event_id']].sender
for n in notifs_by_room[reason['room_id']]
]))
return MESSAGES_FROM_PERSON_AND_OTHERS % {
"person": descriptor_from_member_events([
state_by_room[reason['room_id']][("m.room.member", s)]
for s in sender_ids
]),
"app": self.app_name, "app": self.app_name,
} }

View File

@ -14,6 +14,9 @@
# limitations under the License. # limitations under the License.
import re import re
import logging
logger = logging.getLogger(__name__)
# intentionally looser than what aliases we allow to be registered since # intentionally looser than what aliases we allow to be registered since
# other HSes may allow aliases that we would not # other HSes may allow aliases that we would not
@ -105,13 +108,21 @@ def calculate_room_name(room_state, user_id, fallback_to_members=True):
# or inbound invite, or outbound 3PID invite. # or inbound invite, or outbound 3PID invite.
if all_members[0].sender == user_id: if all_members[0].sender == user_id:
if "m.room.third_party_invite" in room_state_bytype: if "m.room.third_party_invite" in room_state_bytype:
third_party_invites = room_state_bytype["m.room.third_party_invite"] third_party_invites = (
room_state_bytype["m.room.third_party_invite"].values()
)
if len(third_party_invites) > 0: if len(third_party_invites) > 0:
# technically third party invite events are not member # technically third party invite events are not member
# events, but they are close enough # events, but they are close enough
return "Inviting %s" (
descriptor_from_member_events(third_party_invites) # FIXME: no they're not - they look nothing like a member;
) # they have a great big encrypted thing as their name to
# prevent leaking the 3PID name...
# return "Inviting %s" % (
# descriptor_from_member_events(third_party_invites)
# )
return "Inviting email address"
else: else:
return ALL_ALONE return ALL_ALONE
else: else: