From c3e84157e7d7efe5fa183928ae44e753b73f8a0e Mon Sep 17 00:00:00 2001
From: dzhuang <dzhuang.scut@gmail.com>
Date: Wed, 28 Feb 2018 18:26:34 +0800
Subject: [PATCH] For email appelation full_name to skip None first/last name.
---
accounts/models.py | 31 +---
accounts/utils.py | 219 +++++++++++++++++++----------
course/constants.py | 2 +
relate/checks.py | 18 +--
tests/test_accounts/test_models.py | 87 +++++++++++-
tests/test_checks.py | 9 +-
6 files changed, 244 insertions(+), 122 deletions(-)
diff --git a/accounts/models.py b/accounts/models.py
index 85909e95..cf07b8a0 100644
--- a/accounts/models.py
+++ b/accounts/models.py
@@ -151,7 +151,7 @@ class User(AbstractBaseUser, PermissionsMixin):
verbose_blank(first_name), verbose_blank(last_name))
from accounts.utils import relate_user_method_settings
- format_method = relate_user_method_settings.get_custom_full_name_method
+ format_method = relate_user_method_settings.custom_full_name_method
if format_method is None:
format_method = default_fullname
@@ -186,33 +186,14 @@ class User(AbstractBaseUser, PermissionsMixin):
def get_email_appellation(self):
"Return the appellation of the receiver in email."
- from django.conf import settings
-
- # import the user defined priority list
- customized_priority_list = getattr(
- 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
- for e in customized_priority_list:
- if e in ["first_name", "email", "username", "full_name"]:
- priority_list.append(e)
-
- # make sure the default appellations are included in case
- # user defined appellations are not available.
- for e in ["first_name", "email", "username"]:
- if e not in priority_list:
- priority_list.append(e)
+ from accounts.utils import relate_user_method_settings
+ priority_list = (
+ relate_user_method_settings.email_appelation_priority_list)
for attr in priority_list:
if attr == "full_name":
- appellation = self.get_full_name(allow_blank=True)
+ appellation = self.get_full_name(allow_blank=False)
else:
appellation = getattr(self, attr)
@@ -221,8 +202,6 @@ class User(AbstractBaseUser, PermissionsMixin):
else:
continue
- return _("user")
-
def clean(self):
super(User, self).clean()
diff --git a/accounts/utils.py b/accounts/utils.py
index 6100f072..77f73e26 100644
--- a/accounts/utils.py
+++ b/accounts/utils.py
@@ -29,7 +29,11 @@ from django.core.checks import Warning
from django.utils.functional import cached_property
from django.utils.module_loading import import_string
+from relate.checks import INSTANCE_ERROR_PATTERN
+from course.constants import DEFAULT_EMAIL_APPELATION_PRIORITY_LIST
+
RELATE_USER_FULL_NAME_FORMAT_METHOD = "RELATE_USER_FULL_NAME_FORMAT_METHOD"
+RELATE_EMAIL_APPELATION_PRIORITY_LIST = "RELATE_EMAIL_APPELATION_PRIORITY_LIST"
class RelateUserMethodSettingsInitializer(object):
@@ -40,109 +44,172 @@ class RelateUserMethodSettingsInitializer(object):
def __init__(self):
self._custom_full_name_method = None
+ self._email_appelation_priority_list = (
+ DEFAULT_EMAIL_APPELATION_PRIORITY_LIST)
@cached_property
- def get_custom_full_name_method(self):
+ def custom_full_name_method(self):
self.check_custom_full_name_method()
return self._custom_full_name_method
+ @cached_property
+ def email_appelation_priority_list(self):
+ self.check_email_appelation_priority_list()
+ return self._email_appelation_priority_list
+
+ def check_email_appelation_priority_list(self):
+ errors = []
+ self._email_appelation_priority_list = (
+ DEFAULT_EMAIL_APPELATION_PRIORITY_LIST)
+
+ from django.conf import settings
+ custom_email_appelation_priority_list = getattr(
+ settings, RELATE_EMAIL_APPELATION_PRIORITY_LIST, None)
+ if not custom_email_appelation_priority_list:
+ return errors
+
+ if not isinstance(custom_email_appelation_priority_list, (list, tuple)):
+ errors.append(Warning(
+ msg=("%s, %s" % (
+ INSTANCE_ERROR_PATTERN
+ % {"location": RELATE_EMAIL_APPELATION_PRIORITY_LIST,
+ "types": "list or tuple"},
+ "default value '%s' will be used"
+ % repr(DEFAULT_EMAIL_APPELATION_PRIORITY_LIST))),
+ id="relate_email_appelation_priority_list.W001"))
+ return errors
+
+ priority_list = []
+ not_supported_appels = []
+
+ # filter out not allowd appellations in customized list
+ for appel in custom_email_appelation_priority_list:
+ if appel in DEFAULT_EMAIL_APPELATION_PRIORITY_LIST:
+ priority_list.append(appel)
+ else:
+ not_supported_appels.append(appel)
+
+ # make sure the default appellations are included in case
+ # user defined appellations are not available.
+ for appel in DEFAULT_EMAIL_APPELATION_PRIORITY_LIST:
+ if appel not in priority_list:
+ priority_list.append(appel)
+
+ assert len(priority_list)
+ self._email_appelation_priority_list = priority_list
+
+ if not_supported_appels:
+ errors.append(Warning(
+ msg=("%(location)s: not supported email appelation(s) found "
+ "and will be ignored: %(not_supported_appelds)s. "
+ "%(actual)s will be used as "
+ "RELATE_EMAIL_APPELATION_PRIORITY_LIST."
+ % {"location": RELATE_EMAIL_APPELATION_PRIORITY_LIST,
+ "not_supported_appelds": ", ".join(not_supported_appels),
+ "actual": repr(priority_list)}),
+ id="relate_email_appelation_priority_list.W002"))
+ return errors
+
def check_custom_full_name_method(self):
+ self._custom_full_name_method = None
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(
+
+ if relate_user_full_name_format_method is None:
+ return errors
+
+ 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` is not a callable, "
+ "%(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.W002"
+ 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:
- try:
- returned_name = (
- relate_user_full_name_format_method("first_name",
- "last_name"))
- except Exception as e:
- from traceback import format_exc
+ 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` 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"
+ 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:
- 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:
+ 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 a non-empty string, got `%(result)s`, "
- "default format method will be used."
+ "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,
- "result": unexpected_return_value,
+ "location":
+ RELATE_USER_FULL_NAME_FORMAT_METHOD,
+ "method":
+ relate_user_full_name_format_method
}),
- id="relate_user_full_name_format_method.W004"
+ id="relate_user_full_name_format_method.W005"
))
- 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
diff --git a/course/constants.py b/course/constants.py
index e4207e42..9b53876e 100644
--- a/course/constants.py
+++ b/course/constants.py
@@ -30,6 +30,8 @@ if False:
from django.utils.translation import pgettext_lazy, ugettext
# Allow 10x extra credit at the very most.
MAX_EXTRA_CREDIT_FACTOR = 10
+DEFAULT_EMAIL_APPELATION_PRIORITY_LIST = [
+ "first_name", "email", "username", "full_name"]
COURSE_ID_REGEX = "(?P<course_identifier>[-a-zA-Z0-9]+)"
diff --git a/relate/checks.py b/relate/checks.py
index 4e76c63a..c18662ec 100644
--- a/relate/checks.py
+++ b/relate/checks.py
@@ -101,22 +101,12 @@ def check_relate_settings(app_configs, **kwargs):
))
# }}}
- # {{{ check RELATE_EMAIL_APPELATION_PRIORITY_LIST
- relate_email_appelation_priority_list = getattr(
- settings, RELATE_EMAIL_APPELATION_PRIORITY_LIST, None)
- if relate_email_appelation_priority_list is not None:
- if not isinstance(relate_email_appelation_priority_list, (list, tuple)):
- errors.append(RelateCriticalCheckMessage(
- msg=(
- INSTANCE_ERROR_PATTERN
- % {"location": RELATE_EMAIL_APPELATION_PRIORITY_LIST,
- "types": "list or tuple"}),
- id="relate_email_appelation_priority_list.E001")
- )
- # }}}
+ from accounts.utils import relate_user_method_settings
+ # check RELATE_EMAIL_APPELATION_PRIORITY_LIST
+ errors.extend(
+ relate_user_method_settings.check_email_appelation_priority_list())
# 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
diff --git a/tests/test_accounts/test_models.py b/tests/test_accounts/test_models.py
index 09066fd1..761b2173 100644
--- a/tests/test_accounts/test_models.py
+++ b/tests/test_accounts/test_models.py
@@ -84,7 +84,7 @@ class UserModelTest(TestCase):
"tests.resource.my_customized_get_full_name_method")
get_custom_full_name_method_path = (
"accounts.utils.RelateUserMethodSettingsInitializer"
- ".get_custom_full_name_method")
+ ".custom_full_name_method")
with override_settings(
RELATE_USER_FULL_NAME_FORMAT_METHOD=custom_get_full_name_path):
@@ -109,13 +109,13 @@ class UserModelTest(TestCase):
def test_custom_get_full_name_method_is_cached(self):
"""
- Test relate_user_method_settings.get_custom_full_name_method is cached.
+ Test relate_user_method_settings.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 = (
+ custom_full_name_check_path = (
"accounts.utils.RelateUserMethodSettingsInitializer"
".check_custom_full_name_method")
@@ -128,6 +128,85 @@ class UserModelTest(TestCase):
user.get_full_name()
- with mock.patch(get_custom_full_name_check_path) as mock_check:
+ with mock.patch(custom_full_name_check_path) as mock_check:
user.get_full_name()
self.assertEqual(mock_check.call_count, 0)
+
+ def test_get_email_appellation_priority_list(self):
+ user = UserFactory.create(first_name="my_first", last_name="my_last")
+
+ from accounts.utils import relate_user_method_settings
+ relate_user_method_settings.__dict__ = {}
+
+ with override_settings(
+ RELATE_USER_FULL_NAME_FORMAT_METHOD=None):
+ self.assertEqual(user.get_email_appellation(), "my_first")
+
+ relate_user_method_settings.__dict__ = {}
+
+ with override_settings(
+ RELATE_EMAIL_APPELATION_PRIORITY_LIST=[""]):
+ relate_user_method_settings.__dict__ = {}
+ self.assertEqual(user.get_email_appellation(), "my_first")
+
+ relate_user_method_settings.__dict__ = {}
+
+ with override_settings(
+ RELATE_EMAIL_APPELATION_PRIORITY_LIST=["whatever"]):
+ self.assertEqual(user.get_email_appellation(), "my_first")
+
+ relate_user_method_settings.__dict__ = {}
+
+ # not a list
+ with override_settings(
+ RELATE_EMAIL_APPELATION_PRIORITY_LIST="whatever"):
+ self.assertEqual(user.get_email_appellation(), "my_first")
+
+ relate_user_method_settings.__dict__ = {}
+
+ with override_settings(
+ RELATE_EMAIL_APPELATION_PRIORITY_LIST=["full_name"],
+ RELATE_USER_FULL_NAME_FORMAT_METHOD=None):
+ self.assertEqual(user.get_email_appellation(), "my_first my_last")
+
+ # create a user without first_name
+ user = UserFactory.create(last_name="my_last")
+
+ relate_user_method_settings.__dict__ = {}
+
+ # the next appelation is email
+ with override_settings(
+ RELATE_USER_FULL_NAME_FORMAT_METHOD=None):
+ self.assertEqual(user.get_email_appellation(), user.email)
+
+ relate_user_method_settings.__dict__ = {}
+
+ with override_settings(
+ RELATE_EMAIL_APPELATION_PRIORITY_LIST=["full_name", "username"],
+ RELATE_USER_FULL_NAME_FORMAT_METHOD=None):
+ # because full_name is None
+ self.assertEqual(user.get_email_appellation(), user.username)
+
+ def test_get_email_appellation_priority_list_is_cached(self):
+ """
+ Test relate_user_method_settings.email_appelation_priority_list is cached.
+ """
+ user = UserFactory.create(first_name="my_first", last_name="my_last")
+
+ email_appel_priority_list_check_path = (
+ "accounts.utils.RelateUserMethodSettingsInitializer"
+ ".check_email_appelation_priority_list")
+
+ from accounts.utils import relate_user_method_settings
+ relate_user_method_settings.__dict__ = {}
+
+ with override_settings(
+ RELATE_EMAIL_APPELATION_PRIORITY_LIST=["full_name", "username"],
+ RELATE_USER_FULL_NAME_FORMAT_METHOD=None):
+ self.assertEqual(user.get_email_appellation(), "my_first my_last")
+
+ user2 = UserFactory.create(first_name="my_first")
+ with mock.patch(email_appel_priority_list_check_path) as mock_check:
+ self.assertEqual(user2.get_email_appellation(),
+ user2.username)
+ self.assertEqual(mock_check.call_count, 0)
diff --git a/tests/test_checks.py b/tests/test_checks.py
index 338af0ea..c67bfe52 100644
--- a/tests/test_checks.py
+++ b/tests/test_checks.py
@@ -291,8 +291,9 @@ class CheckRelateEmailAppelationPriorityList(CheckRelateSettingsBase):
msg_id_prefix = "relate_email_appelation_priority_list"
VALID_CONF_NONE = None
- VALID_CONF = ["name1", "name2"]
+ VALID_CONF = ["full_name"]
INVALID_CONF_STR = "name1"
+ INVALID_CONF = ["name1", "name2"]
@override_settings(RELATE_EMAIL_APPELATION_PRIORITY_LIST=VALID_CONF_NONE)
def test_valid_relate_email_appelation_priority_list_none(self):
@@ -305,7 +306,11 @@ 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.E001"])
+ ["relate_email_appelation_priority_list.W001"])
+
+ @override_settings(RELATE_EMAIL_APPELATION_PRIORITY_LIST=INVALID_CONF)
+ def test_valid_relate_email_appelation_priority_list_invalid(self):
+ self.assertCheckMessages(["relate_email_appelation_priority_list.W002"])
class CheckRelateEmailConnections(CheckRelateSettingsBase):
--
GitLab