From 42e7ae677ab27f6dcfeb4608ce52ac6238bac036 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Wed, 13 May 2026 11:07:00 -0700 Subject: [PATCH] fix(exports): align CSV roundtrip contracts (#1725) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(exports): align CSV roundtrip contracts * fix(exports): version CSV export contract * fix(exports): stabilize CSV export values * fix(imports): preserve legacy CSV roundtrip contracts * fix(imports): escape pipe characters in CSV tags --------- Signed-off-by: Juan José Mata Co-authored-by: Juan José Mata --- app/models/account_import.rb | 8 +- app/models/family/data_exporter.rb | 21 +++- app/models/import.rb | 75 ++++++++++-- app/models/import/row.rb | 54 +++++++- app/models/trade_import.rb | 8 +- app/models/transaction_import.rb | 8 +- test/models/account_import_test.rb | 32 +++++ test/models/family/data_exporter_test.rb | 149 ++++++++++++++++++++++- test/models/trade_import_test.rb | 32 +++++ test/models/transaction_import_test.rb | 84 +++++++++++++ 10 files changed, 438 insertions(+), 33 deletions(-) diff --git a/app/models/account_import.rb b/app/models/account_import.rb index 02f19c05b..53ee51f1f 100644 --- a/app/models/account_import.rb +++ b/app/models/account_import.rb @@ -59,11 +59,11 @@ class AccountImport < Import end def csv_template - template = <<-CSV + template = <<~CSV Account type*,Name*,Balance*,Currency,Balance Date - Checking,Main Checking Account,1000.00,USD,01/01/2024 - Savings,Emergency Fund,5000.00,USD,01/15/2024 - Credit Card,Rewards Card,-500.00,USD,02/01/2024 + Checking,Main Checking Account,1000.00,USD,2024-01-01 + Savings,Emergency Fund,5000.00,USD,2024-01-15 + Credit Card,Rewards Card,-500.00,USD,2024-02-01 CSV CSV.parse(template, headers: true) diff --git a/app/models/family/data_exporter.rb b/app/models/family/data_exporter.rb index da3f8ac36..0435e1ec3 100644 --- a/app/models/family/data_exporter.rb +++ b/app/models/family/data_exporter.rb @@ -2,6 +2,8 @@ require "zip" require "csv" class Family::DataExporter + EXPORT_VERSION = 2 + def initialize(family) @family = family end @@ -9,6 +11,10 @@ class Family::DataExporter def generate_export # Create a StringIO to hold the zip data in memory zip_data = Zip::OutputStream.write_buffer do |zipfile| + # Add export version marker for downstream tooling + zipfile.put_next_entry("version.txt") + zipfile.write generate_version_txt + # Add accounts.csv zipfile.put_next_entry("accounts.csv") zipfile.write generate_accounts_csv @@ -44,6 +50,11 @@ class Family::DataExporter end private + def generate_version_txt + <<~TEXT + export_version: #{EXPORT_VERSION} + TEXT + end def generate_accounts_csv CSV.generate do |csv| @@ -74,12 +85,12 @@ class Family::DataExporter .includes(:category, :tags, entry: :account) .find_each do |transaction| csv << [ - transaction.entry.date.iso8601, + transaction.entry.date&.iso8601, transaction.entry.account.name, transaction.entry.amount.to_s, transaction.entry.name, transaction.category&.name, - transaction.tags.pluck(:name).join(","), + transaction.tags.map { |tag| escape_legacy_tag_name(tag.name) }.join(","), transaction.entry.notes, transaction.entry.currency ] @@ -87,6 +98,10 @@ class Family::DataExporter end end + def escape_legacy_tag_name(name) + name.to_s.gsub(/[\\,|]/) { |char| "\\#{char}" } + end + def generate_trades_csv CSV.generate do |csv| csv << [ "date", "account_name", "ticker", "quantity", "price", "amount", "currency" ] @@ -96,7 +111,7 @@ class Family::DataExporter .includes(:security, entry: :account) .find_each do |trade| csv << [ - trade.entry.date.iso8601, + trade.entry.date&.iso8601, trade.entry.account.name, trade.security.ticker, trade.qty.to_s, diff --git a/app/models/import.rb b/app/models/import.rb index 24b0aab71..2f1256ff4 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -210,19 +210,19 @@ class Import < ApplicationRecord mapped_rows = csv_rows.map.with_index(1) do |row, index| { source_row_number: index, - account: row[account_col_label].to_s, - date: row[date_col_label].to_s, - qty: sanitize_number(row[qty_col_label]).to_s, - ticker: row[ticker_col_label].to_s, - exchange_operating_mic: row[exchange_operating_mic_col_label].to_s, - price: sanitize_number(row[price_col_label]).to_s, - amount: sanitize_number(row[amount_col_label]).to_s, - currency: (row[currency_col_label] || default_currency).to_s, - name: (row[name_col_label] || default_row_name).to_s, - category: row[category_col_label].to_s, - tags: row[tags_col_label].to_s, - entity_type: row[entity_type_col_label].to_s, - notes: row[notes_col_label].to_s + account: csv_value(row, account_col_label, "account", "account_name").to_s, + date: csv_value(row, date_col_label, "date").to_s, + qty: sanitize_number(csv_value(row, qty_col_label, "qty", "quantity")).to_s, + ticker: csv_value(row, ticker_col_label, "ticker").to_s, + exchange_operating_mic: csv_value(row, exchange_operating_mic_col_label, "exchange_operating_mic").to_s, + price: sanitize_number(csv_value(row, price_col_label, "price")).to_s, + amount: sanitize_number(csv_value(row, amount_col_label, "amount", "balance")).to_s, + currency: (csv_value(row, currency_col_label, "currency") || default_currency).to_s, + name: (csv_value(row, name_col_label, "name") || default_row_name).to_s, + category: csv_value(row, category_col_label, "category").to_s, + tags: csv_value(row, tags_col_label, "tags").to_s, + entity_type: csv_value(row, entity_type_col_label, "entity_type", "account_type", "type").to_s, + notes: csv_value(row, notes_col_label, "notes").to_s } end @@ -381,6 +381,55 @@ class Import < ApplicationRecord account&.currency || family.currency end + def csv_value(row, label, *aliases) + return if label.blank? + + [ label, *aliases ].each do |candidate| + header = header_for(candidate) + next if header.blank? + + value = row[header] + return value if value.present? + end + + nil + end + + def header_for(candidate) + return if candidate.blank? + + normalized_csv_headers[normalize_header(candidate)] + end + + def normalized_csv_headers + @normalized_csv_headers ||= begin + grouped_headers = Array(csv_headers) + .filter_map do |header| + normalized = normalize_header(header) + next if normalized.blank? + + [ normalized, header ] + end + .group_by(&:first) + + duplicate_headers = grouped_headers.values.filter_map do |headers| + originals = headers.map(&:last).uniq + originals if originals.many? + end + + if duplicate_headers.any? + errors.add(:base, "CSV headers normalize to duplicate columns: #{duplicate_headers.map { |headers| headers.join(', ') }.join('; ')}") + raise ActiveRecord::RecordInvalid, self + end + + grouped_headers.transform_values { |headers| headers.first.last } + end + end + + def normalize_header(header) + header.to_s.strip.downcase.gsub(/\*/, "").gsub(/[\s-]+/, "_") + end + def parsed_csv return @parsed_csv if defined?(@parsed_csv) diff --git a/app/models/import/row.rb b/app/models/import/row.rb index 783afa3be..56375a593 100644 --- a/app/models/import/row.rb +++ b/app/models/import/row.rb @@ -14,7 +14,7 @@ class Import::Row < ApplicationRecord if tags.blank? [ "" ] else - tags.split("|").map(&:strip) + split_tags(tags).map(&:strip) end end @@ -37,6 +37,58 @@ class Import::Row < ApplicationRecord end private + # Supports historical comma-delimited exports and pipe-delimited templates. + # Backslash escapes comma, pipe, and backslash so tag names can contain either delimiter. + def split_tags(value) + split_escaped_tags(value, tag_delimiter_for(value)) + end + + def tag_delimiter_for(value) + return "," if unescaped_delimiter?(value, ",") + return "|" if unescaped_delimiter?(value, "|") + + "," + end + + def unescaped_delimiter?(value, delimiter) + escaping = false + + value.each_char do |char| + if escaping + escaping = false + elsif char == "\\" + escaping = true + elsif char == delimiter + return true + end + end + + false + end + + def split_escaped_tags(value, delimiter) + tag_names = [] + current = +"" + escaping = false + + value.each_char do |char| + if escaping + current << (char.in?([ delimiter, ",", "|", "\\" ]) ? char : "\\#{char}") + escaping = false + elsif char == "\\" + escaping = true + elsif char == delimiter + tag_names << current + current = +"" + else + current << char + end + end + + current << "\\" if escaping + tag_names << current + end + # In the Sure system, positive quantities == "inflows" def apply_trade_signage_convention(value) value * (import.signage_convention == "inflows_positive" ? 1 : -1) diff --git a/app/models/trade_import.rb b/app/models/trade_import.rb index e8372bed4..58ade4809 100644 --- a/app/models/trade_import.rb +++ b/app/models/trade_import.rb @@ -65,11 +65,11 @@ class TradeImport < Import end def csv_template - template = <<-CSV + template = <<~CSV date*,ticker*,exchange_operating_mic,currency,qty*,price*,account,name - 05/15/2024,AAPL,XNAS,USD,10,150.00,Trading Account,Apple Inc. Purchase - 05/16/2024,GOOGL,XNAS,USD,-5,2500.00,Investment Account,Alphabet Inc. Sale - 05/17/2024,TSLA,XNAS,USD,2,700.50,Retirement Account,Tesla Inc. Purchase + 2024-05-15,AAPL,XNAS,USD,10,150.00,Trading Account,Apple Inc. Purchase + 2024-05-16,GOOGL,XNAS,USD,-5,2500.00,Investment Account,Alphabet Inc. Sale + 2024-05-17,TSLA,XNAS,USD,2,700.50,Retirement Account,Tesla Inc. Purchase CSV csv = CSV.parse(template, headers: true) diff --git a/app/models/transaction_import.rb b/app/models/transaction_import.rb index 229526046..8b93431e6 100644 --- a/app/models/transaction_import.rb +++ b/app/models/transaction_import.rb @@ -105,11 +105,11 @@ class TransactionImport < Import end def csv_template - template = <<-CSV + template = <<~CSV date*,amount*,name,currency,category,tags,account,notes - 05/15/2024,-45.99,Grocery Store,USD,Food,groceries|essentials,Checking Account,Monthly grocery run - 05/16/2024,1500.00,Salary,,Income,,Main Account, - 05/17/2024,-12.50,Coffee Shop,,,coffee,, + 2024-05-15,-45.99,Grocery Store,USD,Food,groceries|essentials,Checking Account,Monthly grocery run + 2024-05-16,1500.00,Salary,,Income,,Main Account, + 2024-05-17,-12.50,Coffee Shop,,,coffee,, CSV csv = CSV.parse(template, headers: true) diff --git a/test/models/account_import_test.rb b/test/models/account_import_test.rb index d790a3134..c41148407 100644 --- a/test/models/account_import_test.rb +++ b/test/models/account_import_test.rb @@ -7,6 +7,38 @@ class AccountImportTest < ActiveSupport::TestCase @subject = @import = imports(:account) end + test "csv_template uses ISO dates" do + first_row = @import.csv_template.first + + assert_equal "2024-01-01", first_row["Balance Date"] + end + + test "generates rows from legacy account export headers with template labels" do + import_csv = <<~CSV + id,name,type,subtype,balance,currency,created_at + account-1,Main Checking,Depository,checking,1000.00,USD,2024-01-01T00:00:00Z + CSV + + @import.update!( + raw_file_str: import_csv, + entity_type_col_label: "Account type*", + name_col_label: "Name*", + amount_col_label: "Balance*", + currency_col_label: "Currency", + date_col_label: "Balance Date", + date_format: "%Y-%m-%d" + ) + + @import.generate_rows_from_csv + row = @import.rows.reload.first + + assert row.valid? + assert_equal "Depository", row.entity_type + assert_equal "Main Checking", row.name + assert_equal "1000.00", row.amount + assert row.date.blank? + end + test "import creates accounts with valuations" do import_csv = <<~CSV type,name,amount,currency diff --git a/test/models/family/data_exporter_test.rb b/test/models/family/data_exporter_test.rb index 5a8ff650a..206c5e9d6 100644 --- a/test/models/family/data_exporter_test.rb +++ b/test/models/family/data_exporter_test.rb @@ -5,6 +5,7 @@ class Family::DataExporterTest < ActiveSupport::TestCase @family = families(:dylan_family) @other_family = families(:empty) @exporter = Family::DataExporter.new(@family) + @opening_anchor_date = Date.parse("2024-05-01") # Create some test data for the family @account = @family.accounts.create!( @@ -13,6 +14,13 @@ class Family::DataExporterTest < ActiveSupport::TestCase balance: 1000, currency: "USD" ) + @account.entries.create!( + date: @opening_anchor_date, + amount: 1000, + name: "Opening balance", + currency: "USD", + entryable: Valuation.new(kind: "opening_anchor") + ) @category = @family.categories.create!( name: "Test Category", @@ -47,7 +55,7 @@ class Family::DataExporterTest < ActiveSupport::TestCase assert zip_data.is_a?(StringIO) # Check that the zip contains all expected files - expected_files = [ "accounts.csv", "transactions.csv", "trades.csv", "categories.csv", "rules.csv", "attachments.json", "all.ndjson" ] + expected_files = [ "version.txt", "accounts.csv", "transactions.csv", "trades.csv", "categories.csv", "rules.csv", "attachments.json", "all.ndjson" ] Zip::File.open_buffer(zip_data) do |zip| actual_files = zip.entries.map(&:name) @@ -164,15 +172,23 @@ class Family::DataExporterTest < ActiveSupport::TestCase Zip::File.open_buffer(zip_data) do |zip| # Check accounts.csv accounts_csv = zip.read("accounts.csv") - assert accounts_csv.include?("id,name,type,subtype,balance,currency,created_at") + assert_equal [ "id", "name", "type", "subtype", "balance", "currency", "created_at" ], + CSV.parse(accounts_csv, headers: true).headers + + # Check version marker + version_txt = zip.read("version.txt") + assert_includes version_txt, "export_version: 2" + refute_includes version_txt, "csv_export_version" # Check transactions.csv transactions_csv = zip.read("transactions.csv") - assert transactions_csv.include?("date,account_name,amount,name,category,tags,notes,currency") + assert_equal [ "date", "account_name", "amount", "name", "category", "tags", "notes", "currency" ], + CSV.parse(transactions_csv, headers: true).headers # Check trades.csv trades_csv = zip.read("trades.csv") - assert trades_csv.include?("date,account_name,ticker,quantity,price,amount,currency") + assert_equal [ "date", "account_name", "ticker", "quantity", "price", "amount", "currency" ], + CSV.parse(trades_csv, headers: true).headers # Check categories.csv categories_csv = zip.read("categories.csv") @@ -184,6 +200,109 @@ class Family::DataExporterTest < ActiveSupport::TestCase end end + test "exports transaction CSV rows with restored legacy ISO date stored amount account name and comma tags" do + tag2 = @family.tags.create!(name: "Food, Dining|Cafe", color: "#0000FF") + entry = @account.entries.create!( + name: "CSV Grocery", + amount: 42.50, + currency: "USD", + date: Date.parse("2024-05-15"), + notes: "Weekly grocery run", + entryable: Transaction.new(category: @category) + ) + entry.transaction.tags << @tag + entry.transaction.tags << tag2 + + zip_data = @exporter.generate_export + + Zip::File.open_buffer(zip_data) do |zip| + rows = CSV.parse(zip.read("transactions.csv"), headers: true) + row = rows.find { |csv_row| csv_row["name"] == "CSV Grocery" } + + assert_not_nil row + assert_equal "2024-05-15", row["date"] + assert_equal "42.5", row["amount"] + assert_equal @account.name, row["account_name"] + assert_includes row["tags"], "," + assert_includes row["tags"], "\\," + assert_includes row["tags"], "\\|" + assert_equal [ @tag.name, tag2.name ].sort, Import::Row.new(tags: row["tags"]).tags_list.sort + end + end + + test "exported CSV files can generate matching import rows" do + create_csv_export_trade! + @account.entries.create!( + name: "CSV Importable Transaction", + amount: 15.25, + currency: "USD", + date: Date.parse("2024-05-16"), + entryable: Transaction.new(category: @category) + ) + + zip_data = @exporter.generate_export + + Zip::File.open_buffer(zip_data) do |zip| + accounts_csv = zip.read("accounts.csv") + account_import = @other_family.imports.create!( + type: "AccountImport", + raw_file_str: accounts_csv, + entity_type_col_label: "Account type*", + name_col_label: "Name*", + amount_col_label: "Balance*", + currency_col_label: "Currency", + date_col_label: "Balance Date", + date_format: "%Y-%m-%d" + ) + account_import.generate_rows_from_csv + account_row = account_import.rows.reload.find_by!(name: @account.name) + assert account_row.valid? + assert_equal @account.accountable_type, account_row.entity_type + assert_equal BigDecimal(@account.balance.to_s), BigDecimal(account_row.amount) + assert account_row.date.blank? + + transaction_import = @other_family.imports.create!( + type: "TransactionImport", + raw_file_str: zip.read("transactions.csv"), + date_col_label: "date*", + amount_col_label: "amount*", + name_col_label: "name", + currency_col_label: "currency", + category_col_label: "category", + tags_col_label: "tags", + account_col_label: "account", + notes_col_label: "notes", + date_format: "%Y-%m-%d", + signage_convention: "inflows_negative" + ) + transaction_import.generate_rows_from_csv + transaction_row = transaction_import.rows.reload.find_by!(name: "CSV Importable Transaction") + assert transaction_row.valid? + assert_equal @account.name, transaction_row.account + assert_equal BigDecimal("15.25"), transaction_row.signed_amount + + trade_import = @other_family.imports.create!( + type: "TradeImport", + raw_file_str: zip.read("trades.csv"), + date_col_label: "date*", + ticker_col_label: "ticker*", + exchange_operating_mic_col_label: "exchange_operating_mic", + currency_col_label: "currency", + qty_col_label: "qty*", + price_col_label: "price*", + account_col_label: "account", + name_col_label: "name", + date_format: "%Y-%m-%d", + signage_convention: "inflows_positive" + ) + trade_import.generate_rows_from_csv + trade_row = trade_import.rows.reload.find_by!(ticker: @csv_export_trade_ticker) + assert trade_row.valid? + assert_equal @account.name, trade_row.account + assert_equal BigDecimal("10"), BigDecimal(trade_row.qty) + end + end + test "generates valid NDJSON file" do zip_data = @exporter.generate_export @@ -750,4 +869,26 @@ class Family::DataExporterTest < ActiveSupport::TestCase entryable: Transaction.new(kind: "funds_movement") ) end + + def create_csv_export_trade! + security = Security.create!( + ticker: "CSV#{SecureRandom.hex(3).upcase}", + name: "CSV Export Security", + exchange_operating_mic: "XNAS" + ) + @csv_export_trade_ticker = security.ticker + + @account.entries.create!( + date: Date.parse("2024-05-17"), + amount: 1500, + name: "CSV Export Buy", + currency: "USD", + entryable: Trade.new( + security: security, + qty: 10, + price: 150, + currency: "USD" + ) + ) + end end diff --git a/test/models/trade_import_test.rb b/test/models/trade_import_test.rb index 3fbdd3c11..533e8b79f 100644 --- a/test/models/trade_import_test.rb +++ b/test/models/trade_import_test.rb @@ -10,6 +10,38 @@ class TradeImportTest < ActiveSupport::TestCase Security.stubs(:provider).returns(@provider) end + test "csv_template uses ISO dates" do + first_row = @import.csv_template.first + + assert_equal "2024-05-15", first_row["date*"] + end + + test "generates rows from legacy quantity header with template labels" do + import_csv = <<~CSV + date,ticker,quantity,price,account_name + 2024-05-15,AAPL,10,150.00,Trading Account + CSV + + @import.update!( + raw_file_str: import_csv, + date_col_label: "date*", + ticker_col_label: "ticker*", + qty_col_label: "qty*", + price_col_label: "price*", + account_col_label: "account", + date_format: "%Y-%m-%d", + signage_convention: "inflows_positive" + ) + + @import.generate_rows_from_csv + row = @import.rows.reload.first + + assert row.valid? + assert_equal "Trading Account", row.account + assert_equal "10", row.qty + assert_equal BigDecimal("1500"), row.signed_amount + end + test "imports trades and accounts" do aapl_resolver = mock googl_resolver = mock diff --git a/test/models/transaction_import_test.rb b/test/models/transaction_import_test.rb index f3278497e..13d638dbd 100644 --- a/test/models/transaction_import_test.rb +++ b/test/models/transaction_import_test.rb @@ -103,6 +103,90 @@ class TransactionImportTest < ActiveSupport::TestCase assert_equal [ -100, 200, -300 ], @import.entries.order(:date).map(&:amount) end + test "csv_template uses ISO dates" do + first_row = @import.csv_template.first + + assert_equal "2024-05-15", first_row["date*"] + end + + test "generates rows from legacy account_name header with template labels" do + import_csv = <<~CSV + date,account_name,amount,name + 2024-05-15,Checking Account,42.50,Legacy Export Row + CSV + + @import.update!( + raw_file_str: import_csv, + date_col_label: "date*", + amount_col_label: "amount*", + name_col_label: "name", + account_col_label: "account", + date_format: "%Y-%m-%d", + amount_type_strategy: "signed_amount", + signage_convention: "inflows_negative" + ) + + @import.generate_rows_from_csv + row = @import.rows.reload.first + + assert row.valid? + assert_equal "Checking Account", row.account + assert_equal "2024-05-15", row.date + assert_equal BigDecimal("42.50"), row.signed_amount + end + + test "generates rows from alias column when configured column value is blank" do + import_csv = <<~CSV + date*,account,account_name,amount,name + 2024-05-15,,Checking Account,42.50,Legacy Export Row + CSV + + @import.update!( + raw_file_str: import_csv, + date_col_label: "date*", + amount_col_label: "amount*", + name_col_label: "name", + account_col_label: "account", + date_format: "%Y-%m-%d", + amount_type_strategy: "signed_amount", + signage_convention: "inflows_negative" + ) + + @import.generate_rows_from_csv + row = @import.rows.reload.first + + assert row.valid? + assert_equal "Checking Account", row.account + end + + test "rejects duplicate normalized CSV headers" do + import_csv = <<~CSV + date,date*,amount,name + 2024-05-15,2024-05-16,42.50,Duplicate Date Row + CSV + + @import.update!( + raw_file_str: import_csv, + date_col_label: "date", + amount_col_label: "amount", + name_col_label: "name", + date_format: "%Y-%m-%d", + amount_type_strategy: "signed_amount", + signage_convention: "inflows_negative" + ) + + error = assert_raises(ActiveRecord::RecordInvalid) { @import.generate_rows_from_csv } + + assert_includes error.record.errors.full_messages.to_sentence, "date, date*" + end + + test "parses legacy comma tags and escaped pipe tags" do + assert_equal [ "groceries", "essentials" ], Import::Row.new(tags: "groceries,essentials").tags_list + assert_equal [ "Food, Dining", "essentials" ], Import::Row.new(tags: "Food\\, Dining,essentials").tags_list + assert_equal [ "Food|Dining" ], Import::Row.new(tags: "Food\\|Dining").tags_list + assert_equal [ "Food|Dining", "essentials" ], Import::Row.new(tags: "Food\\|Dining|essentials").tags_list + end + test "does not create duplicate when matching transaction exists with same name" do account = accounts(:depository)