Code

Opened 7 years ago

Closed 14 months ago

#5652 closed Bug (fixed)

Straightforward use of startproject command results in broken "Show on Site" link in admin

Reported by: Leo Hourvitz <leovitch@…> Owned by: nobody
Component: Core (Management commands) Version: master
Severity: Normal Keywords: SITE_ID
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you use startproject in the straightforward way (such as following the Django tutorial), and if you as suggested implement get_absolute_url() methods in your models, you will indeed get "Show on Site" links in your admin interface. Unfortunately, they'll be broken because they will redirect to example.com.

This is because the sites app is required, and thus activated; and it installs a default piece of SQL code that lists example.com as the URL for the project.

Obviously, one possible fix is to document an additional step everyone has to do (go into their db interface and edit the sites table) to set up any straightforward app, however there are two downsides to that: the first one is it makes the tutorial more complex. However, the stronger reason is that it introduces an absolute URL when a relative URL would generally be a better choice anyway (for instance, my Django apps have different hosts when running under test versus in deployment; that's why a relative URL is the better solution in most cases).

Fortunately, there's a simpler fix that seems to work just fine. Right now the settings.py file installed by startproject sets SITE_ID to 1. Per some earlier changelists, having this value set is required; but it's not required to set it to a valid site_id. If this setting is changed to any value that doesn't exist, the code in management handles the situation perfectly, and reverts to using the relative URL provided by the model's get_absolute_url method (almost certainly what everyone wants until/unless they're actually taking advantage of the sites application). Thus, the suggested fix is, in django/conf/settings.py, change the current uncommented line:

SITE_ID = 1

to something like:

# Right now, this is set to an invalid ID to show you're not using the sites app.
SITE_ID = -1

Obviously not the most important bug in the world, but it's good to get all the little stumbling blocks out of the way of new users whenever we can!

Leo

Attachments (0)

Change History (13)

comment:1 Changed 7 years ago by Leo Hourvitz

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 7 years ago by Collin Grady <cgrady@…>

Having SITE_ID set wrong will cause problems in many areas, so I don't think it's a valid solution to this.

As well, the reason for generating full URLs in a redirect is because relative URLs often do not work right in all browsers, so that's also a poor alternative.

If there's just a quick note added to indicate that you need to fix that domain name, the issue should vanish, and then people know the proper way to do it right off the bat.

comment:3 Changed 7 years ago by Leo Hourvitz <leovitch at gmail>

Sorry, I used "relative URL" when what I meant was "absolute URL without a hostname component" (i.e., URLs that begin with a /). I really think using those would be a better solution than complicating every new Django app; as I mentioned above, the develop/test/deploy gap for a Django app is much easier if it does not generate a hostname at all as part of its' cross-referenced URLs.

Regarding relative URLs having known browser issues, that's a new one on me and thanks for the info. Is there a specific reference to the issue somewhere? (obviously, I haven't run into the problem before)

What are some of the cases where an invalid SITE_ID will cause a failure even when not using multiple sites?

comment:4 Changed 7 years ago by Collin Grady <cgrady@…>

I know what you meant by relative URL, and they're still not always compatible :)

HTTP spec says that the URL in a Location header must be fully qualified, which is why the domain is added to redirects.

SITE_ID is used by the syndication code for one - I'm not sure where all the other areas are, but I've definitely seen a mis-configured SITE_ID cause problems in other areas in some of the problems that come up on IRC :)

comment:5 Changed 6 years ago by Simon G <dev@…>

  • Summary changed from Startforward use of startproject command results in broken "Show on Site" link in admin to Straightforward use of startproject command results in broken "Show on Site" link in admin
  • Triage Stage changed from Unreviewed to Design decision needed

comment:6 Changed 5 years ago by Gordonjcp

Would it be safe to change the default to "localhost:8000" so that it works directly with runserver? Practically the first thing I do on starting a new project is change this setting...

comment:7 Changed 4 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to Accepted

Since the sites app isn't required any longer (it used to be compulsory for admin), we don't need to include it in the list of default installed apps. It isn't that widely used that people needing it can't write it in manually. So the solution to this is remove sites app from the default INSTALLED_APPS list in the project template.

comment:8 Changed 4 years ago by kmtracey

Sites is still needed by admin, at least for "view on site", unless #8960 has been quietly fixed by some change that did not reference that ticket...

comment:9 Changed 3 years ago by gabrielhurley

  • Severity set to Normal
  • Type set to Bug

comment:10 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:11 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:12 Changed 15 months ago by carljm

With the introduction of RequestSite, the admin no longer requires the sites framework to be installed AFAIK, so I think this ticket will be fixed by something we should have done a while ago already - remove the sites framework from INSTALLED_APPS in the default project template.

Version 0, edited 15 months ago by carljm (next)

comment:13 Changed 14 months ago by claudep

  • Resolution set to fixed
  • Status changed from new to closed

django.contrib.sites is no longer in the default INSTALLED_APPS (3f1c7b70537330435e2ec2fca9550f7b7fa4372e).

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.