gavin/maybe
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 28 Wiki Activity

fix: improve CSV number format handling for non-USD currencies #1820

Closed
onyedikachi-david wants to merge 1 commits from fix/csv-number-format-1713 into main
pull from: fix/csv-number-format-1713
merge into: gavin:main
Conversation 1 Commits 1 Files Changed 8 +169 -4
8 changed files with 169 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
app/models/import.rb
View File

@@ -1,6 +1,12 @@
class Import < ApplicationRecord
TYPES = %w[TransactionImport TradeImport AccountImport MintImport].freeze
SIGNAGE_CONVENTIONS = %w[inflows_positive inflows_negative]
NUMBER_FORMATS = {
"1,234.56" => { separator: ".", delimiter: "," }, # US/UK/Asia
"1.234,56" => { separator: ",", delimiter: "." }, # Most of Europe
"1 234,56" => { separator: ",", delimiter: " " }, # French/Scandinavian
"1,234" => { separator: "", delimiter: "," } # Zero-decimal currencies like JPY
}.freeze
belongs_to :family
@@ -11,6 +17,8 @@ class Import < ApplicationRecord
validates :type, inclusion: { in: TYPES }
validates :col_sep, inclusion: { in: [ ",", ";" ] }
validates :signage_convention, inclusion: { in: SIGNAGE_CONVENTIONS }
validates :currency, presence: true
validates :number_format, inclusion: { in: NUMBER_FORMATS.keys }
has_many :rows, dependent: :destroy
has_many :mappings, dependent: :destroy
@@ -144,7 +152,56 @@ class Import < ApplicationRecord
end
def sanitize_number(value)
return "" if value.nil?
value.gsub(/[^\d.\-]/, "")
# Special case: nil or empty values remain empty strings
return "" if value.nil? || value.to_s.strip.empty?
format = NUMBER_FORMATS[number_format] || NUMBER_FORMATS["1,234.56"]
value = value.to_s.strip
# Remove any currency symbols or other non-numeric characters except the format's delimiter and separator
allowed_chars = [ format[:delimiter], format[:separator], "-" ].compact
sanitized = value.gsub(/[^#{Regexp.escape(allowed_chars.join)}0-9]/, "")
# Return empty string if no digits present (non-numeric input)
return "" unless sanitized.match?(/[0-9]/)
# Convert to standard format (period as decimal separator, no thousands delimiter)
if format[:separator].present?
# Replace delimiter with nothing first, then replace separator with period
sanitized = sanitized.gsub(format[:delimiter], "").gsub(format[:separator], ".")
else
# For zero-decimal currencies, just remove the delimiter
sanitized = sanitized.gsub(format[:delimiter], "")
end
# Extract sign for later
is_negative = sanitized.count("-") > 0
sanitized = sanitized.delete("-")
# Handle decimal points
case sanitized.count(".")
when 0
# No decimal point, nothing to do
when 1
# Single decimal point - handle edge cases
if sanitized == "."
return "" # Just a decimal point, treat as non-numeric
end
# Add leading/trailing zeros if needed
sanitized = "0" + sanitized if sanitized.start_with?(".")
sanitized += "0" if sanitized.end_with?(".")
else
# Multiple decimal points - take everything before first decimal and first decimal place only
parts = sanitized.split(".")
sanitized = parts[0] + "." + (parts[1] || "")
end
# Reapply negative sign if present
sanitized = "-" + sanitized if is_negative
# Final validation - should look like a valid number now
return "" unless sanitized.match?(/\A-?\d+\.?\d*\z/)
sanitized
end
end

5
app/views/import/configurations/_account_import.html.erb
View File

@@ -5,5 +5,10 @@
<%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" } %>
<%= form.select :amount_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Balance" } %>
<div class="flex items-center gap-2">
<%= form.select :currency, Money::Currency.all_instances.map { |c| [c.name, c.iso_code] }, { label: "Currency" }, required: true %>
<%= form.select :number_format, Import::NUMBER_FORMATS.keys.map { |f| [f, f] }, { label: "Number Format" }, required: true %>
</div>
<%= form.submit "Apply configuration", class: "w-full btn btn--primary", disabled: import.complete? %>
<% end %>

5
app/views/import/configurations/_mint_import.html.erb
View File

@@ -16,6 +16,11 @@
<%= form.select :signage_convention, [["Incomes are negative", "inflows_negative"], ["Incomes are positive", "inflows_positive"]], { label: true }, disabled: import.complete? %>
</div>
<div class="flex items-center gap-2">
<%= form.select :currency, Money::Currency.all_instances.map { |c| [c.name, c.iso_code] }, { label: "Currency" }, required: true, disabled: import.complete? %>
<%= form.select :number_format, Import::NUMBER_FORMATS.keys.map { |f| [f, f] }, { label: "Number Format" }, required: true, disabled: import.complete? %>
</div>
<%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" }, disabled: import.complete? %>
<%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" }, disabled: import.complete? %>
<%= form.select :category_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Category (optional)" }, disabled: import.complete? %>

5
app/views/import/configurations/_trade_import.html.erb
View File

@@ -11,6 +11,11 @@
<%= form.select :signage_convention, [["Buys are positive qty", "inflows_positive"], ["Buys are negative qty", "inflows_negative"]], label: true %>
</div>
<div class="flex items-center gap-2">
<%= form.select :currency, Money::Currency.all_instances.map { |c| [c.name, c.iso_code] }, { label: "Currency" }, required: true %>
<%= form.select :number_format, Import::NUMBER_FORMATS.keys.map { |f| [f, f] }, { label: "Number Format" }, required: true %>
</div>
<%= form.select :ticker_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Ticker" } %>
<%= form.select :price_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Price" } %>
<%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" } %>

5
app/views/import/configurations/_transaction_import.html.erb
View File

@@ -11,6 +11,11 @@
<%= form.select :signage_convention, [["Incomes are positive", "inflows_positive"], ["Incomes are negative", "inflows_negative"]], label: true %>
</div>
<div class="flex items-center gap-2">
<%= form.select :currency, Money::Currency.all_instances.map { |c| [c.name, c.iso_code] }, { label: "Currency" }, required: true %>
<%= form.select :number_format, Import::NUMBER_FORMATS.keys.map { |f| [f, f] }, { label: "Number Format" }, required: true %>
</div>
<%= form.select :account_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Account (optional)" } %>
<%= form.select :name_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Name (optional)" } %>
<%= form.select :category_col_label, import.csv_headers, { include_blank: "Leave empty", label: "Category (optional)" } %>

6
db/migrate/20250207004108_add_currency_and_number_format_to_imports.rb Normal file
View File

@@ -0,0 +1,6 @@
class AddCurrencyAndNumberFormatToImports < ActiveRecord::Migration[7.2]
def change
add_column :imports, :currency, :string, default: "USD"
add_column :imports, :number_format, :string, default: "1,234.56"
end
end

6
db/schema.rb generated
View File

@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.2].define(version: 2025_02_06_151825) do
ActiveRecord::Schema[7.2].define(version: 2025_02_07_004108) do
# These are extensions that must be enabled in order to support this database
enable_extension "pgcrypto"
enable_extension "plpgsql"
@@ -102,7 +102,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_06_151825) do
t.decimal "balance", precision: 19, scale: 4
t.string "currency"
t.boolean "is_active", default: true, null: false
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.uuid "import_id"
t.uuid "plaid_account_id"
t.boolean "scheduled_for_deletion", default: false
@@ -415,6 +415,8 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_06_151825) do
t.string "date_format", default: "%m/%d/%Y"
t.string "signage_convention", default: "inflows_positive"
t.string "error"
t.string "currency", default: "USD"
t.string "number_format", default: "1,234.56"
t.index ["family_id"], name: "index_imports_on_family_id"
end

80
test/models/import/number_sanitization_test.rb Normal file
View File

@@ -0,0 +1,80 @@
require "test_helper"
class Import::NumberSanitizationTest < ActiveSupport::TestCase
setup do
@import = imports(:transaction)
end
test "sanitizes US/UK/Asia format (1,234.56)" do
@import.number_format = "1,234.56"
assert_equal "1234.56", @import.send(:sanitize_number, "1,234.56")
assert_equal "1234.56", @import.send(:sanitize_number, "$1,234.56")
assert_equal "1234.56", @import.send(:sanitize_number, "£1,234.56")
assert_equal "1234.56", @import.send(:sanitize_number, "¥1,234.56")
assert_equal "-1234.56", @import.send(:sanitize_number, "-1,234.56")
assert_equal "1234.56", @import.send(:sanitize_number, "1234.56") # No delimiters
assert_equal "1234567.89", @import.send(:sanitize_number, "1,234,567.89")
end
test "sanitizes European format (1.234,56)" do
@import.number_format = "1.234,56"
assert_equal "1234.56", @import.send(:sanitize_number, "1.234,56")
assert_equal "1234.56", @import.send(:sanitize_number, "€1.234,56")
assert_equal "-1234.56", @import.send(:sanitize_number, "-1.234,56")
assert_equal "1234567.89", @import.send(:sanitize_number, "1.234.567,89")
end
test "sanitizes French/Scandinavian format (1 234,56)" do
@import.number_format = "1 234,56"
assert_equal "1234.56", @import.send(:sanitize_number, "1 234,56")
assert_equal "1234.56", @import.send(:sanitize_number, "€1 234,56")
assert_equal "-1234.56", @import.send(:sanitize_number, "-1 234,56")
assert_equal "1234567.89", @import.send(:sanitize_number, "1 234 567,89")
end
test "sanitizes zero-decimal currencies like JPY (1,234)" do
@import.number_format = "1,234"
assert_equal "1234", @import.send(:sanitize_number, "1,234")
assert_equal "1234", @import.send(:sanitize_number, "¥1,234")
assert_equal "-1234", @import.send(:sanitize_number, "-1,234")
assert_equal "1234567", @import.send(:sanitize_number, "1,234,567")
end
test "handles edge cases" do
@import.number_format = "1,234.56"
# Nil and empty values
assert_equal "", @import.send(:sanitize_number, nil)
assert_equal "", @import.send(:sanitize_number, "")
assert_equal "", @import.send(:sanitize_number, " ")
# Non-numeric input
assert_equal "", @import.send(:sanitize_number, "abc")
assert_equal "", @import.send(:sanitize_number, "$")
assert_equal "", @import.send(:sanitize_number, ".")
assert_equal "", @import.send(:sanitize_number, "-")
assert_equal "", @import.send(:sanitize_number, "-.")
# Mixed input with numbers
assert_equal "42", @import.send(:sanitize_number, "abc42def")
assert_equal "42.42", @import.send(:sanitize_number, "abc42.42def")
# Decimal point handling
assert_equal "0.5", @import.send(:sanitize_number, ".5")
assert_equal "5.0", @import.send(:sanitize_number, "5.")
assert_equal "-0.5", @import.send(:sanitize_number, "-.5")
assert_equal "42.1", @import.send(:sanitize_number, "42.1.2.3")
end
test "defaults to US format if number_format is invalid" do
@import.number_format = "invalid_format"
assert_equal "1234.56", @import.send(:sanitize_number, "1,234.56")
assert_equal "-1234.56", @import.send(:sanitize_number, "-1,234.56")
assert_equal "", @import.send(:sanitize_number, "abc")
end
end