From a62227bf5fa5ce055a87b4ce151cbd6f8af9eab5 Mon Sep 17 00:00:00 2001 From: Ben Mehlman Date: Wed, 6 Nov 2019 15:18:47 -0500 Subject: [PATCH 1/6] Added ability to specify query and fragment elements of url when using linkify. Squashed commit of the following: commit f0199f22905843dab101a122545ea897aa2aa60a Author: Ben Mehlman Date: Wed Nov 6 15:07:42 2019 -0500 When passing linkify as dict, QUERY and FRAGMENT now lowercase (ie. query, fragment). Update documentation to explain these features. commit e14b5296f98d00b17b4a368aba7401da49bd5c67 Author: Ben Mehlman Date: Tue Nov 5 22:22:01 2019 -0500 Add query string and fragment identifier capability to linkify. --- django_tables2/columns/base.py | 66 +++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/django_tables2/columns/base.py b/django_tables2/columns/base.py index 2c2a26e9..af9b54a2 100644 --- a/django_tables2/columns/base.py +++ b/django_tables2/columns/base.py @@ -1,5 +1,6 @@ from collections import OrderedDict from itertools import islice +from urllib.parse import urlencode from django.core.exceptions import ImproperlyConfigured from django.urls import reverse @@ -65,7 +66,7 @@ class LinkTransform: accessor = None attrs = None - def __init__(self, url=None, accessor=None, attrs=None, reverse_args=None): + def __init__(self, url=None, accessor=None, attrs=None, reverse_args=None, query=None, fragment=None): """ arguments: url (callable): If supplied, the result of this callable will be used as ``href`` attribute. @@ -79,6 +80,8 @@ def __init__(self, url=None, accessor=None, attrs=None, reverse_args=None): self.url = url self.attrs = attrs self.accessor = accessor + self.query = query + self.fragment = fragment if isinstance(reverse_args, (list, tuple)): viewname, args = reverse_args @@ -95,23 +98,33 @@ def compose_url(self, **kwargs): record = kwargs["record"] if self.reverse_args.get("viewname", None) is not None: - return self.call_reverse(record=record) - - if bound_column is None and self.accessor is None: - accessor = Accessor("") + url = self.call_reverse(record=record) else: - accessor = Accessor(self.accessor if self.accessor is not None else bound_column.name) - context = accessor.resolve(record) - if not hasattr(context, "get_absolute_url"): - if hasattr(record, "get_absolute_url"): - context = record + if bound_column is None and self.accessor is None: + accessor = Accessor("") else: - raise TypeError( - "for linkify=True, '{}' must have a method get_absolute_url".format( - str(context) + accessor = Accessor(self.accessor if self.accessor is not None else bound_column.name) + context = accessor.resolve(record) + if not hasattr(context, "get_absolute_url"): + if hasattr(record, "get_absolute_url"): + context = record + else: + raise TypeError( + "for linkify=True, '{}' must have a method get_absolute_url".format( + str(context) + ) ) - ) - return context.get_absolute_url() + url = context.get_absolute_url() + + if self.query: + url += '?' + urlencode({ + a: v.resolve(record) if isinstance(v, Accessor) else v + for a, v in self.query.items() + }) + if self.fragment: + url += '#' + self.fragment + + return url def call_reverse(self, record): """ @@ -150,6 +163,19 @@ def __call__(self, content, **kwargs): return format_html("{}", attrs.as_html(), content) + @classmethod + def get_callback(cls, *args, **kwargs): + """ + This method constructs a LinkTransform and returns a callback function suitable as the linkify + parameter of ``Column.__init__()`` This may be used if you wish to subclass LinkTransform or to access + (future) features of LinkTransform which may not be exposed via ``linkify``. + """ + link_transform = cls(*args, **kwargs) + + def callback(record, value, **kwargs): + return link_transform.compose_url(record=record, value=value, **kwargs) + + return callback @library.register class Column: @@ -209,7 +235,9 @@ class Column: - If `True`, the ``record.get_absolute_url()`` or the related model's `get_absolute_url()` is used. - If a callable is passed, the returned value is used, if it's not ``None``. - - If a `dict` is passed, it's passed on to ``~django.urls.reverse``. + - If a `dict` is passed, optional items named ``query`` (dict), and ``fragment`` (str), + if present, specify the query string and fragment (hash) elements of the url. + The remaining items are passed on to ``~django.urls.reverse`` as kwargs. - If a `tuple` is passed, it must be either a (viewname, args) or (viewname, kwargs) tuple, which is also passed to ``~django.urls.reverse``. @@ -295,8 +323,12 @@ def __init__( link_kwargs = None if callable(linkify) or hasattr(self, "get_url"): link_kwargs = dict(url=linkify if callable(linkify) else self.get_url) - elif isinstance(linkify, (dict, tuple)): + elif isinstance(linkify, (list, tuple)): link_kwargs = dict(reverse_args=linkify) + elif isinstance(linkify, dict): + # specific keys in linkify are understood to be link_kwargs, and the rest must be reverse_args + link_kwargs = { name: linkify.pop(name) for name in ('query', 'fragment') if name in linkify } + link_kwargs['reverse_args'] = linkify elif linkify is True: link_kwargs = dict(accessor=self.accessor) From af13e95688458c4a0c99dbb43883cf2e1bd405b8 Mon Sep 17 00:00:00 2001 From: Ben Mehlman Date: Thu, 7 Nov 2019 12:26:27 -0500 Subject: [PATCH 2/6] In patch for issue #687, clean up to pass flake8 and spell check. --- django_tables2/columns/base.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/django_tables2/columns/base.py b/django_tables2/columns/base.py index af9b54a2..efa0df45 100644 --- a/django_tables2/columns/base.py +++ b/django_tables2/columns/base.py @@ -76,6 +76,8 @@ def __init__(self, url=None, accessor=None, attrs=None, reverse_args=None, query reverse_args (dict, tuple): Arguments to ``django.urls.reverse()``. If dict, the arguments are assumed to be keyword arguments to ``reverse()``, if tuple, a ``(viewname, args)`` or ``(viewname, kwargs)`` + query (dict): If supplied, field-value pairs to be formatted into a query string. + fragment (str): If supplied, value of URL fragment identifier (hash). """ self.url = url self.attrs = attrs @@ -177,6 +179,7 @@ def callback(record, value, **kwargs): return callback + @library.register class Column: """ @@ -235,8 +238,8 @@ class Column: - If `True`, the ``record.get_absolute_url()`` or the related model's `get_absolute_url()` is used. - If a callable is passed, the returned value is used, if it's not ``None``. - - If a `dict` is passed, optional items named ``query`` (dict), and ``fragment`` (str), - if present, specify the query string and fragment (hash) elements of the url. + - If a `dict` is passed, optional items named ``query`` (dict), and ``fragment`` (str), + if present, specify the query string and fragment (hash) elements of the URL. The remaining items are passed on to ``~django.urls.reverse`` as kwargs. - If a `tuple` is passed, it must be either a (viewname, args) or (viewname, kwargs) tuple, which is also passed to ``~django.urls.reverse``. @@ -327,7 +330,7 @@ def __init__( link_kwargs = dict(reverse_args=linkify) elif isinstance(linkify, dict): # specific keys in linkify are understood to be link_kwargs, and the rest must be reverse_args - link_kwargs = { name: linkify.pop(name) for name in ('query', 'fragment') if name in linkify } + link_kwargs = {name: linkify.pop(name) for name in ('query', 'fragment') if name in linkify} link_kwargs['reverse_args'] = linkify elif linkify is True: link_kwargs = dict(accessor=self.accessor) From 53916f92b798161b8ed6adc23a2daf4d00bde6a2 Mon Sep 17 00:00:00 2001 From: Ben Mehlman Date: Mon, 11 Nov 2019 13:49:23 -0500 Subject: [PATCH 3/6] Add "query" and "fragment" kwargs to LinkColumn, add tests for same. Fix formatting (make black and flake8 happy) --- django_tables2/columns/base.py | 32 +++++++++++++++++----------- django_tables2/columns/linkcolumn.py | 6 ++++++ requirements/common.pip | 1 + tests/columns/test_linkcolumn.py | 25 ++++++++++++++++++++++ tox.ini | 4 ++-- 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/django_tables2/columns/base.py b/django_tables2/columns/base.py index efa0df45..8a2796ab 100644 --- a/django_tables2/columns/base.py +++ b/django_tables2/columns/base.py @@ -66,7 +66,9 @@ class LinkTransform: accessor = None attrs = None - def __init__(self, url=None, accessor=None, attrs=None, reverse_args=None, query=None, fragment=None): + def __init__( + self, url=None, accessor=None, attrs=None, reverse_args=None, query=None, fragment=None + ): """ arguments: url (callable): If supplied, the result of this callable will be used as ``href`` attribute. @@ -105,7 +107,9 @@ def compose_url(self, **kwargs): if bound_column is None and self.accessor is None: accessor = Accessor("") else: - accessor = Accessor(self.accessor if self.accessor is not None else bound_column.name) + accessor = Accessor( + self.accessor if self.accessor is not None else bound_column.name + ) context = accessor.resolve(record) if not hasattr(context, "get_absolute_url"): if hasattr(record, "get_absolute_url"): @@ -119,12 +123,14 @@ def compose_url(self, **kwargs): url = context.get_absolute_url() if self.query: - url += '?' + urlencode({ - a: v.resolve(record) if isinstance(v, Accessor) else v - for a, v in self.query.items() - }) + url += "?" + urlencode( + { + a: v.resolve(record) if isinstance(v, Accessor) else v + for a, v in self.query.items() + } + ) if self.fragment: - url += '#' + self.fragment + url += "#" + self.fragment return url @@ -236,9 +242,9 @@ class Column: ``a`` tag. The different ways to define the ``href`` attribute: - If `True`, the ``record.get_absolute_url()`` or the related model's - `get_absolute_url()` is used. - - If a callable is passed, the returned value is used, if it's not ``None``. - - If a `dict` is passed, optional items named ``query`` (dict), and ``fragment`` (str), + ``get_absolute_url()`` is used. + - If a callable is passed, the returned value is used, if it's not `None`. + - If a `dict` is passed, optional items named `query` (dict), and `fragment` (str), if present, specify the query string and fragment (hash) elements of the URL. The remaining items are passed on to ``~django.urls.reverse`` as kwargs. - If a `tuple` is passed, it must be either a (viewname, args) or (viewname, kwargs) @@ -330,8 +336,10 @@ def __init__( link_kwargs = dict(reverse_args=linkify) elif isinstance(linkify, dict): # specific keys in linkify are understood to be link_kwargs, and the rest must be reverse_args - link_kwargs = {name: linkify.pop(name) for name in ('query', 'fragment') if name in linkify} - link_kwargs['reverse_args'] = linkify + link_kwargs = { + name: linkify.pop(name) for name in ("query", "fragment") if name in linkify + } + link_kwargs["reverse_args"] = linkify elif linkify is True: link_kwargs = dict(accessor=self.accessor) diff --git a/django_tables2/columns/linkcolumn.py b/django_tables2/columns/linkcolumn.py index adf58762..c508cbd8 100644 --- a/django_tables2/columns/linkcolumn.py +++ b/django_tables2/columns/linkcolumn.py @@ -67,6 +67,8 @@ class LinkColumn(BaseLinkColumn): text (str or callable): Either static text, or a callable. If set, this will be used to render the text inside link instead of value (default). The callable gets the record being rendered as argument. + query (dict): Optional name/value pair for query string. See `~urllib.urlencode`. + fragment (str): Optional URL Fragment Identifier (hash). .. [2] In order to create a link to a URL that relies on information in the current row, `.Accessor` objects can be used in the *args* or *kwargs* @@ -130,6 +132,8 @@ def __init__( kwargs=None, current_app=None, attrs=None, + query=None, + fragment=None, **extra ): super().__init__( @@ -140,6 +144,8 @@ def __init__( args=args, kwargs=kwargs, current_app=current_app, + query=query, + fragment=fragment, ), **extra ) diff --git a/requirements/common.pip b/requirements/common.pip index 55dac099..3d93a5b1 100644 --- a/requirements/common.pip +++ b/requirements/common.pip @@ -5,5 +5,6 @@ lxml pytz>0 tablib==0.13.0 mock +Django==2.2.3 psycopg2-binary django-filter==2.2.0 diff --git a/tests/columns/test_linkcolumn.py b/tests/columns/test_linkcolumn.py index cb7d010e..706abc56 100644 --- a/tests/columns/test_linkcolumn.py +++ b/tests/columns/test_linkcolumn.py @@ -1,3 +1,6 @@ +import re +from urllib.parse import urlparse, parse_qsl + from django.template import Context, Template from django.test import TestCase from django.urls import reverse @@ -228,3 +231,25 @@ class Table(tables.Table): table = Table([{"col": "link-text", "id": 1}]) self.assertEqual(table.rows[0].get_cell_value("col"), "link-text") + + def test_query(self): + query = {"a": A("a"), "b": "baker", "c": "12"} + + class PersonTable(tables.Table): + a = tables.LinkColumn("occupation", kwargs={"pk": A("a")}, query=query) + + table = PersonTable([{"a": 0}, {"a": 1}]) + + for a in range(1): + href = re.search('href="(.*)"', table.rows[a].get_cell("a")).group(1) + query["a"] = str(a) + self.assertEqual(query, dict(parse_qsl(urlparse(href).query))) + + def test_fragment(self): + class PersonTable(tables.Table): + a = tables.LinkColumn("occupation", kwargs={"pk": A("a")}, fragment="fragval") + + table = PersonTable([{"a": 0}, {"a": 1}]) + + href = re.search('href="(.*)"', table.rows[0].get_cell("a")).group(1) + self.assertEqual("fragval", urlparse(href).fragment) diff --git a/tox.ini b/tox.ini index 00526408..ac20f3e7 100644 --- a/tox.ini +++ b/tox.ini @@ -52,7 +52,7 @@ deps = -r{toxinidir}/docs/requirements.txt [testenv:flake8] -basepython = python3.6 +basepython = python3.7 deps = flake8==3.7.8 commands = flake8 @@ -62,7 +62,7 @@ exclude = .git,__pycache__,.tox,example/app/migrations max-line-length = 120 [testenv:black] -basepython = python3.6 +basepython = python3.7 passenv = LC_CTYPE deps = black==19.3b0 commands = black --check . From dafcdbd6731884ce1afe4cc94ac15a2cff773464 Mon Sep 17 00:00:00 2001 From: Ben Mehlman Date: Mon, 11 Nov 2019 13:56:27 -0500 Subject: [PATCH 4/6] Remove unintended change to requirements/common.pip --- requirements/common.pip | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements/common.pip b/requirements/common.pip index 3d93a5b1..55dac099 100644 --- a/requirements/common.pip +++ b/requirements/common.pip @@ -5,6 +5,5 @@ lxml pytz>0 tablib==0.13.0 mock -Django==2.2.3 psycopg2-binary django-filter==2.2.0 From 9b78f2e9c6f3f1b2ad5ae0b4a7f54db8ad7df090 Mon Sep 17 00:00:00 2001 From: Ben Mehlman Date: Mon, 11 Nov 2019 14:05:19 -0500 Subject: [PATCH 5/6] Remove unintended change to tox.ini --- tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index ac20f3e7..00526408 100644 --- a/tox.ini +++ b/tox.ini @@ -52,7 +52,7 @@ deps = -r{toxinidir}/docs/requirements.txt [testenv:flake8] -basepython = python3.7 +basepython = python3.6 deps = flake8==3.7.8 commands = flake8 @@ -62,7 +62,7 @@ exclude = .git,__pycache__,.tox,example/app/migrations max-line-length = 120 [testenv:black] -basepython = python3.7 +basepython = python3.6 passenv = LC_CTYPE deps = black==19.3b0 commands = black --check . From adc0b17cd44a7b3b38479bf9cd1e064171e273c7 Mon Sep 17 00:00:00 2001 From: Ben Mehlman Date: Mon, 11 Nov 2019 14:24:05 -0500 Subject: [PATCH 6/6] Fix sorting of imports. --- tests/columns/test_linkcolumn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/columns/test_linkcolumn.py b/tests/columns/test_linkcolumn.py index 706abc56..f463d58a 100644 --- a/tests/columns/test_linkcolumn.py +++ b/tests/columns/test_linkcolumn.py @@ -1,5 +1,5 @@ import re -from urllib.parse import urlparse, parse_qsl +from urllib.parse import parse_qsl, urlparse from django.template import Context, Template from django.test import TestCase