[Django] #24128: Admindocs doesn't account for template loaders
Django
---------------------------------------------+------------------------
Reporter: aaugustin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
`TemplateDetailView` only considers `dirs` and assumes that the
`filesystem` loader is enabled. It doesn't account for other loaders such
as `app_directories`.
The code changed a bit during the multiple-template-engines refactor but
this bug existed before and still exists.
--
Ticket URL: <https://code.djangoproject.com/ticket/24128>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: aaugustin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:1>
Django
Reporter: Aymeric Augustin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Kris Avi):
* cc: Kris Avi (added)
Comment:
It seems it is quite fixable with something like that:
That is probably not the most elegant solution. Tried to get loaders from
all the engines and combine their location directories into one to send to
enumerator.
{{{
...
pass
else:
# This doesn't account for template loaders (#24128).
# Fix for #24128
from django.template import engines
directories = set(default_engine.dirs) #
making set with default engine dirs parameter as initial set
for engine in engines.all(): #
going through all the engines
for loader in engine.engine.template_loaders: #
getting each template loader from engine
directories.update(loader.get_dirs()) #
updating set with new directories from loaders
for index, directory in enumerate(directories): #
new enumerator with all dirs from default and loaders
# for index, directory in enumerate(default_engine.dirs): #
commented out original enumerator
# End of fix for #24128
template_file = os.path.join(directory, template)
templates.append({
...
}}}
instead of
{{{
...
pass
else:
# This doesn't account for template loaders (#24128).
for index, directory in enumerate(default_engine.dirs):
template_file = os.path.join(directory, template)
templates.append({
...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:2>
Django
Reporter: Aymeric Augustin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/7434 PR] without tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:3>
Django
Reporter: Aymeric Augustin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:4>
Django
Reporter: Aymeric Augustin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:5>
Django
Reporter: Aymeric Augustin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Gunnar Ahlberg):
I tested this patch. It works nicely for me
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:6>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Type: Bug | Status: assigned
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Alexander Lazarević):
* owner: nobody => Alexander Lazarević
* status: new => assigned
Comment:
I applied the patch and all tests pass on my machine.
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:7>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Type: Bug | Status: assigned
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Alexander Lazarević):
As an example I get the following for
`http://localhost:8000/admin/doc/templates/base.html/` without the patch:
{{{
Template: "base.html"
Search path for template "base.html":
/home/laza/Projects/Django/empty/templates/base.html
}}}
and with the patch:
{{{
Template: "base.html"
Search path for template "base.html":
/home/laza/Projects/Django/django/django/contrib/admin/templates/base.html
(does not exist)
/home/laza/Projects/Django/empty/templates/base.html
/home/laza/Projects/Django/django/django/contrib/admindocs/templates/base.html
(does not exist)
/home/laza/Projects/Django/django/django/contrib/auth/templates/base.html
(does not exist)
}}}
So that's what the fix is about? Right now I'm not sure how to write a
test for this.
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:8>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Type: Bug | Status: assigned
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Alexander Lazarević):
Just saw that there is also an old PR for the ticket:
https://github.com/django/django/pull/7434
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:9>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Type: Bug | Status: assigned
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Alexander Lazarević):
I created a new (draft) PR: https://github.com/django/django/pull/17725
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:10>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Type: Bug | Status: assigned
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sarah Abderemane):
* cc: Sarah Abderemane (added)
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:11>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Type: Bug | Status: assigned
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:12>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Type: Bug | Status: assigned
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Alexander Lazarević):
* needs_better_patch: 1 => 0
Comment:
I applied the recommendations on the PR
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:13>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Type: Bug | Status: assigned
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution:
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:14>
Django
Reporter: Aymeric Augustin | Owner: Alexander
| Lazarević
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b7154f811f0c01757346f88ee006c8f2034acee3" b7154f8]:
{{{#!CommitTicketReference repository=""
revision="b7154f811f0c01757346f88ee006c8f2034acee3"
Fixed #24128 -- Made admindocs TemplateDetailView respect
template_loaders.
Co-Authored-By: Author: Alexander Lazarević <la...@e11bits.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24128#comment:15>