From 014354d0754de6d29ca3c97fe0f7420f167e842c Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 7 Sep 2018 11:53:50 +0200 Subject: [PATCH] Fixes #986 Affiliation changes aren't displayed in the chat --- CHANGES.md | 1 + dist/converse.js | 50 ++++++++++--- spec/chatroom.js | 146 +++++++++++++++++++++++++++++--------- src/converse-muc-views.js | 47 +++++++++--- 4 files changed, 194 insertions(+), 50 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8b22e9898..6dd0150cf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,7 @@ - #421 XEP-0308: Last Message Correction - #497 XEP-0384: OMEMO encrypted messaging - #968 Use nickname from VCard when joining a room +- #986 Affiliation changes aren't displayed in the chat - #1081 Allow for shift-enter to insert newlines - #1091 There's now only one CSS file for all view modes. - #1094 Show room members who aren't currently online diff --git a/dist/converse.js b/dist/converse.js index 7c77a5632..433e575e3 100644 --- a/dist/converse.js +++ b/dist/converse.js @@ -68264,6 +68264,8 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_ this.model.occupants.on('add', this.showJoinNotification, this); this.model.occupants.on('remove', this.showLeaveNotification, 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); this.createEmojiPicker(); this.createOccupantsView(); this.render().insertIntoDOM(); @@ -68382,10 +68384,34 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_ this.occupantsview = new _converse.ChatRoomOccupantsView({ 'model': this.model.occupants }); - this.occupantsview.model.on('change:role', this.informOfOccupantsRoleChange, this); return this; }, + informOfOccupantsAffiliationChange(occupant, changed) { + const previous_affiliation = occupant._previousAttributes.affiliation, + current_affiliation = occupant.get('affiliation'); + + if (previous_affiliation === 'admin') { + this.showChatEvent(__("%1$s is no longer an admin of this groupchat", occupant.get('nick'))); + } else if (previous_affiliation === 'owner') { + this.showChatEvent(__("%1$s is no longer an owner of this groupchat", occupant.get('nick'))); + } else if (previous_affiliation === 'outcast') { + this.showChatEvent(__("%1$s is no longer banned from this groupchat", occupant.get('nick'))); + } + + if (current_affiliation === 'none' && previous_affiliation === 'member') { + this.showChatEvent(__("%1$s is no longer a permanent member of this groupchat", occupant.get('nick'))); + } + + if (current_affiliation === 'member') { + this.showChatEvent(__("%1$s is now a permanent member of this groupchat", occupant.get('nick'))); + } else if (current_affiliation === 'outcast') { + this.showChatEvent(__("%1$s has been banned from this groupchat", occupant.get('nick'))); + } else if (current_affiliation === 'admin' || current_affiliation == 'owner') { + this.showChatEvent(__(`%1$s is now an ${current_affiliation} of this groupchat`, occupant.get('nick'))); + } + }, + informOfOccupantsRoleChange(occupant, changed) { const previous_role = occupant._previousAttributes.role; @@ -68605,12 +68631,20 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_ /* Check that a command to change a groupchat user's role or * affiliation has anough arguments. */ - // TODO check if first argument is valid if (args.length < 1 || args.length > 2) { this.showErrorMessage(__('Error: the "%1$s" command takes two arguments, the user\'s nickname and optionally a reason.', command)); return false; } + if (!this.model.occupants.findWhere({ + 'nick': args[0] + }) && !this.model.occupants.findWhere({ + 'jid': args[0] + })) { + this.showErrorMessage(__('Error: couldn\'t find a groupchat participant "%1$s"', args[0])); + return false; + } + return true; }, @@ -68692,13 +68726,9 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_ const occupant = this.model.occupants.findWhere({ 'nick': args[0] + }) || this.model.occupants.findWhere({ + 'jid': args[0] }); - - if (!occupant) { - this.showErrorMessage(__(`Error: Can't find a groupchat participant with the nickname "${args[0]}"`)); - break; - } - this.model.setAffiliation('member', [{ 'jid': occupant.get('jid'), 'reason': args[1] @@ -69275,6 +69305,10 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_ }, showLeaveNotification(occupant) { + if (_.includes(occupant.get('states'), '303') || _.includes(occupant.get('states'), '307')) { + return; + } + const nick = occupant.get('nick'), stat = occupant.get('status'), last_el = this.content.lastElementChild; diff --git a/spec/chatroom.js b/spec/chatroom.js index 2458cd81a..fee167686 100644 --- a/spec/chatroom.js +++ b/spec/chatroom.js @@ -2107,7 +2107,7 @@ expect(_converse.connection.send).not.toHaveBeenCalled(); expect(view.el.querySelectorAll('.chat-error').length).toBe(1); expect(view.el.querySelector('.chat-error').textContent.trim()) - .toBe(`Error: Can't find a groupchat participant with the nickname "chris"`) + .toBe(`Error: couldn't find a groupchat participant "chris"`) // Now test with an existing nick textarea.value = '/member marc Welcome to the club!'; @@ -2312,18 +2312,33 @@ null, ['rosterGroupsFetched'], {}, function (done, _converse) { + let sent_IQ, IQ_id; + const sendIQ = _converse.connection.sendIQ; + spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) { + sent_IQ = iq; + IQ_id = sendIQ.bind(this)(iq, callback, errback); + }); + test_utils.openAndEnterChatRoom(_converse, 'lounge', 'localhost', 'dummy').then(function () { - var sent_IQ, IQ_id; - var sendIQ = _converse.connection.sendIQ; - spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) { - sent_IQ = iq; - IQ_id = sendIQ.bind(this)(iq, callback, errback); - }); var view = _converse.chatboxviews.get('lounge@localhost'); spyOn(view, 'onMessageSubmitted').and.callThrough(); spyOn(view.model, 'setAffiliation').and.callThrough(); spyOn(view, 'showErrorMessage').and.callThrough(); spyOn(view, 'validateRoleChangeCommand').and.callThrough(); + + let presence = $pres({ + 'from': 'lounge@localhost/annoyingGuy', + 'id':'27C55F89-1C6A-459A-9EB5-77690145D624', + 'to': 'dummy@localhost/desktop' + }) + .c('x', { 'xmlns': 'http://jabber.org/protocol/muc#user'}) + .c('item', { + 'jid': 'annoyingguy@localhost', + 'affiliation': 'member', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + var textarea = view.el.querySelector('.chat-textarea') textarea.value = '/owner'; view.keyPressed({ @@ -2337,23 +2352,42 @@ "Error: the \"owner\" command takes two arguments, the user's nickname and optionally a reason."); expect(view.model.setAffiliation).not.toHaveBeenCalled(); + view.onMessageSubmitted('/owner nobody You\'re responsible'); + expect(view.showErrorMessage).toHaveBeenCalledWith( + 'Error: couldn\'t find a groupchat participant "nobody"'); + expect(view.model.setAffiliation).not.toHaveBeenCalled(); + // Call now with the correct amount of arguments. // XXX: Calling onMessageSubmitted directly, trying // again via triggering Event doesn't work for some weird // reason. - view.onMessageSubmitted('/owner annoyingGuy@localhost You\'re responsible'); - expect(view.validateRoleChangeCommand.calls.count()).toBe(2); - expect(view.showErrorMessage.calls.count()).toBe(1); + view.onMessageSubmitted('/owner annoyingGuy You\'re responsible'); + expect(view.validateRoleChangeCommand.calls.count()).toBe(3); expect(view.model.setAffiliation).toHaveBeenCalled(); + expect(view.showErrorMessage.calls.count()).toBe(2); // Check that the member list now gets updated expect(sent_IQ.toLocaleString()).toBe( ""+ ""+ - ""+ + ""+ "You're responsible"+ ""+ ""+ ""); + + presence = $pres({ + 'from': 'lounge@localhost/annoyingGuy', + 'id':'27C55F89-1C6A-459A-9EB5-77690145D628', + 'to': 'dummy@localhost/desktop' + }) + .c('x', { 'xmlns': 'http://jabber.org/protocol/muc#user'}) + .c('item', { + 'jid': 'annoyingguy@localhost', + 'affiliation': 'owner', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + expect(view.el.querySelectorAll('.chat-info')[4].textContent).toBe("annoyingGuy is now an owner of this groupchat"); done(); }).catch(_.partial(console.error, _)); })); @@ -2363,19 +2397,35 @@ null, ['rosterGroupsFetched'], {}, function (done, _converse) { - test_utils.openAndEnterChatRoom(_converse, 'lounge', 'localhost', 'dummy').then(function () { - var sent_IQ, IQ_id; - var sendIQ = _converse.connection.sendIQ; - spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) { - sent_IQ = iq; - IQ_id = sendIQ.bind(this)(iq, callback, errback); - }); - var view = _converse.chatboxviews.get('lounge@localhost'); + let sent_IQ, IQ_id; + const sendIQ = _converse.connection.sendIQ; + spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) { + sent_IQ = iq; + IQ_id = sendIQ.bind(this)(iq, callback, errback); + }); + + test_utils.openAndEnterChatRoom(_converse, 'lounge', 'localhost', 'dummy') + .then(() => { + const view = _converse.chatboxviews.get('lounge@localhost'); spyOn(view, 'onMessageSubmitted').and.callThrough(); spyOn(view.model, 'setAffiliation').and.callThrough(); spyOn(view, 'showErrorMessage').and.callThrough(); spyOn(view, 'validateRoleChangeCommand').and.callThrough(); - var textarea = view.el.querySelector('.chat-textarea') + + let presence = $pres({ + 'from': 'lounge@localhost/annoyingGuy', + 'id':'27C55F89-1C6A-459A-9EB5-77690145D624', + 'to': 'dummy@localhost/desktop' + }) + .c('x', { 'xmlns': 'http://jabber.org/protocol/muc#user'}) + .c('item', { + 'jid': 'annoyingguy@localhost', + 'affiliation': 'member', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + + const textarea = view.el.querySelector('.chat-textarea') textarea.value = '/ban'; view.keyPressed({ target: textarea, @@ -2388,10 +2438,11 @@ "Error: the \"ban\" command takes two arguments, the user's nickname and optionally a reason."); expect(view.model.setAffiliation).not.toHaveBeenCalled(); // Call now with the correct amount of arguments. + // XXX: Calling onMessageSubmitted directly, trying // again via triggering Event doesn't work for some weird // reason. - view.onMessageSubmitted('/ban annoyingGuy@localhost You\'re annoying'); + view.onMessageSubmitted('/ban annoyingGuy You\'re annoying'); expect(view.validateRoleChangeCommand.calls.count()).toBe(2); expect(view.showErrorMessage.calls.count()).toBe(1); expect(view.model.setAffiliation).toHaveBeenCalled(); @@ -2399,11 +2450,27 @@ expect(sent_IQ.toLocaleString()).toBe( ""+ ""+ - ""+ + ""+ "You're annoying"+ ""+ ""+ ""); + + presence = $pres({ + 'from': 'lounge@localhost/annoyingGuy', + 'id':'27C55F89-1C6A-459A-9EB5-77690145D628', + 'to': 'dummy@localhost/desktop' + }) + .c('x', { 'xmlns': 'http://jabber.org/protocol/muc#user'}) + .c('item', { + 'jid': 'annoyingguy@localhost', + 'affiliation': 'outcast', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + expect( + view.el.querySelectorAll('.chat-info')[3].textContent).toBe( + "annoyingGuy has been banned from this groupchat"); done(); }).catch(_.partial(console.error, _)); })); @@ -2413,19 +2480,33 @@ null, ['rosterGroupsFetched'], {}, function (done, _converse) { + let sent_IQ, IQ_id; + const sendIQ = _converse.connection.sendIQ; + spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) { + sent_IQ = iq; + IQ_id = sendIQ.bind(this)(iq, callback, errback); + }); + test_utils.openAndEnterChatRoom(_converse, 'lounge', 'localhost', 'dummy').then(function () { - var sent_IQ, IQ_id; - var sendIQ = _converse.connection.sendIQ; - spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) { - sent_IQ = iq; - IQ_id = sendIQ.bind(this)(iq, callback, errback); - }); - var view = _converse.chatboxviews.get('lounge@localhost'); + const view = _converse.chatboxviews.get('lounge@localhost'); spyOn(view, 'onMessageSubmitted').and.callThrough(); spyOn(view, 'modifyRole').and.callThrough(); spyOn(view, 'showErrorMessage').and.callThrough(); spyOn(view, 'validateRoleChangeCommand').and.callThrough(); + let presence = $pres({ + 'from': 'lounge@localhost/annoyingGuy', + 'id':'27C55F89-1C6A-459A-9EB5-77690145D624', + 'to': 'dummy@localhost/desktop' + }) + .c('x', { 'xmlns': 'http://jabber.org/protocol/muc#user'}) + .c('item', { + 'jid': 'annoyingguy@localhost', + 'affiliation': 'none', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + var textarea = view.el.querySelector('.chat-textarea') textarea.value = '/kick'; view.keyPressed({ @@ -2465,7 +2546,7 @@ * * */ - var presence = $pres({ + presence = $pres({ 'from': 'lounge@localhost/annoyingGuy', 'to': 'dummy@localhost/desktop', 'type': 'unavailable' @@ -2477,9 +2558,8 @@ }).up() .c('status', {'code': '307'}); _converse.connection._dataRecv(test_utils.createRequest(presence)); - expect( - view.el.querySelectorAll('.chat-info')[2].textContent).toBe( - "annoyingGuy has been kicked out"); + expect(view.el.querySelectorAll('.chat-info')[3].textContent).toBe("annoyingGuy has been kicked out"); + expect(view.el.querySelectorAll('.chat-info').length).toBe(4); done(); }).catch(_.partial(console.error, _)); })); diff --git a/src/converse-muc-views.js b/src/converse-muc-views.js index bc75165fb..d6f30db9e 100644 --- a/src/converse-muc-views.js +++ b/src/converse-muc-views.js @@ -542,6 +542,8 @@ this.model.occupants.on('add', this.showJoinNotification, this); this.model.occupants.on('remove', this.showLeaveNotification, 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); this.createEmojiPicker(); this.createOccupantsView(); @@ -643,10 +645,32 @@ */ this.model.occupants.chatroomview = this; this.occupantsview = new _converse.ChatRoomOccupantsView({'model': this.model.occupants}); - this.occupantsview.model.on('change:role', this.informOfOccupantsRoleChange, this); return this; }, + informOfOccupantsAffiliationChange(occupant, changed) { + const previous_affiliation = occupant._previousAttributes.affiliation, + current_affiliation = occupant.get('affiliation'); + + if (previous_affiliation === 'admin') { + this.showChatEvent(__("%1$s is no longer an admin of this groupchat", occupant.get('nick'))) + } else if (previous_affiliation === 'owner') { + this.showChatEvent(__("%1$s is no longer an owner of this groupchat", occupant.get('nick'))) + } else if (previous_affiliation === 'outcast') { + this.showChatEvent(__("%1$s is no longer banned from this groupchat", occupant.get('nick'))) + } + + if (current_affiliation === 'none' && previous_affiliation === 'member') { + this.showChatEvent(__("%1$s is no longer a permanent member of this groupchat", occupant.get('nick'))) + } if (current_affiliation === 'member') { + this.showChatEvent(__("%1$s is now a permanent member of this groupchat", occupant.get('nick'))) + } else if (current_affiliation === 'outcast') { + this.showChatEvent(__("%1$s has been banned from this groupchat", occupant.get('nick'))) + } else if (current_affiliation === 'admin' || current_affiliation == 'owner') { + this.showChatEvent(__(`%1$s is now an ${current_affiliation} of this groupchat`, occupant.get('nick'))) + } + }, + informOfOccupantsRoleChange (occupant, changed) { const previous_role = occupant._previousAttributes.role; if (previous_role === 'moderator') { @@ -831,13 +855,16 @@ /* Check that a command to change a groupchat user's role or * affiliation has anough arguments. */ - // TODO check if first argument is valid if (args.length < 1 || args.length > 2) { this.showErrorMessage( __('Error: the "%1$s" command takes two arguments, the user\'s nickname and optionally a reason.', command) ); return false; } + if (!this.model.occupants.findWhere({'nick': args[0]}) && !this.model.occupants.findWhere({'jid': args[0]})) { + this.showErrorMessage(__('Error: couldn\'t find a groupchat participant "%1$s"', args[0])); + return false; + } return true; }, @@ -854,8 +881,8 @@ return false; } const match = text.replace(/^\s*/, "").match(/^\/(.*?)(?: (.*))?$/) || [false, '', ''], - args = match[2] && match[2].splitOnce(' ') || [], - command = match[1].toLowerCase(); + args = match[2] && match[2].splitOnce(' ') || [], + command = match[1].toLowerCase(); switch (command) { case 'admin': if (!this.verifyAffiliations(['owner']) || !this.validateRoleChangeCommand(command, args)) { @@ -873,6 +900,7 @@ if (!this.verifyAffiliations(['owner', 'admin']) || !this.validateRoleChangeCommand(command, args)) { break; } + this.model.setAffiliation('outcast', [{ 'jid': args[0], 'reason': args[1] @@ -929,11 +957,9 @@ if (!this.verifyAffiliations(['admin', 'owner']) || !this.validateRoleChangeCommand(command, args)) { break; } - const occupant = this.model.occupants.findWhere({'nick': args[0]}); - if (!occupant) { - this.showErrorMessage(__(`Error: Can't find a groupchat participant with the nickname "${args[0]}"`)); - break; - } + const occupant = this.model.occupants.findWhere({'nick': args[0]}) || + this.model.occupants.findWhere({'jid': args[0]}); + this.model.setAffiliation('member', [{ 'jid': occupant.get('jid'), 'reason': args[1] @@ -1485,6 +1511,9 @@ }, showLeaveNotification (occupant) { + if (_.includes(occupant.get('states'), '303') || _.includes(occupant.get('states'), '307')) { + return; + } const nick = occupant.get('nick'), stat = occupant.get('status'), last_el = this.content.lastElementChild;