Convert admin views to use TemplateResponse

[Chris Adams]

A number of internal projects and several opensource projects have needed substantial amounts of boilerplate code when customizing Django admin views. ​https://github.com/toastdriven/django-haystack/blob/master/haystack/admin.py illustrates the problem nicely: to customize the search logic requires duplicating most of the standard changelist view rather than simply changing the "cl" context variable.

1.3 adds TemplateResponse which is perfect for this situation and I have a github branch which changes the contrib.auth and contrib.admin views to use TemplateResponse instead of render_to_response:

​https://github.com/acdha/django/compare/master...admin-template-response

The attached patch is a copy of the Github patch view:

​https://github.com/acdha/django/compare/master...admin-template-response.patch

[Chris Adams]

Corrected patch; set needs-documentation since there should be some indication that subclassing admin views is now a lot easier

[Jannis Leidel]

I definitely think that's a good idea but don't like we have to create RequestContexts manually to pass the current app. I suggest opening a separate ticket that adds a current_app parameter to the TemplateResponse which would pass that to the dynamically created RequestContext.

[Chris Adams]

Replying to jezdez:

I definitely think that's a good idea but don't like we have to create RequestContexts manually to pass the current app. I suggest opening a separate ticket that adds a current_app parameter to the TemplateResponse which would pass that to the dynamically created RequestContext.

Just created #15010 and corresponding branch: ​https://github.com/acdha/django/compare/master...template-response-current_app. I think this is reasonable but was debating whether it'd be better to allow for future use by accepting a dict of RequestContext parameters; that felt like overkill.

[Chris Adams]

I've updated the attached patch against #15008; it now avoids creating RequestContexts unnecessarily, which allowed some import cleanup in several cases.

​https://github.com/acdha/django/compare/master...15008-admin-template-response

[Chris Adams]

Updated the patch against current trunk and added some tests to confirm that the views are returning TemplateResponse instances

[Chris Adams]

I've updated the patch against the current trunk (note the new git branch name with the ticket number):

​https://github.com/acdha/django/compare/master...15008-admin-template-response

[Chris Adams]

Patch against r16086

[Jannis Leidel]

In [16087]:

Fixed #15008 -- Replaced all calls in the admin to render_to_response with TemplateResponses for easier customization. Thanks to Chris Adams for the initial patch.

[Luke Plant]

I'm about to fix to #16004, and I've realised this will render [16087] almost completely useless, because csrf_protect is applied to all or almost all the views affected here, and the fix will make all those views render the respective TemplateResponse objects, so they can no longer be customised. This is a necessary fix to make the admin CSRF protection (and any other CSRF protection) work in the absence of CsrfViewMiddleware - otherwise you'll get 403 errors. (I'm actually experiencing this in my tests for one project. The tests use Twill, and so do more realistic testing than any of Django's built in tests).

I should note that with my fix, none of the existing tests actually fail, because they don't test at what point the TemplateResponse is rendered.

So, what to do? #16004 is a bug, and it's a serious one that ought to have been fixed in 1.3. This ticket is a feature that has been added since 1.3. That means it loses, as far as I can see. I brought this issue up here: ​http://groups.google.com/group/django-developers/browse_thread/thread/f96e982254fbe5c3?pli=1

Can the relevant people please respond to this thread? Otherwise I will have to go ahead and fix #16004. Then the only sensible thing would be to revert [16087].

[Luke Plant]

@acdha - your admin.py looks very different from the Django's trunk ModelAdmin.changelist_view, so it's very difficult to see exactly what you are needing to change in the admin. In fact I cannot find any released version of Django that looks close to a simply copy-paste job for this.

Because of that, it's extremely difficult to analyse where the admin could be easier to customise for your needs.

However, from your description, it's possible that simply overriding the 'get_changelist' method is all you need to do. This has existed since Django 1.2.

[Chris Adams]

Replying to lukeplant:

@acdha - your admin.py looks very different from the Django's trunk ModelAdmin.changelist_view, so it's very difficult to see exactly what you are needing to change in the admin. In fact I cannot find any released version of Django that looks close to a simply copy-paste job for this.

That's not actually my code - it was convenient to reference as a public, popular app which needs to customize the admin - but I've run into many cases where it would have been extremely handy to be able to customize the template context before rendering. In this case it would be easy to refactor these classes to have something like a get_context method to make that specific need easier to handle but it feels like csrf_protect should be able to operate lazily as it would otherwise seem to make TemplateResponse useless for many cases where it would otherwise be a good solution.

Chris

[Jannis Leidel]

FTR, I've replied in the #16004 ticket already, this needs a better fix. We can easily fix the problem for 1.3.X, but for trunk we need a better story than rendering TemplateResponse useless.