openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

See inline

Diff comments:

> === modified file 'openlp/core/__init__.py'
> --- openlp/core/__init__.py	2014-09-21 19:23:40 +0000
> +++ openlp/core/__init__.py	2014-10-13 16:43:40 +0000
> @@ -39,11 +39,12 @@
>  import logging
>  from optparse import OptionParser
>  from traceback import format_exception
> -
> +import shutil
> +import time
>  from PyQt4 import QtCore, QtGui
>  
>  from openlp.core.common import Registry, OpenLPMixin, AppLocation, Settings, UiStrings, check_directory_exists, \
> -    is_macosx, is_win
> +    is_macosx, is_win, translate
>  from openlp.core.lib import ScreenList
>  from openlp.core.resources import qInitResources
>  from openlp.core.ui.mainwindow import MainWindow
> @@ -136,6 +137,8 @@
>              self.splash.show()
>          # make sure Qt really display the splash screen
>          self.processEvents()
> +        # Check if OpenLP has been upgrade and if a backup of data should be created
> +        self.backup_on_upgrade(has_run_wizard)
>          # start the main app window
>          self.main_window = MainWindow()
>          Registry().execute('bootstrap_initialise')
> @@ -192,6 +195,40 @@
>          self.set_normal_cursor()
>          self.exception_form.exec_()
>  
> +    def backup_on_upgrade(self, old_install):

don't rename the variable it confused me.

> +        """
> +        Check if OpenLP has been upgraded, and ask if a backup of data should be made
> +
> +        :param old_install: OpenLP has been run before
> +        """
> +        data_version = Settings().value('core/data version')
> +        openlp_version = get_application_version()['version']
> +        # New installation, no need to create backup
> +        if not old_install:
> +            Settings().setValue('core/data version', openlp_version)
> +        # If data_version is different from the current version ask if we should backup the data folder
> +        elif data_version != openlp_version:
> +            if QtGui.QMessageBox.question(None, translate('OpenLP', 'Backup'),
> +                                          translate('OpenLP', 'OpenLP has been upgraded, '
> +                                                              'do you want to create a backup of OpenLPs data folder?'),
> +                                          QtGui.QMessageBox.Yes | QtGui.QMessageBox.No,
> +                                          QtGui.QMessageBox.Yes) == QtGui.QMessageBox.Yes:
> +                # Create copy of data folder
> +                data_folder_path = AppLocation.get_data_path()
> +                timestamp = time.strftime("%Y%m%d-%H%M%S")
> +                data_folder_backup_path = data_folder_path + '-' + timestamp
> +                try:
> +                    shutil.copytree(data_folder_path, data_folder_backup_path)
> +                except OSError:
> +                    QtGui.QMessageBox.warning(None, translate('OpenLP', 'Backup'),
> +                                              translate('OpenLP', 'Backup of the data folder failed!'))
> +                    return
> +                QtGui.QMessageBox.information(None, translate('OpenLP', 'Backup'),
> +                                              translate('OpenLP', 'A backup of the data folder has been created at %s')
> +                                              % data_folder_backup_path)
> +            # Update the version in the settings
> +            Settings().setValue('core/data version', openlp_version)
> +
>      def process_events(self):
>          """
>          Wrapper to make ProcessEvents visible and named correctly
> 
> === modified file 'openlp/core/common/settings.py'
> --- openlp/core/common/settings.py	2014-09-08 21:08:49 +0000
> +++ openlp/core/common/settings.py	2014-10-13 16:43:40 +0000
> @@ -137,6 +137,7 @@
>          # circular dependency.
>          'core/display on monitor': True,
>          'core/override position': False,
> +        'core/data version': '0.0',
>          'images/background color': '#000000',
>          'media/players': 'webkit',
>          'media/override player': QtCore.Qt.Unchecked,
> 
> === modified file 'tests/functional/test_init.py'
> --- tests/functional/test_init.py	2014-04-20 13:00:14 +0000
> +++ tests/functional/test_init.py	2014-10-13 16:43:40 +0000
> @@ -33,9 +33,10 @@
>  
>  from unittest import TestCase
>  from unittest.mock import MagicMock, patch
> -from PyQt4 import QtCore
> +from PyQt4 import QtCore, QtGui
>  
>  from openlp.core import OpenLP
> +from openlp.core.common import Settings
>  from tests.helpers.testmixin import TestMixin
>  
>  
> @@ -44,11 +45,13 @@
>  
>  class TestInit(TestCase, TestMixin):
>      def setUp(self):
> +        self.build_settings()
>          with patch('openlp.core.common.OpenLPMixin.__init__') as constructor:
>              constructor.return_value = None
>              self.openlp = OpenLP(list())
>  
>      def tearDown(self):
> +        self.destroy_settings()
>          del self.openlp
>  
>      def event_test(self):
> @@ -68,3 +71,51 @@
>          self.assertTrue(result, "The method should have returned True.")
>          mocked_file_method.assert_called_once_with()
>          self.assertEqual(self.openlp.args[0], file_path, "The path should be in args.")
> +
> +    def backup_on_upgrade_first_install_test(self):
> +        """
> +        Test that we don't try to backup on a new install
> +        """
> +        # GIVEN: Mocked data version and OpenLP version which are the same
> +        old_install = False
> +        MOCKED_VERSION = {
> +            'full': '2.2.0-bzr000',
> +            'version': '2.2.0',
> +            'build': 'bzr000'
> +        }
> +        Settings().setValue('core/data version', '2.2.0')

This should be default value if not run before.

> +        with patch('openlp.core.get_application_version') as mocked_get_application_version,\
> +                patch('openlp.core.QtGui.QMessageBox.question') as mocked_question:
> +            mocked_get_application_version.return_value = MOCKED_VERSION
> +            mocked_question.return_value = QtGui.QMessageBox.No
> +
> +            # WHEN: We check if a backup should be created
> +            self.openlp.backup_on_upgrade(old_install)
> +
> +            # THEN: It should not ask if we want to create a backup
> +            self.assertEqual(Settings().value('core/data version'), '2.2.0', 'Version should be the same!')
> +            self.assertEqual(mocked_question.call_count, 0, 'No question should have been asked!')
> +
> +    def backup_on_upgrade_test(self):
> +        """
> +        Test that we try to backup on a new install
> +        """
> +        # GIVEN: Mocked data version and OpenLP version which are different
> +        old_install = True
> +        MOCKED_VERSION = {
> +            'full': '2.2.0-bzr000',
> +            'version': '2.2.0',
> +            'build': 'bzr000'
> +        }
> +        Settings().setValue('core/data version', '2.0.5')
> +        with patch('openlp.core.get_application_version') as mocked_get_application_version,\
> +                patch('openlp.core.QtGui.QMessageBox.question') as mocked_question:
> +            mocked_get_application_version.return_value = MOCKED_VERSION
> +            mocked_question.return_value = QtGui.QMessageBox.No
> +
> +            # WHEN: We check if a backup should be created
> +            self.openlp.backup_on_upgrade(old_install)
> +
> +            # THEN: It should ask if we want to create a backup
> +            self.assertEqual(Settings().value('core/data version'), '2.2.0', 'Version should be upgraded!')
> +            self.assertEqual(mocked_question.call_count, 1, 'A question should have been asked!')
> 


-- 
https://code.launchpad.net/~tomasgroth/openlp/backup-on-upgrade/+merge/238192
Your team OpenLP Core is subscribed to branch lp:openlp.