From cf75d375872d83b32a3d27d603bb053ab1bfa390 Mon Sep 17 00:00:00 2001 From: JC Brand <jc@opkode.com> Date: Thu, 11 Oct 2018 14:04:47 +0200 Subject: [PATCH] join/leave notification fixes. * Don't show leave notification twice when someone leaves, joins and then leaves again * Add a test case for a member joining and then leaving --- dist/converse.js | 56 ++++++++++++++++++++++++++------------ spec/chatroom.js | 56 ++++++++++++++++++++++++++++++++++---- src/converse-muc-views.js | 57 +++++++++++++++++++++++++++------------ 3 files changed, 130 insertions(+), 39 deletions(-) diff --git a/dist/converse.js b/dist/converse.js index 49bb6a579..975333bb3 100644 --- a/dist/converse.js +++ b/dist/converse.js @@ -68682,7 +68682,7 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_ this.model.on('destroy', this.hide, this); this.model.on('show', this.show, this); this.model.occupants.on('add', this.onOccupantAdded, this); - this.model.occupants.on('remove', this.showLeaveNotification, this); + this.model.occupants.on('remove', this.onOccupantRemoved, this); this.model.occupants.on('change:show', this.showJoinOrLeaveNotification, this); this.model.occupants.on('change:role', this.informOfOccupantsRoleChange, this); this.model.occupants.on('change:affiliation', this.informOfOccupantsAffiliationChange, this); @@ -69706,6 +69706,12 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_ } }, + onOccupantRemoved(occupant) { + if (occupant.get('show') === 'online') { + this.showLeaveNotification(occupant); + } + }, + showJoinOrLeaveNotification(occupant) { if (_.includes(occupant.get('states'), '303')) { return; @@ -69772,26 +69778,42 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_ const nick = occupant.get('nick'), stat = occupant.get('status'), - last_el = this.content.lastElementChild; + last_el = this.content.lastElementChild, + data = _.get(last_el, 'dataset', {}); - if (last_el && _.includes(_.get(last_el, 'classList', []), 'chat-info') && moment(last_el.getAttribute('data-isodate')).isSame(new Date(), "day") && _.get(last_el, 'dataset', {}).join === `"${nick}"`) { + if (last_el && _.includes(_.get(last_el, 'classList', []), 'chat-info') && moment(last_el.getAttribute('data-isodate')).isSame(new Date(), "day") && (data.join === `"${nick}"` || data.leavejoin === `"${nick}"`)) { let message; - if (_.isNil(stat)) { - message = __('%1$s has entered and left the groupchat', nick); - } else { - message = __('%1$s has entered and left the groupchat. "%2$s"', nick, stat); - } + if (data.join === `"${nick}"`) { + if (_.isNil(stat)) { + message = __('%1$s has entered and left the groupchat', nick); + } else { + message = __('%1$s has entered and left the groupchat. "%2$s"', nick, stat); + } - last_el.outerHTML = tpl_info({ - 'data': `data-joinleave="${nick}"`, - 'isodate': moment().format(), - 'extra_classes': 'chat-event', - 'message': message - }); - const el = this.content.lastElementChild; - setTimeout(() => u.addClass('fade-out', el), 5000); - setTimeout(() => el.parentElement && el.parentElement.removeChild(el), 5250); + last_el.outerHTML = tpl_info({ + 'data': `data-joinleave="${nick}"`, + 'isodate': moment().format(), + 'extra_classes': 'chat-event', + 'message': message + }); + const el = this.content.lastElementChild; + setTimeout(() => u.addClass('fade-out', el), 5000); + setTimeout(() => el.parentElement && el.parentElement.removeChild(el), 5250); + } else if (data.leavejoin === `"${nick}"`) { + if (_.isNil(stat)) { + message = __('%1$s has left the groupchat', nick); + } else { + message = __('%1$s has left the groupchat. "%2$s"', nick, stat); + } + + last_el.outerHTML = tpl_info({ + 'data': `data-leave="${nick}"`, + 'isodate': moment().format(), + 'extra_classes': 'chat-event', + 'message': message + }); + } } else { let message; diff --git a/spec/chatroom.js b/spec/chatroom.js index e611d1bf0..709798669 100644 --- a/spec/chatroom.js +++ b/spec/chatroom.js @@ -421,14 +421,13 @@ test_utils.openChatRoom(_converse, "coven", 'chat.shakespeare.lit', 'some1') .then(() => { - var view = _converse.chatboxviews.get('coven@chat.shakespeare.lit'); - var $chat_content = $(view.el).find('.chat-content'); - + const view = _converse.chatboxviews.get('coven@chat.shakespeare.lit'); + const $chat_content = $(view.el).find('.chat-content'); /* We don't show join/leave messages for existing occupants. We * know about them because we receive their presences before we * receive our own. */ - var presence = $pres({ + let presence = $pres({ to: 'dummy@localhost/_converse.js-29092160', from: 'coven@chat.shakespeare.lit/oldguy' }).c('x', {xmlns: Strophe.NS.MUC_USER}) @@ -609,8 +608,55 @@ _converse.connection._dataRecv(test_utils.createRequest(presence)); expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(5); expect($chat_content.find('div.chat-info:last').html()).toBe("nomorenicks has entered the groupchat"); + + + // Test a member joining and leaving + presence = $pres({ + to: 'dummy@localhost/_converse.js-290918392', + from: 'coven@chat.shakespeare.lit/insider' + }).c('x', {xmlns: Strophe.NS.MUC_USER}) + .c('item', { + 'affiliation': 'member', + 'jid': 'insider@localhost/_converse.js-290929789', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(6); + + /* <presence + * from='coven@chat.shakespeare.lit/thirdwitch' + * to='crone1@shakespeare.lit/desktop' + * type='unavailable'> + * <status>Disconnected: Replaced by new connection</status> + * <x xmlns='http://jabber.org/protocol/muc#user'> + * <item affiliation='member' + * jid='hag66@shakespeare.lit/pda' + * role='none'/> + * </x> + * </presence> + */ + presence = $pres({ + to: 'dummy@localhost/_converse.js-29092160', + type: 'unavailable', + from: 'coven@chat.shakespeare.lit/insider' + }) + .c('status', 'Disconnected: Replaced by new connection').up() + .c('x', {xmlns: Strophe.NS.MUC_USER}) + .c('item', { + 'affiliation': 'member', + 'jid': 'insider@localhost/_converse.js-290929789', + 'role': 'none' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + expect($chat_content.find('div.chat-info').length).toBe(6); + expect($chat_content.find('div.chat-info:last').html()).toBe( + 'insider has entered and left the groupchat. '+ + '"Disconnected: Replaced by new connection"'); + + expect(view.model.occupants.length).toBe(5); + expect(view.model.occupants.findWhere({'jid': 'insider@localhost'}).get('show')).toBe('offline'); done(); - }); + }).catch(_.partial(_converse.log, _, Strophe.LogLevel.FATAL)) })); it("shows a new day indicator if a join/leave message is received on a new day", diff --git a/src/converse-muc-views.js b/src/converse-muc-views.js index 8ff3e7aca..da84b0375 100644 --- a/src/converse-muc-views.js +++ b/src/converse-muc-views.js @@ -541,7 +541,7 @@ this.model.on('show', this.show, this); this.model.occupants.on('add', this.onOccupantAdded, this); - this.model.occupants.on('remove', this.showLeaveNotification, this); + this.model.occupants.on('remove', this.onOccupantRemoved, this); this.model.occupants.on('change:show', this.showJoinOrLeaveNotification, this); this.model.occupants.on('change:role', this.informOfOccupantsRoleChange, this); this.model.occupants.on('change:affiliation', this.informOfOccupantsAffiliationChange, this); @@ -1482,6 +1482,12 @@ } }, + onOccupantRemoved (occupant) { + if (occupant.get('show') === 'online') { + this.showLeaveNotification(occupant); + } + }, + showJoinOrLeaveNotification (occupant) { if (_.includes(occupant.get('states'), '303')) { return; @@ -1546,29 +1552,46 @@ } const nick = occupant.get('nick'), stat = occupant.get('status'), - last_el = this.content.lastElementChild; + last_el = this.content.lastElementChild, + data = _.get(last_el, 'dataset', {}); if (last_el && _.includes(_.get(last_el, 'classList', []), 'chat-info') && moment(last_el.getAttribute('data-isodate')).isSame(new Date(), "day") && - _.get(last_el, 'dataset', {}).join === `"${nick}"`) { + (data.join === `"${nick}"` || data.leavejoin === `"${nick}"`)) { let message; - if (_.isNil(stat)) { - message = __('%1$s has entered and left the groupchat', nick); - } else { - message = __('%1$s has entered and left the groupchat. "%2$s"', nick, stat); + if (data.join === `"${nick}"`) { + if (_.isNil(stat)) { + message = __('%1$s has entered and left the groupchat', nick); + } else { + message = __('%1$s has entered and left the groupchat. "%2$s"', nick, stat); + } + last_el.outerHTML = + tpl_info({ + 'data': `data-joinleave="${nick}"`, + 'isodate': moment().format(), + 'extra_classes': 'chat-event', + 'message': message + }); + const el = this.content.lastElementChild; + setTimeout(() => u.addClass('fade-out', el), 5000); + setTimeout(() => el.parentElement && el.parentElement.removeChild(el), 5250); + + } else if (data.leavejoin === `"${nick}"`) { + if (_.isNil(stat)) { + message = __('%1$s has left the groupchat', nick); + } else { + message = __('%1$s has left the groupchat. "%2$s"', nick, stat); + } + last_el.outerHTML = + tpl_info({ + 'data': `data-leave="${nick}"`, + 'isodate': moment().format(), + 'extra_classes': 'chat-event', + 'message': message + }); } - last_el.outerHTML = - tpl_info({ - 'data': `data-joinleave="${nick}"`, - 'isodate': moment().format(), - 'extra_classes': 'chat-event', - 'message': message - }); - const el = this.content.lastElementChild; - setTimeout(() => u.addClass('fade-out', el), 5000); - setTimeout(() => el.parentElement && el.parentElement.removeChild(el), 5250); } else { let message; if (_.isNil(stat)) { -- 2.30.9