Skip to content

Commit

Permalink
Fixes #1586. Not possible to kick someone with space in nick
Browse files Browse the repository at this point in the history 
Refactored moderation by moving certain methods to the model and
consolidating setting of roles and affiliations into new methods.
  • Loading branch information
@jcbrand
jcbrand committed May 27, 2019
1 parent 962eb42 commit 45c465a
Show file tree
Hide file tree
Showing 4 changed files with 202 additions and 185 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Expand Up @@ -29,6 +29,7 @@
- #1558: `this.get` is not a function error when `forward_messages` is set to `true`.
- #1572: In `fullscreen` view mode the top is cut off on iOS
- #1576: Converse gets stuck with spinner when logging out with `auto_login` set to `true`
- #1586: Not possible to kick someone with a space in their nickname

- **Breaking changes**:
- Rename `muc_disable_moderator_commands` to [muc_disable_slash_commands](https://conversejs.org/docs/html/configuration.html#muc-disable-slash-commands).
Expand Down
58 changes: 29 additions & 29 deletions spec/muc.js
View file
Expand Up @@ -2802,7 +2802,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: couldn't find a groupchat participant "chris"`)
.toBe('Error: couldn\'t find a groupchat participant based on your arguments');

// Now test with an existing nick
textarea.value = '/member marc Welcome to the club!';
Expand Down Expand Up @@ -2993,7 +2993,7 @@
const view = _converse.chatboxviews.get('lounge@localhost');
spyOn(view.model, 'setAffiliation').and.callThrough();
spyOn(view, 'showErrorMessage').and.callThrough();
spyOn(view, 'validateRoleChangeCommand').and.callThrough();
spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();

let presence = $pres({
'from': 'lounge@localhost/annoyingGuy',
Expand All @@ -3015,7 +3015,7 @@
preventDefault: _.noop,
keyCode: 13
});
expect(view.validateRoleChangeCommand).toHaveBeenCalled();
expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
expect(view.showErrorMessage).toHaveBeenCalledWith(
"Error: the \"owner\" command takes two arguments, the user's nickname and optionally a reason.");
expect(view.model.setAffiliation).not.toHaveBeenCalled();
Expand All @@ -3026,7 +3026,7 @@
view.onFormSubmitted(new Event('submit'));

expect(view.showErrorMessage).toHaveBeenCalledWith(
'Error: couldn\'t find a groupchat participant "nobody"');
"Error: couldn't find a groupchat participant based on your arguments");
expect(view.model.setAffiliation).not.toHaveBeenCalled();

// Call now with the correct of arguments.
Expand All @@ -3036,14 +3036,14 @@
textarea.value = '/owner annoyingGuy You\'re responsible';
view.onFormSubmitted(new Event('submit'));

expect(view.validateRoleChangeCommand.calls.count()).toBe(3);
expect(view.validateRoleOrAffiliationChangeArgs.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(
`<iq id="${IQ_id}" to="lounge@localhost" type="set" xmlns="jabber:client">`+
`<query xmlns="http://jabber.org/protocol/muc#admin">`+
`<item affiliation="owner" jid="annoyingGuy">`+
`<item affiliation="owner" jid="annoyingguy@localhost">`+
`<reason>You&apos;re responsible</reason>`+
`</item>`+
`</query>`+
Expand Down Expand Up @@ -3081,7 +3081,7 @@
const view = _converse.chatboxviews.get('lounge@localhost');
spyOn(view.model, 'setAffiliation').and.callThrough();
spyOn(view, 'showErrorMessage').and.callThrough();
spyOn(view, 'validateRoleChangeCommand').and.callThrough();
spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();

let presence = $pres({
'from': 'lounge@localhost/annoyingGuy',
Expand All @@ -3103,7 +3103,7 @@
preventDefault: _.noop,
keyCode: 13
});
expect(view.validateRoleChangeCommand).toHaveBeenCalled();
expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
expect(view.showErrorMessage).toHaveBeenCalledWith(
"Error: the \"ban\" command takes two arguments, the user's nickname and optionally a reason.");
expect(view.model.setAffiliation).not.toHaveBeenCalled();
Expand All @@ -3114,14 +3114,14 @@
textarea.value = '/ban annoyingGuy You\'re annoying';
view.onFormSubmitted(new Event('submit'));

expect(view.validateRoleChangeCommand.calls.count()).toBe(2);
expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
expect(view.showErrorMessage.calls.count()).toBe(1);
expect(view.model.setAffiliation).toHaveBeenCalled();
// Check that the member list now gets updated
expect(sent_IQ.toLocaleString()).toBe(
`<iq id="${IQ_id}" to="lounge@localhost" type="set" xmlns="jabber:client">`+
`<query xmlns="http://jabber.org/protocol/muc#admin">`+
`<item affiliation="outcast" jid="annoyingGuy">`+
`<item affiliation="outcast" jid="annoyingguy@localhost">`+
`<reason>You&apos;re annoying</reason>`+
`</item>`+
`</query>`+
Expand All @@ -3145,7 +3145,7 @@
done();
}));

it("accepts a /kick command to kick a user",
it("takes a /kick command to kick a user",
mock.initConverse(
null, ['rosterGroupsFetched'], {},
async function (done, _converse) {
Expand All @@ -3161,10 +3161,10 @@
const view = _converse.chatboxviews.get('lounge@localhost');
spyOn(view.model, 'setRole').and.callThrough();
spyOn(view, 'showErrorMessage').and.callThrough();
spyOn(view, 'validateRoleChangeCommand').and.callThrough();
spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();

let presence = $pres({
'from': 'lounge@localhost/annoyingGuy',
'from': 'lounge@localhost/annoying guy',
'id':'27C55F89-1C6A-459A-9EB5-77690145D624',
'to': 'dummy@localhost/desktop'
})
Expand All @@ -3183,24 +3183,24 @@
preventDefault: _.noop,
keyCode: 13
});
expect(view.validateRoleChangeCommand).toHaveBeenCalled();
expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
expect(view.showErrorMessage).toHaveBeenCalledWith(
"Error: the \"kick\" command takes two arguments, the user's nickname and optionally a reason.");
expect(view.model.setRole).not.toHaveBeenCalled();
// Call now with the correct amount of arguments.
// XXX: Calling onFormSubmitted directly, trying
// again via triggering Event doesn't work for some weird
// reason.
textarea.value = '/kick annoyingGuy You\'re annoying';
textarea.value = '/kick annoying guy You\'re annoying';
view.onFormSubmitted(new Event('submit'));

expect(view.validateRoleChangeCommand.calls.count()).toBe(2);
expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
expect(view.showErrorMessage.calls.count()).toBe(1);
expect(view.model.setRole).toHaveBeenCalled();
expect(sent_IQ.toLocaleString()).toBe(
`<iq id="${IQ_id}" to="lounge@localhost" type="set" xmlns="jabber:client">`+
`<query xmlns="http://jabber.org/protocol/muc#admin">`+
`<item nick="annoyingGuy" role="none">`+
`<item nick="annoying guy" role="none">`+
`<reason>You&apos;re annoying</reason>`+
`</item>`+
`</query>`+
Expand All @@ -3217,7 +3217,7 @@
* </presence>
*/
presence = $pres({
'from': 'lounge@localhost/annoyingGuy',
'from': 'lounge@localhost/annoying guy',
'to': 'dummy@localhost/desktop',
'type': 'unavailable'
})
Expand All @@ -3228,7 +3228,7 @@
}).up()
.c('status', {'code': '307'});
_converse.connection._dataRecv(test_utils.createRequest(presence));
expect(view.el.querySelectorAll('.chat-info')[3].textContent).toBe("annoyingGuy has been kicked out");
expect(view.el.querySelectorAll('.chat-info')[3].textContent).toBe("annoying guy has been kicked out");
expect(view.el.querySelectorAll('.chat-info').length).toBe(4);
done();
}));
Expand All @@ -3246,11 +3246,11 @@
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.model, 'setRole').and.callThrough();
spyOn(view, 'showErrorMessage').and.callThrough();
spyOn(view, 'showChatEvent').and.callThrough();
spyOn(view, 'validateRoleChangeCommand').and.callThrough();
spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();

// New user enters the groupchat
/* <presence
Expand All @@ -3262,7 +3262,7 @@
* </x>
* </presence>
*/
var presence = $pres({
let presence = $pres({
'from': 'lounge@localhost/trustworthyguy',
'id':'27C55F89-1C6A-459A-9EB5-77690145D624',
'to': 'dummy@localhost/desktop'
Expand All @@ -3285,7 +3285,7 @@
keyCode: 13
});

expect(view.validateRoleChangeCommand).toHaveBeenCalled();
expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
expect(view.showErrorMessage).toHaveBeenCalledWith(
"Error: the \"op\" command takes two arguments, the user's nickname and optionally a reason.");

Expand All @@ -3297,7 +3297,7 @@
textarea.value = '/op trustworthyguy You\'re trustworthy';
view.onFormSubmitted(new Event('submit'));

expect(view.validateRoleChangeCommand.calls.count()).toBe(2);
expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
expect(view.showErrorMessage.calls.count()).toBe(1);
expect(view.model.setRole).toHaveBeenCalled();
expect(sent_IQ.toLocaleString()).toBe(
Expand Down Expand Up @@ -3339,7 +3339,7 @@
textarea.value = '/deop trustworthyguy Perhaps not';
view.onFormSubmitted(new Event('submit'));

expect(view.validateRoleChangeCommand.calls.count()).toBe(3);
expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3);
expect(view.showChatEvent.calls.count()).toBe(1);
expect(view.model.setRole).toHaveBeenCalled();
expect(sent_IQ.toLocaleString()).toBe(
Expand Down Expand Up @@ -3392,7 +3392,7 @@
spyOn(view.model, 'setRole').and.callThrough();
spyOn(view, 'showErrorMessage').and.callThrough();
spyOn(view, 'showChatEvent').and.callThrough();
spyOn(view, 'validateRoleChangeCommand').and.callThrough();
spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();

// New user enters the groupchat
/* <presence
Expand Down Expand Up @@ -3427,7 +3427,7 @@
keyCode: 13
});

expect(view.validateRoleChangeCommand).toHaveBeenCalled();
expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
expect(view.showErrorMessage).toHaveBeenCalledWith(
"Error: the \"mute\" command takes two arguments, the user's nickname and optionally a reason.");
expect(view.model.setRole).not.toHaveBeenCalled();
Expand All @@ -3438,7 +3438,7 @@
textarea.value = '/mute annoyingGuy You\'re annoying';
view.onFormSubmitted(new Event('submit'));

expect(view.validateRoleChangeCommand.calls.count()).toBe(2);
expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
expect(view.showErrorMessage.calls.count()).toBe(1);
expect(view.model.setRole).toHaveBeenCalled();
expect(sent_IQ.toLocaleString()).toBe(
Expand Down Expand Up @@ -3481,7 +3481,7 @@
textarea.value = '/voice annoyingGuy Now you can talk again';
view.onFormSubmitted(new Event('submit'));

expect(view.validateRoleChangeCommand.calls.count()).toBe(3);
expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3);
expect(view.showChatEvent.calls.count()).toBe(1);
expect(view.model.setRole).toHaveBeenCalled();
expect(sent_IQ.toLocaleString()).toBe(
Expand Down

0 comments on commit 45c465a

Please sign in to comment.