Comment puis-je améliorer cette vue "register" dans Django?

Question

J'ai un site basé sur Django qui permet aux utilisateurs de s'inscrire (mais nécessite un administrateur pour approuver le compte avant de pouvoir voir certaines parties du site). Je me base sur django.contrib.auth. Je demande aux utilisateurs de s'inscrire avec une adresse e-mail à partir d'un certain nom de domaine, donc j'ai remplacé les méthodes UserCreationFormsave() et clean_email().

Ma page d'inscription utilise la vue suivante. Je suis intéressé à savoir comment je pourrais améliorer ces améliorations de code de vue ou de processus (ou toute autre chose, vraiment).

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

Vous ne devez même pas ce code, mais je pense que le style:

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

est écrit plus naturellement comme:

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

puis dans votre admin notify l'utilisation du modèle {{ user.id }} au lieu de {{ pk }}. Mais, comme je l'ai dit, vous n'avez pas du tout besoin de ce code parce que vous avez déjà un objet utilisateur de votre appel vers authenticate().

Généralement en Python, il est considéré comme peu pratique que le gestionnaire d'exceptions d'un bloc try/except soit vide. En d'autres termes, capturez toujours une exception spécifique telle que User.DoesNotExist pour ce cas.

Il est également peu pratique d'avoir plusieurs lignes à l'intérieur de la partie try du bloc try/except. Il est préférable de coder sous forme de cette façon:

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

Une recommandation finale, mineur, est de ne pas envoyer l'e-mail directement à votre avis, car ce ne sera pas l'échelle si votre site commence à voir les demandes d'inscription lourdes. Il vaut mieux ajouter dans l'application django-mailer pour décharger le travail dans une file d'attente gérée par un autre processus.

Answer 2

Première réponse: Il semble bien meilleur que 95% des questions "améliorer mon code".

Y a-t-il quelque chose dont vous n'êtes pas satisfait en particulier?

Answer 3

Personnellement, j'essaie de mettre en premier la branche courte d'une instruction if-else. Surtout si ça revient. Ceci pour éviter d'avoir de grandes branches où il est difficile de voir ce qui se termine où. Beaucoup d'autres font comme vous avez fait et mis un commentaire à l'autre déclaration. Mais python n'a pas toujours une instruction de fin de bloc - comme si un formulaire n'était pas valide pour vous.

Ainsi, par exemple:

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