/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Send ALL membership events, including bans to the server that was banned #1758

Merged
merged 2 commits into from Jan 4, 2017

Conversation

Reviewers

@richvdh richvdh

Assignees

@NegativeMjark NegativeMjark

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@NegativeMjark @richvdh
@NegativeMjark
Contributor

NegativeMjark commented Jan 4, 2017

No description provided.

@NegativeMjark NegativeMjark assigned richvdh Jan 4, 2017

@NegativeMjark
Send ALL membership events to the server that was affected. 
Send all membership changes to the server that was affected.
This ensures that if the last member of a room on a server
was kicked or banned they get told about it.

Some checks were not successful

1 errored, 1 failing, and 6 successful checks
@matrixbot
Sytest Dendron (Commit) — Build #1298 origin/markjh/fix_ban_propagation failed in 4 min 4 sec
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #2141 origin/markjh/fix_ban_propagation succeeded in 9 min 34 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #2183 origin/markjh/fix_ban_propagation succeeded in 6 min 22 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
b6b6771

@NegativeMjark NegativeMjark referenced this pull request in matrix-org/sytest Jan 4, 2017

Merged

Check that remote users get told about their ban #335

synapse/federation/transaction_queue.py
@@ -161,9 +161,11 @@ def notify_new_events(self, current_id):
get_domain_from_id(user_id) for user_id in users_in_room
)
+ # Send all membership changes to the server that was affected.
+ # This ensures that if the last member of a room on a server
+ # was kicked or banned they get told about it.
@richvdh

richvdh Jan 4, 2017

Member

still wish we knew why Membership.JOIN had been special-cased previously :/

@richvdh

richvdh approved these changes Jan 4, 2017
View changes

lgtm

@richvdh richvdh assigned NegativeMjark and unassigned richvdh Jan 4, 2017

@NegativeMjark
Get the destinations from the state from before the event 
Rather than the state after then event.

Some checks were not successful

1 failing and 7 successful checks
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Dendron (Commit) — Build #1299 origin/markjh/fix_ban_propagation succeeded in 14 min
Details
@matrixbot
Sytest Postgres (Commit) — Build #2142 origin/markjh/fix_ban_propagation succeeded in 9 min 47 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #2184 origin/markjh/fix_ban_propagation succeeded in 7 min 8 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
e02bdaf
@richvdh

richvdh approved these changes Jan 4, 2017
View changes

uhhh... still lgtm, I guess?

@NegativeMjark
Contributor

NegativeMjark commented Jan 4, 2017

I've removed the

if event.type == EventTypes.Member:
      if event.content["membership"] == Membership.JOIN:
             destinations.add(get_domain_from_id(event.state_key))

because it would only have been used to send a join event to a server that joined, and that server already has event because it created it. (all join events are created by the server that is joining the room.)

@NegativeMjark NegativeMjark merged commit 0d766c8 into release-v0.18.6 Jan 4, 2017
7 of 8 checks passed

7 of 8 checks passed

Sytest Dendron (Merged PR) Build finished.
Details
Sytest Dendron (Commit) Build #1299 origin/markjh/fix_ban_propagation succeeded in 14 min
Details
Sytest Postgres (Commit) Build #2142 origin/markjh/fix_ban_propagation succeeded in 9 min 47 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #2184 origin/markjh/fix_ban_propagation succeeded in 7 min 8 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

This was referenced Jan 5, 2017

Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment