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

* add col_sep to import model

* add validation for col_sep column

* add col_sep option to csv import model

* make use of col_sep option in import model

* add column separator field to new/edit action of an import

* add col_sep parameter to create/update action

* fix spacing between fields

Co-authored-by: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Signed-off-by: Alexander Schrot <alexander@axs-labs.com>

---------

Signed-off-by: Alexander Schrot <alexander@axs-labs.com>
Co-authored-by: Zach Gollwitzer <zach.gollwitzer@gmail.com>

app/controllers/imports_controller.rb

@@ -18,14 +18,14 @@ class ImportsController < ApplicationController
 
   def update
     account = Current.family.accounts.find(params[:import][:account_id])
+    @import.update! account: account, col_sep: params[:import][:col_sep]
 
-    @import.update! account: account
     redirect_to load_import_path(@import), notice: t(".import_updated")
   end
 
   def create
     account = Current.family.accounts.find(params[:import][:account_id])
-    @import = Import.create!(account: account)
+    @import = Import.create! account: account, col_sep: params[:import][:col_sep]
 
     redirect_to load_import_path(@import), notice: t(".import_created")
   end

app/models/import.rb

@@ -2,6 +2,7 @@ class Import < ApplicationRecord
   belongs_to :account
 
   validate :raw_csv_must_be_parsable
+  validates :col_sep, inclusion: { in: Csv::COL_SEP_LIST }
 
   before_save :initialize_csv, if: :should_initialize_csv?
 
@@ -88,7 +89,7 @@ class Import < ApplicationRecord
 
     def get_raw_csv
       return nil if raw_csv_str.nil?
-      Import::Csv.new(raw_csv_str)
+      Import::Csv.new(raw_csv_str, col_sep:)
     end
 
     def should_initialize_csv?
@@ -102,7 +103,7 @@ class Import < ApplicationRecord
 
     # Uses the user-provided raw CSV + mappings to generate a normalized CSV for the import
     def generate_normalized_csv(csv_str)
-      Import::Csv.create_with_field_mappings(csv_str, expected_fields, column_mappings)
+      Import::Csv.create_with_field_mappings(csv_str, expected_fields, column_mappings, col_sep)
     end
 
     def update_csv(row_idx, col_idx, value)
@@ -176,7 +177,7 @@ class Import < ApplicationRecord
 
     def raw_csv_must_be_parsable
       begin
-        CSV.parse(raw_csv_str || "")
+        CSV.parse(raw_csv_str || "", col_sep:)
       rescue CSV::MalformedCSVError
         # i18n-tasks-use t('activerecord.errors.models.import.attributes.raw_csv_str.invalid_csv_format')
         errors.add(:raw_csv_str, :invalid_csv_format)

app/models/import/csv.rb

@@ -1,12 +1,20 @@
 class Import::Csv
-  def self.parse_csv(csv_str)
-    CSV.parse((csv_str || "").strip, headers: true, converters: [ ->(str) { str&.strip } ])
+  DEFAULT_COL_SEP = ",".freeze
+  COL_SEP_LIST = [ DEFAULT_COL_SEP, ";" ].freeze
+
+  def self.parse_csv(csv_str, col_sep: DEFAULT_COL_SEP)
+    CSV.parse(
+      csv_str&.strip || "",
+      headers: true,
+      col_sep:,
+      converters: [ ->(str) { str&.strip } ]
+    )
   end
 
-  def self.create_with_field_mappings(raw_csv_str, fields, field_mappings)
-    raw_csv = self.parse_csv(raw_csv_str)
+  def self.create_with_field_mappings(raw_csv_str, fields, field_mappings, col_sep = DEFAULT_COL_SEP)
+    raw_csv = self.parse_csv(raw_csv_str, col_sep:)
 
-    generated_csv_str = CSV.generate headers: fields.map { |f| f.key }, write_headers: true do |csv|
+    generated_csv_str = CSV.generate headers: fields.map { |f| f.key }, write_headers: true, col_sep: do |csv|
       raw_csv.each do |row|
         row_values = []
 
@@ -22,18 +30,19 @@ class Import::Csv
       end
     end
 
-    new(generated_csv_str)
+    new(generated_csv_str, col_sep:)
   end
 
-  attr_reader :csv_str
+  attr_reader :csv_str, :col_sep
 
-  def initialize(csv_str, column_validators: nil)
+  def initialize(csv_str, column_validators: nil, col_sep: DEFAULT_COL_SEP)
     @csv_str = csv_str
+    @col_sep = col_sep
     @column_validators = column_validators || {}
   end
 
   def table
-    @table ||= self.class.parse_csv(csv_str)
+    @table ||= self.class.parse_csv(csv_str, col_sep:)
   end
 
   def update_cell(row_idx, col_idx, value)

app/views/imports/_form.html.erb

@@ -1,6 +1,7 @@
 <%= styled_form_with model: @import do |form| %>
-  <div class="mb-4">
+  <div class="mb-4 space-y-3">
     <%= form.collection_select :account_id, Current.family.accounts.alphabetically, :id, :name, { prompt: t(".select_account"), label: t(".account"), required: true } %>
+    <%= form.collection_select :col_sep, Import::Csv::COL_SEP_LIST, :to_s, -> { t(".col_sep_char.#{_1.ord}") }, { prompt: t(".select_col_sep"), label: t(".col_sep"), required: true } %>
   </div>
 
   <%= form.submit t(".next"), class: "px-4 py-2 block w-full rounded-lg bg-gray-900 text-white text-sm font-medium cursor-pointer hover:bg-gray-700" %>

config/locales/views/imports/en.yml

@@ -58,8 +58,13 @@ en:
       new: New Import
     form:
       account: Account
+      col_sep: CSV column separator
+      col_sep_char:
+        '44': Comma (,)
+        '59': Semicolon (;)
       next: Next
       select_account: Select account
+      select_col_sep: Select CSV column separator
     import:
       complete: Complete
       completed_on: Completed on %{datetime}

db/migrate/20240816071555_add_col_sep_to_imports.rb

@@ -0,0 +1,5 @@
+class AddColSepToImports < ActiveRecord::Migration[7.2]
+  def change
+    add_column :imports, :col_sep, :string, default: ','
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2024_08_15_190722) do
+ActiveRecord::Schema[7.2].define(version: 2024_08_16_071555) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -118,7 +118,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_08_15_190722) do
     t.boolean "is_active", default: true, null: false
     t.date "last_sync_date"
     t.uuid "institution_id"
-    t.virtual "classification", type: :string, as: "\nCASE\n    WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n    ELSE 'asset'::text\nEND", stored: true
+    t.virtual "classification", type: :string, as: "\nCASE\n    WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n    ELSE 'asset'::text\nEND", stored: true
     t.index ["accountable_type"], name: "index_accounts_on_accountable_type"
     t.index ["family_id"], name: "index_accounts_on_family_id"
     t.index ["institution_id"], name: "index_accounts_on_institution_id"
@@ -293,6 +293,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_08_15_190722) do
     t.string "normalized_csv_str"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.string "col_sep", default: ","
     t.index ["account_id"], name: "index_imports_on_account_id"
   end
 

test/controllers/imports_controller_test.rb

@@ -29,7 +29,7 @@ class ImportsControllerTest < ActionDispatch::IntegrationTest
 
   test "should create import" do
     assert_difference("Import.count") do
-      post imports_url, params: { import: { account_id: @user.family.accounts.first.id } }
+      post imports_url, params: { import: { account_id: @user.family.accounts.first.id, col_sep: "," } }
     end
 
     assert_redirected_to load_import_path(Import.ordered.first)
@@ -41,7 +41,7 @@ class ImportsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "should update import" do
-    patch import_url(@empty_import), params: { import: { account_id: @empty_import.account_id } }
+    patch import_url(@empty_import), params: { import: { account_id: @empty_import.account_id, col_sep: "," } }
     assert_redirected_to load_import_path(@empty_import)
   end
 

test/models/import/csv_test.rb

@@ -36,6 +36,14 @@ class Import::CsvTest < ActiveSupport::TestCase
     assert_not invalid_csv.valid?
   end
 
+  test "CSV with semicolon column separator" do
+    csv = Import::Csv.new(valid_csv_str_with_semicolon_separator, col_sep: ";")
+
+    assert_equal %w[ date name category tags amount ], csv.table.headers
+    assert_equal 4, csv.table.size
+    assert_equal "Paycheck", csv.table[3][1]
+  end
+
   test "csv with additional columns and empty values" do
     csv = Import::Csv.new valid_csv_with_missing_data
     assert csv.valid?
@@ -81,6 +89,35 @@ class Import::CsvTest < ActiveSupport::TestCase
     assert_equal "Amazon stuff", csv.table[1][1]
   end
 
+  test "can create CSV with expected columns, field mappings with validators and semicolon column separator" do
+    date_field = Import::Field.new \
+      key: "date",
+      label: "Date",
+      validator: method(:validate_iso_date)
+
+    name_field = Import::Field.new \
+      key: "name",
+      label: "Name"
+
+    fields = [ date_field, name_field ]
+
+    raw_csv_str = <<-ROWS
+      date;Custom Field Header;extra_field
+      invalid_date_value;Starbucks drink;Food
+      2024-01-02;Amazon stuff;Shopping
+    ROWS
+
+    mappings = {
+      "name" => "Custom Field Header"
+    }
+
+    csv = Import::Csv.create_with_field_mappings(raw_csv_str, fields, mappings, ";")
+
+    assert_equal %w[ date name ], csv.table.headers
+    assert_equal 2, csv.table.size
+    assert_equal "Amazon stuff", csv.table[1][1]
+  end
+
   private
 
     def validate_iso_date(value)

test/models/import_test.rb

@@ -10,6 +10,21 @@ class ImportTest < ActiveSupport::TestCase
     @loaded_import.update! raw_csv_str: valid_csv_str
   end
 
+  test "validates the correct col_sep" do
+    assert_equal ",", @empty_import.col_sep
+
+    assert @empty_import.valid?
+
+    @empty_import.col_sep = "invalid"
+    assert @empty_import.invalid?
+
+    @empty_import.col_sep = ","
+    assert @empty_import.valid?
+
+    @empty_import.col_sep = ";"
+    assert @empty_import.valid?
+  end
+
   test "raw csv input must conform to csv spec" do
     @empty_import.raw_csv_str = malformed_csv_str
     assert_not @empty_import.valid?
@@ -83,4 +98,18 @@ class ImportTest < ActiveSupport::TestCase
     @empty_import.reload
     assert @empty_import.failed?
   end
+
+  test "can create transactions from csv with custom column separator" do
+    loaded_import = @empty_import.dup
+
+    loaded_import.update! raw_csv_str: valid_csv_str_with_semicolon_separator, col_sep: ";"
+    transactions = loaded_import.dry_run
+
+    assert_equal 4, transactions.count
+
+    data = transactions.first.as_json(only: [ :name, :amount, :date ])
+    assert_equal data, { "amount" => "8.55", "date" => "2024-01-01", "name" => "Starbucks drink" }
+
+    assert_equal valid_csv_str, loaded_import.normalized_csv_str
+  end
 end

test/support/import_test_helper.rb

@@ -9,6 +9,16 @@ module ImportTestHelper
     ROWS
   end
 
+  def valid_csv_str_with_semicolon_separator
+    <<~ROWS
+      date;name;category;tags;amount
+      2024-01-01;Starbucks drink;Food & Drink;Tag1|Tag2;-8.55
+      2024-01-01;Etsy;Shopping;Tag1;-80.98
+      2024-01-02;Amazon stuff;Shopping;Tag2;-200
+      2024-01-03;Paycheck;Income;;1000
+    ROWS
+  end
+
   def valid_csv_with_invalid_values
     <<~ROWS
       date,name,category,tags,amount