Update admin views to consistently return TemplateContext

[ababic]

As explained in the Django docs, when using TemplateResponse, the rendering of the template is delayed until necessary, and middleware and view decorators have the opportunity to make changes before rendering happens.

Specifically, the process_template_response() method on custom middleware gets to do its thing (which doesn't happen when using the render() shortcut), giving developers an important and convenient hook to make certain types of customisation (that might otherwise require replacing entire function-based views or monkey-patching).

Most class-based views in Wagtail subclass Django's TemplateResponseMixin somewhere along the line, and so already return a TemplateResponse - So these changes also greatly improve consistency.

While implementing these changes, I also noticed an issue that seems not to have been reported yet: The wagtail.auth.require_admin_access decorator isn't working quite as expected in many cases: Specifically, where views return a TemplateResponse. Because template rendering is delayed, it happens outside the translation.override() context manager, meaning the language is only active while view logic is run - the template itself is still rendered in the default language. I've fixed this here by invoking the response's render() method if it has one - which means the language override is fully applied.

[ncastro-nypr]

@ababic, I'm not understanding why it's ok to just swap render() calls with a TemplateResponse(). because they seem to return different things. I think perhaps I'm not understanding what you meant when you said

Most class-based views in Wagtail subclass Django's TemplateResponseMixin somewhere along the line, and so already return a TemplateResponse - So these changes also greatly improve consistency

[ababic]

Hi @ncastro-nypr. You might find this answer on stackoverflow helpful: https://stackoverflow.com/questions/38838601/django-templateresponse-vs-render

[ncastro-nypr]

I see, thank you! This all looks good to me then.

[mike-hearn]

I'll be honest I don't fully understand the benefits of this (I get that it runs custom middleware, but I guess other than translation I don't know what we use that for), but I'm all for consistency with the Wagtail codebase. That being said, it looks like a lot of these updates are actually taking us further away from Wagtail core (which at least on master still frequently uses the render() call) - are these changes being put into Wagtail core as well?

Aside from that, is this intended to replace every render()? I ask because there are a couple of remaining renders in the code base - two in wagtail/admin/views/userbar.py, one in wagtail/snippets/views/chooser.py, and another in wagtail/contrib/styleguide/views.py.

[ababic]

Thanks for the feedback @mike-hearn. I've now replaced those last few uses of render() for consistency.

To clarify, this is the NYPR equivalent of a PR that I've submitted to upstream Wagtail, which we should hopefully be able to get reviewed/merged soon:
wagtail#5731

I wanted to utilise the process_template_response() middeware method to make some updates to context on a few Wagtail views as part of this multi-site work we're doing. For example, it would be ideal for hiding the site switcher from non superusers (I've overridden the template for now, which isn't quite as nice).