Remove unsaved contact when blocking

src/headless/plugins/roster/api.js

@@ -56,6 +56,18 @@ export default {
             return /** @type {string[]} */(jids).map(_getter);
         },
 
+        /**
+         * Remove a contact from the roster
+         * @param {string} jid
+         * @param {boolean} [unsubscribe] - Whether we should unsubscribe
+         * from the contact's presence updates.
+         */
+        async remove (jid, unsubscribe) {
+            await api.waitUntil('rosterContactsFetched');
+            const contact = await api.contacts.get(jid);
+            contact.remove(unsubscribe);
+        },
+
         /**
          * Add a contact.
          * @param {import('./types').RosterContactAttributes} attributes

src/headless/plugins/roster/contact.js

@@ -128,13 +128,13 @@ class RosterContact extends ColorAwareModel(Model) {
      */
     ackUnsubscribe () {
         api.send($pres({'type': 'unsubscribe', 'to': this.get('jid')}));
-        this.removeFromRoster();
+        this.sendRosterRemoveStanza();
         this.destroy();
     }
 
     /**
      * Unauthorize this contact's presence subscription
-     * @param {string} message - Optional message to send to the person being unauthorized
+     * @param {string} [message] - Optional message to send to the person being unauthorized
      */
     unauthorize (message) {
         rejectPresenceSubscription(this.get('jid'), message);
@@ -154,11 +154,40 @@ class RosterContact extends ColorAwareModel(Model) {
         return this;
     }
 
+
+    /**
+     * Remove this contact from the roster
+     * @param {boolean} unauthorize - Whether to also unauthorize the
+     */
+    remove (unauthorize) {
+        const subscription = this.get('subscription');
+        if (subscription === 'none' && this.get('ask') !== 'subscribe') {
+            this.destroy();
+            return;
+        }
+
+        if (unauthorize) {
+            if (subscription === 'from') {
+                this.unauthorize();
+            } else if (subscription === 'both') {
+                this.unauthorize();
+            }
+        }
+
+        this.sendRosterRemoveStanza();
+        if (this.collection) {
+            // The model might have already been removed as
+            // result of a roster push.
+            this.destroy();
+        }
+    }
+
     /**
      * Instruct the XMPP server to remove this contact from our roster
+     * @async
      * @returns {Promise}
      */
-    removeFromRoster () {
+    sendRosterRemoveStanza () {
         const iq = $iq({type: 'set'})
             .c('query', {xmlns: Strophe.NS.ROSTER})
             .c('item', {jid: this.get('jid'), subscription: "remove"});

src/headless/types/plugins/roster/api.d.ts

@@ -32,6 +32,13 @@ declare namespace _default {
          * });
          */
         function get(jids: (string[] | string)): Promise<any>;
+        /**
+         * Remove a contact from the roster
+         * @param {string} jid
+         * @param {boolean} [unsubscribe] - Whether we should unsubscribe
+         * from the contact's presence updates.
+         */
+        function remove(jid: string, unsubscribe?: boolean): Promise<void>;
         /**
          * Add a contact.
          * @param {import('./types').RosterContactAttributes} attributes

src/headless/types/plugins/roster/contact.d.ts

@@ -116,19 +116,25 @@ declare class RosterContact extends RosterContact_base {
     ackUnsubscribe(): void;
     /**
      * Unauthorize this contact's presence subscription
-     * @param {string} message - Optional message to send to the person being unauthorized
+     * @param {string} [message] - Optional message to send to the person being unauthorized
      */
-    unauthorize(message: string): this;
+    unauthorize(message?: string): this;
     /**
      * Authorize presence subscription
      * @param {string} message - Optional message to send to the person being authorized
      */
     authorize(message: string): this;
+    /**
+     * Remove this contact from the roster
+     * @param {boolean} unauthorize - Whether to also unauthorize the
+     */
+    remove(unauthorize: boolean): void;
     /**
      * Instruct the XMPP server to remove this contact from our roster
+     * @async
      * @returns {Promise}
      */
-    removeFromRoster(): Promise<any>;
+    sendRosterRemoveStanza(): Promise<any>;
 }
 import { Model } from '@converse/skeletor';
 //# sourceMappingURL=contact.d.ts.map

src/plugins/chatview/heading.js

@@ -101,7 +101,9 @@ export default class ChatHeading extends CustomElement {
                             [__('Are you sure you want to block this user?')]
                         );
                         if (result) {
-                            api.blocklist.add(this.model.get('jid'));
+                            const jid = this.model.get('jid');
+                            api.blocklist.add(jid);
+                            api.contacts.remove(jid, true);
                             this.model.close();
                         }
                     },

src/plugins/rosterview/contactview.js

@@ -68,18 +68,10 @@ export default class RosterContact extends CustomElement {
 
         const chat = await api.chats.get(this.model.get('jid'));
         chat?.close();
-
         try {
-            if (this.model.get('subscription') === 'none' && this.model.get('ask') !== 'subscribe') {
-                this.model.destroy();
-            } else {
-                this.model.removeFromRoster();
-                if (this.model.collection) {
-                    // The model might have already been removed as
-                    // result of a roster push.
-                    this.model.destroy();
-                }
-            }
+            // TODO: ask user whether they want to unauthorize the contact's
+            // presence request as well.
+            this.model.remove();
         } catch (e) {
             log.error(e);
             api.alert('error', __('Error'),

src/plugins/rosterview/tests/roster.js

@@ -700,7 +700,7 @@ describe("The Contacts Roster", function () {
             const contact = _converse.roster.get(jid);
             spyOn(_converse.api, 'confirm').and.returnValue(Promise.resolve(true));
             spyOn(contact, 'unauthorize').and.callFake(function () { return contact; });
-            spyOn(contact, 'removeFromRoster').and.callThrough();
+            spyOn(contact, 'sendRosterRemoveStanza').and.callThrough();
             const rosterview = document.querySelector('converse-roster');
             await u.waitUntil(() => sizzle(`.pending-xmpp-contact .contact-name:contains("${name}")`, rosterview).length, 500);
             let sent_IQ;
@@ -711,7 +711,7 @@ describe("The Contacts Roster", function () {
             sizzle(`.remove-xmpp-contact[title="Click to remove ${name} as a contact"]`, rosterview).pop().click();
             await u.waitUntil(() => !sizzle(`.pending-xmpp-contact .contact-name:contains("${name}")`, rosterview).length, 500);
             expect(_converse.api.confirm).toHaveBeenCalled();
-            expect(contact.removeFromRoster).toHaveBeenCalled();
+            expect(contact.sendRosterRemoveStanza).toHaveBeenCalled();
             expect(Strophe.serialize(sent_IQ)).toBe(
                 `<iq type="set" xmlns="jabber:client">`+
                     `<query xmlns="jabber:iq:roster">`+
@@ -913,7 +913,7 @@ describe("The Contacts Roster", function () {
             const jid = name.replace(/ /g,'.').toLowerCase() + '@montague.lit';
             const contact = _converse.roster.get(jid);
             spyOn(_converse.api, 'confirm').and.returnValue(Promise.resolve(true));
-            spyOn(contact, 'removeFromRoster').and.callThrough();
+            spyOn(contact, 'sendRosterRemoveStanza').and.callThrough();
 
             let sent_IQ;
             spyOn(_converse.api.connection.get(), 'sendIQ').and.callFake((iq, callback) => {
@@ -928,7 +928,7 @@ describe("The Contacts Roster", function () {
                 `<iq type="set" xmlns="jabber:client">`+
                     `<query xmlns="jabber:iq:roster"><item jid="mercutio@montague.lit" subscription="remove"/></query>`+
                 `</iq>`);
-            expect(contact.removeFromRoster).toHaveBeenCalled();
+            expect(contact.sendRosterRemoveStanza).toHaveBeenCalled();
             await u.waitUntil(() => sizzle(".open-chat:contains('"+name+"')", rosterview).length === 0);
         }));
 
@@ -949,13 +949,13 @@ describe("The Contacts Roster", function () {
             const rosterview = document.querySelector('converse-roster');
             await u.waitUntil(() => sizzle('.roster-group', rosterview).filter(u.isVisible).map(e => e.querySelector('li')).length, 1000);
             spyOn(_converse.api, 'confirm').and.returnValue(Promise.resolve(true));
-            spyOn(contact, 'removeFromRoster').and.callThrough();
+            spyOn(contact, 'sendRosterRemoveStanza').and.callThrough();
             spyOn(_converse.api.connection.get(), 'sendIQ').and.callFake((_iq, callback) => callback?.());
             expect(u.isVisible(rosterview.querySelector('.roster-group'))).toBe(true);
             sizzle(`.remove-xmpp-contact[title="Click to remove ${name} as a contact"]`, rosterview).pop().click();
             expect(_converse.api.confirm).toHaveBeenCalled();
             await u.waitUntil(() => _converse.api.connection.get().sendIQ.calls.count());
-            expect(contact.removeFromRoster).toHaveBeenCalled();
+            expect(contact.sendRosterRemoveStanza).toHaveBeenCalled();
             await u.waitUntil(() => rosterview.querySelectorAll('.roster-group').length === 0);
         }));
 

src/plugins/rosterview/utils.js

@@ -9,17 +9,21 @@ import { _converse, api, converse, log, constants } from "@converse/headless";
 const { Strophe } = converse.env;
 const { STATUS_WEIGHTS } = constants;
 
-export function removeContact (contact) {
-    contact.removeFromRoster(
-        () => contact.destroy(),
-        (e) => {
-            e && log.error(e);
-            api.alert('error', __('Error'), [
-                __('Sorry, there was an error while trying to remove %1$s as a contact.',
-                contact.getDisplayName())
-            ]);
-        }
-    );
+/**
+ * @param {RosterContact} contact
+ */
+export async function removeContact (contact) {
+    try {
+        await contact.sendRosterRemoveStanza();
+    } catch (e) {
+        log.error(e);
+        api.alert('error', __('Error'), [
+            __('Sorry, there was an error while trying to remove %1$s as a contact.',
+            contact.getDisplayName())
+        ]);
+    } finally {
+        contact.destroy();
+    }
 }
 
 export function highlightRosterItem (chatbox) {

src/shared/modals/tests/user-details-modal.js

@@ -21,7 +21,7 @@ describe("The User Details Modal", function () {
         await u.waitUntil(() => u.isVisible(modal), 1000);
         spyOn(_converse.api, 'confirm').and.returnValue(Promise.resolve(true));
 
-        spyOn(view.model.contact, 'removeFromRoster').and.callFake(callback => callback());
+        spyOn(view.model.contact, 'sendRosterRemoveStanza').and.callFake(callback => callback());
         let remove_contact_button = modal.querySelector('button.remove-contact');
         expect(u.isVisible(remove_contact_button)).toBeTruthy();
         remove_contact_button.click();
@@ -48,7 +48,7 @@ describe("The User Details Modal", function () {
         await u.waitUntil(() => u.isVisible(modal), 2000);
         spyOn(_converse.api, 'confirm').and.returnValue(Promise.resolve(true));
 
-        spyOn(view.model.contact, 'removeFromRoster').and.callFake((callback, errback) => errback());
+        spyOn(view.model.contact, 'sendRosterRemoveStanza').and.callFake((callback, errback) => errback());
         let remove_contact_button = modal.querySelector('button.remove-contact');
         expect(u.isVisible(remove_contact_button)).toBeTruthy();
         remove_contact_button.click();

src/types/plugins/rosterview/utils.d.ts

@@ -1,4 +1,7 @@
-export function removeContact(contact: any): void;
+/**
+ * @param {RosterContact} contact
+ */
+export function removeContact(contact: RosterContact): Promise<void>;
 export function highlightRosterItem(chatbox: any): void;
 export function toggleGroup(ev: any, name: any): void;
 /**