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

Add sending room shares support #6105

Merged
merged 6 commits into from
Oct 4, 2021
Merged

Conversation

gary-kim
Copy link
Member

@gary-kim gary-kim commented Aug 11, 2021

For #5723

This adds the backend for sending room shares.

Also includes tests for some of the sending and receiving functions.

@gary-kim
Copy link
Member Author

Ran into a slight issue while trying to write the reshare system.
Since rooms don't have an owner, for now, the "owner" field is just whoever sent the invitation but the federated id is also used for determining the room's home server, this won't work for reshares (invitations) from another server.
Maybe the bridge-bot account could be used as the default room owner if the room doesn't have a set owner?

cc @nickvergessen

@gary-kim
Copy link
Member Author

Just pushed a commit that will try to find a room owner but if it cannot find one, bridge-bot will be used instead.

@gary-kim gary-kim force-pushed the enh/5723/sending-shares branch 2 times, most recently from 10be4da to 8f02a5d Compare August 20, 2021 00:45
@nickvergessen
Copy link
Member

Maybe the bridge-bot account could be used as the default room owner if the room doesn't have a set owner?

The user might not exist on the other server and I would like to not require a user to be created.

What are you using the owner for?

@gary-kim gary-kim force-pushed the enh/5723/sending-shares branch 2 times, most recently from 4987824 to e6ca245 Compare August 25, 2021 14:51
@gary-kim gary-kim force-pushed the enh/5723/sending-shares branch from 940d92e to 59136b8 Compare August 27, 2021 23:00
@gary-kim gary-kim force-pushed the enh/5723/sending-shares branch 2 times, most recently from 8632221 to 988fc98 Compare August 28, 2021 02:11
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Minor last things

lib/Service/ParticipantService.php Outdated Show resolved Hide resolved
lib/Service/ParticipantService.php Outdated Show resolved Hide resolved
lib/Service/ParticipantService.php Outdated Show resolved Hide resolved
@gary-kim
Copy link
Member Author

👍 Let me fix those up

@gary-kim gary-kim force-pushed the enh/5723/sending-shares branch 2 times, most recently from 012b20b to cc34659 Compare September 20, 2021 18:28
gary-kim and others added 3 commits September 20, 2021 14:30
Signed-off-by: Joas Schilling <[email protected]>
@gary-kim gary-kim force-pushed the enh/5723/sending-shares branch 2 times, most recently from 9cd98fd to ecd0a0c Compare September 20, 2021 18:33
@gary-kim
Copy link
Member Author

gary-kim commented Sep 21, 2021

Moving the highest permission function into ParticipantService has made a circular dependency. That needs to be fixed.

EDIT: Done.

@gary-kim gary-kim force-pushed the enh/5723/sending-shares branch 2 times, most recently from c7d6d8f to 3b18a7c Compare September 21, 2021 15:58
Signed-off-by: Gary Kim <[email protected]>
@gary-kim gary-kim force-pushed the enh/5723/sending-shares branch from 3b18a7c to abbe546 Compare September 21, 2021 16:00
@nickvergessen
Copy link
Member

I guess we should add a config which blocks this for now, so people don't spam content to the databases before we implemented everything.

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Copy link
Member

Wuhu, finally one step further :D

@nickvergessen nickvergessen merged commit 820342e into master Oct 4, 2021
@nickvergessen nickvergessen deleted the enh/5723/sending-shares branch October 4, 2021 15:26
@nickvergessen
Copy link
Member

THanks for all the hard work and sorry for the slow reviews @gary-kim

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

Successfully merging this pull request may close these issues.

2 participants