Move functions around, remove redundancies, add docs

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
+    _renderSummary: function(eventAggregates, orderedTransitionSequences) {
-    // e.g. [1,2,3,4,1,2,3] would resolve true for n = 4
+        let summaries = orderedTransitionSequences.map((transitions) => {
-    _isRepeatedSequence: function(transitions, n) {
+            let nameList = this._renderNameList(eventAggregates[transitions]);
-        let count = 0;
+            let plural = eventAggregates[transitions].length > 1;
-        for (let i = 0; i < transitions.length; i++) {
+
-            if (transitions[i % n] !== transitions[i]) {
+            let splitTransitions = transitions.split(',');
-                return null;
+
-            }
+            // 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 (
+            <span>
+                {summaries.join(", ")}
+            </span>
+        );
     },
 
-    _renderCommaSeparatedList(items, itemLimit) {
+    _renderNameList: function(users) {
-        const remaining = itemLimit === undefined ? 0 : Math.max(items.length - itemLimit, 0);
+        if (users.length === 0) {
-        if (items.length === 0) {
+            return null;
-            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" : "" );
         }
 
-        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) => {
+     * For a certain transition, t, describe what happened to the users that
-            let nameList = this._renderNameList(eventAggregates[transitions]);
+     * underwent the transition.
-            let plural = eventAggregates[transitions].length > 1;
+     * @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 res = null;
-            let repeatExtra = 0;
+        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(',');
+        if (Object.keys(map).includes(t)) {
-
+            res = map[t] + (repeats > 1 ? " " + repeats + " times" : "" );
-            // 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 (
+        return res;
-            <span>
+    },
-                {summaries.join(", ")}
+
-            </span>
+    _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;

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;