SingleObjectTemplateResponseMixin.get_template_names does not return names stack properly.

[Chris Cogdon]

The docstring for SingleObjectTemplateResponseMixin.get_template_names() implies that a particular list of names is being returned. However, if SingleObjectTemplateResponseMixin.get_template_names() (the super) returns anything, then only the template_name will be returned.

In addition, the name obtained by using self.template_name_field is inserted at 0, implying that there is something to insert it in front of, which is impossible given the current implementation, and conflicts with the documentation.

By inference, it appears that the intended implementation is that the following names are returned, in this order:

• a template derived from the template_name_field field of the object.
• a template derived from the super (the template_name) attribute of the view
• a template derived from the object's model name or self.model's name, with the template_name_suffix appended.

Change will need some refactoring of the method, change to the docstring, and the documentation. I'll provide a pull request.

[Chris Cogdon]

Pull request: ​https://github.com/django/django/pull/5892

All tests pass. Documentation compiles.

[Chris Cogdon]

Note: This patch introduces one potential backwards incompatibility:

In the current code, if both template_name and template_field_name is set, template_field_name is always ignored. With the patch, if template_field_name is declared AND there is a non-empty value in the referenced field, the template name from the field is used in preference to template_name.

I think the patch implements the intended behaviour (or why bother trying to insert(0, name), and compare the implementation in MultipleObjectTemplateResponseMixin.get_template_names), and cases were someone has set both template_name and template_field_name is rare or non-existant, but given this possibility, we may want to defer the change to 1.10 with or without accelerated deprecation. Advise?

One possibility is we can detect when both template_name and template_field_name are defined and raise a deprecation warning, and then fix the stack in a later version. If that's preferable, I'll make the change, and would we want it deprecated for 1.10 or 1.11 ?

[Tim Graham]

As noted on the PR, I don't see what implies that template_name_field should be first? Even if the code implies some other behavior, it seems to me that the the documentation and docstring should take precedence.

[Andy Miller]

My personal feeling is the following on this is the following:

1. The docstring could be clearer about what is returned and in what order depending on what is specified on the class
2. Given how list returned from this function is used, then I believe a change could be made so that the code is purely additive to the list generated rather than reseting which is the current case if template_name is specified. This closer reflects the docstring intention.
3. Improve the error messages, specifically the re raising of the previous IncorrectlyConfigured is not correct since you can have other causes that aren't clear from the error provided.

Based on the comment from Tim that would suggest the docstring should remain despite the code implying otherwise in my reading of it.

I would suggest 3 smaller patches to deal with each of the points above which depends on there being a consensus that all 3 changes are good to do.

[Carlton Gibson]

The expectation that when self.template_name is set get_template_names() returns [self.template_name] is very established. TemplateResponseMixin, which defines this, is close to the shared behaviour of ≈all the GCBVs. Adjusting that for just for object detail views would be a strange inconsistency to add. Thus I think, the additional logic from SingleObjectTemplateResponseMixin should continue to only kick in after that, and then, yes, the docstring can be clarified to make that clearer.

Improving the error messages too sounds all for the good.

[Andy Miller]

Thanks Carlton. I'll get working on 1/2 patches to cover the docstring and clearer error message.

[Andy Miller]

​PR

[Andy Miller]

This has been split into 2 PRs

​PR #2

[Andy Miller]

So I completely misread Tim's comment on the PR, I have now closed the 2nd PR and pushed a second commit to the first PR. This is now ready for another review.

[Sarah Boyce <42296566+sarahboyce@…>]

In 0fc6d53:

Fixed #26007 -- Clarified SingleObjectTemplateResponseMixin.get_template_names() docs.

[Sarah Boyce <42296566+sarahboyce@…>]

In 3ee4c6a:

Refs #26007 -- Improved the ImproperlyConfigured error message for SingleObjectTemplateResponseMixin.get_template_names().