Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transfer bans on room upgrade #4642

Merged
merged 5 commits into from Feb 19, 2019
Merged
Diff settings

Always

Just for now

@@ -0,0 +1 @@
Transfer bans on room upgrade.

This comment has been minimized.

Copy link
@richvdh

richvdh Feb 18, 2019

Member

I call it a feature rather than a bugfix.

This comment has been minimized.

Copy link
@anoadragon453

anoadragon453 Feb 18, 2019

Author Member

Really? All the other room upgrade stuff I've been doing have been bug fixes.

This comment has been minimized.

Copy link
@richvdh

richvdh Feb 18, 2019

Member

I'd argue that's incorrect, then. This is a new feature for room upgrades as far as I'm concerned.

This comment has been minimized.

Copy link
@anoadragon453

anoadragon453 Feb 18, 2019

Author Member

Kk, I have no strong feelings one way or the other.

@@ -285,6 +285,7 @@ def clone_existing_room(
(EventTypes.RoomAvatar, ""),
(EventTypes.Encryption, ""),
(EventTypes.ServerACL, ""),
(EventTypes.Member, None),

This comment has been minimized.

Copy link
@richvdh

richvdh Feb 18, 2019

Member

I think it would probably be better to do this with a separate call to get_filtered_current_state_ids.

This comment has been minimized.

Copy link
@anoadragon453

anoadragon453 Feb 18, 2019

Author Member

Hrm, in testing this seems to be a few hundred ms slower if we do another call to the DB.

This comment has been minimized.

Copy link
@richvdh

richvdh Feb 18, 2019

Member

how big was your room? ;-p

This comment has been minimized.

Copy link
@anoadragon453

anoadragon453 Feb 18, 2019

Author Member

Literally 1 ban and no messages lol

)

old_room_state_ids = yield self.store.get_filtered_current_state_ids(
@@ -293,9 +294,15 @@ def clone_existing_room(
# map from event_id to BaseEvent
old_room_state_events = yield self.store.get_events(old_room_state_ids.values())

member_events = []
for k, old_event_id in iteritems(old_room_state_ids):
old_event = old_room_state_events.get(old_event_id)
if old_event:
# Do membership events later
if ("membership" in old_event.content):
member_events.append(old_event)
continue

initial_state[k] = old_event.content

yield self._send_events_for_new_room(
@@ -311,6 +318,21 @@ def clone_existing_room(
creation_content=creation_content,
)

# Transfer membership events
for old_event in member_events:
# Only transfer ban events
logger.info("Event type: " + str(old_event.content))

This comment has been minimized.

Copy link
@richvdh

richvdh Feb 18, 2019

Member

this looks like it needs to go away

if ("membership" in old_event.content and
old_event.content["membership"] == "ban"):
yield self.room_member_handler.update_membership(
requester,
UserID.from_string(old_event['state_key']),
new_room_id,
"ban",
ratelimit=False,
content=old_event.content,
)

# XXX invites/joins
# XXX 3pid invites

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.