diff --git a/constance/__init__.py b/constance/__init__.py index 7cec373..7ba1b9d 100644 --- a/constance/__init__.py +++ b/constance/__init__.py @@ -1,4 +1,5 @@ from django.utils.functional import LazyObject +from . import checks __version__ = '2.4.0' diff --git a/constance/admin.py b/constance/admin.py index 150acc3..51f68c6 100644 --- a/constance/admin.py +++ b/constance/admin.py @@ -24,6 +24,7 @@ from django.utils.module_loading import import_string from django.utils.translation import ugettext_lazy as _ from . import LazyConfig, settings +from .checks import get_inconsistent_fieldnames config = LazyConfig() @@ -163,11 +164,7 @@ class ConstanceForm(forms.Form): if not settings.CONFIG_FIELDSETS: return cleaned_data - field_name_list = [] - for fieldset_title, fields_list in settings.CONFIG_FIELDSETS.items(): - for field_name in fields_list: - field_name_list.append(field_name) - if field_name_list and set(set(settings.CONFIG.keys()) - set(field_name_list)): + if get_inconsistent_fieldnames(): raise forms.ValidationError(_('CONSTANCE_CONFIG_FIELDSETS is missing ' 'field(s) that exists in CONSTANCE_CONFIG.')) diff --git a/constance/checks.py b/constance/checks.py new file mode 100644 index 0000000..de66029 --- /dev/null +++ b/constance/checks.py @@ -0,0 +1,42 @@ +from django.core import checks +from django.utils.translation import ugettext_lazy as _ + +from . import settings + + +@checks.register("constance") +def check_fieldsets(*args, **kwargs): + """ + A Django system check to make sure that, if defined, CONFIG_FIELDSETS accounts for + every entry in settings.CONFIG. + """ + if hasattr(settings, "CONFIG_FIELDSETS") and settings.CONFIG_FIELDSETS: + inconsistent_fieldnames = get_inconsistent_fieldnames() + if inconsistent_fieldnames: + return [ + checks.Warning( + _( + "CONSTANCE_CONFIG_FIELDSETS is missing " + "field(s) that exists in CONSTANCE_CONFIG." + ), + hint=", ".join(sorted(inconsistent_fieldnames)), + obj="settings.CONSTANCE_CONFIG", + id="constance.E001", + ) + ] + return [] + + +def get_inconsistent_fieldnames(): + """ + Returns a set of keys from settings.CONFIG that are not accounted for in + settings.CONFIG_FIELDSETS. + If there are no fieldnames in settings.CONFIG_FIELDSETS, returns an empty set. + """ + field_name_list = [] + for fieldset_title, fields_list in settings.CONFIG_FIELDSETS.items(): + for field_name in fields_list: + field_name_list.append(field_name) + if not field_name_list: + return {} + return set(set(settings.CONFIG.keys()) - set(field_name_list)) diff --git a/tests/test_admin.py b/tests/test_admin.py index e212d61..a6d7706 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -3,6 +3,7 @@ from django.contrib import admin from django.contrib.auth.models import User, Permission from django.contrib.contenttypes.models import ContentType from django.core.exceptions import PermissionDenied +from django.http import HttpResponseRedirect from django.template.defaultfilters import linebreaksbr from django.test import TestCase, RequestFactory from django.utils import six @@ -74,6 +75,46 @@ class TestAdmin(TestCase): self.assertContains(response, '

Numbers

') self.assertContains(response, '

Text

') + @mock.patch('constance.settings.CONFIG_FIELDSETS', { + 'FieldSetOne': ('INT_VALUE',) + }) + @mock.patch('constance.settings.CONFIG', { + 'INT_VALUE': (1, 'some int'), + }) + @mock.patch('constance.settings.IGNORE_ADMIN_VERSION_CHECK', True) + def test_submit(self): + """ + Test that submitting the admin page results in an http redirect when + everything is in order. + """ + self.client.login(username='admin', password='nimda') + request = self.rf.post('/admin/constance/config/', data={ + "INT_VALUE": settings.CONFIG['INT_VALUE'][0], + "version": "123", + }) + request.user = self.superuser + request._dont_enforce_csrf_checks = True + with mock.patch("constance.admin.ConstanceForm.save"): + with mock.patch("django.contrib.messages.add_message"): + response = self.options.changelist_view(request, {}) + self.assertIsInstance(response, HttpResponseRedirect) + + @mock.patch('constance.settings.CONFIG_FIELDSETS', { + 'Numbers': ('LONG_VALUE', 'INT_VALUE',), + 'Text': ('STRING_VALUE', 'UNICODE_VALUE'), + }) + def test_inconsistent_fieldset_submit(self): + """ + Test that the admin page warns users if the CONFIG_FIELDSETS setting + doesn't account for every field in CONFIG. + """ + self.client.login(username='admin', password='nimda') + request = self.rf.post('/admin/constance/config/', data=None) + request.user = self.superuser + request._dont_enforce_csrf_checks = True + response = self.options.changelist_view(request, {}) + self.assertContains(response, 'is missing field(s)') + @mock.patch('constance.settings.CONFIG_FIELDSETS', { 'Numbers': ('LONG_VALUE', 'INT_VALUE',), }) diff --git a/tests/test_checks.py b/tests/test_checks.py new file mode 100644 index 0000000..0abc62e --- /dev/null +++ b/tests/test_checks.py @@ -0,0 +1,45 @@ +# -*- coding: utf-8 -*- + +import datetime +from decimal import Decimal +import mock + +from constance.admin import get_values +from constance.checks import check_fieldsets, get_inconsistent_fieldnames +from constance.management.commands.constance import _set_constance_value +from django.core.exceptions import ValidationError +from django.test import TestCase +from constance import settings + + +class ChecksTestCase(TestCase): + @mock.patch("constance.settings.CONFIG_FIELDSETS", {"Set1": settings.CONFIG.keys()}) + def test_get_inconsistent_fieldnames_none(self): + """ + Test that get_inconsistent_fieldnames returns an empty set and no checks fail + if CONFIG_FIELDSETS accounts for every key in settings.CONFIG. + """ + self.assertFalse(get_inconsistent_fieldnames()) + self.assertEqual(0, len(check_fieldsets())) + + @mock.patch( + "constance.settings.CONFIG_FIELDSETS", + {"Set1": list(settings.CONFIG.keys())[:-1]}, + ) + def test_get_inconsistent_fieldnames_one(self): + """ + Test that get_inconsistent_fieldnames returns a set and the check fails + if CONFIG_FIELDSETS does not account for every key in settings.CONFIG. + """ + self.assertTrue(get_inconsistent_fieldnames()) + self.assertEqual(1, len(check_fieldsets())) + + @mock.patch( + "constance.settings.CONFIG_FIELDSETS", {} + ) + def test_check_fieldsets(self): + """ + check_fieldsets should not output warning if CONFIG_FIELDSETS is not defined. + """ + del settings.CONFIG_FIELDSETS + self.assertEqual(0, len(check_fieldsets()))