From 4527482aa216af436c06e90928a1f77cf93ede49 Mon Sep 17 00:00:00 2001 From: Alexander Schrot Date: Fri, 16 Aug 2024 20:00:16 +0200 Subject: [PATCH] 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 Signed-off-by: Alexander Schrot --------- Signed-off-by: Alexander Schrot Co-authored-by: Zach Gollwitzer --- app/controllers/imports_controller.rb | 4 +- app/models/import.rb | 7 ++-- app/models/import/csv.rb | 27 +++++++++----- app/views/imports/_form.html.erb | 3 +- config/locales/views/imports/en.yml | 5 +++ .../20240816071555_add_col_sep_to_imports.rb | 5 +++ db/schema.rb | 5 ++- test/controllers/imports_controller_test.rb | 4 +- test/models/import/csv_test.rb | 37 +++++++++++++++++++ test/models/import_test.rb | 29 +++++++++++++++ test/support/import_test_helper.rb | 10 +++++ 11 files changed, 117 insertions(+), 19 deletions(-) create mode 100644 db/migrate/20240816071555_add_col_sep_to_imports.rb diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index 90086684..4c2fdfc4 100644 --- a/app/controllers/imports_controller.rb +++ b/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 diff --git a/app/models/import.rb b/app/models/import.rb index 2083113f..72bf883b 100644 --- a/app/models/import.rb +++ b/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) diff --git a/app/models/import/csv.rb b/app/models/import/csv.rb index 873180de..b19fe641 100644 --- a/app/models/import/csv.rb +++ b/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) diff --git a/app/views/imports/_form.html.erb b/app/views/imports/_form.html.erb index f4178d6c..e7503016 100644 --- a/app/views/imports/_form.html.erb +++ b/app/views/imports/_form.html.erb @@ -1,6 +1,7 @@ <%= styled_form_with model: @import do |form| %> -
+
<%= 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 } %>
<%= 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" %> diff --git a/config/locales/views/imports/en.yml b/config/locales/views/imports/en.yml index c76e9721..d6166831 100644 --- a/config/locales/views/imports/en.yml +++ b/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} diff --git a/db/migrate/20240816071555_add_col_sep_to_imports.rb b/db/migrate/20240816071555_add_col_sep_to_imports.rb new file mode 100644 index 00000000..01d0f1d4 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 8c88d122..8bedff06 100644 --- a/db/schema.rb +++ b/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 diff --git a/test/controllers/imports_controller_test.rb b/test/controllers/imports_controller_test.rb index b1798c77..c5e87d4e 100644 --- a/test/controllers/imports_controller_test.rb +++ b/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 diff --git a/test/models/import/csv_test.rb b/test/models/import/csv_test.rb index 5bbd0086..ee7f0f0a 100644 --- a/test/models/import/csv_test.rb +++ b/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) diff --git a/test/models/import_test.rb b/test/models/import_test.rb index a26475af..255c6e38 100644 --- a/test/models/import_test.rb +++ b/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 diff --git a/test/support/import_test_helper.rb b/test/support/import_test_helper.rb index 9d1e13f8..95a1adc8 100644 --- a/test/support/import_test_helper.rb +++ b/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