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

Organize bridge rooms into a space #413

Closed
wants to merge 8 commits into from

Conversation

HelderFSFerreira
Copy link
Contributor

@HelderFSFerreira HelderFSFerreira commented Dec 28, 2021

Features:

  • Create a space for each user
  • Add all the rooms to the bridge
  • Config flag to enable it

To do:

  • Add avatar do space
  • Manage space permissions

@sumnerevans
Copy link
Member

sumnerevans commented Dec 29, 2021

You can add Co-authored-by: Other User <other.user@email.com> to the commit message to credit other contributors.

@clmnin
Copy link
Contributor

clmnin commented Dec 29, 2021

@sumnerevans this feature is not complete.
We need to

  1. Help current users migrate / migration script
  2. Test for federation (currently via is hard-coded)
  3. Discuss the best way to set this in config.yaml
    etc.

Could you review the code and share your feedback?

Copy link
Member

@sumnerevans sumnerevans left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Generally seems pretty good. Looks like there's some TODOs still around config that also need to be addressed.

main.go Show resolved Hide resolved
portal.go Outdated Show resolved Hide resolved
portal.go Outdated Show resolved Hide resolved
portal.go Outdated Show resolved Hide resolved
portal.go Outdated
func (portal *Portal) addToSpace(spaceID id.RoomID, portalID id.RoomID) {

parentSpaceContent := make(map[string]interface{})
parentSpaceContent["via"] = []string{"matrix.wounn.xyz"}
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be the bridge's homeserver URL

Copy link
Contributor

@clmnin clmnin Dec 29, 2021

Choose a reason for hiding this comment

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

Yup. Should have fixed that when this commit was pushed.

From MSC 1772

via key which gives a list of candidate servers that can be used to join the room

It means these servers will be used to bootstrap the join request (permalink to comment in spaces:riot.ovh)

The bridge's homeserver should be part of via - yes. Should we include the user's homeserver too if it is different from the bridge's homeserver? @HelderFSFerreira we'll try testing this on a federated instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the bridge home server as the value of the via.
Are we sure that we want to have the client's home server in there too?

portal.go Outdated Show resolved Hide resolved
user.go Outdated Show resolved Hide resolved
@sumnerevans sumnerevans marked this pull request as draft December 29, 2021 04:48
@sumnerevans
Copy link
Member

As far as how to add to the config, I'd just do something like: bridge.create_space_per_user or something. Might have to think a bit about how to best make that work with multi-tenant bridges.

I think it may make sense to something like what the ResendBridgeInfo function does for when the user enables space creation when it was previously disabled/unspecified.

@clmnin
Copy link
Contributor

clmnin commented Dec 29, 2021

ResendBridgeInfo is a good option to let a user migrate to spaces. Will explore that 👍🏻

So migration will be

  1. bridge.create_space_per_user
  2. resend_bridge_info

New users can just set

  1. bridge.create_space_per_user

HelderFSFerreira and others added 3 commits December 29, 2021 08:37
Co-authored-by: clmnin <clament.john.k@gmail.com>
Co-authored-by: clmnin <clament.john.k@gmail.com>
Co-authored-by: clmnin <clament.john.k@gmail.com>
@tulir
Copy link
Member

tulir commented Dec 29, 2021

Merged in 3a9b3ab

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants