From 14c3d27fedb884e73f029665ae10afa3cb4d0a4b Mon Sep 17 00:00:00 2001
From: Phill Ridout <phill.ridout@gmail.com>
Date: Wed, 18 Feb 2015 20:43:51 +0000
Subject: [PATCH 1/8] revert db upgrade code

---
 openlp/plugins/bibles/lib/upgrade.py | 126 ++++++++++++++++++++++++++-
 1 file changed, 125 insertions(+), 1 deletion(-)

diff --git a/openlp/plugins/bibles/lib/upgrade.py b/openlp/plugins/bibles/lib/upgrade.py
index bd7de8368..53013edbc 100644
--- a/openlp/plugins/bibles/lib/upgrade.py
+++ b/openlp/plugins/bibles/lib/upgrade.py
@@ -24,6 +24,8 @@ 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
 
@@ -34,4 +36,126 @@ def upgrade_1(session, metadata):
 
     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)
+    session.execute(insert(metadata_table).values(
+        key='name',
+        value=select(
+            [metadata_table.c.value],
+            metadata_table.c.key == 'Version'
+        ).as_scalar()
+    ))
+    # Copy "Copyright" to "copyright"
+    session.execute(insert(metadata_table).values(
+        key='copyright',
+        value=select(
+            [metadata_table.c.value],
+            metadata_table.c.key == 'Copyright'
+        ).as_scalar()
+    ))
+    # Copy "Permissions" to "permissions"
+    session.execute(insert(metadata_table).values(
+        key='permissions',
+        value=select(
+            [metadata_table.c.value],
+            metadata_table.c.key == 'Permissions'
+        ).as_scalar()
+    ))
+    # Copy "Bookname language" to "book_name_language"
+    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()
+        ))
+    # Copy "download source" to "download_source"
+    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()
+        ))
+    # Copy "download name" to "download_name"
+    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()
+        ))
+    # Copy "proxy server" to "proxy_server"
+    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()
+        ))
+    # Copy "proxy username" to "proxy_username"
+    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()
+        ))
+    # Copy "proxy password" to "proxy_password"
+    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 == 'dbversion'))
+    session.commit()

From 5e8ad1710b38874d3279a8f685385e8a80a7862f Mon Sep 17 00:00:00 2001
From: Phill Ridout <phill.ridout@gmail.com>
Date: Wed, 18 Feb 2015 20:55:30 +0000
Subject: [PATCH 2/8] remove old redundant keys

---
 openlp/plugins/bibles/lib/upgrade.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/openlp/plugins/bibles/lib/upgrade.py b/openlp/plugins/bibles/lib/upgrade.py
index 53013edbc..378247c7b 100644
--- a/openlp/plugins/bibles/lib/upgrade.py
+++ b/openlp/plugins/bibles/lib/upgrade.py
@@ -45,6 +45,7 @@ def upgrade_1(session, metadata):
             metadata_table.c.key == 'Version'
         ).as_scalar()
     ))
+    session.execute(delete(metadata_table).where(metadata_table.c.key == 'Version'))
     # Copy "Copyright" to "copyright"
     session.execute(insert(metadata_table).values(
         key='copyright',
@@ -53,6 +54,7 @@ def upgrade_1(session, metadata):
             metadata_table.c.key == 'Copyright'
         ).as_scalar()
     ))
+    session.execute(delete(metadata_table).where(metadata_table.c.key == 'Copyright'))
     # Copy "Permissions" to "permissions"
     session.execute(insert(metadata_table).values(
         key='permissions',
@@ -61,6 +63,7 @@ def upgrade_1(session, metadata):
             metadata_table.c.key == 'Permissions'
         ).as_scalar()
     ))
+    session.execute(delete(metadata_table).where(metadata_table.c.key == 'Permissions'))
     # Copy "Bookname language" to "book_name_language"
     value_count = session.execute(
         select(
@@ -76,6 +79,7 @@ def upgrade_1(session, metadata):
                 metadata_table.c.key == 'Bookname language'
             ).as_scalar()
         ))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'Bookname language'))
     # Copy "download source" to "download_source"
     value_count = session.execute(
         select(
@@ -92,6 +96,7 @@ def upgrade_1(session, metadata):
                 metadata_table.c.key == 'download source'
             ).as_scalar()
         ))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'download source'))
     # Copy "download name" to "download_name"
     value_count = session.execute(
         select(
@@ -108,6 +113,7 @@ def upgrade_1(session, metadata):
                 metadata_table.c.key == 'download name'
             ).as_scalar()
         ))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'download name'))
     # Copy "proxy server" to "proxy_server"
     value_count = session.execute(
         select(
@@ -124,6 +130,7 @@ def upgrade_1(session, metadata):
                 metadata_table.c.key == 'proxy server'
             ).as_scalar()
         ))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy server'))
     # Copy "proxy username" to "proxy_username"
     value_count = session.execute(
         select(
@@ -140,6 +147,7 @@ def upgrade_1(session, metadata):
                 metadata_table.c.key == 'proxy username'
             ).as_scalar()
         ))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy username'))
     # Copy "proxy password" to "proxy_password"
     value_count = session.execute(
         select(
@@ -156,6 +164,6 @@ def upgrade_1(session, metadata):
                 metadata_table.c.key == 'proxy password'
             ).as_scalar()
         ))
-    session.execute(delete(metadata_table)\
-        .where(metadata_table.c.key == 'dbversion'))
+        session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy password'))
+    session.execute(delete(metadata_table).where(metadata_table.c.key == 'dbversion'))
     session.commit()

From a6dfbdcf7182703a3960e5e12ea8dc52aa5c5106 Mon Sep 17 00:00:00 2001
From: Phill Ridout <phill.ridout@gmail.com>
Date: Wed, 18 Feb 2015 21:13:12 +0000
Subject: [PATCH 3/8] fix upgrade_db

---
 openlp/core/lib/db.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py
index e1c9c9117..7b8b11296 100644
--- a/openlp/core/lib/db.py
+++ b/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__:

From 4f94b2acd20d936024265db89a2d12e1323a7572 Mon Sep 17 00:00:00 2001
From: Phill Ridout <phill.ridout@gmail.com>
Date: Wed, 18 Feb 2015 21:20:35 +0000
Subject: [PATCH 4/8] remove fallback

---
 openlp/plugins/bibles/lib/db.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py
index 5c8443f23..8f1b0d752 100644
--- a/openlp/plugins/bibles/lib/db.py
+++ b/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
 

From 872d5b9d26dbf753e4ef4a41c01e318d274ba552 Mon Sep 17 00:00:00 2001
From: Ken Roberts <alisonken1@gmail.com>
Date: Wed, 18 Feb 2015 14:04:29 -0800
Subject: [PATCH 5/8] Fix duplicate test names

---
 .../functional/openlp_plugins/songs/test_songselect.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py
index 700933465..92e652149 100644
--- a/tests/functional/openlp_plugins/songs/test_songselect.py
+++ b/tests/functional/openlp_plugins/songs/test_songselect.py
@@ -491,7 +491,6 @@ class TestSongSelectFileImport(TestCase, TestMixin):
     def songselect_import_usr_file_test(self):
         """
         Verify import SongSelect USR file parses file properly
-        :return:
         """
         # GIVEN: Text file to import and mocks
         copyright = '2011 OpenLP Programmer One (Admin. by OpenLP One) | ' \
@@ -510,14 +509,13 @@ class TestSongSelectFileImport(TestCase, TestMixin):
             self.assertEquals(song_import.title, self.title, 'Song title should match')
             self.assertEquals(song_import.ccli_number, self.ccli_number, 'CCLI Song Number should match')
             self.assertEquals(song_import.authors, self.authors, 'Author(s) should match')
-            self.assertEquals(song_import.copyright, self.copyright_usr, 'Copyright should match')
+            self.assertEquals(song_import.copyright, copyright, 'Copyright should match')
             self.assertEquals(song_import.topics, self.topics, 'Theme(s) should match')
-            self.assertEquals(song_import.verses, self.verses, 'Verses should match with test verses')
+            self.assertEquals(song_import.verses, verses, 'Verses should match with test verses')
 
-    def songselect_import_usr_file_test(self):
+    def songselect_import_usr_file_2_test(self):
         """
-        Verify import SongSelect USR file parses file properly
-        :return:
+        Verify import SongSelect TEXT file parses file properly
         """
         # GIVEN: Text file to import and mocks
         copyright = '© 2011 OpenLP Programmer One (Admin. by OpenLP One)'

From e0c44c4b9fc7b49624801042ddbefa70fae5d825 Mon Sep 17 00:00:00 2001
From: Ken Roberts <alisonken1@gmail.com>
Date: Wed, 18 Feb 2015 14:39:02 -0800
Subject: [PATCH 6/8] Rename test to match file import type

---
 tests/functional/openlp_plugins/songs/test_songselect.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py
index 92e652149..0357f4477 100644
--- a/tests/functional/openlp_plugins/songs/test_songselect.py
+++ b/tests/functional/openlp_plugins/songs/test_songselect.py
@@ -471,7 +471,6 @@ class TestSongSelectFileImport(TestCase, TestMixin):
     def setUp(self):
         """
         Initial setups
-        :return:
         """
         Registry.create()
         test_song_name = 'TestSong'
@@ -484,7 +483,6 @@ class TestSongSelectFileImport(TestCase, TestMixin):
     def tearDown(self):
         """
         Test cleanups
-        :return:
         """
         pass
 
@@ -513,7 +511,7 @@ class TestSongSelectFileImport(TestCase, TestMixin):
             self.assertEquals(song_import.topics, self.topics, 'Theme(s) should match')
             self.assertEquals(song_import.verses, verses, 'Verses should match with test verses')
 
-    def songselect_import_usr_file_2_test(self):
+    def songselect_import_text_file_test(self):
         """
         Verify import SongSelect TEXT file parses file properly
         """

From 75c9391d8e46f27ea69148f08fd515acfb7b2f3d Mon Sep 17 00:00:00 2001
From: Phill Ridout <phill.ridout@gmail.com>
Date: Thu, 19 Feb 2015 18:57:05 +0000
Subject: [PATCH 7/8] Tests

---
 .../openlp_core_ui/test_thememanager.py       | 43 ++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/tests/functional/openlp_core_ui/test_thememanager.py b/tests/functional/openlp_core_ui/test_thememanager.py
index 9985eb3b3..85d470e4f 100644
--- a/tests/functional/openlp_core_ui/test_thememanager.py
+++ b/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

From fb20f9243f3353a8460d502ad7a5c8bb077b71a6 Mon Sep 17 00:00:00 2001
From: Phill Ridout <phill.ridout@gmail.com>
Date: Thu, 19 Feb 2015 21:24:12 +0000
Subject: [PATCH 8/8] Further fixes

---
 openlp/core/common/historycombobox.py |   6 +-
 openlp/plugins/bibles/lib/upgrade.py  | 259 ++++++++++++++------------
 openlp/plugins/songs/lib/upgrade.py   |   1 +
 3 files changed, 149 insertions(+), 117 deletions(-)

diff --git a/openlp/core/common/historycombobox.py b/openlp/core/common/historycombobox.py
index e55d08924..cc5e40649 100644
--- a/openlp/core/common/historycombobox.py
+++ b/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()
 
diff --git a/openlp/plugins/bibles/lib/upgrade.py b/openlp/plugins/bibles/lib/upgrade.py
index 378247c7b..63bb1c27f 100644
--- a/openlp/plugins/bibles/lib/upgrade.py
+++ b/openlp/plugins/bibles/lib/upgrade.py
@@ -30,6 +30,7 @@ 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.
@@ -38,132 +39,162 @@ def upgrade_1(session, metadata):
     """
     metadata_table = metadata.tables['metadata']
     # Copy "Version" to "name" ("version" used by upgrade system)
-    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'))
+    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"
-    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'))
+    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"
-    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'))
+    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"
-    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],
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
                 metadata_table.c.key == 'Bookname language'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(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"
-    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],
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
                 metadata_table.c.key == 'download source'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(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"
-    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],
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
                 metadata_table.c.key == 'download name'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(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"
-    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],
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
                 metadata_table.c.key == 'proxy server'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(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"
-    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],
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
                 metadata_table.c.key == 'proxy username'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(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"
-    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],
+    try:
+        value_count = session.execute(
+            select(
+                [func.count(metadata_table.c.value)],
                 metadata_table.c.key == 'proxy password'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy password'))
-    session.execute(delete(metadata_table).where(metadata_table.c.key == 'dbversion'))
+            )
+        ).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()
diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py
index 21d488b88..9b2cf70a2 100644
--- a/openlp/plugins/songs/lib/upgrade.py
+++ b/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.