Show roster contacts with subscription set to none

Remove the `allow_chat_pending_contacts` config option.

CHANGES.md

@@ -3,6 +3,8 @@
 ## 10.0.0 (Unreleased)
 
 - Don't automatically convert OpenStreetMap URLs into `geo:` URIs in sent messages
+- Remove the `allow_chat_pending_contacts` config option.
+- Show roster contacts with `subscription` set to `none`
 
 ## 9.1.1 (2022-05-05)
 

docs/source/configuration.rst

@@ -150,13 +150,6 @@ This setting is only applicable if the ``converse-bookmarks`` plugin is loaded.
 
 See also: `allow_public_bookmarks`_
 
-allow_chat_pending_contacts
----------------------------
-
-* Default:  ``false``
-
-Allow the user to chat with pending contacts.
-
 allow_contact_removal
 ---------------------
 

src/headless/plugins/roster/contacts.js

@@ -58,7 +58,6 @@ const RosterContacts = Collection.extend({
     /**
      * Fetches the roster contacts, first by trying the browser cache,
      * and if that's empty, then by querying the XMPP server.
-     * @private
      * @returns {promise} Promise which resolves once the contacts have been fetched.
      */
     async fetchRosterContacts () {
@@ -111,7 +110,6 @@ const RosterContacts = Collection.extend({
     /**
      * Add a roster contact and then once we have confirmation from
      * the XMPP server we subscribe to that contact's presence updates.
-     * @private
      * @method _converse.RosterContacts#addAndSubscribe
      * @param { String } jid - The Jabber ID of the user being added and subscribed to.
      * @param { String } name - The name of that user
@@ -128,7 +126,6 @@ const RosterContacts = Collection.extend({
 
     /**
      * Send an IQ stanza to the XMPP server to add a new roster contact.
-     * @private
      * @method _converse.RosterContacts#sendContactAddIQ
      * @param { String } jid - The Jabber ID of the user being added
      * @param { String } name - The name of that user
@@ -149,7 +146,6 @@ const RosterContacts = Collection.extend({
      * Adds a RosterContact instance to _converse.roster and
      * registers the contact on the XMPP server.
      * Returns a promise which is resolved once the XMPP server has responded.
-     * @private
      * @method _converse.RosterContacts#addContactToRoster
      * @param { String } jid - The Jabber ID of the user being added and subscribed to.
      * @param { String } name - The name of that user
@@ -199,7 +195,6 @@ const RosterContacts = Collection.extend({
     /**
      * Handle roster updates from the XMPP server.
      * See: https://xmpp.org/rfcs/rfc6121.html#roster-syntax-actions-push
-     * @private
      * @method _converse.RosterContacts#onRosterPush
      * @param { XMLElement } IQ - The IQ stanza received from the XMPP server.
      */
@@ -250,7 +245,6 @@ const RosterContacts = Collection.extend({
 
     /**
      * Fetch the roster from the XMPP server
-     * @private
      * @emits _converse#roster
      * @returns {promise}
      */
@@ -269,11 +263,11 @@ const RosterContacts = Collection.extend({
             const query = sizzle(`query[xmlns="${Strophe.NS.ROSTER}"]`, iq).pop();
             if (query) {
                 const items = sizzle(`item`, query);
-                if (!this.data.get('version')) {
+                if (!this.data.get('version') && this.models.length) {
                     // We're getting the full roster, so remove all cached
                     // contacts that aren't included in it.
                     const jids = items.map(item => item.getAttribute('jid'));
-                    this.models.forEach(m => !m.get('requesting') && !jids.includes(m.get('jid')) && m.destroy());
+                    this.forEach(m => !m.get('requesting') && !jids.includes(m.get('jid')) && m.destroy());
                 }
                 items.forEach(item => this.updateContact(item));
                 this.data.save('version', query.getAttribute('ver'));
@@ -298,43 +292,30 @@ const RosterContacts = Collection.extend({
         api.trigger('roster', iq);
     },
 
-    /* Update or create RosterContact models based on the given `item` XML
+    /**
+     * Update or create RosterContact models based on the given `item` XML
      * node received in the resulting IQ stanza from the server.
-     * @private
      * @param { XMLElement } item
      */
     updateContact (item) {
         const jid = item.getAttribute('jid');
         const contact = this.get(jid);
         const subscription = item.getAttribute("subscription");
-        const ask = item.getAttribute("ask");
-        const groups = [...new Set(sizzle('group', item).map(e => e.textContent))];
-        if (!contact) {
-            if ((subscription === "none" && ask === null) || (subscription === "remove")) {
-                return; // We're lazy when adding contacts.
-            }
-            this.create({
-                'ask': ask,
-                'nickname': item.getAttribute("name"),
-                'groups': groups,
-                'jid': jid,
-                'subscription': subscription
-            }, {sort: false});
-        } else {
         if (subscription === "remove") {
-                return contact.destroy();
+            return contact?.destroy();
         }
+
+        const ask = item.getAttribute("ask");
+        const nickname = item.getAttribute('name');
+        const groups = [...new Set(sizzle('group', item).map(e => e.textContent))];
+
+        if (contact) {
             // We only find out about requesting contacts via the
             // presence handler, so if we receive a contact
             // here, we know they aren't requesting anymore.
-            // see docs/DEVELOPER.rst
+            contact.save({ subscription, ask, nickname, groups, 'requesting': null });
-            contact.save({
+        } else {
-                'subscription': subscription,
+            this.create({ nickname, ask, groups, jid, subscription }, {sort: false});
-                'ask': ask,
-                'nickname': item.getAttribute("name"),
-                'requesting': null,
-                'groups': groups
-            });
         }
     },
 
@@ -429,10 +410,10 @@ const RosterContacts = Collection.extend({
 
     presenceHandler (presence) {
         const presence_type = presence.getAttribute('type');
-        if (presence_type === 'error') { return true; }
+        if (presence_type === 'error') return true;
 
-        const jid = presence.getAttribute('from'),
+        const jid = presence.getAttribute('from');
-              bare_jid = Strophe.getBareJidFromJid(jid);
+        const bare_jid = Strophe.getBareJidFromJid(jid);
         if (this.isSelf(bare_jid)) {
             return this.handleOwnPresence(presence);
         } else if (sizzle(`query[xmlns="${Strophe.NS.MUC}"]`, presence).length) {

src/modals/templates/add-contact.js

@@ -36,8 +36,12 @@ export default (el) => {
 
                         <div class="form-group add-xmpp-contact__name">
                             <label class="clearfix" for="name">${i18n_nickname}:</label>
+                            <div class="suggestion-box suggestion-box__name">
+                                <ul class="suggestion-box__results suggestion-box__results--above" hidden=""></ul>
                                 <input type="text" name="name" value="${el.model.get('nickname') || ''}"
                                     class="form-control suggestion-box__input"/>
+                                <span class="suggestion-box__additions visually-hidden" role="status" aria-live="assertive" aria-relevant="additions"></span>
+                            </div>
                         </div>
 
                         <div class="form-group add-xmpp-contact__group">

src/plugins/controlbox/tests/controlbox.js

@@ -233,14 +233,12 @@ describe("The 'Add Contact' widget", function () {
         );
     }));
 
-
     it("integrates with xhr_user_search_url to search for contacts",
             mock.initConverse([], { 'xhr_user_search_url': 'http://example.org/?' },
             async function (_converse) {
 
         await mock.waitForRoster(_converse, 'all', 0);
 
-
         class MockXHR extends XMLHttpRequest {
             open () {} // eslint-disable-line
             responseText  = ''

src/plugins/muc-views/tests/emojis.js

@@ -4,7 +4,9 @@ const { $pres, sizzle } = converse.env;
 const u = converse.env.utils;
 
 describe("Emojis", function () {
+
     describe("The emoji picker", function () {
+
         it("is opened to autocomplete emojis in the textarea",
                 mock.initConverse(['chatBoxesFetched'], {}, async function (_converse) {
 

src/plugins/rosterview/contactview.js

@@ -1,12 +1,9 @@
 import log from "@converse/headless/log.js";
-import tpl_pending_contact from "./templates/pending_contact.js";
 import tpl_requesting_contact from "./templates/requesting_contact.js";
 import tpl_roster_item from "./templates/roster_item.js";
 import { CustomElement } from 'shared/components/element.js';
 import { __ } from 'i18n';
-import { _converse, api, converse } from "@converse/headless/core";
+import { _converse, api } from "@converse/headless/core";
-
-const u = converse.env.utils;
 
 
 export default class RosterContact extends CustomElement {
@@ -25,32 +22,7 @@ export default class RosterContact extends CustomElement {
     }
 
     render () {
-        const ask = this.model.get('ask');
+        if (this.model.get('requesting') === true) {
-        const requesting  = this.model.get('requesting');
-        const subscription = this.model.get('subscription');
-        const jid = this.model.get('jid');
-
-        if ((ask === 'subscribe') || (subscription === 'from')) {
-            /* ask === 'subscribe'
-             *      Means we have asked to subscribe to them.
-             *
-             * subscription === 'from'
-             *      They are subscribed to use, but not vice versa.
-             *      We assume that there is a pending subscription
-             *      from us to them (otherwise we're in a state not
-             *      supported by converse.js).
-             *
-             *  So in both cases the user is a "pending" contact.
-             */
-            const display_name = this.model.getDisplayName();
-            return tpl_pending_contact(Object.assign(
-                this.model.toJSON(), {
-                    display_name,
-                    'openChat': ev => this.openChat(ev),
-                    'removeContact':  ev => this.removeContact(ev)
-                }));
-
-        } else if (requesting === true) {
             const display_name = this.model.getDisplayName();
             return tpl_requesting_contact(
                 Object.assign(this.model.toJSON(), {
@@ -60,18 +32,13 @@ export default class RosterContact extends CustomElement {
                     'declineRequest': ev => this.declineRequest(ev),
                     'desc_accept': __("Click to accept the contact request from %1$s", display_name),
                     'desc_decline': __("Click to decline the contact request from %1$s", display_name),
-                    'allow_chat_pending_contacts': api.settings.get('allow_chat_pending_contacts')
                 })
             );
-        } else if (subscription === 'both' || subscription === 'to' || u.isSameBareJID(jid, _converse.connection.jid)) {
+        } else {
-            return this.renderRosterItem(this.model);
+            return tpl_roster_item(this, this.model);
         }
     }
 
-    renderRosterItem (item) {
-        return tpl_roster_item(this, item);
-    }
-
     openChat (ev) {
         ev?.preventDefault?.();
         this.model.openChat();

src/plugins/rosterview/index.js

@@ -24,7 +24,6 @@ converse.plugins.add('converse-rosterview', {
     initialize () {
         api.settings.extend({
             'autocomplete_add_contact': true,
-            'allow_chat_pending_contacts': true,
             'allow_contact_removal': true,
             'hide_offline_users': false,
             'roster_groups': true,

src/plugins/rosterview/templates/pending_contact.js

@@ -1,12 +0,0 @@
-import { __ } from 'i18n';
-import { api } from "@converse/headless/core";
-import { html } from "lit";
-
-const tpl_pending_contact = o => html`<span class="pending-contact-name" title="JID: ${o.jid}">${o.display_name}</span>`;
-
-export default  (o) => {
-    const i18n_remove = __('Click to remove %1$s as a contact', o.display_name);
-    return html`
-        ${ api.settings.get('allow_chat_pending_contacts') ? html`<a class="list-item-link open-chat w-100" href="#" @click=${o.openChat}>${tpl_pending_contact(o)}</a>` : tpl_pending_contact(o) }
-        <a class="list-item-action remove-xmpp-contact far fa-trash-alt" @click=${o.removeContact} title="${i18n_remove}" href="#"></a>`;
-}

src/plugins/rosterview/templates/requesting_contact.js

@@ -1,10 +1,9 @@
-import { api } from "@converse/headless/core";
 import { html } from "lit";
 
-const tpl_requesting_contact = o => html`<span class="req-contact-name w-100" title="JID: ${o.jid}">${o.display_name}</span>`;
-
 export default (o) => html`
-   ${ api.settings.get('allow_chat_pending_contacts') ? html`<a class="open-chat w-100" href="#" @click=${o.openChat}>${tpl_requesting_contact(o) }</a>` : tpl_requesting_contact(o) }
+   <a class="open-chat w-100" href="#" @click=${o.openChat}>
+      <span class="req-contact-name w-100" title="JID: ${o.jid}">${o.display_name}</span>
+   </a>
    <a class="accept-xmpp-request list-item-action list-item-action--visible fa fa-check"
       @click=${o.acceptRequest}
       aria-label="${o.desc_accept}" title="${o.desc_accept}" href="#"></a>

src/plugins/rosterview/templates/roster.js

@@ -4,34 +4,7 @@ import { _converse, api } from "@converse/headless/core";
 import { contactsComparator, groupsComparator } from '@converse/headless/plugins/roster/utils.js';
 import { html } from "lit";
 import { repeat } from 'lit/directives/repeat.js';
-import { shouldShowContact, shouldShowGroup } from '../utils.js';
+import { shouldShowContact, shouldShowGroup, populateContactsMap } from '../utils.js';
-
-
-function populateContactsMap (contacts_map, contact) {
-    if (contact.get('ask') === 'subscribe') {
-        const name = _converse.HEADER_PENDING_CONTACTS;
-        contacts_map[name] ? contacts_map[name].push(contact) : (contacts_map[name] = [contact]);
-    } else if (contact.get('requesting')) {
-        const name = _converse.HEADER_REQUESTING_CONTACTS;
-        contacts_map[name] ? contacts_map[name].push(contact) : (contacts_map[name] = [contact]);
-    } else {
-        let contact_groups;
-        if (api.settings.get('roster_groups')) {
-            contact_groups = contact.get('groups');
-            contact_groups = (contact_groups.length === 0) ? [_converse.HEADER_UNGROUPED] : contact_groups;
-        } else {
-            contact_groups = [_converse.HEADER_CURRENT_CONTACTS];
-        }
-        for (const name of contact_groups) {
-            contacts_map[name] ? contacts_map[name].push(contact) : (contacts_map[name] = [contact]);
-        }
-    }
-    if (contact.get('num_unread')) {
-        const name = _converse.HEADER_UNREAD;
-        contacts_map[name] ? contacts_map[name].push(contact) : (contacts_map[name] = [contact]);
-    }
-    return contacts_map;
-}
 
 
 export default (el) => {

src/plugins/rosterview/tests/protocol.js

@@ -7,9 +7,6 @@ const Strophe = converse.env.Strophe;
 describe("The Protocol", function () {
 
     describe("Integration of Roster Items and Presence Subscriptions", function () {
-        // Stub the trimChat method. It causes havoc when running with
-        // phantomJS.
-
         /* Some level of integration between roster items and presence
          * subscriptions is normally expected by an instant messaging user
          * regarding the user's subscriptions to and from other contacts. This
@@ -42,7 +39,7 @@ describe("The Protocol", function () {
                 mock.initConverse([], { roster_groups: false }, async function (_converse) {
 
             const { u, $iq, $pres, sizzle, Strophe } = converse.env;
-            let contact, stanza;
+            let stanza;
             await mock.waitForRoster(_converse, 'current', 0);
             await mock.waitUntilDiscoConfirmed(_converse, 'montague.lit', [], ['vcard-temp']);
             await u.waitUntil(() => _converse.xmppstatus.vcard.get('fullname'), 300);
@@ -60,12 +57,13 @@ describe("The Protocol", function () {
             cbview.querySelector('.add-contact').click()
             const modal = _converse.api.modal.get('add-contact-modal');
             await u.waitUntil(() => u.isVisible(modal.el), 1000);
-            spyOn(modal, "addContactFromForm").and.callThrough();
             modal.delegateEvents();
 
             // Fill in the form and submit
             const form = modal.el.querySelector('form.add-xmpp-contact');
-            form.querySelector('input').value = 'contact@example.org';
+            form.querySelector('input[name="jid"]').value = 'contact@example.org';
+            form.querySelector('input[name="name"]').value = 'Chris Contact';
+            form.querySelector('input[name="group"]').value = 'My Buddies';
             form.querySelector('[type="submit"]').click();
 
             /* In preparation for being able to render the contact in the
@@ -73,11 +71,10 @@ describe("The Protocol", function () {
              * subscription, the user's client SHOULD perform a "roster set"
              * for the new roster item.
              */
-            expect(modal.addContactFromForm).toHaveBeenCalled();
             expect(_converse.roster.addAndSubscribe).toHaveBeenCalled();
             expect(_converse.roster.addContactToRoster).toHaveBeenCalled();
 
-            /* _converse request consists of sending an IQ
+            /* The request consists of sending an IQ
              * stanza of type='set' containing a <query/> element qualified by
              * the 'jabber:iq:roster' namespace, which in turn contains an
              * <item/> element that defines the new roster item; the <item/>
@@ -99,19 +96,28 @@ describe("The Protocol", function () {
             expect(_converse.roster.sendContactAddIQ).toHaveBeenCalled();
 
             const IQ_stanzas = _converse.connection.IQ_stanzas;
-            const roster_fetch_stanza = IQ_stanzas.filter(s => sizzle('query[xmlns="jabber:iq:roster"]', s)).pop();
+            const roster_set_stanza = IQ_stanzas.filter(s => sizzle('query[xmlns="jabber:iq:roster"]', s)).pop();
 
-            expect(Strophe.serialize(roster_fetch_stanza)).toBe(
+            expect(Strophe.serialize(roster_set_stanza)).toBe(
-                `<iq id="${roster_fetch_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">`+
+                `<iq id="${roster_set_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">`+
                     `<query xmlns="jabber:iq:roster">`+
-                        `<item jid="contact@example.org"/>`+
+                        `<item jid="contact@example.org" name="Chris Contact">`+
+                            `<group>My Buddies</group>`+
+                        `</item>`+
                     `</query>`+
                 `</iq>`
             );
 
+            const sent_stanzas = [];
+            let sent_stanza;
+            spyOn(_converse.connection, 'send').and.callFake(function (stanza) {
+                sent_stanza = stanza;
+                sent_stanzas.push(stanza);
+            });
+
             /* As a result, the user's server (1) MUST initiate a roster push
              * for the new roster item to all available resources associated
-             * with _converse user that have requested the roster, setting the
+             * with the user that have requested the roster, setting the
              * 'subscription' attribute to a value of "none"; and (2) MUST
              * reply to the sending resource with an IQ result indicating the
              * success of the roster set:
@@ -127,34 +133,27 @@ describe("The Protocol", function () {
              *     </query>
              * </iq>
              */
-            const create = _converse.roster.create;
+            _converse.connection._dataRecv(mock.createRequest(
-            const sent_stanzas = [];
+                $iq({'type': 'set'})
-            let sent_stanza;
+                    .c('query', {'xmlns': 'jabber:iq:roster'})
-            spyOn(_converse.connection, 'send').and.callFake(function (stanza) {
-                sent_stanza = stanza;
-                sent_stanzas.push(stanza);
-            });
-            spyOn(_converse.roster, 'create').and.callFake(function () {
-                contact = create.apply(_converse.roster, arguments);
-                spyOn(contact, 'subscribe').and.callThrough();
-                return contact;
-            });
-
-            stanza = $iq({'type': 'set'}).c('query', {'xmlns': 'jabber:iq:roster'})
                         .c('item', {
                             'jid': 'contact@example.org',
                             'subscription': 'none',
-                    'name': 'contact@example.org'});
+                            'name': 'Chris Contact'
-            _converse.connection._dataRecv(mock.createRequest(stanza));
+                        }).c('group').t('My Buddies')
+            ));
 
-            stanza = $iq({'type': 'result', 'id': roster_fetch_stanza.getAttribute('id')});
+            _converse.connection._dataRecv(mock.createRequest(
-            _converse.connection._dataRecv(mock.createRequest(stanza));
+                $iq({'type': 'result', 'id': roster_set_stanza.getAttribute('id')})
+            ));
 
-            await u.waitUntil(() => _converse.roster.create.calls.count(), 1000);
+            await u.waitUntil(() => _converse.roster.length === 1);
 
             // A contact should now have been created
-            expect(_converse.roster.get('contact@example.org') instanceof _converse.RosterContact).toBeTruthy();
+            const contact = _converse.roster.at(0);
             expect(contact.get('jid')).toBe('contact@example.org');
+            expect(contact.get('nickname')).toBe('Chris Contact');
+            expect(contact.get('groups')).toEqual(['My Buddies']);
             await u.waitUntil(() => contact.initialized);
 
             /* To subscribe to the contact's presence information,
@@ -164,16 +163,16 @@ describe("The Protocol", function () {
              *  <presence to='contact@example.org' type='subscribe'/>
              */
             const sent_presence = await u.waitUntil(() => sent_stanzas.filter(s => s.matches('presence')).pop());
-            expect(contact.subscribe).toHaveBeenCalled();
             expect(Strophe.serialize(sent_presence)).toBe(
                 `<presence to="contact@example.org" type="subscribe" xmlns="jabber:client">`+
                     `<nick xmlns="http://jabber.org/protocol/nick">Romeo Montague</nick>`+
                 `</presence>`
             );
+
             /* As a result, the user's server MUST initiate a second roster
              * push to all of the user's available resources that have
              * requested the roster, setting the contact to the pending
-             * sub-state of the 'none' subscription state; _converse pending
+             * sub-state of the 'none' subscription state; The pending
              * sub-state is denoted by the inclusion of the ask='subscribe'
              * attribute in the roster item:
              *
@@ -189,21 +188,20 @@ describe("The Protocol", function () {
              *    </query>
              *  </iq>
              */
-
+            _converse.connection._dataRecv(mock.createRequest(
-            spyOn(_converse.roster, "updateContact").and.callThrough();
+                $iq({'type': 'set', 'from': _converse.bare_jid})
-            stanza = $iq({'type': 'set', 'from': _converse.bare_jid})
                     .c('query', {'xmlns': 'jabber:iq:roster'})
                         .c('item', {
                             'jid': 'contact@example.org',
                             'subscription': 'none',
                             'ask': 'subscribe',
-                    'name': 'contact@example.org'});
+                            'name': 'Chris Contact'
-            _converse.connection._dataRecv(mock.createRequest(stanza));
+                        }).c('group').t('My Buddies')
-            expect(_converse.roster.updateContact).toHaveBeenCalled();
+            ));
 
             const rosterview = document.querySelector('converse-roster');
-            // Check that the user is now properly shown as a pending
+
-            // contact in the roster.
+            // Check that the user is now properly shown as a pending contact in the roster.
             await u.waitUntil(() => {
                 const header = sizzle('a:contains("Pending contacts")', rosterview).pop();
                 const contacts = Array.from(header?.parentElement.querySelectorAll('li') ?? []).filter(u.isVisible);
@@ -214,8 +212,10 @@ describe("The Protocol", function () {
             let contacts = header.parentElement.querySelectorAll('li');
             expect(contacts.length).toBe(1);
             expect(u.isVisible(contacts[0])).toBe(true);
+            sent_stanza = ""; // Reset
 
             spyOn(contact, "ackSubscribe").and.callThrough();
+
             /* Here we assume the "happy path" that the contact
              * approves the subscription request
              *
@@ -224,13 +224,14 @@ describe("The Protocol", function () {
              *      from='contact@example.org'
              *      type='subscribed'/>
              */
+            _converse.connection._dataRecv(mock.createRequest(
                 stanza = $pres({
                     'to': _converse.bare_jid,
                     'from': 'contact@example.org',
                     'type': 'subscribed'
-            });
+                })
-            sent_stanza = ""; // Reset
+            ));
-            _converse.connection._dataRecv(mock.createRequest(stanza));
+
             /* Upon receiving the presence stanza of type "subscribed",
              * the user SHOULD acknowledge receipt of that
              * subscription state notification by sending a presence
@@ -270,7 +271,6 @@ describe("The Protocol", function () {
             expect(Strophe.serialize(sent_stanza)).toBe( // Strophe adds the xmlns attr (although not in spec)
                 `<iq from="romeo@montague.lit/orchard" id="${IQ_id}" type="result" xmlns="jabber:client"/>`
             );
-            expect(_converse.roster.updateContact).toHaveBeenCalled();
 
             // The contact should now be visible as an existing contact (but still offline).
             await u.waitUntil(() => {
@@ -344,13 +344,13 @@ describe("The Protocol", function () {
              *    </query>
              *  </iq>
              */
-            stanza = $iq({'type': 'set'}).c('query', {'xmlns': 'jabber:iq:roster'})
+            _converse.connection._dataRecv(mock.createRequest(
+                $iq({'type': 'set'}).c('query', {'xmlns': 'jabber:iq:roster'})
                     .c('item', {
                         'jid': 'contact@example.org',
                         'subscription': 'both',
-                    'name': 'contact@example.org'});
+                        'name': 'contact@example.org'})
-            _converse.connection._dataRecv(mock.createRequest(stanza));
+            ));
-            expect(_converse.roster.updateContact).toHaveBeenCalled();
 
             // The class on the contact will now have switched.
             await u.waitUntil(() => !u.hasClass('to', contacts[0]));

src/plugins/rosterview/tests/roster.js

@@ -27,7 +27,6 @@ const checkHeaderToggling = async function (group) {
 describe("The Contacts Roster", function () {
 
     it("verifies the origin of roster pushes", mock.initConverse(['chatBoxesFetched'], {}, async function (_converse) {
-
         // See: https://gultsch.de/gajim_roster_push_and_message_interception.html
         const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
         await mock.waitForRoster(_converse, 'current', 1);
@@ -132,6 +131,40 @@ describe("The Contacts Roster", function () {
         expect(_converse.roster.at(0).get('jid')).toBe('nurse@example.com');
     }));
 
+    it("also contains contacts with subscription of none", mock.initConverse(
+        [], {}, async function (_converse) {
+
+        const sent_IQs = _converse.connection.IQ_stanzas;
+        const stanza = await u.waitUntil(() => sent_IQs.filter(iq => iq.querySelector('iq query[xmlns="jabber:iq:roster"]')).pop());
+        _converse.connection._dataRecv(mock.createRequest($iq({
+            to: _converse.connection.jid,
+            type: 'result',
+            id: stanza.getAttribute('id')
+        }).c('query', {
+            xmlns: 'jabber:iq:roster',
+        }).c('item', {
+            jid: 'juliet@example.net',
+            name: 'Juliet',
+            subscription:'both'
+        }).c('group').t('Friends').up().up()
+        .c('item', {
+            jid: 'mercutio@example.net',
+            name: 'Mercutio',
+            subscription: 'from'
+        }).c('group').t('Friends').up().up()
+        .c('item', {
+            jid: 'lord.capulet@example.net',
+            name: 'Lord Capulet',
+            subscription:'none'
+        }).c('group').t('Acquaintences')));
+
+        while (sent_IQs.length) sent_IQs.pop();
+
+        await u.waitUntil(() => _converse.roster.length === 3);
+        expect(_converse.roster.pluck('jid')).toEqual(['juliet@example.net', 'mercutio@example.net', 'lord.capulet@example.net']);
+        expect(_converse.roster.get('lord.capulet@example.net').get('subscription')).toBe('none');
+    }));
+
     it("can be refreshed", mock.initConverse(
         [], {}, async function (_converse) {
 
@@ -402,7 +435,7 @@ describe("The Contacts Roster", function () {
             // Check that the groups appear alphabetically and that
             // requesting and pending contacts are last.
             const rosterview = document.querySelector('converse-roster');
-            await u.waitUntil(() => sizzle('.roster-group a.group-toggle', rosterview).length === 7);
+            await u.waitUntil(() => sizzle('.roster-group a.group-toggle', rosterview).length === 6);
             let group_titles = sizzle('.roster-group a.group-toggle', rosterview).map(o => o.textContent.trim());
             expect(group_titles).toEqual([
                 "Contact requests",
@@ -411,14 +444,13 @@ describe("The Contacts Roster", function () {
                 "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', rosterview).length === 8);
+            await u.waitUntil(() => sizzle('.roster-group a.group-toggle', rosterview).length === 7);
             group_titles = sizzle('.roster-group a.group-toggle', rosterview).map(o => o.textContent.trim());
 
             expect(group_titles).toEqual([
@@ -428,8 +460,7 @@ describe("The Contacts Roster", function () {
                 "Family",
                 "friends & acquaintences",
                 "ænemies",
-                "Ungrouped",
+                "Ungrouped"
-                "Pending contacts"
             ]);
             const contacts = sizzle('.roster-group[data-group="New messages"] li', rosterview);
             expect(contacts.length).toBe(1);
@@ -437,7 +468,7 @@ describe("The Contacts Roster", function () {
             expect(contacts[0].querySelector('.msgs-indicator').textContent).toBe("5");
 
             contact.save({'num_unread': 0});
-            await u.waitUntil(() => sizzle('.roster-group a.group-toggle', rosterview).length === 7);
+            await u.waitUntil(() => sizzle('.roster-group a.group-toggle', rosterview).length === 6);
             group_titles = sizzle('.roster-group a.group-toggle', rosterview).map(o => o.textContent.trim());
             expect(group_titles).toEqual([
                 "Contact requests",
@@ -445,8 +476,7 @@ describe("The Contacts Roster", function () {
                 "Family",
                 "friends & acquaintences",
                 "ænemies",
-                "Ungrouped",
+                "Ungrouped"
-                "Pending contacts"
             ]);
         }));
 
@@ -459,10 +489,8 @@ describe("The Contacts Roster", function () {
             await mock.openControlBox(_converse);
             await mock.waitForRoster(_converse, 'all');
             await mock.createContacts(_converse, 'requesting');
-            // Check that the groups appear alphabetically and that
-            // requesting and pending contacts are last.
             const rosterview = document.querySelector('converse-roster');
-            await u.waitUntil(() => sizzle('.roster-group a.group-toggle', rosterview).length === 7);
+            await u.waitUntil(() => sizzle('.roster-group a.group-toggle', rosterview).length === 6);
             const group_titles = sizzle('.roster-group a.group-toggle', rosterview).map(o => o.textContent.trim());
             expect(group_titles).toEqual([
                 "Contact requests",
@@ -471,7 +499,6 @@ describe("The Contacts Roster", function () {
                 "friends & acquaintences",
                 "ænemies",
                 "Ungrouped",
-                "Pending contacts"
             ]);
             // Check that usernames appear alphabetically per group
             Object.keys(mock.groups).forEach(name  => {
@@ -497,8 +524,6 @@ describe("The Contacts Roster", function () {
 
             const rosterview = document.querySelector('converse-roster');
             await u.waitUntil(() => sizzle('.roster-group a.group-toggle', rosterview).length === 1);
-            // Check that the groups appear alphabetically and that
-            // requesting and pending contacts are last.
             let group_titles = await u.waitUntil(() => {
                 const toggles = sizzle('.roster-group a.group-toggle', rosterview);
                 if (toggles.reduce((result, t) => result && u.isVisible(t), true)) {
@@ -588,8 +613,8 @@ describe("The Contacts Roster", function () {
 
     describe("Pending Contacts", function () {
 
-        it("can be collapsed under their own header",
+        it("can be collapsed under their own header (if roster_groups is false)",
-                mock.initConverse([], {}, async function (_converse) {
+                mock.initConverse([], {'roster_groups': false}, async function (_converse) {
 
             await mock.openControlBox(_converse);
             await mock.waitForRoster(_converse, 'all');
@@ -632,27 +657,25 @@ describe("The Contacts Roster", function () {
             expect(sizzle('ul.roster-group-contacts', rosterview).filter(u.isVisible).length).toBe(1);
         }));
 
-        it("can be removed by the user",
+        it("can be removed by the user", mock.initConverse([], {'roster_groups': false}, async function (_converse) {
-                mock.initConverse([], {}, async function (_converse) {
-
             await mock.openControlBox(_converse);
             await mock.waitForRoster(_converse, 'all');
             await Promise.all(_converse.roster.map(contact => u.waitUntil(() => contact.vcard.get('fullname'))));
             const name = mock.pend_names[0];
             const jid = name.replace(/ /g,'.').toLowerCase() + '@montague.lit';
             const contact = _converse.roster.get(jid);
-            var sent_IQ;
+            spyOn(_converse.api, 'confirm').and.returnValue(Promise.resolve(true));
-            spyOn(_converse.api, 'confirm').and.callFake(() => Promise.resolve(true));
             spyOn(contact, 'unauthorize').and.callFake(function () { return contact; });
             spyOn(contact, 'removeFromRoster').and.callThrough();
             const rosterview = document.querySelector('converse-roster');
-            await u.waitUntil(() => sizzle(".pending-contact-name:contains('"+name+"')", rosterview).length, 700);
+            await u.waitUntil(() => sizzle(`.pending-xmpp-contact .contact-name:contains("${name}")`, rosterview).length, 500);
+            let sent_IQ;
             spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback) {
                 sent_IQ = iq;
                 callback();
             });
             sizzle(`.remove-xmpp-contact[title="Click to remove ${name} as a contact"]`, rosterview).pop().click();
-            await u.waitUntil(() => (sizzle(".pending-contact-name:contains('"+name+"')", rosterview).length === 0), 1000);
+            await u.waitUntil(() => !sizzle(`.pending-xmpp-contact .contact-name:contains("${name}")`, rosterview).length, 500);
             expect(_converse.api.confirm).toHaveBeenCalled();
             expect(contact.removeFromRoster).toHaveBeenCalled();
             expect(Strophe.serialize(sent_IQ)).toBe(
@@ -665,7 +688,7 @@ describe("The Contacts Roster", function () {
 
         it("do not have a header if there aren't any",
             mock.initConverse(
-                ['VCardsInitialized'], {},
+                ['VCardsInitialized'], {'roster_groups': false},
                 async function (_converse) {
 
             await mock.openControlBox(_converse);
@@ -702,8 +725,8 @@ describe("The Contacts Roster", function () {
             await u.waitUntil(() => rosterview.querySelector(`ul[data-group="Pending contacts"]`) === null);
         }));
 
-        it("is shown when a new private message is received",
+        it("can be removed by the user",
-                mock.initConverse([], {}, async function (_converse) {
+                mock.initConverse([], {'roster_groups': false}, async function (_converse) {
 
             await mock.openControlBox(_converse);
             await mock.waitForRoster(_converse, 'all');
@@ -716,12 +739,11 @@ describe("The Contacts Roster", function () {
                 sizzle(`.remove-xmpp-contact[title="Click to remove ${name} as a contact"]`, rosterview).pop().click();
             }
             await u.waitUntil(() => rosterview.querySelector(`ul[data-group="Pending contacts"]`) === null);
-            expect(rosterview.querySelectorAll('ul').length).toBe(5);
         }));
 
         it("can be added to the roster and they will be sorted alphabetically",
             mock.initConverse(
-                [], {},
+                [], {'roster_groups': false},
                 async function (_converse) {
 
             await mock.openControlBox(_converse);

src/plugins/rosterview/utils.js

@@ -5,7 +5,6 @@ export function highlightRosterItem (chatbox) {
     _converse.roster?.get(chatbox.get('jid'))?.trigger('highlight');
 }
 
-
 export function toggleGroup (ev, name) {
     ev?.preventDefault?.();
     const collapsed = _converse.roster.state.get('collapsed_groups');
@@ -72,3 +71,30 @@ export function shouldShowGroup (group) {
     }
     return true;
 }
+
+export function populateContactsMap (contacts_map, contact) {
+    if (contact.get('requesting')) {
+        const name = _converse.HEADER_REQUESTING_CONTACTS;
+        contacts_map[name] ? contacts_map[name].push(contact) : (contacts_map[name] = [contact]);
+    } else {
+        let contact_groups;
+        if (api.settings.get('roster_groups')) {
+            contact_groups = contact.get('groups');
+            contact_groups = (contact_groups.length === 0) ? [_converse.HEADER_UNGROUPED] : contact_groups;
+        } else {
+            if (contact.get('ask') === 'subscribe') {
+                contact_groups = [_converse.HEADER_PENDING_CONTACTS];
+            } else {
+                contact_groups = [_converse.HEADER_CURRENT_CONTACTS];
+            }
+        }
+        for (const name of contact_groups) {
+            contacts_map[name] ? contacts_map[name].push(contact) : (contacts_map[name] = [contact]);
+        }
+    }
+    if (contact.get('num_unread')) {
+        const name = _converse.HEADER_UNREAD;
+        contacts_map[name] ? contacts_map[name].push(contact) : (contacts_map[name] = [contact]);
+    }
+    return contacts_map;
+}