-
-
Notifications
You must be signed in to change notification settings - Fork 393
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Excel format exporter #2099
Conversation
@@ -16,13 +16,13 @@ class CSV < Exporter | |||
# | |||
# Returns an ExportData instance. | |||
def export | |||
data = ::CSV.generate(headers: headers, write_headers: true, col_sep: ";") do |csv| | |||
data = ::CSV.generate(headers: headers, write_headers: true, col_sep: "\t") do |csv| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separated with tabs? O_o'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excel is freaking weird, I'm looking at an alternate solution.
a8d4cf9
to
2475484
Compare
Codecov Report
@@ Coverage Diff @@
## master #2099 +/- ##
==========================================
+ Coverage 98.55% 98.56% +<.01%
==========================================
Files 1174 1176 +2
Lines 26847 26889 +42
==========================================
+ Hits 26460 26502 +42
Misses 387 387 |
@@ -5,14 +5,15 @@ module Exporters | |||
autoload :Exporter, "decidim/exporters/exporter" | |||
autoload :JSON, "decidim/exporters/json" | |||
autoload :CSV, "decidim/exporters/csv" | |||
autoload :Excel, "decidim/exporters/excel" | |||
autoload :ExportData, "decidim/exporters/export_data" | |||
autoload :Serializer, "decidim/exporters/serializer" | |||
|
|||
# Get the exporter class constant from the format as a string. | |||
# | |||
# format - The exporter format as a string. i.e "csv" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the docs (upcase "CSV")?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right!
@@ -10,7 +10,7 @@ class Exporter | |||
# | |||
# collection - An Array with the collection to be exported. | |||
# serializer - A Serializer to be used during the export. | |||
def initialize(collection, serializer) | |||
def initialize(collection, serializer = Serializer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this Serializer?
? Is this something from our code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's a passthrough serializer, returns the object itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
馃帺 What? Why?
This adds an explicit
Excel
exporter that exports to a Spreadsheet instead of a CSV, which is causing lots of issues (mainly due to the undocumented nature of CSV itself).馃搶 Related Issues
None
馃搵 Subtasks
None
馃摲 Screenshots (optional)
None
馃懟 GIF
None