Fixed #282964: 'data' may point to a callable attribute while still leaving the colum...

diff --git a/django_tables/models.py b/django_tables/models.py
index 5f07a44339b93b482f3176d8ee83433c01d42004..be0cf5fda3a54b7b39ddf1ae1d5e5f6e88fd1a6b 100644 (file)
--- a/django_tables/models.py
+++ b/django_tables/models.py
@@ -97,15 +97,36 @@ class BaseModelTable(BaseTable):
 \r
         if purpose == 'order_by':\r
             column = self.columns[name]\r
-            lookup = column.declared_name\r
-            if column.column.data and not callable(column.column.data):\r
-                lookup = column.column.data\r
-\r
-            try:\r
-                # let django validate the lookup\r
-                _temp = self.queryset.order_by(lookup)\r
-                _temp.query.as_sql()\r
-            except FieldError:\r
+\r
+            # "data" can really be used in two different ways. It is\r
+            # slightly confusing and potentially should be changed.\r
+            # It can either refer to an attribute/field which the table\r
+            # column should represent, or can be a callable (or a string\r
+            # pointing to a callable attribute) that is used to render to\r
+            # cell. The difference is that in the latter case, there may\r
+            # still be an actual source model field behind the column,\r
+            # stored in "declared_name". In other words, we want to filter\r
+            # out column names that are not oderable, and the column name\r
+            # we need to check may either be stored in "data" or in\r
+            # "declared_name", depending on if and what kind of value is\r
+            # in "data". This is the reason why we try twice.\r
+            #\r
+            # See also bug #282964.\r
+            #\r
+            # TODO: It might be faster to try to resolve the given name\r
+            # manually recursing the model metadata rather than\r
+            # constructing a queryset.\r
+            for lookup in (column.column.data, column.declared_name):\r
+                if not lookup or callable(lookup):\r
+                    continue\r
+                try:\r
+                    # let django validate the lookup\r
+                    _temp = self.queryset.order_by(lookup)\r
+                    _temp.query.as_sql()\r
+                    break\r
+                except FieldError:\r
+                    pass\r
+            else:\r
                 return False\r
 \r
         # if we haven't failed by now, the column should be valid\r

diff --git a/tests/test_models.py b/tests/test_models.py
index c7f94bc6290f05eb3a32dbea1ac6f51ffd5323fe..61f97b03d69c7ece376b53e4b84ad34775e14f39 100644 (file)
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -258,6 +258,7 @@ def test_column_data():
 \r
     class CountryTable(tables.ModelTable):\r
         name = tables.Column(data=lambda d: "hidden")\r
+        tld = tables.Column(data='example_domain', name="domain")\r
         default_and_data = tables.Column(data=lambda d: None, default=4)\r
         class Meta:\r
             model = Country\r
@@ -270,6 +271,10 @@ def test_column_data():
     # to correct model column; can be used to rewrite what is displayed\r
     countries.order_by = 'name'\r
     assert countries.order_by == ('name',)\r
+    # [bug 282964] this trick also works if the callable is an attribute\r
+    # and we refer to it per string, rather than giving a function object\r
+    countries.order_by = 'domain'\r
+    assert countries.order_by == ('domain',)\r
 \r
 def test_pagination():\r
     """Pretty much the same as static table pagination, but make sure we\r