From aa8924243e82327bd79a83034b0b92a56a1f676e Mon Sep 17 00:00:00 2001 From: Aaron Lindsay Date: Sun, 26 Nov 2017 14:42:05 -0500 Subject: [PATCH 1/6] Add initial OFX import test So far, this only checks to ensure that the import doesn't return an error code. --- internal/handlers/common_test.go | 44 ++++++ internal/handlers/gnucash_test.go | 46 +----- .../handlers_testdata/checking_20171126.ofx | 140 ++++++++++++++++++ .../handlers_testdata/checking_20171129.ofx | 123 +++++++++++++++ internal/handlers/ofx_test.go | 36 +++++ internal/handlers/testdata_test.go | 2 +- 6 files changed, 345 insertions(+), 46 deletions(-) create mode 100644 internal/handlers/handlers_testdata/checking_20171126.ofx create mode 100644 internal/handlers/handlers_testdata/checking_20171129.ofx create mode 100644 internal/handlers/ofx_test.go diff --git a/internal/handlers/common_test.go b/internal/handlers/common_test.go index 071ec05..a0141d0 100644 --- a/internal/handlers/common_test.go +++ b/internal/handlers/common_test.go @@ -10,6 +10,7 @@ import ( "io" "io/ioutil" "log" + "mime/multipart" "net/http" "net/http/httptest" "os" @@ -158,6 +159,49 @@ func remove(client *http.Client, urlsuffix string) error { return nil } +func uploadFile(client *http.Client, filename, urlsuffix string) error { + var buf bytes.Buffer + mw := multipart.NewWriter(&buf) + + file, err := os.Open(filename) + if err != nil { + return err + } + defer file.Close() + + filewriter, err := mw.CreateFormFile("file", filename) + if err != nil { + return err + } + if _, err := io.Copy(filewriter, file); err != nil { + return err + } + + mw.Close() + + response, err := client.Post(server.URL+urlsuffix, mw.FormDataContentType(), &buf) + if err != nil { + return err + } + + body, err := ioutil.ReadAll(response.Body) + response.Body.Close() + if err != nil { + return err + } + + var e handlers.Error + err = (&e).Read(string(body)) + if err != nil { + return err + } + if e.ErrorId != 0 || len(e.ErrorString) != 0 { + return &e + } + + return nil +} + func RunWith(t *testing.T, d *TestData, fn TestDataFunc) { testdata, err := d.Initialize() if err != nil { diff --git a/internal/handlers/gnucash_test.go b/internal/handlers/gnucash_test.go index 5efd41e..bf66e57 100644 --- a/internal/handlers/gnucash_test.go +++ b/internal/handlers/gnucash_test.go @@ -1,57 +1,13 @@ package handlers_test import ( - "bytes" "github.com/aclindsa/moneygo/internal/handlers" - "io" - "io/ioutil" - "mime/multipart" "net/http" - "os" "testing" ) func importGnucash(client *http.Client, filename string) error { - var buf bytes.Buffer - mw := multipart.NewWriter(&buf) - - file, err := os.Open(filename) - if err != nil { - return err - } - defer file.Close() - - filewriter, err := mw.CreateFormFile("gnucash", filename) - if err != nil { - return err - } - if _, err := io.Copy(filewriter, file); err != nil { - return err - } - - mw.Close() - - response, err := client.Post(server.URL+"/v1/imports/gnucash", mw.FormDataContentType(), &buf) - if err != nil { - return err - } - - body, err := ioutil.ReadAll(response.Body) - response.Body.Close() - if err != nil { - return err - } - - var e handlers.Error - err = (&e).Read(string(body)) - if err != nil { - return err - } - if e.ErrorId != 0 || len(e.ErrorString) != 0 { - return &e - } - - return nil + return uploadFile(client, filename, "/v1/imports/gnucash") } func gnucashAccountBalanceHelper(t *testing.T, client *http.Client, account *handlers.Account, balance string) { diff --git a/internal/handlers/handlers_testdata/checking_20171126.ofx b/internal/handlers/handlers_testdata/checking_20171126.ofx new file mode 100644 index 0000000..1cb07bd --- /dev/null +++ b/internal/handlers/handlers_testdata/checking_20171126.ofx @@ -0,0 +1,140 @@ + + + + + + 0 + INFO + + 20171126184401.091[0:GMT] + ENG + + YCKVJ + 0351 + + + + + 0549c828-f02c-43c7-81a3-de0b3f23c393 + + 0 + INFO + + + USD + + 115483849 + 14839128817 + CHECKING + + + 20170828174401.637[0:GMT] + 20171126184401.637[0:GMT] + + CREDIT + 20170830120000.000[0:GMT] + 2843.08 + ce2cf749-dd15-4dc7-b78d-2f9e88d8a702 + SALARY + ACH Deposit 18181818199 + + + DEBIT + 20170830120000.000[0:GMT] + -2354.66 + 2bda11f4-9a9c-43fb-b67a-71f747dcf684 + BILLPAY TO CREDIT CARD + ACH Debit 8181819191919 + + + CHECK + 20170830120000.000[0:GMT] + -15.00 + 62dcc92d-ba0f-4fe6-8611-d6c1b86594fc + 3304 + Check + INCLEARING CHECK + + + DEBIT + 20171107120000.000[0:GMT] + -282.68 + 29c74a94-f226-4980-b54c-da6fa2721d7e + DAYCARE o SIGONFILE + ACH Debit 11818191919191 + + + CREDIT + 20171109120000.000[0:GMT] + 1300.98 + 32e40e98-61c3-421c-acaa-55ae67a5f8fe + DIRECT DEPOSIT + ACH Deposit 8282828282828 + + + DEBIT + 20171109120000.000[0:GMT] + -98.20 + 4b73dbbf-aa27-4f62-b54a-ee0a9a3486d8 + DUKEENGYPROGRESS DUKEENGYPR + ACH Debit 017313004099621 + + + CREDIT + 20171115120000.000[0:GMT] + 1.01 + 51c47252-4cf0-442c-b619-8a31b17ac489 + Dividend Earned + + + DEBIT + 20171116120000.000[0:GMT] + -51.75 + 51cb12bb-cdd9-4333-8d8d-c423f9e8f833 + TARGET DEBIT CRD ACH TRAN + ACH Debit + + + DEBIT + 20171120120000.000[0:GMT] + -25.18 + 366a5b23-2f2e-4cf0-a714-6a306bd4e909 + TARGET DEBIT CRD ACH TRAN + ACH Debit + + + DEBIT + 20171121120000.000[0:GMT] + -10.71 + 9a463f21-c6e1-4fe0-b37b-f9a8cc942cf0 + NETFLIX COM NETFLIX COM + Point of Sale Debit L999 DATE 11-20 + + + CREDIT + 20171122120000.000[0:GMT] + 1300.98 + 31f165e5-569f-4530-8438-a6ceb2301335 + DIRECT DEPOSIT + ACH Deposit 838383838383838 + + + CREDIT + 20171122120000.000[0:GMT] + 12.50 + 215a10dd-f3a2-4336-ab8c-f22276cad552 + CIRCLE INTERNET CIRCLE + ACH Deposit 017326000283477 + + + + 2620.37 + 20171126184401.637[0:GMT] + + + 3620.37 + 20171126184401.637[0:GMT] + + + + diff --git a/internal/handlers/handlers_testdata/checking_20171129.ofx b/internal/handlers/handlers_testdata/checking_20171129.ofx new file mode 100644 index 0000000..57d440a --- /dev/null +++ b/internal/handlers/handlers_testdata/checking_20171129.ofx @@ -0,0 +1,123 @@ + + + + + + 0 + INFO + + 20171129025346.132[0:GMT] + ENG + + YCKVJ + 0351 + + + + + 0549c828-f02c-43c7-81a3-de0b3f23c393 + + 0 + INFO + + + USD + + 115483849 + 14839128817 + CHECKING + + + 20170831174401.637[0:GMT] + 20171129184401.637[0:GMT] + + DEBIT + 20171107120000.000[0:GMT] + -282.68 + 29c74a94-f226-4980-b54c-da6fa2721d7e + DAYCARE o SIGONFILE + ACH Debit 11818191919191 + + + CREDIT + 20171109120000.000[0:GMT] + 1300.98 + 32e40e98-61c3-421c-acaa-55ae67a5f8fe + DIRECT DEPOSIT + ACH Deposit 8282828282828 + + + DEBIT + 20171109120000.000[0:GMT] + -98.20 + 4b73dbbf-aa27-4f62-b54a-ee0a9a3486d8 + DUKEENGYPROGRESS DUKEENGYPR + ACH Debit 017313004099621 + + + CREDIT + 20171115120000.000[0:GMT] + 1.01 + 51c47252-4cf0-442c-b619-8a31b17ac489 + Dividend Earned + + + DEBIT + 20171116120000.000[0:GMT] + -51.75 + 51cb12bb-cdd9-4333-8d8d-c423f9e8f833 + TARGET DEBIT CRD ACH TRAN + ACH Debit + + + DEBIT + 20171120120000.000[0:GMT] + -25.18 + 366a5b23-2f2e-4cf0-a714-6a306bd4e909 + TARGET DEBIT CRD ACH TRAN + ACH Debit + + + DEBIT + 20171121120000.000[0:GMT] + -10.71 + 9a463f21-c6e1-4fe0-b37b-f9a8cc942cf0 + NETFLIX COM NETFLIX COM + Point of Sale Debit L999 DATE 11-20 + + + CREDIT + 20171122120000.000[0:GMT] + 1300.98 + 31f165e5-569f-4530-8438-a6ceb2301335 + DIRECT DEPOSIT + ACH Deposit 838383838383838 + + + CREDIT + 20171122120000.000[0:GMT] + 12.50 + 215a10dd-f3a2-4336-ab8c-f22276cad552 + CIRCLE INTERNET CIRCLE + ACH Deposit 017326000283477 + + + CREDIT + 20171129120000.000[0:GMT] + 2843.08 + 9a52df4b-3a8d-41bb-9141-96e1e3f294cf + SALARY + ACH Deposit 18181818199 + + + + 5463.45 + 20171129025346.132[0:GMT] + + + 6463.45 + 20171129025346.132[0:GMT] + + + + diff --git a/internal/handlers/ofx_test.go b/internal/handlers/ofx_test.go new file mode 100644 index 0000000..74ce496 --- /dev/null +++ b/internal/handlers/ofx_test.go @@ -0,0 +1,36 @@ +package handlers_test + +import ( + "net/http" + "strconv" + "testing" +) + +func importOFX(client *http.Client, accountid int64, filename string) error { + return uploadFile(client, filename, "/v1/accounts/"+strconv.FormatInt(accountid, 10)+"/imports/ofxfile") +} + +func TestImportOFX(t *testing.T) { + RunWith(t, &data[0], func(t *testing.T, d *TestData) { + // Ensure there's only one USD currency + oldDefault, err := getSecurity(d.clients[0], d.users[0].DefaultCurrency) + if err != nil { + t.Fatalf("Error fetching default security: %s\n", err) + } + d.users[0].DefaultCurrency = d.securities[0].SecurityId + if _, err := updateUser(d.clients[0], &d.users[0]); err != nil { + t.Fatalf("Error updating user: %s\n", err) + } + if err := deleteSecurity(d.clients[0], oldDefault); err != nil { + t.Fatalf("Error removing default security: %s\n", err) + } + + // Import and ensure it didn't return a nasty error code + if err = importOFX(d.clients[0], d.accounts[1].AccountId, "handlers_testdata/checking_20171126.ofx"); err != nil { + t.Fatalf("Error importing OFX: %s\n", err) + } + if err = importOFX(d.clients[0], d.accounts[1].AccountId, "handlers_testdata/checking_20171129.ofx"); err != nil { + t.Fatalf("Error importing OFX: %s\n", err) + } + }) +} diff --git a/internal/handlers/testdata_test.go b/internal/handlers/testdata_test.go index 5ca0eb2..88303ba 100644 --- a/internal/handlers/testdata_test.go +++ b/internal/handlers/testdata_test.go @@ -257,7 +257,7 @@ var data = []TestData{ UserId: 0, SecurityId: 0, ParentAccountId: 0, - Type: handlers.Asset, + Type: handlers.Bank, Name: "Credit Union Checking", }, { From d656f15e841fbfb4451e442c60a899d9503c40f0 Mon Sep 17 00:00:00 2001 From: Aaron Lindsay Date: Sun, 26 Nov 2017 21:01:26 -0500 Subject: [PATCH 2/6] testing: Ensure account balance is correct after OFX import --- internal/handlers/common_test.go | 12 ++++++++++++ internal/handlers/gnucash_test.go | 22 +++++----------------- internal/handlers/ofx_test.go | 3 +++ 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/internal/handlers/common_test.go b/internal/handlers/common_test.go index a0141d0..5ff8c02 100644 --- a/internal/handlers/common_test.go +++ b/internal/handlers/common_test.go @@ -202,6 +202,18 @@ func uploadFile(client *http.Client, filename, urlsuffix string) error { return nil } +func accountBalanceHelper(t *testing.T, client *http.Client, account *handlers.Account, balance string) { + t.Helper() + transactions, err := getAccountTransactions(client, account.AccountId, 0, 0, "") + if err != nil { + t.Fatalf("Couldn't fetch account transactions for '%s': %s\n", account.Name, err) + } + + if transactions.EndingBalance != balance { + t.Errorf("Expected ending balance for '%s' to be '%s', but found %s\n", account.Name, balance, transactions.EndingBalance) + } +} + func RunWith(t *testing.T, d *TestData, fn TestDataFunc) { testdata, err := d.Initialize() if err != nil { diff --git a/internal/handlers/gnucash_test.go b/internal/handlers/gnucash_test.go index bf66e57..1cdf9d0 100644 --- a/internal/handlers/gnucash_test.go +++ b/internal/handlers/gnucash_test.go @@ -10,18 +10,6 @@ func importGnucash(client *http.Client, filename string) error { return uploadFile(client, filename, "/v1/imports/gnucash") } -func gnucashAccountBalanceHelper(t *testing.T, client *http.Client, account *handlers.Account, balance string) { - t.Helper() - transactions, err := getAccountTransactions(client, account.AccountId, 0, 0, "") - if err != nil { - t.Fatalf("Couldn't fetch account transactions for '%s': %s\n", account.Name, err) - } - - if transactions.EndingBalance != balance { - t.Errorf("Expected ending balance for '%s' to be '%s', but found %s\n", account.Name, balance, transactions.EndingBalance) - } -} - func TestImportGnucash(t *testing.T) { RunWith(t, &data[0], func(t *testing.T, d *TestData) { // Ensure there's only one USD currency @@ -100,11 +88,11 @@ func TestImportGnucash(t *testing.T) { t.Fatalf("Couldn't find 'Expenses/Cable' account") } - gnucashAccountBalanceHelper(t, d.clients[0], salary, "-998.34") - gnucashAccountBalanceHelper(t, d.clients[0], creditcard, "-272.03") - gnucashAccountBalanceHelper(t, d.clients[0], openingbalances, "-21014.33") - gnucashAccountBalanceHelper(t, d.clients[0], groceries, "287.56") // 87.19 from preexisting transactions and 200.37 from Gnucash - gnucashAccountBalanceHelper(t, d.clients[0], cable, "89.98") + accountBalanceHelper(t, d.clients[0], salary, "-998.34") + accountBalanceHelper(t, d.clients[0], creditcard, "-272.03") + accountBalanceHelper(t, d.clients[0], openingbalances, "-21014.33") + accountBalanceHelper(t, d.clients[0], groceries, "287.56") // 87.19 from preexisting transactions and 200.37 from Gnucash + accountBalanceHelper(t, d.clients[0], cable, "89.98") var ge *handlers.Security securities, err := getSecurities(d.clients[0]) diff --git a/internal/handlers/ofx_test.go b/internal/handlers/ofx_test.go index 74ce496..1e6a440 100644 --- a/internal/handlers/ofx_test.go +++ b/internal/handlers/ofx_test.go @@ -29,8 +29,11 @@ func TestImportOFX(t *testing.T) { if err = importOFX(d.clients[0], d.accounts[1].AccountId, "handlers_testdata/checking_20171126.ofx"); err != nil { t.Fatalf("Error importing OFX: %s\n", err) } + accountBalanceHelper(t, d.clients[0], &d.accounts[1], "2493.19") + if err = importOFX(d.clients[0], d.accounts[1].AccountId, "handlers_testdata/checking_20171129.ofx"); err != nil { t.Fatalf("Error importing OFX: %s\n", err) } + accountBalanceHelper(t, d.clients[0], &d.accounts[1], "5336.27") }) } From 238809cd46a676bb589e99bc1998c619f8355b1e Mon Sep 17 00:00:00 2001 From: Aaron Lindsay Date: Tue, 28 Nov 2017 20:14:38 -0500 Subject: [PATCH 3/6] testing: Add initial OFX credit card import test --- .../handlers/handlers_testdata/creditcard.ofx | 11 ++++++++ internal/handlers/ofx_test.go | 25 ++++++++++++++++++- internal/handlers/testdata_test.go | 11 ++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 internal/handlers/handlers_testdata/creditcard.ofx diff --git a/internal/handlers/handlers_testdata/creditcard.ofx b/internal/handlers/handlers_testdata/creditcard.ofx new file mode 100644 index 0000000..e2ba6de --- /dev/null +++ b/internal/handlers/handlers_testdata/creditcard.ofx @@ -0,0 +1,11 @@ +OFXHEADER:100 +DATA:OFXSGML +VERSION:102 +SECURITY:NONE +ENCODING:USASCII +CHARSET:1252 +COMPRESSION:NONE +OLDFILEUID:NONE +NEWFILEUID:NONE + +0INFOSUCCESS20171128054239.013[-5:EST]ENGC2292921cc61e4b-1f74-7d9a-b143-b8c80d5fda580INFOUSD123412341234123420170731054239.277[-4:EDT]20171128054239.277[-5:EST]DEBIT20171016120000[0:GMT]-99.982017101624445727288300440999736KROGER #111DEBIT20170910120000[0:GMT]-1502017091024493987251438675718282CHARITY DONATIONDEBIT20170814120000[0:GMT]-44.992017081424692167225100642481235CABLECREDIT20171101120000[0:GMT]185.712017110123053057200000291455612Payment Thank You ElectroDEBIT20171016120000[0:GMT]-4.492017101624510727289100677772726CRAFTSCREDIT20170815120000[0:GMT]109.262017081574692167226100322807539Example.com-4.4920171128070000.000[-5:EST]995.5120171128070000.000[-5:EST] diff --git a/internal/handlers/ofx_test.go b/internal/handlers/ofx_test.go index 1e6a440..ddbdf6e 100644 --- a/internal/handlers/ofx_test.go +++ b/internal/handlers/ofx_test.go @@ -10,7 +10,7 @@ func importOFX(client *http.Client, accountid int64, filename string) error { return uploadFile(client, filename, "/v1/accounts/"+strconv.FormatInt(accountid, 10)+"/imports/ofxfile") } -func TestImportOFX(t *testing.T) { +func TestImportOFXChecking(t *testing.T) { RunWith(t, &data[0], func(t *testing.T, d *TestData) { // Ensure there's only one USD currency oldDefault, err := getSecurity(d.clients[0], d.users[0].DefaultCurrency) @@ -37,3 +37,26 @@ func TestImportOFX(t *testing.T) { accountBalanceHelper(t, d.clients[0], &d.accounts[1], "5336.27") }) } + +func TestImportOFXCreditCard(t *testing.T) { + RunWith(t, &data[0], func(t *testing.T, d *TestData) { + // Ensure there's only one USD currency + oldDefault, err := getSecurity(d.clients[0], d.users[0].DefaultCurrency) + if err != nil { + t.Fatalf("Error fetching default security: %s\n", err) + } + d.users[0].DefaultCurrency = d.securities[0].SecurityId + if _, err := updateUser(d.clients[0], &d.users[0]); err != nil { + t.Fatalf("Error updating user: %s\n", err) + } + if err := deleteSecurity(d.clients[0], oldDefault); err != nil { + t.Fatalf("Error removing default security: %s\n", err) + } + + // Import and ensure it didn't return a nasty error code + if err = importOFX(d.clients[0], d.accounts[7].AccountId, "handlers_testdata/creditcard.ofx"); err != nil { + t.Fatalf("Error importing OFX: %s\n", err) + } + accountBalanceHelper(t, d.clients[0], &d.accounts[7], "-4.49") + }) +} diff --git a/internal/handlers/testdata_test.go b/internal/handlers/testdata_test.go index 88303ba..db13381 100644 --- a/internal/handlers/testdata_test.go +++ b/internal/handlers/testdata_test.go @@ -295,6 +295,13 @@ var data = []TestData{ Type: handlers.Expense, Name: "Expenses", }, + { + UserId: 0, + SecurityId: 0, + ParentAccountId: -1, + Type: handlers.Liability, + Name: "Credit Card", + }, }, transactions: []handlers.Transaction{ { @@ -462,6 +469,10 @@ end`, }, }, }, + "Credit Card": { + Values: []float64{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + Series: map[string]*handlers.Series{}, + }, }, }, }, From 30d4515780451fde5c33a906077eee846f132ce9 Mon Sep 17 00:00:00 2001 From: Aaron Lindsay Date: Tue, 28 Nov 2017 21:25:50 -0500 Subject: [PATCH 4/6] testing: Add initial OFX investment import tests Also fix up a "bug" caused by financial institutions not deciding on which sign to use for an OFX field... --- .../handlers_testdata/401k_mutualfunds.ofx | 1 + internal/handlers/ofx.go | 9 +++++ internal/handlers/ofx_test.go | 37 +++++++++++++++++++ 3 files changed, 47 insertions(+) create mode 100644 internal/handlers/handlers_testdata/401k_mutualfunds.ofx diff --git a/internal/handlers/handlers_testdata/401k_mutualfunds.ofx b/internal/handlers/handlers_testdata/401k_mutualfunds.ofx new file mode 100644 index 0000000..18ba784 --- /dev/null +++ b/internal/handlers/handlers_testdata/401k_mutualfunds.ofx @@ -0,0 +1 @@ + 0INFOSUCCESS20171128203521.622[-5:EST]ENGofx.bank.com9199 d87db96a-c872-7f73-7637-7e9e2816c25a 0INFOSUCCESS20171128193521.926[-5:EST]USDofx.bank.com12321 20170829213521.814[-4:EDT]20171127203521.814[-5:EST]20170901OAEL01120170901070000.000[-4:EDT]CONTRIBUTION;VANGUARD TARGET 2045 OAEL;as of 09/01/2017OAELCUSIP1.75656.97100.05OTHEROTHERBUY 20170915OAEL01120170915070000.000[-4:EDT]CONTRIBUTION;VANGUARD TARGET 2045 OAEL;as of 09/15/2017OAELCUSIP1.73757.59100.05OTHEROTHERBUY 20170901OAEL13120170901070000.000[-4:EDT]FEES;VANGUARD TARGET 2045 OAEL;as of 09/01/2017OAELCUSIP0.0756.974.0OTHEROTHERSELL 20171002OAEL13120171002070000.000[-4:EDT]FEES;VANGUARD TARGET 2045 OAEL;as of 10/02/2017OAELCUSIP0.06958.14.0OTHEROTHERSELL OAELCUSIPOTHERLONG2792.37359.64200.03 20171127160000.000[-5:EST] Market close as of 11/27/2017;VANGUARD TARGET 2045 000MarketValueMarketValueDOLLAR200.0320171128193521.926[-5:EST]VestedValueVestedValueDOLLAR200.0320171128193521.926[-5:EST]TotalAssetsValueTotalAssetsValueDOLLAR200.0320171128193521.926[-5:EST]QC 401(K) PLAN200.03MarketValueMarketValueDOLLAR200.0320171128193521.926[-5:EST]VestedValueVestedValueDOLLAR200.0320171128193521.926[-5:EST]TotalAssetsValueTotalAssetsValueDOLLAR200.0320171128193521.926[-5:EST] OAELCUSIPVANGUARD TARGET 2045OAEL59.6420171127160000.000[-5:EST]Market close as of 11/27/2017;VANGUARD TARGET 2045OTHER diff --git a/internal/handlers/ofx.go b/internal/handlers/ofx.go index 1acd120..b559000 100644 --- a/internal/handlers/ofx.go +++ b/internal/handlers/ofx.go @@ -233,6 +233,9 @@ func (i *OFXImport) GetInvBuyTran(buy *ofxgo.InvBuy, curdef *Security, account * fees.Set(&buy.Fees.Rat) load.Set(&buy.Load.Rat) total.Set(&buy.Total.Rat) + if total.Sign() > 0 { + total.Neg(&total) + } tradingTotal.Neg(&total) tradingTotal.Sub(&tradingTotal, &commission) @@ -495,6 +498,9 @@ func (i *OFXImport) GetReinvestTran(reinvest *ofxgo.Reinvest, curdef *Security, fees.Set(&reinvest.Fees.Rat) load.Set(&reinvest.Load.Rat) total.Set(&reinvest.Total.Rat) + if total.Sign() > 0 { + total.Neg(&total) + } tradingTotal.Neg(&total) tradingTotal.Sub(&tradingTotal, &commission) @@ -694,6 +700,9 @@ func (i *OFXImport) GetInvSellTran(sell *ofxgo.InvSell, curdef *Security, accoun fees.Set(&sell.Fees.Rat) load.Set(&sell.Load.Rat) total.Set(&sell.Total.Rat) + if total.Sign() < 0 { + total.Neg(&total) + } tradingTotal.Neg(&total) tradingTotal.Sub(&tradingTotal, &commission) diff --git a/internal/handlers/ofx_test.go b/internal/handlers/ofx_test.go index ddbdf6e..3e4814e 100644 --- a/internal/handlers/ofx_test.go +++ b/internal/handlers/ofx_test.go @@ -1,6 +1,7 @@ package handlers_test import ( + "github.com/aclindsa/moneygo/internal/handlers" "net/http" "strconv" "testing" @@ -60,3 +61,39 @@ func TestImportOFXCreditCard(t *testing.T) { accountBalanceHelper(t, d.clients[0], &d.accounts[7], "-4.49") }) } + +func TestImportOFX401kMutualFunds(t *testing.T) { + RunWith(t, &data[0], func(t *testing.T, d *TestData) { + // Ensure there's only one USD currency + oldDefault, err := getSecurity(d.clients[0], d.users[0].DefaultCurrency) + if err != nil { + t.Fatalf("Error fetching default security: %s\n", err) + } + d.users[0].DefaultCurrency = d.securities[0].SecurityId + if _, err := updateUser(d.clients[0], &d.users[0]); err != nil { + t.Fatalf("Error updating user: %s\n", err) + } + if err := deleteSecurity(d.clients[0], oldDefault); err != nil { + t.Fatalf("Error removing default security: %s\n", err) + } + + account := &handlers.Account{ + SecurityId: d.securities[0].SecurityId, + UserId: d.users[0].UserId, + ParentAccountId: -1, + Type: handlers.Investment, + Name: "401k", + } + + account, err = createAccount(d.clients[0], account) + if err != nil { + t.Fatalf("Error creating 401k account: %s\n", err) + } + + // Import and ensure it didn't return a nasty error code + if err = importOFX(d.clients[0], account.AccountId, "handlers_testdata/401k_mutualfunds.ofx"); err != nil { + t.Fatalf("Error importing OFX: %s\n", err) + } + accountBalanceHelper(t, d.clients[0], account, "-192.10") + }) +} From 05fdaaeb425d1cc2742f0959f8b10d99e72a21d9 Mon Sep 17 00:00:00 2001 From: Aaron Lindsay Date: Wed, 29 Nov 2017 19:59:02 -0500 Subject: [PATCH 5/6] testing: Add checks for OFX investment balances Also fix a "bug" uncovered, relating to at least one FI's providing unexpected signs for some OFX fields. --- internal/handlers/ofx.go | 62 +++++++++++++++++------------------ internal/handlers/ofx_test.go | 49 +++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 31 deletions(-) diff --git a/internal/handlers/ofx.go b/internal/handlers/ofx.go index b559000..68896b5 100644 --- a/internal/handlers/ofx.go +++ b/internal/handlers/ofx.go @@ -228,14 +228,13 @@ func (i *OFXImport) GetInvBuyTran(buy *ofxgo.InvBuy, curdef *Security, account * } var commission, taxes, fees, load, total, tradingTotal big.Rat - commission.Set(&buy.Commission.Rat) - taxes.Set(&buy.Taxes.Rat) - fees.Set(&buy.Fees.Rat) - load.Set(&buy.Load.Rat) - total.Set(&buy.Total.Rat) - if total.Sign() > 0 { - total.Neg(&total) - } + commission.Abs(&buy.Commission.Rat) + taxes.Abs(&buy.Taxes.Rat) + fees.Abs(&buy.Fees.Rat) + load.Abs(&buy.Load.Rat) + total.Abs(&buy.Total.Rat) + + total.Neg(&total) tradingTotal.Neg(&total) tradingTotal.Sub(&tradingTotal, &commission) @@ -322,8 +321,8 @@ func (i *OFXImport) GetInvBuyTran(buy *ofxgo.InvBuy, curdef *Security, account * Amount: tradingTotal.FloatString(curdef.Precision), }) - units := big.NewRat(0, 1) - units.Set(&buy.Units.Rat) + var units big.Rat + units.Abs(&buy.Units.Rat) t.Splits = append(t.Splits, &Split{ // TODO ReversalFiTID? Status: Imported, @@ -334,7 +333,7 @@ func (i *OFXImport) GetInvBuyTran(buy *ofxgo.InvBuy, curdef *Security, account * Memo: memo, Amount: units.FloatString(security.Precision), }) - units.Neg(units) + units.Neg(&units) t.Splits = append(t.Splits, &Split{ // TODO ReversalFiTID? Status: Imported, @@ -493,14 +492,13 @@ func (i *OFXImport) GetReinvestTran(reinvest *ofxgo.Reinvest, curdef *Security, } var commission, taxes, fees, load, total, tradingTotal big.Rat - commission.Set(&reinvest.Commission.Rat) - taxes.Set(&reinvest.Taxes.Rat) - fees.Set(&reinvest.Fees.Rat) - load.Set(&reinvest.Load.Rat) - total.Set(&reinvest.Total.Rat) - if total.Sign() > 0 { - total.Neg(&total) - } + commission.Abs(&reinvest.Commission.Rat) + taxes.Abs(&reinvest.Taxes.Rat) + fees.Abs(&reinvest.Fees.Rat) + load.Abs(&reinvest.Load.Rat) + total.Abs(&reinvest.Total.Rat) + + total.Neg(&total) tradingTotal.Neg(&total) tradingTotal.Sub(&tradingTotal, &commission) @@ -610,7 +608,7 @@ func (i *OFXImport) GetReinvestTran(reinvest *ofxgo.Reinvest, curdef *Security, }) var units big.Rat - units.Set(&reinvest.Units.Rat) + units.Abs(&reinvest.Units.Rat) t.Splits = append(t.Splits, &Split{ // TODO ReversalFiTID? Status: Imported, @@ -695,14 +693,16 @@ func (i *OFXImport) GetInvSellTran(sell *ofxgo.InvSell, curdef *Security, accoun } var commission, taxes, fees, load, total, tradingTotal big.Rat - commission.Set(&sell.Commission.Rat) - taxes.Set(&sell.Taxes.Rat) - fees.Set(&sell.Fees.Rat) - load.Set(&sell.Load.Rat) - total.Set(&sell.Total.Rat) - if total.Sign() < 0 { - total.Neg(&total) - } + commission.Abs(&sell.Commission.Rat) + taxes.Abs(&sell.Taxes.Rat) + fees.Abs(&sell.Fees.Rat) + load.Abs(&sell.Load.Rat) + total.Abs(&sell.Total.Rat) + + commission.Neg(&commission) + taxes.Neg(&taxes) + fees.Neg(&fees) + load.Neg(&load) tradingTotal.Neg(&total) tradingTotal.Sub(&tradingTotal, &commission) @@ -790,11 +790,11 @@ func (i *OFXImport) GetInvSellTran(sell *ofxgo.InvSell, curdef *Security, accoun }) var units big.Rat - units.Set(&sell.Units.Rat) + units.Abs(&sell.Units.Rat) t.Splits = append(t.Splits, &Split{ // TODO ReversalFiTID? Status: Imported, - ImportSplitType: SubAccount, + ImportSplitType: TradingAccount, AccountId: -1, SecurityId: security.SecurityId, RemoteId: "ofx:" + sell.InvTran.FiTID.String(), @@ -805,7 +805,7 @@ func (i *OFXImport) GetInvSellTran(sell *ofxgo.InvSell, curdef *Security, accoun t.Splits = append(t.Splits, &Split{ // TODO ReversalFiTID? Status: Imported, - ImportSplitType: TradingAccount, + ImportSplitType: SubAccount, AccountId: -1, SecurityId: security.SecurityId, RemoteId: "ofx:" + sell.InvTran.FiTID.String(), diff --git a/internal/handlers/ofx_test.go b/internal/handlers/ofx_test.go index 3e4814e..961d0b0 100644 --- a/internal/handlers/ofx_test.go +++ b/internal/handlers/ofx_test.go @@ -1,6 +1,7 @@ package handlers_test import ( + "fmt" "github.com/aclindsa/moneygo/internal/handlers" "net/http" "strconv" @@ -62,6 +63,32 @@ func TestImportOFXCreditCard(t *testing.T) { }) } +func findSecurity(client *http.Client, symbol string, tipe handlers.SecurityType) (*handlers.Security, error) { + securities, err := getSecurities(client) + if err != nil { + return nil, err + } + for _, security := range *securities.Securities { + if security.Symbol == symbol && security.Type == tipe { + return security, nil + } + } + return nil, fmt.Errorf("Unable to find security: \"%s\"", symbol) +} + +func findAccount(client *http.Client, name string, tipe handlers.AccountType, securityid int64) (*handlers.Account, error) { + accounts, err := getAccounts(client) + if err != nil { + return nil, err + } + for _, account := range *accounts.Accounts { + if account.Name == name && account.Type == tipe && account.SecurityId == securityid { + return &account, nil + } + } + return nil, fmt.Errorf("Unable to find account: \"%s\"", name) +} + func TestImportOFX401kMutualFunds(t *testing.T) { RunWith(t, &data[0], func(t *testing.T, d *TestData) { // Ensure there's only one USD currency @@ -95,5 +122,27 @@ func TestImportOFX401kMutualFunds(t *testing.T) { t.Fatalf("Error importing OFX: %s\n", err) } accountBalanceHelper(t, d.clients[0], account, "-192.10") + + // Make sure the security was created and that the trading account has + // the right value + security, err := findSecurity(d.clients[0], "VANGUARD TARGET 2045", handlers.Stock) + if err != nil { + t.Fatalf("Error finding VANGUARD TARGET 2045 security: %s\n", err) + } + tradingaccount, err := findAccount(d.clients[0], "VANGUARD TARGET 2045", handlers.Trading, security.SecurityId) + if err != nil { + t.Fatalf("Error finding VANGUARD TARGET 2045 trading account: %s\n", err) + } + accountBalanceHelper(t, d.clients[0], tradingaccount, "-3.35400") + + // Ensure actual holding account was created and in the correct place + investmentaccount, err := findAccount(d.clients[0], "VANGUARD TARGET 2045", handlers.Investment, security.SecurityId) + if err != nil { + t.Fatalf("Error finding VANGUARD TARGET 2045 investment account: %s\n", err) + } + accountBalanceHelper(t, d.clients[0], investmentaccount, "3.35400") + if investmentaccount.ParentAccountId != account.AccountId { + t.Errorf("Expected imported security account to be child of investment account it's imported into\n") + } }) } From ffc860233678d8ac50349dd3415c59b401699032 Mon Sep 17 00:00:00 2001 From: Aaron Lindsay Date: Fri, 1 Dec 2017 21:21:55 -0500 Subject: [PATCH 6/6] testing: Add another brokerage account OFX import test --- .../handlers/handlers_testdata/brokerage.ofx | 11 +++ internal/handlers/ofx.go | 8 +- internal/handlers/ofx_test.go | 81 +++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 internal/handlers/handlers_testdata/brokerage.ofx diff --git a/internal/handlers/handlers_testdata/brokerage.ofx b/internal/handlers/handlers_testdata/brokerage.ofx new file mode 100644 index 0000000..5e74125 --- /dev/null +++ b/internal/handlers/handlers_testdata/brokerage.ofx @@ -0,0 +1,11 @@ +0INFOSuccessful Sign On20171130013742ENG20160713012000Somewhere9277201927017240917209172407124984652986a59df4c3-9408-00bb-fd6d-0a06695885b10INFO20171129160000.000[-5:EST]USDinvesting.example.com7372829220160529160000.000[-5:EST]160000.000[-5:EST]20171130013742.000[-5:EST] +99397105620170315160000.000[-5:EST]20170316160000.000[-5:EST]BUY921937108CUSIP37.70010.61-400.0CASHCASHBUY +20604694120160620160000.000[-5:EST]20160623160000.000[-5:EST]BUY921909768CUSIP15.045.17987-677.70CASHCASHBUY +6359059020160729160000.000[-5:EST]20160729160000.000[-5:EST]DIVIDEND PAYMENTDIVIDEND PAYMENT78462F103CUSIPDIV1.08CASHCASH +76922351720160606160000.000[-5:EST]20160606160000.000[-5:EST]DIVIDEND REINVESTMENTDIVIDEND REINVESTMENT049560105CUSIPDIV-6.43CASH0.08674.9777 +61968901820160627160000.000[-5:EST]20160627160000.000[-5:EST]MONEY FUND REDEMPTION922906300CUSIP-21.571.021.57CASHCASHSELL +32844449920160708160000.000[-5:EST]20160713160000.000[-5:EST]SELL921909768CUSIP-10.044.260130.07442.53CASHCASHSELL +OTHER20160607160000.000[-5:EST]1000.0230048208CASH + +049560105CUSIPCASHLONG6.08690.51550.84320171128160000.000[-5:EST]Price as of date based on closing priceY921937108CUSIPCASHLONG37.7710.75406.0220171128160000.000[-5:EST]Price as of date based on closing priceYY922906300CUSIPCASHLONG24.871.024.8720171128160000.000[-5:EST]Price as of date based on closing priceNN +387.480.00.0049560105CUSIPATMOS ENERGY CORPATO90.51Price as of date based on closing priceCOMMON2.1491921909768CUSIPVANGUARD TOTAL INTL STOCK INDE921909768BUY78462F103CUSIP20SPDR SP 500 ETF78462F103SELL921937108CUSIPVanguard Total Bond Market Index Fund Investor SharesVBMFX10.75Price as of date based on closing priceOPENEND922906300CUSIPVanguard Federal Money Market FundVMFXX1.0Price as of date based on closing priceOPENEND diff --git a/internal/handlers/ofx.go b/internal/handlers/ofx.go index 68896b5..8c08a67 100644 --- a/internal/handlers/ofx.go +++ b/internal/handlers/ofx.go @@ -705,10 +705,10 @@ func (i *OFXImport) GetInvSellTran(sell *ofxgo.InvSell, curdef *Security, accoun load.Neg(&load) tradingTotal.Neg(&total) - tradingTotal.Sub(&tradingTotal, &commission) - tradingTotal.Sub(&tradingTotal, &taxes) - tradingTotal.Sub(&tradingTotal, &fees) - tradingTotal.Sub(&tradingTotal, &load) + tradingTotal.Add(&tradingTotal, &commission) + tradingTotal.Add(&tradingTotal, &taxes) + tradingTotal.Add(&tradingTotal, &fees) + tradingTotal.Add(&tradingTotal, &load) // Convert amounts to account's currency if Currency is set if ok, _ := sell.Currency.Valid(); ok { diff --git a/internal/handlers/ofx_test.go b/internal/handlers/ofx_test.go index 961d0b0..baf452a 100644 --- a/internal/handlers/ofx_test.go +++ b/internal/handlers/ofx_test.go @@ -146,3 +146,84 @@ func TestImportOFX401kMutualFunds(t *testing.T) { } }) } + +func TestImportOFXBrokerage(t *testing.T) { + RunWith(t, &data[0], func(t *testing.T, d *TestData) { + // Ensure there's only one USD currency + oldDefault, err := getSecurity(d.clients[0], d.users[0].DefaultCurrency) + if err != nil { + t.Fatalf("Error fetching default security: %s\n", err) + } + d.users[0].DefaultCurrency = d.securities[0].SecurityId + if _, err := updateUser(d.clients[0], &d.users[0]); err != nil { + t.Fatalf("Error updating user: %s\n", err) + } + if err := deleteSecurity(d.clients[0], oldDefault); err != nil { + t.Fatalf("Error removing default security: %s\n", err) + } + + // Create the brokerage account + account := &handlers.Account{ + SecurityId: d.securities[0].SecurityId, + UserId: d.users[0].UserId, + ParentAccountId: -1, + Type: handlers.Investment, + Name: "Personal Brokerage", + } + + account, err = createAccount(d.clients[0], account) + if err != nil { + t.Fatalf("Error creating 'Personal Brokerage' account: %s\n", err) + } + + // Import and ensure it didn't return a nasty error code + if err = importOFX(d.clients[0], account.AccountId, "handlers_testdata/brokerage.ofx"); err != nil { + t.Fatalf("Error importing OFX: %s\n", err) + } + accountBalanceHelper(t, d.clients[0], account, "387.48") + + // Make sure the USD trading account was created and has the right + // value + usdtrading, err := findAccount(d.clients[0], "USD", handlers.Trading, d.users[0].DefaultCurrency) + if err != nil { + t.Fatalf("Error finding USD trading account: %s\n", err) + } + accountBalanceHelper(t, d.clients[0], usdtrading, "619.96") + + // Check investment/trading balances for all securities traded + checks := []struct { + Ticker string + Name string + Balance string + TradingBalance string + }{ + {"VBMFX", "Vanguard Total Bond Market Index Fund Investor Shares", "37.70000", "-37.70000"}, + {"921909768", "VANGUARD TOTAL INTL STOCK INDE", "5.00000", "-5.00000"}, + {"ATO", "ATMOS ENERGY CORP", "0.08600", "-0.08600"}, + {"VMFXX", "Vanguard Federal Money Market Fund", "-21.57000", "21.57000"}, + } + + for _, check := range checks { + security, err := findSecurity(d.clients[0], check.Ticker, handlers.Stock) + if err != nil { + t.Fatalf("Error finding security: %s\n", err) + } + + account, err := findAccount(d.clients[0], check.Name, handlers.Investment, security.SecurityId) + if err != nil { + t.Fatalf("Error finding trading account: %s\n", err) + } + + accountBalanceHelper(t, d.clients[0], account, check.Balance) + + tradingaccount, err := findAccount(d.clients[0], check.Name, handlers.Trading, security.SecurityId) + if err != nil { + t.Fatalf("Error finding trading account: %s\n", err) + } + + accountBalanceHelper(t, d.clients[0], tradingaccount, check.TradingBalance) + } + + // TODO check reinvestment/income to make sure they're registered as income? + }) +}