Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix email notifications for invites without local state. #8627

Merged
merged 2 commits into from
Oct 23, 2020
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions changelog.d/8627.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix email notifications for invites without local state.
41 changes: 28 additions & 13 deletions synapse/push/mailer.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import bleach
import jinja2

from synapse.api.constants import EventTypes
from synapse.api.constants import EventTypes, Membership
from synapse.api.errors import StoreError
from synapse.config.emailconfig import EmailSubjectConfig
from synapse.logging.context import make_deferred_yieldable
Expand Down Expand Up @@ -317,9 +317,14 @@ async def send_email(self, email_address, subject, extra_template_vars):
async def get_room_vars(
self, room_id, user_id, notifs, notif_events, room_state_ids
):
my_member_event_id = room_state_ids[("m.room.member", user_id)]
my_member_event = await self.store.get_event(my_member_event_id)
is_invite = my_member_event.content["membership"] == "invite"
# Check if one of the notifs is an invite event for the user.
is_invite = False
for n in notifs:
ev = notif_events[n["event_id"]]
if ev.type == EventTypes.Member and ev.state_key == user_id:
if ev.content.get("membership") == Membership.INVITE:
is_invite = True
break
Comment on lines +321 to +327
Copy link
Member

Choose a reason for hiding this comment

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

This looks OK, I think, but would it be more efficient to still just try to grab the membership event directly?

my_member_event_id = room_state_ids.get((EventTypes.Member, user_id))
is_invite = my_member_event_id and notif_events[my_member_event_id].content.get("membership") == Membership.INVITE

I'm not really sure if that makes sense ^ since I'm having trouble following what the inputs to this method are.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is really that room_state_ids may be empty if the server is no longer in the room (or has never been, in the case of remote invites, if they worked))

Copy link
Member

Choose a reason for hiding this comment

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

Ah, OK. Thanks for explaining! I think it looks good. 👍


room_name = await calculate_room_name(self.store, room_state_ids, user_id)

Expand Down Expand Up @@ -461,16 +466,26 @@ async def make_summary_text(
self.store, room_state_ids[room_id], user_id, fallback_to_members=False
)

my_member_event_id = room_state_ids[room_id][("m.room.member", user_id)]
my_member_event = await self.store.get_event(my_member_event_id)
if my_member_event.content["membership"] == "invite":
inviter_member_event_id = room_state_ids[room_id][
("m.room.member", my_member_event.sender)
]
inviter_member_event = await self.store.get_event(
inviter_member_event_id
# See if one of the notifs is an invite event for the user
invite_event = None
for n in notifs_by_room[room_id]:
ev = notif_events[n["event_id"]]
if ev.type == EventTypes.Member and ev.state_key == user_id:
if ev.content.get("membership") == Membership.INVITE:
invite_event = ev
break

if invite_event:
inviter_member_event_id = room_state_ids[room_id].get(
("m.room.member", invite_event.sender)
)
inviter_name = name_from_member_event(inviter_member_event)
inviter_name = invite_event.sender
if inviter_member_event_id:
inviter_member_event = await self.store.get_event(
inviter_member_event_id, allow_none=True
)
if inviter_member_event:
inviter_name = name_from_member_event(inviter_member_event)

if room_name is None:
return self.email_subjects.invite_from_person % {
Expand Down
29 changes: 29 additions & 0 deletions tests/push/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,35 @@ def test_simple_sends_email(self):
# We should get emailed about that message
self._check_for_mail()

def test_invite_sends_email(self):
# Create a room and invite the user to it
room = self.helper.create_room_as(self.others[0].id, tok=self.others[0].token)
self.helper.invite(
room=room,
src=self.others[0].id,
tok=self.others[0].token,
targ=self.user_id,
)

# We should get emailed about the invite
self._check_for_mail()

def test_invite_to_empty_room_sends_email(self):
# Create a room and invite the user to it
room = self.helper.create_room_as(self.others[0].id, tok=self.others[0].token)
self.helper.invite(
room=room,
src=self.others[0].id,
tok=self.others[0].token,
targ=self.user_id,
)

# Then have the original user leave
self.helper.leave(room, self.others[0].id, tok=self.others[0].token)

# We should get emailed about the invite
self._check_for_mail()

def test_multiple_members_email(self):
# We want to test multiple notifications, so we pause processing of push
# while we send messages.
Expand Down