fix: Change power level without changing memory #1929
Conversation
krille-chan
force-pushed
the
krille/fix-set-power-level
branch
from
c3e282a
to
7014278
Compare
krille-chan
force-pushed
the
krille/fix-set-power-level
branch
from
7014278
to
1fd31c2
Compare
The problem here is that we have not created a deep copy of the power level map. By using .copy() we create a deep copy now.
krille-chan
force-pushed
the
krille/fix-set-power-level
branch
from
1fd31c2
to
3de1c17
Compare
| : (powerMap['users'] = <String, Object?>{}); | ||
| Future<String> setPower(String userId, int power) async { | ||
| final powerLevelMapCopy = | ||
| getState(EventTypes.RoomPowerLevels)?.content.copy() ?? {}; |
There was a problem hiding this comment.
Can you also apply this fix to enableGroupCalls() (and possibly a few others)?
|
|
||
| usersPowerMap[userID] = power; | ||
| powerLevelMapCopy['users'] ??= <String, Object?>{}; | ||
| powerLevelMapCopy.tryGetMap<String, Object?>('users')?[userId] = power; |
There was a problem hiding this comment.
This operates on a copy of powerLevelMapCopy, so this actually never modifies the power levels of that user! That's probably also why the tests hang, but for me at least the command test with the op command explicitly fails. Maybe we can add a proper test for this?
|
|
||
| usersPowerMap[userID] = power; | ||
| powerLevelMapCopy['users'] ??= <String, Object?>{}; | ||
| powerLevelMapCopy.tryGetMap<String, Object?>('users')?[userId] = power; | ||
|
|
||
| return await client.setRoomStateWithKey( |
There was a problem hiding this comment.
this actually now means that the changes will only be reflected in the state event later. I would bet there is some code that depends on this function having immediately reflected the changes. Should we consider this a breaking change or maybe wait for the event to come down in sync? Maybe we can just ignore that, but I think it makes sense to consider the impact of that. For example the following code would now only set the permissions for one of the users:
await room.setPower('@a:example.org', 10);
await room.setPower('@b:example.org', 10);
The problem here is that we
have not created a deep copy
of the power level map. By using .copy() we create a
deep copy now.
Closes https://github.com/famedly/product-management/issues/2412