From 5cf77d6f5efa496ecd8701de75a3ae7214b9372f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Michael=20Elsd=C3=B6rfer?= Date: Wed, 18 Jun 2008 12:43:57 +0000 Subject: [PATCH] static tables were implemented incorrectly, were using column aliases to read from source --- django_tables/models.py | 18 ---------- django_tables/tables.py | 78 +++++++++++++++++++++++++++++++++-------- tests/test_basic.py | 8 ++--- tests/test_models.py | 8 ++--- tests/test_templates.py | 9 +++-- 5 files changed, 76 insertions(+), 45 deletions(-) diff --git a/django_tables/models.py b/django_tables/models.py index ab40038..1747442 100644 --- a/django_tables/models.py +++ b/django_tables/models.py @@ -79,24 +79,6 @@ class BaseModelTable(BaseTable): super(BaseModelTable, self).__init__(self.queryset, *args, **kwargs) - def _cols_to_fields(self, names): - """Utility function. Given a list of column names (as exposed to the - user), converts overwritten column names to their corresponding model - field name. - - Supports prefixed field names as used e.g. in order_by ("-field"). - """ - result = [] - for ident in names: - if ident[:1] == '-': - name = ident[1:] - prefix = '-' - else: - name = ident - prefix = '' - result.append(prefix + self.columns[name].declared_name) - return result - def _validate_column_name(self, name, purpose): """Overridden. Only allow model-based fields to be sorted.""" if purpose == 'order_by': diff --git a/django_tables/tables.py b/django_tables/tables.py index 556fff1..eeecde0 100644 --- a/django_tables/tables.py +++ b/django_tables/tables.py @@ -117,18 +117,34 @@ class BaseTable(object): self.base_columns = copy.deepcopy(type(self).base_columns) def _build_snapshot(self): + """Rebuilds the table whenever it's options change. + + Whenver the table options change, e.g. say a new sort order, + this method will be asked to regenerate the actual table from + the linked data source. + + In the case of this base table implementation, a copy of the + source data is created, and then modified appropriately. + """ snapshot = copy.copy(self._data) for row in snapshot: - # delete unknown columns and add missing ones; note that - # self.columns already accounts for column name overrides. - for column in row.keys(): - if not column in self.columns: - del row[column] - for colname, colobj in self.columns.items(): - if not colname in row: - row[colname] = colobj.column.default + # delete unknown columns that are in the source data, but that + # we don't know about. + # TODO: does this even make sense? we might in some cases save + # memory, but at what performance cost? + decl_colnames = [c.declared_name for c in self.columns.all()] + for key in row.keys(): + if not key in decl_colnames: + del row[key] + # add data for defined columns that is missing from the source. + # we do this so that colunn default values can affect sorting, + # which is the current design decision. + for column in self.columns.all(): + if not column.declared_name in row: + row[column.declared_name] = column.column.default + if self.order_by: - sort_table(snapshot, self.order_by) + sort_table(snapshot, self._cols_to_fields(self.order_by)) self._snapshot = snapshot def _get_data(self): @@ -137,6 +153,28 @@ class BaseTable(object): return self._snapshot data = property(lambda s: s._get_data()) + def _cols_to_fields(self, names): + """Utility function. Given a list of column names (as exposed to + the user), converts column names to the names we have to use to + retrieve a column's data from the source. + + Right now, the name used in the table declaration is used for + access, but a column can define it's own alias-like name that will + be used to refer to the column from outside. + + Supports prefixed column names as used e.g. in order_by ("-field"). + """ + result = [] + for ident in names: + if ident[:1] == '-': + name = ident[1:] + prefix = '-' + else: + name = ident + prefix = '' + result.append(prefix + self.columns[name].declared_name) + return result + def _validate_column_name(self, name, purpose): """Return True/False, depending on whether the column ``name`` is valid for ``purpose``. Used to validate things like ``order_by`` @@ -160,7 +198,6 @@ class BaseTable(object): # validate, remove all invalid order instructions self._order_by = OrderByTuple([o for o in self._order_by if self._validate_column_name((o[:1]=='-' and [o[1:]] or [o])[0], "order_by")]) - # TODO: optionally, throw an exception order_by = property(lambda s: s._order_by, _set_order_by) def __unicode__(self): @@ -225,6 +262,11 @@ class Columns(object): self._columns = new_columns def all(self): + """Iterate through all columns, regardless of visiblity (as + opposed to ``__iter__``. + + This is used internally a lot. + """ self._spawn_columns() for column in self._columns.values(): yield column @@ -234,7 +276,7 @@ class Columns(object): for r in self._columns.items(): yield r - def keys(self): + def names(self): self._spawn_columns() for r in self._columns.keys(): yield r @@ -244,6 +286,10 @@ class Columns(object): return self._columns.keyOrder.index(name) def __iter__(self): + """Iterate through all *visible* bound columns. + + This is primarily geared towards table rendering. + """ for column in self.all(): if column.column.visible: yield column @@ -252,7 +298,7 @@ class Columns(object): """Check by both column object and column name.""" self._spawn_columns() if isinstance(item, basestring): - return item in self.keys() + return item in self.names() else: return item in self.all() @@ -266,7 +312,11 @@ class BoundColumn(StrAndUnicode): """'Runtime' version of ``Column`` that is bound to a table instance, and thus knows about the table's data. - Note name... TODO + Note that the name that is passed in tells us how this field is + delared in the bound table. The column itself can overwrite this name. + While the overwritten name will be hat mostly counts, we need to + remember the one used for declaration as well, or we won't know how + to read a column's value from the source. """ def __init__(self, table, column, name): self.table = table @@ -306,7 +356,7 @@ class BoundRow(object): def __getitem__(self, name): """Returns this row's value for a column. All other access methods, e.g. __iter__, lead ultimately to this.""" - return self.data[name] + return self.data[self.table.columns[name].declared_name] def __contains__(self, item): """Check by both row object and column name.""" diff --git a/tests/test_basic.py b/tests/test_basic.py index dee5d60..7b5cfab 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -87,10 +87,10 @@ def test_sort(): language = tables.Column(default='en') # default affects sorting books = BookTable([ - {'id': 1, 'num_pages': 60, 'name': 'Z: The Book'}, # language: en - {'id': 2, 'num_pages': 100, 'language': 'de', 'name': 'A: The Book'}, - {'id': 3, 'num_pages': 80, 'language': 'de', 'name': 'A: The Book, Vol. 2'}, - {'id': 4, 'num_pages': 110, 'language': 'fr', 'name': 'A: The Book, French Edition'}, + {'id': 1, 'pages': 60, 'name': 'Z: The Book'}, # language: en + {'id': 2, 'pages': 100, 'language': 'de', 'name': 'A: The Book'}, + {'id': 3, 'pages': 80, 'language': 'de', 'name': 'A: The Book, Vol. 2'}, + {'id': 4, 'pages': 110, 'language': 'fr', 'name': 'A: The Book, French Edition'}, ]) def test_order(order, result): diff --git a/tests/test_models.py b/tests/test_models.py index eb32509..9d9f3e4 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -144,11 +144,11 @@ def test_sort(): test_order('-population', [2,3,4,1]) test_order('system,-population', [2,4,3,1]) - # test invalid order instructions + # test invalid order instructions... countries.order_by = 'invalid_field,population' assert countries.order_by == ('population',) - # ...for modeltables, this primarily means that only model-based colunns - # are currently sortable at all. + # ...in case of ModelTables, this primarily means that only + # model-based colunns are currently sortable at all. countries.order_by = ('custom1', 'custom2') assert countries.order_by == () @@ -161,4 +161,4 @@ def test_pagination(): # TODO: manual base columns change -> update() call (add as example in docstr here) -> rebuild snapshot: is row cache, column cache etc. reset? # TODO: test that boundcolumn.name works with name overrides and without # TODO: more beautiful auto column names -# TODO: normal tables should handle name overrides differently; the backend data should still use declared_name \ No newline at end of file +# TODO: throw an exception on invalid order_by \ No newline at end of file diff --git a/tests/test_templates.py b/tests/test_templates.py index 9c0852f..e49c168 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -67,7 +67,6 @@ def test_columns_and_rows(): assert countries.columns['name'].visible == True assert countries.columns['tld'].visible == False - def test_render(): """For good measure, render some actual templates.""" @@ -80,10 +79,10 @@ def test_render(): calling_code = tables.NumberColumn(name="cc", verbose_name="Phone Ext.") countries = CountryTable( - [{'name': 'Germany', 'capital': 'Berlin', 'population': 83, 'currency': 'Euro (€)', 'tld': 'de', 'cc': 49}, - {'name': 'France', 'population': 64, 'currency': 'Euro (€)', 'tld': 'fr', 'cc': 33}, - {'name': 'Netherlands', 'capital': 'Amsterdam', 'cc': '31'}, - {'name': 'Austria', 'cc': 43, 'currency': 'Euro (€)', 'population': 8}]) + [{'name': 'Germany', 'capital': 'Berlin', 'population': 83, 'currency': 'Euro (€)', 'tld': 'de', 'calling_code': 49}, + {'name': 'France', 'population': 64, 'currency': 'Euro (€)', 'tld': 'fr', 'calling_code': 33}, + {'name': 'Netherlands', 'capital': 'Amsterdam', 'calling_code': '31'}, + {'name': 'Austria', 'calling_code': 43, 'currency': 'Euro (€)', 'population': 8}]) from django.template import Template, Context -- 2.26.2