¿Cómo puedo mejorar este "registro" ver en Django?

Question

Tengo una Django-basado que permite a los usuarios registrarse (pero requiere un administrador para aprobar la cuenta antes de poder ver a ciertas partes del sitio).Me estoy basando fuera de django.contrib.auth.Yo requerir a los usuarios a registrarse con una dirección de correo electrónico a partir de un determinado nombre de dominio, por lo que he anulado la UserCreationForm's save() y clean_email() métodos.

Mi página de registro, se utiliza la siguiente vista.Estoy interesado en escuchar acerca de cómo podría mejorar este punto de vista—mejoras en el código o mejoras en los procesos (o cualquier otra cosa, la verdad).

def register(request):
    if request.method == 'POST':
        form = UserCreationForm(request.POST)

        if form.is_valid():
            message = None

            form.save()

            username = form.cleaned_data['username']
            password = form.cleaned_data['password1']

            user = authenticate(username=username, password=password)

            first_name = form.cleaned_data['first_name']
            last_name = form.cleaned_data['last_name']
            email = user.email

            # If valid new user account
            if (user is not None) and (user.is_active):
                login(request, user)
                message = "<strong>Congratulations!</strong> You have been registered."

                # Send emails
                try:
                    # Admin email
                    pk = None
                    try: pk = User.objects.filter(username=username)[0].pk
                    except: pass

                    admin_email_template = loader.get_template('accounts/email_notify_admin_of_registration.txt')
                    admin_email_context = Context({
                        'first_name': first_name,
                        'last_name': last_name,
                        'username': username,
                        'email': email,
                        'pk': pk,
                    })
                    admin_email_body = admin_email_template.render(admin_email_context)
                    mail_admins("New User Registration", admin_email_body)

                    # User email
                    user_email_template = loader.get_template('accounts/email_registration_success.txt')
                    user_email_context = Context({
                        'first_name': form.cleaned_data['first_name'],
                        'username': username,
                        'password': password,
                    })
                    user_email_body = user_email_template.render(user_email_context)
                    user.email_user("Successfully Registered at example.com", user_email_body)
                except:
                    message = "There was an error sending you the confirmation email. You should still be able to login normally."
            else:
                message = "There was an error automatically logging you in. Try <a href=\"/login/\">logging in</a> manually."

            # Display success page
            return render_to_response('accounts/register_success.html', {
                    'username': username,
                    'message': message,
                },
                context_instance=RequestContext(request)
            )
    else: # If not POST
        form = UserCreationForm()

    return render_to_response('accounts/register.html', {
            'form': form,
        },
        context_instance=RequestContext(request)
    )

Answer 1

Ni siquiera necesita este código, pero creo que el estilo:

pk = None
try: pk = User.objects.filter(username=username)[0].pk
except: pass

es más natural escrita como:

try:
    user = User.objects.get(username=username)
except User.DoesNotExist:
    user = None

y luego en el administrador notificar el uso de plantilla {{ user.id }} en lugar de {{ pk }}.

Pero, como he dicho, no es necesario que el código en absoluto, porque ya tiene un objeto de usuario de su llamada a authenticate().

En general, en Python, se considera una mala práctica de tener el controlador de excepciones en un bloque try / except estar vacío. En otras palabras, siempre capturar una excepción específica como User.DoesNotExist para este caso.

También es mala práctica tener muchas líneas dentro del try parte del bloque try / except. Es mejor forma de codificar de esta manera:

try:
    ... a line of code that can generate exceptions to be handled ...
except SomeException:
    ... handle this particular exception ...
else:
    ... the rest of the code to execute if there were no exceptions ...

Una última, de menor importancia, la recomendación es no enviar el correo electrónico directamente en su opinión, porque esto no se escala si su sitio empieza a ver las solicitudes de registro pesados. Es mejor agregar en el django-anuncio publicitario aplicación para descargar el trabajo en una cola manejado por otro proceso.

Answer 2

Yo personalmente trato de poner la rama corta de una instrucción if-else en primer lugar. Sobre todo si se devuelve. Esto para evitar conseguir grandes ramas donde su difícil ver lo que termina donde. Muchos otros lo hacen como si han hecho y poner un comentario en la sentencia else. Pero tampoco pitón siempre tienen un final del bloque de sentencias - como si un formulario no es válido para usted.

Así, por ejemplo:

def register(request):
    if request.method != 'POST':
       form = UserCreationForm()
       return render_to_response('accounts/register.html', 
                                 { 'form': form, },
                                 context_instance=RequestContext(request)
                                 )

    form = UserCreationForm(request.POST)
    if not form.is_valid():
       return render_to_response('accounts/register.html', 
                                 { 'form': form, },
                                 context_instance=RequestContext(request)
                                 )
    ...

Answer 3

Primera respuesta: Se ve un diablos de mucho mejor que el 95% de las preguntas "mejorar mi código"

.

¿Hay algo en particular que no está satisfecho con?