Allow per-model template overrides for snippets

[laymonage]

Part of wagtail/rfcs#85. Incorporates #10256 to make merging easier later.

This allows per-model template overrides for snippets. It works pretty much the same way as ModelAdmin does. Copying the docs:

For all views that are used for a snippet model, Wagtail looks for templates in the following directories within your project or app, before resorting to the defaults:

1. templates/wagtailsnippets/snippets/{app_label}/{model_name}/
2. templates/wagtailsnippets/snippets/{app_label}/
3. templates/wagtailsnippets/snippets/

So, to override the template used by the IndexView for example, you could create a new index.html template and put it in one of those locations. For example, if you wanted to do this for a Shirt model in a shirts app, you could add your custom template as shirts/templates/wagtailsnippets/snippets/shirts/shirt/index.html.

For some common views, Wagtail also allows you to override the template used by either specifying the {view_name}_template_name attribute or overriding the get_{view_name}_template() method on the viewset. The following is a list of customisation points for the views:

• IndexView: index.html, index_template_name, or get_index_template()
  • For the results fragment used in AJAX responses (e.g. when searching), customise index_results.html, index_results_template_name, or get_index_results_template().
• CreateView: create.html, create_template_name, or get_create_template()
• EditView: edit.html, edit_template_name, or get_edit_template()
• DeleteView: delete.html, delete_template_name, or get_delete_template()
• HistoryView: history.html, history_template_name, or get_history_template()

---

Again, docs are still crammed together in that section, I'll try to rewrite it closer to (or during) the feature freeze.

Note: I don't know if we should support this for the chooser views as well. It would be nice to have it, but they currently use the generic templates (except results.html), and the base chooser views do not extend TemplateResponseMixin, so we cannot use get_template_names() without much refactoring. I'll leave it out of this PR for now.

Note 2: I also renamed the index view's template name from type_index.html to index.html. I think the name type_index.html is slightly misleading, since we also have an index view that lists the snippet types.

For context, pre-4.0, the snippets type listing (/admin/snippets/) uses the index.html template, while the listing view for a given model (/admin/snippets/app_label/model_name/) uses the type_index.html. I don't know what the original intention for the names was, but to me it seems like they should've been the other way around.

Then, in 4.0, I refactored the type listing view to use the generic IndexView (specifically in 4f83cbb), deleting the wagtailsnippets/snippets/index.html template and reusing the wagtailadmin/generic/index.html template instead. I didn't bother renaming type_index.html to index.html because of the potential implications. However, we've had a few releases since then, and I think this PR is a good time to rename it.

To re-introduce a simple way to override the type listing template, I made it so that the view looks for model_index.html template before resorting to self.template_name, which is the inherited wagtailadmin/generic/index.html. Using the name model_index.html instead of type_index.html as it aligns better with the view name (ModelIndexView) and avoids confusion/conflicts with the previous type_index.html template for the actual index view.

Please check the following:

• Do the tests still pass?¹
• Does the code comply with the style guide?
  • Run make lint from the Wagtail root.
• For Python changes: Have you added tests to cover the new/fixed behaviour?
• For new features: Has the documentation been updated accordingly?

Footnotes

1. Development Testing ↩

[squash-labs]

Manage this branch in Squash

Test this branch here: https://laymonagesnippets-per-model-te-ispyf.squash.io

[laymonage]

Just realised this should be self.template_name for consistency with the others. Happy to fix this, but I'll hold on in case your review is in progress @gasman.

[gasman]

Not reviewing right now - go ahead!

[laymonage]

Thanks! Should be all good to review now.

[gasman]

Maybe this ship has already sailed and I didn't notice, but I'd very much prefer it if views weren't required to be part of a viewset in order to function. It should always be possible to construct a standalone view independently of any viewset, even if there's not much use for that in practice... as I see it, viewsets are containers for configuration options that are shared between multiple views, and once they've pushed the appropriate bits of configuration to the views at construction time, those views should operate independently from there on. Otherwise, we risk ending up with view logic being spread between views and viewsets in a haphazard way.

I'd suggest that any common code shared by multiple views (such as get_template here) should be a mixin in wagtail.admin.views.generic.mixins, and mixed into the relevant views.

[laymonage]

The ship sails from this PR onwards, so as long as this hasn't been merged, there's still time to change the approach 😄

I strongly agree with your points there, and in fact I considered extracting the get_templates() into a mixin as it makes a lot of sense for this use case. However, this approach doesn't allow you to do the customisation through SnippetViewSet, so I decided to copy how ModelAdmin does it (the views have self.model_admin reference).

How do you think such method overrides could be supported? Or do we just not support it, and say that "if you need to do such customisations then you'll need to make subclasses of the view classes with your own get_templates(), and override all the foo_view_class attribute on the viewset to use those subclasses"?

Or, do we possibly add something like a SnippetViewSet.get_templates_mixin and apply the mixin dynamically with type(), e.g.

@property
def index_view_class(self):
    return type('IndexView', (self.get_templates_mixin, IndexView))

So developers can define their own mixin with their own get_templates() logic, then only swap SnippetViewSet.get_templates_mixin?

That does mean we'll need to change all the foo_view_class = ... for the affected views into a @property, though (no big deal).

Also, does this mean we don't want to implement ModelAdmin's view-specific methods like ModelAdmin.get_index_results_template() that ModelAdmin supports?

[laymonage]

OK, I managed to avoid this pattern by allowing the view's template_name to be a list. Don't know why Django's TemplateResponseMixin always forcefully wrap it in a list without checking. Even if get_template_names() returns a string (instead of a list as the docstring says it must), it still works fine as long as the response_class resolves it correctly (which SimpleTemplateResponse does by default).

Anyway, the new approach is much cleaner and lets all the logic be encapsulated within SnippetViewSet, and all the views know is that the template_name has been overridden. Using the views without the viewset should still work because they still have the default template_names, and they're only overridden through SnippetViewSet (or if the view is subclassed).

That said, removing the viewset from the views makes it impossible to override things like the get_queryset() methods on a per-request basis (like ModelAdmin supports), unless we want to pass the methods themselves to the views... 🤔

[gasman]

Very happy with this now - thanks for the updates @laymonage!

[lb-]

This is awesome. Thanks so much @laymonage - great to see one of my favourite ModelAdmin features come across and with a really well documented implementation.