chatboxes: wait until messages are fixed before returning new chatbox

Fixes #1691
This commit is contained in:
JC Brand 2019-10-04 18:26:33 +02:00
parent 8e4e918575
commit b63b080419
9 changed files with 68 additions and 54 deletions

View File

@ -1,11 +1,19 @@
# Changelog # Changelog
## 5.0.5 ## 6.0.0 (Unreleased)
- #1691 Fix `collection.chatbox is undefined` errors
- Prevent editing of sent file uploads. - 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) - 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. 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. - 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.

View File

@ -267,6 +267,7 @@
'nick': ' Othello' 'nick': ' Othello'
}); });
expect(_converse.bookmarks.length).toBe(1); expect(_converse.bookmarks.length).toBe(1);
await u.waitUntil(() => _converse.chatboxes.length >= 1);
expect(view.model.get('bookmarked')).toBeTruthy(); expect(view.model.get('bookmarked')).toBeTruthy();
let bookmark_icon = await u.waitUntil(() => view.el.querySelector('.toggle-bookmark')); let bookmark_icon = await u.waitUntil(() => view.el.querySelector('.toggle-bookmark'));
expect(u.hasClass('button-on', bookmark_icon)).toBeTruthy(); expect(u.hasClass('button-on', bookmark_icon)).toBeTruthy();

View File

@ -261,26 +261,27 @@
_converse.api.trigger('rosterContactsFetched'); _converse.api.trigger('rosterContactsFetched');
// Test on chat that doesn't exist. // 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 jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
const jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit'; const jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit';
// Test on chat that's not open // Test on chat that's not open
let box = _converse.api.chats.get(jid); chat = await _converse.api.chats.get(jid);
expect(typeof box === 'undefined').toBeTruthy(); expect(typeof chat === 'undefined').toBeTruthy();
expect(_converse.chatboxes.length).toBe(1); expect(_converse.chatboxes.length).toBe(1);
// Test for one JID // Test for one JID
box = await _converse.api.chats.open(jid); chat = await _converse.api.chats.open(jid);
expect(box instanceof Object).toBeTruthy(); expect(chat instanceof Object).toBeTruthy();
expect(box.get('box_id')).toBe(`box-${btoa(jid)}`); expect(chat.get('box_id')).toBe(`box-${btoa(jid)}`);
const view = _converse.chatboxviews.get(jid); const view = _converse.chatboxviews.get(jid);
await u.waitUntil(() => u.isVisible(view.el)); await u.waitUntil(() => u.isVisible(view.el));
// Test for multiple JIDs // Test for multiple JIDs
test_utils.openChatBoxFor(_converse, jid2); test_utils.openChatBoxFor(_converse, jid2);
await u.waitUntil(() => _converse.chatboxes.length == 2); 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(Array.isArray(list)).toBeTruthy();
expect(list[0].get('box_id')).toBe(`box-${btoa(jid)}`); expect(list[0].get('box_id')).toBe(`box-${btoa(jid)}`);
expect(list[1].get('box_id')).toBe(`box-${btoa(jid2)}`); 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 jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
const jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit'; const jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit';
// Test on chat that doesn't exist. // 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); chat = await _converse.api.chats.open(jid);
expect(box instanceof Object).toBeTruthy(); expect(chat instanceof Object).toBeTruthy();
expect(box.get('box_id')).toBe(`box-${btoa(jid)}`); expect(chat.get('box_id')).toBe(`box-${btoa(jid)}`);
expect( expect(
_.keys(box), Object.keys(chat),
['close', 'endOTR', 'focus', 'get', 'initiateOTR', 'is_chatroom', 'maximize', 'minimize', 'open', 'set'] ['close', 'endOTR', 'focus', 'get', 'initiateOTR', 'is_chatroom', 'maximize', 'minimize', 'open', 'set']
); );
const view = _converse.chatboxviews.get(jid); const view = _converse.chatboxviews.get(jid);

View File

@ -1045,7 +1045,7 @@
const message = 'This message is sent from this chatbox'; const message = 'This message is sent from this chatbox';
await test_utils.sendMessage(view, message); 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); expect(chatbox.messages.models.length, 1);
const msg_object = chatbox.messages.models[0]; const msg_object = chatbox.messages.models[0];

View File

@ -56,7 +56,7 @@
await u.waitUntil(() => _converse.rosterview.el.querySelectorAll('.roster-group .group-toggle').length, 300); await u.waitUntil(() => _converse.rosterview.el.querySelectorAll('.roster-group .group-toggle').length, 300);
let muc_jid = 'chillout@montague.lit'; let muc_jid = 'chillout@montague.lit';
await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo'); 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(); expect(room instanceof Object).toBeTruthy();
let chatroomview = _converse.chatboxviews.get(muc_jid); let chatroomview = _converse.chatboxviews.get(muc_jid);
@ -68,19 +68,19 @@
// Test with mixed case // Test with mixed case
muc_jid = 'Leisure@montague.lit'; muc_jid = 'Leisure@montague.lit';
await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo'); 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(); expect(room instanceof Object).toBeTruthy();
chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase()); chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase());
expect(u.isVisible(chatroomview.el)).toBeTruthy(); expect(u.isVisible(chatroomview.el)).toBeTruthy();
muc_jid = 'leisure@montague.lit'; 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(); expect(room instanceof Object).toBeTruthy();
chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase()); chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase());
expect(u.isVisible(chatroomview.el)).toBeTruthy(); expect(u.isVisible(chatroomview.el)).toBeTruthy();
muc_jid = 'leiSure@montague.lit'; 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(); expect(room instanceof Object).toBeTruthy();
chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase()); chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase());
expect(u.isVisible(chatroomview.el)).toBeTruthy(); expect(u.isVisible(chatroomview.el)).toBeTruthy();
@ -88,7 +88,7 @@
// Non-existing room // Non-existing room
muc_jid = 'chillout2@montague.lit'; 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(); expect(typeof room === 'undefined').toBeTruthy();
done(); done();
})); }));

View File

@ -99,19 +99,23 @@ converse.plugins.add('converse-bookmarks', {
comparator: (item) => item.get('name').toLowerCase(), comparator: (item) => item.get('name').toLowerCase(),
initialize () { 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.markRoomAsUnbookmarked, this);
this.on('remove', this.sendBookmarkStanza, this); this.on('remove', this.sendBookmarkStanza, this);
const storage = _converse.config.get('storage'), const storage = _converse.config.get('storage');
cache_key = `converse.room-bookmarks${_converse.bare_jid}`; const cache_key = `converse.room-bookmarks${_converse.bare_jid}`;
this.fetched_flag = cache_key+'fetched'; this.fetched_flag = cache_key+'fetched';
this.browserStorage = new BrowserStorage[storage](cache_key); this.browserStorage = new BrowserStorage[storage](cache_key);
}, },
openBookmarkedRoom (bookmark) { async openBookmarkedRoom (bookmark) {
if ( _converse.muc_respect_autojoin && bookmark.get('autojoin')) { 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(); groupchat.maybeShow();
} }
return bookmark; return bookmark;

View File

@ -326,9 +326,9 @@ converse.plugins.add('converse-chatboxes', {
initMessages () { initMessages () {
this.messages = new this.messagesCollection(); this.messages = new this.messagesCollection();
this.messages.chatbox = this;
const storage = _converse.config.get('storage'); const storage = _converse.config.get('storage');
this.messages.browserStorage = new BrowserStorage[storage](this.getMessagesCacheKey()); this.messages.browserStorage = new BrowserStorage[storage](this.getMessagesCacheKey());
this.messages.chatbox = this;
this.listenTo(this.messages, 'change:upload', message => { this.listenTo(this.messages, 'change:upload', message => {
if (message.get('upload') === _converse.SUCCESS) { if (message.get('upload') === _converse.SUCCESS) {
_converse.api.send(this.createMessageStanza(message)); _converse.api.send(this.createMessageStanza(message));
@ -1167,7 +1167,7 @@ converse.plugins.add('converse-chatboxes', {
if (utils.isSameBareJID(from_jid, _converse.bare_jid)) { if (utils.isSameBareJID(from_jid, _converse.bare_jid)) {
return; return;
} }
const chatbox = this.getChatBox(from_jid); const chatbox = await this.getChatBox(from_jid);
if (!chatbox) { if (!chatbox) {
return; return;
} }
@ -1203,7 +1203,7 @@ converse.plugins.add('converse-chatboxes', {
/** /**
* Handler method for all incoming single-user chat "message" stanzas. * Handler method for all incoming single-user chat "message" stanzas.
* @private * @private
* @method _converse.ChatBox#onMessage * @method _converse.ChatBoxes#onMessage
* @param { XMLElement } stanza - The incoming message stanza * @param { XMLElement } stanza - The incoming message stanza
*/ */
async onMessage (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 // 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 has_body = sizzle(`body, encrypted[xmlns="${Strophe.NS.OMEMO}"]`, stanza).length > 0;
const roster_nick = get(contact, 'attributes.nickname'); 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) { if (chatbox) {
const message = await chatbox.getDuplicateMessage(stanza); const message = await chatbox.getDuplicateMessage(stanza);
@ -1319,7 +1319,7 @@ converse.plugins.add('converse-chatboxes', {
* @param { object } attrs - Optional chat box atributes. If the * @param { object } attrs - Optional chat box atributes. If the
* chat box already exists, its attributes will be updated. * chat box already exists, its attributes will be updated.
*/ */
getChatBox (jid, attrs={}, create) { async getChatBox (jid, attrs={}, create) {
if (isObject(jid)) { if (isObject(jid)) {
create = attrs; create = attrs;
attrs = jid; attrs = jid;
@ -1337,6 +1337,7 @@ converse.plugins.add('converse-chatboxes', {
_converse.log(response.responseText); _converse.log(response.responseText);
} }
}); });
await chatbox.messages.fetched;
if (!chatbox.isValid()) { if (!chatbox.isValid()) {
chatbox.destroy(); chatbox.destroy();
return null; 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: * // To open a single chat, provide the JID of the contact you're chatting with in that chat:
* converse.plugins.add('myplugin', { * converse.plugins.add('myplugin', {
* initialize: function() { * initialize: function() {
* var _converse = this._converse; * const _converse = this._converse;
* // Note, buddy@example.org must be in your contacts roster! * // Note, buddy@example.org must be in your contacts roster!
* _converse.api.chats.open('buddy@example.com').then(chat => { * _converse.api.chats.open('buddy@example.com').then(chat => {
* // Now you can do something with the chat model * // 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: * // To open an array of chats, provide an array of JIDs:
* converse.plugins.add('myplugin', { * converse.plugins.add('myplugin', {
* initialize: function () { * initialize: function () {
* var _converse = this._converse; * const _converse = this._converse;
* // Note, these users must first be in your contacts roster! * // Note, these users must first be in your contacts roster!
* _converse.api.chats.open(['buddy1@example.com', 'buddy2@example.com']).then(chats => { * _converse.api.chats.open(['buddy1@example.com', 'buddy2@example.com']).then(chats => {
* // Now you can do something with the chat models * // Now you can do something with the chat models
@ -1518,7 +1519,7 @@ converse.plugins.add('converse-chatboxes', {
* *
* @method _converse.api.chats.get * @method _converse.api.chats.get
* @param {String|string[]} jids - e.g. 'buddy@example.com' or ['buddy1@example.com', 'buddy2@example.com'] * @param {String|string[]} jids - e.g. 'buddy@example.com' or ['buddy1@example.com', 'buddy2@example.com']
* @returns {_converse.ChatBox} * @returns { Promise<_converse.ChatBox> }
* *
* @example * @example
* // To return a single chat, provide the JID of the contact you're chatting with in that chat: * // 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) { get (jids) {
if (jids === undefined) { if (jids === undefined) {
const result = []; // FIXME: Leaky abstraction from MUC. We need to add a
_converse.chatboxes.each(function (chatbox) { // base type for chat boxes, and check for that.
// FIXME: Leaky abstraction from MUC. We need to add a return _converse.chatboxes.filter(c => (c.get('type') !== _converse.CHATROOMS_TYPE));
// base type for chat boxes, and check for that.
if (chatbox.get('type') !== _converse.CHATROOMS_TYPE) {
result.push(chatbox);
}
});
return result;
} else if (isString(jids)) { } else if (isString(jids)) {
return _converse.chatboxes.getChatBox(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)));
} }
} }
}); });

View File

@ -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 /* Opens a groupchat, making sure that certain attributes
* are correct, for example that the "type" is set to * are correct, for example that the "type" is set to
* "chatroom". * "chatroom".
*/ */
settings.type = _converse.CHATROOMS_TYPE; settings.type = _converse.CHATROOMS_TYPE;
settings.id = jid; settings.id = jid;
const chatbox = _converse.chatboxes.getChatBox(jid, settings, true); const chatbox = await _converse.chatboxes.getChatBox(jid, settings, true);
chatbox.maybeShow(true); chatbox.maybeShow(true);
return chatbox; return chatbox;
} }
@ -2040,7 +2040,7 @@ converse.plugins.add('converse-muc', {
* @method _converse.ChatRoom#onDirectMUCInvitation * @method _converse.ChatRoom#onDirectMUCInvitation
* @param { XMLElement } message - The message stanza containing the invitation. * @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(), const x_el = sizzle('x[xmlns="jabber:x:conference"]', message).pop(),
from = Strophe.getBareJidFromJid(message.getAttribute('from')), from = Strophe.getBareJidFromJid(message.getAttribute('from')),
room_jid = x_el.getAttribute('jid'), room_jid = x_el.getAttribute('jid'),
@ -2066,7 +2066,7 @@ converse.plugins.add('converse-muc', {
} }
} }
if (result === true) { 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) { if (chatroom.get('connection_status') === converse.ROOMSTATUS.DISCONNECTED) {
_converse.chatboxes.get(room_jid).join(); _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 * @param {(string[]|string)} jid|jids The JID or array of
* JIDs of the chatroom(s) to create * JIDs of the chatroom(s) to create
* @param {object} [attrs] attrs The room attributes * @param {object} [attrs] attrs The room attributes
* @returns {Promise} Promise which resolves with the Backbone.Model representing the chat.
*/ */
create (jids, attrs={}) { create (jids, attrs={}) {
attrs = _.isString(attrs) ? {'nick': attrs} : (attrs || {}); attrs = _.isString(attrs) ? {'nick': attrs} : (attrs || {});
@ -2239,6 +2240,7 @@ converse.plugins.add('converse-muc', {
* another chat already in the foreground. * another chat already in the foreground.
* Set `force` to `true` if you want to force the room to be * Set `force` to `true` if you want to force the room to be
* maximized or shown. * maximized or shown.
* @returns {Promise} Promise which resolves with the Backbone.Model representing the chat.
* *
* @example * @example
* this._converse.api.rooms.open('group@muc.example.com') * 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); _converse.log(err_msg, Strophe.LogLevel.ERROR);
throw(new TypeError(err_msg)); throw(new TypeError(err_msg));
} else if (_.isString(jids)) { } else if (_.isString(jids)) {
const room = _converse.api.rooms.create(jids, attrs); const room = await _converse.api.rooms.create(jids, attrs);
if (room) { room && room.maybeShow(force);
room.maybeShow(force);
}
return room; return room;
} else { } 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. * the user's JID will be used.
* @param {boolean} create A boolean indicating whether the room should be created * @param {boolean} create A boolean indicating whether the room should be created
* if not found (default: `false`) * if not found (default: `false`)
* @returns {Promise} Promise which resolves with the Backbone.Model representing the chat.
* @example * @example
* _converse.api.waitUntil('roomsAutoJoined').then(() => { * _converse.api.waitUntil('roomsAutoJoined').then(() => {
* const create_if_not_found = true; * const create_if_not_found = true;

View File

@ -281,8 +281,8 @@ converse.plugins.add('converse-vcard', {
* fetched again even if it's been fetched before. * fetched again even if it's been fetched before.
* @returns {promise} A promise which resolves once the update has completed. * @returns {promise} A promise which resolves once the update has completed.
* @example * @example
* _converse.api.waitUntil('rosterContactsFetched').then(() => { * _converse.api.waitUntil('rosterContactsFetched').then(async () => {
* const chatbox = _converse.chatboxes.getChatBox('someone@example.org'); * const chatbox = await _converse.chatboxes.getChatBox('someone@example.org');
* _converse.api.vcard.update(chatbox); * _converse.api.vcard.update(chatbox);
* }); * });
*/ */