[Django] #25153: The polls tutorial shows INSTALLED_APPS in incorrect order
Django
-------------------------------+--------------------
Reporter: pjdelport | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------
The polls tutorial currently gives an example value for `INSTALLED_APPS`
that's not in the correct order
([https://github.com/django/django/blob/3653466bdf211ca603ec976c28d4a8da566dc671/docs/intro/tutorial02.txt#L208-L216
source]):
{{{
INSTALLED_APPS = [
'django.contrib.admin',
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',
'django.contrib.messages',
'django.contrib.staticfiles',
'polls',
]
}}}
As the Django docs explain elsewhere
([https://docs.djangoproject.com/en/1.8/ref/settings/#installed-apps
settings reference],
[https://docs.djangoproject.com/en/1.8/ref/templates/api/#django.template.loaders.app_directories.Loader
templates API]), `INSTALLED_APPS` should be listed in precedence order,
with higher-level apps preceding the apps that they depend on and
override.
Later parts of the tutorial actually depend on this, when it discusses
customizing the admin templates from the polls app. (Every time I've
helped someone through the polls tutorial, and they attempt to customize
the admin template, I've had to explain and help them fix this.)
--
Ticket URL: <https://code.djangoproject.com/ticket/25153>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: pjdelport | Owner: nobody
Type: Bug | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: worksforme
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* needs_docs: => 0
* resolution: => worksforme
* needs_tests: => 0
* needs_better_patch: => 0
Comment:
It sounds to me like you are suggesting to put 'polls' first in the
`INSTALLED_APPS` list because the custom admin templates in
`polls/templates` wouldn't otherwise take effect. That seems like it would
be correct, but the custom admin templates should actually be in
`mysite/templates` so this isn't necessary.
--
Ticket URL: <https://code.djangoproject.com/ticket/25153#comment:1>
Django
Reporter: pjdelport | Owner: nobody
Type: Bug | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: worksforme
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by pjdelport):
Overriding templates from a separate project-level template directory does
work, but that's a different solution, and not a replacement for
application-level template overrides. (The same for static files and other
app resources.)
The tutorial discusses both kinds of overriding later on, including the
specific example of overriding admin templates at the application (not
project) level
([https://github.com/django/django/blob/3653466bdf211ca603ec976c28d4a8da566dc671/docs/intro/tutorial07.txt#L385-L390
link]), which simply won't work with the example's `INSTALLED_APPS`
ordering until it is fixed.
To summarize, I think that the example's ordering is harmful, or at least
counter-productive, for the following reasons,:
1. It's inconsistent with the Django's reference documentation about what
the ordering should be.
2. It's inconsistent with the remainder of the tutorial.
3. It gives beginners the wrong working intuition about `INSTALLED_APPS`'s
precedence at a key learning window, which they'll invariably have to get
bitten by and unlearn when they actually start using or creating Django
apps and libraries more seriously.
Fixing the example should not have any drawbacks, while resolving these
problems.
Would you reconsider the decision to close this issue? (I would be happy
to submit a PR.)
--
Ticket URL: <https://code.djangoproject.com/ticket/25153#comment:2>
Django
Reporter: pjdelport | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* resolution: worksforme =>
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
When overriding admin templates at the project level, you should still
namespace them such as `polls/templates/admin/polls/app_index.html` in
which case there shouldn't be any problem with clashing names unless I am
missing something.
That said, I don't see a problem with the proposed reordering.
--
Ticket URL: <https://code.djangoproject.com/ticket/25153#comment:3>
Django
Reporter: pjdelport | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by carljm):
Replying to [comment:3 timgraham]:
> When overriding admin templates at the project level, you should still
namespace them such as `polls/templates/admin/polls/app_index.html` in
which case there shouldn't be any problem with clashing names unless I am
missing something.
I'm not sure how this is relevant to the OP's point - there's no reference
here to "a problem with clashing names" that I can see.
It is a bit confusing, because there are two very different kinds of
"overriding" admin templates, both
[https://docs.djangoproject.com/en/1.8/ref/contrib/admin/#admin-
overriding-templates conflated in the docs] under the heading "overriding
admin templates." The first kind is per-app or per-model templates (e.g.
`admin/polls/app_index.html`), which isn't really an override at all
(there's no such template in the admin to override), it's just exploiting
a specialized admin feature where it looks for templates named according
to a certain per-app or per-model convention. This type of "override" can
be done either in your project templates or in the app templates,
regardless of `INSTALLED_APP` ordering, because it's not really an
override at all.
But the tutorial also discusses overriding `admin/base_site.html` to edit
the header of your admin site. This is a real template override, and the
OP is correct that it won't work in an app template with the current
`INSTALLED_APPS` ordering (OTOH, this is an override that doesn't really
make much sense in an app level template anyway).
I think that the tutorial is internally consistent here (barely, assuming
you accept that it wouldn't make sense to override admin-wide templates
like `admin/base_site.html` in an app), but the OP is correct that the
`INSTALLED_APPS` ordering in the tutorial contradicts our own
documentation of best practices for `INSTALLED_APPS` ordering, and is the
opposite of what you'd usually want in order to get the right precedence
for overrides. So I think this is a change worth making.
--
Ticket URL: <https://code.djangoproject.com/ticket/25153#comment:4>
Django
Reporter: pjdelport | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by timgraham):
Is anything more than modifying the order of `INSTALLED_APPS` needed?
--
Ticket URL: <https://code.djangoproject.com/ticket/25153#comment:5>
Django
Reporter: pjdelport | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* Attachment "25153.diff" added.
Django
Reporter: pjdelport | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25153#comment:6>
Django
Reporter: pjdelport | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by pjdelport):
Replying to [comment:5 timgraham]:
> Is anything more than modifying the order of `INSTALLED_APPS` needed?
I don't think so.
I was hoping to find some time to try and make the surrounding
documentation clearer and more explicit, but I don't want that to block
this modification in the mean time.
Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/25153#comment:7>
Django
Reporter: pjdelport | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"bda408f60bf901620e88d25aad38210985e5c758" bda408f]:
{{{
#!CommitTicketReference repository=""
revision="bda408f60bf901620e88d25aad38210985e5c758"
Fixed #25153 -- Moved 'polls' first in tutorial's INSTALLED_APPS.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25153#comment:8>