Fix #413, ordering by custum field raises error

django_tables2/tables.py

@@ -19,6 +19,9 @@
 
 
 class TableData(object):
+    '''
+    Base class for table data containers.
+    '''
     def __init__(self, data, table):
         self.data = data
         self.table = table
@@ -30,28 +33,21 @@ def __getitem__(self, key):
         '''
         return self.data[key]
 
-    def accessors(self, aliases):
-        bound_column = None
-        accessors = []
-        for alias in aliases:
-            bound_column = self.table.columns[OrderBy(alias).bare]
-            # bound_column.order_by reflects the current ordering applied to
-            # the table. As such we need to check the current ordering on the
-            # column and use the opposite if it doesn't match the alias prefix.
-            if alias[0] != bound_column.order_by_alias[0]:
-                accessors += bound_column.order_by.opposite
-            else:
-                accessors += bound_column.order_by
-
-        return accessors
-
     def get_model(self):
-        return None
+        return getattr(self.data, 'model', None)
 
     @property
     def ordering(self):
         return None
 
+    @property
+    def verbose_name(self):
+        return 'item'
+
+    @property
+    def verbose_name_plural(self):
+        return 'items'
+
     @staticmethod
     def from_data(data, table):
         if TableQuerysetData.validate(data):
@@ -66,8 +62,25 @@ def from_data(data, table):
 
 
 class TableListData(TableData):
+    '''
+    Table data container for a list of dicts, for example::
+
+    [
+        {'name': 'John', 'age': 20},
+        {'name': 'Brian', 'age': 25}
+    ]
+
+    .. note::
+
+        Other structures might have worked in the past, but are not explicitly
+        supported or tested.
+    '''
+
     @staticmethod
     def validate(data):
+        '''
+        Validates `data` for use in this container
+        '''
         return (
             hasattr(data, '__iter__') or
             (hasattr(data, '__len__') and hasattr(data, '__getitem__'))
@@ -86,25 +99,31 @@ def order_by(self, aliases):
                 columns ('-' indicates descending order) in order of
                 significance with regard to data ordering.
         '''
-        accessors = self.accessors(aliases)
-        print 'accessors', accessors
-        self.data.sort(key=OrderByTuple(accessors).key)
+        accessors = []
+        for alias in aliases:
+            bound_column = self.table.columns[OrderBy(alias).bare]
 
-    @cached_property
-    def verbose_name(self):
-        return getattr(self.data, 'verbose_name', 'item')
+            # bound_column.order_by reflects the current ordering applied to
+            # the table. As such we need to check the current ordering on the
+            # column and use the opposite if it doesn't match the alias prefix.
+            if alias[0] != bound_column.order_by_alias[0]:
+                accessors += bound_column.order_by.opposite
+            else:
+                accessors += bound_column.order_by
 
-    @cached_property
-    def verbose_name_plural(self):
-        return getattr(self.data, 'verbose_name_plural', 'items')
+        self.data.sort(key=OrderByTuple(accessors).key)
 
 
 class TableQuerysetData(TableData):
-    def get_model(self):
-        return getattr(self.data, 'model', None)
+    '''
+    Table data container for a queryset.
+    '''
 
     @staticmethod
     def validate(data):
+        '''
+        Validates `data` for use in this container
+        '''
         return (
             hasattr(data, 'count') and callable(data.count) and
             hasattr(data, 'order_by') and callable(data.order_by)
@@ -150,7 +169,7 @@ def order_by(self, aliases):
                 columns ('-' indicates descending order) in order of
                 significance with regard to data ordering.
         '''
-        bound_column = None
+        modified_any = False
         accessors = []
         for alias in aliases:
             bound_column = self.table.columns[OrderBy(alias).bare]
@@ -162,11 +181,16 @@ def order_by(self, aliases):
             else:
                 accessors += bound_column.order_by
 
-        # Custom ordering
-        if bound_column:
-            self.data, modified = bound_column.order(self.data, alias[0] == '-')
-            if modified:
-                return
+            if bound_column:
+                queryset, modified = bound_column.order(self.data, alias[0] == '-')
+
+                if modified:
+                    self.data = queryset
+                    modified_any = True
+
+        # custom ordering
+        if modified_any:
+            return True
 
         # Traditional ordering
         if accessors:

tests/test_ordering.py

@@ -2,10 +2,11 @@
 from __future__ import absolute_import, unicode_literals
 
 import django_tables2 as tables
-import pytest
 from django.utils import six
 from django_tables2.tables import RequestConfig
 
+import pytest
+
 from .app.models import Person
 from .utils import build_request
 
@@ -121,14 +122,16 @@ def test_ordering_different_types():
     assert [] == table.rows[0].get_cell('beta')
 
 
-def test_multi_column_ordering():
-    brad = {'first_name': 'Bradley', 'last_name': 'Ayers'}
-    brad2 = {'first_name': 'Bradley', 'last_name': 'Fake'}
-    chris = {'first_name': 'Chris', 'last_name': 'Doble'}
-    davina = {'first_name': 'Davina', 'last_name': 'Adisusila'}
-    ross = {'first_name': 'Ross', 'last_name': 'Ayers'}
+brad = {'first_name': 'Bradley', 'last_name': 'Ayers'}
+brad2 = {'first_name': 'Bradley', 'last_name': 'Fake'}
+chris = {'first_name': 'Chris', 'last_name': 'Doble'}
+davina = {'first_name': 'Davina', 'last_name': 'Adisusila'}
+ross = {'first_name': 'Ross', 'last_name': 'Ayers'}
 
-    people = [brad, brad2, chris, davina, ross]
+people = [brad, brad2, chris, davina, ross]
+
+
+def test_multi_column_ordering_by_table():
 
     class PersonTable(tables.Table):
         first_name = tables.Column()
@@ -140,6 +143,8 @@ class PersonTable(tables.Table):
     table = PersonTable(people, order_by=('first_name', '-last_name'))
     assert [brad2, brad, chris, davina, ross] == [r.record for r in table.rows]
 
+
+def test_multi_column_ordering_by_column():
     # let's try column order_by using multiple keys
     class PersonTable(tables.Table):
         name = tables.Column(order_by=('first_name', 'last_name'))