openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxx>]

Phill has proposed merging lp:~phill-ridout/openlp/bug1416703 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1416703 in OpenLP: "Exception when saving a service"
  https://bugs.launchpad.net/openlp/+bug/1416703

For more details, see:
https://code.launchpad.net/~phill-ridout/openlp/bug1416703/+merge/249724

Fixes bug 1416703 by implementing the ability to provide a function to do the conversion.

Tests fail on the cross walk bible tests, but all pass (except crosswalk) locally

Add this to your merge proposal:
--------------------------------
lp:~phill-ridout/openlp/bug1416703 (revision 2497)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/942/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/868/
[FAILURE] http://ci.openlp.org/job/Branch-03-Interface-Tests/813/
Stopping after failure

Process finished with exit code 0


-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~phill-ridout/openlp/bug1416703 into lp:openlp.

=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py	2015-01-18 13:39:21 +0000
+++ openlp/core/common/settings.py	2015-02-13 21:02:19 +0000
@@ -45,6 +45,13 @@
         X11_BYPASS_DEFAULT = False
 
 
+def recent_files_conv(value):
+    if isinstance(value, list):
+        return value
+    elif isinstance(value, str):
+        return [value]
+
+
 class Settings(QtCore.QSettings):
     """
     Class to wrap QSettings.
@@ -66,10 +73,9 @@
         The first entry is the *old key*; it will be removed.
 
         The second entry is the *new key*; we will add it to the config. If this is just an empty string, we just remove
-        the old key.
-
-        The last entry is a list containing two-pair tuples. If the list is empty, no conversion is made. Otherwise each
-        pair describes how to convert the old setting's value::
+        the old key. The last entry is a list containing two-pair tuples. If the list is empty, no conversion is made.
+        If the first value is callable i.e. a function, the function will be called with the old setting's value.
+        Otherwise each pair describes how to convert the old setting's value::
 
             (SlideLimits.Wrap, True)
 
@@ -328,7 +334,7 @@
         ('general/language', 'core/language', []),
         ('general/last version test', 'core/last version test', []),
         ('general/loop delay', 'core/loop delay', []),
-        ('general/recent files', 'core/recent files', []),
+        ('general/recent files', 'core/recent files', [(recent_files_conv, None)]),
         ('general/save prompt', 'core/save prompt', []),
         ('general/screen blank', 'core/screen blank', []),
         ('general/show splash', 'core/show splash', []),
@@ -353,7 +359,7 @@
 
         :param default_values: A dict with setting keys and their default values.
         """
-        Settings.__default_settings__ = dict(list(default_values.items()) + list(Settings.__default_settings__.items()))
+        Settings.__default_settings__.update(default_values)
 
     @staticmethod
     def set_filename(ini_file):
@@ -410,7 +416,9 @@
                     for new, old in rules:
                         # If the value matches with the condition (rule), then use the provided value. This is used to
                         # convert values. E. g. an old value 1 results in True, and 0 in False.
-                        if old == old_value:
+                        if callable(new):
+                            old_value = new(old_value)
+                        elif old == old_value:
                             old_value = new
                             break
                     self.setValue(new_key, old_value)

=== modified file 'tests/functional/openlp_core_common/test_settings.py'
--- tests/functional/openlp_core_common/test_settings.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_core_common/test_settings.py	2015-02-13 21:02:19 +0000
@@ -25,6 +25,7 @@
 from unittest import TestCase
 
 from openlp.core.common import Settings
+from tests.functional import patch
 from tests.helpers.testmixin import TestMixin
 
 
@@ -120,3 +121,19 @@
 
         # THEN: An exception with the non-existing key should be thrown
         self.assertEqual(str(cm.exception), "'core/does not exists'", 'We should get an exception')
+
+    def extend_default_settings_test(self):
+        """
+        Test that the extend_default_settings method extends the default settings
+        """
+        # GIVEN: A patched __default_settings__ dictionary
+        with patch.dict(Settings.__default_settings__,
+                   {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 3}, True):
+
+            # WHEN: Calling extend_default_settings
+            Settings.extend_default_settings({'test/setting 3': 4, 'test/extended 1': 1, 'test/extended 2': 2})
+
+            # THEN: The _default_settings__ dictionary_ should have the new keys
+            self.assertEqual(
+                Settings.__default_settings__, {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 4,
+                                                'test/extended 1': 1, 'test/extended 2': 2})