Merge branch 'shunju-fix/upgrade-fixture-try2' into master

.gitignore

@@ -78,3 +78,5 @@ Session.vim
 tags
 
 # End of https://www.gitignore.io/api/vim
+
+htmlcov/

flaskbb/cli/main.py

@@ -225,8 +225,8 @@ def upgrade(all_latest, fixture, force):
         count = update_settings_from_fixture(
             fixture=settings, overwrite_group=force, overwrite_setting=force
         )
-        click.secho("[+] {} groups and {} settings updated.".format(
-            len(count.keys()), len(count.values())), fg="green"
+        click.secho("[+] {settings} settings in {groups} setting groups updated.".format(
+            groups=len(count), settings=sum(len(settings) for settings in count.values())), fg="green"
         )
 
 

flaskbb/utils/populate.py

@@ -9,6 +9,7 @@
     :license: BSD, see LICENSE for more details.
 """
 from __future__ import unicode_literals
+import collections
 
 from sqlalchemy_utils.functions import create_database, database_exists
 
@@ -88,7 +89,7 @@ def update_settings_from_fixture(fixture, overwrite_group=False,
                               setting if it already exists.
                               Defaults to ``False``.
     """
-    updated_settings = {}
+    updated_settings = collections.defaultdict(list)
 
     for settingsgroup in fixture:
 
@@ -112,8 +113,17 @@ def update_settings_from_fixture(fixture, overwrite_group=False,
 
             setting = Setting.query.filter_by(key=settings[0]).first()
 
-            if (setting is not None and overwrite_setting) or setting is None:
+            if setting is not None:
+                setting_is_different = (
+                    setting.value != settings[1]["value"]
+                    or setting.value_type != settings[1]["value_type"]
+                    or setting.name != settings[1]["name"]
+                    or setting.description != settings[1]["description"]
+                    or setting.extra != settings[1].get("extra", "")
+                    or setting.settingsgroup != group.key
+                )
 
+            if (setting is not None and overwrite_setting and setting_is_different) or setting is None:
                 if setting is not None:
                     setting.value = settings[1]["value"]
                     setting.value_type = settings[1]["value_type"]
@@ -133,7 +143,6 @@ def update_settings_from_fixture(fixture, overwrite_group=False,
                     )
 
                 setting.save()
-                updated_settings[group] = []
                 updated_settings[group].append(setting)
     return updated_settings
 

tests/conftest.py

@@ -2,3 +2,4 @@ from tests.fixtures.app import *  # noqa
 from tests.fixtures.forum import *  # noqa
 from tests.fixtures.user import *  # noqa
 from tests.fixtures.message import *  # noqa
+from tests.fixtures.settings_fixture import *  # noqa

tests/fixtures/settings_fixture.py

@@ -0,0 +1,56 @@
+import pytest
+
+
+@pytest.fixture
+def updated_fixture():
+    return (
+        # a group where we change a lot
+        ('general', {
+            'name': "General Settings",
+            'description': "This description is wrong.",
+            'settings': (
+                # change value
+                ('project_title', {
+                    'value': "FlaskBB is cool!",
+                    'value_type': "string",
+                    'name': "Project title",
+                    'description': "The title of the project.",
+                }),
+                # add
+                ('test_fixture', {
+                    'description': 'This is a test fixture',
+                    'name': 'Test Fixture',
+                    'value': 'FlaskBBTest',
+                    'value_type': 'string'
+                }),
+            )
+        }),
+        # a group where we change nothing
+        ('auth', {
+            'name': 'Authentication Settings',
+            'description': 'Settings for the Login and Register process.',
+            # the same as in flaskbb/settings/fixtures/settings.py
+            'settings': (
+                ('registration_enabled', {
+                    'value': True,
+                    'value_type': "boolean",
+                    'name': "Enable Registration",
+                    'description': "Enable or disable the registration",
+                }),
+            )
+        }),
+        # a wholly new group
+        ('testgroup', {
+            'name': "Important settings",
+            'description': "Some settings without the world would not work.",
+            'settings': (
+                # change value
+                ('monty_python', {
+                    'value': "And now for something completely different...",
+                    'value_type': "string",
+                    'name': "Monty Python",
+                    'description': "A random quote from Monty Python.",
+                }),
+            )
+        })
+    )

tests/unit/utils/test_populate.py

@@ -14,6 +14,14 @@ from flaskbb.forum.models import Category, Topic, Post
 from flaskbb.management.models import Setting, SettingsGroup
 
 
+def _individual_settings(update_result):
+    """Helper that returns the number of settings that were updated."""
+    return sum(
+        len(settings_in_a_group)
+        for settings_in_a_group in update_result.values()
+    )
+
+
 def test_delete_settings_from_fixture(default_settings):
     groups_count = SettingsGroup.query.count()
     assert len(settings_fixture) == groups_count
@@ -36,42 +44,60 @@ def test_create_settings_from_fixture(database):
 
 
 def test_update_settings_from_fixture(database):
-    # No force-overwrite - the fixtures will be created because
-    # do not exist.
+    settings_fixture_group_count = len(settings_fixture)
+    settings_fixture_setting_count = sum(
+        len(settings_fixture[k][1]['settings'])
+        for k in range(len(settings_fixture))
+    )
+
     assert not SettingsGroup.query.count()
     assert not Setting.query.count()
+
+    # No force-overwrite - the fixtures will be created because they
+    # do not exist.
     updated = update_settings_from_fixture(settings_fixture)
-    assert len(updated) == SettingsGroup.query.count()
+    assert settings_fixture_group_count == len(updated)
+    assert settings_fixture_group_count == SettingsGroup.query.count()
+    assert settings_fixture_setting_count == _individual_settings(updated)
+    assert settings_fixture_setting_count == Setting.query.count()
+
+
+def test_update_settings_from_fixture_overwrite(database, default_settings,
+                                                updated_fixture):
+    # should add groups: testgroup
+    # should add testgroup/monty_python, general/test_fixture
+    pre_update_group_count = SettingsGroup.query.count()
+    pre_update_setting_count = Setting.query.count()
+    updated = update_settings_from_fixture(updated_fixture)
+    assert len(updated) == 2
+    assert _individual_settings(updated) == 2
+    assert pre_update_group_count + 1 == SettingsGroup.query.count()
+    assert pre_update_setting_count + 2 == Setting.query.count()
 
-    # force-overwrite - the fixtures exist, but they will be overwritten now.
+
+def test_update_settings_from_fixture_force(database, default_settings,
+                                            updated_fixture):
+    # force-overwrite - nothing changed so nothing should happen here
+    pre_update_group_count = SettingsGroup.query.count()
+    pre_update_setting_count = Setting.query.count()
     force_updated = update_settings_from_fixture(settings_fixture,
                                                  overwrite_group=True,
                                                  overwrite_setting=True)
-    assert len(force_updated) == SettingsGroup.query.count()
-
-    updated_fixture = (
-        ('general', {
-            'name': "General Settings",
-            'description': "How many items per page are displayed.",
-            'settings': (
-                ('project_title', {
-                    'value': "FlaskBB",
-                    'value_type': "string",
-                    'name': "Project title",
-                    'description': "The title of the project.",
-                }),
-                ('test_fixture', {
-                    'description': 'This is a test fixture',
-                    'name': 'Test Fixture',
-                    'value': 'FlaskBBTest',
-                    'value_type': 'string'
-                })
-            )
-        }),
-    )
 
-    updated = update_settings_from_fixture(updated_fixture)
-    assert len(updated) == 1
+    assert len(force_updated) == 0
+    assert _individual_settings(force_updated) == 0
+    assert pre_update_group_count == SettingsGroup.query.count()
+    assert pre_update_setting_count == Setting.query.count()
+
+    # should update groups: general
+    # should update settings: 2 in general, 1 in testgroup
+    force_updated_1 = update_settings_from_fixture(updated_fixture,
+                                                   overwrite_group=True,
+                                                   overwrite_setting=True)
+    assert len(force_updated_1) == 2
+    assert _individual_settings(force_updated_1) == 3
+    assert pre_update_group_count + 1 == SettingsGroup.query.count()
+    assert pre_update_setting_count + 2 == Setting.query.count()
 
 
 def test_create_user(default_groups):