issue#2375 allow change of delimiter for csv download - #2403
Conversation
briri
left a comment
briri
left a comment
There was a problem hiding this comment.
looks good @raycarrick-ed.
My comments were just informational about things I think we should consider post Rails5 upgrade.
|
|
||
| // attach listener to separator select menu | ||
| // on change look for "stat" elements and chnage their query param | ||
| document.getElementById('csv-field-sep').addEventListener('click', (e) => { |
There was a problem hiding this comment.
we should look into converting this over to a <form> submission instead of a <a> link, that way we wouldn't need to do any manual JS it would just come through in the submission params.
Something to do after the Rails 5 upgrade
| end | ||
|
|
||
| CSV.generate do |csv| | ||
| CSV.generate({:col_sep => sep}) do |csv| |
There was a problem hiding this comment.
preferred style is { col_sep: sep } instead of { :col_sep => sep }.
No need to worry about it now though, when we run Rubocop on the entire codebase it will auto-change these for us.
|
looks like there are some JS linter issues in the checks. feel free to merge yourself once those are cleared up. You can run |
|
I fixed up the eslint issues, and also tweaked the styling to make the new drop-down display inline with the button text. |
…es not enforce tests on the separator param working
| StatCreatedPlan.all.order(:date).limit(1).pluck(:date).first \ | ||
| || Date.today.last_month.end_of_month |
There was a problem hiding this comment.
fixes a problem when there are no stats in the db (for example after a fresh seed)
3 participants
Fixes #2375 .
Changes proposed in this PR: