Come posso migliorare questo “registro” Vista in Django?

Question

Ho un sito Django-based che permette agli utenti di registrarsi (ma richiede un amministratore per l'approvazione del conto prima di poter visualizzare alcune parti del sito). Sto basando fuori di django.contrib.auth. Richiedo agli utenti di registrarsi con un indirizzo email da un certo nome di dominio, quindi ho sovrascritto s 'UserCreationForm e save() metodi.

il clean_email()

La mia pagina di registrazione utilizza la seguente vista. Sono interessato a sentir parlare di come avrei potuto migliorare questa miglioramenti vista-code o miglioramenti di processo (o qualsiasi altra cosa, in realtà).

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

Non hai nemmeno bisogno di questo codice, ma penso che lo stile:

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

è più naturale scritto così:

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

e poi nella vostra amministrazione notificare Usa modello {{ user.id }} invece di {{ pk }}.

Ma, come ho detto, non è necessario che il codice a tutti perché si dispone già di un oggetto utente dalla chiamata a authenticate().

In generale, in Python, è considerato pratica poveri per avere il gestore di eccezioni in un blocco try / except essere vuoto. In altre parole, catturare sempre un'eccezione specifico come User.DoesNotExist per questo caso.

E 'anche scarsa pratica di avere molte linee all'interno del try parte del blocco try / except. E 'una migliore forma di codice in questo modo:

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 ...

Un finale, minore, raccomandazione è quella di non inviare l'e-mail direttamente nella vista, perché questo non scala se il vostro sito inizia a vedere le richieste di registrazione pesanti. E 'meglio aggiungere nella django-mailer app di scaricare il lavoro in una coda gestito da un altro processo.

Answer 2

Io personalmente cerco di mettere la breve ramo di un'istruzione if-else prima. Soprattutto se si torna. Questo per evitare di ottenere grandi rami in cui la sua difficile vedere che cosa finisce dove. Molti altri fanno come avete fatto e mettere un commento alla istruzione else. Ma pitone doesnt hanno sempre una fine del blocco di istruzioni - come se un modulo non è valido per voi.

Così, per esempio:

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

Per prima risposta: Sembra un diavolo di molto meglio del 95% delle domande "migliorare il mio codice"

.

C'è qualcosa in particolare non sei soddisfatto?