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

Handle rejections of invites from local users locally #615

Merged
merged 10 commits into from
Mar 4, 2016
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Mar 1, 2016

Slightly hacky fix to SYN-642, which avoids the federation codepath when trying
to reject invites from local users.

richvdh added 2 commits March 1, 2016 17:27
Slightly hacky fix to SYN-642, which avoids the federation codepath when trying
to reject invites from local users.
@richvdh
Copy link
Member Author

richvdh commented Mar 1, 2016

See matrix-org/sytest#198 for test

# user, and all other local users, have left, making
# is_host_in_room return false).
#
context_events = (e.event_id for e in context.current_state.values())
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit confused about this. The canonical check for if we received the invite over federation is if inviter.domain != self.server_name?

Copy link
Member Author

Choose a reason for hiding this comment

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

True, but I guess here we're actually less interested in whether the event arrived over federation, and more in whether we already have it in context.current_state. If the event is in context.current_state despite being received over federation, there's no point dropping the context.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the comment is unclear. If you're happy otherwise I could update the comment.

Copy link
Member

Choose a reason for hiding this comment

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

Aha, right, yes, it was mainly the comment that confused me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Call this context_event_ids

Copy link
Member Author

Choose a reason for hiding this comment

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

done, and comment rewritten.

@richvdh
Copy link
Member Author

richvdh commented Mar 2, 2016

maybe @illicitonion should also take a look at this since he's been in the area recently.

@erikjohnston
Copy link
Member

Looks ok to me, though I think @illicitonion should have a quick peek since I don't really know how any of this works anymore :)

if not inviter:
raise SynapseError(404, "Not a known room")

if inviter.domain == self.server_name:
Copy link
Contributor

Choose a reason for hiding this comment

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

self.hs.is_mine(inviter)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just lifted the code from do_remotely_reject_invite. I guess your way is better

if anybody had bothered to document what self.hs actually is, this would have been somewhat easier to figure out.

@illicitonion
Copy link
Contributor

LGTM, small handful of comments

@richvdh
Copy link
Member Author

richvdh commented Mar 2, 2016

@illicitonion ptal?

@illicitonion
Copy link
Contributor

LGTM, one minor comment

Also don't overwrite the list that gets passed in.
@richvdh richvdh assigned richvdh and unassigned illicitonion Mar 2, 2016
@richvdh
Copy link
Member Author

richvdh commented Mar 2, 2016

retest this please

1 similar comment
@richvdh
Copy link
Member Author

richvdh commented Mar 2, 2016

retest this please

@richvdh
Copy link
Member Author

richvdh commented Mar 3, 2016

retest this please

pleeeease

@richvdh
Copy link
Member Author

richvdh commented Mar 3, 2016

retest this please

1 similar comment
@erikjohnston
Copy link
Member

retest this please

@richvdh
Copy link
Member Author

richvdh commented Mar 3, 2016

retest this please

richvdh added a commit that referenced this pull request Mar 4, 2016
Handle rejections of invites from local users locally
@richvdh richvdh merged commit fa6d6bb into develop Mar 4, 2016
@richvdh richvdh deleted the rav/SYN-642 branch March 4, 2016 00:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants