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

CSV humanize name option #3145

Merged
merged 3 commits into from Jul 3, 2014

Conversation

@iurifq
Copy link
Contributor

commented May 16, 2014

CSVBuilder let's you pass in custom column names with a symbol or a string. However the name is always humanized on the generated CSV. If you want to parse the header with another tool, you may want to explicitly determine the column's name. This PR allows you to do that by passing humanize_name option to either CSVBuilderand CSVBuilder#column. It's important to notice that this is backwards compatible with the previous behavior.

closes #3096

Thanks @bonyiii and @vbalazs

iurifq added 2 commits May 16, 2014
Add humanize_name option to CSVBuilder too and pass it to its columns.
This allows a much DRYer code if you have many column namess you don't
want to be humanized. As a bonus, any other transitive option like this
one can be added easily in the future.
@iurifq

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2014

ping?

expect(column.options[:humanize_name]).to be_false
end
end
end

This comment has been minimized.

Copy link
@seanlinsley

seanlinsley Jul 3, 2014

Member

This test doesn't actually confirm that the column name is indeed my_title

@seanlinsley

This comment has been minimized.

Copy link
Member

commented Jul 3, 2014

is the only issue I see. If you can update the test I'll be happy to merge this :octocat:

@iurifq

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2014

@seanlinsley I added another test for the actual column name and not only the option. Everything passing 🎉

I tried to rebase this branch on top of the current master and solve the conflicts but I keep getting this strange error when I run tests.

active_admin/spec/rails/rails-3.2.18/spec/models/admin_user_spec.rb:3:in `<top (required)>': uninitialized constant AdminUser (NameError)
...

Do I need to worry about this?

@seanlinsley seanlinsley merged commit 0d279fe into activeadmin:master Jul 3, 2014

seanlinsley added a commit that referenced this pull request Jul 3, 2014
Merge pull request #3145
Conflicts:
	lib/active_admin/csv_builder.rb
@seanlinsley

This comment has been minimized.

Copy link
Member

commented Jul 3, 2014

I resolved the conflicts & merged this locally. I'm not sure what's causing that error, but in the future it might help to remove the test Rails app so it can be regenerated:

rm -rf spec/rails
rake test
@iurifq

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2014

Thanks, I'll keep this in mind next time 👍

@iurifq iurifq deleted the digitalnatives:csv_humanize_name_option branch Jul 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.