From d91d1932f48cc641bf9acc50875a616b1232fa4d Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 28 Feb 2018 14:52:53 +0000 Subject: [PATCH 1/5] Add tests for RoomSettings For setting: - name - topic - history visibility - power levels Testing RoomSettings required more stubbing on the matrix client. The power level tests should be failing at this commit, with fixes being made in upcoming commits. Some tests are marked as known failures that we should fix but aren't necessarily bugs: - SettingStore.setValue is used when saving despite the user not having made a change. - Testing directory publicity changes cannot be tested because we update state asynchronously in componentWillMount (which we do not block on in beforeEach). Also, we needed to use `export default` to make sure everything uses the same client peg and client. --- src/MatrixClientPeg.js | 2 +- .../views/rooms/RoomSettings-test.js | 192 ++++++++++++++++++ test/test-utils.js | 14 +- 3 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 test/components/views/rooms/RoomSettings-test.js diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index 14dfa91fa4..99841c986e 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -175,4 +175,4 @@ class MatrixClientPeg { if (!global.mxMatrixClientPeg) { global.mxMatrixClientPeg = new MatrixClientPeg(); } -module.exports = global.mxMatrixClientPeg; +export default global.mxMatrixClientPeg; diff --git a/test/components/views/rooms/RoomSettings-test.js b/test/components/views/rooms/RoomSettings-test.js new file mode 100644 index 0000000000..12fb734bf4 --- /dev/null +++ b/test/components/views/rooms/RoomSettings-test.js @@ -0,0 +1,192 @@ +import React from 'react'; +import ReactTestUtils from 'react-addons-test-utils'; +import ReactDOM from 'react-dom'; +import expect, {createSpy} from 'expect'; +import sinon from 'sinon'; +import Promise from 'bluebird'; +import * as testUtils from '../../../test-utils'; +import sdk from 'matrix-react-sdk'; +const WrappedRoomSettings = testUtils.wrapInMatrixClientContext(sdk.getComponent('views.rooms.RoomSettings')); +import MatrixClientPeg from '../../../../src/MatrixClientPeg'; +import SettingsStore from '../../../../src/settings/SettingsStore'; + + +describe('RoomSettings', () => { + let parentDiv = null; + let sandbox = null; + let client = null; + let roomSettings = null; + const room = testUtils.mkStubRoom('!DdJkzRliezrwpNebLk:matrix.org'); + + function expectSentStateEvent(roomId, eventType, expectedEventContent) { + let found = false; + for (const call of client.sendStateEvent.calls) { + const [ + actualRoomId, + actualEventType, + actualEventContent, + ] = call.arguments.slice(0, 3); + + if (roomId === actualRoomId && actualEventType === eventType) { + expect(actualEventContent).toEqual(expectedEventContent); + found = true; + break; + } + } + expect(found).toBe(true); + } + + beforeEach(function(done) { + testUtils.beforeEach(this); + sandbox = testUtils.stubClient(); + client = MatrixClientPeg.get(); + client.credentials = {userId: '@me:domain.com'}; + + client.setRoomName = createSpy().andReturn(Promise.resolve()); + client.setRoomTopic = createSpy().andReturn(Promise.resolve()); + client.setRoomDirectoryVisibility = createSpy().andReturn(Promise.resolve()); + + // Covers any room state event (e.g. name, avatar, topic) + client.sendStateEvent = createSpy().andReturn(Promise.resolve()); + + // Covers room tagging + client.setRoomTag = createSpy().andReturn(Promise.resolve()); + client.deleteRoomTag = createSpy().andReturn(Promise.resolve()); + + // Covers any setting in the SettingsStore + // (including local client settings not stored via matrix) + SettingsStore.setValue = createSpy().andReturn(Promise.resolve()); + + parentDiv = document.createElement('div'); + document.body.appendChild(parentDiv); + + const gatherWrappedRef = (r) => {roomSettings = r;}; + + // get use wrappedRef because we're using wrapInMatrixClientContext + ReactDOM.render( + , + parentDiv, + done, + ); + }); + + afterEach((done) => { + if (parentDiv) { + ReactDOM.unmountComponentAtNode(parentDiv); + parentDiv.remove(); + parentDiv = null; + } + sandbox.restore(); + done(); + }); + + it('should not set when no setting is changed', (done) => { + roomSettings.save().then(() => { + expect(client.sendStateEvent).toNotHaveBeenCalled(); + expect(client.setRoomTag).toNotHaveBeenCalled(); + expect(client.deleteRoomTag).toNotHaveBeenCalled(); + done(); + }); + }); + + // XXX: Apparently we do call SettingsStore.setValue + xit('should not settings via the SettingsStore when no setting is changed', (done) => { + roomSettings.save().then(() => { + expect(SettingsStore.setValue).toNotHaveBeenCalled(); + done(); + }); + }); + + it('should set room name when it has changed', (done) => { + const name = "My Room Name"; + roomSettings.setName(name); + + roomSettings.save().then(() => { + expect(client.setRoomName.calls[0].arguments.slice(0, 2)) + .toEqual(['!DdJkzRliezrwpNebLk:matrix.org', name]); + + done(); + }); + }); + + it('should set room topic when it has changed', (done) => { + const topic = "this is a topic"; + roomSettings.setTopic(topic); + + roomSettings.save().then(() => { + expect(client.setRoomTopic.calls[0].arguments.slice(0, 2)) + .toEqual(['!DdJkzRliezrwpNebLk:matrix.org', topic]); + + done(); + }); + }); + + it('should set history visibility when it has changed', (done) => { + const historyVisibility = "translucent"; + roomSettings.setState({ + history_visibility: historyVisibility, + }); + + roomSettings.save().then(() => { + expectSentStateEvent( + "!DdJkzRliezrwpNebLk:matrix.org", + "m.room.history_visibility", {history_visibility: historyVisibility}, + ); + done(); + }); + }); + + // XXX: Can't test this because we `getRoomDirectoryVisibility` in `componentWillMount` + xit('should set room directory publicity when set to true', (done) => { + const isRoomPublished = true; + roomSettings.setState({ + isRoomPublished, + }, () => { + roomSettings.save().then(() => { + expect(client.setRoomDirectoryVisibility.calls[0].arguments.slice(0, 2)) + .toEqual("!DdJkzRliezrwpNebLk:matrix.org", isRoomPublished ? "public" : "private"); + done(); + }); + }); + }); + + it('should set power levels when changed', (done) => { + roomSettings.onPowerLevelsChanged(42, "invite"); + + roomSettings.save().then(() => { + expectSentStateEvent( + "!DdJkzRliezrwpNebLk:matrix.org", + "m.room.power_levels", { invite: 42 }, + ); + done(); + }); + }); + + it('should set event power levels when changed', (done) => { + roomSettings.onPowerLevelsChanged(42, "event_levels_m.room.message"); + + roomSettings.save().then(() => { + // We expect all state events to be set to the state_default (50) + // See powerLevelDescriptors in RoomSettings + expectSentStateEvent( + "!DdJkzRliezrwpNebLk:matrix.org", + "m.room.power_levels", { + events: { + 'm.room.message': 42, + 'm.room.avatar': 50, + 'm.room.name': 50, + 'm.room.canonical_alias': 50, + 'm.room.history_visibility': 50, + 'm.room.power_levels': 50, + 'm.room.topic': 50, + 'im.vector.modular.widgets': 50, + }, + }, + ); + done(); + }); + }); +}); diff --git a/test/test-utils.js b/test/test-utils.js index 5753c02665..b593761bd4 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -68,6 +68,8 @@ export function createTestClient() { return { getHomeserverUrl: sinon.stub(), getIdentityServerUrl: sinon.stub(), + getDomain: sinon.stub().returns("matrix.rog"), + getUserId: sinon.stub().returns("@userId:matrix.rog"), getPushActionsForEvent: sinon.stub(), getRoom: sinon.stub().returns(mkStubRoom()), @@ -81,6 +83,7 @@ export function createTestClient() { paginateEventTimeline: sinon.stub().returns(Promise.resolve()), sendReadReceipt: sinon.stub().returns(Promise.resolve()), getRoomIdForAlias: sinon.stub().returns(Promise.resolve()), + getRoomDirectoryVisibility: sinon.stub().returns(Promise.resolve()), getProfileInfo: sinon.stub().returns(Promise.resolve({})), getAccountData: (type) => { return mkEvent({ @@ -244,6 +247,7 @@ export function mkStubRoom(roomId = null) { roomId: roomId, getAvatarUrl: () => 'mxc://avatar.url/image.png', }), + getMembersWithMembership: sinon.stub().returns([]), getJoinedMembers: sinon.stub().returns([]), getPendingEvents: () => [], getLiveTimeline: () => stubTimeline, @@ -252,8 +256,16 @@ export function mkStubRoom(roomId = null) { hasMembershipState: () => null, currentState: { getStateEvents: sinon.stub(), + mayClientSendStateEvent: sinon.stub().returns(true), + maySendStateEvent: sinon.stub().returns(true), members: [], }, + tags: { + "m.favourite": { + order: 0.5, + }, + }, + setBlacklistUnverifiedDevices: sinon.stub(), }; } @@ -284,7 +296,7 @@ export function wrapInMatrixClientContext(WrappedComponent) { } render() { - return ; + return ; } } return Wrapper; From dd529791fbbf1e4fdcdf508e8ba79ede75db4234 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 28 Feb 2018 16:13:01 +0000 Subject: [PATCH 2/5] Remove unused prop --- src/components/views/rooms/RoomSettings.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/RoomSettings.js b/src/components/views/rooms/RoomSettings.js index ea4a29615e..3a5075ba92 100644 --- a/src/components/views/rooms/RoomSettings.js +++ b/src/components/views/rooms/RoomSettings.js @@ -117,7 +117,6 @@ module.exports = React.createClass({ propTypes: { room: PropTypes.object.isRequired, - onSaveClick: PropTypes.func, }, getInitialState: function() { From 9a72e69a43b092ff4425934cfe10fb1e291500be Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 28 Feb 2018 16:14:18 +0000 Subject: [PATCH 3/5] Handle lack of room directory visibility response --- src/components/views/rooms/RoomSettings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomSettings.js b/src/components/views/rooms/RoomSettings.js index 3a5075ba92..2bd2fbb8f1 100644 --- a/src/components/views/rooms/RoomSettings.js +++ b/src/components/views/rooms/RoomSettings.js @@ -150,7 +150,7 @@ module.exports = React.createClass({ MatrixClientPeg.get().getRoomDirectoryVisibility( this.props.room.roomId, - ).done((result) => { + ).done((result = {}) => { this.setState({ isRoomPublished: result.visibility === "public" }); this._originalIsRoomPublished = result.visibility === "public"; }, (err) => { From 567d83ba52719ea0f05ed01d2dcd301c713fad6e Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 28 Feb 2018 16:15:20 +0000 Subject: [PATCH 4/5] Update PowerSelector to support powerLevelKey prop As a key to send as second argument to onChange. This is useful when passing the same callback to multiple PowerSelectors. --- src/components/views/elements/PowerSelector.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/PowerSelector.js b/src/components/views/elements/PowerSelector.js index f8443c6ecd..d1f102d9fe 100644 --- a/src/components/views/elements/PowerSelector.js +++ b/src/components/views/elements/PowerSelector.js @@ -42,6 +42,9 @@ module.exports = React.createClass({ // should the user be able to change the value? false by default. disabled: PropTypes.bool, onChange: PropTypes.func, + + // Optional key to pass as the second argument to `onChange` + powerLevelKey: PropTypes.string, }, getInitialState: function() { @@ -84,17 +87,17 @@ module.exports = React.createClass({ onSelectChange: function(event) { this.setState({ custom: event.target.value === "SELECT_VALUE_CUSTOM" }); if (event.target.value !== "SELECT_VALUE_CUSTOM") { - this.props.onChange(event.target.value); + this.props.onChange(event.target.value, this.props.powerLevelKey); } }, onCustomBlur: function(event) { - this.props.onChange(parseInt(this.refs.custom.value)); + this.props.onChange(parseInt(this.refs.custom.value), this.props.powerLevelKey); }, onCustomKeyDown: function(event) { if (event.key == "Enter") { - this.props.onChange(parseInt(this.refs.custom.value)); + this.props.onChange(parseInt(this.refs.custom.value), this.props.powerLevelKey); } }, From d3dc2a33b48861139b3ecee0eb31537b6a58b4cf Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 28 Feb 2018 16:16:44 +0000 Subject: [PATCH 5/5] Fix bug preventing setting room power levels - don't use refs, use onChange of PowerSelector - store power levels as state in the RoomSetting component --- src/components/views/rooms/RoomSettings.js | 222 ++++++++++-------- .../views/rooms/RoomSettings-test.js | 2 - 2 files changed, 128 insertions(+), 96 deletions(-) diff --git a/src/components/views/rooms/RoomSettings.js b/src/components/views/rooms/RoomSettings.js index 2bd2fbb8f1..e2dad93698 100644 --- a/src/components/views/rooms/RoomSettings.js +++ b/src/components/views/rooms/RoomSettings.js @@ -131,7 +131,8 @@ module.exports = React.createClass({ join_rule: this._yankValueFromEvent("m.room.join_rules", "join_rule"), history_visibility: this._yankValueFromEvent("m.room.history_visibility", "history_visibility"), guest_access: this._yankValueFromEvent("m.room.guest_access", "guest_access"), - power_levels_changed: false, + powerLevels: this._yankContentFromEvent("m.room.power_levels", {}), + powerLevelsChanged: false, tags_changed: false, tags: tags, // isRoomPublished is loaded async in componentWillMount so when the component @@ -271,8 +272,8 @@ module.exports = React.createClass({ // power levels - const powerLevels = this._getPowerLevels(); - if (powerLevels) { + const powerLevels = this.state.powerLevels; + if (this.state.powerLevelsChanged) { promises.push(MatrixClientPeg.get().sendStateEvent( roomId, "m.room.power_levels", powerLevels, "", )); @@ -383,36 +384,32 @@ module.exports = React.createClass({ return strA !== strB; }, - _getPowerLevels: function() { - if (!this.state.power_levels_changed) return undefined; + onPowerLevelsChanged: function(value, powerLevelKey) { + const powerLevels = Object.assign({}, this.state.powerLevels); + const eventsLevelPrefix = "event_levels_"; - let powerLevels = this.props.room.currentState.getStateEvents('m.room.power_levels', ''); - powerLevels = powerLevels ? powerLevels.getContent() : {}; + value = parseInt(value); - for (const key of Object.keys(this.refs).filter((k) => k.startsWith("event_levels_"))) { - const eventType = key.substring("event_levels_".length); - powerLevels.events[eventType] = parseInt(this.refs[key].getValue()); + if (powerLevelKey.startsWith(eventsLevelPrefix)) { + // deep copy "events" object, Object.assign itself won't deep copy + powerLevels["events"] = Object.assign({}, this.state.powerLevels["events"] || {}); + powerLevels["events"][powerLevelKey.slice(eventsLevelPrefix.length)] = value; + } else { + powerLevels[powerLevelKey] = value; } - - const newPowerLevels = { - ban: parseInt(this.refs.ban.getValue()), - kick: parseInt(this.refs.kick.getValue()), - redact: parseInt(this.refs.redact.getValue()), - invite: parseInt(this.refs.invite.getValue()), - events_default: parseInt(this.refs.events_default.getValue()), - state_default: parseInt(this.refs.state_default.getValue()), - users_default: parseInt(this.refs.users_default.getValue()), - users: powerLevels.users, - events: powerLevels.events, - }; - - return newPowerLevels; + this.setState({ + powerLevels, + powerLevelsChanged: true, + }); }, - onPowerLevelsChanged: function() { - this.setState({ - power_levels_changed: true, - }); + _yankContentFromEvent: function(stateEventType, defaultValue) { + // E.g.("m.room.name") would yank the content of "m.room.name" + const event = this.props.room.currentState.getStateEvents(stateEventType, ''); + if (!event) { + return defaultValue; + } + return event.getContent() || defaultValue; }, _yankValueFromEvent: function(stateEventType, keyName, defaultValue) { @@ -632,29 +629,61 @@ module.exports = React.createClass({ const cli = MatrixClientPeg.get(); const roomState = this.props.room.currentState; - const user_id = cli.credentials.userId; + const myUserId = cli.credentials.userId; - const power_level_event = roomState.getStateEvents('m.room.power_levels', ''); - const power_levels = power_level_event ? power_level_event.getContent() : {}; - const events_levels = power_levels.events || {}; - const user_levels = power_levels.users || {}; + const powerLevels = this.state.powerLevels; + const eventsLevels = powerLevels.events || {}; + const userLevels = powerLevels.users || {}; - const ban_level = parseIntWithDefault(power_levels.ban, 50); - const kick_level = parseIntWithDefault(power_levels.kick, 50); - const redact_level = parseIntWithDefault(power_levels.redact, 50); - const invite_level = parseIntWithDefault(power_levels.invite, 50); - const send_level = parseIntWithDefault(power_levels.events_default, 0); - const state_level = power_level_event ? parseIntWithDefault(power_levels.state_default, 50) : 0; - const default_user_level = parseIntWithDefault(power_levels.users_default, 0); + const powerLevelDescriptors = { + users_default: { + desc: _t('The default role for new room members is'), + defaultValue: 0, + }, + events_default: { + desc: _t('To send messages, you must be a'), + defaultValue: 0, + }, + invite: { + desc: _t('To invite users into the room, you must be a'), + defaultValue: 50, + }, + state_default: { + desc: _t('To configure the room, you must be a'), + defaultValue: 50, + }, + kick: { + desc: _t('To kick users, you must be a'), + defaultValue: 50, + }, + ban: { + desc: _t('To ban users, you must be a'), + defaultValue: 50, + }, + redact: { + desc: _t('To remove other users\' messages, you must be a'), + defaultValue: 50, + }, + }; - this._populateDefaultPlEvents(events_levels, state_level, send_level); + const banLevel = parseIntWithDefault(powerLevels.ban, powerLevelDescriptors.ban.defaultValue); + const defaultUserLevel = parseIntWithDefault( + powerLevels.users_default, + powerLevelDescriptors.users_default.defaultValue, + ); - let current_user_level = user_levels[user_id]; - if (current_user_level === undefined) { - current_user_level = default_user_level; + this._populateDefaultPlEvents( + eventsLevels, + parseIntWithDefault(powerLevels.state_default, powerLevelDescriptors.state_default.defaultValue), + parseIntWithDefault(powerLevels.events_default, powerLevelDescriptors.events_default.defaultValue), + ); + + let currentUserLevel = userLevels[myUserId]; + if (currentUserLevel === undefined) { + currentUserLevel = defaultUserLevel; } - const can_change_levels = roomState.mayClientSendStateEvent("m.room.power_levels", cli); + const canChangeLevels = roomState.mayClientSendStateEvent("m.room.power_levels", cli); const canSetTag = !cli.isGuest(); @@ -667,15 +696,16 @@ module.exports = React.createClass({ />; let userLevelsSection; - if (Object.keys(user_levels).length) { + if (Object.keys(userLevels).length) { userLevelsSection =

{ _t('Privileged Users') }

    - { Object.keys(user_levels).map(function(user, i) { + { Object.keys(userLevels).map(function(user, i) { return (
  • - { _t("%(user)s is a", {user: user}) } + { _t("%(user)s is a", {user: user}) } +
    • ); }) } @@ -688,7 +718,7 @@ module.exports = React.createClass({ const banned = this.props.room.getMembersWithMembership("ban"); let bannedUsersSection; if (banned.length) { - const canBanUsers = current_user_level >= ban_level; + const canBanUsers = currentUserLevel >= banLevel; bannedUsersSection =

    { _t('Banned users') }

    @@ -710,13 +740,13 @@ module.exports = React.createClass({ if (this._yankValueFromEvent("m.room.create", "m.federate", true) === false) { unfederatableSection = (
    - { _t('This room is not accessible by remote Matrix servers') }. + { _t('This room is not accessible by remote Matrix servers') }.
    ); } let leaveButton = null; - const myMember = this.props.room.getMember(user_id); + const myMember = this.props.room.getMember(myUserId); if (myMember) { if (myMember.membership === "join") { leaveButton = ( @@ -799,6 +829,50 @@ module.exports = React.createClass({
    ; } + const powerSelectors = Object.keys(powerLevelDescriptors).map((key, index) => { + const descriptor = powerLevelDescriptors[key]; + + const value = parseIntWithDefault(powerLevels[key], descriptor.defaultValue); + return
    + + { descriptor.desc } + + +
    ; + }); + + const eventPowerSelectors = Object.keys(eventsLevels).map(function(eventType, i) { + let label = plEventsToLabels[eventType]; + if (label) { + label = _t(label); + } else { + label = _t( + "To send events of type , you must be a", {}, + { 'eventType': { eventType } }, + ); + } + return ( +
    + { label } + +
    + ); + }); + return (
    @@ -898,49 +972,9 @@ module.exports = React.createClass({

    { _t('Permissions') }

    -
    - { _t('The default role for new room members is') } - -
    -
    - { _t('To send messages, you must be a') } - -
    -
    - { _t('To invite users into the room, you must be a') } - -
    -
    - { _t('To configure the room, you must be a') } - -
    -
    - { _t('To kick users, you must be a') } - -
    -
    - { _t('To ban users, you must be a') } - -
    -
    - { _t('To remove other users\' messages, you must be a') } - -
    - - { Object.keys(events_levels).map(function(event_type, i) { - let label = plEventsToLabels[event_type]; - if (label) label = _t(label); - else label = _t("To send events of type , you must be a", {}, { 'eventType': { event_type } }); - return ( -
    - { label } - -
    - ); - }) } - - { unfederatableSection } + { powerSelectors } + { eventPowerSelectors } + { unfederatableSection }
    { userLevelsSection } diff --git a/test/components/views/rooms/RoomSettings-test.js b/test/components/views/rooms/RoomSettings-test.js index 12fb734bf4..ffcecf1725 100644 --- a/test/components/views/rooms/RoomSettings-test.js +++ b/test/components/views/rooms/RoomSettings-test.js @@ -1,8 +1,6 @@ import React from 'react'; -import ReactTestUtils from 'react-addons-test-utils'; import ReactDOM from 'react-dom'; import expect, {createSpy} from 'expect'; -import sinon from 'sinon'; import Promise from 'bluebird'; import * as testUtils from '../../../test-utils'; import sdk from 'matrix-react-sdk';