Allow setting the name and a group when accepting a contact request.

Remove the concept of "pending" contacts.

Fixes #1941
Fixes #2844

CHANGES.md

@@ -18,6 +18,7 @@
 - #2586: Add support for XEP-0402 Bookmarks
 - #2623: Merge MUC join and bookmark, leave and unset autojoin 
 - #2716: Fix issue with chat display when opening via URL
+- #2844: Contact stays in "pending contacts"
 - #2980: Allow setting an avatar for MUCs
 - #3033: Add the `muc_grouped_by_domain` option to display MUCs on the same domain in collapsible groups
 - #3038: Message to self from other client is ignored
@@ -36,7 +37,7 @@
 - #3464: Missing localization: the online status is not localized
 - #3476: better UI for form "fixed" fields
 - #3478: MUC participant status indicator misplaced 
-- #3510: MUC's configuration panel loads endlessly, if it's the second one you want to configure 
+- #3510: MUC's configuration panel loads endlessly, if it's jthe second one you want to configure 
 - #3529: Unbookmarked channels no longer change their name when clicked with an unread indicator (or text icon)
 - #3579: Changing nickname in a groupchat once, forbids to change nickname in another groupchat afterwards
 - #3589: Hats namespace change

src/headless/plugins/roster/contact.js

@@ -7,7 +7,7 @@ import converse from '../../shared/api/public.js';
 import ColorAwareModel from '../../shared/color.js';
 import { rejectPresenceSubscription } from './utils.js';
 
-const { Strophe, $iq, $pres } = converse.env;
+const { Strophe, $iq, $pres, stx } = converse.env;
 
 class RosterContact extends ColorAwareModel(Model) {
     get idAttribute () {
@@ -133,11 +133,18 @@ class RosterContact extends ColorAwareModel(Model) {
      * @param {string} message - Optional message to send to the person being authorized
      */
     authorize (message) {
-        const pres = $pres({'to': this.get('jid'), 'type': "subscribed"});
-        if (message && message !== "") {
-            pres.c("status").t(message);
-        }
-        api.send(pres);
+        api.send(stx`
+            <presence
+                to="${this.get('jid')}"
+                type="subscribed"
+                xmlns="jabber:client">
+                    ${message && message !== "" ? stx`<status>${message}</status>` : '' }
+            </presence>`);
+
+        this.save({
+            requesting: false,
+            subscription: 'from',
+        });
         return this;
     }
 

src/headless/plugins/roster/plugin.js

@@ -41,7 +41,6 @@ converse.plugins.add('converse-roster', {
         const labels = {
             HEADER_UNSAVED_CONTACTS: __('Unsaved contacts'),
             HEADER_CURRENT_CONTACTS: __('My contacts'),
-            HEADER_PENDING_CONTACTS: __('Pending contacts'),
             HEADER_REQUESTING_CONTACTS: __('Contact requests'),
             HEADER_UNGROUPED: __('Ungrouped'),
             HEADER_UNREAD: __('New messages'),

src/plugins/rosterview/contactview.js

@@ -52,7 +52,7 @@ export default class RosterContact extends CustomElement {
      */
     addContact(ev) {
         ev?.preventDefault?.();
-        api.modal.show('converse-add-contact-modal', { 'model': new Model() }, ev);
+        api.modal.show('converse-add-contact-modal', { model: new Model() }, ev);
     }
 
     /**
@@ -78,13 +78,11 @@ export default class RosterContact extends CustomElement {
      */
     async acceptRequest(ev) {
         ev?.preventDefault?.();
-
-        await _converse.state.roster.sendContactAddIQ({
-            jid: this.model.get('jid'),
-            name: this.model.getFullname(),
-            groups: [],
-        });
-        this.model.authorize().subscribe();
+        api.modal.show(
+            'converse-accept-contact-request-modal',
+            { model: new Model(), contact: this.model },
+            ev
+        );
     }
 
     /**

src/plugins/rosterview/index.js

@@ -7,6 +7,7 @@ import RosterContactView from './contactview.js';
 import { highlightRosterItem } from './utils.js';
 import "../modal";
 import "./modals/add-contact.js";
+import "./modals/accept-contact-request.js";
 import "./modals/new-chat.js";
 import './rosterview.js';
 

src/plugins/rosterview/modals/accept-contact-request.js

@@ -0,0 +1,63 @@
+import { _converse, api, log } from "@converse/headless";
+import "shared/autocomplete/index.js";
+import BaseModal from "plugins/modal/modal.js";
+import tplAcceptContactRequest from "./templates/accept-contact-request.js";
+import { __ } from "i18n";
+
+export default class AcceptContactRequest extends BaseModal {
+
+    /**
+     * @param {Object} options
+     */
+    constructor(options) {
+        super(options);
+        this.contact = null;
+    }
+
+    initialize() {
+        super.initialize();
+        this.listenTo(this.model, "change", () => this.requestUpdate());
+        this.listenTo(this.contact, "change", () => this.requestUpdate());
+        this.requestUpdate();
+        this.addEventListener(
+            "shown.bs.modal",
+            () => /** @type {HTMLInputElement} */ (this.querySelector('input[name="name"]'))?.focus(),
+            false
+        );
+    }
+
+    renderModal() {
+        return tplAcceptContactRequest(this);
+    }
+
+    getModalTitle() {
+        return __("Contact Request");
+    }
+
+    /**
+     * @param {Event} ev
+     */
+    async acceptContactRequest(ev) {
+        ev.preventDefault();
+        const form = /** @type {HTMLFormElement} */ (ev.target);
+        const data = new FormData(form);
+        const name = /** @type {string} */ (data.get("name") || "").trim();
+        const group = data.get('group');
+        try {
+            await _converse.state.roster.sendContactAddIQ({
+                jid: this.contact.get("jid"),
+                name,
+                group,
+            });
+            this.contact.authorize().subscribe();
+        } catch (e) {
+            log.error(e);
+            this.model.set("error", __("Sorry, something went wrong"));
+            return;
+        }
+        this.contact.save({ groups: [group] });
+        this.modal.hide();
+    }
+}
+
+api.elements.define("converse-accept-contact-request-modal", AcceptContactRequest);

src/plugins/rosterview/modals/templates/accept-contact-request.js

@@ -0,0 +1,28 @@
+import { __ } from "i18n";
+import { getGroupsAutoCompleteList } from "../../utils.js";
+import { html } from "lit";
+
+/**
+ * @param {import('../accept-contact-request.js').default} el
+ */
+export default (el) => {
+    const i18n_add = __("Add");
+    const i18n_group = __("Group");
+    const i18n_nickname = __("Name");
+    const error = el.model.get("error");
+
+    return html` <div class="modal-body">
+        ${error ? html`<div class="alert alert-danger" role="alert">${error}</div>` : ""}
+        <form class="converse-form add-xmpp-contact" @submit=${(ev) => el.acceptContactRequest(ev)}>
+            <div class="add-xmpp-contact__name mb-3">
+                <label class="form-label clearfix" for="name">${i18n_nickname}:</label>
+                <input type="text" name="name" value="${el.contact.vcard?.get('fullname') || ''}" class="form-control" />
+            </div>
+            <div class="add-xmpp-contact__group mb-3">
+                <label class="form-label clearfix" for="name">${i18n_group}:</label>
+                <converse-autocomplete .list=${getGroupsAutoCompleteList()} name="group"></converse-autocomplete>
+            </div>
+            <button type="submit" class="btn btn-primary">${i18n_add}</button>
+        </form>
+    </div>`;
+};

src/plugins/rosterview/tests/protocol.js

@@ -79,16 +79,6 @@ describe("Presence subscriptions", function () {
              * element MUST possess a 'jid' attribute, MAY possess a 'name'
              * attribute, MUST NOT possess a 'subscription' attribute, and MAY
              * contain one or more <group/> child elements:
-             *
-             *   <iq type='set' id='set1'>
-             *   <query xmlns='jabber:iq:roster'>
-             *       <item
-             *           jid='contact@example.org'
-             *           name='MyContact'>
-             *       <group>MyBuddies</group>
-             *       </item>
-             *   </query>
-             *   </iq>
              */
             await mock.waitForRoster(_converse, 'all', 0);
             expect(_converse.roster.sendContactAddIQ).toHaveBeenCalled();
@@ -119,26 +109,15 @@ describe("Presence subscriptions", function () {
              * '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:
-             *
-             * <iq type='set'>
-             *     <query xmlns='jabber:iq:roster'>
-             *         <item
-             *             jid='contact@example.org'
-             *             subscription='none'
-             *             name='MyContact'>
-             *         <group>MyBuddies</group>
-             *         </item>
-             *     </query>
-             * </iq>
              */
             _converse.api.connection.get()._dataRecv(mock.createRequest(
-                $iq({'type': 'set'})
-                    .c('query', {'xmlns': 'jabber:iq:roster'})
-                        .c('item', {
-                            'jid': 'contact@example.org',
-                            'subscription': 'none',
-                            'name': 'Chris Contact'
-                        }).c('group').t('My Buddies')
+                stx`<iq type="set" xmlns="jabber:client">
+                    <query xmlns="jabber:iq:roster">
+                        <item jid="contact@example.org" subscription="none" name="Chris Contact">
+                            <group>My Buddies</group>
+                        </item>
+                    </query>
+                </iq>`
             ));
 
             _converse.api.connection.get()._dataRecv(mock.createRequest(
@@ -175,40 +154,27 @@ describe("Presence subscriptions", function () {
              * 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:
-             *
-             *  <iq type='set'>
-             *    <query xmlns='jabber:iq:roster'>
-             *      <item
-             *          jid='contact@example.org'
-             *          subscription='none'
-             *          ask='subscribe'
-             *          name='MyContact'>
-             *      <group>MyBuddies</group>
-             *      </item>
-             *    </query>
-             *  </iq>
              */
             _converse.api.connection.get()._dataRecv(mock.createRequest(
-                $iq({'type': 'set', 'from': _converse.bare_jid})
-                    .c('query', {'xmlns': 'jabber:iq:roster'})
-                        .c('item', {
-                            'jid': 'contact@example.org',
-                            'subscription': 'none',
-                            'ask': 'subscribe',
-                            'name': 'Chris Contact'
-                        }).c('group').t('My Buddies')
+                stx`<iq type="set" from="${_converse.bare_jid}" xmlns="jabber:client">
+                    <query xmlns="jabber:iq:roster">
+                        <item jid="contact@example.org" subscription="none" ask="subscribe" name="Chris Contact">
+                            <group>My Buddies</group>
+                        </item>
+                    </query>
+                </iq>`
             ));
 
             const rosterview = document.querySelector('converse-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 header = sizzle('a:contains("My Buddies")', rosterview).pop();
                 const contacts = Array.from(header?.parentElement.querySelectorAll('li') ?? []).filter(u.isVisible);
                 return contacts.length;
             }, 600);
 
-            let header = sizzle('a:contains("Pending contacts")', rosterview).pop();
+            let header = sizzle('a:contains("My Buddies")', rosterview).pop();
             let contacts = header.parentElement.querySelectorAll('li');
             expect(contacts.length).toBe(1);
             expect(u.isVisible(contacts[0])).toBe(true);
@@ -218,18 +184,9 @@ describe("Presence subscriptions", function () {
 
             /* Here we assume the "happy path" that the contact
              * approves the subscription request
-             *
-             *  <presence
-             *      to='user@example.com'
-             *      from='contact@example.org'
-             *      type='subscribed'/>
              */
             _converse.api.connection.get()._dataRecv(mock.createRequest(
-                stanza = $pres({
-                    'to': _converse.bare_jid,
-                    'from': 'contact@example.org',
-                    'type': 'subscribed'
-                })
+                stanza = stx`<presence to="${_converse.bare_jid}" from="contact@example.org" type="subscribed" xmlns="jabber:client"/>`
             ));
 
             /* Upon receiving the presence stanza of type "subscribed",
@@ -238,33 +195,21 @@ describe("Presence subscriptions", function () {
              * stanza of type "subscribe".
              */
             expect(contact.ackSubscribe).toHaveBeenCalled();
-            expect(Strophe.serialize(sent_stanza)).toBe( // Strophe adds the xmlns attr (although not in spec)
-                `<presence to="contact@example.org" type="subscribe" xmlns="jabber:client"/>`
+            expect(sent_stanza).toEqualStanza(
+                stx`<presence to="contact@example.org" type="subscribe" xmlns="jabber:client"/>`
             );
 
             /* The user's server MUST initiate a roster push to all of the user's
              * available resources that have requested the roster,
              * containing an updated roster item for the contact with
              * the 'subscription' attribute set to a value of "to";
-             *
-             *  <iq type='set'>
-             *    <query xmlns='jabber:iq:roster'>
-             *      <item
-             *          jid='contact@example.org'
-             *          subscription='to'
-             *          name='MyContact'>
-             *        <group>MyBuddies</group>
-             *      </item>
-             *    </query>
-             *  </iq>
              */
             const IQ_id = _converse.api.connection.get().getUniqueId('roster');
-            stanza = $iq({'type': 'set', 'id': IQ_id})
-                .c('query', {'xmlns': 'jabber:iq:roster'})
-                .c('item', {
-                    'jid': 'contact@example.org',
-                    'subscription': 'to',
-                    'name': 'Nicky'});
+            stanza = stx`<iq type="set" id="${IQ_id}" xmlns="jabber:client">
+                <query xmlns="jabber:iq:roster">
+                    <item jid="contact@example.org" subscription="to" name="Nicky"/>
+                </query>
+            </iq>`;
 
             _converse.api.connection.get()._dataRecv(mock.createRequest(stanza));
             // Check that the IQ set was acknowledged.
@@ -324,38 +269,26 @@ describe("Presence subscriptions", function () {
              *  <presence to='contact@example.org' type='subscribed'/>
              */
             expect(contact.authorize).toHaveBeenCalled();
-            expect(Strophe.serialize(sent_stanza)).toBe(
-                `<presence to="contact@example.org" type="subscribed" xmlns="jabber:client"/>`
+            expect(sent_stanza).toEqualStanza(
+                stx`<presence to="contact@example.org" type="subscribed" xmlns="jabber:client"/>`
             );
 
             /* As a result, the user's server MUST initiate a
              * roster push containing a roster item for the
              * contact with the 'subscription' attribute set to
              * a value of "both".
-             *
-             *  <iq type='set'>
-             *    <query xmlns='jabber:iq:roster'>
-             *      <item
-             *          jid='contact@example.org'
-             *          subscription='both'
-             *          name='MyContact'>
-             *      <group>MyBuddies</group>
-             *      </item>
-             *    </query>
-             *  </iq>
              */
             _converse.api.connection.get()._dataRecv(mock.createRequest(
-                $iq({'type': 'set'}).c('query', {'xmlns': 'jabber:iq:roster'})
-                    .c('item', {
-                        'jid': 'contact@example.org',
-                        'subscription': 'both',
-                        'name': 'contact@example.org'})
+                stx`<iq type="set" xmlns="jabber:client">
+                    <query xmlns="jabber:iq:roster">
+                        <item jid="contact@example.org" subscription="both" name="contact@example.org"/>
+                    </query>
+                </iq>`
             ));
 
             // The class on the contact will now have switched.
             await u.waitUntil(() => !u.hasClass('to', contacts[0]));
             expect(u.hasClass('both', contacts[0])).toBe(true);
-
         }));
 
         it("Alternate Flow: Contact Declines Subscription Request",

src/plugins/rosterview/tests/roster.js

@@ -427,7 +427,7 @@ describe("The Contacts Roster", function () {
 
             const roster = rosterview.querySelector('.roster-contacts');
             await u.waitUntil(() => sizzle('li', roster).filter(u.isVisible).length === 21, 900);
-            expect(sizzle('ul.roster-group-contacts', roster).filter(u.isVisible).length).toBe(6);
+            expect(sizzle('ul.roster-group-contacts', roster).filter(u.isVisible).length).toBe(5);
 
             filter.value = "online";
             u.triggerEvent(filter, 'change');
@@ -465,7 +465,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",
@@ -474,14 +474,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([
@@ -492,7 +491,6 @@ describe("The Contacts Roster", function () {
                 "friends & acquaintences",
                 "ænemies",
                 "Ungrouped",
-                "Pending contacts",
             ]);
             const contacts = sizzle('.roster-group[data-group="New messages"] li', rosterview);
             expect(contacts.length).toBe(1);
@@ -500,7 +498,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",
@@ -509,7 +507,6 @@ describe("The Contacts Roster", function () {
                 "friends & acquaintences",
                 "ænemies",
                 "Ungrouped",
-                "Pending contacts",
             ]);
         }));
 
@@ -522,7 +519,7 @@ describe("The Contacts Roster", function () {
             await mock.waitForRoster(_converse, 'all');
             await mock.createContacts(_converse, 'requesting');
             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",
@@ -531,7 +528,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  => {
@@ -650,17 +646,6 @@ describe("The Contacts Roster", function () {
 
     describe("Pending Contacts", function () {
 
-        it("can be collapsed under their own header (if roster_groups is false)",
-                mock.initConverse([], {'roster_groups': false}, 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 rosterview = document.querySelector('converse-roster');
-            await u.waitUntil(() => sizzle('.roster-group', rosterview).filter(u.isVisible).map(e => e.querySelector('li')).length, 1000);
-            await checkHeaderToggling.apply(_converse, [rosterview.querySelector('[data-group="Pending contacts"]')]);
-        }));
-
         it("can be added to the roster",
             mock.initConverse(
                 [], {},
@@ -691,7 +676,9 @@ describe("The Contacts Roster", function () {
             await u.waitUntil(() => sizzle('li', rosterview).filter(u.isVisible).length, 500)
             expect(u.isVisible(rosterview)).toBe(true);
             expect(sizzle('li', rosterview).filter(u.isVisible).length).toBe(4);
-            expect(sizzle('ul.roster-group-contacts', rosterview).filter(u.isVisible).length).toBe(2);
+            expect(sizzle('ul.roster-group-contacts', rosterview).filter(u.isVisible).length).toBe(1);
+            const el = sizzle('ul.roster-group-contacts', rosterview).filter(u.isVisible).pop();
+            expect(el.getAttribute('data-group')).toBe('Ungrouped');
         }));
 
         it("can be removed by the user", mock.initConverse([], {'roster_groups': false}, async function (_converse) {
@@ -715,51 +702,12 @@ describe("The Contacts Roster", function () {
             await u.waitUntil(() => !sizzle(`.pending-xmpp-contact .contact-name:contains("${name}")`, rosterview).length, 500);
             expect(_converse.api.confirm).toHaveBeenCalled();
             expect(contact.sendRosterRemoveStanza).toHaveBeenCalled();
-            expect(Strophe.serialize(sent_IQ)).toBe(
-                `<iq type="set" xmlns="jabber:client">`+
-                    `<query xmlns="jabber:iq:roster">`+
-                        `<item jid="lord.capulet@montague.lit" subscription="remove"/>`+
-                    `</query>`+
-                `</iq>`);
-        }));
-
-        it("do not have a header if there aren't any",
-            mock.initConverse(
-                ['VCardsInitialized'], {'roster_groups': false},
-                async function (_converse) {
-
-            await mock.openControlBox(_converse);
-            await mock.waitForRoster(_converse, 'current', 0);
-            const name = mock.pend_names[0];
-            _converse.roster.create({
-                jid: name.replace(/ /g,'.').toLowerCase() + '@montague.lit',
-                subscription: 'none',
-                ask: 'subscribe',
-                fullname: name
-            });
-            const rosterview = document.querySelector('converse-roster');
-            await u.waitUntil(() => {
-                const el = rosterview.querySelector(`ul[data-group="Pending contacts"]`);
-                return u.isVisible(el) && Array.from(el.querySelectorAll('li')).filter(li => u.isVisible(li)).length;
-            }, 700)
-
-            const remove_el = await u.waitUntil(() => sizzle(`.remove-xmpp-contact[title="Click to remove ${name} as a contact"]`, rosterview).pop());
-            spyOn(_converse.api, 'confirm').and.callFake(() => Promise.resolve(true));
-            remove_el.click();
-            expect(_converse.api.confirm).toHaveBeenCalled();
-
-            const iq_stanzas = _converse.api.connection.get().IQ_stanzas;
-            await u.waitUntil(() => Strophe.serialize(iq_stanzas.at(-1)) ===
-                `<iq id="${iq_stanzas.at(-1).getAttribute('id')}" type="set" xmlns="jabber:client">`+
-                    `<query xmlns="jabber:iq:roster">`+
-                        `<item jid="lord.capulet@montague.lit" subscription="remove"/>`+
-                    `</query>`+
-                `</iq>`);
-
-            const iq = iq_stanzas.at(-1);
-            const stanza = stx`<iq id="${iq.getAttribute('id')}" to="romeo@montague.lit/orchard" type="result" xmlns="jabber:client"/>`;
-            _converse.api.connection.get()._dataRecv(mock.createRequest(stanza));
-            await u.waitUntil(() => rosterview.querySelector(`ul[data-group="Pending contacts"]`) === null);
+            expect(sent_IQ).toEqualStanza(stx`
+                <iq type="set" xmlns="jabber:client">
+                    <query xmlns="jabber:iq:roster">
+                        <item jid="lord.capulet@montague.lit" subscription="remove"/>
+                    </query>
+                </iq>`);
         }));
 
         it("can be removed by the user",
@@ -793,36 +741,6 @@ describe("The Contacts Roster", function () {
             }
             await u.waitUntil(() => rosterview.querySelector(`ul[data-group="Pending contacts"]`) === null);
         }));
-
-        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);
-            await mock.waitForRoster(_converse, 'current');
-            await Promise.all(_converse.roster.map(contact => u.waitUntil(() => contact.vcard.get('fullname'))));
-            let i;
-            for (i=0; i<mock.pend_names.length; i++) {
-                _converse.roster.create({
-                    jid: mock.pend_names[i].replace(/ /g,'.').toLowerCase() + '@montague.lit',
-                    subscription: 'none',
-                    ask: 'subscribe',
-                    fullname: mock.pend_names[i]
-                });
-            }
-            const rosterview = document.querySelector('converse-roster');
-            await u.waitUntil(() => sizzle('li', rosterview.querySelector(`ul[data-group="Pending contacts"]`)).filter(u.isVisible).length);
-            // Check that they are sorted alphabetically
-            const el = await u.waitUntil(() => rosterview.querySelector(`ul[data-group="Pending contacts"]`));
-            const spans = el.querySelectorAll('.pending-xmpp-contact .contact-name');
-
-            await u.waitUntil(
-                () => Array.from(spans).reduce((result, value) => result + value.textContent?.trim(), '') ===
-                mock.pend_names.slice(0,i+1).sort().join('')
-            );
-            expect(true).toBe(true);
-        }));
     });
 
     describe("Existing Contacts", function () {
@@ -1279,20 +1197,46 @@ describe("The Contacts Roster", function () {
             await mock.createContacts(_converse, 'requesting');
             const name = mock.req_names.sort()[0];
             const jid =  name.replace(/ /g,'.').toLowerCase() + '@montague.lit';
-            const contact = _converse.roster.get(jid);
-            spyOn(contact, 'authorize').and.callFake(() => contact);
+            const { api, roster } = _converse;
+            const contact = roster.get(jid);
+            spyOn(contact, 'authorize').and.callThrough();
             const rosterview = document.querySelector('converse-roster');
             await u.waitUntil(() => rosterview.querySelectorAll('.roster-group li').length)
-            // TODO: Testing can be more thorough here, the user is
-            // actually not accepted/authorized because of
-            // mock_connection.
-            spyOn(_converse.roster, 'sendContactAddIQ').and.callFake(() => Promise.resolve());
 
             const req_contact = sizzle(`.contact-name:contains("${contact.getDisplayName()}")`, rosterview).pop();
             req_contact.parentElement.parentElement.querySelector('.accept-xmpp-request').click();
-            expect(_converse.roster.sendContactAddIQ).toHaveBeenCalled();
+
+            const modal = _converse.api.modal.get('converse-accept-contact-request-modal');
+            await u.waitUntil(() => u.isVisible(modal), 1000);
+
+            expect(modal.querySelector('input[name="name"]')?.value).toBe('Escalus, prince of Verona');
+            const group_input = modal.querySelector('input[name="group"]');
+            group_input.value = 'Princes';
+
+            const sent_stanzas = _converse.api.connection.get().sent_stanzas;
+            while (sent_stanzas.length) sent_stanzas.pop();
+
+            modal.querySelector('button[type="submit"]').click();
+
+            let stanza = await u.waitUntil(() => sent_stanzas.filter(s => s.matches('iq[type="set"]')).pop());
+            expect(stanza).toEqualStanza(
+                stx`<iq type="set" xmlns="jabber:client" id="${stanza.getAttribute('id')}">
+                        <query xmlns="jabber:iq:roster">
+                            <item jid="${contact.get('jid')}" name="Escalus, prince of Verona"/>
+                        </query>
+                    </iq>`);
+
+            const result = stx`
+                <iq to="${api.connection.get().jid}" type="result" id="${stanza.getAttribute('id')}" xmlns="jabber:client"/>`;
+            api.connection.get()._dataRecv(mock.createRequest(result));
+
+            stanza = await u.waitUntil(() => sent_stanzas.filter(s => s.matches('presence[type="subscribed"]')).pop());
+            expect(stanza).toEqualStanza(
+                stx`<presence to="${contact.get('jid')}" type="subscribed" xmlns="jabber:client"/>`);
+
             await u.waitUntil(() => contact.authorize.calls.count());
             expect(contact.authorize).toHaveBeenCalled();
+            expect(contact.get('groups')).toEqual(['Princes']);
         }));
 
         it("can have their requests denied by the user",

src/plugins/rosterview/utils.js

@@ -212,8 +212,6 @@ export function populateContactsMap(contacts_map, contact) {
 
     if (contact.get('requesting')) {
         contact_groups.push(/** @type {string} */ (labels.HEADER_REQUESTING_CONTACTS));
-    } else if (contact.get('ask') === 'subscribe') {
-        contact_groups.push(/** @type {string} */ (labels.HEADER_PENDING_CONTACTS));
     } else if (contact.get('subscription') === 'none') {
         contact_groups.push(/** @type {string} */ (labels.HEADER_UNSAVED_CONTACTS));
     } else if (!api.settings.get('roster_groups')) {
@@ -264,14 +262,12 @@ export function groupsComparator(a, b) {
         HEADER_REQUESTING_CONTACTS,
         HEADER_CURRENT_CONTACTS,
         HEADER_UNGROUPED,
-        HEADER_PENDING_CONTACTS,
     } = _converse.labels;
 
     HEADER_WEIGHTS[HEADER_UNREAD] = 0;
     HEADER_WEIGHTS[HEADER_REQUESTING_CONTACTS] = 1;
     HEADER_WEIGHTS[HEADER_CURRENT_CONTACTS] = 2;
     HEADER_WEIGHTS[HEADER_UNGROUPED] = 3;
-    HEADER_WEIGHTS[HEADER_PENDING_CONTACTS] = 4;
 
     const WEIGHTS = HEADER_WEIGHTS;
     const special_groups = Object.keys(HEADER_WEIGHTS);

src/shared/tests/mock.js

@@ -440,11 +440,11 @@ async function createContact (_converse, name, ask, requesting, subscription) {
     }
     const contact = await new Promise((success, error) => {
         _converse.roster.create({
-            'ask': ask,
             'fullname': name,
-            'jid': jid,
-            'requesting': requesting,
-            'subscription': subscription
+            ask,
+            jid,
+            requesting,
+            subscription,
         }, {success, error});
     });
     return contact;
@@ -465,7 +465,7 @@ async function createContacts (_converse, type, length) {
         ask = null;
     } else if (type === 'pending') {
         names = pend_names;
-        subscription = 'none';
+        subscription = 'from';
         requesting = false;
         ask = 'subscribe';
     } else if (type === 'current') {
@@ -501,7 +501,7 @@ async function waitForRoster (_converse, type='current', length=-1, include_nick
             result.c('item', {
                 jid: name.replace(/ /g,'.').toLowerCase() + '@montague.lit',
                 name: include_nick ? name : undefined,
-                subscription: 'none',
+                subscription: 'from',
                 ask: 'subscribe'
             }).up()
         );