Light refactring of bookmark code

- Convert xs:boolean values in bookmark module
- Use python booleans for minimize and autojoin in other parts of Gajim
- Use add_bookmark() instead of manipulating the bookmarks dict

gajim/common/helpers.py

@@ -499,16 +499,6 @@ def get_file_path_from_dnd_dropped_uri(uri):
         path = path[5:] # 5 is len('file:')
     return path
 
-def from_xs_boolean_to_python_boolean(value):
-    # this is xs:boolean so 'true', 'false', '1', '0'
-    # convert those to True/False (python booleans)
-    if value in ('1', 'true'):
-        val = True
-    else: # '0', 'false' or anything else
-        val = False
-
-    return val
-
 def get_xmpp_show(show):
     if show in ('online', 'offline'):
         return None

gajim/common/modules/bookmarks.py

@@ -24,6 +24,8 @@ from gajim.common import app
 from gajim.common import helpers
 from gajim.common.const import BookmarkStorageType
 from gajim.common.nec import NetworkIncomingEvent
+from gajim.common.modules.util import from_xs_boolean
+from gajim.common.modules.util import to_xs_boolean
 
 log = logging.getLogger('gajim.c.m.bookmarks')
 
@@ -193,8 +195,8 @@ class Bookmarks:
             log.debug('Found Bookmark: %s', jid)
             self.bookmarks[jid] = {
                 'name': conf.getAttr('name'),
-                'autojoin': autojoin_val,
+                'autojoin': from_xs_boolean(autojoin_val),
-                'minimize': minimize_val,
+                'minimize': from_xs_boolean(minimize_val),
                 'password': conf.getTagData('password'),
                 'nick': conf.getTagData('nick'),
                 'print_status': print_status}
@@ -208,10 +210,10 @@ class Bookmarks:
         for jid, bm in self.bookmarks.items():
             conf_node = storage_node.addChild(name="conference")
             conf_node.setAttr('jid', jid)
-            conf_node.setAttr('autojoin', bm['autojoin'])
+            conf_node.setAttr('autojoin', to_xs_boolean(bm['autojoin']))
             conf_node.setAttr('name', bm['name'])
-            conf_node.setTag(
+            conf_node.setTag('minimize', namespace=NS_GAJIM_BM).setData(
-                'minimize', namespace=NS_GAJIM_BM).setData(bm['minimize'])
+                to_xs_boolean(bm['minimize']))
             # Only add optional elements if not empty
             # Note: need to handle both None and '' as empty
             #   thus shouldn't use "is not None"
@@ -279,15 +281,14 @@ class Bookmarks:
         if app.is_invisible(self._account):
             return
         for jid, bm in self.bookmarks.items():
-            if bm['autojoin'] in ('1', 'true'):
+            if bm['autojoin']:
                 # Only join non-opened groupchats. Opened one are already
                 # auto-joined on re-connection
                 if jid not in app.gc_connected[self._account]:
                     # we are not already connected
-                    minimize = bm['minimize'] in ('1', 'true')
                     app.interface.join_gc_room(
                         self._account, jid, bm['nick'],
-                        bm['password'], minimize=minimize)
+                        bm['password'], minimize=bm['minimize'])
 
     def add_bookmark(self, name, jid, autojoin,
                      minimize, password, nick):

gajim/common/modules/util.py

@@ -39,3 +39,36 @@ def is_muc_pm(message, jid, groupchat=False):
         if app.logger.jid_is_room_jid(jid.getStripped()):
             return True
         return False
+
+
+def from_xs_boolean(value):
+    # Convert a xs:boolean ('true', 'false', '1', '0', '')
+    # to a python boolean (True, False)
+    if value in (True, False):
+        return value
+
+    if value in ('1', 'true'):
+        return True
+
+    # '0', 'false' or empty
+    if value in ('0', 'false', ''):
+        return False
+
+    raise ValueError(
+        'Cant convert %s to python boolean' % value)
+
+
+def to_xs_boolean(value):
+    # Convert to xs:boolean ('true', 'false')
+    # from a python boolean (True, False) or None
+    if value in ('true', 'false'):
+        return value
+
+    if value is True:
+        return 'true'
+
+    if value in (False, None):
+        return 'false'
+
+    raise ValueError(
+        'Cant convert %s to xs:boolean' % value)

gajim/disco.py

@@ -1708,19 +1708,11 @@ class MucBrowser(AgentBrowser):
 
     def on_bookmark_button_clicked(self, *args):
         con = app.connections[self.account]
-        model, iter = self.window.services_treeview.get_selection().get_selected()
+        model, iter_ = self.window.services_treeview.get_selection().get_selected()
-        if not iter:
+        if not iter_:
             return
-        name = app.config.get_per('accounts', self.account, 'name')
-        room_jid = model[iter][0]
-        bm = {
-            'name': room_jid.split('@')[0],
-            'autojoin': '0',
-            'minimize': '0',
-            'password': '',
-            'nick': name
-        }
 
+        room_jid = model[iter_][0]
         if room_jid in con.get_module('Bookmarks').bookmarks:
             ErrorDialog(
                 _('Bookmark already set'),
@@ -1728,10 +1720,12 @@ class MucBrowser(AgentBrowser):
                 transient_for=self.window.window)
             return
 
-        con.get_module('Bookmarks').bookmarks[room_jid] = bm
+        con.get_module('Bookmarks').add_bookmark(room_jid.split('@')[0],
-        con.get_module('Bookmarks').store_bookmarks()
+                                                 room_jid,
-
+                                                 False,
-        gui_menu_builder.build_bookmark_menu(self.account)
+                                                 False,
+                                                 ''
+                                                 '')
 
         InformationDialog(
             _('Bookmark has been added successfully'),

gajim/groupchat_control.py

@@ -715,9 +715,12 @@ class GroupchatControl(ChatControlBase):
         """
         password = app.gc_passwords.get(self.room_jid, '')
         con = app.connections[self.account]
-        con.get_module('Bookmarks').add_bookmark(
+        con.get_module('Bookmarks').add_bookmark(self.name,
-            self.name, self.room_jid,
+                                                 self.room_jid,
-            '1', '1', password, self.nick)
+                                                 True,
+                                                 True,
+                                                 password,
+                                                 self.nick)
         self.update_actions()
 
     def _on_request_voice(self, action, param):

gajim/gtk/bookmarks.py

@@ -48,22 +48,14 @@ class ManageBookmarksWindow:
             bookmarks = con.get_module('Bookmarks').get_sorted_bookmarks()
 
             for jid, bookmark in bookmarks.items():
-
-                # make '1', '0', 'true', 'false' (or other) to True/False
-                autojoin = helpers.from_xs_boolean_to_python_boolean(
-                    bookmark['autojoin'])
-
-                minimize = helpers.from_xs_boolean_to_python_boolean(
-                    bookmark['minimize'])
-
                 print_status = bookmark.get('print_status', '')
                 if print_status not in ('', 'all', 'in_and_out', 'none'):
                     print_status = ''
                 self.treestore.append(iter_, [account,
                                               bookmark['name'],
                                               jid,
-                                              autojoin,
+                                              bookmark['autojoin'],
-                                              minimize,
+                                              bookmark['minimize'],
                                               bookmark['password'],
                                               bookmark['nick'],
                                               print_status,
@@ -211,23 +203,16 @@ class ManageBookmarksWindow:
             con.get_module('Bookmarks').bookmarks = {}
 
             for bm in account.iterchildren():
-                # Convert True/False/None to '1' or '0'
-                autojoin = str(int(bm[3]))
-                minimize = str(int(bm[4]))
-                name = bm[1]
-                jid = bm[2]
-                pw = bm[5]
-                nick = bm[6]
-
                 # create the bookmark-dict
                 bmdict = {
-                    'name': name,
+                    'name': bm[1],
-                    'autojoin': autojoin,
+                    'autojoin': bm[3],
-                    'minimize': minimize,
+                    'minimize': bm[4],
-                    'password': pw,
+                    'password': bm[5],
-                    'nick': nick,
+                    'nick': bm[6],
                     'print_status': bm[7]}
 
+                jid = bm[2]
                 con.get_module('Bookmarks').bookmarks[jid] = bmdict
 
             con.get_module('Bookmarks').store_bookmarks()

gajim/gtk/join_groupchat.py

@@ -261,12 +261,16 @@ class JoinGroupchatWindow(Gtk.ApplicationWindow):
         if not add_bookmark:
             return
 
-        autojoin = int(self.autojoin_switch.get_active())
+        autojoin = self.autojoin_switch.get_active()
 
         # Add as bookmark, with autojoin and not minimized
         name = app.get_nick_from_jid(self.room_jid)
-        con.get_module('Bookmarks').add_bookmark(
+        con.get_module('Bookmarks').add_bookmark(name,
-            name, self.room_jid, autojoin, 1, password, nickname)
+                                                 self.room_jid,
+                                                 autojoin,
+                                                 True,
+                                                 password,
+                                                 nickname)
 
     def _on_search_clicked(self, widget):
         server = self.server_combo.get_active_text().strip()