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)

25153.diff (596 bytes ) - added by Tim Graham 9 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by Tim Graham, 9 years ago

Resolution: worksforme
Status: newclosed

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.

comment:2 by Pi Delport, 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,:

  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.)

comment:3 by Tim Graham, 9 years ago

Resolution: worksforme
Status: closednew
Triage Stage: UnreviewedAccepted
Type: BugCleanup/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.

in reply to:  3 comment:4 by Carl Meyer, 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.

comment:5 by Tim Graham, 9 years ago

Is anything more than modifying the order of INSTALLED_APPS needed?

by Tim Graham, 9 years ago

Attachment: 25153.diff added

comment:6 by Tim Graham, 9 years ago

Has patch: set

in reply to:  5 comment:7 by Pi Delport, 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!

comment:8 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In bda408f:

Fixed #25153 -- Moved 'polls' first in tutorial's INSTALLED_APPS.

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