Opened 9 years ago

Closed 7 weeks ago

#24128 closed Bug (fixed)

Admindocs doesn't account for template loaders

Reported by: Aymeric Augustin Owned by: Alexander Lazarević
Component: contrib.admindocs Version: dev
Severity: Normal Keywords:
Cc: Kris Avi, Sarah Abderemane Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (15)

comment:1 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Kris Avi, 7 years ago

Cc: Kris Avi added

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({
                ...

comment:3 by Tim Graham, 7 years ago

Has patch: set
Needs tests: set

PR without tests.

comment:4 by Tim Graham, 7 years ago

Needs tests: unset

comment:5 by Tim Graham, 7 years ago

Patch needs improvement: set

comment:6 by Gunnar Ahlberg, 6 years ago

I tested this patch. It works nicely for me

comment:7 by Alexander Lazarević, 2 months ago

Owner: changed from nobody to Alexander Lazarević
Status: newassigned

I applied the patch and all tests pass on my machine.

comment:8 by Alexander Lazarević, 2 months ago

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.

comment:9 by Alexander Lazarević, 2 months ago

Just saw that there is also an old PR for the ticket: https://github.com/django/django/pull/7434

comment:10 by Alexander Lazarević, 2 months ago

Last edited 2 months ago by Alexander Lazarević (previous) (diff)

comment:11 by Sarah Abderemane, 8 weeks ago

Cc: Sarah Abderemane added
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:12 by Mariusz Felisiak, 8 weeks ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

comment:13 by Alexander Lazarević, 8 weeks ago

Patch needs improvement: unset

I applied the recommendations on the PR

comment:14 by Mariusz Felisiak, 7 weeks ago

Triage Stage: AcceptedReady for checkin

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 7 weeks ago

Resolution: fixed
Status: assignedclosed

In b7154f8:

Fixed #24128 -- Made admindocs TemplateDetailView respect template_loaders.

Co-Authored-By: Author: Alexander Lazarević <laza@…>

Note: See TracTickets for help on using tickets.
Back to Top