Opened 9 years ago
Closed 9 years ago
#25153 closed Cleanup/optimization (fixed)
The polls tutorial shows INSTALLED_APPS in incorrect order
Reported by: | Pi Delport | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
The polls tutorial currently gives an example value for INSTALLED_APPS
that's not in the correct order (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 (settings reference, 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.)
Attachments (1)
Change History (9)
comment:1 by , 9 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:2 by , 9 years ago
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 (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,:
- It's inconsistent with the Django's reference documentation about what the ordering should be.
- It's inconsistent with the remainder of the tutorial.
- 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.)
follow-up: 4 comment:3 by , 9 years ago
Resolution: | worksforme |
---|---|
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
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.
comment:4 by , 9 years ago
Replying to 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 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.
follow-up: 7 comment:5 by , 9 years ago
Is anything more than modifying the order of INSTALLED_APPS
needed?
by , 9 years ago
Attachment: | 25153.diff added |
---|
comment:6 by , 9 years ago
Has patch: | set |
---|
comment:7 by , 9 years ago
Replying to 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!
It sounds to me like you are suggesting to put 'polls' first in the
INSTALLED_APPS
list because the custom admin templates inpolls/templates
wouldn't otherwise take effect. That seems like it would be correct, but the custom admin templates should actually be inmysite/templates
so this isn't necessary.