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

Thanks Mike Tigas (mtigas) for the initial patch.

2 files changed, 41 insertions, 29 deletions

diff --git a/django/contrib/contenttypes/views.py b/django/contrib/contenttypes/views.py
index cdbc58fa3f..8c194839c8 100644
--- a/django/contrib/contenttypes/views.py
+++ b/django/contrib/contenttypes/views.py
@@ -1,6 +1,6 @@
 from django.apps import apps
 from django.contrib.contenttypes.models import ContentType
-from django.contrib.sites.requests import RequestSite
+from django.contrib.sites.shortcuts import get_current_site
 from django.core.exceptions import ObjectDoesNotExist
 from django.http import Http404, HttpResponseRedirect
 from django.utils.translation import gettext as _
@@ -43,27 +43,32 @@ def shortcut(request, content_type_id, object_id):
 
     # Otherwise, we need to introspect the object's relationships for a
     # relation to the Site object
-    object_domain = None
+    try:
+        object_domain = get_current_site(request).domain
+    except ObjectDoesNotExist:
+        object_domain = None
 
     if apps.is_installed('django.contrib.sites'):
         Site = apps.get_model('sites.Site')
-
         opts = obj._meta
 
-        # First, look for a many-to-many relationship to Site.
         for field in opts.many_to_many:
+            # Look for a many-to-many relationship to Site.
             if field.remote_field.model is Site:
-                try:
-                    # Caveat: In the case of multiple related Sites, this just
-                    # selects the *first* one, which is arbitrary.
-                    object_domain = getattr(obj, field.name).all()[0].domain
-                except IndexError:
-                    pass
-                if object_domain is not None:
+                site_qs = getattr(obj, field.name).all()
+                if object_domain and site_qs.filter(domain=object_domain).exists():
+                    # The current site's domain matches a site attached to the
+                    # object.
                     break
-
-        # Next, look for a many-to-one relationship to Site.
-        if object_domain is None:
+                # Caveat: In the case of multiple related Sites, this just
+                # selects the *first* one, which is arbitrary.
+                site = site_qs.first()
+                if site:
+                    object_domain = site.domain
+                    break
+        else:
+            # No many-to-many relationship to Site found. Look for a
+            # many-to-one relationship to Site.
             for field in obj._meta.fields:
                 if field.remote_field and field.remote_field.model is Site:
                     try:
@@ -72,20 +77,8 @@ def shortcut(request, content_type_id, object_id):
                         continue
                     if site is not None:
                         object_domain = site.domain
-                    if object_domain is not None:
                         break
 
-        # Fall back to the current site (if possible).
-        if object_domain is None:
-            try:
-                object_domain = Site.objects.get_current(request).domain
-            except Site.DoesNotExist:
-                pass
-
-    else:
-        # Fall back to the current request's site.
-        object_domain = RequestSite(request).domain
-
     # If all that malarkey found an object domain, use it. Otherwise, fall back
     # to whatever get_absolute_url() returned.
     if object_domain is not None:
diff --git a/tests/contenttypes_tests/test_views.py b/tests/contenttypes_tests/test_views.py
index 72e87550d0..a26f654a02 100644
--- a/tests/contenttypes_tests/test_views.py
+++ b/tests/contenttypes_tests/test_views.py
@@ -106,13 +106,15 @@ class ContentTypesViewsTests(TestCase):
         obj = ModelWithNullFKToSite.objects.create(title='title')
         url = '/shortcut/%s/%s/' % (ContentType.objects.get_for_model(ModelWithNullFKToSite).id, obj.pk)
         response = self.client.get(url)
-        self.assertRedirects(response, '%s' % obj.get_absolute_url(), fetch_redirect_response=False)
+        expected_url = 'http://testserver%s' % obj.get_absolute_url()
+        self.assertRedirects(response, expected_url, fetch_redirect_response=False)
 
     @mock.patch('django.apps.apps.get_model')
     def test_shortcut_view_with_site_m2m(self, get_model):
         """
-        When the object has a ManyToManyField to Site, redirect to the
-        domain of the first site found in the m2m relationship.
+        When the object has a ManyToManyField to Site, redirect to the current
+        site if it's attached to the object or to the domain of the first site
+        found in the m2m relationship.
         """
         get_model.side_effect = lambda *args, **kwargs: MockSite if args[0] == 'sites.Site' else ModelWithM2MToSite
 
@@ -138,6 +140,23 @@ class ContentTypesViewsTests(TestCase):
             response = self.client.get('/shortcut/%s/%s/' % (ct.pk, site_3_obj.pk))
             self.assertRedirects(response, expected_url, fetch_redirect_response=False)
 
+        obj_with_sites = ModelWithM2MToSite.objects.create(title='Linked to Current Site')
+        obj_with_sites.sites.set(MockSite.objects.all())
+        shortcut_url = '/shortcut/%s/%s/' % (ct.pk, obj_with_sites.pk)
+        expected_url = 'http://example2.com%s' % obj_with_sites.get_absolute_url()
+
+        with self.settings(SITE_ID=2):
+            # Redirects to the domain of the Site matching the current site's
+            # domain.
+            response = self.client.get(shortcut_url)
+            self.assertRedirects(response, expected_url, fetch_redirect_response=False)
+
+        with self.settings(SITE_ID=None, ALLOWED_HOSTS=['example2.com']):
+            # Redirects to the domain of the Site matching the request's host
+            # header.
+            response = self.client.get(shortcut_url, SERVER_NAME='example2.com')
+            self.assertRedirects(response, expected_url, fetch_redirect_response=False)
+
 
 class ShortcutViewTests(TestCase):