[Django] #26007: SingleObjectTemplateResponseMixin.get_template_names does not return names stack properly.

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.
-------------------------------+--------------------
Reporter: chmarr | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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.

--
Ticket URL: <https://code.djangoproject.com/ticket/26007>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+--------------------------------------
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+--------------------------------------
Changes (by chmarr):

* owner: nobody => chmarr
* status: new => assigned
* needs_docs: => 0
* needs_tests: => 0
* needs_better_patch: => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:1>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+------------------------------------

Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+------------------------------------
Changes (by chmarr):

* has_patch: 0 => 1
* stage: Unreviewed => Accepted

Comment:

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

All tests pass. Documentation compiles.

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:2>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+------------------------------------
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+------------------------------------

Comment (by chmarr):

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 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?

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:3>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+------------------------------------
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------+------------------------------------
Changes (by timgraham):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:4>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+------------------------------------
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------+------------------------------------

Comment (by timgraham):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:5>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+----------------------------------------
Reporter: Chris Cogdon | Owner: Chris Cogdon
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------+----------------------------------------
Changes (by Andy Miller):

* cc: Andy Miller (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:6>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+---------------------------------------
Reporter: Chris Cogdon | Owner: Andy Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------+---------------------------------------
Changes (by Andy Miller):

* owner: Chris Cogdon => Andy Miller

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:7>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+---------------------------------------
Reporter: Chris Cogdon | Owner: Andy Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------+---------------------------------------
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:8>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+---------------------------------------
Reporter: Chris Cogdon | Owner: Andy Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------+---------------------------------------
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:9>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+---------------------------------------
Reporter: Chris Cogdon | Owner: Andy Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------+---------------------------------------
Comment (by Andy Miller):

Thanks Carlton. I'll get working on 1/2 patches to cover the docstring and
clearer error message.
--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:10>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+---------------------------------------
Reporter: Chris Cogdon | Owner: Andy Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+---------------------------------------
Changes (by Andy Miller):

* cc: Andy Miller (removed)
* needs_better_patch: 1 => 0

Comment:

[https://github.com/django/django/pull/18923 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:11>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+---------------------------------------
Reporter: Chris Cogdon | Owner: Andy Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------+---------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:12>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+---------------------------------------
Reporter: Chris Cogdon | Owner: Andy Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+---------------------------------------
Changes (by Andy Miller):

* needs_better_patch: 1 => 0

Comment:

This has been split into 2 PRs

[https://github.com/django/django/pull/18932 PR #2]
--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:13>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------+---------------------------------------
Reporter: Chris Cogdon | Owner: Andy Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+---------------------------------------
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:14>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------------+-------------------------------------

Reporter: Chris Cogdon | Owner: Andy
| Miller
Type: Bug | Status: assigned

Component: Generic views | Version: 1.9

Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:15>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------------+-------------------------------------
Reporter: Chris Cogdon | Owner: Andy
| Miller

Type: Bug | Status: closed

Component: Generic views | Version: 1.9

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"0fc6d5362bfed3e2222fa3476139126ee08a4ff9" 0fc6d53]:
{{{#!CommitTicketReference repository=""
revision="0fc6d5362bfed3e2222fa3476139126ee08a4ff9"
Fixed #26007 -- Clarified
SingleObjectTemplateResponseMixin.get_template_names() docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:16>

[Django]

#26007: SingleObjectTemplateResponseMixin.get_template_names does not return names
stack properly.

-------------------------------------+-------------------------------------
Reporter: Chris Cogdon | Owner: Andy
| Miller
Type: Bug | Status: closed

Component: Generic views | Version: 1.9

Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"3ee4c6a27ad520d4ecc3cded260d47cbccafb144" 3ee4c6a]:
{{{#!CommitTicketReference repository=""
revision="3ee4c6a27ad520d4ecc3cded260d47cbccafb144"
Refs #26007 -- Improved the ImproperlyConfigured error message for
SingleObjectTemplateResponseMixin.get_template_names().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26007#comment:17>