From b63b0804197cf24b2893d127eb190052149c92ea Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 4 Oct 2019 18:26:33 +0200 Subject: [PATCH] chatboxes: wait until messages are fixed before returning new chatbox Fixes #1691 --- CHANGES.md | 12 ++++++++++-- spec/bookmarks.js | 1 + spec/converse.js | 26 +++++++++++++------------ spec/messages.js | 2 +- spec/muc.js | 10 +++++----- src/headless/converse-bookmarks.js | 14 +++++++++----- src/headless/converse-chatboxes.js | 31 +++++++++++++----------------- src/headless/converse-muc.js | 22 ++++++++++++--------- src/headless/converse-vcard.js | 4 ++-- 9 files changed, 68 insertions(+), 54 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bc7ea9616..ce84a03bf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,11 +1,19 @@ # Changelog -## 5.0.5 +## 6.0.0 (Unreleased) +- #1691 Fix `collection.chatbox is undefined` errors - Prevent editing of sent file uploads. -## 5.0.4 (2019-10-08) +### Breaking changes +- The following API methods now return promises: + * `_converse.api.chats.get` + * `_converse.api.chats.create` + * `_converse.api.rooms.get` + * `_converse.api.rooms.create` + +## 5.0.4 (2019-10-08) - New config option [allow_message_corrections](https://conversejs.org/docs/html/configuration.html#allow_message_corrections) which, if set to `last`, limits editing of sent messages to the last message sent. - Bugfix: Don't treat every duplicate message ID as a message correction; since some clients don't use globally unique ID's this causes false positives. diff --git a/spec/bookmarks.js b/spec/bookmarks.js index 632c45398..588483aff 100644 --- a/spec/bookmarks.js +++ b/spec/bookmarks.js @@ -267,6 +267,7 @@ 'nick': ' Othello' }); expect(_converse.bookmarks.length).toBe(1); + await u.waitUntil(() => _converse.chatboxes.length >= 1); expect(view.model.get('bookmarked')).toBeTruthy(); let bookmark_icon = await u.waitUntil(() => view.el.querySelector('.toggle-bookmark')); expect(u.hasClass('button-on', bookmark_icon)).toBeTruthy(); diff --git a/spec/converse.js b/spec/converse.js index 292561f6f..f9f7bf227 100644 --- a/spec/converse.js +++ b/spec/converse.js @@ -261,26 +261,27 @@ _converse.api.trigger('rosterContactsFetched'); // Test on chat that doesn't exist. - expect(_converse.api.chats.get('non-existing@jabber.org')).toBeFalsy(); + let chat = await _converse.api.chats.get('non-existing@jabber.org'); + expect(chat).toBeFalsy(); const jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit'; const jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit'; // Test on chat that's not open - let box = _converse.api.chats.get(jid); - expect(typeof box === 'undefined').toBeTruthy(); + chat = await _converse.api.chats.get(jid); + expect(typeof chat === 'undefined').toBeTruthy(); expect(_converse.chatboxes.length).toBe(1); // Test for one JID - box = await _converse.api.chats.open(jid); - expect(box instanceof Object).toBeTruthy(); - expect(box.get('box_id')).toBe(`box-${btoa(jid)}`); + chat = await _converse.api.chats.open(jid); + expect(chat instanceof Object).toBeTruthy(); + expect(chat.get('box_id')).toBe(`box-${btoa(jid)}`); const view = _converse.chatboxviews.get(jid); await u.waitUntil(() => u.isVisible(view.el)); // Test for multiple JIDs test_utils.openChatBoxFor(_converse, jid2); await u.waitUntil(() => _converse.chatboxes.length == 2); - const list = _converse.api.chats.get([jid, jid2]); + const list = await _converse.api.chats.get([jid, jid2]); expect(Array.isArray(list)).toBeTruthy(); expect(list[0].get('box_id')).toBe(`box-${btoa(jid)}`); expect(list[1].get('box_id')).toBe(`box-${btoa(jid2)}`); @@ -298,13 +299,14 @@ const jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit'; const jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit'; // Test on chat that doesn't exist. - expect(_converse.api.chats.get('non-existing@jabber.org')).toBeFalsy(); + let chat = await _converse.api.chats.get('non-existing@jabber.org'); + expect(chat).toBeFalsy(); - const box = await _converse.api.chats.open(jid); - expect(box instanceof Object).toBeTruthy(); - expect(box.get('box_id')).toBe(`box-${btoa(jid)}`); + chat = await _converse.api.chats.open(jid); + expect(chat instanceof Object).toBeTruthy(); + expect(chat.get('box_id')).toBe(`box-${btoa(jid)}`); expect( - _.keys(box), + Object.keys(chat), ['close', 'endOTR', 'focus', 'get', 'initiateOTR', 'is_chatroom', 'maximize', 'minimize', 'open', 'set'] ); const view = _converse.chatboxviews.get(jid); diff --git a/spec/messages.js b/spec/messages.js index 917a2ce43..7d2b1a780 100644 --- a/spec/messages.js +++ b/spec/messages.js @@ -1045,7 +1045,7 @@ const message = 'This message is sent from this chatbox'; await test_utils.sendMessage(view, message); - const chatbox = _converse.api.chats.get(contact_jid); + const chatbox = await _converse.api.chats.get(contact_jid); expect(chatbox.messages.models.length, 1); const msg_object = chatbox.messages.models[0]; diff --git a/spec/muc.js b/spec/muc.js index dda85aa53..b21c8a5e6 100644 --- a/spec/muc.js +++ b/spec/muc.js @@ -56,7 +56,7 @@ await u.waitUntil(() => _converse.rosterview.el.querySelectorAll('.roster-group .group-toggle').length, 300); let muc_jid = 'chillout@montague.lit'; await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo'); - let room = _converse.api.rooms.get(muc_jid); + let room = await _converse.api.rooms.get(muc_jid); expect(room instanceof Object).toBeTruthy(); let chatroomview = _converse.chatboxviews.get(muc_jid); @@ -68,19 +68,19 @@ // Test with mixed case muc_jid = 'Leisure@montague.lit'; await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo'); - room = _converse.api.rooms.get(muc_jid); + room = await _converse.api.rooms.get(muc_jid); expect(room instanceof Object).toBeTruthy(); chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase()); expect(u.isVisible(chatroomview.el)).toBeTruthy(); muc_jid = 'leisure@montague.lit'; - room = _converse.api.rooms.get(muc_jid); + room = await _converse.api.rooms.get(muc_jid); expect(room instanceof Object).toBeTruthy(); chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase()); expect(u.isVisible(chatroomview.el)).toBeTruthy(); muc_jid = 'leiSure@montague.lit'; - room = _converse.api.rooms.get(muc_jid); + room = await _converse.api.rooms.get(muc_jid); expect(room instanceof Object).toBeTruthy(); chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase()); expect(u.isVisible(chatroomview.el)).toBeTruthy(); @@ -88,7 +88,7 @@ // Non-existing room muc_jid = 'chillout2@montague.lit'; - room = _converse.api.rooms.get(muc_jid); + room = await _converse.api.rooms.get(muc_jid); expect(typeof room === 'undefined').toBeTruthy(); done(); })); diff --git a/src/headless/converse-bookmarks.js b/src/headless/converse-bookmarks.js index bf3542cb8..1a685606f 100644 --- a/src/headless/converse-bookmarks.js +++ b/src/headless/converse-bookmarks.js @@ -99,19 +99,23 @@ converse.plugins.add('converse-bookmarks', { comparator: (item) => item.get('name').toLowerCase(), initialize () { - this.on('add', _.flow(this.openBookmarkedRoom, this.markRoomAsBookmarked)); + this.on('add', bm => this.openBookmarkedRoom(bm) + .then(bm => this.markRoomAsBookmarked(bm)) + .catch(e => _converse.log(e, Strophe.LogLevel.FATAL)) + ); + this.on('remove', this.markRoomAsUnbookmarked, this); this.on('remove', this.sendBookmarkStanza, this); - const storage = _converse.config.get('storage'), - cache_key = `converse.room-bookmarks${_converse.bare_jid}`; + const storage = _converse.config.get('storage'); + const cache_key = `converse.room-bookmarks${_converse.bare_jid}`; this.fetched_flag = cache_key+'fetched'; this.browserStorage = new BrowserStorage[storage](cache_key); }, - openBookmarkedRoom (bookmark) { + async openBookmarkedRoom (bookmark) { if ( _converse.muc_respect_autojoin && bookmark.get('autojoin')) { - const groupchat = _converse.api.rooms.create(bookmark.get('jid'), bookmark.get('nick')); + const groupchat = await _converse.api.rooms.create(bookmark.get('jid'), bookmark.get('nick')); groupchat.maybeShow(); } return bookmark; diff --git a/src/headless/converse-chatboxes.js b/src/headless/converse-chatboxes.js index d56d6fd27..c70847091 100644 --- a/src/headless/converse-chatboxes.js +++ b/src/headless/converse-chatboxes.js @@ -326,9 +326,9 @@ converse.plugins.add('converse-chatboxes', { initMessages () { this.messages = new this.messagesCollection(); + this.messages.chatbox = this; const storage = _converse.config.get('storage'); this.messages.browserStorage = new BrowserStorage[storage](this.getMessagesCacheKey()); - this.messages.chatbox = this; this.listenTo(this.messages, 'change:upload', message => { if (message.get('upload') === _converse.SUCCESS) { _converse.api.send(this.createMessageStanza(message)); @@ -1167,7 +1167,7 @@ converse.plugins.add('converse-chatboxes', { if (utils.isSameBareJID(from_jid, _converse.bare_jid)) { return; } - const chatbox = this.getChatBox(from_jid); + const chatbox = await this.getChatBox(from_jid); if (!chatbox) { return; } @@ -1203,7 +1203,7 @@ converse.plugins.add('converse-chatboxes', { /** * Handler method for all incoming single-user chat "message" stanzas. * @private - * @method _converse.ChatBox#onMessage + * @method _converse.ChatBoxes#onMessage * @param { XMLElement } stanza - The incoming message stanza */ async onMessage (stanza) { @@ -1279,7 +1279,7 @@ converse.plugins.add('converse-chatboxes', { // Get chat box, but only create when the message has something to show to the user const has_body = sizzle(`body, encrypted[xmlns="${Strophe.NS.OMEMO}"]`, stanza).length > 0; const roster_nick = get(contact, 'attributes.nickname'); - const chatbox = this.getChatBox(contact_jid, {'nickname': roster_nick}, has_body); + const chatbox = await this.getChatBox(contact_jid, {'nickname': roster_nick}, has_body); if (chatbox) { const message = await chatbox.getDuplicateMessage(stanza); @@ -1319,7 +1319,7 @@ converse.plugins.add('converse-chatboxes', { * @param { object } attrs - Optional chat box atributes. If the * chat box already exists, its attributes will be updated. */ - getChatBox (jid, attrs={}, create) { + async getChatBox (jid, attrs={}, create) { if (isObject(jid)) { create = attrs; attrs = jid; @@ -1337,6 +1337,7 @@ converse.plugins.add('converse-chatboxes', { _converse.log(response.responseText); } }); + await chatbox.messages.fetched; if (!chatbox.isValid()) { chatbox.destroy(); return null; @@ -1470,7 +1471,7 @@ converse.plugins.add('converse-chatboxes', { * // To open a single chat, provide the JID of the contact you're chatting with in that chat: * converse.plugins.add('myplugin', { * initialize: function() { - * var _converse = this._converse; + * const _converse = this._converse; * // Note, buddy@example.org must be in your contacts roster! * _converse.api.chats.open('buddy@example.com').then(chat => { * // Now you can do something with the chat model @@ -1482,7 +1483,7 @@ converse.plugins.add('converse-chatboxes', { * // To open an array of chats, provide an array of JIDs: * converse.plugins.add('myplugin', { * initialize: function () { - * var _converse = this._converse; + * const _converse = this._converse; * // Note, these users must first be in your contacts roster! * _converse.api.chats.open(['buddy1@example.com', 'buddy2@example.com']).then(chats => { * // Now you can do something with the chat models @@ -1518,7 +1519,7 @@ converse.plugins.add('converse-chatboxes', { * * @method _converse.api.chats.get * @param {String|string[]} jids - e.g. 'buddy@example.com' or ['buddy1@example.com', 'buddy2@example.com'] - * @returns {_converse.ChatBox} + * @returns { Promise<_converse.ChatBox> } * * @example * // To return a single chat, provide the JID of the contact you're chatting with in that chat: @@ -1535,19 +1536,13 @@ converse.plugins.add('converse-chatboxes', { */ get (jids) { if (jids === undefined) { - const result = []; - _converse.chatboxes.each(function (chatbox) { - // FIXME: Leaky abstraction from MUC. We need to add a - // base type for chat boxes, and check for that. - if (chatbox.get('type') !== _converse.CHATROOMS_TYPE) { - result.push(chatbox); - } - }); - return result; + // FIXME: Leaky abstraction from MUC. We need to add a + // base type for chat boxes, and check for that. + return _converse.chatboxes.filter(c => (c.get('type') !== _converse.CHATROOMS_TYPE)); } else if (isString(jids)) { return _converse.chatboxes.getChatBox(jids); } - return jids.map(jid => _converse.chatboxes.getChatBox(jid, {}, true)); + return Promise.all(jids.map(jid => _converse.chatboxes.getChatBox(jid, {}, true))); } } }); diff --git a/src/headless/converse-muc.js b/src/headless/converse-muc.js index c9d2bcf5a..39163f628 100644 --- a/src/headless/converse-muc.js +++ b/src/headless/converse-muc.js @@ -230,14 +230,14 @@ converse.plugins.add('converse-muc', { } } - function openChatRoom (jid, settings) { + async function openChatRoom (jid, settings) { /* Opens a groupchat, making sure that certain attributes * are correct, for example that the "type" is set to * "chatroom". */ settings.type = _converse.CHATROOMS_TYPE; settings.id = jid; - const chatbox = _converse.chatboxes.getChatBox(jid, settings, true); + const chatbox = await _converse.chatboxes.getChatBox(jid, settings, true); chatbox.maybeShow(true); return chatbox; } @@ -2040,7 +2040,7 @@ converse.plugins.add('converse-muc', { * @method _converse.ChatRoom#onDirectMUCInvitation * @param { XMLElement } message - The message stanza containing the invitation. */ - _converse.onDirectMUCInvitation = function (message) { + _converse.onDirectMUCInvitation = async function (message) { const x_el = sizzle('x[xmlns="jabber:x:conference"]', message).pop(), from = Strophe.getBareJidFromJid(message.getAttribute('from')), room_jid = x_el.getAttribute('jid'), @@ -2066,7 +2066,7 @@ converse.plugins.add('converse-muc', { } } if (result === true) { - const chatroom = openChatRoom(room_jid, {'password': x_el.getAttribute('password') }); + const chatroom = await openChatRoom(room_jid, {'password': x_el.getAttribute('password') }); if (chatroom.get('connection_status') === converse.ROOMSTATUS.DISCONNECTED) { _converse.chatboxes.get(room_jid).join(); } @@ -2196,6 +2196,7 @@ converse.plugins.add('converse-muc', { * @param {(string[]|string)} jid|jids The JID or array of * JIDs of the chatroom(s) to create * @param {object} [attrs] attrs The room attributes + * @returns {Promise} Promise which resolves with the Backbone.Model representing the chat. */ create (jids, attrs={}) { attrs = _.isString(attrs) ? {'nick': attrs} : (attrs || {}); @@ -2239,6 +2240,7 @@ converse.plugins.add('converse-muc', { * another chat already in the foreground. * Set `force` to `true` if you want to force the room to be * maximized or shown. + * @returns {Promise} Promise which resolves with the Backbone.Model representing the chat. * * @example * this._converse.api.rooms.open('group@muc.example.com') @@ -2275,13 +2277,14 @@ converse.plugins.add('converse-muc', { _converse.log(err_msg, Strophe.LogLevel.ERROR); throw(new TypeError(err_msg)); } else if (_.isString(jids)) { - const room = _converse.api.rooms.create(jids, attrs); - if (room) { - room.maybeShow(force); - } + const room = await _converse.api.rooms.create(jids, attrs); + room && room.maybeShow(force); return room; } else { - return jids.map(jid => _converse.api.rooms.create(jid, attrs).maybeShow(force)); + return jids.map(async jid => { + const room = await _converse.api.rooms.create(jid, attrs); + room.maybeShow(force); + }); } }, @@ -2298,6 +2301,7 @@ converse.plugins.add('converse-muc', { * the user's JID will be used. * @param {boolean} create A boolean indicating whether the room should be created * if not found (default: `false`) + * @returns {Promise} Promise which resolves with the Backbone.Model representing the chat. * @example * _converse.api.waitUntil('roomsAutoJoined').then(() => { * const create_if_not_found = true; diff --git a/src/headless/converse-vcard.js b/src/headless/converse-vcard.js index 62091d5e5..3ea956c82 100644 --- a/src/headless/converse-vcard.js +++ b/src/headless/converse-vcard.js @@ -281,8 +281,8 @@ converse.plugins.add('converse-vcard', { * fetched again even if it's been fetched before. * @returns {promise} A promise which resolves once the update has completed. * @example - * _converse.api.waitUntil('rosterContactsFetched').then(() => { - * const chatbox = _converse.chatboxes.getChatBox('someone@example.org'); + * _converse.api.waitUntil('rosterContactsFetched').then(async () => { + * const chatbox = await _converse.chatboxes.getChatBox('someone@example.org'); * _converse.api.vcard.update(chatbox); * }); */