-
Notifications
You must be signed in to change notification settings - Fork 436
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
Fix adding and removing permissions #13027
Conversation
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When adding permissions to all attendees the permissions are added to the call/room and to the attendees. Attendees with default permissions will get the effective permissions from the call/room permissions, so the permissions should be added only to those attendees that already have custom permissions. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When adding or removing attendee permissions the permissions are decomposed in each separate permission and added or removed individually. However, the same query object was reused, so once it was executed for the first permission found then it no longer worked as expected for the following permissions. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When adding or removing call permissions the default room permissions should be taken as a base if no call permission is set. Otherwise adding will cause the permissions to be set instead of added, while removing them will have no effect. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
/backport to stable30 |
/backport to stable29 |
/backport to stable28 |
Yeah, that was my next plan. Also since there is no client/UI allowing that atm (or is the lobby bypass using that?) |
It does (although for specific participants, not for the whole room): spreed/src/components/RightSidebar/Participants/Participant.vue Lines 1022 to 1029 in ee85e5f
spreed/lib/Controller/RoomController.php Line 2148 in c8b9e8d
|
Double click 🤦 |
@@ -227,6 +221,8 @@ public function modifyPermissions(int $roomId, string $mode, int $newState): voi | |||
Attendee::PERMISSIONS_LOBBY_IGNORE, | |||
] as $permission) { | |||
if ($permission & $newState) { | |||
$query = $this->getModifyPermissionsBaseQuery($roomId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be outside the loop? We don't need to create the query builder 7 times?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The query builder would be created as many times as individual permissions are added/removed, so it would be created seven times only if the seven permissions were added/removed at once.
A single query object needs to be created for each individual permission because adding or removing a single permission executes the statement. Is there any way to reset the query builder to reuse the same base update
query and change the set
and andWhere
parts?
Even if call permissions will be removed in Talk 21 right now they are available through the API, so they should to be fixed. Moreover, technically adding and removing permissions could be done too with room permissions, even if right now the API only allows that with call permissions (which on the other hand could be a hint to also remove adding/removing permissions and just allow setting them).
For the details please refer to the integration tests and the description in each individual commit.