From fb68fff536a9dec35d8cf14c786b93e2d8f471ff Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Mon, 16 Jan 2017 13:45:42 +0100 Subject: [PATCH 01/16] Refactor renderCommaSeparated for reuse --- .../views/elements/MemberEventListSummary.js | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index ab4a89eb69..975e2aebf3 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -56,17 +56,8 @@ module.exports = React.createClass({ if (users.length === 0) { return null; } - let originalNumber = users.length; - users = users.slice(0, this.props.summaryLength); - - let remaining = originalNumber - this.props.summaryLength; - if (remaining < 0) { - remaining = 0; - } - let other = " other" + (remaining > 1 ? "s" : ""); - - return this._renderCommaSeparatedList(users, remaining) + (remaining ? ' and ' + remaining + other : ''); + return this._renderCommaSeparatedList(users, this.props.summaryLength); }, // Test whether the first n items repeat for the duration @@ -81,14 +72,16 @@ module.exports = React.createClass({ return true; }, - _renderCommaSeparatedList(items, disableAnd) { - if (disableAnd) { - return items.join(', '); - } + _renderCommaSeparatedList(items, itemLimit) { + const remaining = itemLimit === undefined ? 0 : Math.max(items.length - itemLimit, 0); if (items.length === 0) { return ""; } else if (items.length === 1) { return items[0]; + } else if (remaining) { + items = items.slice(0, itemLimit); + const other = " other" + (remaining > 1 ? "s" : ""); + return items.join(', ') + ' and ' + remaining + other; } else { let last = items.pop(); return items.join(', ') + ' and ' + last; From 82d6805a718f4e7f1616c7552dae85e968091c74 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Mon, 16 Jan 2017 14:49:07 +0100 Subject: [PATCH 02/16] Canonicalise certain transition pairs, handle arbitrary consecutive transitions Transition pairs joined,left and left,joined are now transformed into single meta-transitions "joined_and_left" and "left_and_joined" respectively. These are described as "joined and left", "left and rejoined". Treat consecutive sequences of transitions as repetitions, and handle any arbitrary repetitions of transitions: ...,joined,left,joined,left,joined,left,... is canonicalised into ...,joined_and_left, joined_and_left, joined_and_left,... which is truncated and described as ... , joined and left 3 times, ... This also works if there are multiple consecutive sequences separated by other transitions: ..., banned, banned, banned, joined, unbanned, unbanned, unbanned,... becomes ... was banned 3 times, joined, was unbanned 3 times ... --- .../views/elements/MemberEventListSummary.js | 113 ++++++++++++------ 1 file changed, 78 insertions(+), 35 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 975e2aebf3..dc16127017 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -88,62 +88,105 @@ module.exports = React.createClass({ } }, - _getDescriptionForTransition(t, plural) { + _getDescriptionForTransition(t, plural, repeats) { let beConjugated = plural ? "were" : "was"; let invitation = plural ? "invitations" : "an invitation"; - switch (t) { - case 'joined': return "joined"; - case 'left': return "left"; - case 'invite_reject': return "rejected " + invitation; - case 'invite_withdrawal': return "withdrew " + invitation; - case 'invited': return beConjugated + " invited"; - case 'banned': return beConjugated + " banned"; - case 'unbanned': return beConjugated + " unbanned"; - case 'kicked': return beConjugated + " kicked"; + let res = null; + let map = { + "joined": "joined", + "left": "left", + "joined_and_left": "joined and left", + "left_and_joined": "left and rejoined", + "invite_reject": "rejected " + invitation, + "invite_withdrawal": "withdrew " + invitation, + "invited": beConjugated + " invited", + "banned": beConjugated + " banned", + "unbanned": beConjugated + " unbanned", + "kicked": beConjugated + " kicked", + }; + + if (Object.keys(map).includes(t)) { + res = map[t] + (repeats > 1 ? " " + repeats + " times" : "" ); } - return null; + return res; + }, + + _getCanonicalTransitions: function(transitions) { + let modMap = { + 'joined' : { + 'after' : 'left', + 'newTransition' : 'joined_and_left', + }, + 'left' : { + 'after' : 'joined', + 'newTransition' : 'left_and_joined', + }, + // $currentTransition : { + // 'after' : $nextTransition, + // 'newTransition' : 'new_transition_type', + // }, + }; + const res = []; + + for (let i = 0; i < transitions.length; i++) { + let t = transitions[i]; + let t2 = transitions[i + 1]; + + let transition = t; + + if (i < transitions.length - 1 && modMap[t] && modMap[t].after === t2) { + transition = modMap[t].newTransition; + i++; + } + + res.push(transition); + } + return res; + }, + + _getTruncatedTransitions: function(transitions) { + let res = []; + for (let i = 0; i < transitions.length; i++) { + if (res.length > 0 && res[res.length - 1].transitionType === transitions[i]) { + res[res.length - 1].repeats += 1; + } else { + res.push({ + transitionType: transitions[i], + repeats: 1, + }); + } + } + // returns [{ + // transitionType: "joined_and_left" + // repeats: 123 + // }, ... ] + return res; }, _renderSummary: function(eventAggregates) { let summaries = Object.keys(eventAggregates).map((transitions) => { let nameList = this._renderNameList(eventAggregates[transitions]); + let plural = eventAggregates[transitions].length > 1; let repeats = 1; let repeatExtra = 0; let splitTransitions = transitions.split(','); - let describedTransitions = splitTransitions; - let plural = eventAggregates[transitions].length > 1; - for (let modulus = 1; modulus <= 2; modulus++) { - // Sequences that are repeating through modulus transitions will be truncated - if (this._isRepeatedSequence(describedTransitions, modulus)) { - // Extra repeating sequence on the end that should be treated separately - // so as to avoid j,l,j,l,j => "... joined and left 2.5 times" - repeatExtra = describedTransitions.length % modulus; + // Some pairs of transitions are common and are repeated a lot, so canonicalise them into "pair" transitions + let canonicalTransitions = this._getCanonicalTransitions(splitTransitions); + // Remove consecutive repetitions of the same transition (like 5 consecutive 'join_and_leave's) + let truncatedTransitions = this._getTruncatedTransitions(canonicalTransitions); - repeats = (describedTransitions.length - repeatExtra) / modulus; - describedTransitions = describedTransitions.slice(0, modulus); - break; - } - } - - let numberOfTimes = repeats > 1 ? " " + repeats + " times" : ""; - - let descs = describedTransitions.map((t) => { - return this._getDescriptionForTransition(t, plural); - }); - - let afterRepeatDescs = splitTransitions.slice(splitTransitions.length - repeatExtra).map((t) => { - return this._getDescriptionForTransition(t, plural); + let descs = truncatedTransitions.map((t) => { + return this._getDescriptionForTransition(t.transitionType, plural, t.repeats); }); let desc = this._renderCommaSeparatedList(descs); - let afterRepeatDesc = this._renderCommaSeparatedList(afterRepeatDescs); - return nameList + " " + desc + numberOfTimes + (afterRepeatDesc ? " and then " + afterRepeatDesc : ""); + return nameList + " " + desc; }); if (!summaries) { From 4be444d52482883b4b87d6fc5cac851626b4cb50 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Mon, 16 Jan 2017 15:12:00 +0100 Subject: [PATCH 03/16] Move shouldComponentUpdate --- .../views/elements/MemberEventListSummary.js | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index dc16127017..5474865117 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -46,6 +46,19 @@ module.exports = React.createClass({ }; }, + shouldComponentUpdate: function(nextProps, nextState) { + // Update if + // - The number of summarised events has changed + // - or if the summary is currently expanded + // - or if the summary is about to toggle to become collapsed + // - or if there are fewEvents, meaning the child eventTiles are shown as-is + return ( + nextProps.events.length !== this.props.events.length || + this.state.expanded || nextState.expanded || + nextProps.events.length < this.props.threshold + ); + }, + _toggleSummary: function() { this.setState({ expanded: !this.state.expanded, @@ -214,19 +227,6 @@ module.exports = React.createClass({ ); }, - shouldComponentUpdate: function(nextProps, nextState) { - // Update if - // - The number of summarised events has changed - // - or if the summary is currently expanded - // - or if the summary is about to toggle to become collapsed - // - or if there are fewEvents, meaning the child eventTiles are shown as-is - return ( - nextProps.events.length !== this.props.events.length || - this.state.expanded || nextState.expanded || - nextProps.events.length < this.props.threshold - ); - }, - _getTransition: function(e) { switch (e.getContent().membership) { case 'invite': return 'invited'; From a79dc886ba5a146633aaf478587efb6ea35d1c90 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Mon, 16 Jan 2017 18:46:17 +0100 Subject: [PATCH 04/16] Order sequences by occurance of the first event in each sequence --- .../views/elements/MemberEventListSummary.js | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 5474865117..7adc4c2f85 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -178,8 +178,8 @@ module.exports = React.createClass({ return res; }, - _renderSummary: function(eventAggregates) { - let summaries = Object.keys(eventAggregates).map((transitions) => { + _renderSummary: function(eventAggregates, orderedTransitionSequences) { + let summaries = orderedTransitionSequences.map((transitions) => { let nameList = this._renderNameList(eventAggregates[transitions]); let plural = eventAggregates[transitions].length > 1; @@ -228,18 +228,18 @@ module.exports = React.createClass({ }, _getTransition: function(e) { - switch (e.getContent().membership) { + switch (e.mxEvent.getContent().membership) { case 'invite': return 'invited'; case 'ban': return 'banned'; case 'join': return 'joined'; case 'leave': - if (e.getSender() === e.getStateKey()) { - switch (e.getPrevContent().membership) { + if (e.mxEvent.getSender() === e.mxEvent.getStateKey()) { + switch (e.mxEvent.getPrevContent().membership) { case 'invite': return 'invite_reject'; default: return 'left'; } } - switch (e.getPrevContent().membership) { + switch (e.mxEvent.getPrevContent().membership) { case 'invite': return 'invite_withdrawal'; case 'ban': return 'unbanned'; case 'join': return 'kicked'; @@ -276,44 +276,64 @@ module.exports = React.createClass({ // $userId : [] }; - eventsToRender.forEach((e) => { + eventsToRender.forEach((e, index) => { const userId = e.getStateKey(); // Initialise a user's events if (!userEvents[userId]) { userEvents[userId] = []; } - userEvents[userId].push(e); + userEvents[userId].push({ + mxEvent: e, + displayName: e.getContent().displayname || userId, + index: index, + }); }); - // A map of agregate type to arrays of display names. Each aggregate type + // A map of aggregate type to arrays of display names. Each aggregate type // is a comma-delimited string of transitions, e.g. "joined,left,kicked". // The array of display names is the array of users who went through that // sequence during eventsToRender. let aggregate = { // $aggregateType : []:string }; + // A map of aggregate types to the indices that order them (the index of + // the first event for a given transition sequence) + let aggregateIndices = { + // $aggregateType : int + }; + let avatarMembers = []; let users = Object.keys(userEvents); users.forEach( (userId) => { - let displayName = userEvents[userId][0].getContent().displayname || userId; + let firstEvent = userEvents[userId][0]; + let displayName = firstEvent.displayName; let seq = this._getTransitionSequence(userEvents[userId]); if (!aggregate[seq]) { aggregate[seq] = []; + aggregateIndices[seq] = -1; } // Assumes display names are unique if (aggregate[seq].indexOf(displayName) === -1) { aggregate[seq].push(displayName); } - avatarMembers.push(userEvents[userId][0].target); + + if (aggregateIndices[seq] === -1 || firstEvent.index < aggregateIndices[seq]) { + aggregateIndices[seq] = firstEvent.index; + } + + avatarMembers.push(firstEvent.mxEvent.target); } ); + // Sort types by order of lowest event index within sequence + let orderedTransitionSequences = Object.keys(aggregate).sort((seq1, seq2) => aggregateIndices[seq1] > aggregateIndices[seq2]); + let avatars = this._renderAvatars(avatarMembers); - let summary = this._renderSummary(aggregate); + let summary = this._renderSummary(aggregate, orderedTransitionSequences); let toggleButton = ( {expanded ? 'collapse' : 'expand'} From 5ab287fa1ad1f2f5456bf160b988a15b4d882a4c Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Mon, 16 Jan 2017 18:57:49 +0100 Subject: [PATCH 05/16] Use pre-calculated displaynames to handle dupes --- src/components/views/elements/MemberEventListSummary.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 7adc4c2f85..425404dab9 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -284,7 +284,7 @@ module.exports = React.createClass({ } userEvents[userId].push({ mxEvent: e, - displayName: e.getContent().displayname || userId, + displayName: e.target.name || userId, index: index, }); }); From aa6e168505ef139c67d66270bf69cf3750a7e85e Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Mon, 16 Jan 2017 18:58:53 +0100 Subject: [PATCH 06/16] Remove comment --- src/components/views/elements/MemberEventListSummary.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 425404dab9..e9ec793953 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -316,7 +316,6 @@ module.exports = React.createClass({ aggregateIndices[seq] = -1; } - // Assumes display names are unique if (aggregate[seq].indexOf(displayName) === -1) { aggregate[seq].push(displayName); } From 45655f4de3611ce9391175abc22988d719cbf8ec Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Tue, 17 Jan 2017 12:01:19 +0100 Subject: [PATCH 07/16] Modified desc for invitation rejections, withdrawals --- src/components/views/elements/MemberEventListSummary.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index e9ec793953..1f05ba000e 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -103,7 +103,7 @@ module.exports = React.createClass({ _getDescriptionForTransition(t, plural, repeats) { let beConjugated = plural ? "were" : "was"; - let invitation = plural ? "invitations" : "an invitation"; + let invitation = "their invitation" + (plural || (repeats > 1) ? "s" : ""); let res = null; let map = { @@ -112,7 +112,7 @@ module.exports = React.createClass({ "joined_and_left": "joined and left", "left_and_joined": "left and rejoined", "invite_reject": "rejected " + invitation, - "invite_withdrawal": "withdrew " + invitation, + "invite_withdrawal": "had " + invitation + " withdrawn", "invited": beConjugated + " invited", "banned": beConjugated + " banned", "unbanned": beConjugated + " unbanned", From ade7c65617cb43bf654f3421a64a75afbc0f393e Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Tue, 17 Jan 2017 12:01:54 +0100 Subject: [PATCH 08/16] Add test for MemberEventListSummary --- .../elements/MemberEventListSummary-test.js | 556 ++++++++++++++++++ 1 file changed, 556 insertions(+) create mode 100644 test/components/views/elements/MemberEventListSummary-test.js diff --git a/test/components/views/elements/MemberEventListSummary-test.js b/test/components/views/elements/MemberEventListSummary-test.js new file mode 100644 index 0000000000..af4d92fa61 --- /dev/null +++ b/test/components/views/elements/MemberEventListSummary-test.js @@ -0,0 +1,556 @@ +const expect = require('expect'); +const React = require('react'); +const ReactDOM = require("react-dom"); +const ReactTestUtils = require('react-addons-test-utils'); +const sdk = require('matrix-react-sdk'); +const MemberEventListSummary = sdk.getComponent('views.elements.MemberEventListSummary'); + +const testUtils = require('../../../test-utils'); +describe.only('MemberEventListSummary', function() { + let sandbox; + let parentDiv; + + const generateTiles = (events) => { + return events.map((e) => { + return ( +
+ Expanded membership +
+ ); + }); + }; + + const generateMembershipEvent = (eventId, parameters) => { + let membership = parameters.membership; + let userId = parameters.userId; + let prevMembership = parameters.prevMembership; + let senderId = parameters.senderId; + return { + content: { + membership: membership, + }, + target: { + name: userId.match(/@([^:]*):/)[1], + getAvatarUrl: () => { + return "avatar.jpeg"; + }, + userId: userId, + }, + getId: () => { + return eventId; + }, + getContent: function() { + return this.content; + }, + getPrevContent: function() { + return { + membership: prevMembership ? prevMembership : this.content, + }; + }, + getSender: () => { + return senderId || userId; + }, + getStateKey: () => { + return userId; + }, + }; + }; + + const generateEvents = (parameters) => { + const res = []; + for (let i = 0; i < parameters.length; i++) { + res.push(generateMembershipEvent(`event${i}`, parameters[i])); + } + return res; + }; + + const generateEventsForUsers = (userIdTemplate, n, events) => { + let eventsForUsers = []; + let userId = ""; + for (let i = 0; i < n; i++) { + userId = userIdTemplate.replace('$', i); + events.forEach((e) => { + e.userId = userId; + return e; + }); + eventsForUsers = eventsForUsers.concat(generateEvents(events)); + } + return eventsForUsers; + }; + + beforeEach(function() { + testUtils.beforeEach(this); + sandbox = testUtils.stubClient(); + parentDiv = document.createElement('div'); + document.body.appendChild(parentDiv); + }); + + afterEach(function() { + sandbox.restore(); + }); + + it('renders expanded events if there are less than props.threshold', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const renderer = ReactTestUtils.createRenderer(); + renderer.render(); + const result = renderer.getRenderOutput(); + + expect(result.type).toBe('div'); + expect(result.props.children).toEqual([ +
Expanded membership
, + ]); + done(); + }); + + it('renders expanded events if there are less than props.threshold', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const renderer = ReactTestUtils.createRenderer(); + renderer.render(); + const result = renderer.getRenderOutput(); + + expect(result.type).toBe('div'); + expect(result.props.children).toEqual([ +
Expanded membership
, +
Expanded membership
, + ]); + done(); + }); + + it('renders collapsed events if events.length = props.threshold', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe("user_1 joined and left and joined"); + + done(); + }); + + it('truncates long join,leave repetitions', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe("user_1 joined and left 7 times"); + + done(); + }); + + it('truncates long join,leave repetitions inbetween other events', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "invite", senderId: "@some_other_user:some.domain"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe("user_1 was unbanned, joined and left 7 times and was invited"); + + done(); + }); + + it('truncates multiple sequences of repetitions with other events inbetween', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, + {userId : "@user_1:some.domain", prevMembership: "ban", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "invite", senderId: "@some_other_user:some.domain"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe("user_1 was unbanned, joined and left 2 times, was banned, joined and left 3 times and was invited"); + + done(); + }); + + it('handles multple users following the same sequence of memberships', function(done) { + const events = generateEvents([ + // user_1 + {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, + // user_2 + {userId : "@user_2:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_2:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_2:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_2:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_2:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_2:some.domain", prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe("user_1 and 1 other were unbanned, joined and left 2 times and were banned"); + + done(); + }); + + it('handles multple users following the same sequence of memberships', function(done) { + const events = generateEvents([ + // user_1 + {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, + // user_2 + {userId : "@user_2:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_2:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_2:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_2:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_2:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_2:some.domain", prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe("user_1 and 1 other were unbanned, joined and left 2 times and were banned"); + + done(); + }); + + it('handles many users following the same sequence of memberships', function(done) { + const events = generateEventsForUsers("@user_$:some.domain", 20, [ + {prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {prevMembership: "leave", membership: "join"}, + {prevMembership: "join", membership: "leave"}, + {prevMembership: "leave", membership: "join"}, + {prevMembership: "join", membership: "leave"}, + {prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe("user_0 and 19 others were unbanned, joined and left 2 times and were banned"); + + done(); + }); + + it('correctly orders sequences of transitions by the order of their first event', function(done) { + const events = generateEvents([ + {userId : "@user_2:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, + {userId : "@user_2:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_2:some.domain", prevMembership: "join", membership: "leave"}, + {userId : "@user_2:some.domain", prevMembership: "leave", membership: "join"}, + {userId : "@user_2:some.domain", prevMembership: "join", membership: "leave"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe( + "user_2 was unbanned and joined and left 2 times, user_1 was unbanned, joined and left 2 times and was banned" + ); + + done(); + }); + + it('correctly identifies transitions', function(done) { + const events = generateEvents([ + // invited + {userId : "@user_1:some.domain", membership: "invite"}, + // banned + {userId : "@user_1:some.domain", membership: "ban"}, + // joined + {userId : "@user_1:some.domain", membership: "join"}, + // invite_reject + {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave"}, + // left + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, + // invite_withdrawal + {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave", senderId: "@some_other_user:some.domain"}, + // unbanned + {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, + // kicked + {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave", senderId: "@some_other_user:some.domain"}, + // default = left + {userId : "@user_1:some.domain", prevMembership: "????", membership: "leave", senderId: "@some_other_user:some.domain"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe( + "user_1 was invited, was banned, joined, rejected their invitation, left, had their invitation withdrawn, was unbanned, was kicked and left" + ); + + done(); + }); + + it('handles invitation plurals correctly when there are multiple users', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave", senderId: "@some_other_user:some.domain"}, + {userId : "@user_2:some.domain", prevMembership: "invite", membership: "leave"}, + {userId : "@user_2:some.domain", prevMembership: "invite", membership: "leave", senderId: "@some_other_user:some.domain"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe( + "user_1 and 1 other rejected their invitations and had their invitations withdrawn" + ); + + done(); + }); + + it('handles invitation plurals correctly when there are multiple invites', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave"}, + {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 1, + avatarsMaxLength : 5, + threshold : 1, // threshold = 1 to force collapse + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe( + "user_1 rejected their invitations 2 times" + ); + + done(); + }); + + it('handles a summary length = 2, with no "others"', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", membership: "join"}, + {userId : "@user_1:some.domain", membership: "join"}, + {userId : "@user_2:some.domain", membership: "join"}, + {userId : "@user_2:some.domain", membership: "join"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 2, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe( + "user_1 and user_2 joined 2 times" + ); + + done(); + }); + + it('handles a summary length = 2, with 1 "other"', function(done) { + const events = generateEvents([ + {userId : "@user_1:some.domain", membership: "join"}, + {userId : "@user_2:some.domain", membership: "join"}, + {userId : "@user_3:some.domain", membership: "join"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 2, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe( + "user_1, user_2 and 1 other joined" + ); + + done(); + }); + + it('handles a summary length = 2, with many "others"', function(done) { + const events = generateEventsForUsers("@user_$:some.domain", 20, [ + {membership: "join"}, + ]); + const props = { + events : events, + children : generateTiles(events), + summaryLength : 2, + avatarsMaxLength : 5, + threshold : 3, + }; + + const instance = ReactDOM.render(, parentDiv); + const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); + const summaryText = summary.innerText; + + expect(summaryText).toBe( + "user_0, user_1 and 18 others joined" + ); + + done(); + }); +}); \ No newline at end of file From 49f2b9df88c33187020900174e1ced591d66e035 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Tue, 17 Jan 2017 18:53:38 +0100 Subject: [PATCH 09/16] Remove duplicate test --- .../elements/MemberEventListSummary-test.js | 36 +------------------ 1 file changed, 1 insertion(+), 35 deletions(-) diff --git a/test/components/views/elements/MemberEventListSummary-test.js b/test/components/views/elements/MemberEventListSummary-test.js index af4d92fa61..39a79eaf6c 100644 --- a/test/components/views/elements/MemberEventListSummary-test.js +++ b/test/components/views/elements/MemberEventListSummary-test.js @@ -263,41 +263,7 @@ describe.only('MemberEventListSummary', function() { done(); }); - it('handles multple users following the same sequence of memberships', function(done) { - const events = generateEvents([ - // user_1 - {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, - {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, - {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, - {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, - {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, - {userId : "@user_1:some.domain", prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, - // user_2 - {userId : "@user_2:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, - {userId : "@user_2:some.domain", prevMembership: "leave", membership: "join"}, - {userId : "@user_2:some.domain", prevMembership: "join", membership: "leave"}, - {userId : "@user_2:some.domain", prevMembership: "leave", membership: "join"}, - {userId : "@user_2:some.domain", prevMembership: "join", membership: "leave"}, - {userId : "@user_2:some.domain", prevMembership: "leave", membership: "ban", senderId: "@some_other_user:some.domain"}, - ]); - const props = { - events : events, - children : generateTiles(events), - summaryLength : 1, - avatarsMaxLength : 5, - threshold : 3, - }; - - const instance = ReactDOM.render(, parentDiv); - const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); - const summaryText = summary.innerText; - - expect(summaryText).toBe("user_1 and 1 other were unbanned, joined and left 2 times and were banned"); - - done(); - }); - - it('handles multple users following the same sequence of memberships', function(done) { + it('handles multiple users following the same sequence of memberships', function(done) { const events = generateEvents([ // user_1 {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, From 9574a0b663f9e4e6e4e013e6fe73707f2d41d831 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Tue, 17 Jan 2017 18:56:57 +0100 Subject: [PATCH 10/16] Remove pointless length guard --- src/components/views/elements/MemberEventListSummary.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 1f05ba000e..945a5c28fd 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -66,10 +66,6 @@ module.exports = React.createClass({ }, _renderNameList: function(users) { - if (users.length === 0) { - return null; - } - return this._renderCommaSeparatedList(users, this.props.summaryLength); }, From 3ba9f5087320122f4af807b123b00224d4d23e0a Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Tue, 17 Jan 2017 19:07:45 +0100 Subject: [PATCH 11/16] Move functions around, remove redundancies, add docs --- .../views/elements/MemberEventListSummary.js | 159 +++++++++--------- .../elements/MemberEventListSummary-test.js | 2 +- 2 files changed, 81 insertions(+), 80 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 945a5c28fd..0c2861a023 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -69,57 +69,44 @@ module.exports = React.createClass({ return this._renderCommaSeparatedList(users, this.props.summaryLength); }, - // Test whether the first n items repeat for the duration - // e.g. [1,2,3,4,1,2,3] would resolve true for n = 4 - _isRepeatedSequence: function(transitions, n) { - let count = 0; - for (let i = 0; i < transitions.length; i++) { - if (transitions[i % n] !== transitions[i]) { - return null; - } + _renderSummary: function(eventAggregates, orderedTransitionSequences) { + let summaries = orderedTransitionSequences.map((transitions) => { + let nameList = this._renderNameList(eventAggregates[transitions]); + let plural = eventAggregates[transitions].length > 1; + + let splitTransitions = transitions.split(','); + + // Some pairs of transitions are common and are repeated a lot, so canonicalise them into "pair" transitions + let canonicalTransitions = this._getCanonicalTransitions(splitTransitions); + // Remove consecutive repetitions of the same transition (like 5 consecutive 'join_and_leave's) + let truncatedTransitions = this._getTruncatedTransitions(canonicalTransitions); + + let descs = truncatedTransitions.map((t) => { + return this._getDescriptionForTransition(t.transitionType, plural, t.repeats); + }); + + let desc = this._renderCommaSeparatedList(descs); + + return nameList + " " + desc; + }); + + if (!summaries) { + return null; } - return true; + + return ( + + {summaries.join(", ")} + + ); }, - _renderCommaSeparatedList(items, itemLimit) { - const remaining = itemLimit === undefined ? 0 : Math.max(items.length - itemLimit, 0); - if (items.length === 0) { - return ""; - } else if (items.length === 1) { - return items[0]; - } else if (remaining) { - items = items.slice(0, itemLimit); - const other = " other" + (remaining > 1 ? "s" : ""); - return items.join(', ') + ' and ' + remaining + other; - } else { - let last = items.pop(); - return items.join(', ') + ' and ' + last; - } - }, - - _getDescriptionForTransition(t, plural, repeats) { - let beConjugated = plural ? "were" : "was"; - let invitation = "their invitation" + (plural || (repeats > 1) ? "s" : ""); - - let res = null; - let map = { - "joined": "joined", - "left": "left", - "joined_and_left": "joined and left", - "left_and_joined": "left and rejoined", - "invite_reject": "rejected " + invitation, - "invite_withdrawal": "had " + invitation + " withdrawn", - "invited": beConjugated + " invited", - "banned": beConjugated + " banned", - "unbanned": beConjugated + " unbanned", - "kicked": beConjugated + " kicked", - }; - - if (Object.keys(map).includes(t)) { - res = map[t] + (repeats > 1 ? " " + repeats + " times" : "" ); + _renderNameList: function(users) { + if (users.length === 0) { + return null; } - return res; + return this._renderCommaSeparatedList(users, this.props.summaryLength); }, _getCanonicalTransitions: function(transitions) { @@ -174,39 +161,53 @@ module.exports = React.createClass({ return res; }, - _renderSummary: function(eventAggregates, orderedTransitionSequences) { - let summaries = orderedTransitionSequences.map((transitions) => { - let nameList = this._renderNameList(eventAggregates[transitions]); - let plural = eventAggregates[transitions].length > 1; + /** + * For a certain transition, t, describe what happened to the users that + * underwent the transition. + * @param {string} t the transition type + * @param {boolean} plural whether there were multiple users undergoing the same transition + * @param {number} repeats the number of times the transition was repeated in a row + * @returns {string} the spoken English equivalent of the transition + */ + _getDescriptionForTransition(t, plural, repeats) { + let beConjugated = plural ? "were" : "was"; + let invitation = "their invitation" + (plural || (repeats > 1) ? "s" : ""); - let repeats = 1; - let repeatExtra = 0; + let res = null; + let map = { + "joined": "joined", + "left": "left", + "joined_and_left": "joined and left", + "left_and_joined": "left and rejoined", + "invite_reject": "rejected " + invitation, + "invite_withdrawal": "had " + invitation + " withdrawn", + "invited": beConjugated + " invited", + "banned": beConjugated + " banned", + "unbanned": beConjugated + " unbanned", + "kicked": beConjugated + " kicked", + }; - let splitTransitions = transitions.split(','); - - // Some pairs of transitions are common and are repeated a lot, so canonicalise them into "pair" transitions - let canonicalTransitions = this._getCanonicalTransitions(splitTransitions); - // Remove consecutive repetitions of the same transition (like 5 consecutive 'join_and_leave's) - let truncatedTransitions = this._getTruncatedTransitions(canonicalTransitions); - - let descs = truncatedTransitions.map((t) => { - return this._getDescriptionForTransition(t.transitionType, plural, t.repeats); - }); - - let desc = this._renderCommaSeparatedList(descs); - - return nameList + " " + desc; - }); - - if (!summaries) { - return null; + if (Object.keys(map).includes(t)) { + res = map[t] + (repeats > 1 ? " " + repeats + " times" : "" ); } - return ( - - {summaries.join(", ")} - - ); + return res; + }, + + _renderCommaSeparatedList(items, itemLimit) { + const remaining = itemLimit === undefined ? 0 : Math.max(items.length - itemLimit, 0); + if (items.length === 0) { + return ""; + } else if (items.length === 1) { + return items[0]; + } else if (remaining) { + items = items.slice(0, itemLimit); + const other = " other" + (remaining > 1 ? "s" : ""); + return items.join(', ') + ' and ' + remaining + other; + } else { + let last = items.pop(); + return items.join(', ') + ' and ' + last; + } }, _renderAvatars: function(roomMembers) { @@ -223,6 +224,10 @@ module.exports = React.createClass({ ); }, + _getTransitionSequence: function(events) { + return events.map(this._getTransition); + }, + _getTransition: function(e) { switch (e.mxEvent.getContent().membership) { case 'invite': return 'invited'; @@ -245,10 +250,6 @@ module.exports = React.createClass({ } }, - _getTransitionSequence: function(events) { - return events.map(this._getTransition); - }, - render: function() { let eventsToRender = this.props.events; let fewEvents = eventsToRender.length < this.props.threshold; diff --git a/test/components/views/elements/MemberEventListSummary-test.js b/test/components/views/elements/MemberEventListSummary-test.js index 39a79eaf6c..61e0f9627b 100644 --- a/test/components/views/elements/MemberEventListSummary-test.js +++ b/test/components/views/elements/MemberEventListSummary-test.js @@ -6,7 +6,7 @@ const sdk = require('matrix-react-sdk'); const MemberEventListSummary = sdk.getComponent('views.elements.MemberEventListSummary'); const testUtils = require('../../../test-utils'); -describe.only('MemberEventListSummary', function() { +describe('MemberEventListSummary', function() { let sandbox; let parentDiv; From 484549e50be36266c581d4583c84c9c55db1e5cf Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Wed, 18 Jan 2017 10:26:25 +0100 Subject: [PATCH 12/16] Refactor a few things and document everything --- .../views/elements/MemberEventListSummary.js | 108 +++++++++++++----- 1 file changed, 82 insertions(+), 26 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 0c2861a023..41307969d6 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -65,20 +65,27 @@ module.exports = React.createClass({ }); }, - _renderNameList: function(users) { - return this._renderCommaSeparatedList(users, this.props.summaryLength); - }, - + /** + * Render the JSX for users aggregated by their transition sequences (`eventAggregates`) where + * the sequences are ordered by `orderedTransitionSequences`. + * @param {object[]} eventAggregates a map of transition sequence to array of user display names + * or user IDs. + * @param {string[]} orderedTransitionSequences an array which is some ordering of + * `Object.keys(eventAggregates)`. + * @returns {ReactElement} a single containing the textual summary of the aggregated + * events that occurred. + */ _renderSummary: function(eventAggregates, orderedTransitionSequences) { let summaries = orderedTransitionSequences.map((transitions) => { - let nameList = this._renderNameList(eventAggregates[transitions]); - let plural = eventAggregates[transitions].length > 1; + let userNames = eventAggregates[transitions]; + let nameList = this._renderNameList(userNames); + let plural = userNames.length > 1; let splitTransitions = transitions.split(','); - // Some pairs of transitions are common and are repeated a lot, so canonicalise them into "pair" transitions + // Some neighbouring transitions are common, so canonicalise some into "pair" transitions let canonicalTransitions = this._getCanonicalTransitions(splitTransitions); - // Remove consecutive repetitions of the same transition (like 5 consecutive 'join_and_leave's) + // Transform into consecutive repetitions of the same transition (like 5 consecutive 'joined_and_left's) let truncatedTransitions = this._getTruncatedTransitions(canonicalTransitions); let descs = truncatedTransitions.map((t) => { @@ -101,14 +108,30 @@ module.exports = React.createClass({ ); }, + /** + * @param {string[]} users an array of user display names or user IDs. + * @returns {string} a comma-separated list that ends with "and [n] others" if there are + * more items in `users` than `this.props.summaryLength`, which is the number of names + * included before "and [n] others". + */ _renderNameList: function(users) { - if (users.length === 0) { - return null; - } - return this._renderCommaSeparatedList(users, this.props.summaryLength); }, + /** + * Canonicalise an array of transitions into an array of transitions and how many times + * they are repeated consecutively. + * + * An array of 123 "joined_and_left" transitions, would result in: + * ``` + * [{ + * transitionType: "joined_and_left" + * repeats: 123 + * }, ... ] + * ``` + * @param {string[]} transitions the array of transitions to transform. + * @returns {object[]} an array of truncated transitions. + */ _getCanonicalTransitions: function(transitions) { let modMap = { 'joined' : { @@ -142,6 +165,20 @@ module.exports = React.createClass({ return res; }, + /** + * Transform an array of transitions into an array of transitions and how many times + * they are repeated consecutively. + * + * An array of 123 "joined_and_left" transitions, would result in: + * ``` + * [{ + * transitionType: "joined_and_left" + * repeats: 123 + * }, ... ] + * ``` + * @param {string[]} transitions the array of transitions to transform. + * @returns {object[]} an array of truncated transitions. + */ _getTruncatedTransitions: function(transitions) { let res = []; for (let i = 0; i < transitions.length; i++) { @@ -154,20 +191,16 @@ module.exports = React.createClass({ }); } } - // returns [{ - // transitionType: "joined_and_left" - // repeats: 123 - // }, ... ] return res; }, /** * For a certain transition, t, describe what happened to the users that * underwent the transition. - * @param {string} t the transition type - * @param {boolean} plural whether there were multiple users undergoing the same transition - * @param {number} repeats the number of times the transition was repeated in a row - * @returns {string} the spoken English equivalent of the transition + * @param {string} t the transition type. + * @param {boolean} plural whether there were multiple users undergoing the same transition. + * @param {number} repeats the number of times the transition was repeated in a row. + * @returns {string} the written English equivalent of the transition. */ _getDescriptionForTransition(t, plural, repeats) { let beConjugated = plural ? "were" : "was"; @@ -194,6 +227,16 @@ module.exports = React.createClass({ return res; }, + /** + * Constructs a written English string representing `items`, with an optional limit on the number + * of items included in the result. If specified and if the length of `items` is greater than the + * limit, the string "and n others" will be appended onto the result. + * If `items` is empty, returns the empty string. If there is only one item, return it. + * @param {string[]} items the items to construct a string from. + * @param {number?} itemLimit the number by which to limit the list. + * @returns {string} a string constructed by joining `items` with a comma between each + * item, but with the last item appended as " and [lastItem]". + */ _renderCommaSeparatedList(items, itemLimit) { const remaining = itemLimit === undefined ? 0 : Math.max(items.length - itemLimit, 0); if (items.length === 0) { @@ -228,6 +271,14 @@ module.exports = React.createClass({ return events.map(this._getTransition); }, + /** + * Enumerate a given membership event, `e`, where `getContent().membership` has + * changed for each transition allowed by the Matrix protocol. This attempts to + * enumerate the membership changes that occur in `../../../TextForEvent.js`. + * @param {MatrixEvent} e the membership change event to enumerate. + * @returns {string?} the transition type given to this event. This defaults to `null` + * if a transition is not recognised. + */ _getTransition: function(e) { switch (e.mxEvent.getContent().membership) { case 'invite': return 'invited'; @@ -268,16 +319,25 @@ module.exports = React.createClass({ ); } - // Map user IDs to all of the user's member events in eventsToRender + // Map user IDs to an array of objects: let userEvents = { - // $userId : [] + // $userId : [{ + // // The original event + // mxEvent: e, + // // The display name of the user (if not, then user ID) + // displayName: e.target.name || userId, + // // The original index of the event in this.props.events + // index: index, + // }] }; + let avatarMembers = []; eventsToRender.forEach((e, index) => { const userId = e.getStateKey(); // Initialise a user's events if (!userEvents[userId]) { userEvents[userId] = []; + avatarMembers.push(e.target); } userEvents[userId].push({ mxEvent: e, @@ -299,8 +359,6 @@ module.exports = React.createClass({ // $aggregateType : int }; - let avatarMembers = []; - let users = Object.keys(userEvents); users.forEach( (userId) => { @@ -320,8 +378,6 @@ module.exports = React.createClass({ if (aggregateIndices[seq] === -1 || firstEvent.index < aggregateIndices[seq]) { aggregateIndices[seq] = firstEvent.index; } - - avatarMembers.push(firstEvent.mxEvent.target); } ); From 5dd1512ff27e7299d89d604a91ebfbedff441780 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Wed, 18 Jan 2017 10:59:19 +0100 Subject: [PATCH 13/16] Move aggregation code to dedicated function --- .../views/elements/MemberEventListSummary.js | 79 ++++++++++--------- .../elements/MemberEventListSummary-test.js | 2 +- 2 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index 41307969d6..57686344ed 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -301,6 +301,46 @@ module.exports = React.createClass({ } }, + _getAggregate: function(userEvents) { + // A map of aggregate type to arrays of display names. Each aggregate type + // is a comma-delimited string of transitions, e.g. "joined,left,kicked". + // The array of display names is the array of users who went through that + // sequence during eventsToRender. + let aggregate = { + // $aggregateType : []:string + }; + // A map of aggregate types to the indices that order them (the index of + // the first event for a given transition sequence) + let aggregateIndices = { + // $aggregateType : int + }; + + let users = Object.keys(userEvents); + users.forEach( + (userId) => { + let firstEvent = userEvents[userId][0]; + let displayName = firstEvent.displayName; + + let seq = this._getTransitionSequence(userEvents[userId]); + if (!aggregate[seq]) { + aggregate[seq] = []; + aggregateIndices[seq] = -1; + } + + aggregate[seq].push(displayName); + + if (aggregateIndices[seq] === -1 || firstEvent.index < aggregateIndices[seq]) { + aggregateIndices[seq] = firstEvent.index; + } + } + ); + + return { + names: aggregate, + indices: aggregateIndices, + }; + }, + render: function() { let eventsToRender = this.props.events; let fewEvents = eventsToRender.length < this.props.threshold; @@ -346,46 +386,13 @@ module.exports = React.createClass({ }); }); - // A map of aggregate type to arrays of display names. Each aggregate type - // is a comma-delimited string of transitions, e.g. "joined,left,kicked". - // The array of display names is the array of users who went through that - // sequence during eventsToRender. - let aggregate = { - // $aggregateType : []:string - }; - // A map of aggregate types to the indices that order them (the index of - // the first event for a given transition sequence) - let aggregateIndices = { - // $aggregateType : int - }; - - let users = Object.keys(userEvents); - users.forEach( - (userId) => { - let firstEvent = userEvents[userId][0]; - let displayName = firstEvent.displayName; - - let seq = this._getTransitionSequence(userEvents[userId]); - if (!aggregate[seq]) { - aggregate[seq] = []; - aggregateIndices[seq] = -1; - } - - if (aggregate[seq].indexOf(displayName) === -1) { - aggregate[seq].push(displayName); - } - - if (aggregateIndices[seq] === -1 || firstEvent.index < aggregateIndices[seq]) { - aggregateIndices[seq] = firstEvent.index; - } - } - ); + let aggregate = this._getAggregate(userEvents); // Sort types by order of lowest event index within sequence - let orderedTransitionSequences = Object.keys(aggregate).sort((seq1, seq2) => aggregateIndices[seq1] > aggregateIndices[seq2]); + let orderedTransitionSequences = Object.keys(aggregate.names).sort((seq1, seq2) => aggregate.indices[seq1] > aggregate.indices[seq2]); let avatars = this._renderAvatars(avatarMembers); - let summary = this._renderSummary(aggregate, orderedTransitionSequences); + let summary = this._renderSummary(aggregate.names, orderedTransitionSequences); let toggleButton = (
{expanded ? 'collapse' : 'expand'} diff --git a/test/components/views/elements/MemberEventListSummary-test.js b/test/components/views/elements/MemberEventListSummary-test.js index 61e0f9627b..39a79eaf6c 100644 --- a/test/components/views/elements/MemberEventListSummary-test.js +++ b/test/components/views/elements/MemberEventListSummary-test.js @@ -6,7 +6,7 @@ const sdk = require('matrix-react-sdk'); const MemberEventListSummary = sdk.getComponent('views.elements.MemberEventListSummary'); const testUtils = require('../../../test-utils'); -describe('MemberEventListSummary', function() { +describe.only('MemberEventListSummary', function() { let sandbox; let parentDiv; From 78e2c787e08b7fef7d50cf02cf9aceb907d8670a Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Wed, 18 Jan 2017 11:53:17 +0100 Subject: [PATCH 14/16] Refactor and document test helpers. --- .../elements/MemberEventListSummary-test.js | 64 ++++++++++--------- test/test-utils.js | 12 +++- 2 files changed, 45 insertions(+), 31 deletions(-) diff --git a/test/components/views/elements/MemberEventListSummary-test.js b/test/components/views/elements/MemberEventListSummary-test.js index 39a79eaf6c..93c085f88c 100644 --- a/test/components/views/elements/MemberEventListSummary-test.js +++ b/test/components/views/elements/MemberEventListSummary-test.js @@ -4,12 +4,15 @@ const ReactDOM = require("react-dom"); const ReactTestUtils = require('react-addons-test-utils'); const sdk = require('matrix-react-sdk'); const MemberEventListSummary = sdk.getComponent('views.elements.MemberEventListSummary'); +var jssdk = require('matrix-js-sdk'); +var MatrixEvent = jssdk.MatrixEvent; const testUtils = require('../../../test-utils'); describe.only('MemberEventListSummary', function() { let sandbox; let parentDiv; + // Generate dummy event tiles for use in simulating an expanded MELS const generateTiles = (events) => { return events.map((e) => { return ( @@ -20,42 +23,41 @@ describe.only('MemberEventListSummary', function() { }); }; + /** + * Generates a membership event with the target of the event set as a mocked RoomMember based + * on `parameters.userId`. + * @param {string} eventId the ID of the event. + * @param {object} parameters the parameters to use to create the event. + * @param {string} parameters.membership the membership to assign to `content.membership` + * @param {string} parameters.userId the state key and target userId of the event. If + * `parameters.senderId` is not specified, this is also used as the event sender. + * @param {string} parameters.prevMembership the membership to assign to + * `prev_content.membership`. + * @param {string} parameters.senderId the user ID of the sender of the event. Optional. + * Defaults to `parameters.userId`. + * @returns {MatrixEvent} the event created. + */ const generateMembershipEvent = (eventId, parameters) => { - let membership = parameters.membership; - let userId = parameters.userId; - let prevMembership = parameters.prevMembership; - let senderId = parameters.senderId; - return { - content: { - membership: membership, - }, - target: { - name: userId.match(/@([^:]*):/)[1], + let e = testUtils.mkMembership({ + event: true, + user: parameters.senderId || parameters.userId, + skey: parameters.userId, + mship: parameters.membership, + prevMship: parameters.prevMembership, + target : { + name: parameters.userId.match(/@([^:]*):/)[1], // Use localpart as display name + userId: parameters.userId, getAvatarUrl: () => { return "avatar.jpeg"; }, - userId: userId, }, - getId: () => { - return eventId; - }, - getContent: function() { - return this.content; - }, - getPrevContent: function() { - return { - membership: prevMembership ? prevMembership : this.content, - }; - }, - getSender: () => { - return senderId || userId; - }, - getStateKey: () => { - return userId; - }, - }; + }); + // Override random event ID + e.event.event_id = eventId; + return e; }; + // Generate mock MatrixEvents from the array of parameters const generateEvents = (parameters) => { const res = []; for (let i = 0; i < parameters.length; i++) { @@ -64,6 +66,9 @@ describe.only('MemberEventListSummary', function() { return res; }; + // Generate the same sequence of `events` for `n` users, where each user ID + // is created by replacing the first "$" in userIdTemplate with `i` for + // `i = 0 .. n`. const generateEventsForUsers = (userIdTemplate, n, events) => { let eventsForUsers = []; let userId = ""; @@ -71,7 +76,6 @@ describe.only('MemberEventListSummary', function() { userId = userIdTemplate.replace('$', i); events.forEach((e) => { e.userId = userId; - return e; }); eventsForUsers = eventsForUsers.concat(generateEvents(events)); } diff --git a/test/test-utils.js b/test/test-utils.js index db405c2e1a..cdfae4421c 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -108,6 +108,7 @@ export function mkEvent(opts) { room_id: opts.room, sender: opts.user, content: opts.content, + prev_content: opts.prev_content, event_id: "$" + Math.random() + "-" + Math.random(), origin_server_ts: opts.ts, }; @@ -150,7 +151,9 @@ export function mkPresence(opts) { * @param {Object} opts Values for the membership. * @param {string} opts.room The room ID for the event. * @param {string} opts.mship The content.membership for the event. + * @param {string} opts.prevMship The prev_content.membership for the event. * @param {string} opts.user The user ID for the event. + * @param {RoomMember} opts.target The target of the event. * @param {string} opts.skey The other user ID for the event if applicable * e.g. for invites/bans. * @param {string} opts.name The content.displayname for the event. @@ -169,9 +172,16 @@ export function mkMembership(opts) { opts.content = { membership: opts.mship }; + if (opts.prevMship) { + opts.prev_content = { membership: opts.prevMship }; + } if (opts.name) { opts.content.displayname = opts.name; } if (opts.url) { opts.content.avatar_url = opts.url; } - return mkEvent(opts); + let e = mkEvent(opts); + if (opts.target) { + e.target = opts.target; + } + return e; }; /** From 867a532e5e5aa5bce40e2eec68c79d7c6c5b5352 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Wed, 18 Jan 2017 11:58:54 +0100 Subject: [PATCH 15/16] Remove parentDiv from tests --- .../elements/MemberEventListSummary-test.js | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/test/components/views/elements/MemberEventListSummary-test.js b/test/components/views/elements/MemberEventListSummary-test.js index 93c085f88c..325b1d6b17 100644 --- a/test/components/views/elements/MemberEventListSummary-test.js +++ b/test/components/views/elements/MemberEventListSummary-test.js @@ -10,7 +10,6 @@ var MatrixEvent = jssdk.MatrixEvent; const testUtils = require('../../../test-utils'); describe.only('MemberEventListSummary', function() { let sandbox; - let parentDiv; // Generate dummy event tiles for use in simulating an expanded MELS const generateTiles = (events) => { @@ -85,8 +84,6 @@ describe.only('MemberEventListSummary', function() { beforeEach(function() { testUtils.beforeEach(this); sandbox = testUtils.stubClient(); - parentDiv = document.createElement('div'); - document.body.appendChild(parentDiv); }); afterEach(function() { @@ -155,7 +152,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -189,7 +186,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -225,7 +222,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -258,7 +255,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -292,7 +289,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -318,7 +315,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -349,7 +346,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -389,7 +386,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -415,7 +412,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -439,7 +436,7 @@ describe.only('MemberEventListSummary', function() { threshold : 1, // threshold = 1 to force collapse }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -465,7 +462,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -490,7 +487,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; @@ -513,7 +510,7 @@ describe.only('MemberEventListSummary', function() { threshold : 3, }; - const instance = ReactDOM.render(, parentDiv); + const instance = ReactTestUtils.renderIntoDocument(); const summary = ReactTestUtils.findRenderedDOMComponentWithClass(instance, "mx_MemberEventListSummary_summary"); const summaryText = summary.innerText; From 41d2697e28719bd45ff041ca33c45d6867588ee8 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Wed, 18 Jan 2017 12:03:38 +0100 Subject: [PATCH 16/16] Remove `done`, const instead of var for `requier`s --- .../elements/MemberEventListSummary-test.js | 62 +++++-------------- 1 file changed, 17 insertions(+), 45 deletions(-) diff --git a/test/components/views/elements/MemberEventListSummary-test.js b/test/components/views/elements/MemberEventListSummary-test.js index 325b1d6b17..7094520f7b 100644 --- a/test/components/views/elements/MemberEventListSummary-test.js +++ b/test/components/views/elements/MemberEventListSummary-test.js @@ -4,8 +4,8 @@ const ReactDOM = require("react-dom"); const ReactTestUtils = require('react-addons-test-utils'); const sdk = require('matrix-react-sdk'); const MemberEventListSummary = sdk.getComponent('views.elements.MemberEventListSummary'); -var jssdk = require('matrix-js-sdk'); -var MatrixEvent = jssdk.MatrixEvent; +const jssdk = require('matrix-js-sdk'); +const MatrixEvent = jssdk.MatrixEvent; const testUtils = require('../../../test-utils'); describe.only('MemberEventListSummary', function() { @@ -90,7 +90,7 @@ describe.only('MemberEventListSummary', function() { sandbox.restore(); }); - it('renders expanded events if there are less than props.threshold', function(done) { + it('renders expanded events if there are less than props.threshold', function() { const events = generateEvents([ {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, ]); @@ -110,10 +110,9 @@ describe.only('MemberEventListSummary', function() { expect(result.props.children).toEqual([
Expanded membership
, ]); - done(); }); - it('renders expanded events if there are less than props.threshold', function(done) { + it('renders expanded events if there are less than props.threshold', function() { const events = generateEvents([ {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, @@ -135,10 +134,9 @@ describe.only('MemberEventListSummary', function() {
Expanded membership
,
Expanded membership
, ]); - done(); }); - it('renders collapsed events if events.length = props.threshold', function(done) { + it('renders collapsed events if events.length = props.threshold', function() { const events = generateEvents([ {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, @@ -157,11 +155,9 @@ describe.only('MemberEventListSummary', function() { const summaryText = summary.innerText; expect(summaryText).toBe("user_1 joined and left and joined"); - - done(); }); - it('truncates long join,leave repetitions', function(done) { + it('truncates long join,leave repetitions', function() { const events = generateEvents([ {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, {userId : "@user_1:some.domain", prevMembership: "join", membership: "leave"}, @@ -191,11 +187,9 @@ describe.only('MemberEventListSummary', function() { const summaryText = summary.innerText; expect(summaryText).toBe("user_1 joined and left 7 times"); - - done(); }); - it('truncates long join,leave repetitions inbetween other events', function(done) { + it('truncates long join,leave repetitions inbetween other events', function() { const events = generateEvents([ {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, @@ -227,11 +221,9 @@ describe.only('MemberEventListSummary', function() { const summaryText = summary.innerText; expect(summaryText).toBe("user_1 was unbanned, joined and left 7 times and was invited"); - - done(); }); - it('truncates multiple sequences of repetitions with other events inbetween', function(done) { + it('truncates multiple sequences of repetitions with other events inbetween', function() { const events = generateEvents([ {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, {userId : "@user_1:some.domain", prevMembership: "leave", membership: "join"}, @@ -260,11 +252,9 @@ describe.only('MemberEventListSummary', function() { const summaryText = summary.innerText; expect(summaryText).toBe("user_1 was unbanned, joined and left 2 times, was banned, joined and left 3 times and was invited"); - - done(); }); - it('handles multiple users following the same sequence of memberships', function(done) { + it('handles multiple users following the same sequence of memberships', function() { const events = generateEvents([ // user_1 {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, @@ -294,11 +284,9 @@ describe.only('MemberEventListSummary', function() { const summaryText = summary.innerText; expect(summaryText).toBe("user_1 and 1 other were unbanned, joined and left 2 times and were banned"); - - done(); }); - it('handles many users following the same sequence of memberships', function(done) { + it('handles many users following the same sequence of memberships', function() { const events = generateEventsForUsers("@user_$:some.domain", 20, [ {prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, {prevMembership: "leave", membership: "join"}, @@ -320,11 +308,9 @@ describe.only('MemberEventListSummary', function() { const summaryText = summary.innerText; expect(summaryText).toBe("user_0 and 19 others were unbanned, joined and left 2 times and were banned"); - - done(); }); - it('correctly orders sequences of transitions by the order of their first event', function(done) { + it('correctly orders sequences of transitions by the order of their first event', function() { const events = generateEvents([ {userId : "@user_2:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, {userId : "@user_1:some.domain", prevMembership: "ban", membership: "leave", senderId: "@some_other_user:some.domain"}, @@ -353,11 +339,9 @@ describe.only('MemberEventListSummary', function() { expect(summaryText).toBe( "user_2 was unbanned and joined and left 2 times, user_1 was unbanned, joined and left 2 times and was banned" ); - - done(); }); - it('correctly identifies transitions', function(done) { + it('correctly identifies transitions', function() { const events = generateEvents([ // invited {userId : "@user_1:some.domain", membership: "invite"}, @@ -393,11 +377,9 @@ describe.only('MemberEventListSummary', function() { expect(summaryText).toBe( "user_1 was invited, was banned, joined, rejected their invitation, left, had their invitation withdrawn, was unbanned, was kicked and left" ); - - done(); }); - it('handles invitation plurals correctly when there are multiple users', function(done) { + it('handles invitation plurals correctly when there are multiple users', function() { const events = generateEvents([ {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave"}, {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave", senderId: "@some_other_user:some.domain"}, @@ -419,11 +401,9 @@ describe.only('MemberEventListSummary', function() { expect(summaryText).toBe( "user_1 and 1 other rejected their invitations and had their invitations withdrawn" ); - - done(); }); - it('handles invitation plurals correctly when there are multiple invites', function(done) { + it('handles invitation plurals correctly when there are multiple invites', function() { const events = generateEvents([ {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave"}, {userId : "@user_1:some.domain", prevMembership: "invite", membership: "leave"}, @@ -443,11 +423,9 @@ describe.only('MemberEventListSummary', function() { expect(summaryText).toBe( "user_1 rejected their invitations 2 times" ); - - done(); }); - it('handles a summary length = 2, with no "others"', function(done) { + it('handles a summary length = 2, with no "others"', function() { const events = generateEvents([ {userId : "@user_1:some.domain", membership: "join"}, {userId : "@user_1:some.domain", membership: "join"}, @@ -469,11 +447,9 @@ describe.only('MemberEventListSummary', function() { expect(summaryText).toBe( "user_1 and user_2 joined 2 times" ); - - done(); }); - it('handles a summary length = 2, with 1 "other"', function(done) { + it('handles a summary length = 2, with 1 "other"', function() { const events = generateEvents([ {userId : "@user_1:some.domain", membership: "join"}, {userId : "@user_2:some.domain", membership: "join"}, @@ -494,11 +470,9 @@ describe.only('MemberEventListSummary', function() { expect(summaryText).toBe( "user_1, user_2 and 1 other joined" ); - - done(); }); - it('handles a summary length = 2, with many "others"', function(done) { + it('handles a summary length = 2, with many "others"', function() { const events = generateEventsForUsers("@user_$:some.domain", 20, [ {membership: "join"}, ]); @@ -517,7 +491,5 @@ describe.only('MemberEventListSummary', function() { expect(summaryText).toBe( "user_0, user_1 and 18 others joined" ); - - done(); }); }); \ No newline at end of file