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.

Sign up for GitHub

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

Jump to bottom

Do not honor string power levels #1754

Merged
merged 3 commits into from
Jun 30, 2021
Merged

Do not honor string power levels #1754

merged 3 commits into from
Jun 30, 2021

Conversation

aaronraimist
Copy link
Contributor

@aaronraimist aaronraimist commented Jun 23, 2021 •
edited
Loading

@aaronraimist aaronraimist mentioned this pull request Jun 23, 2021
Merged
@turt2live turt2live requested a review from a team June 23, 2021 06:32
jryans
jryans requested changes Jun 23, 2021
View reviewed changes
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Thanks, seems reasonable overall, but would be good to use the same checks in all power level paths.

src/models/room-member.ts Show resolved Hide resolved
@aaronraimist
Merge branch 'develop' into string-pl
047180e
@aaronraimist
Also prevent sending unless safe integer
55fb3d4 
Synapse enforces this but I guess it doesn't hurt to also check here

Signed-off-by: Aaron Raimist <[email protected]>
@aaronraimist
Copy link
Contributor Author

I'm not sure where else you want changed. As far as I can tell this is the only place Element uses the JS SDK to show a PL.

These string power levels aren't going to last a much longer anyway since I also made a PR to enforce integers in Synapse matrix-org/synapse#10232 so I don't think it's worth putting a whole lot more time in on the client side.

jryans
jryans approved these changes Jun 30, 2021
View reviewed changes
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

That's all I had in mind, thanks. Just seems good to have similar checks all around.

@jryans jryans merged commit e3a00c2 into matrix-org:develop Jun 30, 2021
@aaronraimist aaronraimist deleted the string-pl branch June 30, 2021 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jryans jryans jryans approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aaronraimist @jryans