Skip to content
This repository was archived by the owner on Jul 27, 2025. It is now read-only.

Allow custom column separator for CSV parsing in uploads controller#1470

Merged
zachgoll merged 2 commits intomaybe-finance:mainfrom
acflint:fix-csv-import
Nov 18, 2024
Merged

Allow custom column separator for CSV parsing in uploads controller#1470
zachgoll merged 2 commits intomaybe-finance:mainfrom
acflint:fix-csv-import

Conversation

@acflint
Copy link
Contributor

@acflint acflint commented Nov 17, 2024

Fix for this issue: https://github.com/maybe-finance/maybe/issues/1462

csv_valid? wasn't using the submitted col_sep argument, so semicolon separated CSVs were failing to be validated.

@zachgoll
Copy link
Contributor

@acflint disregard my comment on the other PR that was closed. This looks good!

@acflint
Copy link
Contributor Author

acflint commented Nov 18, 2024

@zachgoll I can fix the failing specs, if you'd like. Let me know.

@zachgoll
Copy link
Contributor

@acflint yep, that would be great. Looks like we just need to pass col_sep as params for each of those failing tests

@acflint
Copy link
Contributor Author

acflint commented Nov 18, 2024

@zachgoll done 👍🏻

@zachgoll zachgoll merged commit 9cc9f42 into maybe-finance:main Nov 18, 2024
@acflint
Copy link
Contributor Author

acflint commented Nov 18, 2024

I know it's small, but after almost 10 years of being a dev, this is the first OSS contribution I've had merged. So that's pretty cool :) Thanks @zachgoll!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants