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

[Bug] sequence in combination with extra_columns does not show extra_columns #450

Closed
gabn88 opened this issue Jun 11, 2017 · 19 comments · Fixed by Karaage-Cluster/karaage#309, mozilla/addons-server#5669, rlmv/doc-trips#90 or drummonds/bene#50
Labels

Comments

@gabn88
Copy link

gabn88 commented Jun 11, 2017

If I have a table

class MyTable(tables.Table):
    class Meta:
        model=MyModel
        fields=('field1',  'field2')
        sequence=('field1', '...')

a column:

class Field3Column(tables.Column):
    def __init__(*args, **kwargs):
       someinit
   def render(self,record, table, value, bound_column, **kwargs):
       return somerender()

and a view:
table = MyTable(extra_columns=('myfield3',Field3Column))

the table from the view will not contain Field3Column. However, by omitting the sequence on the Meta it does show the Field3.

@jieter
Copy link
Owner

jieter commented Jun 11, 2017

Interesting, thanks for reporting. Are you able to transform your example into a test case runnable in the test suite?

@gabn88
Copy link
Author

gabn88 commented Jun 11, 2017

I think the whole extra_columns attribute is untested in the test suite and needs testing. I'm not very good at writing tests, but I think a testcase that tests if extra_columns are actually added will catch this.

@gabn88
Copy link
Author

gabn88 commented Jun 11, 2017

Or is extra_columns not the preferred way to add extra columns in the view?

@jieter
Copy link
Owner

jieter commented Jun 11, 2017

table = MyTable(extra_columns=Field3Column)

Just to be sure, this is not correct syntax. extra_columns must be a list of (name, column)-tuples.

@jieter jieter added the bug label Jun 11, 2017
@gabn88
Copy link
Author

gabn88 commented Jun 11, 2017

@jieter I have updated the example, you are correct, I just typed in the minimal example to demonstrate the problem from my head.

@jieter
Copy link
Owner

jieter commented Jun 13, 2017

Failing test:

def test_dynamically_add_column_with_sequence():
    class MyTable(tables.Table):
        name = tables.Column()

        class Meta:
            sequence = ('...', 'name')

    assert list(MyTable(data, extra_columns=[
        ('country', tables.Column())
    ]).columns.columns.keys()) == ['country', 'name']

@jieter
Copy link
Owner

jieter commented Jun 13, 2017

the Meta sequence gets expand()ed here. Looks like the reordering could be deferred to TableBase.__init__().

@jieter jieter closed this as completed in f2f06e6 Jun 13, 2017
@jieter
Copy link
Owner

jieter commented Jun 13, 2017

@gabn88 just pushed a fix, please test branch master by installing it using the zip file github provides:

pip install -U https://github.com/bradleyayers/django-tables2/archive/master.zip

@gabn88
Copy link
Author

gabn88 commented Jun 15, 2017

Thanks for the quick fix! I will test this now.

@gabn88
Copy link
Author

gabn88 commented Jun 15, 2017

It seems the bug is not solved with the latest release.

I still get a table with only 'name'. The other fields added via 'extra_columns' are still omitted.

@jieter
Copy link
Owner

jieter commented Jun 15, 2017

That's correct, not released yet. That's why I asked to test with master.zip from github.

@gabn88
Copy link
Author

gabn88 commented Jun 15, 2017

I mean with the master, I'm sorry, it was a busy day ;)

@jieter
Copy link
Owner

jieter commented Jun 15, 2017

hmm, can you provide a minimal test case like I did above demonstrating what fails?

@gabn88
Copy link
Author

gabn88 commented Jun 15, 2017

ok, will do :)

@gabn88
Copy link
Author

gabn88 commented Jun 15, 2017

hmm, stupid question, but how do I include django_tables2 WITH tests? Because now there are no tests in my site-packages/django_tables2/?

@jieter
Copy link
Owner

jieter commented Jun 15, 2017

Download the zip and add a test in the tests directory?

@gabn88
Copy link
Author

gabn88 commented Jun 15, 2017

Ok, I cannot get the new code to work at all with my example. However the current release is working.

File mytable/tables.py" in __init__
  131.         super(MyTable, self).__init__(*args, **kwargs)
File "/django_tables2/tables.py" in __init__
  504.             order_by = self.data.ordering
File "/django_tables2/tables.py" in ordering
  173.         for bound_column in self.table.columns:
File "/django_tables2/columns/base.py" in <genexpr>
  605.         return (x for x in self.iterall() if x.visible)
File "/django_tables2/columns/base.py" in <genexpr>
  562.         return (column for name, column in self.iteritems())
File "/django_tables2/columns/base.py" in iteritems
  579.                 yield (name, self.columns[name])


Exception Type: KeyError 
Exception Value: 'myfield3'

@jieter
Copy link
Owner

jieter commented Jun 15, 2017

Quite hard to see why if you only include part of a stack trace.

Can you:

  1. pip install -U https://github.com/bradleyayers/django-tables2/archive/master.zip
  2. show the definition of MyTable or whatever the name of the table is you try to add extra columns to
  3. show the code initializing said table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment