-
Notifications
You must be signed in to change notification settings - Fork 1
Add support for XLSX with data entry enhancements #6
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
base: master
Are you sure you want to change the base?
Conversation
| # Create a new Excel workbook | ||
| workbook = WriteXLSX.new(temp_file_path) | ||
| def self.from_spreadsheet(spreadsheet, options = {}) | ||
| io = StringIO.new |
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.
Let's us skip using a tempfile
lib/spreadsheet_exporter/xlsx.rb
Outdated
| raise ArgumentError, "invalid error_type `#{error_type}` for validation for column '#{column_name}'" | ||
| end | ||
|
|
||
| list_values.compact! |
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.
There should be a validation here ensuring every list value is unique
6ef934c to
c3a6e2d
Compare
c3a6e2d to
bd2545a
Compare
5bbaa96 to
2832fb1
Compare
| # Used by the CSV Import/Export process to match CSV headers to model attribute names | ||
| def self.han(klass, *attributes) | ||
| options = attributes.extract_options! | ||
| def self.han(headers, humanize_headers_class:, downcase: false, **) |
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.
The return type for this use to be scalar or array but now is always an array. This change didn't break any other code that I could see
| from_spreadsheet(spreadsheet) | ||
| extend Writexlsx::Utility # gets us `xl_rowcol_to_cell` and `xl_col_to_name` | ||
|
|
||
| ROW_MAX = 65_536 - 1 |
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.
Modern Excel can actual handle 1,048,576 rows but that was causing issues with the gem, so I chose the old Excel standard of 65536
| def self.from_spreadsheet(spreadsheet, temp_file_path = 'tmp/items.xlsx') | ||
| # Create a new Excel workbook | ||
| workbook = WriteXLSX.new(temp_file_path) | ||
| def self.from_spreadsheet(spreadsheet, validations: {}, data_sources: {}, freeze_panes: false, **options) |
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.
This is technically a breaking change since temp_file_path is not accepted anymore, but it doesn't affect any of our apps
| } | ||
|
|
||
| validations = { | ||
| "favourite_food" => SpreadsheetExporter::ColumnValidation.new( |
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.
This is my main question about code style. It's nice to have these structs for safety, etc... but they're kind of ugly. Would this be better as
validations = {
"favourite_food" => { data_source: "food_types" }
}?
Here's the output of the test script: output.xlsx