Fixes Bug #1421568: After downloading KJV through FTW, searching in KJV causes traceback

The bibles in the ftw wizard have been upgraded to fix this issue. However, there will be already downloaded bibles out there which need upgrading.

bzr-revno: 2506

openlp/core/common/historycombobox.py

@@ -28,9 +28,9 @@ from PyQt4 import QtCore, QtGui
 
 class HistoryComboBox(QtGui.QComboBox):
     """
-    The :class:`~openlp.core.common.historycombobox.HistoryComboBox` widget emulates the QLineEdit ``returnPressed`` signal
-    for when the :kbd:`Enter` or :kbd:`Return` keys are pressed, and saves anything that is typed into the edit box into
-    its list.
+    The :class:`~openlp.core.common.historycombobox.HistoryComboBox` widget emulates the QLineEdit ``returnPressed``
+    signal for when the :kbd:`Enter` or :kbd:`Return` keys are pressed, and saves anything that is typed into the edit
+    box into its list.
     """
     returnPressed = QtCore.pyqtSignal()
 

openlp/core/lib/db.py

@@ -145,9 +145,13 @@ def upgrade_db(url, upgrade):
     version_meta = session.query(Metadata).get('version')
     if version_meta is None:
         # Tables have just been created - fill the version field with the most recent version
-        version = upgrade.__version__
+        if session.query(Metadata).get('dbversion'):
+            version = 0
+        else:
+            version = upgrade.__version__
         version_meta = Metadata.populate(key='version', value=version)
         session.add(version_meta)
+        session.commit()
     else:
         version = int(version_meta.value)
     if version > upgrade.__version__:

openlp/plugins/bibles/lib/db.py

@@ -164,9 +164,6 @@ class BibleDB(QtCore.QObject, Manager, RegistryProperties):
         Returns the version name of the Bible.
         """
         version_name = self.get_object(BibleMeta, 'name')
-        # Fallback to old way of naming
-        if not version_name:
-            version_name = self.get_object(BibleMeta, 'Version')
         self.name = version_name.value if version_name else None
         return self.name
 

openlp/plugins/bibles/lib/upgrade.py

@@ -24,14 +24,177 @@ The :mod:`upgrade` module provides a way for the database and schema that is the
 """
 import logging
 
+from sqlalchemy import delete, func, insert, select
+
 log = logging.getLogger(__name__)
 __version__ = 1
 
 
+# TODO: When removing an upgrade path the ftw-data needs updating to the minimum supported version
 def upgrade_1(session, metadata):
     """
     Version 1 upgrade.
 
     This upgrade renames a number of keys to a single naming convention.
     """
-    log.info('No upgrades to perform')
+    metadata_table = metadata.tables['metadata']
+    # Copy "Version" to "name" ("version" used by upgrade system)
+    try:
+        session.execute(insert(metadata_table).values(
+            key='name',
+            value=select(
+                [metadata_table.c.value],
+                metadata_table.c.key == 'Version'
+            ).as_scalar()
+        ))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'Version'))
+    except:
+        log.exception('Exception when upgrading Version')
+    # Copy "Copyright" to "copyright"
+    try:
+        session.execute(insert(metadata_table).values(
+            key='copyright',
+            value=select(
+                [metadata_table.c.value],
+                metadata_table.c.key == 'Copyright'
+            ).as_scalar()
+        ))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'Copyright'))
+    except:
+        log.exception('Exception when upgrading Copyright')
+    # Copy "Permissions" to "permissions"
+    try:
+        session.execute(insert(metadata_table).values(
+            key='permissions',
+            value=select(
+                [metadata_table.c.value],
+                metadata_table.c.key == 'Permissions'
+            ).as_scalar()
+        ))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'Permissions'))
+    except:
+        log.exception('Exception when upgrading Permissions')
+    # Copy "Bookname language" to "book_name_language"
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
+                metadata_table.c.key == 'Bookname language'
+            )
+        ).scalar()
+        if value_count > 0:
+            session.execute(insert(metadata_table).values(
+                key='book_name_language',
+                value=select(
+                    [metadata_table.c.value],
+                    metadata_table.c.key == 'Bookname language'
+                ).as_scalar()
+            ))
+            session.execute(delete(metadata_table).where(metadata_table.c.key == 'Bookname language'))
+    except:
+        log.exception('Exception when upgrading Bookname language')
+    # Copy "download source" to "download_source"
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
+                metadata_table.c.key == 'download source'
+            )
+        ).scalar()
+        log.debug('download source: %s', value_count)
+        if value_count > 0:
+            session.execute(insert(metadata_table).values(
+                key='download_source',
+                value=select(
+                    [metadata_table.c.value],
+                    metadata_table.c.key == 'download source'
+                ).as_scalar()
+            ))
+            session.execute(delete(metadata_table).where(metadata_table.c.key == 'download source'))
+    except:
+        log.exception('Exception when upgrading download source')
+    # Copy "download name" to "download_name"
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
+                metadata_table.c.key == 'download name'
+            )
+        ).scalar()
+        log.debug('download name: %s', value_count)
+        if value_count > 0:
+            session.execute(insert(metadata_table).values(
+                key='download_name',
+                value=select(
+                    [metadata_table.c.value],
+                    metadata_table.c.key == 'download name'
+                ).as_scalar()
+            ))
+            session.execute(delete(metadata_table).where(metadata_table.c.key == 'download name'))
+    except:
+        log.exception('Exception when upgrading download name')
+    # Copy "proxy server" to "proxy_server"
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
+                metadata_table.c.key == 'proxy server'
+            )
+        ).scalar()
+        log.debug('proxy server: %s', value_count)
+        if value_count > 0:
+            session.execute(insert(metadata_table).values(
+                key='proxy_server',
+                value=select(
+                    [metadata_table.c.value],
+                    metadata_table.c.key == 'proxy server'
+                ).as_scalar()
+            ))
+            session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy server'))
+    except:
+        log.exception('Exception when upgrading proxy server')
+    # Copy "proxy username" to "proxy_username"
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
+                metadata_table.c.key == 'proxy username'
+            )
+        ).scalar()
+        log.debug('proxy username: %s', value_count)
+        if value_count > 0:
+            session.execute(insert(metadata_table).values(
+                key='proxy_username',
+                value=select(
+                    [metadata_table.c.value],
+                    metadata_table.c.key == 'proxy username'
+                ).as_scalar()
+            ))
+            session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy username'))
+    except:
+        log.exception('Exception when upgrading proxy username')
+    # Copy "proxy password" to "proxy_password"
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
+                metadata_table.c.key == 'proxy password'
+            )
+        ).scalar()
+        log.debug('proxy password: %s', value_count)
+        if value_count > 0:
+            session.execute(insert(metadata_table).values(
+                key='proxy_password',
+                value=select(
+                    [metadata_table.c.value],
+                    metadata_table.c.key == 'proxy password'
+                ).as_scalar()
+            ))
+            session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy password'))
+    except:
+        log.exception('Exception when upgrading proxy password')
+    try:
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'dbversion'))
+    except:
+        log.exception('Exception when deleting dbversion')
+    session.commit()

openlp/plugins/songs/lib/upgrade.py

@@ -35,6 +35,7 @@ log = logging.getLogger(__name__)
 __version__ = 4
 
 
+# TODO: When removing an upgrade path the ftw-data needs updating to the minimum supported version
 def upgrade_1(session, metadata):
     """
     Version 1 upgrade.

tests/functional/openlp_core_ui/test_thememanager.py

@@ -22,13 +22,14 @@
 """
 Package to test the openlp.core.ui.thememanager package.
 """
-import zipfile
 import os
 import shutil
 
 from unittest import TestCase
 from tempfile import mkdtemp
 
+from PyQt4 import QtGui
+
 from openlp.core.ui import ThemeManager
 from openlp.core.common import Registry
 
@@ -130,6 +131,46 @@ class TestThemeManager(TestCase):
             # THEN: The mocked_copyfile should not have been called
             self.assertTrue(mocked_copyfile.called, 'shutil.copyfile should be called')
 
+    def over_write_message_box_yes_test(self):
+        """
+        Test that theme_manager.over_write_message_box returns True when the user clicks yes.
+        """
+        # GIVEN: A patched QMessageBox.question and an instance of ThemeManager
+        with patch('openlp.core.ui.thememanager.QtGui.QMessageBox.question', return_value=QtGui.QMessageBox.Yes)\
+                as mocked_qmessagebox_question,\
+                patch('openlp.core.ui.thememanager.translate') as mocked_translate:
+            mocked_translate.side_effect = lambda context, text: text
+            theme_manager = ThemeManager(None)
+
+            # WHEN: Calling over_write_message_box with 'Theme Name'
+            result = theme_manager.over_write_message_box('Theme Name')
+
+            # THEN: over_write_message_box should return True and the message box should contain the theme name
+            self.assertTrue(result)
+            mocked_qmessagebox_question.assert_called_once_with(
+                theme_manager, 'Theme Already Exists', 'Theme Theme Name already exists. Do you want to replace it?',
+                ANY, ANY)
+
+    def over_write_message_box_no_test(self):
+        """
+        Test that theme_manager.over_write_message_box returns False when the user clicks no.
+        """
+        # GIVEN: A patched QMessageBox.question and an instance of ThemeManager
+        with patch('openlp.core.ui.thememanager.QtGui.QMessageBox.question', return_value=QtGui.QMessageBox.No)\
+                as mocked_qmessagebox_question,\
+                patch('openlp.core.ui.thememanager.translate') as mocked_translate:
+            mocked_translate.side_effect = lambda context, text: text
+            theme_manager = ThemeManager(None)
+
+            # WHEN: Calling over_write_message_box with 'Theme Name'
+            result = theme_manager.over_write_message_box('Theme Name')
+
+            # THEN: over_write_message_box should return False and the message box should contain the theme name
+            self.assertFalse(result)
+            mocked_qmessagebox_question.assert_called_once_with(
+                theme_manager, 'Theme Already Exists', 'Theme Theme Name already exists. Do you want to replace it?',
+                ANY, ANY)
+
     def unzip_theme_test(self):
         """
         Test that unzipping of themes works