-
-
Notifications
You must be signed in to change notification settings - Fork 585
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
maySendRedactionForEvent for userId #435
Conversation
done using a private helper so kick/ban etc perms can be done easily at a later stage Signed-off-by: Michael Telatynski <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
`--max-warnings 115` :') Signed-off-by: Michael Telatynski <[email protected]>
src/models/room-state.js
Outdated
* @param {string} userId The user ID of the user to test permission for | ||
* @return {boolean} true if the given user ID has sufficient power level | ||
*/ | ||
RoomState.prototype._hasSufficientPowerLevelFor = function(type, userId) { |
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.
I'm unclear how this differs from maySendEvent ?
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.
maySendEvent
checks inside power_levels.events.X
yet this checks power_levels.X
Can't check whether we can redact using maySendEvent
src/models/room-state.js
Outdated
requiredLevel = powerLevels[type]; | ||
} | ||
|
||
return member.powerLevel >= requiredLevel; |
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.
member may be null/undef, and this will throw. Also: need to check for leave
membership
(Alternatively, if you're relying on maySendRedactionForEvent
having checked this already, just pass in member.powerlevel rather than the userid.)
src/models/room-state.js
Outdated
powerLevels = powerLevelsEvent.getContent(); | ||
} | ||
|
||
let requiredLevel = powerLevels.state_default || 50; |
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.
powerLevels may be undef, this will throw
default is 50, not state_default
src/models/room-state.js
Outdated
}; | ||
|
||
/** | ||
* Returns true if the given user ID has sufficient power level for type |
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.
type
confused me here: maybe action
?
Signed-off-by: Michael Telatynski <[email protected]>
} | ||
|
||
let requiredLevel = 50; | ||
if (powerLevels[action] !== undefined) { |
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 can still be undef?
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.
Now it can't :)
PTAL @richvdh
Signed-off-by: Michael Telatynski <[email protected]>
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.
lgtm
allows to more nicely fix element-hq/element-web/issues/273
Signed-off-by: Michael Telatynski [email protected]
edit:
d182fd6 means can't redact things with that string/enum set as they're not really complete