Refactored bookmarks to use option parser, allow bookmarking rooms with passwords

12 files changed, 233 insertions, 278 deletions

diff --git a/src/command/command.c b/src/command/command.c
index 9fc7f9e5..131f861e 100644
--- a/src/command/command.c
+++ b/src/command/command.c
@@ -300,12 +300,19 @@ static struct cmd_t command_defs[] =
           NULL } } },
 
     { "/bookmark",
-        cmd_bookmark, parse_args, 0, 4, NULL,
-        { "/bookmark [add|list|remove] [room@server] [autojoin] [nick]",
-          "Manage bookmarks.",
-        { "/bookmark [add|list|remove] [room@server] [autojoin] [nick]",
+        cmd_bookmark, parse_args, 1, 8, NULL,
+        { "/bookmark list|add|update|remove|join [room@server] [nick value] [password value] [autojoin on|off]", "Manage bookmarks.",
+        { "/bookmark list|add|update|remove|join [room@server] [nick value] [password value] [autojoin on|off]",
           "-----------------------------------------------------------",
           "Manage bookmarks.",
+          "list: List all bookmarks.",
+          "add: Add a bookmark for room@server with the following optional properties:",
+          "  nick: Nickname used in the chat room",
+          "  password: Password for private rooms, note this may be stored in plaintext on your server",
+          "  autojoin: Whether to join the room automatically on login \"on\" or \"off\".",
+          "update: Update any of the above properties associated with the bookmark.",
+          "remove: Remove the bookmark for room@server.",
+          "join: Join room@server using the properties associated with the bookmark.",
           NULL } } },
 
     { "/disco",
@@ -1057,9 +1064,11 @@ cmd_init(void)
     autocomplete_add(who_ac, "any");
 
     bookmark_ac = autocomplete_new();
-    autocomplete_add(bookmark_ac, "add");
     autocomplete_add(bookmark_ac, "list");
+    autocomplete_add(bookmark_ac, "add");
+    autocomplete_add(bookmark_ac, "update");
     autocomplete_add(bookmark_ac, "remove");
+    autocomplete_add(bookmark_ac, "join");
 
     otr_ac = autocomplete_new();
     autocomplete_add(otr_ac, "gen");
@@ -1651,11 +1660,15 @@ _bookmark_autocomplete(char *input, int *size)
         return result;
     }
 
-    result = autocomplete_param_with_func(input, size, "/bookmark list", bookmark_find);
+    result = autocomplete_param_with_func(input, size, "/bookmark remove", bookmark_find);
     if (result != NULL) {
         return result;
     }
-    result = autocomplete_param_with_func(input, size, "/bookmark remove", bookmark_find);
+    result = autocomplete_param_with_func(input, size, "/bookmark join", bookmark_find);
+    if (result != NULL) {
+        return result;
+    }
+    result = autocomplete_param_with_func(input, size, "/bookmark update", bookmark_find);
     if (result != NULL) {
         return result;
     }
diff --git a/src/command/commands.c b/src/command/commands.c
index ad508cc7..8a1b66bf 100644
--- a/src/command/commands.c
+++ b/src/command/commands.c
@@ -1734,77 +1734,76 @@ cmd_bookmark(gchar **args, struct cmd_help_t help)
         return TRUE;
     }
 
-    /* TODO: /bookmark list room@server */
-
     if (strcmp(cmd, "list") == 0) {
         const GList *bookmarks = bookmark_get_list();
         cons_show_bookmarks(bookmarks);
     } else {
-        gboolean autojoin = FALSE;
-        gchar *jid = NULL;
-        gchar *nick = NULL;
-        int idx = 1;
-
-        while (args[idx] != NULL) {
-            gchar *opt = args[idx];
-
-            if (strcmp(opt, "autojoin") == 0) {
-                autojoin = TRUE;
-            } else if (jid == NULL) {
-                jid = opt;
-            } else if (nick == NULL) {
-                nick = opt;
+        char *jid = args[1];
+        if (jid == NULL) {
+            cons_show("Usage: %s", help.usage);
+            cons_show("");
+            return TRUE;
+        }
+
+        if (strcmp(cmd, "remove") == 0) {
+            gboolean removed = bookmark_remove(jid);
+            if (removed) {
+                cons_show("Bookmark removed for %s.", jid);
             } else {
-                cons_show("Usage: %s", help.usage);
+                cons_show("No bookmark exists for %s.", jid);
             }
+            return TRUE;
+        }
 
-            ++idx;
+        if (strcmp(cmd, "join") == 0) {
+            gboolean joined = bookmark_join(jid);
+            if (!joined) {
+                cons_show("No bookmark exists for %s.", jid);
+            }
+            return TRUE;
         }
 
-        if (jid == NULL) {
-            win_type_t win_type = ui_current_win_type();
+        gchar *opt_keys[] = { "autojoin", "nick", "password", NULL };
+        gboolean parsed;
 
-            if (win_type == WIN_MUC) {
-                jid = ui_current_recipient();
-                nick = muc_get_room_nick(jid);
-            } else {
+        GHashTable *options = parse_options(&args[2], opt_keys, &parsed);
+        if (!parsed) {
+            cons_show("Usage: %s", help.usage);
+            cons_show("");
+            return TRUE;
+        }
+
+        char *nick = g_hash_table_lookup(options, "nick");
+        char *password = g_hash_table_lookup(options, "password");
+        char *autojoin = g_hash_table_lookup(options, "autojoin");
+
+        if (autojoin != NULL) {
+            if ((strcmp(autojoin, "on") != 0) && (strcmp(autojoin, "off") != 0)) {
                 cons_show("Usage: %s", help.usage);
+                cons_show("");
                 return TRUE;
             }
         }
 
         if (strcmp(cmd, "add") == 0) {
-            gboolean added = bookmark_add(jid, nick, autojoin);
+            gboolean added = bookmark_add(jid, nick, password, autojoin);
             if (added) {
-                GString *msg = g_string_new("Bookmark added for ");
-                g_string_append(msg, jid);
-                if (nick != NULL) {
-                    g_string_append(msg, ", nickname: ");
-                    g_string_append(msg, nick);
-                }
-                if (autojoin) {
-                    g_string_append(msg, ", autojoin enabled");
-                }
-                g_string_append(msg, ".");
-                cons_show(msg->str);
-                g_string_free(msg, TRUE);
+                cons_show("Bookmark added for %s.", jid);
             } else {
-                cons_show("Bookmark updated for %s.", jid);
+                cons_show("Bookmark already exists, use /bookmark update to edit.");
             }
-        } else if (strcmp(cmd, "remove") == 0) {
-            gboolean removed = bookmark_remove(jid, autojoin);
-            if (removed) {
-                if (autojoin) {
-                    cons_show("Autojoin disabled for %s.", jid);
-                } else {
-                    cons_show("Bookmark removed for %s.", jid);
-                }
+        } else if (strcmp(cmd, "update") == 0) {
+            gboolean updated = bookmark_update(jid, nick, password, autojoin);
+            if (updated) {
+                cons_show("Bookmark updated.");
             } else {
                 cons_show("No bookmark exists for %s.", jid);
             }
         } else {
             cons_show("Usage: %s", help.usage);
         }
+
+        options_destroy(options);
     }
 
     return TRUE;
diff --git a/src/ui/console.c b/src/ui/console.c
index 2aff43a5..588f6001 100644
--- a/src/ui/console.c
+++ b/src/ui/console.c
@@ -663,6 +663,9 @@ _cons_show_bookmarks(const GList *list)
             if (item->autojoin) {
                 wprintw(console->win, " (autojoin)");
             }
+            if (item->password != NULL) {
+                wprintw(console->win, " (private)");
+            }
             wprintw(console->win, "\n");
             list = g_list_next(list);
         }
diff --git a/src/xmpp/bookmark.c b/src/xmpp/bookmark.c
index c73f3db3..a252841d 100644
--- a/src/xmpp/bookmark.c
+++ b/src/xmpp/bookmark.c
@@ -35,6 +35,7 @@
 #include "xmpp/stanza.h"
 #include "xmpp/xmpp.h"
 #include "xmpp/bookmark.h"
+#include "ui/ui.h"
 
 #define BOOKMARK_TIMEOUT 5000
 /* TODO: replace with a preference */
@@ -83,69 +84,128 @@ bookmark_request(void)
 }
 
 static gboolean
-_bookmark_add(const char *jid, const char *nick, gboolean autojoin)
+_bookmark_add(const char *jid, const char *nick, const char *password, const char *autojoin_str)
 {
-    gboolean added = TRUE;
     if (autocomplete_contains(bookmark_ac, jid)) {
-        added = FALSE;
+        return FALSE;
+    } else {
+        Bookmark *item = malloc(sizeof(*item));
+        item->jid = strdup(jid);
+        if (nick != NULL) {
+            item->nick = strdup(nick);
+        } else {
+            item->nick = NULL;
+        }
+        if (password != NULL) {
+            item->password = strdup(password);
+        } else {
+            item->password = NULL;
+        }
+
+        if (g_strcmp0(autojoin_str, "on") == 0) {
+            item->autojoin = TRUE;
+        } else {
+            item->autojoin = FALSE;
+        }
+
+        bookmark_list = g_list_append(bookmark_list, item);
+        autocomplete_add(bookmark_ac, jid);
+        _send_bookmarks();
+
+        return TRUE;
     }
+}
 
-    /* this may be command for modifying */
+static gboolean
+_bookmark_update(const char *jid, const char *nick, const char *password, const char *autojoin_str)
+{
     Bookmark *item = malloc(sizeof(*item));
     item->jid = strdup(jid);
-    if (nick != NULL) {
-        item->nick = strdup(nick);
-    } else {
-        item->nick = NULL;
-    }
-    item->autojoin = autojoin;
+    item->nick = NULL;
+    item->password = NULL;
+    item->autojoin = FALSE;
 
     GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid);
-    if (found != NULL) {
-        bookmark_list = g_list_remove_link(bookmark_list, found);
-        _bookmark_item_destroy(found->data);
-        g_list_free(found);
+    _bookmark_item_destroy(item);
+    if (found == NULL) {
+        return FALSE;
+    } else {
+        Bookmark *bm = found->data;
+        if (nick != NULL) {
+            free(bm->nick);
+            bm->nick = strdup(nick);
+        }
+        if (password != NULL) {
+            free(bm->password);
+            bm->password = strdup(password);
+        }
+        if (autojoin_str != NULL) {
+            if (g_strcmp0(autojoin_str, "on") == 0) {
+                bm->autojoin = TRUE;
+            } else if (g_strcmp0(autojoin_str, "off") == 0) {
+                bm->autojoin = FALSE;
+            }
+        }
+        _send_bookmarks();
+        return TRUE;
     }
-    bookmark_list = g_list_append(bookmark_list, item);
-
-    autocomplete_remove(bookmark_ac, jid);
-    autocomplete_add(bookmark_ac, jid);
+}
 
-    _send_bookmarks();
+static gboolean
+_bookmark_join(const char *jid)
+{
+    Bookmark *item = malloc(sizeof(*item));
+    item->jid = strdup(jid);
+    item->nick = NULL;
+    item->password = NULL;
+    item->autojoin = FALSE;
 
-    return added;
+    GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid);
+    _bookmark_item_destroy(item);
+    if (found == NULL) {
+        return FALSE;
+    } else {
+        char *account_name = jabber_get_account_name();
+        ProfAccount *account = accounts_get_account(account_name);
+        Bookmark *item = found->data;
+        if (!muc_room_is_active(item->jid)) {
+            char *nick = item->nick;
+            if (nick == NULL) {
+                nick = account->muc_nick;
+            }
+            presence_join_room(item->jid, nick, item->password);
+            muc_join_room(item->jid, nick, item->password, FALSE);
+            account_free(account);
+        } else if (muc_get_roster_received(item->jid)) {
+            ui_room_join(item->jid, TRUE);
+        }
+        return TRUE;
+    }
 }
 
 static gboolean
-_bookmark_remove(const char *jid, gboolean autojoin)
+_bookmark_remove(const char *jid)
 {
     Bookmark *item = malloc(sizeof(*item));
     item->jid = strdup(jid);
     item->nick = NULL;
-    item->autojoin = autojoin;
+    item->password = NULL;
+    item->autojoin = FALSE;
 
     GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid);
     _bookmark_item_destroy(item);
     gboolean removed = found != NULL;
 
     if (removed) {
-        // set autojoin FALSE
-        if (autojoin) {
-            Bookmark *bookmark = found->data;
-            bookmark->autojoin = FALSE;
-
-        // remove bookmark
-        } else {
-            bookmark_list = g_list_remove_link(bookmark_list, found);
-            _bookmark_item_destroy(found->data);
-            g_list_free(found);
-            autocomplete_remove(bookmark_ac, jid);
-        }
-
+        bookmark_list = g_list_remove_link(bookmark_list, found);
+        _bookmark_item_destroy(found->data);
+        g_list_free(found);
+        autocomplete_remove(bookmark_ac, jid);
         _send_bookmarks();
+        return TRUE;
+    } else {
+        return FALSE;
     }
-
-    return removed;
 }
 
 static const GList *
@@ -176,9 +236,11 @@ _bookmark_handle_result(xmpp_conn_t * const conn,
     char *id = (char *)userdata;
     xmpp_stanza_t *ptr;
     xmpp_stanza_t *nick;
+    xmpp_stanza_t *password_st;
     char *name;
     char *jid;
     char *autojoin;
+    char *password;
     gboolean autojoin_val;
     Jid *my_jid;
     Bookmark *item;
@@ -231,6 +293,17 @@ _bookmark_handle_result(xmpp_conn_t * const conn,
             }
         }
 
+        password = NULL;
+        password_st = xmpp_stanza_get_child_by_name(ptr, "password");
+        if (password_st) {
+            char *tmp;
+            tmp = xmpp_stanza_get_text(password_st);
+            if (tmp) {
+                password = strdup(tmp);
+                xmpp_free(ctx, tmp);
+            }
+        }
+
         autojoin = xmpp_stanza_get_attribute(ptr, "autojoin");
         if (autojoin && (strcmp(autojoin, "1") == 0 || strcmp(autojoin, "true") == 0)) {
             autojoin_val = TRUE;
@@ -242,6 +315,7 @@ _bookmark_handle_result(xmpp_conn_t * const conn,
         item = malloc(sizeof(*item));
         item->jid = strdup(jid);
         item->nick = name;
+        item->password = password;
         item->autojoin = autojoin_val;
         bookmark_list = g_list_append(bookmark_list, item);
 
@@ -259,8 +333,8 @@ _bookmark_handle_result(xmpp_conn_t * const conn,
                 log_debug("Autojoin %s with nick=%s", jid, name);
                 room_jid = jid_create_from_bare_and_resource(jid, name);
                 if (!muc_room_is_active(room_jid->barejid)) {
-                    presence_join_room(jid, name, NULL);
-                    muc_join_room(jid, name, NULL, TRUE);
+                    presence_join_room(jid, name, password);
+                    muc_join_room(jid, name, password, TRUE);
                 }
                 jid_destroy(room_jid);
             } else {
@@ -303,6 +377,7 @@ _bookmark_item_destroy(gpointer item)
 
     free(p->jid);
     free(p->nick);
+    free(p->password);
     free(p);
 }
 
@@ -364,6 +439,18 @@ _send_bookmarks(void)
             xmpp_stanza_release(nick_st);
         }
 
+        if (bookmark->password != NULL) {
+            xmpp_stanza_t *password_st = xmpp_stanza_new(ctx);
+            xmpp_stanza_set_name(password_st, STANZA_NAME_PASSWORD);
+            xmpp_stanza_t *password_text = xmpp_stanza_new(ctx);
+            xmpp_stanza_set_text(password_text, bookmark->password);
+            xmpp_stanza_add_child(password_st, password_text);
+            xmpp_stanza_add_child(conference, password_st);
+
+            xmpp_stanza_release(password_text);
+            xmpp_stanza_release(password_st);
+        }
+
         xmpp_stanza_add_child(storage, conference);
         xmpp_stanza_release(conference);
 
@@ -383,7 +470,9 @@ void
 bookmark_init_module(void)
 {
     bookmark_add = _bookmark_add;
+    bookmark_update = _bookmark_update;
     bookmark_remove = _bookmark_remove;
+    bookmark_join = _bookmark_join;
     bookmark_get_list = _bookmark_get_list;
     bookmark_find = _bookmark_find;
     bookmark_autocomplete_reset = _bookmark_autocomplete_reset;
diff --git a/src/xmpp/bookmark.h b/src/xmpp/bookmark.h
index f57e1527..3dda8611 100644
--- a/src/xmpp/bookmark.h
+++ b/src/xmpp/bookmark.h
@@ -28,6 +28,7 @@
 struct bookmark_t {
     char *jid;
     char *nick;
+    char *password;
     gboolean autojoin;
 };
 
diff --git a/src/xmpp/stanza.h b/src/xmpp/stanza.h
index cddc37ea..9b8707f5 100644
--- a/src/xmpp/stanza.h
+++ b/src/xmpp/stanza.h
@@ -59,6 +59,7 @@
 #define STANZA_NAME_FIELD "field"
 #define STANZA_NAME_STORAGE "storage"
 #define STANZA_NAME_NICK "nick"
+#define STANZA_NAME_PASSWORD "password"
 #define STANZA_NAME_CONFERENCE "conference"
 #define STANZA_NAME_VALUE "value"
 
diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h
index 2f2481fb..8b23d6f2 100644
--- a/src/xmpp/xmpp.h
+++ b/src/xmpp/xmpp.h
@@ -132,8 +132,10 @@ void (*iq_set_autoping)(int seconds);
 Capabilities* (*caps_get)(const char * const caps_str);
 void (*caps_close)(void);
 
-gboolean (*bookmark_add)(const char *jid, const char *nick, gboolean autojoin);
-gboolean (*bookmark_remove)(const char *jid, gboolean autojoin);
+gboolean (*bookmark_add)(const char *jid, const char *nick, const char *password, const char *autojoin_str);
+gboolean (*bookmark_update)(const char *jid, const char *nick, const char *password, const char *autojoin_str);
+gboolean (*bookmark_remove)(const char *jid);
+gboolean (*bookmark_join)(const char *jid);
 const GList * (*bookmark_get_list)(void);
 char * (*bookmark_find)(char *search_str);
 void (*bookmark_autocomplete_reset)(void);
diff --git a/tests/test_cmd_bookmark.c b/tests/test_cmd_bookmark.c
index b6bcd484..3a78ba5b 100644
--- a/tests/test_cmd_bookmark.c
+++ b/tests/test_cmd_bookmark.c
@@ -125,40 +125,6 @@ void cmd_bookmark_list_shows_bookmarks(void **state)
     g_list_free_full(bookmarks, (GDestroyNotify)_free_bookmark);
 }
 
-void cmd_bookmark_add_shows_usage_when_no_args_not_muc(void **state)
-{
-    mock_cons_show();
-    CommandHelp *help = malloc(sizeof(CommandHelp));
-    help->usage = "some usage";
-    gchar *args[] = { "add", NULL };
-
-    mock_connection_status(JABBER_CONNECTED);
-    mock_current_win_type(WIN_CONSOLE);
-    expect_cons_show("Usage: some usage");
-
-    gboolean result = cmd_bookmark(args, *help);
-    assert_true(result);
-
-    free(help);
-}
-
-void cmd_bookmark_remove_shows_message_when_no_args_not_muc(void **state)
-{
-    mock_cons_show();
-    CommandHelp *help = malloc(sizeof(CommandHelp));
-    help->usage = "some usage";
-    gchar *args[] = { "remove", NULL };
-
-    mock_connection_status(JABBER_CONNECTED);
-    mock_current_win_type(WIN_CONSOLE);
-    expect_cons_show("Usage: some usage");
-
-    gboolean result = cmd_bookmark(args, *help);
-    assert_true(result);
-
-    free(help);
-}
-
 void cmd_bookmark_add_adds_bookmark_with_jid(void **state)
 {
     mock_bookmark_add();
@@ -169,7 +135,7 @@ void cmd_bookmark_add_adds_bookmark_with_jid(void **state)
 
     mock_connection_status(JABBER_CONNECTED);
 
-    expect_and_return_bookmark_add(jid, NULL, FALSE, TRUE);
+    expect_and_return_bookmark_add(jid, NULL, NULL, NULL, TRUE);
     expect_cons_show("Bookmark added for room@conf.server.");
 
     gboolean result = cmd_bookmark(args, *help);
@@ -185,12 +151,12 @@ void cmd_bookmark_add_adds_bookmark_with_jid_nick(void **state)
     char *jid = "room@conf.server";
     char *nick = "bob";
     CommandHelp *help = malloc(sizeof(CommandHelp));
-    gchar *args[] = { "add", jid, nick, NULL };
+    gchar *args[] = { "add", jid, "nick", nick, NULL };
 
     mock_connection_status(JABBER_CONNECTED);
 
-    expect_and_return_bookmark_add(jid, nick, FALSE, TRUE);
-    expect_cons_show("Bookmark added for room@conf.server, nickname: bob.");
+    expect_and_return_bookmark_add(jid, nick, NULL, NULL, TRUE);
+    expect_cons_show("Bookmark added for room@conf.server.");
 
     gboolean result = cmd_bookmark(args, *help);
     assert_true(result);
@@ -204,12 +170,12 @@ void cmd_bookmark_add_adds_bookmark_with_jid_autojoin(void **state)
     mock_cons_show();
     char *jid = "room@conf.server";
     CommandHelp *help = malloc(sizeof(CommandHelp));
-    gchar *args[] = { "add", jid, "autojoin", NULL };
+    gchar *args[] = { "add", jid, "autojoin", "on", NULL };
 
     mock_connection_status(JABBER_CONNECTED);
 
-    expect_and_return_bookmark_add(jid, NULL, TRUE, TRUE);
-    expect_cons_show("Bookmark added for room@conf.server, autojoin enabled.");
+    expect_and_return_bookmark_add(jid, NULL, NULL, "on", TRUE);
+    expect_cons_show("Bookmark added for room@conf.server.");
 
     gboolean result = cmd_bookmark(args, *help);
     assert_true(result);
@@ -224,71 +190,17 @@ void cmd_bookmark_add_adds_bookmark_with_jid_nick_autojoin(void **state)
     char *jid = "room@conf.server";
     char *nick = "bob";
     CommandHelp *help = malloc(sizeof(CommandHelp));
-    gchar *args[] = { "add", jid, nick, "autojoin", NULL };
-
-    mock_connection_status(JABBER_CONNECTED);
-
-    expect_and_return_bookmark_add(jid, nick, TRUE, TRUE);
-    expect_cons_show("Bookmark added for room@conf.server, nickname: bob, autojoin enabled.");
-
-    gboolean result = cmd_bookmark(args, *help);
-    assert_true(result);
-
-    free(help);
-}
-
-void cmd_bookmark_add_adds_bookmark_with_room_details(void **state)
-{
-    mock_bookmark_add();
-    mock_ui_current_recipient();
-    mock_cons_show();
-    char *jid = "room@conf.server";
-    char *nick = "bob";
-    CommandHelp *help = malloc(sizeof(CommandHelp));
-    gchar *args[] = { "add", NULL };
-
-    muc_init();
-    muc_join_room(jid, nick, NULL, TRUE);
+    gchar *args[] = { "add", jid, "nick", nick, "autojoin", "on", NULL };
 
     mock_connection_status(JABBER_CONNECTED);
-    mock_current_win_type(WIN_MUC);
-    ui_current_recipient_returns(jid);
-
-    expect_and_return_bookmark_add(jid, nick, FALSE, TRUE);
-    expect_cons_show("Bookmark added for room@conf.server, nickname: bob.");
-
-    gboolean result = cmd_bookmark(args, *help);
-    assert_true(result);
-
-    free(help);
-    muc_close();
-}
 
-void cmd_bookmark_add_adds_bookmark_with_room_details_autojoin(void **state)
-{
-    mock_bookmark_add();
-    mock_ui_current_recipient();
-    mock_cons_show();
-    char *jid = "room@conf.server";
-    char *nick = "bob";
-    CommandHelp *help = malloc(sizeof(CommandHelp));
-    gchar *args[] = { "add", "autojoin", NULL };
-
-    muc_init();
-    muc_join_room(jid, nick, NULL, TRUE);
-
-    mock_connection_status(JABBER_CONNECTED);
-    mock_current_win_type(WIN_MUC);
-    ui_current_recipient_returns(jid);
-
-    expect_and_return_bookmark_add(jid, nick, TRUE, TRUE);
-    expect_cons_show("Bookmark added for room@conf.server, nickname: bob, autojoin enabled.");
+    expect_and_return_bookmark_add(jid, nick, NULL, "on", TRUE);
+    expect_cons_show("Bookmark added for room@conf.server.");
 
     gboolean result = cmd_bookmark(args, *help);
     assert_true(result);
 
     free(help);
-    muc_close();
 }
 
 void cmd_bookmark_remove_removes_bookmark(void **state)
@@ -301,7 +213,7 @@ void cmd_bookmark_remove_removes_bookmark(void **state)
 
     mock_connection_status(JABBER_CONNECTED);
 
-    expect_and_return_bookmark_remove(jid, FALSE, TRUE);
+    expect_and_return_bookmark_remove(jid, TRUE);
     expect_cons_show("Bookmark removed for room@conf.server.");
 
     gboolean result = cmd_bookmark(args, *help);
@@ -310,44 +222,6 @@ void cmd_bookmark_remove_removes_bookmark(void **state)
     free(help);
 }
 
-void cmd_bookmark_remove_removes_autojoin(void **state)
-{
-    mock_bookmark_remove();
-    mock_cons_show();
-    char *jid = "room@conf.server";
-    CommandHelp *help = malloc(sizeof(CommandHelp));
-    gchar *args[] = { "remove", jid, "autojoin", NULL };
-
-    mock_connection_status(JABBER_CONNECTED);
-
-    expect_and_return_bookmark_remove(jid, TRUE, TRUE);
-    expect_cons_show("Autojoin disabled for room@conf.server.");
-
-    gboolean result = cmd_bookmark(args, *help);
-    assert_true(result);
-
-    free(help);
-}
-
-void cmd_bookmark_add_shows_message_when_upated(void **state)
-{
-    mock_bookmark_add();
-    mock_cons_show();
-    char *jid = "room@conf.server";
-    CommandHelp *help = malloc(sizeof(CommandHelp));
-    gchar *args[] = { "add", jid, NULL };
-
-    mock_connection_status(JABBER_CONNECTED);
-
-    expect_and_return_bookmark_add(jid, NULL, FALSE, FALSE);
-    expect_cons_show("Bookmark updated for room@conf.server.");
-
-    gboolean result = cmd_bookmark(args, *help);
-    assert_true(result);
-
-    free(help);
-}
-
 void cmd_bookmark_remove_shows_message_when_no_bookmark(void **state)
 {
     mock_bookmark_remove();
@@ -358,30 +232,11 @@ void cmd_bookmark_remove_shows_message_when_no_bookmark(void **state)
 
     mock_connection_status(JABBER_CONNECTED);
 
-    expect_and_return_bookmark_remove(jid, FALSE, FALSE);
-    expect_cons_show("No bookmark exists for room@conf.server.");
-
-    gboolean result = cmd_bookmark(args, *help);
-    assert_true(result);
-
-    free(help);
-}
-
-void cmd_bookmark_remove_autojoin_shows_message_when_no_bookmark(void **state)
-{
-    mock_bookmark_remove();
-    mock_cons_show();
-    char *jid = "room@conf.server";
-    CommandHelp *help = malloc(sizeof(CommandHelp));
-    gchar *args[] = { "remove", jid, "autojoin", NULL };
-
-    mock_connection_status(JABBER_CONNECTED);
-
-    expect_and_return_bookmark_remove(jid, TRUE, FALSE);
+    expect_and_return_bookmark_remove(jid, FALSE);
     expect_cons_show("No bookmark exists for room@conf.server.");
 
     gboolean result = cmd_bookmark(args, *help);
     assert_true(result);
 
     free(help);
-}
+}
\ No newline at end of file
diff --git a/tests/test_cmd_bookmark.h b/tests/test_cmd_bookmark.h
index ef8729ca..f5999e41 100644
--- a/tests/test_cmd_bookmark.h
+++ b/tests/test_cmd_bookmark.h
@@ -5,16 +5,9 @@ void cmd_bookmark_shows_message_when_started(void **state);
 void cmd_bookmark_shows_message_when_undefined(void **state);
 void cmd_bookmark_shows_usage_when_no_args(void **state);
 void cmd_bookmark_list_shows_bookmarks(void **state);
-void cmd_bookmark_add_shows_usage_when_no_args_not_muc(void **state);
-void cmd_bookmark_remove_shows_message_when_no_args_not_muc(void **state);
 void cmd_bookmark_add_adds_bookmark_with_jid(void **state);
 void cmd_bookmark_add_adds_bookmark_with_jid_nick(void **state);
 void cmd_bookmark_add_adds_bookmark_with_jid_autojoin(void **state);
 void cmd_bookmark_add_adds_bookmark_with_jid_nick_autojoin(void **state);
-void cmd_bookmark_add_adds_bookmark_with_room_details(void **state);
-void cmd_bookmark_add_adds_bookmark_with_room_details_autojoin(void **state);
 void cmd_bookmark_remove_removes_bookmark(void **state);
-void cmd_bookmark_remove_removes_autojoin(void **state);
-void cmd_bookmark_add_shows_message_when_upated(void **state);
 void cmd_bookmark_remove_shows_message_when_no_bookmark(void **state);
-void cmd_bookmark_remove_autojoin_shows_message_when_no_bookmark(void **state);
diff --git a/tests/testsuite.c b/tests/testsuite.c
index f7d3619a..89ac5dcf 100644
--- a/tests/testsuite.c
+++ b/tests/testsuite.c
@@ -432,19 +432,12 @@ int main(int argc, char* argv[]) {
         unit_test(cmd_bookmark_shows_message_when_undefined),
         unit_test(cmd_bookmark_shows_usage_when_no_args),
         unit_test(cmd_bookmark_list_shows_bookmarks),
-        unit_test(cmd_bookmark_add_shows_usage_when_no_args_not_muc),
-        unit_test(cmd_bookmark_remove_shows_message_when_no_args_not_muc),
         unit_test(cmd_bookmark_add_adds_bookmark_with_jid),
         unit_test(cmd_bookmark_add_adds_bookmark_with_jid_nick),
         unit_test(cmd_bookmark_add_adds_bookmark_with_jid_autojoin),
         unit_test(cmd_bookmark_add_adds_bookmark_with_jid_nick_autojoin),
-        unit_test(cmd_bookmark_add_adds_bookmark_with_room_details),
-        unit_test(cmd_bookmark_add_adds_bookmark_with_room_details_autojoin),
         unit_test(cmd_bookmark_remove_removes_bookmark),
-        unit_test(cmd_bookmark_remove_removes_autojoin),
-        unit_test(cmd_bookmark_add_shows_message_when_upated),
         unit_test(cmd_bookmark_remove_shows_message_when_no_bookmark),
-        unit_test(cmd_bookmark_remove_autojoin_shows_message_when_no_bookmark),
 
 #ifdef HAVE_LIBOTR
         unit_test(cmd_otr_shows_usage_when_no_args),
diff --git a/tests/xmpp/mock_xmpp.c b/tests/xmpp/mock_xmpp.c
index c2ded651..7faf7170 100644
--- a/tests/xmpp/mock_xmpp.c
+++ b/tests/xmpp/mock_xmpp.c
@@ -65,19 +65,19 @@ _mock_bookmark_get_list(void)
 }
 
 static gboolean
-_mock_bookmark_add(const char *jid, const char *nick, gboolean autojoin)
+_mock_bookmark_add(const char *jid, const char *nick, const char *password, const char *autojoin_str)
 {
     check_expected(jid);
     check_expected(nick);
-    check_expected(autojoin);
+    check_expected(password);
+    check_expected(autojoin_str);
     return (gboolean)mock();
 }
 
 static gboolean
-_mock_bookmark_remove(const char *jid, gboolean autojoin)
+_mock_bookmark_remove(const char *jid)
 {
     check_expected(jid);
-    check_expected(autojoin);
     return (gboolean)mock();
 }
 
@@ -258,7 +258,7 @@ presence_update_expect(resource_presence_t presence, char *msg, int idle)
 
 void
 expect_and_return_bookmark_add(char *expected_jid, char *expected_nick,
-    gboolean expected_autojoin, gboolean added)
+    const char *expected_password, const char *expected_autojoin_str, gboolean added)
 {
     expect_string(_mock_bookmark_add, jid, expected_jid);
     if (expected_nick != NULL) {
@@ -266,17 +266,24 @@ expect_and_return_bookmark_add(char *expected_jid, char *expected_nick,
     } else {
         expect_value(_mock_bookmark_add, nick, NULL);
     }
-    expect_value(_mock_bookmark_add, autojoin, expected_autojoin);
+    if (expected_password != NULL) {
+        expect_string(_mock_bookmark_add, password, expected_password);
+    } else {
+        expect_value(_mock_bookmark_add, password, NULL);
+    }
+    if (expected_autojoin_str != NULL) {
+        expect_string(_mock_bookmark_add, autojoin_str, expected_autojoin_str);
+    } else {
+        expect_value(_mock_bookmark_add, autojoin_str, NULL);
+    }
 
     will_return(_mock_bookmark_add, added);
 }
 
 void
-expect_and_return_bookmark_remove(char *expected_jid, gboolean expected_autojoin,
-    gboolean removed)
+expect_and_return_bookmark_remove(char *expected_jid, gboolean removed)
 {
     expect_string(_mock_bookmark_remove, jid, expected_jid);
-    expect_value(_mock_bookmark_remove, autojoin, expected_autojoin);
 
     will_return(_mock_bookmark_remove, removed);
 }
diff --git a/tests/xmpp/mock_xmpp.h b/tests/xmpp/mock_xmpp.h
index a5f93fd2..ed742971 100644
--- a/tests/xmpp/mock_xmpp.h
+++ b/tests/xmpp/mock_xmpp.h
@@ -25,11 +25,10 @@ void bookmark_get_list_returns(GList *bookmarks);
 
 void mock_bookmark_add(void);
 void expect_and_return_bookmark_add(char *expected_jid, char *expected_nick,
-    gboolean expected_autojoin, gboolean added);
+    char *expected_password, char *expected_autojoin_str, gboolean added);
 
 void mock_bookmark_remove(void);
-void expect_and_return_bookmark_remove(char *expected_jid, gboolean expected_autojoin,
-    gboolean removed);
+void expect_and_return_bookmark_remove(char *expected_jid, gboolean removed);
 
 void message_send_expect(char *message, char *recipient);