From 63a7b86eac3489f28060e6132f25ed7df2506dd3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 20 Dec 2018 14:56:18 -0700 Subject: [PATCH 1/7] Flatten and simplify the memberlist sorting algorithm The previous algorithm had a bug where it was getting stuck on the power level comparison, leaving the memberlist incorrectly ordered. The new algorithm uses less branching to try and walk through the different cases instead. Additionally, the steps used to determine the order have changed slightly to better represent an active member list. This commit also includes changes to try and re-sort the member list more often during presence changes. Events are not always emitted, however. This may be a js-sdk bug but appears to happen prior to these changes as well. Fixes https://github.com/vector-im/riot-web/issues/6953 --- src/components/views/rooms/MemberList.js | 94 ++++++++++++++---------- 1 file changed, 55 insertions(+), 39 deletions(-) diff --git a/src/components/views/rooms/MemberList.js b/src/components/views/rooms/MemberList.js index 92c486825c..053d3cc1f5 100644 --- a/src/components/views/rooms/MemberList.js +++ b/src/components/views/rooms/MemberList.js @@ -68,7 +68,9 @@ module.exports = React.createClass({ // We listen for changes to the lastPresenceTs which is essentially // listening for all presence events (we display most of not all of // the information contained in presence events). - cli.on("User.lastPresenceTs", this.onUserLastPresenceTs); + cli.on("User.lastPresenceTs", this.onUserPresenceChange); + cli.on("User.presence", this.onUserPresenceChange); + cli.on("User.currentlyActive", this.onUserPresenceChange); // cli.on("Room.timeline", this.onRoomTimeline); }, @@ -81,7 +83,9 @@ module.exports = React.createClass({ cli.removeListener("Room.myMembership", this.onMyMembership); cli.removeListener("RoomState.events", this.onRoomStateEvent); cli.removeListener("Room", this.onRoom); - cli.removeListener("User.lastPresenceTs", this.onUserLastPresenceTs); + cli.removeListener("User.lastPresenceTs", this.onUserPresenceChange); + cli.removeListener("User.presence", this.onUserPresenceChange); + cli.removeListener("User.currentlyActive", this.onUserPresenceChange); } // cancel any pending calls to the rate_limited_funcs @@ -132,12 +136,12 @@ module.exports = React.createClass({ }; }, - onUserLastPresenceTs(event, user) { + onUserPresenceChange(event, user) { // Attach a SINGLE listener for global presence changes then locate the // member tile and re-render it. This is more efficient than every tile - // evar attaching their own listener. - // console.log("explicit presence from " + user.userId); + // ever attaching their own listener. const tile = this.refs[user.userId]; + console.log(`Got presence update for ${user.userId}. hasTile=${!!tile}`); if (tile) { this._updateList(); // reorder the membership list } @@ -267,7 +271,8 @@ module.exports = React.createClass({ if (!member) { return "(null)"; } else { - return "(" + member.name + ", " + member.powerLevel + ", " + member.user.lastActiveAgo + ", " + member.user.currentlyActive + ")"; + const u = member.user; + return "(" + member.name + ", " + member.powerLevel + ", " + (u ? u.lastActiveAgo : "") + ", " + (u ? u.getLastActiveTs() : "") + ", " + (u ? u.currentlyActive : "") + ")"; } }, @@ -275,48 +280,59 @@ module.exports = React.createClass({ // returns 0 if a and b are equivalent in ordering // returns positive if a comes after b. memberSort: function(memberA, memberB) { - // order by last active, with "active now" first. - // ...and then by power - // ...and then alphabetically. - // We could tiebreak instead by "last recently spoken in this room" if we wanted to. + // order by presence, with "active now" first. + // ...and then by power level + // ...and then by last active + // ...and then alphabetically. + // We could tiebreak instead by "last recently spoken in this room" if we wanted to. - const userA = memberA.user; - const userB = memberB.user; + console.log(`Comparing userA=${this.memberString(memberA)} userB=${this.memberString(memberB)}`); - // if (!userA || !userB) { - // console.log("comparing " + memberA.name + " user=" + memberA.user + " with " + memberB.name + " user=" + memberB.user); - // } + const userA = memberA.user; + const userB = memberB.user; - if (!userA && !userB) return 0; - if (userA && !userB) return -1; - if (!userA && userB) return 1; + if (!userA) console.log("!! MISSING USER FOR A-SIDE: " + memberA.name + " !!"); + if (!userB) console.log("!! MISSING USER FOR B-SIDE: " + memberB.name + " !!"); - // console.log("comparing " + this.memberString(memberA) + " and " + this.memberString(memberB)); + if (!userA && !userB) return 0; + if (userA && !userB) return -1; + if (!userA && userB) return 1; - if ((userA.currentlyActive && userB.currentlyActive) || !this._showPresence) { - // console.log(memberA.name + " and " + memberB.name + " are both active"); - if (memberA.powerLevel === memberB.powerLevel) { - // console.log(memberA + " and " + memberB + " have same power level"); - if (memberA.name && memberB.name) { - // console.log("comparing names: " + memberA.name + " and " + memberB.name); - const nameA = memberA.name[0] === '@' ? memberA.name.substr(1) : memberA.name; - const nameB = memberB.name[0] === '@' ? memberB.name.substr(1) : memberB.name; - return nameA.localeCompare(nameB); - } else { - return 0; - } - } else { - // console.log("comparing power: " + memberA.powerLevel + " and " + memberB.powerLevel); - return memberB.powerLevel - memberA.powerLevel; - } + // First by presence + if (this._showPresence) { + const convertPresence = (p) => p === 'unavailable' ? 'online' : p; + const presenceIndex = p => { + const order = ['active', 'online', 'offline']; + const idx = order.indexOf(convertPresence(p)); + return idx === -1 ? order.length : idx; // unknown states at the end + }; + + const idxA = presenceIndex(userA.currentlyActive ? 'active' : userA.presence); + const idxB = presenceIndex(userB.currentlyActive ? 'active' : userB.presence); + console.log(`userA_presenceGroup=${idxA} userB_presenceGroup=${idxB}`); + if (idxA !== idxB) { + console.log("Comparing on presence group - returning"); + return idxA - idxB; } + } - if (userA.currentlyActive && !userB.currentlyActive) return -1; - if (!userA.currentlyActive && userB.currentlyActive) return 1; + // Second by power level + if (memberA.powerLevel !== memberB.powerLevel) { + console.log("Comparing on power level - returning"); + return memberB.powerLevel - memberA.powerLevel; + } - // For now, let's just order things by timestamp. It's really annoying - // that a user disappears from sight just because they temporarily go offline + // Third by last active + if (this._showPresence && userA.getLastActiveTs() !== userB.getLastActiveTs) { + console.log("Comparing on last active timestamp - returning"); return userB.getLastActiveTs() - userA.getLastActiveTs(); + } + + // Fourth by name (alphabetical) + const nameA = memberA.name[0] === '@' ? memberA.name.substr(1) : memberA.name; + const nameB = memberB.name[0] === '@' ? memberB.name.substr(1) : memberB.name; + console.log(`Comparing userA_name=${nameA} against userB_name=${nameB} - returning`); + return nameA.localeCompare(nameB); }, onSearchQueryChanged: function(ev) { From 95844ebf9d795fa64a73ee710c5b0ea160273333 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 20 Dec 2018 14:57:27 -0700 Subject: [PATCH 2/7] Comment out debugging statements They are useful to have around, but not to have enabled all the time. --- src/components/views/rooms/MemberList.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/components/views/rooms/MemberList.js b/src/components/views/rooms/MemberList.js index 053d3cc1f5..dc97a3689c 100644 --- a/src/components/views/rooms/MemberList.js +++ b/src/components/views/rooms/MemberList.js @@ -141,7 +141,7 @@ module.exports = React.createClass({ // member tile and re-render it. This is more efficient than every tile // ever attaching their own listener. const tile = this.refs[user.userId]; - console.log(`Got presence update for ${user.userId}. hasTile=${!!tile}`); + // console.log(`Got presence update for ${user.userId}. hasTile=${!!tile}`); if (tile) { this._updateList(); // reorder the membership list } @@ -286,13 +286,13 @@ module.exports = React.createClass({ // ...and then alphabetically. // We could tiebreak instead by "last recently spoken in this room" if we wanted to. - console.log(`Comparing userA=${this.memberString(memberA)} userB=${this.memberString(memberB)}`); + // console.log(`Comparing userA=${this.memberString(memberA)} userB=${this.memberString(memberB)}`); const userA = memberA.user; const userB = memberB.user; - if (!userA) console.log("!! MISSING USER FOR A-SIDE: " + memberA.name + " !!"); - if (!userB) console.log("!! MISSING USER FOR B-SIDE: " + memberB.name + " !!"); + // if (!userA) console.log("!! MISSING USER FOR A-SIDE: " + memberA.name + " !!"); + // if (!userB) console.log("!! MISSING USER FOR B-SIDE: " + memberB.name + " !!"); if (!userA && !userB) return 0; if (userA && !userB) return -1; @@ -309,29 +309,29 @@ module.exports = React.createClass({ const idxA = presenceIndex(userA.currentlyActive ? 'active' : userA.presence); const idxB = presenceIndex(userB.currentlyActive ? 'active' : userB.presence); - console.log(`userA_presenceGroup=${idxA} userB_presenceGroup=${idxB}`); + // console.log(`userA_presenceGroup=${idxA} userB_presenceGroup=${idxB}`); if (idxA !== idxB) { - console.log("Comparing on presence group - returning"); + // console.log("Comparing on presence group - returning"); return idxA - idxB; } } // Second by power level if (memberA.powerLevel !== memberB.powerLevel) { - console.log("Comparing on power level - returning"); + // console.log("Comparing on power level - returning"); return memberB.powerLevel - memberA.powerLevel; } // Third by last active if (this._showPresence && userA.getLastActiveTs() !== userB.getLastActiveTs) { - console.log("Comparing on last active timestamp - returning"); + // console.log("Comparing on last active timestamp - returning"); return userB.getLastActiveTs() - userA.getLastActiveTs(); } // Fourth by name (alphabetical) const nameA = memberA.name[0] === '@' ? memberA.name.substr(1) : memberA.name; const nameB = memberB.name[0] === '@' ? memberB.name.substr(1) : memberB.name; - console.log(`Comparing userA_name=${nameA} against userB_name=${nameB} - returning`); + // console.log(`Comparing userA_name=${nameA} against userB_name=${nameB} - returning`); return nameA.localeCompare(nameB); }, From 77dfba8a225b9d74e1a366373a3d61af07d69b41 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 3 Jan 2019 21:55:52 -0700 Subject: [PATCH 3/7] Add unit tests for MemberList Particularly the ordering of the tiles. --- src/components/views/rooms/MemberList.js | 8 +- .../components/views/rooms/MemberList-test.js | 295 ++++++++++++++++++ 2 files changed, 301 insertions(+), 2 deletions(-) create mode 100644 test/components/views/rooms/MemberList-test.js diff --git a/src/components/views/rooms/MemberList.js b/src/components/views/rooms/MemberList.js index dc97a3689c..52201b31be 100644 --- a/src/components/views/rooms/MemberList.js +++ b/src/components/views/rooms/MemberList.js @@ -185,6 +185,10 @@ module.exports = React.createClass({ }, _updateList: new rate_limited_func(function() { + this._updateListNow(); + }, 500), + + _updateListNow: function() { // console.log("Updating memberlist"); const newState = { loading: false, @@ -193,7 +197,7 @@ module.exports = React.createClass({ newState.filteredJoinedMembers = this._filterMembers(newState.members, 'join', this.state.searchQuery); newState.filteredInvitedMembers = this._filterMembers(newState.members, 'invite', this.state.searchQuery); this.setState(newState); - }, 500), + }, getMembersWithUser: function() { if (!this.props.roomId) return []; @@ -272,7 +276,7 @@ module.exports = React.createClass({ return "(null)"; } else { const u = member.user; - return "(" + member.name + ", " + member.powerLevel + ", " + (u ? u.lastActiveAgo : "") + ", " + (u ? u.getLastActiveTs() : "") + ", " + (u ? u.currentlyActive : "") + ")"; + return "(" + member.name + ", " + member.powerLevel + ", " + (u ? u.lastActiveAgo : "") + ", " + (u ? u.getLastActiveTs() : "") + ", " + (u ? u.currentlyActive : "") + ", " + (u ? u.presence : "") + ")"; } }, diff --git a/test/components/views/rooms/MemberList-test.js b/test/components/views/rooms/MemberList-test.js new file mode 100644 index 0000000000..01d351c688 --- /dev/null +++ b/test/components/views/rooms/MemberList-test.js @@ -0,0 +1,295 @@ +import React from 'react'; +import ReactTestUtils from 'react-addons-test-utils'; +import ReactDOM from 'react-dom'; +import expect from 'expect'; +import lolex from 'lolex'; + +import * as TestUtils from 'test-utils'; + +import sdk from '../../../../src/index'; +import MatrixClientPeg from '../../../../src/MatrixClientPeg'; + +import {MatrixClient, Room, RoomMember, User} from 'matrix-js-sdk'; + +function generateRoomId() { + return '!' + Math.random().toString().slice(2, 10) + ':domain'; +} + + +describe('MemberList', () => { + function createRoom(opts) { + const room = new Room(generateRoomId(), null, client.getUserId()); + if (opts) { + Object.assign(room, opts); + } + return room; + } + + let parentDiv = null; + let sandbox = null; + let client = null; + let root = null; + let clock = null; + let memberListRoom; + let memberList = null; + + let adminUsers = []; + let moderatorUsers = []; + let defaultUsers = []; + + beforeEach(function () { + TestUtils.beforeEach(this); + sandbox = TestUtils.stubClient(sandbox); + client = MatrixClientPeg.get(); + client.hasLazyLoadMembersEnabled = () => false; + + clock = lolex.install(); + + parentDiv = document.createElement('div'); + document.body.appendChild(parentDiv); + + // Make room + memberListRoom = createRoom(); + expect(memberListRoom.roomId).toBeTruthy(); + + // Make users + const usersPerLevel = 2; + for (let i = 0; i < usersPerLevel; i++) { + const adminUser = new RoomMember(memberListRoom.roomId, `@admin${i}:localhost`); + adminUser.membership = "join"; + adminUser.powerLevel = 100; + adminUser.user = new User(adminUser.userId); + adminUser.user.currentlyActive = true; + adminUser.user.presence = 'online'; + adminUser.user.lastPresenceTs = 1000; + adminUser.user.lastActiveAgo = 10; + adminUsers.push(adminUser); + + const moderatorUser = new RoomMember(memberListRoom.roomId, `@moderator${i}:localhost`); + moderatorUser.membership = "join"; + moderatorUser.powerLevel = 50; + moderatorUser.user = new User(moderatorUser.userId); + moderatorUser.user.currentlyActive = true; + moderatorUser.user.presence = 'online'; + moderatorUser.user.lastPresenceTs = 1000; + moderatorUser.user.lastActiveAgo = 10; + moderatorUsers.push(moderatorUser); + + const defaultUser = new RoomMember(memberListRoom.roomId, `@default${i}:localhost`); + defaultUser.membership = "join"; + defaultUser.powerLevel = 0; + defaultUser.user = new User(defaultUser.userId); + defaultUser.user.currentlyActive = true; + defaultUser.user.presence = 'online'; + defaultUser.user.lastPresenceTs = 1000; + defaultUser.user.lastActiveAgo = 10; + defaultUsers.push(defaultUser); + } + + client.getRoom = (roomId) => { + if (roomId === memberListRoom.roomId) return memberListRoom; + else return null; + }; + memberListRoom.currentState = { + members: {}, + getStateEvents: () => [], // ignore 3pid invites + }; + for (const member of [...adminUsers, ...moderatorUsers, ...defaultUsers]) { + memberListRoom.currentState.members[member.userId] = member; + } + + const MemberList = sdk.getComponent('views.rooms.MemberList'); + const WrappedMemberList = TestUtils.wrapInMatrixClientContext(MemberList); + const gatherWrappedRef = (r) => { + memberList = r; + }; + root = ReactDOM.render(, parentDiv); + }); + + afterEach((done) => { + if (parentDiv) { + ReactDOM.unmountComponentAtNode(parentDiv); + parentDiv.remove(); + parentDiv = null; + } + sandbox.restore(); + + clock.uninstall(); + + done(); + }); + + function expectOrderedByPresenceAndPowerLevel(memberTiles, isPresenceEnabled) { + let prevMember = null; + for (const tile of memberTiles) { + const memberA = prevMember; + const memberB = tile.props.member; + prevMember = memberB; // just in case an expect fails, set this early + if (!memberA) { + continue; + } + + console.log("COMPARING A VS B:"); + console.log(memberList.memberString(memberA)); + console.log(memberList.memberString(memberB)); + + const userA = memberA.user; + const userB = memberB.user; + + let groupChange = false; + + if (isPresenceEnabled) { + const convertPresence = (p) => p === 'unavailable' ? 'online' : p; + const presenceIndex = p => { + const order = ['active', 'online', 'offline']; + const idx = order.indexOf(convertPresence(p)); + return idx === -1 ? order.length : idx; // unknown states at the end + }; + + const idxA = presenceIndex(userA.currentlyActive ? 'active' : userA.presence); + const idxB = presenceIndex(userB.currentlyActive ? 'active' : userB.presence); + console.log("Comparing presence groups..."); + expect(idxB).toBeGreaterThanOrEqual(idxA); + groupChange = idxA !== idxB; + } else { + console.log("Skipped presence groups"); + } + + if (!groupChange) { + console.log("Comparing power levels..."); + expect(memberA.powerLevel).toBeGreaterThanOrEqual(memberB.powerLevel); + groupChange = memberA.powerLevel !== memberB.powerLevel; + } else { + console.log("Skipping power level check due to group change"); + } + + if (!groupChange) { + if (isPresenceEnabled) { + console.log("Comparing last active timestamp..."); + expect(userB.getLastActiveTs()).toBeGreaterThanOrEqual(userA.getLastActiveTs()); + groupChange = userA.getLastActiveTs() !== userB.getLastActiveTs(); + } else { + console.log("Skipping last active timestamp"); + } + } else { + console.log("Skipping last active timestamp check due to group change"); + } + + if (!groupChange) { + const nameA = memberA.name[0] === '@' ? memberA.name.substr(1) : memberA.name; + const nameB = memberB.name[0] === '@' ? memberB.name.substr(1) : memberB.name; + const nameCompare = nameB.localeCompare(nameA); + console.log("Comparing name"); + expect(nameCompare).toBeGreaterThanOrEqual(0); + } else { + console.log("Skipping name check due to group change"); + } + } + } + + function itDoesOrderMembersCorrectly(enablePresence) { + const MemberTile = sdk.getComponent("rooms.MemberTile"); + describe('does order members correctly', () => { + // Note: even if presence is disabled, we still expect that the presence + // tests will pass. All expectOrderedByPresenceAndPowerLevel does is ensure + // the order is perceived correctly, regardless of what we did to the members. + + // Each of the 4 tests here is done to prove that the member list can meet + // all 4 criteria independently. Together, they should work. + + it('by presence state', () => { + // Intentionally pick users that will confuse the power level sorting + const activeUsers = [defaultUsers[0]]; + const onlineUsers = [adminUsers[0]]; + const offlineUsers = [...moderatorUsers, ...adminUsers.slice(1), ...defaultUsers.slice(1)]; + activeUsers.forEach((u) => { + u.user.currentlyActive = true; + u.user.presence = 'online'; + }); + onlineUsers.forEach((u) => { + u.user.currentlyActive = false; + u.user.presence = 'online'; + }); + offlineUsers.forEach((u) => { + u.user.currentlyActive = false; + u.user.presence = 'offline'; + }); + + // Bypass all the event listeners and skip to the good part + memberList._showPresence = enablePresence; + memberList._updateListNow(); + + const tiles = ReactTestUtils.scryRenderedComponentsWithType(root, MemberTile); + expectOrderedByPresenceAndPowerLevel(tiles, enablePresence); + }); + + it('by power level', () => { + // We already have admin, moderator, and default users so leave them alone + + // Bypass all the event listeners and skip to the good part + memberList._showPresence = enablePresence; + memberList._updateListNow(); + + const tiles = ReactTestUtils.scryRenderedComponentsWithType(root, MemberTile); + expectOrderedByPresenceAndPowerLevel(tiles, enablePresence); + }); + + it('by last active timestamp', () => { + // Intentionally pick users that will confuse the power level sorting + // lastActiveAgoTs == lastPresenceTs - lastActiveAgo + const activeUsers = [defaultUsers[0]]; + const semiActiveUsers = [adminUsers[0]]; + const inactiveUsers = [...moderatorUsers, ...adminUsers.slice(1), ...defaultUsers.slice(1)]; + activeUsers.forEach((u) => { + u.user.lastPresenceTs = 1000; + u.user.lastActiveAgo = 0; + }); + semiActiveUsers.forEach((u) => { + u.user.lastPresenceTs = 1000; + u.user.lastActiveAgo = 50; + }); + inactiveUsers.forEach((u) => { + u.user.lastPresenceTs = 1000; + u.user.lastActiveAgo = 100; + }); + + // Bypass all the event listeners and skip to the good part + memberList._showPresence = enablePresence; + memberList._updateListNow(); + + const tiles = ReactTestUtils.scryRenderedComponentsWithType(root, MemberTile); + expectOrderedByPresenceAndPowerLevel(tiles, enablePresence); + }); + + it('by name', () => { + // Intentionally put everyone on the same level to force a name comparison + const allUsers = [...adminUsers, ...moderatorUsers, ...defaultUsers]; + allUsers.forEach((u) => { + u.user.currentlyActive = true; + u.user.presence = "online"; + u.user.lastPresenceTs = 1000; + u.user.lastActiveAgo = 0; + u.powerLevel = 100; + }); + + // Bypass all the event listeners and skip to the good part + memberList._showPresence = enablePresence; + memberList._updateListNow(); + + const tiles = ReactTestUtils.scryRenderedComponentsWithType(root, MemberTile); + expectOrderedByPresenceAndPowerLevel(tiles, enablePresence); + }); + }); + } + + describe('when presence is enabled', () => { + itDoesOrderMembersCorrectly(true); + }); + + describe('when presence is not enabled', () => { + itDoesOrderMembersCorrectly(false); + }); +}); + + From 5a2113e1a9458cbbb346660f54725baeab70f4bd Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 3 Jan 2019 21:56:10 -0700 Subject: [PATCH 4/7] Fix a comparison bug in ordering the MemberList --- src/components/views/rooms/MemberList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/MemberList.js b/src/components/views/rooms/MemberList.js index 52201b31be..0924a5ec38 100644 --- a/src/components/views/rooms/MemberList.js +++ b/src/components/views/rooms/MemberList.js @@ -327,7 +327,7 @@ module.exports = React.createClass({ } // Third by last active - if (this._showPresence && userA.getLastActiveTs() !== userB.getLastActiveTs) { + if (this._showPresence && userA.getLastActiveTs() !== userB.getLastActiveTs()) { // console.log("Comparing on last active timestamp - returning"); return userB.getLastActiveTs() - userA.getLastActiveTs(); } From 34d5870a03e3149c77bedfa6bb7567f6ea4cea63 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 3 Jan 2019 22:07:09 -0700 Subject: [PATCH 5/7] Appease the linter --- test/components/views/rooms/MemberList-test.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/components/views/rooms/MemberList-test.js b/test/components/views/rooms/MemberList-test.js index 01d351c688..57adde1717 100644 --- a/test/components/views/rooms/MemberList-test.js +++ b/test/components/views/rooms/MemberList-test.js @@ -9,7 +9,7 @@ import * as TestUtils from 'test-utils'; import sdk from '../../../../src/index'; import MatrixClientPeg from '../../../../src/MatrixClientPeg'; -import {MatrixClient, Room, RoomMember, User} from 'matrix-js-sdk'; +import {Room, RoomMember, User} from 'matrix-js-sdk'; function generateRoomId() { return '!' + Math.random().toString().slice(2, 10) + ':domain'; @@ -37,7 +37,7 @@ describe('MemberList', () => { let moderatorUsers = []; let defaultUsers = []; - beforeEach(function () { + beforeEach(function (){ TestUtils.beforeEach(this); sandbox = TestUtils.stubClient(sandbox); client = MatrixClientPeg.get(); @@ -53,6 +53,9 @@ describe('MemberList', () => { expect(memberListRoom.roomId).toBeTruthy(); // Make users + adminUsers = []; + moderatorUsers = []; + defaultUsers = []; const usersPerLevel = 2; for (let i = 0; i < usersPerLevel; i++) { const adminUser = new RoomMember(memberListRoom.roomId, `@admin${i}:localhost`); @@ -104,7 +107,7 @@ describe('MemberList', () => { memberList = r; }; root = ReactDOM.render(, parentDiv); + wrappedRef={gatherWrappedRef} />, parentDiv); }); afterEach((done) => { From f59625f7bdfaef32a9cf740edc97d2532b362e74 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 3 Jan 2019 22:24:24 -0700 Subject: [PATCH 6/7] Fix last active test Time is backwards from all the other tests: larger is older, so we want LessThanOrEqual. Also ensure all the power levels are the same to prevent the sort algorithm from running a PL ordering. --- test/components/views/rooms/MemberList-test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/components/views/rooms/MemberList-test.js b/test/components/views/rooms/MemberList-test.js index 57adde1717..ae54ed42ea 100644 --- a/test/components/views/rooms/MemberList-test.js +++ b/test/components/views/rooms/MemberList-test.js @@ -170,7 +170,7 @@ describe('MemberList', () => { if (!groupChange) { if (isPresenceEnabled) { console.log("Comparing last active timestamp..."); - expect(userB.getLastActiveTs()).toBeGreaterThanOrEqual(userA.getLastActiveTs()); + expect(userB.getLastActiveTs()).toBeLessThanOrEqual(userA.getLastActiveTs()); groupChange = userA.getLastActiveTs() !== userB.getLastActiveTs(); } else { console.log("Skipping last active timestamp"); @@ -245,14 +245,17 @@ describe('MemberList', () => { const semiActiveUsers = [adminUsers[0]]; const inactiveUsers = [...moderatorUsers, ...adminUsers.slice(1), ...defaultUsers.slice(1)]; activeUsers.forEach((u) => { + u.powerLevel = 100; // set everyone to the same PL to avoid running that check u.user.lastPresenceTs = 1000; u.user.lastActiveAgo = 0; }); semiActiveUsers.forEach((u) => { + u.powerLevel = 100; u.user.lastPresenceTs = 1000; u.user.lastActiveAgo = 50; }); inactiveUsers.forEach((u) => { + u.powerLevel = 100; u.user.lastPresenceTs = 1000; u.user.lastActiveAgo = 100; }); From 0ebde5266ebbe63e3a1b74123e9388872cf66cf7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 3 Jan 2019 22:33:01 -0700 Subject: [PATCH 7/7] Appease the linter again --- test/components/views/rooms/MemberList-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/views/rooms/MemberList-test.js b/test/components/views/rooms/MemberList-test.js index ae54ed42ea..b9d96635a2 100644 --- a/test/components/views/rooms/MemberList-test.js +++ b/test/components/views/rooms/MemberList-test.js @@ -37,7 +37,7 @@ describe('MemberList', () => { let moderatorUsers = []; let defaultUsers = []; - beforeEach(function (){ + beforeEach(function() { TestUtils.beforeEach(this); sandbox = TestUtils.stubClient(sandbox); client = MatrixClientPeg.get();