#24944 closed New feature (fixed)
Have password_reset pass extra_context to the email template rendering as well
Reported by: | twelveeighty | Owned by: | Sujay S Kumar |
---|---|---|---|
Component: | contrib.auth | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | sujay.skumar141295@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
The password_reset function in auth.views currently allows for extra context parameters to be passed to the rendered reset password form, but it does not pass through those extra context parameters to the 'opt' dictionary used for the email template. This should be an easy feature to add: just before the form.save(), add the extra_context to the opts dictionary:
# inside the request.method == 'POST' and form.isValid() block: opts = { ... } if extra_context is not None: opts.update(extra_context) form.save(**opts)
Change History (15)
comment:1 by , 9 years ago
Component: | Uncategorized → contrib.auth |
---|---|
Has patch: | unset |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 9 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:3 by , 9 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Patch needs improvement: | set |
Resolution: | → fixed |
Status: | assigned → closed |
The link to the github repo can be found here:
https://github.com/SujaySKumar/django/tree/ticket_24944
follow-up: 6 comment:4 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
A ticket isn't marked fixed until the patch is committed to Django. Also your patch is missing tests and documentation as outlined in our patch review checklist. Also your patch has a problem if someone is using extra_context
with a key that clashes with one in opts
as I mentioned in comment 1.
comment:5 by , 9 years ago
Needs tests: | set |
---|
comment:6 by , 9 years ago
Replying to timgraham:
A ticket isn't marked fixed until the patch is committed to Django. Also your patch is missing tests and documentation as outlined in our patch review checklist. Also your patch has a problem if someone is using
extra_context
with a key that clashes with one inopts
as I mentioned in comment 1.
I am new to contributing to open source. I am not able to understand where to put the documentation for this new feature. I would be grateful if you could point to the file in which I should add the documentation.
comment:7 by , 9 years ago
The password reset function is documented in docs/topics/auth/default.txt
.
comment:8 by , 9 years ago
Status: | new → assigned |
---|
comment:9 by , 9 years ago
Needs documentation: | unset |
---|
comment:10 by , 9 years ago
Patch needs improvement: | unset |
---|
comment:11 by , 9 years ago
Needs tests: | unset |
---|
comment:12 by , 9 years ago
Patch needs improvement: | set |
---|
Left some trivial review comments. Tim also reviewed the pull request yesterday.
comment:13 by , 9 years ago
Patch needs improvement: | unset |
---|
I guess it's unlikely, but we'd have a small backwards compatibility issue if someone is using
extra_context
with a key that clashes with one inopts
. For that reason, it might be better to use a separate parameter.There is also the idea of converting these views to class-based views in #17209 which might help here.