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

Don't rely on members to query if syncing user can post to room #717

Merged
merged 1 commit into from
Sep 5, 2018

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Sep 3, 2018

if (power_levels_event) {
power_levels = power_levels_event.getContent();
events_levels = power_levels.events || {};

if (utils.isNumber(power_levels.state_default)) {
if (Number.isFinite(power_levels.state_default)) {
Copy link
Member

Choose a reason for hiding this comment

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

why the change?

Copy link
Contributor Author

@bwindels bwindels Sep 3, 2018

Choose a reason for hiding this comment

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

Better to use things that are now available in standard JS as opposed to rolling our own?

Copy link
Member

Choose a reason for hiding this comment

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

There is another utils.isNumber below we should probably change for consistency

@ara4n
Copy link
Member

ara4n commented Sep 3, 2018

lgtm modulo one random q :)

@bwindels
Copy link
Contributor Author

bwindels commented Sep 4, 2018

making changes to avoid looking up the member, don't merge yet.

@bwindels
Copy link
Contributor Author

bwindels commented Sep 4, 2018

ready for another look

@bwindels bwindels changed the title add method to query default room power levels Don't rely on members to query if syncing user can post to room Sep 4, 2018
as this might not be known for the syncing user.
instead, add a method to room which always knows the syncing user's membership
@bwindels bwindels merged commit 49badd9 into develop Sep 5, 2018
@bwindels bwindels deleted the bwindels/fixllroompermission branch September 5, 2018 11:07
dbkr added a commit to matrix-org/matrix-react-sdk that referenced this pull request Sep 6, 2018
This fixes the tests that broke with matrix-org/matrix-js-sdk#717

This is because of https://github.com/vector-im/riot-web/blob/master/test/app-tests/joining.js#L63
which prevents the DOM nodes from actually ending up in the DOM, even though the react components
get rendered. This means that WillMount and WillUnmount are called, but not DidMount.

Using WillMount is more symmertrical anyway since the resulting teardown code must be in
WillUnmount (since there is no DidUnmount).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants