Skip to content

Conversation

@alexdunae
Copy link
Contributor

@alexdunae alexdunae commented Aug 9, 2022

Here's the output of the test script: output.xlsx

# Create a new Excel workbook
workbook = WriteXLSX.new(temp_file_path)
def self.from_spreadsheet(spreadsheet, options = {})
io = StringIO.new
Copy link
Contributor Author

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

raise ArgumentError, "invalid error_type `#{error_type}` for validation for column '#{column_name}'"
end

list_values.compact!
Copy link
Contributor Author

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

# 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, **)
Copy link
Contributor Author

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
Copy link
Contributor Author

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)
Copy link
Contributor Author

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

@alexdunae alexdunae changed the title Add support for data validation [WIP] Add support for XLSX with data entry enhancements Sep 28, 2022
}

validations = {
"favourite_food" => SpreadsheetExporter::ColumnValidation.new(
Copy link
Contributor Author

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" }
}

?

@alexdunae alexdunae marked this pull request as ready for review September 28, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant