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

Fixes #20461 - csv_exporter results in empty rows #4707

Closed
wants to merge 1 commit into from

Conversation

sjha4
Copy link
Contributor

@sjha4 sjha4 commented Jul 31, 2017

While trying to extend csv export functionality to katello pages, we observed that the csv_exporter was rendering empty rows in the exported csv.
The root cause is the line "columns.map!{|c| c.to_s.split('.').map(&:to_sym)}" getting executed more than once in the enumerator block causing the column names to be prepended with multiple ' : '

@mention-bot
Copy link

@sjha4, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tbrisker to be a potential reviewer.

@theforeman-bot
Copy link
Member

Issues: #20461

@tbrisker
Copy link
Member

tbrisker commented Aug 1, 2017

While the change itself seems harmless to me, I'm curious why on certain pages the enumerator is created multiple times? This may be an indicator of some deeper issue, since on most pages this works without a problem.

@sjha4
Copy link
Contributor Author

sjha4 commented Aug 4, 2017

@tbrisker : To put this into context, I tried this for the PR: Katello/katello#6889

@xprazak2
Copy link
Contributor

@sjha4, I think this has been fixed already as map! was replaced with
map. Could you re-test if the issue still present?

@sjha4
Copy link
Contributor Author

sjha4 commented Jun 1, 2018

Hey @xprazak2 : Tested this on a recent PR in katello around csv export. I do not see this issue anymore.
I will go ahead and close this PR.

@sjha4 sjha4 closed this Jun 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants