Opened 9 years ago

Closed 9 years ago

Last modified 5 years ago

#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 Tim Graham, 9 years ago

Component: Uncategorizedcontrib.auth
Has patch: unset
Triage Stage: UnreviewedAccepted

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

comment:2 by Sujay S Kumar, 9 years ago

Cc: sujay.skumar141295@… added
Owner: changed from nobody to Sujay S Kumar
Status: newassigned

comment:3 by Sujay S Kumar, 9 years ago

Has patch: set
Needs documentation: set
Patch needs improvement: set
Resolution: fixed
Status: assignedclosed

The link to the github repo can be found here:
https://github.com/SujaySKumar/django/tree/ticket_24944

Last edited 9 years ago by Sujay S Kumar (previous) (diff)

comment:4 by Tim Graham, 9 years ago

Resolution: fixed
Status: closednew

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 Sujay S Kumar, 9 years ago

Needs tests: set

in reply to:  4 comment:6 by Sujay S Kumar, 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 in opts 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 Tim Graham, 9 years ago

The password reset function is documented in docs/topics/auth/default.txt.

comment:8 by Sujay S Kumar, 9 years ago

Status: newassigned

comment:9 by Sujay S Kumar, 9 years ago

Needs documentation: unset

comment:10 by Sujay S Kumar, 9 years ago

Patch needs improvement: unset

comment:11 by Sujay S Kumar, 9 years ago

Needs tests: unset

comment:12 by Berker Peksag, 9 years ago

Patch needs improvement: set

Left some trivial review comments. Tim also reviewed the pull request yesterday.

comment:13 by Sujay S Kumar, 9 years ago

Patch needs improvement: unset

comment:14 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In d8d85337:

Fixed #24944 -- Added extra_email_context parameter to password_reset() view.

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In aff61790:

Refs #24944 -- Added test for overriding domain in email context in PasswordResetView.

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