[Django] #18620: Prefer current Site when checking M2M in "shortcut"/"view_on_site" redirect
Django
--------------------------------------+--------------------
Reporter: mtigas | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.contenttypes | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
Given an object with a many-to-many relationship to
`django.contrib.sites.models.Site`, the
`django.contrib.contenttypes.views.shortcut` view simply picks the first
domain returned, basically `obj.<field>[0].domain`.
(https://github.com/django/django/blob/4a103086/django/contrib/contenttypes/views.py#L45
-- it even notes that the ordering is arbitrary.)
This can be confusing when operating several (shared database) Django
sites where sometimes content is cross-posted between said sites.
Example that led to my noticing this issue: A story being edited on
www.spokesman.com but posted to sites [www.spokesman.com,
www.downtoearthnw.com] redirects you to story's representation on the
latter website when clicking "view on site" in the admin (or using
anything else that uses that underlying `shortcut` view). (Ostensibly
because "www.downtoearthnw.com" sorts before "www.spokesman.com" and is
therefore picked by the simple code I linked above.) Although this is a
''theoretically acceptable'' thing to happen, it's confusing to (even
technical) users because they're ''on'' one of the acceptable sites for
the content object and do not expect to be redirected to another site.
I propose that the view should prefer the current site in the event that
both of the following are true:
a) the content object "Site" field is an M2M
b) the current Site object is one of the values set in that M2M
This shouldn't change behavior in any other circumstance (current site not
in that M2M, site field is not M2M, etc), but it should alleviate some
confusion to this specific usecase.
Have code and will post in a GitHub fork shortly.
--
Ticket URL: <https://code.djangoproject.com/ticket/18620>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: Cleanup/optimization | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by aaugustin):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
This makes sense and is backwards compatible.
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:1>
Django
Type: Cleanup/optimization | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by mtigas):
* has_patch: 0 => 1
Comment:
Fork: https://github.com/mtigas/django/tree/patches/ticket-18620
Pull request: https://github.com/django/django/pull/203
Test isn't the best but does fail without the views.py patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:2>
Django
Type: Cleanup/optimization | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Comment (by mtigas):
Also I'm on the fence as to whether or not it should optimize and not
query for `current_site` until absolutely necessary (i.e. use `lazy` or
use some if-blocks). Basically to avoid one unnecessary query in the
situation if we have a site FK.
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:3>
Django
Reporter: mtigas | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:4>
Django
Reporter: Mike Tigas | Owner: Paulo
Type: Cleanup/optimization | Status: assigned
Component: contrib.contenttypes | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Paulo):
* owner: nobody => Paulo
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:5>
Django
Reporter: Mike Tigas | Owner: Paulo
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Paulo):
* needs_better_patch: 1 => 0
Comment:
PR: https://github.com/django/django/pull/9120
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:6>
Django
Reporter: Mike Tigas | Owner: Paulo
Cleanup/optimization |
Component: | Version: 1.4
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:7>
Django
Reporter: Mike Tigas | Owner: Paulo
Component: contrib.contenttypes | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:8>
Django
Reporter: Mike Tigas | Owner: Paulo
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:9>
Django
Reporter: Mike Tigas | Owner: Paulo
Cleanup/optimization |
Component: | Version: 1.4
contrib.contenttypes |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:10>
Django
Reporter: Mike Tigas | Owner: Paulo
Cleanup/optimization |
Component: | Version: 1.4
contrib.contenttypes |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d14850e525ccf95d42b8e9f11571dc5d18b15f03" d14850e]:
{{{
#!CommitTicketReference repository=""
revision="d14850e525ccf95d42b8e9f11571dc5d18b15f03"
Fixed #18620 -- Made ContentTypes shortcut view prefer current site if
available.
Thanks Mike Tigas (mtigas) for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:11>
Django
Reporter: Mike Tigas | Owner: Paulo
Type: | Status: closed
Cleanup/optimization |
Component: | Version: 1.4
contrib.contenttypes |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"fa679db1ff72667749757d62324a2797cb4a2cc4" fa679db1]:
{{{
#!CommitTicketReference repository=""
revision="fa679db1ff72667749757d62324a2797cb4a2cc4"
Refs #18620 -- Refactored ContentTypes view tests to group related field
test cases.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18620#comment:12>