Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Ticket #18620: Prefer current Site when checking M2M in "shortcut"/"view_on_site" redirect #203

Closed
wants to merge 7 commits into from

6 participants

Mike Tigas Tim Graham Stefan Kjartansson Adrian Holovaty Alex Gaynor Claude Paroz
Tim Graham
Owner

This PR needs to be reworked so it doesn't include unrelated commits (caused by when someone accidentally force pushed to (django/django)). There have also been a lot of changes in the last 2 years so the PR probably needs updating.

Tim Graham timgraham closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
42  django/contrib/contenttypes/tests.py
@@ -8,6 +8,7 @@
8 8
 from django.contrib.sites.models import Site
9 9
 from django.http import HttpRequest, Http404
10 10
 from django.test import TestCase
  11
+from django.utils import unittest
11 12
 from django.utils.encoding import smart_str
12 13
 
13 14
 
@@ -45,6 +46,13 @@ class FooWithBrokenAbsoluteUrl(FooWithoutUrl):
45 46
     def get_absolute_url(self):
46 47
         return "/users/%s/" % self.unknown_field
47 48
 
  49
+if Site._meta.installed:
  50
+    class FooWithSiteM2MAndUrl(FooWithUrl):
  51
+        """
  52
+        Fake model containing a `Site` many-to-many relationship.
  53
+        """
  54
+        sites = models.ManyToManyField(Site)
  55
+
48 56
 class ContentTypesTests(TestCase):
49 57
 
50 58
     def setUp(self):
@@ -260,6 +268,40 @@ def test_shortcut_view_with_broken_get_absolute_url(self):
260 268
 
261 269
         self.assertRaises(AttributeError, shortcut, request, user_ct.id, obj.id)
262 270
 
  271
+    @unittest.skipIf(not Site._meta.installed,
  272
+                     "this tests against relationship behavior with the Sites framework")
  273
+    def test_shortcut_view_m2m(self):
  274
+        """
  275
+        Check that the shortcut view (used for the admin "view on site"
  276
+        functionality) returns a complete URL regardless of whether the sites
  277
+        framework is installed
  278
+        """
  279
+        # Tests ticket #18620; prefer current_site rather than allowing
  280
+        # M2M to select first domain via ABC order.
  281
+        current_site = Site.objects.get_current()  # example.com
  282
+        other_site = Site(domain="a.example.com", name="a.example.com")
  283
+        other_site.save()
  284
+
  285
+        # Build an object that belongs to both sites.
  286
+        obj = FooWithSiteM2MAndUrl(name="john")
  287
+        obj.save()  # need saved object to set M2M relationship
  288
+        obj.sites = [other_site, current_site]
  289
+
  290
+        # Check that shortcut view returns absolute URI for `current_site`
  291
+        # and not `other_site`.
  292
+        request = HttpRequest()
  293
+        request.META = {
  294
+            "SERVER_NAME": "Example.com",
  295
+            "SERVER_PORT": "80",
  296
+        }
  297
+        user_ct = ContentType.objects.get_for_model(FooWithSiteM2MAndUrl)
  298
+        response = shortcut(request, user_ct.id, obj.id)
  299
+        self.assertEqual("http://%s/users/john/" % current_site.domain,
  300
+                         response._headers.get("location")[1])
  301
+
  302
+        obj.delete()
  303
+        other_site.delete()
  304
+
263 305
     def test_missing_model(self):
264 306
         """
265 307
         Ensures that displaying content types in admin (or anywhere) doesn't
33  django/contrib/contenttypes/views.py
@@ -38,6 +38,15 @@ def shortcut(request, content_type_id, object_id):
38 38
     # Otherwise, we need to introspect the object's relationships for a
39 39
     # relation to the Site object
40 40
     object_domain = None
  41
+    current_domain = None
  42
+
  43
+    # Get current site (if possible): this decides the preferred domain
  44
+    # if this object has an M2M site field and provides a fallback if this
  45
+    # object does not have a site FK or M2M field.
  46
+    try:
  47
+        current_domain = get_current_site(request).domain
  48
+    except Site.DoesNotExist:
  49
+        pass
41 50
 
42 51
     if Site._meta.installed:
43 52
         opts = obj._meta
@@ -45,10 +54,21 @@ def shortcut(request, content_type_id, object_id):
45 54
         # First, look for an many-to-many relationship to Site.
46 55
         for field in opts.many_to_many:
47 56
             if field.rel.to is Site:
  57
+                site_qs = getattr(obj, field.name).all()
  58
+                try:
  59
+                    # If one of the sites this object belongs to is the current
  60
+                    # site, use it.
  61
+                    object_domain = site_qs.get(domain=current_domain).domain
  62
+                except Site.DoesNotExist:
  63
+                    pass
  64
+                if object_domain is not None:
  65
+                    break
48 66
                 try:
49  
-                    # Caveat: In the case of multiple related Sites, this just
50  
-                    # selects the *first* one, which is arbitrary.
51  
-                    object_domain = getattr(obj, field.name).all()[0].domain
  67
+                    # Current site was not in the M2M relationship for this
  68
+                    # object. Caveat: This just selects the *first* one, which
  69
+                    # is arbitrary. (Generally based on Site model ordering,
  70
+                    # which is alphabetical by domain.)
  71
+                    object_domain = site_qs[0].domain
52 72
                 except IndexError:
53 73
                     pass
54 74
                 if object_domain is not None:
@@ -65,12 +85,9 @@ def shortcut(request, content_type_id, object_id):
65 85
                     if object_domain is not None:
66 86
                         break
67 87
 
68  
-    # Fall back to the current site (if possible).
  88
+    # Fall back to the current site (if possible) or None (which is fine).
69 89
     if object_domain is None:
70  
-        try:
71  
-            object_domain = get_current_site(request).domain
72  
-        except Site.DoesNotExist:
73  
-            pass
  90
+        object_domain = current_domain
74 91
 
75 92
     # If all that malarkey found an object domain, use it. Otherwise, fall back
76 93
     # to whatever get_absolute_url() returned.
2  django/db/models/__init__.py
@@ -15,8 +15,6 @@
15 15
 from django.db.models import signals
16 16
 from django.utils.decorators import wraps
17 17
 
18  
-# Admin stages.
19  
-ADD, CHANGE, BOTH = 1, 2, 3
20 18
 
21 19
 def permalink(func):
22 20
     """
50  docs/ref/contrib/gis/install.txt
@@ -1034,61 +1034,11 @@ Optional packages to consider:
1034 1034
     do not plan on doing any database transformation of geometries to the
1035 1035
     Google projection (900913).
1036 1036
 
1037  
-.. _heron:
1038  
-
1039  
-8.04 and lower
1040  
-~~~~~~~~~~~~~~
1041  
-
1042  
-The 8.04 (and lower) versions of Ubuntu use GEOS v2.2.3 in their binary packages,
1043  
-which is incompatible with GeoDjango.  Thus, do *not* use the binary packages
1044  
-for GEOS or PostGIS and build some prerequisites from source, per the instructions
1045  
-in this document; however, it is okay to use the PostgreSQL binary packages.
1046  
-
1047  
-For more details, please see the Debian instructions for :ref:`etch` below.
1048  
-
1049 1037
 .. _debian:
1050 1038
 
1051 1039
 Debian
1052 1040
 ------
1053 1041
 
1054  
-.. _etch:
1055  
-
1056  
-4.0 (Etch)
1057  
-^^^^^^^^^^
1058  
-
1059  
-The situation here is the same as that of Ubuntu :ref:`heron` -- in other words,
1060  
-some packages must be built from source to work properly with GeoDjango.
1061  
-
1062  
-Binary packages
1063  
-~~~~~~~~~~~~~~~
1064  
-The following command will install acceptable binary packages, as well as
1065  
-the development tools necessary to build the rest of the requirements:
1066  
-
1067  
-.. code-block:: bash
1068  
-
1069  
-    $ sudo apt-get install binutils bzip2 gcc g++ flex make postgresql-8.1 \
1070  
-        postgresql-server-dev-8.1 python-ctypes python-psycopg2 python-setuptools
1071  
-
1072  
-Required package information:
1073  
-
1074  
-* ``binutils``: for ctypes to find libraries
1075  
-* ``bzip2``: for decompressing the source packages
1076  
-* ``gcc``, ``g++``, ``make``: GNU developer tools used to compile the libraries
1077  
-* ``flex``: required to build PostGIS
1078  
-* ``postgresql-8.1``
1079  
-* ``postgresql-server-dev-8.1``: for ``pg_config``
1080  
-* ``python-psycopg2``
1081  
-
1082  
-Optional packages:
1083  
-
1084  
-* ``libgeoip``: for :ref:`GeoIP <ref-geoip>` support
1085  
-
1086  
-Source packages
1087  
-~~~~~~~~~~~~~~~
1088  
-You will still have to install :ref:`geosbuild`, :ref:`proj4`,
1089  
-:ref:`postgis`, and :ref:`gdalbuild` from source.  Please follow the
1090  
-directions carefully.
1091  
-
1092 1042
 .. _lenny:
1093 1043
 
1094 1044
 5.0 (Lenny)
2  docs/topics/python3.txt
@@ -181,7 +181,7 @@ xrange                          range                                   xrange
181 181
 =============================== ======================================  ======================
182 182
 
183 183
 
184  
-Ouptut encoding now Unicode
  184
+Output encoding now Unicode
185 185
 ===========================
186 186
 
187 187
 If you want to catch stdout/stderr output, the output content is UTF-8 encoded
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.