From e91a38547a8e402645ea6215ac479d7e16d13d38 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Mon, 13 Apr 2020 11:10:25 +0200 Subject: [PATCH] MUC: create role/affiliation change message objects in @converse/headless instead of injecting HTML via the view --- spec/muc.js | 69 +++++++++++++++++----------- spec/retractions.js | 52 ++++++++++++--------- src/converse-muc-views.js | 50 +-------------------- src/headless/converse-muc.js | 87 ++++++++++++++++++++++++++++++++++-- 4 files changed, 157 insertions(+), 101 deletions(-) diff --git a/spec/muc.js b/spec/muc.js index 07014926e..61155396b 100644 --- a/spec/muc.js +++ b/spec/muc.js @@ -2804,8 +2804,8 @@ 'role': 'visitor' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - let info_msgs = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info'), 0); - expect(info_msgs.pop().textContent.trim()).toBe("annoyingGuy has been muted"); + const info_msg = await u.waitUntil(() => view.el.querySelector('.chat-info__message')); + expect(info_msg.textContent.trim()).toBe("annoyingGuy has been muted"); presence = $pres({ 'from': 'lounge@montague.lit/annoyingGuy', @@ -2818,8 +2818,10 @@ 'role': 'participant' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - info_msgs = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info'), 0); - expect(info_msgs.pop().textContent.trim()).toBe("annoyingGuy has been given a voice"); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "annoyingGuy has been given a voice" + ); // Check that we don't see an info message concerning the role, // if the affiliation has changed. @@ -2834,8 +2836,10 @@ 'role': 'visitor' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - info_msgs = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info'), 0); - expect(info_msgs.pop().textContent.trim()).toBe("annoyingGuy is no longer a member of this groupchat"); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "annoyingGuy is no longer a member of this groupchat" + ); done(); })); @@ -3331,7 +3335,10 @@ 'role': 'participant' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - expect(view.el.querySelectorAll('.chat-info')[2].textContent.trim()).toBe("annoyingGuy is now an owner of this groupchat"); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "annoyingGuy is now an owner of this groupchat" + ); done(); })); @@ -3547,7 +3554,6 @@ }); spyOn(view.model, 'setRole').and.callThrough(); spyOn(view, 'showErrorMessage').and.callThrough(); - spyOn(view, 'showChatEvent').and.callThrough(); spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough(); // New user enters the groupchat @@ -3628,8 +3634,11 @@ 'role': 'moderator' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - let info_msgs = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info'), 0); - expect(info_msgs.pop().textContent.trim()).toBe("trustworthyguy is now a moderator"); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "trustworthyguy is now a moderator" + ); + // Call now with the correct amount of arguments. // XXX: Calling onFormSubmitted directly, trying // again via triggering Event doesn't work for some weird @@ -3638,7 +3647,6 @@ view.onFormSubmitted(new Event('submit')); expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3); - expect(view.showChatEvent.calls.count()).toBe(1); expect(view.model.setRole).toHaveBeenCalled(); expect(sent_IQ.toLocaleString()).toBe( ``+ @@ -3667,10 +3675,12 @@ 'jid': 'trustworthyguy@montague.lit', 'affiliation': 'member', 'role': 'participant' - }); + }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - info_msgs = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info'), 0); - expect(info_msgs.pop().textContent.trim()).toBe("trustworthyguy is no longer a moderator"); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "trustworthyguy is no longer a moderator" + ); done(); })); @@ -3690,7 +3700,6 @@ }); spyOn(view.model, 'setRole').and.callThrough(); spyOn(view, 'showErrorMessage').and.callThrough(); - spyOn(view, 'showChatEvent').and.callThrough(); spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough(); // New user enters the groupchat @@ -3770,8 +3779,10 @@ 'role': 'visitor' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - let info_msgs = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info'), 0); - expect(info_msgs.pop().textContent.trim()).toBe("annoyingGuy has been muted"); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "annoyingGuy has been muted" + ); // Call now with the correct of arguments. // XXX: Calling onFormSubmitted directly, trying @@ -3781,7 +3792,6 @@ view.onFormSubmitted(new Event('submit')); expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3); - expect(view.showChatEvent.calls.count()).toBe(1); expect(view.model.setRole).toHaveBeenCalled(); expect(sent_IQ.toLocaleString()).toBe( ``+ @@ -3813,8 +3823,10 @@ 'role': 'participant' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - info_msgs = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info'), 0); - expect(info_msgs.pop().textContent.trim()).toBe("annoyingGuy has been given a voice"); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "annoyingGuy has been given a voice" + ); done(); })); @@ -5231,9 +5243,10 @@ expect(bottom_panel.textContent.trim()).toBe("You're not allowed to send messages in this room"); // Check now that things get restored when the user is given a voice - let info_msgs = sizzle('.chat-info', view.el); - expect(info_msgs.length).toBe(1); - expect(info_msgs[0].textContent.trim()).toBe("troll is no longer an owner of this groupchat"); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "troll is no longer an owner of this groupchat" + ); stanza = u.toStanza(` `); _converse.connection._dataRecv(test_utils.createRequest(stanza)); - info_msgs = sizzle('.chat-info', view.el); - bottom_panel = view.el.querySelector('.muc-bottom-panel'); expect(bottom_panel).toBe(null); textarea = view.el.querySelector('.chat-textarea'); expect(textarea === null).toBe(false); - expect(info_msgs.length).toBe(2); - expect(info_msgs[1].textContent.trim()).toBe("troll has been given a voice"); + // Check now that things get restored when the user is given a voice + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "troll has been given a voice" + ); + expect(view.el.querySelectorAll('.chat-info__message').length).toBe(2); done(); })); }); diff --git a/spec/retractions.js b/spec/retractions.js index 069bd93aa..b105676ff 100644 --- a/spec/retractions.js +++ b/spec/retractions.js @@ -12,8 +12,8 @@ async function sendAndThenRetractMessage (_converse, view) { view.model.sendMessage('hello world'); - await u.waitUntil(() => view.el.querySelectorAll('.chat-msg').length === 1); - const msg_obj = view.model.messages.at(0); + await u.waitUntil(() => view.el.querySelectorAll('.chat-msg__text').length === 1); + const msg_obj = view.model.messages.last(); const reflection_stanza = u.toStanza(` view.el.querySelectorAll('.chat-msg--retracted').length === 1); - const msg_obj = view.model.messages.at(0); + const msg_obj = view.model.messages.last(); expect(Strophe.serialize(retraction_stanza)).toBe( ``+ ``+ @@ -634,7 +634,7 @@ ``+ ``); - const message = view.model.messages.at(0); + const message = view.model.messages.last(); expect(message.get('retracted')).toBeTruthy(); expect(message.get('is_ephemeral')).toBe(false); expect(message.get('editable')).toBeFalsy(); @@ -654,10 +654,10 @@ _converse.connection._dataRecv(test_utils.createRequest(reflection)); await u.waitUntil(() => view.model.handleRetraction.calls.count() === 1); - expect(view.model.messages.length).toBe(1); - expect(view.model.messages.at(0).get('retracted')).toBeTruthy(); - expect(view.model.messages.at(0).get('is_ephemeral')).toBe(false); - expect(view.model.messages.at(0).get('editable')).toBe(false); + expect(view.model.messages.length).toBe(2); + expect(view.model.messages.last().get('retracted')).toBeTruthy(); + expect(view.model.messages.last().get('is_ephemeral')).toBe(false); + expect(view.model.messages.last().get('editable')).toBe(false); expect(view.el.querySelectorAll('.chat-msg--retracted').length).toBe(1); const el = view.el.querySelector('.chat-msg--retracted .chat-msg__message div'); expect(el.textContent).toBe('romeo has removed this message'); @@ -676,15 +676,19 @@ const occupant = view.model.getOwnOccupant(); expect(occupant.get('role')).toBe('moderator'); occupant.save('role', 'member'); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "romeo is no longer a moderator" + ); const retraction_stanza = await sendAndThenRetractMessage(_converse, view); await u.waitUntil(() => view.el.querySelectorAll('.chat-msg--retracted').length === 1); - expect(view.model.messages.length).toBe(1); - expect(view.model.messages.at(0).get('retracted')).toBeTruthy(); + expect(view.model.messages.length).toBe(2); + expect(view.model.messages.last().get('retracted')).toBeTruthy(); const el = view.el.querySelector('.chat-msg--retracted .chat-msg__message div'); expect(el.textContent.trim()).toBe('romeo has removed this message'); - const message = view.model.messages.at(0); + const message = view.model.messages.last(); const stanza_id = message.get(`stanza_id ${view.model.get('jid')}`); // The server responds with an error message const error = u.toStanza(` @@ -702,10 +706,10 @@ _converse.connection._dataRecv(test_utils.createRequest(error)); await u.waitUntil(() => view.el.querySelectorAll('.chat-error').length === 1); await u.waitUntil(() => view.el.querySelectorAll('.chat-msg--retracted').length === 0); - expect(view.model.messages.length).toBe(1); - expect(view.model.messages.at(0).get('retracted')).toBeFalsy(); - expect(view.model.messages.at(0).get('is_ephemeral')).toBeFalsy(); - expect(view.model.messages.at(0).get('editable')).toBeTruthy(); + expect(view.model.messages.length).toBe(2); + expect(view.model.messages.last().get('retracted')).toBeFalsy(); + expect(view.model.messages.last().get('is_ephemeral')).toBeFalsy(); + expect(view.model.messages.last().get('editable')).toBeTruthy(); expect(view.el.querySelectorAll('.chat-error').length).toBe(1); const errmsg = view.el.querySelector('.chat-error'); @@ -713,7 +717,7 @@ done(); })); - it("can be retracted by its author, causing an timeout error in response", + it("can be retracted by its author, causing a timeout error in response", mock.initConverse( ['rosterGroupsFetched', 'chatBoxesFetched'], {}, async function (done, _converse) { @@ -727,21 +731,25 @@ const occupant = view.model.getOwnOccupant(); expect(occupant.get('role')).toBe('moderator'); occupant.save('role', 'member'); + await u.waitUntil(() => + Array.from(view.el.querySelectorAll('.chat-info__message')).pop()?.textContent.trim() === + "romeo is no longer a moderator" + ); await sendAndThenRetractMessage(_converse, view); await u.waitUntil(() => view.el.querySelectorAll('.chat-msg--retracted').length === 1); - expect(view.model.messages.length).toBe(1); - expect(view.model.messages.at(0).get('retracted')).toBeTruthy(); + expect(view.model.messages.length).toBe(2); + expect(view.model.messages.last().get('retracted')).toBeTruthy(); const el = view.el.querySelector('.chat-msg--retracted .chat-msg__message div'); expect(el.textContent.trim()).toBe('romeo has removed this message'); await u.waitUntil(() => view.el.querySelectorAll('.chat-msg').length === 1); await u.waitUntil(() => view.el.querySelectorAll('.chat-msg--retracted').length === 0); - expect(view.model.messages.length).toBe(1); - expect(view.model.messages.at(0).get('retracted')).toBeFalsy(); - expect(view.model.messages.at(0).get('is_ephemeral')).toBeFalsy(); - expect(view.model.messages.at(0).get('editable')).toBeTruthy(); + expect(view.model.messages.length).toBe(2); + expect(view.model.messages.last().get('retracted')).toBeFalsy(); + expect(view.model.messages.last().get('is_ephemeral')).toBeFalsy(); + expect(view.model.messages.last().get('editable')).toBeTruthy(); const error_messages = view.el.querySelectorAll('.chat-error'); expect(error_messages.length).toBe(2); diff --git a/src/converse-muc-views.js b/src/converse-muc-views.js index bc587b6e4..a87479b65 100644 --- a/src/converse-muc-views.js +++ b/src/converse-muc-views.js @@ -1123,60 +1123,12 @@ converse.plugins.add('converse-muc-views', { if (occupant.get('jid') === _converse.bare_jid) { this.renderHeading(); } - this.informOfOccupantsAffiliationChange(occupant); }, - informOfOccupantsAffiliationChange (occupant) { - const previous_affiliation = occupant._previousAttributes.affiliation; - const 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 member of this groupchat", occupant.get('nick'))) - } if (current_affiliation === 'member') { - this.showChatEvent(__("%1$s is now a member of this groupchat", occupant.get('nick'))) - } else if (current_affiliation === 'admin' || current_affiliation == 'owner') { - // For example: AppleJack is now an (admin|owner) of this groupchat - this.showChatEvent(__('%1$s is now an %2$s of this groupchat', occupant.get('nick'), current_affiliation)) - } - }, - - onOccupantRoleChanged (occupant, changed) { + onOccupantRoleChanged (occupant) { if (occupant.get('jid') === _converse.bare_jid) { this.renderBottomPanel(); } - this.informOfOccupantsRoleChange(occupant, changed); - }, - - informOfOccupantsRoleChange (occupant, changed) { - if (changed === "none" || occupant.changed.affiliation) { - // We don't inform of role changes if they accompany affiliation changes. - return; - } - const previous_role = occupant._previousAttributes.role; - if (previous_role === 'moderator') { - this.showChatEvent(__("%1$s is no longer a moderator", occupant.get('nick'))) - } - if (previous_role === 'visitor') { - this.showChatEvent(__("%1$s has been given a voice", occupant.get('nick'))) - } - if (occupant.get('role') === 'visitor') { - this.showChatEvent(__("%1$s has been muted", occupant.get('nick'))) - } - if (occupant.get('role') === 'moderator') { - if (!['owner', 'admin'].includes(occupant.get('affiliation'))) { - // We only show this message if the user isn't already - // an admin or owner, otherwise this isn't new - // information. - this.showChatEvent(__("%1$s is now a moderator", occupant.get('nick'))) - } - } }, /** diff --git a/src/headless/converse-muc.js b/src/headless/converse-muc.js index 8ddedb54e..e57d6da92 100644 --- a/src/headless/converse-muc.js +++ b/src/headless/converse-muc.js @@ -383,6 +383,8 @@ converse.plugins.add('converse-muc', { this.listenTo(this.occupants, 'add', this.onOccupantAdded); this.listenTo(this.occupants, 'remove', this.onOccupantRemoved); this.listenTo(this.occupants, 'change:show', this.onOccupantShowChanged); + this.listenTo(this.occupants, 'change:affiliation', this.createAffiliationChangeMessage); + this.listenTo(this.occupants, 'change:role', this.createRoleChangeMessage); const restored = await this.restoreFromCache() if (!restored) { @@ -2109,6 +2111,86 @@ converse.plugins.add('converse-muc', { } }, + createAffiliationChangeMessage (occupant) { + const previous_affiliation = occupant._previousAttributes.affiliation; + const current_affiliation = occupant.get('affiliation'); + if (previous_affiliation === 'admin') { + this.createMessage({ + 'type': 'info', + 'message': __("%1$s is no longer an admin of this groupchat", occupant.get('nick')) + }); + } else if (previous_affiliation === 'owner') { + this.createMessage({ + 'type': 'info', + 'message': __("%1$s is no longer an owner of this groupchat", occupant.get('nick')) + }); + } else if (previous_affiliation === 'outcast') { + this.createMessage({ + 'type': 'info', + 'message': __("%1$s is no longer banned from this groupchat", occupant.get('nick')) + }); + } + + if (current_affiliation === 'none' && previous_affiliation === 'member') { + this.createMessage({ + 'type': 'info', + 'message': __("%1$s is no longer a member of this groupchat", occupant.get('nick')) + }); + } + + if (current_affiliation === 'member') { + this.createMessage({ + 'type': 'info', + 'message': __("%1$s is now a member of this groupchat", occupant.get('nick')) + }); + } else if (current_affiliation === 'admin' || current_affiliation == 'owner') { + // For example: AppleJack is now an (admin|owner) of this groupchat + this.createMessage({ + 'type': 'info', + 'message': __( + '%1$s is now an %2$s of this groupchat', + occupant.get('nick'), + current_affiliation + ) + }); + } + }, + + createRoleChangeMessage (occupant, changed) { + if (changed === "none" || occupant.changed.affiliation) { + // We don't inform of role changes if they accompany affiliation changes. + return; + } + const previous_role = occupant._previousAttributes.role; + if (previous_role === 'moderator') { + this.createMessage({ + 'type': 'info', + 'message': __("%1$s is no longer a moderator", occupant.get('nick')) + }); + } + if (previous_role === 'visitor') { + this.createMessage({ + 'type': 'info', + 'message': __("%1$s has been given a voice", occupant.get('nick')) + }); + } + if (occupant.get('role') === 'visitor') { + this.createMessage({ + 'type': 'info', + 'message': __("%1$s has been muted", occupant.get('nick')) + }); + } + if (occupant.get('role') === 'moderator') { + if (!['owner', 'admin'].includes(occupant.get('affiliation'))) { + // Oly show this message if the user isn't already + // an admin or owner, otherwise this isn't new information. + this.createMessage({ + 'type': 'info', + 'message': __("%1$s is now a moderator", occupant.get('nick')) + }); + } + } + }, /** * Create info messages based on a received presence or message stanza @@ -2122,10 +2204,9 @@ converse.plugins.add('converse-muc', { if (!x) { return; } - const codes = sizzle('status', x).map(s => s.getAttribute('code')); - codes.forEach(code => { + sizzle('status', x).map(s => s.getAttribute('code')).forEach(code => { const data = { - 'type': 'info' + 'type': 'info', }; if (code === '110' || (code === '100' && !is_self)) { return;