Opened 12 years ago

Closed 7 years ago

Last modified 7 years ago

#18620 closed Cleanup/optimization (fixed)

Prefer current Site when checking M2M in "shortcut"/"view_on_site" redirect

Reported by: Mike Tigas Owned by: Paulo
Component: contrib.contenttypes Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (12)

comment:1 by Aymeric Augustin, 12 years ago

Triage Stage: UnreviewedAccepted

This makes sense and is backwards compatible.

comment:2 by Mike Tigas, 12 years ago

Has patch: set

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 / tests the behavior pretty well.

Last edited 12 years ago by Mike Tigas (previous) (diff)

comment:3 by Mike Tigas, 12 years ago

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.

comment:4 by Tim Graham, 11 years ago

Patch needs improvement: set

comment:5 by Paulo, 7 years ago

Owner: changed from nobody to Paulo
Status: newassigned

comment:6 by Paulo, 7 years ago

Patch needs improvement: unset

comment:7 by Carlton Gibson, 7 years ago

Triage Stage: AcceptedReady for checkin

comment:8 by Tim Graham, 7 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

comment:9 by Tim Graham, 7 years ago

Patch needs improvement: unset

comment:10 by Carlton Gibson, 7 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In d14850e:

Fixed #18620 -- Made ContentTypes shortcut view prefer current site if available.

Thanks Mike Tigas (mtigas) for the initial patch.

comment:12 by Tim Graham <timograham@…>, 7 years ago

In fa679db1:

Refs #18620 -- Refactored ContentTypes view tests to group related field test cases.

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