Bugfix. Need to first create bookmark

Otherwise we don't include it in the stanza.
So if an error occurs, remove the bookmark again.

dist/converse.js

@@ -59675,11 +59675,12 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_
         },
 
         createBookmark(options) {
-          _converse.bookmarks.sendBookmarkStanza().then(() => _converse.bookmarks.create(options)).catch(() => this.onBookmarkError.apply(this, arguments));
+          this.create(options);
+          this.sendBookmarkStanza().catch(iq => this.onBookmarkError(iq, options));
         },
 
         sendBookmarkStanza() {
-          let stanza = $iq({
+          const stanza = $iq({
             'type': 'set',
             'from': _converse.connection.jid
           }).c('pubsub', {
@@ -59691,8 +59692,8 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_
           }).c('storage', {
             'xmlns': 'storage:bookmarks'
           });
-          this.each(function (model) {
-            stanza = stanza.c('conference', {
+          this.each(model => {
+            stanza.c('conference', {
               'name': model.get('name'),
               'autojoin': model.get('autojoin'),
               'jid': model.get('jid')
@@ -59713,12 +59714,16 @@ var __WEBPACK_AMD_DEFINE_FACTORY__, __WEBPACK_AMD_DEFINE_ARRAY__, __WEBPACK_AMD_
           return _converse.api.sendIQ(stanza);
         },
 
-        onBookmarkError(iq) {
+        onBookmarkError(iq, options) {
           _converse.log("Error while trying to add bookmark", Strophe.LogLevel.ERROR);
 
           _converse.log(iq);
 
           _converse.api.alert.show(Strophe.LogLevel.ERROR, __('Error'), [__("Sorry, something went wrong while trying to save your bookmark.")]);
+
+          this.findWhere({
+            'jid': options.jid
+          }).destroy();
         },
 
         fetchBookmarksFromServer(deferred) {

spec/bookmarks.js

@@ -26,15 +26,15 @@
                 [{'category': 'pubsub', 'type': 'pep'}],
                 ['http://jabber.org/protocol/pubsub#publish-options']
             ).then(function () {
-                var sent_stanza, IQ_id;
-                var sendIQ = _converse.connection.sendIQ;
+                let sent_stanza, IQ_id;
+                const sendIQ = _converse.connection.sendIQ;
                 spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) {
                     sent_stanza = iq;
                     IQ_id = sendIQ.bind(this)(iq, callback, errback);
                 });
                 spyOn(_converse.connection, 'getUniqueId').and.callThrough();
 
-                let view;
+                let view, bookmark;
                 test_utils.openChatRoom(_converse, 'theplay', 'conference.shakespeare.lit', 'JC')
                 .then(() => {
                     var jid = 'theplay@conference.shakespeare.lit';
@@ -43,7 +43,7 @@
                     spyOn(view, 'closeForm').and.callThrough();
                     return test_utils.waitUntil(() => !_.isNull(view.el.querySelector('.toggle-bookmark')));
                 }).then(() => {
-                    var bookmark = view.el.querySelector('.toggle-bookmark');
+                    bookmark = view.el.querySelector('.toggle-bookmark');
                     bookmark.click();
                     expect(view.renderBookmarkForm).toHaveBeenCalled();
 
@@ -55,46 +55,47 @@
                     expect(view.renderBookmarkForm).toHaveBeenCalled();
 
                     /* Client uploads data:
-                    * --------------------
-                    *  <iq from='juliet@capulet.lit/balcony' type='set' id='pip1'>
-                    *      <pubsub xmlns='http://jabber.org/protocol/pubsub'>
-                    *          <publish node='storage:bookmarks'>
-                    *              <item id='current'>
-                    *                  <storage xmlns='storage:bookmarks'>
-                    *                      <conference name='The Play&apos;s the Thing'
-                    *                                  autojoin='true'
-                    *                                  jid='theplay@conference.shakespeare.lit'>
-                    *                          <nick>JC</nick>
-                    *                      </conference>
-                    *                  </storage>
-                    *              </item>
-                    *          </publish>
-                    *          <publish-options>
-                    *              <x xmlns='jabber:x:data' type='submit'>
-                    *                  <field var='FORM_TYPE' type='hidden'>
-                    *                      <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>
-                    */
+                     * --------------------
+                     *  <iq from='juliet@capulet.lit/balcony' type='set' id='pip1'>
+                     *      <pubsub xmlns='http://jabber.org/protocol/pubsub'>
+                     *          <publish node='storage:bookmarks'>
+                     *              <item id='current'>
+                     *                  <storage xmlns='storage:bookmarks'>
+                     *                      <conference name='The Play&apos;s the Thing'
+                     *                                  autojoin='true'
+                     *                                  jid='theplay@conference.shakespeare.lit'>
+                     *                          <nick>JC</nick>
+                     *                      </conference>
+                     *                  </storage>
+                     *              </item>
+                     *          </publish>
+                     *          <publish-options>
+                     *              <x xmlns='jabber:x:data' type='submit'>
+                     *                  <field var='FORM_TYPE' type='hidden'>
+                     *                      <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>
+                     */
                     expect(view.model.get('bookmarked')).toBeFalsy();
-                    var $form = $(view.el).find('.chatroom-form');
-                    $form.find('input[name="name"]').val('Play&apos;s the Thing');
-                    $form.find('input[name="autojoin"]').prop('checked', true);
-                    $form.find('input[name="nick"]').val('JC');
-                    view.el.querySelector('.btn-primary').click();
+                    const form = view.el.querySelector('.chatroom-form');
+                    form.querySelector('input[name="name"]').value = 'Play&apos;s the Thing';
+                    form.querySelector('input[name="autojoin"]').checked = 'checked';
+                    form.querySelector('input[name="nick"]').value = 'JC';
 
-                    expect(view.model.get('bookmarked')).toBeTruthy();
-                    expect(u.hasClass('on-button', bookmark), true);
+                    _converse.connection.IQ_stanzas = [];
+                    view.el.querySelector('.btn-primary').click();
 
+                    return test_utils.waitUntil(() => sent_stanza);
+                }).then(() => {
                     expect(sent_stanza.toLocaleString()).toBe(
                         "<iq type='set' from='dummy@localhost/resource' xmlns='jabber:client' id='"+IQ_id+"'>"+
                             "<pubsub xmlns='http://jabber.org/protocol/pubsub'>"+
@@ -123,21 +124,24 @@
                             "</pubsub>"+
                         "</iq>"
                     );
-
                     /* Server acknowledges successful storage
-                    *
-                    * <iq to='juliet@capulet.lit/balcony' type='result' id='pip1'/>
-                    */
-                    var stanza = $iq({
+                     *
+                     * <iq to='juliet@capulet.lit/balcony' type='result' id='pip1'/>
+                     */
+                    const stanza = $iq({
                         'to':_converse.connection.jid,
                         'type':'result',
                         'id':IQ_id
                     });
                     _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                    return test_utils.waitUntil(() => view.model.get('bookmarked'));
+                }).then(() => {
+                    expect(view.model.get('bookmarked')).toBeTruthy();
+                    expect(u.hasClass('on-button', bookmark), true);
                     // We ignore this IQ stanza... (unless it's an error stanza), so
                     // nothing to test for here.
                     done();
-                });
+                }).catch(_.partial(_converse.log, _, Strophe.LogLevel.FATAL));
             });
         }));
 

src/converse-bookmarks.js

@@ -290,13 +290,12 @@
                 },
 
                 createBookmark (options) {
-                    _converse.bookmarks.sendBookmarkStanza()
-                        .then(() => _converse.bookmarks.create(options))
-                        .catch(() => this.onBookmarkError.apply(this, arguments));
+                    this.create(options);
+                    this.sendBookmarkStanza().catch(iq => this.onBookmarkError(iq, options));
                 },
 
                 sendBookmarkStanza () {
-                    let stanza = $iq({
+                    const stanza = $iq({
                             'type': 'set',
                             'from': _converse.connection.jid,
                         })
@@ -304,8 +303,8 @@
                             .c('publish', {'node': 'storage:bookmarks'})
                                 .c('item', {'id': 'current'})
                                     .c('storage', {'xmlns':'storage:bookmarks'});
-                    this.each(function (model) {
-                        stanza = stanza.c('conference', {
+                    this.each(model => {
+                        stanza.c('conference', {
                             'name': model.get('name'),
                             'autojoin': model.get('autojoin'),
                             'jid': model.get('jid'),
@@ -323,13 +322,14 @@
                     return _converse.api.sendIQ(stanza);
                 },
 
-                onBookmarkError (iq) {
+                onBookmarkError (iq, options) {
                     _converse.log("Error while trying to add bookmark", Strophe.LogLevel.ERROR);
                     _converse.log(iq);
                     _converse.api.alert.show(
                         Strophe.LogLevel.ERROR,
                         __('Error'), [__("Sorry, something went wrong while trying to save your bookmark.")]
                     )
+                    this.findWhere({'jid': options.jid}).destroy();
                 },
 
                 fetchBookmarksFromServer (deferred) {