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

Table with a column name of "items" is rendered empty #316

Closed
mahmoodkhan opened this issue Apr 21, 2016 · 11 comments · Fixed by #317
Closed

Table with a column name of "items" is rendered empty #316

mahmoodkhan opened this issue Apr 21, 2016 · 11 comments · Fixed by #317

Comments

@mahmoodkhan
Copy link

No description provided.

@jieter
Copy link
Owner

jieter commented Apr 22, 2016

Thanks for the report, would be nice if you included an example.

However, if I try this:

def test_column_named_items():
    class ItemsTable(tables.Table):
        items = tables.Column()

    table = ItemsTable([{'items': 123}, {'items': 2345}])

    print table.as_html(request)

I do not get an empty table, but an error: django/template/defaulttags.py:167: TypeError : 'int' object is not iterable.

Clearly something to be addressed.

@mahmoodkhan
Copy link
Author

mahmoodkhan commented Apr 22, 2016

Sorry, but I wasn't sure if I should post my actual code because it wasn't the most straight forward example:

def define_table(columns):
    #from django.template.base import add_to_builtins
    #add_to_builtins('silo.templatetags.underscoretags')

    """
    Dynamically builds a django-tables2 table without specifying the column names
    It is important to build the django-tables2 dynamically because each time a silo
    is loaded from MongoDB, it is not known what columns heading it has or how mnay columns it has
    """
    EDIT_DEL_TEMPLATE = '''
        <a class="btn btn-default btn-xs" role="button" href="/value_edit/{{ record|get:'_id'|get:'$oid' }}">Edit</a>
        <a class="btn btn-danger btn-xs btn-del" style="color: #FFF;" role="button" href="/value_delete/{{ record|get:'_id'|get:'$oid'  }}" title="Are you sure you want to delete this record?">Delete</a>
        '''
    #add the operations to the table first then append the dynamic table columns
    attrs = {}
    attrs['Operation'] = tables.TemplateColumn(EDIT_DEL_TEMPLATE)
    attrs.update(dict((c, tables.Column()) for c in columns))
    attrs['Meta'] = type('Meta', (), dict(exclude=["_id", "edit_date", "create_date"], attrs={"class":"paleblue", "orderable":"True", "width":"100%"}) )

    klass = type('DynamicTable', (tables.Table,), attrs)
    return klass

silo_table = define_table(column_names)(json_data)

#This is needed in order for table sorting to work
RequestConfig(request).configure(silo_table)

return render(request, "display/silo_detail.html", {"silo_table": silo_table, 'silo': silo, 'id':id, 'cols': cols})

then in my template:

<div id="table_data">
    {% render_table silo_table %}
</div>

@jieter
Copy link
Owner

jieter commented Apr 23, 2016

Thanks, having an example is always better than nothing, but it certainly helps if you try to narrow it down to the actual problem.

It seems that the root of the problem is in the table.html template combined with [class BoundRow in rows.py](https://github.com/bradleyayers/django-tables2/blob/master/django_tables2/rows.py#L158-L166). Since the method items()` is used to fetch the items from the row, it makes sense that it does not work if we try to access a column named items...

To be more precise: because of the nature of Django templates, with {% for column, cell in row.items %} a dictionary lookup is made first (row['items']), and if that fails, we the template system tries calling it (row.items()). Since we then have the value of row['items'] as the list of column values, things fail.

This behaviour is described in django template documentation (in the 'behind the scenes' block).

Now, to fix this, I think removing the magic __getitem__ functionality in the BoundRow class is the way to go. It's not really a public API anyway, but used extensively in the tests, so some work on that is needed...

@mahmoodkhan
Copy link
Author

Thanks for making the fix so quickly. Do you know when you're going to release this fix?

@jieter
Copy link
Owner

jieter commented Apr 26, 2016

I'm trying to fix #257 before releasing too, if that's not going fast enough I'll release before the end of the week anyway.

@jieter
Copy link
Owner

jieter commented Apr 26, 2016

Just released v1.1.7.

@mahmoodkhan
Copy link
Author

Thanks! much appreciated your prompt response and action.

@qpkorr
Copy link

qpkorr commented Jun 30, 2016

Just checking here - it seems to me that "items" is effectively a form of reserved keyword in the django template - just like the iteritems keyword example in the documentation jieter linked to above ("django template documentation"). The documentation claimed to have a workaround for this - though I confess I didn't understand it.

The drawback of this change is that some of us used to rely upon the getitem magic to do things like accessing a row dictionary eg "row.my_item" would access row['my_item'] explicitly. Since django templates don't support passing parameters, I can't, in a template, change this to row.get_cell('my_item') - and I'm yet to find another way to get the value of my_item.

The only method I've thought of is creating a loop to loop through all keys/values in row and only printing the value if the key is my_item.

Any chance you could tell me the new, intended access method, in a template, given this change?

Thank you!

@jieter
Copy link
Owner

jieter commented Jul 1, 2016

@qpkorr You are right, removing the getitem magic did reduce the functionality in the template a bit, but you are the first to report using it.

Why do you need a specific column in the template? I'm open to add a way to access the column from the template, but since we often cannot control the keys of the data coming in, no name collisions should be possible.

@jieter jieter reopened this Jul 1, 2016
@qpkorr
Copy link

qpkorr commented Jul 1, 2016

My specific use case was to apply a class to a whole row that depended upon the value in the column. I then used that class to control CSS to replace a word ("ok", "warning", or "error") with a traffic light picture. I was following the first answer here:
http://stackoverflow.com/questions/9525670/django-tables2-specify-different-properties-for-different-rows

  • so presumably I'm not the only one to do it.

In my case, I found a workaround to get the value from boundRowObject.record. - and whilst the specific column name (tr_class, as per the example) wasn't available, the data underpinning it was. So I'm fine either way. Perhaps the question becomes "How many people have columns named "items", "iteritems" etc, vs how many might (using an older version of django_tables2) be accessing column values explicitly? I might have guess there'd be more people in the latter camp than the former... but that's 100% a guess!

Either way, thanks for your response!

@jieter
Copy link
Owner

jieter commented Jul 2, 2016

I'm sorry for the problems caused, but in general, users should not have to edit the template to apply these kinds of customizations.

In your case, you can use row attributes to apply classes to rows:

class Table(tables.Table):
    class Meta:
        model = User
        row_attrs = {
            'class': lambda record: 'ok' if record.foo is None else 'warning'
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants