Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RFC] Fix #42: Work with suntrust double column csv files #48

Merged
merged 11 commits into from
Jul 7, 2015
Merged

[RFC] Fix #42: Work with suntrust double column csv files #48

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a7c952f
Issue #42: Added test for SUNTRUST CSV file
BlackEdder Apr 23, 2015
c8e60e0
MoneyColumn.merge! works when other column is set to 0
BlackEdder Apr 23, 2015
95a3010
Improve from_s function
BlackEdder Apr 23, 2015
d3e9913
Use MoneyColumn.merge when looking for multiple money columns
BlackEdder Apr 23, 2015
c796bc2
Store original prefix and postfix when converting to money
BlackEdder Apr 23, 2015
821b6a7
MoneyColumn.merge should take prefix and postfix into account
BlackEdder Apr 23, 2015
a2b53df
Threat empty string as Money with amount == 0
BlackEdder Apr 23, 2015
8950f47
Undo adding original_post/prefix
BlackEdder Apr 23, 2015
f31b27f
Readd check for ( which indicates negative values for some banks
BlackEdder Apr 23, 2015
354733f
Remove obsolete tests for merge_columns
BlackEdder Apr 23, 2015
2330452
Cleanup Money::from_s method to be better explained
BlackEdder Apr 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions lib/reckon/csv_parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,9 @@ def merge_columns(a, b)
output_columns = []
columns.each_with_index do |column, index|
if index == a
new_column = []
column.each_with_index do |row, row_index|
new_column << row + " " + (columns[b][row_index] || '')
end
new_column = MoneyColumn.new( column )
.merge!( MoneyColumn.new( columns[b] ) )
.map { |m| m.amount.to_s }
output_columns << new_column
elsif index == b
# skip
Expand Down
21 changes: 15 additions & 6 deletions lib/reckon/money.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,19 @@ def pretty( negate = false )
end

def Money::from_s( value, options = {} )
return nil if value.empty?
return Money.new( 0.00, options ) if value.empty?
value = value.gsub(/\./, '').gsub(/,/, '.') if options[:comma_separates_cents]
amount = value.gsub(/[^\d\.]/, '').to_f
amount *= -1 if value =~ /[\(\-]/
Money.new( amount, options )
value = value.gsub(/,/, '')
m = value.match( /(\D*)(\d+\.\d\d)(\D*)/ ) || value.match(/^(.*?)([\d\.]+)(\D*)$/)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could pull these Regexes into CONSTANTS with descriptive names?

if m
amount = m[2].to_f
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment about what this method is doing might be helpful. I'm having trouble following it.

if (m[1].match( /^[\(-]/ ) || m[1].match( /-$/ ))
amount *= -1
end
return Money.new( amount, options )
else
return nil
end
end

def Money::likelihood( entry )
Expand Down Expand Up @@ -79,11 +87,12 @@ def merge!( other_column )
invert = true if self.positive? && other_column.positive?
self.each_with_index do |mon, i|
other = other_column[i]
if mon && !other
return nil if (!mon || !other)
if mon != 0.00 && other == 0.0
if invert
self[i]= -mon
end
elsif !mon && other
elsif mon == 0.00 && other != 0.00
self[i] = other
else
return nil
Expand Down
22 changes: 12 additions & 10 deletions spec/reckon/csv_parser_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
@chase = Reckon::CSVParser.new(:string => CHASE_CSV)
@some_other_bank = Reckon::CSVParser.new(:string => SOME_OTHER_CSV)
@two_money_columns = Reckon::CSVParser.new(:string => TWO_MONEY_COLUMNS_BANK)
@suntrust_csv = Reckon::CSVParser.new(:string => SUNTRUST_CSV)
@simple_csv = Reckon::CSVParser.new(:string => SIMPLE_CSV)
@nationwide = Reckon::CSVParser.new( :string => NATIONWIDE_CSV, :csv_separator => ',', :suffixed => true, :currency => "POUND" )
@german_date = Reckon::CSVParser.new(:string => GERMAN_DATE_EXAMPLE)
Expand Down Expand Up @@ -65,6 +66,7 @@
@chase.money_column_indices.should == [3]
@some_other_bank.money_column_indices.should == [3]
@two_money_columns.money_column_indices.should == [3, 4]
@suntrust_csv.money_column_indices.should == [3, 4]
@nationwide.money_column_indices.should == [3, 4]
@harder_date_example_csv.money_column_indices.should == [1]
@danish_kroner_nordea.money_column_indices.should == [3]
Expand Down Expand Up @@ -220,16 +222,6 @@
end
end

describe "merge_columns" do
it "should work on adjacent columns" do
@simple_csv.merge_columns(0,1).should == [["entry1 entry2", "entry4 entry5"], ["entry3", "entry6"]]
end

it "should work on non-adjacent columns" do
@simple_csv.merge_columns(0,2).should == [["entry1 entry3", "entry4 entry6"], ["entry2", "entry5"]]
end
end

# Data

SIMPLE_CSV = "entry1,entry2,entry3\nentry4,entry5,entry6"
Expand Down Expand Up @@ -285,6 +277,16 @@
3/26/2008,Check - 0000000251,251,"","+$88.55","$1,298.57"
CSV

SUNTRUST_CSV = (<<-CSV).strip
11/01/2014,0, Deposit,0,500.00,500.00
11/02/2014,101,Check,100.00,0,400.00
11/03/2014,102,Check,100.00,0,300.00
11/04/2014,103,Check,100.00,0,200.00
11/05/2014,104,Check,100.00,0,100.00
11/06/2014,105,Check,100.00,0,0.00
11/17/2014,0, Deposit,0,700.00,700.00
CSV

NATIONWIDE_CSV = (<<-CSV).strip
07 Nov 2013,Bank credit,Bank credit,,£500.00,£500.00
09 Oct 2013,ATM Withdrawal,Withdrawal,£20.00,,£480.00
Expand Down
14 changes: 13 additions & 1 deletion spec/reckon/money_column_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,23 @@
Reckon::MoneyColumn.new( ["1.00", ""] ).merge!(
Reckon::MoneyColumn.new( ["", "-2.00"] ) ).should == [
Reckon::Money.new( 1.00 ), Reckon::Money.new( -2.00 ) ]
end
Reckon::MoneyColumn.new( ["1.00", "0"] ).merge!(
Reckon::MoneyColumn.new( ["0", "-2.00"] ) ).should == [
Reckon::Money.new( 1.00 ), Reckon::Money.new( -2.00 ) ]
Reckon::MoneyColumn.new( ["AB1.00C", ""] ).merge!(
Reckon::MoneyColumn.new( ["", "AB-2.00C"] ) ).should == [
Reckon::Money.new( 1.00 ), Reckon::Money.new( -2.00 ) ]
Reckon::MoneyColumn.new( ["AB1.00C", "AB0C"] ).merge!(
Reckon::MoneyColumn.new( ["AB0C", "AB-2.00C"] ) ).should == [
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example CSV that has numbers in this format? It might be worth adding above... or is this the Suntrust one? I just don't see why that one was having issues.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no real example, but thought it was nice to allow arbitrary postfix/prefix with numbers in them. In many ways this is more for the columns that are likely to not be money, but something like account numbers. It will still parse them as money and give them the benefit of the doubt.

I've updated the description of this PR to explain why the suntrust csv didn't work.

Reckon::Money.new( 1.00 ), Reckon::Money.new( -2.00 ) ]
end

it "should return nil if columns cannot be merged" do
Reckon::MoneyColumn.new( ["1.00", ""] ).merge!(
Reckon::MoneyColumn.new( ["1.00", "-2.00"] ) ).should == nil

Reckon::MoneyColumn.new( ["From1", "Names"] ).merge!(
Reckon::MoneyColumn.new( ["Acc", "NL28 INGB 1200 3244 16,21817"] ) ).should == nil
end

it "should invert first column if both positive" do
Expand Down
18 changes: 15 additions & 3 deletions spec/reckon/money_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,27 @@
Reckon::Money::from_s( "$-1025,67", :comma_separates_cents => true ).should == -1025.67
end

it "should return nil for an empty string" do
Reckon::Money::from_s( "" ).should == nil
Reckon::Money::from_s( "" ).should_not == 0
it "should return 0 for an empty string" do
Reckon::Money::from_s( "" ).should == 0
end

it "should handle 1000 indicators correctly" do
Reckon::Money::from_s( "$2.000,00", :comma_separates_cents => true ).should == 2000.00
Reckon::Money::from_s( "-$1,025.67" ).should == -1025.67
end

it "should keep numbers together" do
Reckon::Money::from_s( "1A1" ).should == 1
end

it "should prefer numbers with precision of two" do
Reckon::Money::from_s( "1A2.00" ).should == 2
Reckon::Money::from_s( "2.00A1" ).should == 2
end

it "should return nil if no numbers are found" do
Reckon::Money::from_s( "BAC" ).should == nil
end
end

describe "pretty" do
Expand Down