17890: added support for extra_context in django.contrib.admin.site.password_change #499

Closed
wants to merge 12 commits into
from

Conversation

Projects
None yet
7 participants
@stephenmuss

This pull request replaces the one at #64.

Rebased my commits to be up to date with master branch.

Alex Gaynor and others added some commits Nov 4, 2012

Fixed #18949 -- Improve performance of model_to_dict with many-to-many
When calling model_to_dict, improve performance of the generated SQL by
using values_list to determine primary keys of many to many objects. Add
a specific test for this function, test_model_to_dict_many_to_many

Thanks to brian for the original report and suggested fix.
Merge pull request #495 from aisipos/ticket_18949
Fixed #18949 -- Improve performance of model_to_dict with many-to-many
Fixed #19241 -- Improved floatformat docs
Demonstrate how to round to integers using floatformat templatetag
Bryan Veloso
Merge pull request #496 from pydanny/ticket_19241
Demonstrate how to round to integers using floatformat templatetag
Fixed #18949 -- Fix broken test interactions in ModelForms tests
A test in Model Forms test was specifically referring to a fixed
primary key, which was now being used up in a newly committed.
This has been worked around by specifying a higher primary
key.
@@ -264,7 +264,8 @@ def password_change(self, request):
url = reverse('admin:password_change_done', current_app=self.name)
defaults = {
'current_app': self.name,
- 'post_change_redirect': url
+ 'post_change_redirect': url,
+ 'extra_context': extra_context

This comment has been minimized.

Show comment Hide comment
@timgraham

timgraham Aug 13, 2013

Owner

should be extra_context or {}, (include a trailing comma)

@timgraham

timgraham Aug 13, 2013

Owner

should be extra_context or {}, (include a trailing comma)

@@ -24,6 +24,9 @@ class Admin2(admin.AdminSite):
def index(self, request, extra_context=None):
return super(Admin2, self).index(request, {'foo': '*bar*'})
+ def password_change(self, request, extra_context={'EXTRA_CONTEXT': 'this is some extra context'}):

This comment has been minimized.

Show comment Hide comment
@timgraham

timgraham Aug 13, 2013

Owner

pass the context like the index method above does. it's bad practice to include mutables (in this case a dictionary) as the default of a kwarg

@timgraham

timgraham Aug 13, 2013

Owner

pass the context like the index method above does. it's bad practice to include mutables (in this case a dictionary) as the default of a kwarg

@timgraham

This comment has been minimized.

Show comment Hide comment
@timgraham

timgraham Aug 13, 2013

Owner

As noted in the ticket, you'll need to update the docs including a ".. versionchanged:: 1.7" note as well as a mention in docs/releases/1.7.txt. Do you think you could make those updates?

Owner

timgraham commented Aug 13, 2013

As noted in the ticket, you'll need to update the docs including a ".. versionchanged:: 1.7" note as well as a mention in docs/releases/1.7.txt. Do you think you could make those updates?

@timgraham

This comment has been minimized.

Show comment Hide comment
@timgraham

timgraham Sep 18, 2013

Owner

Please open a new PR if you can update as per my comments, thanks!

Owner

timgraham commented Sep 18, 2013

Please open a new PR if you can update as per my comments, thanks!

@timgraham timgraham closed this Sep 18, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment