Skip to content

Commit

Permalink
Fixes #3362: Don't create empty nick element in bookmarks
Browse files Browse the repository at this point in the history
  • Loading branch information
jcbrand committed Apr 3, 2024
1 parent fbda510 commit b8f8d36
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- #3305: New config option [muc_search_service](https://conversejs.org/docs/html/configuration.html#muc-search-service)
- #3307: Fix inconsistency between browsers on textarea outlines
- #3337: Correctly display multiline nested quotes
- #3362: Don't create empty nick element in bookmarks
- Fix: MUC occupant list does not sort itself on nicknames or roles changes
- Fix: refresh the MUC sidebar when participants collection is sorted
- Fix: room information not correctly refreshed when modifications are made by other users
Expand Down
1 change: 1 addition & 0 deletions karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module.exports = function(config) {
},
{ pattern: "src/shared/tests/mock.js", type: 'module' },

{ pattern: "src/headless/plugins/bookmarks/tests/bookmarks.js", type: 'module' },
{ pattern: "src/headless/plugins/caps/tests/caps.js", type: 'module' },
{ pattern: "src/headless/plugins/chat/tests/api.js", type: 'module' },
{ pattern: "src/headless/plugins/disco/tests/disco.js", type: 'module' },
Expand Down
20 changes: 15 additions & 5 deletions src/headless/plugins/bookmarks/collection.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
/**
* @typedef {import('@converse/headless').MUC} MUC
*/
import { Collection } from "@converse/skeletor";
import { getOpenPromise } from '@converse/openpromise';
import "../../plugins/muc/index.js";
import Bookmark from './model.js';
import _converse from '../../shared/_converse.js';
import api from '../../shared/api/index.js';
import converse from '../../shared/api/public.js';
import log from "../../log.js";
import { Collection } from "@converse/skeletor";
import { getOpenPromise } from '@converse/openpromise';
import { initStorage } from '../../utils/storage.js';

const { Strophe, $iq, sizzle } = converse.env;
Expand Down Expand Up @@ -33,7 +36,7 @@ class Bookmarks extends Collection {
* Triggered once the _converse.Bookmarks collection
* has been created and cached bookmarks have been fetched.
* @event _converse#bookmarksInitialized
* @type { Bookmarks }
* @type {Bookmarks}
* @example _converse.api.listen.on('bookmarksInitialized', (bookmarks) => { ... });
*/
api.trigger('bookmarksInitialized', this);
Expand Down Expand Up @@ -98,12 +101,19 @@ class Bookmarks extends Collection {
.c('publish', {'node': Strophe.NS.BOOKMARKS})
.c('item', {'id': 'current'})
.c('storage', {'xmlns': Strophe.NS.BOOKMARKS});
this.forEach(model => {

this.forEach(/** @param {MUC} model */(model) => {
stanza.c('conference', {
'name': model.get('name'),
'autojoin': model.get('autojoin'),
'jid': model.get('jid'),
}).c('nick').t(model.get('nick')).up().up();
});
const nick = model.get('nick');
if (nick) {
stanza.c('nick').t(nick).up().up();
} else {
stanza.up();
}
});
stanza.up().up().up();
stanza.c('publish-options')
Expand Down
116 changes: 116 additions & 0 deletions src/headless/plugins/bookmarks/tests/bookmarks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/* global mock, converse */

describe("A chat room", function () {

describe("when autojoin is set", function () {

it("will be be opened and joined automatically upon login", mock.initConverse(
[], {}, async function (_converse) {

await mock.waitForRoster(_converse, 'current', 0);
await mock.waitUntilBookmarksReturned(_converse);
spyOn(_converse.api.rooms, 'create').and.callThrough();
const jid = '[email protected]';
const model = _converse.bookmarks.create({
'jid': jid,
'autojoin': false,
'name': 'The Play',
'nick': ''
});
expect(_converse.api.rooms.create).not.toHaveBeenCalled();
_converse.bookmarks.remove(model);
_converse.bookmarks.create({
'jid': jid,
'autojoin': true,
'name': 'Hamlet',
'nick': ''
});
expect(_converse.api.rooms.create).toHaveBeenCalled();
}));
});
});


describe("A bookmark", function () {

it("can be created and sends out a stanza", mock.initConverse(
['connected', 'chatBoxesFetched'], {}, async function (_converse) {

await mock.waitForRoster(_converse, 'current', 0);
await mock.waitUntilBookmarksReturned(_converse);

const jid = _converse.session.get('jid');
const muc1_jid = '[email protected]';
const { Strophe, sizzle, u } = converse.env;

_converse.bookmarks.createBookmark({
jid: muc1_jid,
autojoin: true,
name: 'Hamlet',
nick: ''
});

const IQ_stanzas = _converse.api.connection.get().IQ_stanzas;
let sent_stanza = await u.waitUntil(
() => IQ_stanzas.filter(s => sizzle('item[id="current"]', s).length).pop());

expect(Strophe.serialize(sent_stanza)).toBe(
`<iq from="${jid}" id="${sent_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">`+
'<pubsub xmlns="http://jabber.org/protocol/pubsub">'+
'<publish node="storage:bookmarks">'+
'<item id="current">'+
'<storage xmlns="storage:bookmarks">'+
`<conference autojoin="true" jid="${muc1_jid}" name="Hamlet"/>`+
'</storage>'+
'</item>'+
'</publish>'+
'<publish-options>'+
'<x type="submit" xmlns="jabber:x:data">'+
'<field type="hidden" var="FORM_TYPE">'+
'<value>http://jabber.org/protocol/pubsub#publish-options</value>'+
'</field>'+
'<field var="pubsub#persist_items"><value>true</value></field>'+
'<field var="pubsub#access_model"><value>whitelist</value></field>'+
'</x>'+
'</publish-options>'+
'</pubsub>'+
'</iq>');


const muc2_jid = '[email protected]';
_converse.bookmarks.createBookmark({
jid: muc2_jid,
autojoin: true,
name: 'Balcony',
nick: 'romeo'
});

sent_stanza = await u.waitUntil(
() => IQ_stanzas.filter(s => sizzle('item[id="current"] conference[name="Balcony"]', s).length).pop());

expect(Strophe.serialize(sent_stanza)).toBe(
`<iq from="${jid}" id="${sent_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">`+
'<pubsub xmlns="http://jabber.org/protocol/pubsub">'+
'<publish node="storage:bookmarks">'+
'<item id="current">'+
'<storage xmlns="storage:bookmarks">'+
`<conference autojoin="true" jid="${muc2_jid}" name="Balcony">`+
'<nick>romeo</nick>'+
'</conference>'+
`<conference autojoin="true" jid="${muc1_jid}" name="Hamlet"/>`+
'</storage>'+
'</item>'+
'</publish>'+
'<publish-options>'+
'<x type="submit" xmlns="jabber:x:data">'+
'<field type="hidden" var="FORM_TYPE">'+
'<value>http://jabber.org/protocol/pubsub#publish-options</value>'+
'</field>'+
'<field var="pubsub#persist_items"><value>true</value></field>'+
'<field var="pubsub#access_model"><value>whitelist</value></field>'+
'</x>'+
'</publish-options>'+
'</pubsub>'+
'</iq>');
}));
});
36 changes: 6 additions & 30 deletions src/plugins/bookmark-views/tests/bookmarks.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ describe("A chat room", function () {
expect(room.get('nick')).toBe('Othello');
}));

it("displays that it's bookmarked through its bookmark icon", mock.initConverse([], {}, async function (_converse) {
it("displays that it's bookmarked through its bookmark icon",
mock.initConverse([], {}, async function (_converse) {

const { u } = converse.env;
await mock.waitForRoster(_converse, 'current', 0);
Expand Down Expand Up @@ -293,33 +294,6 @@ describe("A chat room", function () {
);
}));
});

describe("and when autojoin is set", function () {

it("will be be opened and joined automatically upon login", mock.initConverse(
[], {}, async function (_converse) {

await mock.waitForRoster(_converse, 'current', 0);
await mock.waitUntilBookmarksReturned(_converse);
spyOn(_converse.api.rooms, 'create').and.callThrough();
const jid = '[email protected]';
const model = _converse.bookmarks.create({
'jid': jid,
'autojoin': false,
'name': 'The Play',
'nick': ''
});
expect(_converse.api.rooms.create).not.toHaveBeenCalled();
_converse.bookmarks.remove(model);
_converse.bookmarks.create({
'jid': jid,
'autojoin': true,
'name': 'Hamlet',
'nick': ''
});
expect(_converse.api.rooms.create).toHaveBeenCalled();
}));
});
});

describe("Bookmarks", function () {
Expand Down Expand Up @@ -407,7 +381,9 @@ describe("Bookmarks", function () {
_converse.api.connection.get()._dataRecv(mock.createRequest(stanza));

await u.waitUntil(() => _converse.bookmarks.length === 3);
expect(_converse.bookmarks.map(b => b.get('name'))).toEqual(['Second bookmark', 'The Play&apos;s the Thing', 'Yet another bookmark']);
expect(_converse.bookmarks.map(b => b.get('name'))).toEqual(
['Second bookmark', 'The Play&apos;s the Thing', 'Yet another bookmark']
);
expect(_converse.chatboxviews.get('[email protected]')).not.toBeUndefined();
expect(Object.keys(_converse.chatboxviews.getAll()).length).toBe(2);
}));
Expand Down Expand Up @@ -468,7 +444,7 @@ describe("Bookmarks", function () {
expect(_converse.bookmarks.models.length).toBe(0);

spyOn(_converse.bookmarks, 'onBookmarksReceived').and.callThrough();
var stanza = $iq({'to': _converse.api.connection.get().jid, 'type':'result', 'id':sent_stanza.getAttribute('id')})
const stanza = $iq({'to': _converse.api.connection.get().jid, 'type':'result', 'id':sent_stanza.getAttribute('id')})
.c('pubsub', {'xmlns': Strophe.NS.PUBSUB})
.c('items', {'node': 'storage:bookmarks'})
.c('item', {'id': 'current'})
Expand Down

0 comments on commit b8f8d36

Please sign in to comment.