diff --git a/accounts/models.py b/accounts/models.py index 69eed4c3dea891fcbf9f38b82dada980c11a6c1b..85909e9504a1ac46feea0c9b6dc6ed6d88e9d6da 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -131,7 +131,7 @@ class User(AbstractBaseUser, PermissionsMixin): def get_full_name(self, allow_blank=True, force_verbose_blank=False): if (not allow_blank - and not self.first_name or not self.last_name): + and (not self.first_name or not self.last_name)): return None def verbose_blank(s): @@ -150,11 +150,10 @@ class User(AbstractBaseUser, PermissionsMixin): return '%s %s' % ( verbose_blank(first_name), verbose_blank(last_name)) - from django.conf import settings - format_method = getattr( - settings, - "RELATE_USER_FULL_NAME_FORMAT_METHOD", - default_fullname) + from accounts.utils import relate_user_method_settings + format_method = relate_user_method_settings.get_custom_full_name_method + if format_method is None: + format_method = default_fullname try: full_name = format_method( @@ -194,6 +193,10 @@ class User(AbstractBaseUser, PermissionsMixin): settings, "RELATE_EMAIL_APPELATION_PRIORITY_LIST", []) + # When the settings explicitly set it to None + if not customized_priority_list: + customized_priority_list = [] + priority_list = [] # filter out not allowd appellations in customized list diff --git a/accounts/utils.py b/accounts/utils.py new file mode 100644 index 0000000000000000000000000000000000000000..6100f072979207ea0323ff0b525cdf623180cf70 --- /dev/null +++ b/accounts/utils.py @@ -0,0 +1,153 @@ +# -*- coding: utf-8 -*- + +from __future__ import division, unicode_literals + +__copyright__ = "Copyright (C) 2018 Dong Zhuang" + +__license__ = """ +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +""" + +import six +from django.core.checks import Warning +from django.utils.functional import cached_property +from django.utils.module_loading import import_string + +RELATE_USER_FULL_NAME_FORMAT_METHOD = "RELATE_USER_FULL_NAME_FORMAT_METHOD" + + +class RelateUserMethodSettingsInitializer(object): + """ + This is used to check (validate) settings.RELATE_CSV_SETTINGS (optional) + and initialize the settings for csv export for csv-related forms. + """ + + def __init__(self): + self._custom_full_name_method = None + + @cached_property + def get_custom_full_name_method(self): + self.check_custom_full_name_method() + return self._custom_full_name_method + + def check_custom_full_name_method(self): + errors = [] + + from django.conf import settings + relate_user_full_name_format_method = getattr( + settings, RELATE_USER_FULL_NAME_FORMAT_METHOD, None) + self._custom_full_name_method = None + if relate_user_full_name_format_method is not None: + if isinstance(relate_user_full_name_format_method, six.string_types): + try: + relate_user_full_name_format_method = ( + import_string(relate_user_full_name_format_method)) + except ImportError: + errors = [Warning( + msg=( + "%(location)s: `%(method)s` failed to be imported, " + "default format method will be used." + % {"location": RELATE_USER_FULL_NAME_FORMAT_METHOD, + "method": relate_user_full_name_format_method + } + ), + id="relate_user_full_name_format_method.W001" + )] + return errors + + self._custom_full_name_method = relate_user_full_name_format_method + if not callable(relate_user_full_name_format_method): + errors.append(Warning( + msg=( + "%(location)s: `%(method)s` is not a callable, " + "default format method will be used." + % {"location": RELATE_USER_FULL_NAME_FORMAT_METHOD, + "method": relate_user_full_name_format_method + } + ), + id="relate_user_full_name_format_method.W002" + )) + else: + try: + returned_name = ( + relate_user_full_name_format_method("first_name", + "last_name")) + except Exception as e: + from traceback import format_exc + errors.append(Warning( + msg=( + "%(location)s: `%(method)s` called with '" + "args 'first_name', 'last_name' failed with" + "exception below:\n" + "%(err_type)s: %(err_str)s\n" + "%(format_exc)s\n\n" + "Default format method will be used." + % {"location": RELATE_USER_FULL_NAME_FORMAT_METHOD, + "method": relate_user_full_name_format_method, + "err_type": type(e).__name__, + "err_str": str(e), + 'format_exc': format_exc()} + ), + id="relate_user_full_name_format_method.W003" + )) + else: + unexpected_return_value = "" + if returned_name is None: + unexpected_return_value = "None" + if not isinstance(returned_name, six.string_types): + unexpected_return_value = type(returned_name).__name__ + elif not returned_name.strip(): + unexpected_return_value = "empty string %s" % returned_name + if unexpected_return_value: + errors.append(Warning( + msg=("%(location)s: `%(method)s` is expected to " + "return a non-empty string, got `%(result)s`, " + "default format method will be used." + % { + "location": RELATE_USER_FULL_NAME_FORMAT_METHOD, + "method": relate_user_full_name_format_method, + "result": unexpected_return_value, + }), + id="relate_user_full_name_format_method.W004" + )) + else: + returned_name2 = ( + relate_user_full_name_format_method("first_name2", + "last_name2")) + if returned_name == returned_name2: + errors.append(Warning( + msg=("%(location)s: `%(method)s` is expected to " + "return different value with different " + "input, default format method will be used." + % { + "location": + RELATE_USER_FULL_NAME_FORMAT_METHOD, + "method": + relate_user_full_name_format_method + }), + id="relate_user_full_name_format_method.W005" + )) + + if errors: + self._custom_full_name_method = None + + return errors + + +relate_user_method_settings = RelateUserMethodSettingsInitializer() diff --git a/relate/checks.py b/relate/checks.py index 0e7527d29fde826392e24ccea76e319b0efbb539..4e76c63abcb5ea2c51c1b64b69648e70ce1ef788 100644 --- a/relate/checks.py +++ b/relate/checks.py @@ -29,6 +29,7 @@ import six from django.conf import settings from django.core.checks import Critical, Warning, register from django.core.exceptions import ImproperlyConfigured +from django.utils.module_loading import import_string REQUIRED_CONF_ERROR_PATTERN = ( "You must configure %(location)s for RELATE to run properly.") @@ -110,10 +111,14 @@ def check_relate_settings(app_configs, **kwargs): INSTANCE_ERROR_PATTERN % {"location": RELATE_EMAIL_APPELATION_PRIORITY_LIST, "types": "list or tuple"}), - id="relate_email_appelation_priority_list.E002") + id="relate_email_appelation_priority_list.E001") ) # }}} + # check RELATE_CSV_SETTINGS + from accounts.utils import relate_user_method_settings + errors.extend(relate_user_method_settings.check_custom_full_name_method()) + # {{{ check EMAIL_CONNECTIONS email_connections = getattr(settings, EMAIL_CONNECTIONS, None) if email_connections is not None: @@ -138,7 +143,6 @@ def check_relate_settings(app_configs, **kwargs): )) else: if "backend" in c: - from django.utils.module_loading import import_string try: import_string(c["backend"]) except ImportError as e: @@ -292,7 +296,7 @@ def check_relate_settings(app_configs, **kwargs): # }}} - # {{{ check RELATE_SESSION_RESTART_COOLDOWN_SECONDS + # {{{ check RELATE_TICKET_MINUTES_VALID_AFTER_USE relate_ticket_minutes_valid_after_use = getattr( settings, RELATE_TICKET_MINUTES_VALID_AFTER_USE, None) if relate_ticket_minutes_valid_after_use is not None: @@ -526,7 +530,6 @@ def register_startup_checks_extra(): "types": "list or tuple" } ) - from django.utils.module_loading import import_string for c in startup_checks_extra: try: check_item = import_string(c) diff --git a/tests/resource.py b/tests/resource.py new file mode 100644 index 0000000000000000000000000000000000000000..0985a7ef7719b2d3eb4cca68f66528df212b4610 --- /dev/null +++ b/tests/resource.py @@ -0,0 +1,34 @@ +from __future__ import division + +__copyright__ = "Copyright (C) 2018 Dong Zhuang" + +__license__ = """ +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +""" + + +def my_customized_get_full_name_method(first_name, last_name): + return "%s %s" % (first_name.title(), last_name.title()) + + +def my_customized_get_full_name_method_invalid(first_name, last_name): # noqa + return None + + +my_customized_get_full_name_method_invalid_str = "some_string" diff --git a/tests/test_accounts/test_models.py b/tests/test_accounts/test_models.py index dc2352959f3232acd504017b646b1d5ac1629442..09066fd1d1d726fe1a184c51e6265501a485279b 100644 --- a/tests/test_accounts/test_models.py +++ b/tests/test_accounts/test_models.py @@ -24,10 +24,11 @@ THE SOFTWARE. from django.core.exceptions import ValidationError from django.db.utils import IntegrityError -from django.test import TestCase +from django.test import TestCase, override_settings from django.contrib.auth import get_user_model from tests.factories import UserFactory +from tests.utils import mock class UserModelTest(TestCase): @@ -69,3 +70,64 @@ class UserModelTest(TestCase): UserFactory.create() with self.assertRaises(IntegrityError): UserFactory.create(email=None) + + def test_custom_get_full_name_method_failed(self): + """ + Test when RELATE_USER_FULL_NAME_FORMAT_METHOD failed, default method + is used. + """ + user = UserFactory.create(first_name="my_first", last_name="my_last") + + default_get_full_name = user.get_full_name() + + custom_get_full_name_path = ( + "tests.resource.my_customized_get_full_name_method") + get_custom_full_name_method_path = ( + "accounts.utils.RelateUserMethodSettingsInitializer" + ".get_custom_full_name_method") + + with override_settings( + RELATE_USER_FULL_NAME_FORMAT_METHOD=custom_get_full_name_path): + + from accounts.utils import relate_user_method_settings + # clear cached value + relate_user_method_settings.__dict__ = {} + + # If custom method works, the returned value is different with + # default value. + self.assertNotEqual(default_get_full_name, user.get_full_name()) + + with mock.patch(get_custom_full_name_method_path) as mock_custom_method: + # clear cached value + relate_user_method_settings.__dict__ = {} + + # raise an error when calling custom method + mock_custom_method.side_effect = Exception() + + # the value falls back to default value + self.assertEqual(user.get_full_name(), default_get_full_name) + + def test_custom_get_full_name_method_is_cached(self): + """ + Test relate_user_method_settings.get_custom_full_name_method is cached. + """ + + user = UserFactory.create(first_name="my_first", last_name="my_last") + custom_get_full_name_path = ( + "tests.resource.my_customized_get_full_name_method") + get_custom_full_name_check_path = ( + "accounts.utils.RelateUserMethodSettingsInitializer" + ".check_custom_full_name_method") + + with override_settings( + RELATE_USER_FULL_NAME_FORMAT_METHOD=custom_get_full_name_path): + + from accounts.utils import relate_user_method_settings + # clear cached value + relate_user_method_settings.__dict__ = {} + + user.get_full_name() + + with mock.patch(get_custom_full_name_check_path) as mock_check: + user.get_full_name() + self.assertEqual(mock_check.call_count, 0) diff --git a/tests/test_checks.py b/tests/test_checks.py index 76aeacfb2afbe1d1603c3de681fd231fef8f2c73..338af0ea1eb2ad3346aaab87d5e0b78d9c21bc00 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -22,6 +22,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ +import six import os from datetime import datetime @@ -29,7 +30,9 @@ from django.test import SimpleTestCase from django.test.utils import override_settings from django.utils.translation import ugettext_lazy as _ from django.conf import settings +from unittest import skipIf from tests.utils import mock +from tests.factories import UserFactory class CheckRelateSettingsBase(SimpleTestCase): @@ -127,6 +130,163 @@ class CheckRelateURL(CheckRelateSettingsBase): self.assertCheckMessages(["relate_base_url.E003"]) +class CheckRelateUserFullNameFormatMethod(CheckRelateSettingsBase): + # This TestCase is not pure for check, but also make sure it returned + # expected result + allow_database_queries = True + + msg_id_prefix = "relate_user_full_name_format_method" + + @skipIf(six.PY2, "PY2 doesn't support subTest") + def test_get_full_name(self): + def valid_method(first_name, last_name): + return "%s %s" % (last_name, first_name) + + def invalid_method1(first_name): + return first_name + + def invalid_method2(first_name, last_name): # noqa + return None + + def invalid_method3(first_name, last_name): # noqa + return " " + + def invalid_method4(first_name, last_name): # noqa + return b"my_name" + + def invalid_method5(first_name, last_name): # noqa + return "my_name" + + def invalid_method6(first_name, last_name): # noqa + return Exception() + + default_user_dict = {"first_name": "first_name", "last_name": "last_name"} + default_result = "first_name last_name" + + user_get_full_name_test_kwargs_list = ( + ({"id": 1, + "custom_method": None, + "user_dict": {}, + "default": '', + "not_allow_blank": None, + "force_verbose_blank": "(blank) (blank)"}), + ({"id": 2, + "custom_method": None, + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result}), + ({"id": 3, + "custom_method": valid_method, + "user_dict": default_user_dict, + "default": "last_name first_name", + "not_allow_blank": "last_name first_name", + "force_verbose_blank": "last_name first_name"}), + ({"id": 4, + "custom_method": invalid_method1, + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W003']}), + ({"id": 5, + "custom_method": invalid_method2, + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W004']}), + ({"id": 6, + "custom_method": invalid_method3, + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W004']}), + ({"id": 7, + "custom_method": invalid_method4, + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W004']}), + ({"id": 8, + "custom_method": invalid_method5, + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W005']}), + ({"id": 9, + "custom_method": invalid_method6, + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W004']}), + ({"id": 10, + "custom_method": "abcd", # a string + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W001']}), + ({"id": 11, + "custom_method": + "tests.resource.my_customized_get_full_name_method", + "user_dict": default_user_dict, + "default": "First_Name Last_Name", + "not_allow_blank": "First_Name Last_Name", + "force_verbose_blank": "First_Name Last_Name"}), + ({"id": 12, + "custom_method": + "tests.resource.my_customized_get_full_name_method_invalid", + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W004']}), + ({"id": 13, + "custom_method": + "tests.resource.my_customized_get_full_name_method_invalid_str", + "user_dict": default_user_dict, + "default": default_result, + "not_allow_blank": default_result, + "force_verbose_blank": default_result, + "check_messages": ['relate_user_full_name_format_method.W002']}), + ({"id": 14, + "custom_method": + "tests.resource.my_customized_get_full_name_method", + "user_dict": {"first_name": "first_name"}, + "default": "First_Name", + "not_allow_blank": None, + "force_verbose_blank": "First_Name (Blank)"}), + ) + + # Ensure no duplicate entries in user_get_full_name_test_kwargs_list + # to generate error info when subTests fail. + ids = set([kwargs["id"] for kwargs in user_get_full_name_test_kwargs_list]) + assert len(ids) == len(user_get_full_name_test_kwargs_list) + + for kwargs in user_get_full_name_test_kwargs_list: + # clear cached_property + from accounts.utils import relate_user_method_settings + relate_user_method_settings.__dict__ = {} + with self.subTest(id=kwargs["id"]): + with override_settings( + RELATE_USER_FULL_NAME_FORMAT_METHOD=kwargs[ + "custom_method"]): + check_messages = kwargs.get("check_messages", []) + self.assertCheckMessages(check_messages) + + user = UserFactory(**kwargs["user_dict"]) + self.assertEqual(user.get_full_name(), kwargs["default"]) + self.assertEqual(user.get_full_name(allow_blank=False), + kwargs["not_allow_blank"]) + self.assertEqual(user.get_full_name(force_verbose_blank=True), + kwargs["force_verbose_blank"]) + + class CheckRelateEmailAppelationPriorityList(CheckRelateSettingsBase): msg_id_prefix = "relate_email_appelation_priority_list" @@ -145,7 +305,7 @@ class CheckRelateEmailAppelationPriorityList(CheckRelateSettingsBase): @override_settings(RELATE_EMAIL_APPELATION_PRIORITY_LIST=INVALID_CONF_STR) def test_invalid_relate_email_appelation_priority_list_str(self): self.assertCheckMessages( - ["relate_email_appelation_priority_list.E002"]) + ["relate_email_appelation_priority_list.E001"]) class CheckRelateEmailConnections(CheckRelateSettingsBase): @@ -679,6 +839,25 @@ class CheckRelateTemplatesDirs(CheckRelateSettingsBase): "relate_override_templates_dirs.W001"]) +class CheckRelateCustomPageTypesRemovedDeadline(CheckRelateSettingsBase): + msg_id_prefix = "relate_custom_page_types_removed_deadline" + VALID_CONF = datetime(2017, 12, 31, 0, 0) + INVALID_CONF = "2017-12-31 00:00" + + @override_settings(RELATE_CUSTOM_PAGE_TYPES_REMOVED_DEADLINE=None) + def test_valid_conf_none(self): + self.assertCheckMessages([]) + + @override_settings(RELATE_CUSTOM_PAGE_TYPES_REMOVED_DEADLINE=VALID_CONF) + def test_valid_conf(self): + self.assertCheckMessages([]) + + @override_settings(RELATE_CUSTOM_PAGE_TYPES_REMOVED_DEADLINE=INVALID_CONF) + def test_invalid_conf(self): + self.assertCheckMessages( + ["relate_custom_page_types_removed_deadline.E001"]) + + class CheckRelateDisableCodehiliteMarkdownExtensions(CheckRelateSettingsBase): msg_id_prefix = "relate_disable_codehilite_markdown_extension" VALID_CONF = None @@ -714,22 +893,3 @@ class CheckRelateDisableCodehiliteMarkdownExtensions(CheckRelateSettingsBase): def test_warning_conf_false(self): self.assertCheckMessages( ["relate_disable_codehilite_markdown_extension.W002"]) - - -class CheckRelateCustomPageTypesRemovedDeadline(CheckRelateSettingsBase): - msg_id_prefix = "relate_custom_page_types_removed_deadline" - VALID_CONF = datetime(2017, 12, 31, 0, 0) - INVALID_CONF = "2017-12-31 00:00" - - @override_settings(RELATE_CUSTOM_PAGE_TYPES_REMOVED_DEADLINE=None) - def test_valid_conf_none(self): - self.assertCheckMessages([]) - - @override_settings(RELATE_CUSTOM_PAGE_TYPES_REMOVED_DEADLINE=VALID_CONF) - def test_valid_conf(self): - self.assertCheckMessages([]) - - @override_settings(RELATE_CUSTOM_PAGE_TYPES_REMOVED_DEADLINE=INVALID_CONF) - def test_invalid_conf(self): - self.assertCheckMessages( - ["relate_custom_page_types_removed_deadline.E001"])