[Django] #29030: django.contrib.sites Documnets incorrectly its use in django.contrib.admin "View Site" functionality

9 views
Skip to first unread message

Django

unread,
Jan 16, 2018, 3:17:41 PM1/16/18
to django-...@googlegroups.com
#29030: django.contrib.sites Documnets incorrectly its use in django.contrib.admin
"View Site" functionality
-----------------------------------------+------------------------
Reporter: Kaleb Hornsby | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-----------------------------------------+------------------------
The [https://docs.djangoproject.com/en/dev/ref/contrib/sites/#how-django-
uses-the-sites-framework How Django Uses the Sites Framework]
documentation states the following:

In the admin framework, the “view on site” link uses the current
[https://docs.djangoproject.com/en/dev/ref/contrib/sites/#django.contrib.sites.models.Site
Site] to work out the domain for the site that it will redirect to.

This statement seems to be false. In
[https://docs.djangoproject.com/en/2.0/ref/contrib/admin/#django.contrib.admin.AdminSite.site_url
the admin site docs], it states the following:

The URL for the “View site” link at the top of each admin page. By
default, `site_url` is `/`. Set it to `None` to remove the link.
For sites running on a subpath, the `each_context()` method checks if
the current request has `request.META['SCRIPT_NAME']` set and uses that
value if `site_url` isn’t set to something other than `/`.

This seems to be backed up by what I am actually seeing, and what the code
says that it is doing. Changing the current site's domain in the sites
framework has no effect on this link. I suggest removing the incorrect
information from the sites documentation. The alternative would be to add
this functionality to the admin site.

--
Ticket URL: <https://code.djangoproject.com/ticket/29030>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 16, 2018, 3:39:45 PM1/16/18
to django-...@googlegroups.com
#29030: django.contrib.sites Documnets incorrectly its use in django.contrib.admin
"View Site" functionality
-------------------------------+--------------------------------------

Reporter: Kaleb Hornsby | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Kaleb Hornsby):

Looks like this was the original change:
https://github.com/django/django/pull/2319

--
Ticket URL: <https://code.djangoproject.com/ticket/29030#comment:1>

Django

unread,
Jan 16, 2018, 3:53:19 PM1/16/18
to django-...@googlegroups.com
#29030: django.contrib.sites Documnets incorrectly its use in django.contrib.admin
"View Site" functionality
-------------------------------+--------------------------------------

Reporter: Kaleb Hornsby | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Kaleb Hornsby):

So when the previous docs change was made, the only references to
`django.contrib.sites` with the admin app were in
[https://github.com/django/django/blob/4fd2b0ca77c135e39b52bd8081067b9365f9fb9f/django/contrib/admin/views/template.py
views/template.py] and
[https://github.com/django/django/blob/4fd2b0ca77c135e39b52bd8081067b9365f9fb9f/django/contrib/admin/views/doc.py
views/doc.py]. Neither of them were used to use the appropriate domain in
the 'view site' or 'view on site' functionality.

--
Ticket URL: <https://code.djangoproject.com/ticket/29030#comment:2>

Django

unread,
Jan 16, 2018, 4:58:41 PM1/16/18
to django-...@googlegroups.com
#29030: django.contrib.sites Documnets incorrectly its use in django.contrib.admin
"View Site" functionality
-------------------------------+--------------------------------------

Reporter: Kaleb Hornsby | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Kaleb Hornsby):

And now looking into it further, "view on site" does in deed have this
functionality... "view site" does not. This is an inconsistency in my
opinion. I should probably close this, and re-open another ticket. I would
like to have some input from anybody else whether it the behavior is
inconsistent and should be addressed.

--
Ticket URL: <https://code.djangoproject.com/ticket/29030#comment:3>

Django

unread,
Jan 17, 2018, 10:54:25 AM1/17/18
to django-...@googlegroups.com
#29030: contrib.sites incorrectly documents its use in django.contrib.admin "View
Site" functionality
-------------------------------+--------------------------------------

Reporter: Kaleb Hornsby | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Tim Graham):

Is your suggestion to change the implementation of
a81af7f49de7ff3f51f111de28ed3a682f67ea89 to use the sites framework
instead of hardcoding '/'? I'm not exactly sure what case that would fix.
If you're browsing a particular site like "www.example.com/admin/"
wouldn't the site URL be "www.example.com", same as linking to "/"?

--
Ticket URL: <https://code.djangoproject.com/ticket/29030#comment:4>

Django

unread,
Jan 17, 2018, 1:17:17 PM1/17/18
to django-...@googlegroups.com
#29030: contrib.sites incorrectly documents its use in django.contrib.admin "View
Site" functionality
-------------------------------+--------------------------------------

Reporter: Kaleb Hornsby | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Kaleb Hornsby):

Replying to [comment:4 Tim Graham]:


> Is your suggestion to change the implementation of
a81af7f49de7ff3f51f111de28ed3a682f67ea89 to use the sites framework
instead of hardcoding '/'? I'm not exactly sure what case that would fix.
If you're browsing a particular site like "www.example.com/admin/"
wouldn't the site URL be "www.example.com", same as linking to "/"?

Right now if you are hosting at `www.example.com` that is accessible to
the internet, and you have a `admin.example.com` that is not publicly
accessible and in your settings, you have `SITE_ID = 1` and
`sites.models.Site.objects.get_current().domain == 'www.example.com'` then
when you are on the admin (at `https://admin.example.com/admin`) and you
click on the "View on site" link for an object that defines its
`get_absolute_url` method to return `'/things/1/'`, then you get
redirected to `https://www.example.com/things/1`. If you click on the
"View site" link, however, you get sent to `https://admin.example.com/`.

Now I am not even sure if changing it to check if `sites` is installed and
prepending the domain is desired behavior or not. Anybody can set
`admin.site.site_url = sites.models.Site.objects.get_current().domain`.
But when I was reading the documentation as linked in the ticket, I did
get confused. It is highly likely that I am the only one to have confused
"View on site" with "View site". But maybe not. I think that the two links
should probably be consistent with each other by default, though.

Thank you for taking the time to look at this ticket and inquiring more
about the issue.

--
Ticket URL: <https://code.djangoproject.com/ticket/29030#comment:5>

Django

unread,
Jan 17, 2018, 2:40:05 PM1/17/18
to django-...@googlegroups.com
#29030: contrib.sites incorrectly documents its use in django.contrib.admin "View
Site" functionality
-------------------------------+-----------------------------------------
Reporter: Kaleb Hornsby | Owner: ChiragMaliwal
Type: Bug | Status: assigned
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:

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 ChiragMaliwal):

* owner: nobody => ChiragMaliwal
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/29030#comment:6>

Django

unread,
Feb 6, 2018, 7:18:48 PM2/6/18
to django-...@googlegroups.com
#29030: Make construction of "View on site" and "View Site" URLs consistent
-------------------------------------+-------------------------------------
Reporter: Kaleb Hornsby | Owner:
Type: | ChiragMaliwal
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
* component: Documentation => contrib.admin
* easy: 1 => 0


Comment:

It might be feasible to make the links consistent, although backwards
compatibility could be an issue.

--
Ticket URL: <https://code.djangoproject.com/ticket/29030#comment:7>

Reply all
Reply to author
Forward
0 new messages