Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional context_object_name to TemplateColumn and make extra_context optionally callable #931

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions django_tables2/columns/templatecolumn.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from django.template.loader import get_template
from django.utils.html import strip_tags

from ..utils import call_with_appropriate
from .base import Column, library


Expand All @@ -14,7 +15,9 @@ class TemplateColumn(Column):
Arguments:
template_code (str): template code to render
template_name (str): name of the template to render
extra_context (dict): optional extra template context
context_object_name (str): name of the context variable to pas the record in, defaults to "record".
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
context_object_name (str): name of the context variable to pas the record in, defaults to "record".
context_object_name (str): name of the context variable to pass the record in, defaults to "record".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to pass the record in" klinkt gek. Wellicht "name of the context variable that represents the record, defaults to "record". ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
context_object_name (str): name of the context variable to pas the record in, defaults to "record".
context_object_name (str): name of the context variable that represents the record, defaults to "record".

extra_context (dict): optional extra template context. If a callable is passed, it will be called with
optional record, table, value, bound_column arguments.
Comment on lines +19 to +20
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
extra_context (dict): optional extra template context. If a callable is passed, it will be called with
optional record, table, value, bound_column arguments.
extra_context (dict): optional extra template context. Any callables passed will be called with the following
optional arguments: record, table, value, and bound_column.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kan je meer dan 1 callable meegeven in de dict?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gegeven mijn verwarring over extra_context hieronder kunnen we niet zeggen:
extra_context: accepts dict or callable. If a called is passed it will be called with the following optional arguments: record, ...


A `~django.template.Template` object is created from the
*template_code* or *template_name* and rendered with a context containing:
Expand All @@ -40,11 +43,19 @@ class ExampleTable(tables.Table):

empty_values = ()

def __init__(self, template_code=None, template_name=None, extra_context=None, **extra):
def __init__(
self,
template_code=None,
template_name=None,
context_object_name="record",
extra_context=None,
**extra
):
super().__init__(**extra)
self.template_code = template_code
self.template_name = template_name
self.extra_context = extra_context or {}
self.context_object_name = context_object_name

if not self.template_code and not self.template_name:
raise ValueError("A template must be provided")
Expand All @@ -56,11 +67,18 @@ def render(self, record, table, value, bound_column, **kwargs):
additional_context = {
"default": bound_column.default,
"column": bound_column,
"record": record,
self.context_object_name: record,
"value": value,
"row_counter": kwargs["bound_row"].row_counter,
}
additional_context.update(self.extra_context)

extra_context = self.extra_context
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ik vind "additional" en "extra" content wel dubbelop? Waar is additional voor en waar is extra voor? Kunnen we de namen niet wat aanpassen in termen van waar ze voor dienen?

if callable(extra_context):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ik had uit de docstring begrepen dat extra_context een dict was - dus dan zou ik de callable meegeven in een dict ... maar blijkbaar moet je een callable toewijzen aan extra_context? Dus voor mij is de docstring van verwarrend

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should return a callable returning a dict.

extra_context = call_with_appropriate(
extra_context,
{"record": record, "table": table, "value": value, "bound_column": bound_column},
)
additional_context.update(extra_context)
with context.update(additional_context):
if self.template_code:
return Template(self.template_code).render(context)
Expand Down
16 changes: 16 additions & 0 deletions tests/columns/test_templatecolumn.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,19 @@ class Table(tables.Table):
table = Table([{"track": "Space Oddity"}])

self.assertEqual(list(table.as_values()), [["Track"], ["Space Oddity"]])

def test_context_object_name(self):
class Table(tables.Table):
name = tables.TemplateColumn("{{ user.name }}", context_object_name="user")

table = Table([{"name": "Bob"}])
self.assertEqual(list(table.as_values()), [["Name"], ["Bob"]])

def test_extra_context_callable(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test voor extra context dict was er al toch?

class Table(tables.Table):
size = tables.TemplateColumn(
"{{ size }}", extra_context=lambda record: {"size": record["clothes"]["size"]}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ik zou het wel beter vinden als er meer optional arguments getest zouden worden? table / value / bound column - ik zie niet goed hoe dat hier terug komt.

)

table = Table([{"clothes": {"size": "XL"}}])
self.assertEqual(list(table.as_values()), [["Size"], ["XL"]])