Fixes #1253: Show contacts with unread messages at the top of the roster

CHANGES.md

@@ -22,6 +22,7 @@
 
 - #129: Add support for [XEP-0156: Disovering Alternative XMPP Connection Methods](https://xmpp.org/extensions/xep-0156.html). Only XML is supported for now.
 - #1105: Support for storing persistent data in IndexedDB
+- #1253: Show contacts with unread messages at the top of the roster
 - #1322 Display occupants’ avatars in the occupants list
 - #1640: Add the ability to resize the occupants sidebar in MUCs
 - #1666: Allow scrolling of the OMEMO fingerprints list

spec/roster.js

@@ -370,6 +370,70 @@
 
         describe("A Roster Group", function () {
 
+            it("is created to show contacts with unread messages",
+                mock.initConverse(
+                    ['rosterGroupsFetched'], {'roster_groups': true},
+                    async function (done, _converse) {
+
+                spyOn(_converse.rosterview, 'update').and.callThrough();
+                _converse.rosterview.render();
+                await test_utils.openControlBox(_converse);
+                await test_utils.waitForRoster(_converse, 'all');
+                await test_utils.createContacts(_converse, 'requesting');
+
+
+                // Check that the groups appear alphabetically and that
+                // requesting and pending contacts are last.
+                await u.waitUntil(() => sizzle('.roster-group a.group-toggle', _converse.rosterview.el).length);
+                let group_titles = sizzle('.roster-group a.group-toggle', _converse.rosterview.el).map(o => o.textContent.trim());
+                expect(group_titles).toEqual([
+                    "Contact requests",
+                    "Colleagues",
+                    "Family",
+                    "friends & acquaintences",
+                    "ænemies",
+                    "Ungrouped",
+                    "Pending contacts"
+                ]);
+
+                const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
+                const contact = await _converse.api.contacts.get(contact_jid);
+                contact.save({'num_unread': 5});
+
+                await u.waitUntil(() => sizzle('.roster-group a.group-toggle', _converse.rosterview.el).length === 8);
+                group_titles = sizzle('.roster-group a.group-toggle', _converse.rosterview.el).map(o => o.textContent.trim());
+
+                expect(group_titles).toEqual([
+                    "New messages",
+                    "Contact requests",
+                    "Colleagues",
+                    "Family",
+                    "friends & acquaintences",
+                    "ænemies",
+                    "Ungrouped",
+                    "Pending contacts"
+                ]);
+                const contacts = sizzle('.roster-group[data-group="New messages"] li', _converse.rosterview.el);
+                expect(contacts.length).toBe(1);
+                expect(contacts[0].querySelector('.contact-name').textContent).toBe("Mercutio");
+                expect(contacts[0].querySelector('.msgs-indicator').textContent).toBe("5");
+
+                contact.save({'num_unread': 0});
+                await u.waitUntil(() => sizzle('.roster-group a.group-toggle', _converse.rosterview.el).length === 7);
+                group_titles = sizzle('.roster-group a.group-toggle', _converse.rosterview.el).map(o => o.textContent.trim());
+                expect(group_titles).toEqual([
+                    "Contact requests",
+                    "Colleagues",
+                    "Family",
+                    "friends & acquaintences",
+                    "ænemies",
+                    "Ungrouped",
+                    "Pending contacts"
+                ]);
+                done();
+            }));
+
+
             it("can be used to organize existing contacts",
                 mock.initConverse(
                     ['rosterGroupsFetched'], {'roster_groups': true},
@@ -396,7 +460,7 @@
                 // Check that usernames appear alphabetically per group
                 Object.keys(mock.groups).forEach(name  => {
                     const contacts = sizzle('.roster-group[data-group="'+name+'"] ul', _converse.rosterview.el);
-                    const names = _.map(contacts, o => o.textContent.trim());
+                    const names = contacts.map(o => o.textContent.trim());
                     expect(names).toEqual(_.clone(names).sort());
                 });
                 done();

src/converse-rosterview.js

@@ -423,9 +423,12 @@ converse.plugins.add('converse-rosterview', {
                 return this;
             },
 
-            highlight () {
-                /* If appropriate, highlight the contact (by adding the 'open' class).
+            /**
+             * If appropriate, highlight the contact (by adding the 'open' class).
+             * @private
+             * @method _converse.RosterContactView#highlight
              */
+            highlight () {
                 if (_converse.isUniView()) {
                     const chatbox = _converse.chatboxes.get(this.model.get('jid'));
                     if ((chatbox && chatbox.get('hidden')) || !chatbox) {
@@ -558,8 +561,23 @@ converse.plugins.add('converse-rosterview', {
 
             initialize () {
                 OrderedListView.prototype.initialize.apply(this, arguments);
-                this.listenTo(this.model.contacts, "change:subscription", this.onContactSubscriptionChange);
-                this.listenTo(this.model.contacts, "change:requesting", this.onContactRequestChange);
+
+                if (this.model.get('name') === _converse.HEADER_UNREAD) {
+                    this.listenTo(this.model.contacts, "change:num_unread",
+                        c => !this.model.get('unread_messages') && this.removeContact(c)
+                    );
+                }
+                if (this.model.get('name') === _converse.HEADER_REQUESTING_CONTACTS) {
+                    this.listenTo(this.model.contacts, "change:requesting",
+                        c => !c.get('requesting') && this.removeContact(c)
+                    );
+                }
+                if (this.model.get('name') === _converse.HEADER_PENDING_CONTACTS) {
+                    this.listenTo(this.model.contacts, "change:subscription",
+                        c => (c.get('subscription') !== 'from') && this.removeContact(c)
+                    );
+                }
+
                 this.listenTo(this.model.contacts, "remove", this.onRemove);
                 this.listenTo(_converse.roster, 'change:groups', this.onContactGroupChange);
 
@@ -639,11 +657,12 @@ converse.plugins.add('converse-rosterview', {
                 let matches;
                 q = q.toLowerCase();
                 if (type === 'state') {
-                    if (this.model.get('name') === _converse.HEADER_REQUESTING_CONTACTS) {
+                    const sticky_groups = [_converse.HEADER_REQUESTING_CONTACTS, _converse.HEADER_UNREAD];
+                    if (sticky_groups.includes(this.model.get('name'))) {
                         // When filtering by chat state, we still want to
-                        // show requesting contacts, even though they don't
-                        // have the state in question.
-                        matches = this.model.contacts.filter(c => !c.presence.get('show').includes(q) && !c.get('requesting'));
+                        // show sticky groups, even though they don't
+                        // match the state in question.
+                        return [];
                     } else if (q === 'unread_messages') {
                         matches = this.model.contacts.filter({'num_unread': 0});
                     } else if (q === 'online') {
@@ -710,18 +729,6 @@ converse.plugins.add('converse-rosterview', {
                 }
             },
 
-            onContactSubscriptionChange (contact) {
-                if ((this.model.get('name') === _converse.HEADER_PENDING_CONTACTS) && contact.get('subscription') !== 'from') {
-                    this.removeContact(contact);
-                }
-            },
-
-            onContactRequestChange (contact) {
-                if ((this.model.get('name') === _converse.HEADER_REQUESTING_CONTACTS) && !contact.get('requesting')) {
-                    this.removeContact(contact);
-                }
-            },
-
             removeContact (contact) {
                 // We suppress events, otherwise the remove event will
                 // also cause the contact's view to be removed from the
@@ -894,6 +901,9 @@ converse.plugins.add('converse-rosterview', {
                         this.addExistingContact(contact);
                     }
                 }
+                if (has(contact.changed, 'num_unread') && contact.get('num_unread')) {
+                    this.addContactToGroup(contact, _converse.HEADER_UNREAD);
+                }
                 if (has(contact.changed, 'ask') && contact.changed.ask === 'subscribe') {
                     this.addContactToGroup(contact, _converse.HEADER_PENDING_CONTACTS);
                 }
@@ -931,6 +941,9 @@ converse.plugins.add('converse-rosterview', {
                 } else {
                     groups = [_converse.HEADER_CURRENT_CONTACTS];
                 }
+                if (contact.get('num_unread')) {
+                    groups.append(_converse.HEADER_UNREAD);
+                }
                 groups.forEach(g => this.addContactToGroup(contact, g, options));
             },
 

src/headless/converse-roster.js

@@ -44,12 +44,14 @@ converse.plugins.add('converse-roster', {
         _converse.HEADER_PENDING_CONTACTS = __('Pending contacts');
         _converse.HEADER_REQUESTING_CONTACTS = __('Contact requests');
         _converse.HEADER_UNGROUPED = __('Ungrouped');
+        _converse.HEADER_UNREAD = __('New messages');
 
         const HEADER_WEIGHTS = {};
-        HEADER_WEIGHTS[_converse.HEADER_REQUESTING_CONTACTS] = 0;
-        HEADER_WEIGHTS[_converse.HEADER_CURRENT_CONTACTS]    = 1;
-        HEADER_WEIGHTS[_converse.HEADER_UNGROUPED]           = 2;
-        HEADER_WEIGHTS[_converse.HEADER_PENDING_CONTACTS]    = 3;
+        HEADER_WEIGHTS[_converse.HEADER_UNREAD] = 0;
+        HEADER_WEIGHTS[_converse.HEADER_REQUESTING_CONTACTS] = 1;
+        HEADER_WEIGHTS[_converse.HEADER_CURRENT_CONTACTS]    = 2;
+        HEADER_WEIGHTS[_converse.HEADER_UNGROUPED]           = 3;
+        HEADER_WEIGHTS[_converse.HEADER_PENDING_CONTACTS]    = 4;
 
 
         _converse.registerPresenceHandler = function () {
@@ -839,17 +841,20 @@ converse.plugins.add('converse-roster', {
             comparator (a, b) {
                 a = a.get('name');
                 b = b.get('name');
+                const WEIGHTS =  HEADER_WEIGHTS;
                 const special_groups = Object.keys(HEADER_WEIGHTS);
                 const a_is_special = special_groups.includes(a);
                 const b_is_special = special_groups.includes(b);
                 if (!a_is_special && !b_is_special ) {
                     return a.toLowerCase() < b.toLowerCase() ? -1 : (a.toLowerCase() > b.toLowerCase() ? 1 : 0);
                 } else if (a_is_special && b_is_special) {
-                    return HEADER_WEIGHTS[a] < HEADER_WEIGHTS[b] ? -1 : (HEADER_WEIGHTS[a] > HEADER_WEIGHTS[b] ? 1 : 0);
+                    return WEIGHTS[a] < WEIGHTS[b] ? -1 : (WEIGHTS[a] > WEIGHTS[b] ? 1 : 0);
                 } else if (!a_is_special && b_is_special) {
-                    return (b === _converse.HEADER_REQUESTING_CONTACTS) ? 1 : -1;
+                    const a_header = _converse.HEADER_CURRENT_CONTACTS;
+                    return WEIGHTS[a_header] < WEIGHTS[b] ? -1 : (WEIGHTS[a_header] > WEIGHTS[b] ? 1 : 0);
                 } else if (a_is_special && !b_is_special) {
-                    return (a === _converse.HEADER_REQUESTING_CONTACTS) ? -1 : 1;
+                    const b_header = _converse.HEADER_CURRENT_CONTACTS;
+                    return WEIGHTS[a] < WEIGHTS[b_header] ? -1 : (WEIGHTS[a] > WEIGHTS[b_header] ? 1 : 0);
                 }
             },