From fb930f235710b5d762f9a95b734fd719c04c1ceb Mon Sep 17 00:00:00 2001 From: shawnnapora Date: Thu, 8 Sep 2016 21:58:01 -0400 Subject: [PATCH 1/3] Titlise base column verbose_name when derived from model --- django_tables2/columns/base.py | 25 ++++++++++++---- django_tables2/columns/booleancolumn.py | 5 ++-- django_tables2/columns/datecolumn.py | 4 ++- django_tables2/columns/datetimecolumn.py | 4 ++- django_tables2/columns/emailcolumn.py | 4 ++- django_tables2/columns/filecolumn.py | 3 +- django_tables2/columns/timecolumn.py | 4 ++- django_tables2/columns/urlcolumn.py | 4 ++- tests/app/models.py | 6 ++++ tests/test_models.py | 38 ++++++++++++++---------- tests/test_templatetags.py | 2 +- 11 files changed, 70 insertions(+), 29 deletions(-) diff --git a/django_tables2/columns/base.py b/django_tables2/columns/base.py index d5f86dde..cad4c966 100644 --- a/django_tables2/columns/base.py +++ b/django_tables2/columns/base.py @@ -5,6 +5,7 @@ from itertools import islice from django.utils import six +from django.utils.safestring import SafeData from django_tables2.templatetags.django_tables2 import title from django_tables2.utils import (Accessor, AttributeDict, OrderBy, @@ -223,7 +224,7 @@ def from_field(cls, field): verbose_name = field.get_related_field().verbose_name else: verbose_name = getattr(field, 'verbose_name', field.name) - return cls(verbose_name=verbose_name) + return cls(verbose_name=title(verbose_name)) @six.python_2_unicode_compatible @@ -437,8 +438,17 @@ def orderable(self): @property def verbose_name(self): """ - Return the verbose name for this column, or fallback to the titlised - column name. + Return the verbose name for this column. + + In order of preference, this will return: + 1) The column's explicitly defined verbose_name + 2) The titlised model's verbose name (if applicable) + 3) Fallback to the titlised column name. + + Any verbose_name that was not passed explicitly in the column + definition is returned titlised in keeping with the Django convention + of verbose_names being defined in lowercase and uppercased/titlised + as needed by the application. If the table is using queryset data, then use the corresponding model field's `~.db.Field.verbose_name`. If it's traversing a relationship, @@ -452,7 +462,7 @@ def verbose_name(self): # This is our reasonable fallback, should the next section not result # in anything useful. - name = title(self.name.replace('_', ' ')) + name = self.name.replace('_', ' ') # Try to use a model field's verbose_name if hasattr(self.table.data, 'queryset') and hasattr(self.table.data.queryset, 'model'): @@ -463,7 +473,12 @@ def verbose_name(self): name = field.field.verbose_name else: name = getattr(field, 'verbose_name', field.name) - return name + + # If verbose_name was mark_safe()'d, return intact to keep safety + if isinstance(name, SafeData): + return name + + return title(name) @property def visible(self): diff --git a/django_tables2/columns/booleancolumn.py b/django_tables2/columns/booleancolumn.py index 8c6a8ac7..10c011d5 100644 --- a/django_tables2/columns/booleancolumn.py +++ b/django_tables2/columns/booleancolumn.py @@ -5,6 +5,7 @@ from django.utils import six from django.utils.html import escape, format_html +from django_tables2.templatetags.django_tables2 import title from django_tables2.utils import AttributeDict from .base import Column, library @@ -58,6 +59,6 @@ def render(self, value, record, bound_column): @classmethod def from_field(cls, field): if isinstance(field, models.BooleanField): - return cls(verbose_name=field.verbose_name, null=False) + return cls(verbose_name=title(field.verbose_name), null=False) if isinstance(field, models.NullBooleanField): - return cls(verbose_name=field.verbose_name, null=True) + return cls(verbose_name=title(field.verbose_name), null=True) diff --git a/django_tables2/columns/datecolumn.py b/django_tables2/columns/datecolumn.py index c61b5f08..c5b0b196 100644 --- a/django_tables2/columns/datecolumn.py +++ b/django_tables2/columns/datecolumn.py @@ -3,6 +3,8 @@ from django.db import models +from django_tables2.templatetags.django_tables2 import title + from .base import library from .templatecolumn import TemplateColumn @@ -27,4 +29,4 @@ def __init__(self, format=None, short=True, *args, **kwargs): @classmethod def from_field(cls, field): if isinstance(field, models.DateField): - return cls(verbose_name=field.verbose_name) + return cls(verbose_name=title(field.verbose_name)) diff --git a/django_tables2/columns/datetimecolumn.py b/django_tables2/columns/datetimecolumn.py index 061188a4..360f0d33 100644 --- a/django_tables2/columns/datetimecolumn.py +++ b/django_tables2/columns/datetimecolumn.py @@ -3,6 +3,8 @@ from django.db import models +from django_tables2.templatetags.django_tables2 import title + from .base import library from .templatecolumn import TemplateColumn @@ -27,4 +29,4 @@ def __init__(self, format=None, short=True, *args, **kwargs): @classmethod def from_field(cls, field): if isinstance(field, models.DateTimeField): - return cls(verbose_name=field.verbose_name) + return cls(verbose_name=title(field.verbose_name)) diff --git a/django_tables2/columns/emailcolumn.py b/django_tables2/columns/emailcolumn.py index d9486ef5..46e0f9ba 100644 --- a/django_tables2/columns/emailcolumn.py +++ b/django_tables2/columns/emailcolumn.py @@ -3,6 +3,8 @@ from django.db import models +from django_tables2.templatetags.django_tables2 import title + from .base import library from .linkcolumn import BaseLinkColumn @@ -43,4 +45,4 @@ def render(self, record, value): @classmethod def from_field(cls, field): if isinstance(field, models.EmailField): - return cls(verbose_name=field.verbose_name) + return cls(verbose_name=title(field.verbose_name)) diff --git a/django_tables2/columns/filecolumn.py b/django_tables2/columns/filecolumn.py index 40d9a9b1..04bf3bb9 100644 --- a/django_tables2/columns/filecolumn.py +++ b/django_tables2/columns/filecolumn.py @@ -6,6 +6,7 @@ from django.db import models from django.utils.html import format_html +from django_tables2.templatetags.django_tables2 import title from django_tables2.utils import AttributeDict from .base import library @@ -84,4 +85,4 @@ def render(self, record, value): @classmethod def from_field(cls, field): if isinstance(field, models.FileField): - return cls(verbose_name=field.verbose_name) + return cls(verbose_name=title(field.verbose_name)) diff --git a/django_tables2/columns/timecolumn.py b/django_tables2/columns/timecolumn.py index dd97077b..9701f024 100644 --- a/django_tables2/columns/timecolumn.py +++ b/django_tables2/columns/timecolumn.py @@ -4,6 +4,8 @@ from django.conf import settings from django.db import models +from django_tables2.templatetags.django_tables2 import title + from .base import library from .templatecolumn import TemplateColumn @@ -28,4 +30,4 @@ def __init__(self, format=None, *args, **kwargs): @classmethod def from_field(cls, field): if isinstance(field, models.TimeField): - return cls(verbose_name=field.verbose_name) + return cls(verbose_name=title(field.verbose_name)) diff --git a/django_tables2/columns/urlcolumn.py b/django_tables2/columns/urlcolumn.py index 635e76f4..cbe6d61a 100644 --- a/django_tables2/columns/urlcolumn.py +++ b/django_tables2/columns/urlcolumn.py @@ -3,6 +3,8 @@ from django.db import models +from django_tables2.templatetags.django_tables2 import title + from .base import library from .linkcolumn import BaseLinkColumn @@ -32,4 +34,4 @@ def render(self, record, value): @classmethod def from_field(cls, field): if isinstance(field, models.URLField): - return cls(verbose_name=field.verbose_name) + return cls(verbose_name=title(field.verbose_name)) diff --git a/tests/app/models.py b/tests/app/models.py index 120c8a07..7f60744b 100644 --- a/tests/app/models.py +++ b/tests/app/models.py @@ -31,6 +31,12 @@ class Person(models.Model): safe = models.CharField( max_length=200, blank=True, verbose_name=mark_safe("Safe")) + website = models.URLField( + max_length=200, null=True, blank=True, + verbose_name="web site") + + birthdate = models.DateField(null=True) + content_type = models.ForeignKey(ContentType, null=True, blank=True) object_id = models.PositiveIntegerField(null=True, blank=True) foreign_key = GenericForeignKey() diff --git a/tests/test_models.py b/tests/test_models.py index 50b874f5..f08bcd4f 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -100,6 +100,7 @@ class PersonTable(tables.Table): fn1 = tables.Column(accessor='first_name') fn2 = tables.Column(accessor='first_name.upper') fn3 = tables.Column(accessor='last_name', verbose_name='OVERRIDE') + fn4 = tables.Column(accessor='last_name', verbose_name='override') last_name = tables.Column() ln1 = tables.Column(accessor='last_name') ln2 = tables.Column(accessor='last_name.upper') @@ -116,35 +117,42 @@ class PersonTable(tables.Table): # that we should expect that the two columns that use the ``last_name`` # field should both use the model's ``last_name`` field's ``verbose_name``, # however both fields that use the ``first_name`` field should just use a - # capitalized version of the column name as the column header. + # titlised version of the column name as the column header. table = PersonTable(Person.objects.all()) # Should be generated (capitalized column name) - assert 'first name' == table.columns['first_name'].verbose_name - assert 'first name' == table.columns['fn1'].verbose_name - assert 'first name' == table.columns['fn2'].verbose_name + assert 'First Name' == table.columns['first_name'].verbose_name + assert 'First Name' == table.columns['fn1'].verbose_name + assert 'First Name' == table.columns['fn2'].verbose_name assert 'OVERRIDE' == table.columns['fn3'].verbose_name - # Should use the model field's verbose_name - assert 'surname' == table.columns['last_name'].verbose_name - assert 'surname' == table.columns['ln1'].verbose_name - assert 'surname' == table.columns['ln2'].verbose_name + assert 'override' == table.columns['fn4'].verbose_name + # Should use the titlised model field's verbose_name + assert 'Surname' == table.columns['last_name'].verbose_name + assert 'Surname' == table.columns['ln1'].verbose_name + assert 'Surname' == table.columns['ln2'].verbose_name assert 'OVERRIDE' == table.columns['ln3'].verbose_name - assert 'name' == table.columns['region'].verbose_name - assert 'name' == table.columns['r1'].verbose_name - assert 'name' == table.columns['r2'].verbose_name + assert 'Name' == table.columns['region'].verbose_name + assert 'Name' == table.columns['r1'].verbose_name + assert 'Name' == table.columns['r2'].verbose_name assert 'OVERRIDE' == table.columns['r3'].verbose_name - assert "translation test" == table.columns["trans_test"].verbose_name - assert "translation test lazy" == table.columns["trans_test_lazy"].verbose_name + assert "Translation Test" == table.columns["trans_test"].verbose_name + assert "Translation Test Lazy" == table.columns["trans_test_lazy"].verbose_name # ------------------------------------------------------------------------- # Now we'll try using a table with Meta.model class PersonTable(tables.Table): + first_name = tables.Column(verbose_name="OVERRIDE") + class Meta: model = Person + # Issue #16 table = PersonTable([]) - assert "translation test" == table.columns["trans_test"].verbose_name - assert "translation test lazy" == table.columns["trans_test_lazy"].verbose_name + assert "Translation Test" == table.columns["trans_test"].verbose_name + assert "Translation Test Lazy" == table.columns["trans_test_lazy"].verbose_name + assert "Web Site" == table.columns["website"].verbose_name + assert "Birthdate" == table.columns["birthdate"].verbose_name + assert "OVERRIDE" == table.columns["first_name"].verbose_name def test_data_verbose_name(): diff --git a/tests/test_templatetags.py b/tests/test_templatetags.py index e2ed1efd..9733a4b1 100644 --- a/tests/test_templatetags.py +++ b/tests/test_templatetags.py @@ -106,7 +106,7 @@ def test_render_table_supports_queryset(): 'request': build_request('/')})) root = parse(html) - assert [e.text for e in root.findall('.//thead/tr/th/a')] == ['ID', 'name', 'mayor'] + assert [e.text for e in root.findall('.//thead/tr/th/a')] == ['ID', 'Name', 'Mayor'] td = [[td.text for td in tr.findall('td')] for tr in root.findall('.//tbody/tr')] db = [] for region in Region.objects.all(): From 79c42644e61a1fc90c3658ca48c2c4dff5352426 Mon Sep 17 00:00:00 2001 From: shawnnapora Date: Fri, 9 Sep 2016 08:57:16 -0400 Subject: [PATCH 2/3] Fixed comments in columns/base.py --- django_tables2/columns/base.py | 8 ++++---- django_tables2/columns/booleancolumn.py | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/django_tables2/columns/base.py b/django_tables2/columns/base.py index cad4c966..70291500 100644 --- a/django_tables2/columns/base.py +++ b/django_tables2/columns/base.py @@ -441,13 +441,13 @@ def verbose_name(self): Return the verbose name for this column. In order of preference, this will return: - 1) The column's explicitly defined verbose_name - 2) The titlised model's verbose name (if applicable) + 1) The column's explicitly defined `verbose_name` + 2) The titlised model's `verbose_name` (if applicable) 3) Fallback to the titlised column name. - Any verbose_name that was not passed explicitly in the column + Any `verbose_name` that was not passed explicitly in the column definition is returned titlised in keeping with the Django convention - of verbose_names being defined in lowercase and uppercased/titlised + of `verbose_name` being defined in lowercase and uppercased/titlised as needed by the application. If the table is using queryset data, then use the corresponding model diff --git a/django_tables2/columns/booleancolumn.py b/django_tables2/columns/booleancolumn.py index 10c011d5..534ea566 100644 --- a/django_tables2/columns/booleancolumn.py +++ b/django_tables2/columns/booleancolumn.py @@ -58,7 +58,8 @@ def render(self, value, record, bound_column): @classmethod def from_field(cls, field): + verbose_name = title(field.verbose_name) if isinstance(field, models.BooleanField): - return cls(verbose_name=title(field.verbose_name), null=False) + return cls(verbose_name=verbose_name, null=False) if isinstance(field, models.NullBooleanField): - return cls(verbose_name=title(field.verbose_name), null=True) + return cls(verbose_name=verbose_name, null=True) From be825fa8a2dc9acc49b75aa0a5351a1649501bca Mon Sep 17 00:00:00 2001 From: shawnnapora Date: Fri, 9 Sep 2016 10:43:04 -0400 Subject: [PATCH 3/3] Reverted booleantable simplification --- django_tables2/columns/booleancolumn.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/django_tables2/columns/booleancolumn.py b/django_tables2/columns/booleancolumn.py index 534ea566..10c011d5 100644 --- a/django_tables2/columns/booleancolumn.py +++ b/django_tables2/columns/booleancolumn.py @@ -58,8 +58,7 @@ def render(self, value, record, bound_column): @classmethod def from_field(cls, field): - verbose_name = title(field.verbose_name) if isinstance(field, models.BooleanField): - return cls(verbose_name=verbose_name, null=False) + return cls(verbose_name=title(field.verbose_name), null=False) if isinstance(field, models.NullBooleanField): - return cls(verbose_name=verbose_name, null=True) + return cls(verbose_name=title(field.verbose_name), null=True)