Add support for different column separator in csv import logic #1096

2024-08-16T23:14:13+08:00

code-constructor commented

2024-08-16 23:14:13 +08:00

(Migrated from github.com)

As we all know, CSV is not always "comma separated values". It is often separated by other characters such as ";" or "|". This PR opens the model layer to support multiple column separators. The implementation is now able to import files with "," and ";" separators. More may come.

Important to know: The normalized representation normalized_csv_str is still separated by comma (",").

I would also like to implement the view support. But I don't want to do anything without a little guidance on what would be a good solution. Maybe just a selector below the upload field or a hidden section with "advanced import options".

As we all know, CSV is not always "comma separated values". It is often separated by other characters such as ";" or "|". This PR opens the model layer to support multiple column separators. The implementation is now able to import files with "," and ";" separators. More may come. Important to know: The normalized representation `normalized_csv_str` is still separated by comma (","). I would also like to implement the view support. But I don't want to do anything without a little guidance on what would be a good solution. Maybe just a selector below the upload field or a hidden section with "advanced import options".

zachgoll commented

2024-08-16 23:47:45 +08:00

(Migrated from github.com)

@code-constructor I think this all looks good and I'm definitely supportive of adding it in since a lot of non-US imports will have ; as the separator.

In terms of the UI, what do you think of grabbing this information on the very first step?

Long term, we'll probably take a second pass at the design of this entire feature and will put this in a more permanent place, but for now this seems like it gets the job done.

@code-constructor I think this all looks good and I'm definitely supportive of adding it in since a lot of non-US imports will have `;` as the separator. In terms of the UI, what do you think of grabbing this information on the very first step? ![CleanShot 2024-08-16 at 11 47 17@2x](https://github.com/user-attachments/assets/d7ee0a4e-eb6d-4ea2-988a-64b3402c6f51) Long term, we'll probably take a second pass at the design of this entire feature and will put this in a more permanent place, but for now this seems like it gets the job done.

👍 1

code-constructor commented

2024-08-17 00:33:10 +08:00

(Migrated from github.com)

Did an update of the Pull-Request. I've implemented your idea 👍. Thanks for the really fast response. Was a pleasure 🙏

Did an update of the Pull-Request. I've implemented your idea :+1:. Thanks for the really fast response. Was a pleasure 🙏

zachgoll (Migrated from github.com) reviewed 2024-08-17 00:37:43 +08:00

zachgoll (Migrated from github.com) left a comment

Awesome, very nice addition! Just added a little suggestion to fix some form spacing. Otherwise looks and works great.

app/views/imports/_form.html.erb

						
				@@ -1,6 +1,7 @@

				<%= styled_form_with model: @import do |form| %>

zachgoll (Migrated from github.com) commented

2024-08-17 00:37:09 +08:00

  <div class="mb-4 space-y-3">

```suggestion <div class="mb-4 space-y-3"> ```

zachgoll (Migrated from github.com) reviewed 2024-08-17 00:39:39 +08:00

app/views/imports/_form.html.erb

						
				@@ -1,6 +1,7 @@

				<%= styled_form_with model: @import do |form| %>

zachgoll (Migrated from github.com) commented

2024-08-17 00:39:39 +08:00

![CleanShot 2024-08-16 at 12 39 22@2x](https://github.com/user-attachments/assets/de3d5885-735b-4947-b1b3-3552fe24869a)

code-constructor commented

2024-08-17 00:46:11 +08:00

(Migrated from github.com)

Good catch & Committed 👍

🎉 1

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gavin/maybe#1096

				`@@ -1,6 +1,7 @@`
				`<%= styled_form_with model: @import do \|form\| %>`