Opened 10 years ago

Last modified 21 months ago

#22752 new Cleanup/optimization

Allow PasswordResetForm email to render URLs based on the current namespace

Reported by: Ben Davis Owned by: nobody
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: Ben Davis Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have multiple namespace instances for password reset urls. The email template rendered by the default PasswordResetForm does not included a current_app context. It's an easy fix, though, ass the PasswordResetForm already has self.current_app. Just need to wrap the context instance passed to the email template:

  • django/contrib/auth/forms.py

    diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py
    index 6e07d45..baef873 100644
    a b from collections import OrderedDict  
    44
    55from django import forms
    66from django.forms.utils import flatatt
    7 from django.template import loader
     7from django.template import loader, Context
    88from django.utils.encoding import force_bytes
    99from django.utils.html import format_html, format_html_join
    1010from django.utils.http import urlsafe_base64_encode
    class PasswordResetForm(forms.Form):  
    264264                'token': token_generator.make_token(user),
    265265                'protocol': 'https' if use_https else 'http',
    266266            }
    267             subject = loader.render_to_string(subject_template_name, c)
     267            context_instance = Context(current_app=self.current_app)
     268            subject = loader.render_to_string(
     269                subject_template_name, c, context_instance)
    268270            # Email subject *must not* contain newlines
    269271            subject = ''.join(subject.splitlines())
    270             email = loader.render_to_string(email_template_name, c)
     272            email = loader.render_to_string(
     273                email_template_name, c, context_instance)
    271274
    272275            if html_email_template_name:
    273276                html_email = loader.render_to_string(html_email_template_name, c)

Change History (6)

comment:1 by Ben Davis, 10 years ago

Correction, PasswordResetForm does not have current_app built in. That needs to be added as well.

comment:2 by Daniele Procida, 10 years ago

Easy pickings: unset

comment:3 by Baptiste Mispelon, 10 years ago

Has patch: set
Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Hi,

I agree that it would be good to have the current app while rendering the body of the email because it would help with reversing URLs inside it.
However, I don't really understand the use-case behind having the current app when rendering the subject line.

In any case, the provided patch doesn't apply on master and it's also going to require tests and documentation.

Thanks.

comment:4 by Ben Davis, 10 years ago

@bmispelon, I think one would expect the context to be identical in both the body and the subject. Whether or not there's a use case, I can't think of any justification for making them separate. I plan on working on this (as well as various other tickets I've submitted) once I get a little more free time.

comment:5 by Ben Davis, 10 years ago

Cc: Ben Davis added

comment:7 by Tim Graham, 3 years ago

Has patch: unset
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Summary: PasswordResetForm email context is missing current_appAllow PasswordResetForm email to render URLs based on the current namespace

current_app is no longer an argument to render_to_string() but I think this would be resolved by passing request instead (which can be passed to send_mail() from save()). The url template tag uses the namespace of the currently resolved view as the current application in a RequestContext.

Note: See TracTickets for help on using tickets.
Back to Top