I don't see what makes this gross.  The function and exceptions are used in
a number of places; only in this particular instance do we need to make a
JSON message based on their content, so this feels like the right place for
that logic.


On Mon, Mar 17, 2014 at 3:53 PM, Greg Price <notifications@github.com>wrote:

> In common/djangoapps/student/views.py:
>
> > @@ -1150,11 +1154,20 @@ def create_account(request, post_override=None):
> >              return JsonResponse(js, status=400)
> >
> >      # Ok, looks like everything is legit.  Create the account.
> > -    ret = _do_create_account(post_vars)
> > -    if isinstance(ret, HttpResponse):  # if there was an error then return that
> > -        return ret
> > +    try:
> > +        with transaction.commit_on_success():
> > +            ret = _do_create_account(post_vars)
> > +    except (EmailAlreadyExists, UsernameAlreadyExists) as e:
> > +        field_name = 'email' if isinstance(e, EmailAlreadyExists) else 'username'
>
> This is kind of gross; I think it would be cleaner to have a single
> exception type that took the field name as a parameter.
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/edx/edx-platform/pull/2957/files#r10675418>
> .
>