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

Template columns not rendered on pinned rows (maybe an issue?) #482

Closed
khirstinova opened this issue Oct 4, 2017 · 5 comments · Fixed by Karaage-Cluster/karaage#344, mozilla/addons-server#6632, rlmv/doc-trips#95 or drummonds/bene#50

Comments

@khirstinova
Copy link
Contributor

khirstinova commented Oct 4, 2017

I don't know if this is considered an issue or not because the code for rendering columns in pinned rows explicitly renders only the value. in rows.py, for BoundPinnedRows

    def _get_and_render_with(self, name, render_func, default):

            accessor = A(name)
            value = accessor.resolve(context=self._record, quiet=True) or default
            return value

This however, causes only the value of the column to show up, regardless of what type of column it is. I coded a "fix" for it for template columns, but this would work only for template columns.

    def _get_and_render_with(self, name, render_func, default):
             bound_column = self.table.columns[name]

            if isinstance(bound_column.column, TemplateColumn):
                return super(BoundPinnedRow, self)._get_and_render_with(name, render_func, default)
            else:
                accessor = A(name)
                value = accessor.resolve(context=self._record, quiet=True) or default
                return value

So, again, I don't know the use case of pinned rows only rendering values. It doesn't happen to serve my purpose in this case, so I can fork the repo and use this code for my purposes only, or we can communicate about the broader philosophy here, because I don't think the above code would be great for everybody, necessarily. Let me know what you think.

@jieter
Copy link
Owner

jieter commented Oct 5, 2017

Thanks for opening an issue, @khirstinova.

I see why this might confuse someone. I'm not sure which was first: pinned rows or the value/render distinction, anyway, it might not have been implemented completely in the pinnen rows feature.

Maybe @djk2 has an opinion on this?

I'd say this is just something to be fixed, if you can, please make a patch with proper test coverage.

@djk2
Copy link
Contributor

djk2 commented Oct 5, 2017

I will try looks on problem maybe in weekend.
I not promise anything

@khirstinova
Copy link
Contributor Author

khirstinova commented Oct 5, 2017

I submitted a pull request with an added unit test on TemplateColumn.

The python 3.4 job seems to fail, the others succeed, but I can't trace anything in the output to what I did, although I fixed an import sorting issue for 3.6. Honestly, tox is flakey as hell in my environment too. Again, let me know what you think.

@khirstinova
Copy link
Contributor Author

Why would django throw an invalid syntax error in 3.4 only?

@jieter
Copy link
Owner

jieter commented Oct 7, 2017

Fixed by #483 and 9fa47f7

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