From e055655d32fd1901abab1f8c803b1d03e553d3bb Mon Sep 17 00:00:00 2001
From: Michael Elsdoerfer <michael@elsdoerfer.info>
Date: Sun, 11 Jan 2009 14:38:18 +0100
Subject: [PATCH] Fixed #282964: 'data' may point to a callable attribute while
 still leaving the column sortable.

---
 django_tables/models.py | 39 ++++++++++++++++++++++++++++++---------
 tests/test_models.py    |  5 +++++
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/django_tables/models.py b/django_tables/models.py
index 5f07a44..be0cf5f 100644
--- a/django_tables/models.py
+++ b/django_tables/models.py
@@ -97,15 +97,36 @@ class BaseModelTable(BaseTable):
 
         if purpose == 'order_by':
             column = self.columns[name]
-            lookup = column.declared_name
-            if column.column.data and not callable(column.column.data):
-                lookup = column.column.data
-
-            try:
-                # let django validate the lookup
-                _temp = self.queryset.order_by(lookup)
-                _temp.query.as_sql()
-            except FieldError:
+
+            # "data" can really be used in two different ways. It is
+            # slightly confusing and potentially should be changed.
+            # It can either refer to an attribute/field which the table
+            # column should represent, or can be a callable (or a string
+            # pointing to a callable attribute) that is used to render to
+            # cell. The difference is that in the latter case, there may
+            # still be an actual source model field behind the column,
+            # stored in "declared_name". In other words, we want to filter
+            # out column names that are not oderable, and the column name
+            # we need to check may either be stored in "data" or in
+            # "declared_name", depending on if and what kind of value is
+            # in "data". This is the reason why we try twice.
+            #
+            # See also bug #282964.
+            #
+            # TODO: It might be faster to try to resolve the given name
+            # manually recursing the model metadata rather than
+            # constructing a queryset.
+            for lookup in (column.column.data, column.declared_name):
+                if not lookup or callable(lookup):
+                    continue
+                try:
+                    # let django validate the lookup
+                    _temp = self.queryset.order_by(lookup)
+                    _temp.query.as_sql()
+                    break
+                except FieldError:
+                    pass
+            else:
                 return False
 
         # if we haven't failed by now, the column should be valid
diff --git a/tests/test_models.py b/tests/test_models.py
index c7f94bc..61f97b0 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -258,6 +258,7 @@ def test_column_data():
 
     class CountryTable(tables.ModelTable):
         name = tables.Column(data=lambda d: "hidden")
+        tld = tables.Column(data='example_domain', name="domain")
         default_and_data = tables.Column(data=lambda d: None, default=4)
         class Meta:
             model = Country
@@ -270,6 +271,10 @@ def test_column_data():
     # to correct model column; can be used to rewrite what is displayed
     countries.order_by = 'name'
     assert countries.order_by == ('name',)
+    # [bug 282964] this trick also works if the callable is an attribute
+    # and we refer to it per string, rather than giving a function object
+    countries.order_by = 'domain'
+    assert countries.order_by == ('domain',)
 
 def test_pagination():
     """Pretty much the same as static table pagination, but make sure we
-- 
2.26.2