From 52197fba88c122eae2cfc4c062a12cda671e6959 Mon Sep 17 00:00:00 2001 From: Po Tsui Date: Thu, 13 Jul 2017 11:08:15 -0700 Subject: [PATCH] Add TODO for duplicate email/username inconsistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function `create_account_with_params` calls `_do_create_account`, which exhibits some discrepant behavior with throwing errors when handling duplicate email and/or username. A duplicate email raises a `ValidationError` (rather than the expected `AccountValidationError`) from the first part of `_do_create_account`, when errors from `form` (the `AccountCreationForm`) are raised. A duplicate username raises the expected `AccountValidationError`, but from a later part of `_do_create_account`. As a result, registering with both duplicate username and email raises a `ValidationError` for email only. The user message for username is “An account with the Public Username '{username}' already exists.” which differs from that of email, “It looks like {email} belongs to an existing account. Try again with a different email." The latter is more consistent with other user messages. --- common/djangoapps/student/views.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 56b98ebf73..4519a42801 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1728,6 +1728,11 @@ def _do_create_account(form, custom_form=None, site=None): UserAttribute.set_user_attribute(user, 'created_on_site', site.domain) except IntegrityError: # Figure out the cause of the integrity error + # TODO duplicate email is already handled by form.errors above as a ValidationError. + # The checks for duplicate email/username should occur in the same place with an + # AccountValidationError and a consistent user message returned (i.e. both should + # return "It looks like {username} belongs to an existing account. Try again with a + # different username.") if len(User.objects.filter(username=user.username)) > 0: raise AccountValidationError( _("An account with the Public Username '{username}' already exists.").format(username=user.username), @@ -1794,6 +1799,14 @@ def create_account_with_params(request, params): * The user-facing text is rather unfriendly (e.g. "Username must be a minimum of two characters long" rather than "Please use a username of at least two characters"). + * Duplicate email raises a ValidationError (rather than the expected + AccountValidationError). Duplicate username returns an inconsistent + user message (i.e. "An account with the Public Username '{username}' + already exists." rather than "It looks like {username} belongs to an + existing account. Try again with a different username.") The two checks + occur at different places in the code; as a result, registering with + both a duplicate username and email raises only a ValidationError for + email only. """ # Copy params so we can modify it; we can't just do dict(params) because if # params is request.POST, that results in a dict containing lists of values