Second pass in adding column.linkify() api

django_tables2/columns/base.py

@@ -65,7 +65,13 @@ def __init__(self, **kwargs):
         for key, value in kwargs.items():
             setattr(self, key, value)
 
-    def compose_url(self, record, bound_column):
+    def compose_url(self, **kwargs):
+        if hasattr(self, "uri"):
+            return call_with_appropriate(self.uri, kwargs)
+
+        bound_column = kwargs["bound_column"]
+        record = kwargs["record"]
+
         accessor = Accessor(self.accessor if self.accessor is not None else bound_column.name)
         if self.viewname is None:
             context = accessor.resolve(record)
@@ -74,8 +80,8 @@ def compose_url(self, record, bound_column):
                     context = record
                 else:
                     raise TypeError(
-                        "if viewname=None, record {} must define a get_absolute_url".format(
-                            context.__repr__()
+                        "if viewname=None, '{}' must have a method get_absolute_url".format(
+                            str(context)
                         )
                     )
             return context.get_absolute_url()
@@ -98,9 +104,9 @@ def resolve_if_accessor(val):
 
         return reverse(viewname, **params)
 
-    def get_attrs(self, record, bound_column):
+    def get_attrs(self, **kwargs):
         attrs = AttributeDict(self.attrs or {})
-        attrs["href"] = self.compose_url(record, bound_column)
+        attrs["href"] = self.compose_url(**kwargs)
 
         return attrs
 
@@ -308,7 +314,7 @@ def linkify(
         accessor=None,
     ):
         """
-        Wraps the content of the current cell into an <a> tag.
+        Wraps the content of the current cell into an <a> tag. Overrides the currenly setup link.
         """
         self.link = CellLink(
             viewname=viewname,

django_tables2/columns/emailcolumn.py

@@ -5,7 +5,7 @@
 
 from django_tables2.utils import ucfirst
 
-from .base import library
+from .base import CellLink, library
 from .linkcolumn import BaseLinkColumn
 
 
@@ -36,8 +36,12 @@ class PeopleTable(tables.Table):
         # [...]<a href="mailto:email@example.com">email@example.com</a>
     """
 
-    def render(self, record, value):
-        return self.render_link(uri="mailto:{}".format(value), record=record, value=value)
+    def __init__(self, *args, **kwargs):
+        super(EmailColumn, self).__init__(*args, **kwargs)
+
+        self.link = CellLink(
+            column=self, uri=lambda value: "mailto:{}".format(value), attrs=self.attrs.get("a")
+        )
 
     @classmethod
     def from_field(cls, field):

django_tables2/columns/filecolumn.py

@@ -8,7 +8,7 @@
 
 from django_tables2.utils import AttributeDict, ucfirst
 
-from .base import library
+from .base import CellLink, library
 from .linkcolumn import BaseLinkColumn
 
 
@@ -19,13 +19,13 @@ class FileColumn(BaseLinkColumn):
     hyperlink.
 
     When the file is accessible via a URL, the file is rendered as a
-    hyperlink. The `.basename` is used as the text::
+    hyperlink. The `.basename` is used as the text, wrapped in a span::
 
         <a href="/media/path/to/receipt.pdf" title="path/to/receipt.pdf">receipt.pdf</a>
 
     When unable to determine the URL, a ``span`` is used instead::
 
-        <span title="path/to/receipt.pdf">receipt.pdf</span>
+        <span title="path/to/receipt.pdf" class>receipt.pdf</span>
 
     `.Column.attrs` keys ``a`` and ``span`` can be used to add additional attributes.
 
@@ -42,45 +42,48 @@ def __init__(self, verify_exists=True, **kwargs):
         self.verify_exists = verify_exists
         super(FileColumn, self).__init__(**kwargs)
 
+        self.link = CellLink(column=self, uri=self.get_uri, attrs=self.attrs.get("a"))
+
+    def get_uri(self, value, record):
+        storage = getattr(value, "storage", None)
+        if not storage:
+            return None
+
+        return storage.url(value.name)
+
     def text_value(self, record, value):
         if self.text is None:
             return os.path.basename(value.name)
         return super(FileColumn, self).text_value(record, value)
 
     def render(self, record, value):
-        storage = getattr(value, "storage", None)
+        attrs = AttributeDict(self.attrs.get("span", {}))
+        classes = [c for c in attrs.get("class", "").split(" ") if c]
+
         exists = None
-        url = None
+        storage = getattr(value, "storage", None)
         if storage:
             # we'll assume value is a `django.db.models.fields.files.FieldFile`
             if self.verify_exists:
                 exists = storage.exists(value.name)
-            url = storage.url(value.name)
-
         else:
             if self.verify_exists and hasattr(value, "name"):
                 # ignore negatives, perhaps the file has a name but it doesn't
                 # represent a local path... better to stay neutral than give a
                 # false negative.
                 exists = os.path.exists(value.name) or exists
 
-        tag = "a" if url else "span"
-        attrs = AttributeDict(self.attrs.get(tag, {}))
-        attrs["title"] = value.name
-
-        classes = [c for c in attrs.get("class", "").split(" ") if c]
         if exists is not None:
             classes.append("exists" if exists else "missing")
+
+        attrs["title"] = value.name
         attrs["class"] = " ".join(classes)
 
-        if url:
-            return self.render_link(url, record=record, value=value, attrs=attrs)
-        else:
-            return format_html(
-                "<span {attrs}>{text}</span>",
-                attrs=attrs.as_html(),
-                text=self.text_value(record, value),
-            )
+        return format_html(
+            "<span {attrs}>{text}</span>",
+            attrs=attrs.as_html(),
+            text=self.text_value(record, value),
+        )
 
     @classmethod
     def from_field(cls, field):

django_tables2/columns/linkcolumn.py

@@ -1,11 +1,6 @@
 # coding: utf-8
 from __future__ import absolute_import, unicode_literals
 
-from django.urls import reverse
-from django.utils.html import format_html
-
-from django_tables2.utils import Accessor, AttributeDict
-
 from .base import CellLink, Column, library
 
 
@@ -23,41 +18,25 @@ class BaseLinkColumn(Column):
              - `a` -- ``<a>`` in ``<td>`` elements.
     """
 
-    def __init__(self, attrs=None, text=None, *args, **kwargs):
-        kwargs["attrs"] = attrs
-        self.text = text
+    def __init__(self, text=None, *args, **kwargs):
         super(BaseLinkColumn, self).__init__(*args, **kwargs)
+        self.text = text
 
     def text_value(self, record, value):
         if self.text is None:
             return value
         return self.text(record) if callable(self.text) else self.text
 
-    def render_link(self, uri, record, value, attrs=None):
-        """
-        Render a link (`<a>`).
-
-        Arguments:
-            uri (str): URI for the link
-            record: record currently being rendered
-            value (str): value to be wrapped in ``<a></a>``, might be overridden
-                by ``self.text``
-            attrs (dict): ``<a>`` tag attributes
-        """
-        attrs = AttributeDict(attrs if attrs is not None else self.attrs.get("a", {}))
-        attrs["href"] = uri
-
-        return format_html(
-            "<a {attrs}>{text}</a>", attrs=attrs.as_html(), text=self.text_value(record, value)
-        )
-
     def value(self, record, value):
         """
         Returns the content for a specific cell similarly to `.render` however
         without any html content.
         """
         return self.text_value(record, value)
 
+    def render(self, record, value):
+        return self.text_value(record, value)
+
 
 @library.register
 class LinkColumn(BaseLinkColumn):
@@ -146,12 +125,13 @@ def __init__(
         viewname=None,
         urlconf=None,
         args=None,
+        uri=None,
         kwargs=None,
         current_app=None,
         attrs=None,
         **extra
     ):
-        super(LinkColumn, self).__init__(attrs, **extra)
+        super(LinkColumn, self).__init__(attrs=attrs, **extra)
 
         self.link = CellLink(
             column=self,
@@ -192,11 +172,3 @@ class PersonTable(tables.Table):
     Alternative contents of ``<a>`` can be supplied using the ``text`` keyword argument as
     documented for `~.columns.LinkColumn`.
     """
-
-    def compose_url(self, record, bound_column):
-        accessor = self.accessor if self.accessor else Accessor(bound_column.name)
-
-        try:
-            return accessor.resolve(record).get_absolute_url()
-        except (AttributeError, TypeError):
-            return "#"

django_tables2/columns/urlcolumn.py

@@ -5,7 +5,7 @@
 
 from django_tables2.utils import ucfirst
 
-from .base import library
+from .base import CellLink, library
 from .linkcolumn import BaseLinkColumn
 
 
@@ -29,8 +29,10 @@ class URLColumn(BaseLinkColumn):
         '<a href="http://google.com">http://google.com</a>'
     """
 
-    def render(self, record, value):
-        return self.render_link(value, record=record, value=value)
+    def __init__(self, *args, **kwargs):
+        super(URLColumn, self).__init__(*args, **kwargs)
+
+        self.link = CellLink(column=self, uri=lambda value: value, attrs=self.attrs.get("a"))
 
     @classmethod
     def from_field(cls, field):

django_tables2/rows.py

@@ -216,9 +216,9 @@ def _call_render(self, bound_column, value=None):
         if not bound_column.link:
             return content
 
-        attrs = bound_column.link.get_attrs(
-            bound_column=render_kwargs["bound_column"], record=render_kwargs["record"]
-        )
+        attrs = bound_column.link.get_attrs(**render_kwargs)
+        if attrs["href"] is None:
+            return content
         return format_html("<a {}>{}</a>", attrs.as_html(), content)
 
     def get_cell_value(self, name):

example/app/tables.py

@@ -2,7 +2,6 @@
 from __future__ import unicode_literals
 
 import django_tables2 as tables
-from django_tables2 import A
 
 from .models import Country, Person
 

tests/columns/test_filecolumn.py

@@ -36,59 +36,60 @@ class Table(tables.Table):
             class Meta:
                 model = FileModel
 
-        assert type(Table.base_columns["field"]) == tables.FileColumn
+        self.assertEqual(type(Table.base_columns["field"]), tables.FileColumn)
 
     def test_filecolumn_supports_storage_file(self):
         file_ = storage().open("child/foo.html")
         try:
             root = parse(column().render(value=file_, record=None))
         finally:
             file_.close()
-        path = file_.name
-        assert root.tag == "span"
-        assert root.attrib == {"class": "span exists", "title": path}
-        assert root.text == "foo.html"
+
+        self.assertEqual(root.tag, "span")
+        self.assertEqual(root.attrib, {"class": "span exists", "title": file_.name})
+        self.assertEqual(root.text, "foo.html")
 
     def test_filecolumn_supports_contentfile(self):
         name = "foobar.html"
         file_ = ContentFile("")
         file_.name = name
+
         root = parse(column().render(value=file_, record=None))
-        assert root.tag == "span"
-        assert root.attrib == {"title": name, "class": "span"}
-        assert root.text == "foobar.html"
+        self.assertEqual(root.tag, "span")
+        self.assertEqual(root.attrib, {"title": name, "class": "span"})
+        self.assertEqual(root.text, "foobar.html")
 
     def test_filecolumn_supports_fieldfile(self):
         field = models.FileField(storage=storage())
         name = "child/foo.html"
-        fieldfile = FieldFile(instance=None, field=field, name=name)
-        root = parse(column().render(value=fieldfile, record=None))
-        assert root.tag == "a"
-        assert root.attrib == {
-            "class": "a exists",
-            "title": name,
-            "href": "/baseurl/child/foo.html",
-        }
-        assert root.text == "foo.html"
+
+        class Table(tables.Table):
+            filecolumn = column()
+
+        table = Table([{"filecolumn": FieldFile(instance=None, field=field, name=name)}])
+        html = table.rows[0].get_cell("filecolumn")
+        root = parse(html)
+
+        self.assertEqual(root.tag, "a")
+        self.assertEqual(root.attrib, {"class": "a", "href": "/baseurl/child/foo.html"})
+        span = root.find("span")
+        self.assertEqual(span.tag, "span")
+        self.assertEqual(span.text, "foo.html")
 
         # Now try a file that doesn't exist
         name = "child/does_not_exist.html"
         fieldfile = FieldFile(instance=None, field=field, name=name)
-        html = column().render(value=fieldfile, record=None)
-        root = parse(html)
-        assert root.tag == "a"
-        assert root.attrib == {
-            "class": "a missing",
-            "title": name,
-            "href": "/baseurl/child/does_not_exist.html",
-        }
-        assert root.text == "does_not_exist.html"
+        root = parse(column().render(value=fieldfile, record=None))
+
+        self.assertEqual(root.tag, "span")
+        self.assertEqual(root.attrib, {"class": "span missing", "title": name})
+        self.assertEqual(root.text, "does_not_exist.html")
 
     def test_filecolumn_text_custom_value(self):
-        name = "foobar.html"
         file_ = ContentFile("")
-        file_.name = name
+        file_.name = "foobar.html"
+
         root = parse(tables.FileColumn(text="Download").render(value=file_, record=None))
-        assert root.tag == "span"
-        assert root.attrib == {"title": name, "class": ""}
-        assert root.text == "Download"
+        self.assertEqual(root.tag, "span")
+        self.assertEqual(root.attrib, {"title": file_.name, "class": ""})
+        self.assertEqual(root.text, "Download")

tests/columns/test_linkcolumn.py

@@ -203,7 +203,9 @@ class Table(tables.Table):
 
         table = Table([{"occupation": "Fabricator"}])
 
-        self.assertEqual(table.rows[0].cells["occupation"], '<a href="#">Fabricator</a>')
+        msg = "if viewname=None, 'Fabricator' must have a method get_absolute_url"
+        with self.assertRaisesMessage(TypeError, msg):
+            table.rows[0].cells["occupation"]
 
     def test_value_returns_a_raw_value_without_html(self):
         class Table(tables.Table):