Repository: reubano/csv2ofx (language: python)

Issue:
Add Schwab bank mapping, LEDGERBAL tag support, MS Money compatible output
This PR adds the ability to convert CSV files downloaded from Charles Schwab checking accounts to OFX files supported by MS Money.

For convenience, I broke the changes down into the following logically-distinct commits:

* Ignore vim temp files
* Add option for MS Money compatible output
  * Deal with MS Money idiosyncrasies RE: OFX formatting
  * MS Money specific OFX header
  * Provide dummy value for TRNUID
  * Don't include blank CHECKNUM
  * Limit payee length to 32 chars
  * Use noon for dates lacking a time (Recommended by MS Money troubleshooting OFX guide)
* Include LEDGERBAL in non-split OFX output if possible
  * These are required fields per OFX spec & their absence breaks MS Money import.
* Add mapping for Charles Schwab Bank
* Add tests for ending balance analysis
* Update example --help output

Confirmed no new errors from `manage lint`.  Confirmed all `manage test` tests pass.

Tested with >18 months of real-world data and manually verified transaction sanity in MS Money.

(Please let me know if anything doesn't meet coding standards, etc.  Unfortunately, I'm not a native Python speaker.  😃)


Discussion / hints:
I don't have Charles Schwab, but my CC stopped creating OFX files and now just provides CSV files. So I locally patched with 69140f3, and the resulting OFX file does seem to generally match what I historically had for my CC account OFX files before, especially the header. The biggest difference is that the new csv2ofx output closes all tags, while the CC original leaves many tags open. However, I'm not able to import the generated OFX into MS Money because it says it contains invalid or corrupt data. I'm not sure if it's a problem with csv2ofx or my mapping file.

This is the original CSV from my CC:
```CSV
Trans. Date,Post Date,Description,Amount,Category
12/18/2021,12/18/2021,"ATMOS ENERGY 888-286-6700",151.75,"Services"
12/11/2021,12/13/2021,"KOHL'S",-66.22,"Payments and Credits"
```

|Trans. Date|Post Date |Description                      |Amount|Category            |
|-----------|----------|---------------------------------|------|--------------------|
|12/18/2021 |12/18/2021|ATMOS ENERGY 888-286-6700        |151.75|Services            |
|12/11/2021 |12/13/2021|KOHL'S                           |-66.22|Payments and Credits|

I luckily still had the original OFX from the CC, and comparing it to the generated OFX, the transaction type seems backward. All purchases are listed as CREDIT with a positive value, while payments are DEBIT with a negative value. This is backwards from the original.
**Original OFX**
```XML
<STMTTRN>
<TRNTYPE>DEBIT
<DTPOSTED>20211218170000.000
<TRNAMT>-151.75
<FITID>FITID20211219-151.75Q65G3
<NAME>ATMOS ENERGY 888-286-6700 TX
</STMTTRN>
```
**csv2ofx OFX**
```XML
<STMTTRN>
<TRNTYPE>CREDIT</TRNTYPE>
<DTPOSTED>20211218120000</DTPOSTED>
<TRNAMT>151.75</TRNAMT>
<FITID>5ff442c1347782c556acb7fc52c477cc</FITID>
<NAME>ATMOS ENERGY 888-286-6700 TXATMO</NAME>
</STMTTRN>
```

The wrapping tags for the whole OFX are also different. The original uses `<CREDITCARDMSGSRSV1><CCSTMTTRNRS>`, while the new one uses `<BANKMSGSRSV1><STMTTRNRS>`.

So I'm not sure if the import failure is specific to the MS money patch from here or general to csv2ofx. Will happily create a new issue if it's general so as to not hijack this PR.
@nieder You'll also want to grab commit 893e6a8531ed1f2aa2f57bddb174eda08ccb08c5 if you want to produce an OFX file that MS Money is happy with.  Also, since it looks like your CSV doesn't have a column with balances in it, you'll need to specify an ending balance at the command line (with the `-B` or `--ending-balance` option) when running csv2ofx.  Without an ending balance in the OFX file, MS Money rejects the file (or at least, it does for bank statements).

This is a tool that I used to check the OFX files for errors while attempting to produce something that will satisfy MS Money: [http://moneymvps.org/faq/article/366.aspx](http://moneymvps.org/faq/article/366.aspx).  Unfortunately, the tool doesn't appear to check everything, but it was a good start.  I think the ledger-balance requirement wasn't actually checked by the tool, IIRC.

As for the incorrect transaction types, it looks like by default, csv2ofx [treats negative values as debits and positive values as credits](https://github.com/reubano/csv2ofx/blob/b00f9cbedfe4eb7c4a0ebd11737c9d81efe05dd4/csv2ofx/__init__.py#L239-L240).  If I'm interpreting things correctly, you should be able to correct both the `<TRNTYPE>` and the sign in `<TRNAMT>` by adding a "type" line to your mapping.  For example:

```python
    'type': lambda tr: 'debit' if tr.get('Amount') >= 0 '' else 'credit',
```

It looks like the `<BANKMSGSRSV1>` tags may be [hard-coded](https://github.com/reubano/csv2ofx/blob/b00f9cbedfe4eb7c4a0ebd11737c9d81efe05dd4/csv2ofx/ofx.py#L99).  If MS Money refuses to accept the OFX as a credit card statement with the `<BANKMSGSRSV1>` tags, you may need to open a new issue or PR for changing that.
Thanks. The extra commit for the balances does help in passing validation.
I was able to previously fix the debit/credit issue with this line in the mapping:
```
    "amount": lambda r: convert_amount(r["Amount"]) * -1,
```
Now, purchases show up as DEBIT, with a negative value in the OFX. This is clearly a problem with the CSV from Discover that uses  logic backwards from what Money expects, but so be it (as long as they stay consistent).

For the  `BANKMSGSRSV1` tags, etc, if I keep them as is, the OFX analyzer complains. Money takes the file but asks me to identify the account during import (which it didn't with the original files before they switched to CSV). But if I do the following changes on the OFX:
```
1) Change:
STMTTRNRS -> CCSTMTTRNRS
BANKMSGSRSV1 -> CREDITCARDMSGSRSV1
BANKACCTFROM -> CCACCTFROM
STMTRS -> CCSTMTRS
```
then Money imports fine.
The OFX analyzer also complains about these lines
```
2) BANKID in CCACCTFROM
3) ACCTTYPE in CCACCTFROM
```
But I can leave them in and Money seems happy.

Lastly, csv2ofx converted literal ampersands (`&`) to `&amp` (missing ending semicolon), and Money complained about that. If I edit them to `&amp;` everything is fine. Yay! Now to confirm the import imported everything correctly...
Just getting a chance to look this over. Can you sep it into two PRs? I'd like to avoid having library changes and new mappings in the same PR.
@reubano Thanks for considering this PR.  It looks like when split, there will still be a dependency between the two PRs.  Namely, the tests for the ending-balance code (commit e0143e9695c2bd880ade20a24973364a7d09dbe9) use the new Schwab Bank mapping.  (There is also a test in the new-mapping commit (8a4e7b40a8df79cbaafd51cce52a5d74de2f885d) that uses the new MS-Money mode, but that test could easily be split-out / moved to the new-features PR.)

Should I just create the new-mapping PR first (minus the one test that uses MS-Money mode), wait for that to merge, then create another for the other features?  Or is there a specific way you'd like me to structure the two PRs so that they could both be created at once?
@tstabrawa if you don't need any new lib features for the mapping then I suggest a PR just for the mapping. So what you said in your second to last sentence. That will be very easy to review and approve. Then new PR(s) with the features so we can discuss the problems and proposed solutions. Thanks!

Fix patch:
diff --git a/csv2ofx/mappings/schwabchecking.py b/csv2ofx/mappings/schwabchecking.py
new file mode 100644
index 0000000..495dd1b
--- /dev/null
+++ b/csv2ofx/mappings/schwabchecking.py
@@ -0,0 +1,29 @@
+from __future__ import (
+    absolute_import, division, print_function, unicode_literals)
+
+from operator import itemgetter
+
+# Schwab Bank includes three lines of text in their CSV which otherwise break
+# date parsing.  Filtering by lines with no "Type" (and faking out the date for
+# those lines) avoids the problem.
+
+# Note: Ideally, we could get account_id from the first line of the CSV.
+
+mapping = {
+    'first_row': 1,
+    'has_header': True,
+    'filter': lambda tr: tr.get('Type'),
+    'is_split': False,
+    'bank': 'Charles Schwab Bank, N.A.',
+    'bank_id': '121202211',
+    'account_id': '12345',  # Change to your actual account number if desired
+    'currency': 'USD',
+    'account': 'Charles Schwab Checking',
+    'date': lambda tr: tr.get('Date') if tr.get('Type') else "1-1-1970",
+    'check_num': itemgetter('Check #'),
+    'payee': itemgetter('Description'),
+    'desc': itemgetter('Description'),
+    'type': lambda tr: 'debit' if tr.get('Withdrawal (-)') != '' else 'credit',
+    'amount': lambda tr: tr.get('Deposit (+)') or tr.get('Withdrawal (-)'),
+    'balance': itemgetter('RunningBalance'),
+}
diff --git a/data/converted/ingesp.ofx b/data/converted/ingesp.ofx
index 7009cd1..534c10e 100644
--- a/data/converted/ingesp.ofx
+++ b/data/converted/ingesp.ofx
@@ -7,7 +7,7 @@ ENCODING:UTF-8
 				<CODE>0</CODE>
 				<SEVERITY>INFO</SEVERITY>
 			</STATUS>
-			<DTSERVER>20230108223613</DTSERVER>
+			<DTSERVER>20161031112908</DTSERVER>
 			<LANGUAGE>ENG</LANGUAGE>
 		</SONRS>
 	</SIGNONMSGSRSV1>
diff --git a/data/converted/schwab-checking.ofx b/data/converted/schwab-checking.ofx
new file mode 100644
index 0000000..b5ae9bc
--- /dev/null
+++ b/data/converted/schwab-checking.ofx
@@ -0,0 +1,71 @@
+DATA:OFXSGML
+ENCODING:UTF-8
+<OFX>
+	<SIGNONMSGSRSV1>
+		<SONRS>
+			<STATUS>
+				<CODE>0</CODE>
+				<SEVERITY>INFO</SEVERITY>
+			</STATUS>
+			<DTSERVER>20161031112908</DTSERVER>
+			<LANGUAGE>ENG</LANGUAGE>
+		</SONRS>
+	</SIGNONMSGSRSV1>
+	<BANKMSGSRSV1>
+		<STMTTRNRS>
+			<TRNUID></TRNUID>
+			<STATUS>
+				<CODE>0</CODE>
+				<SEVERITY>INFO</SEVERITY>
+			</STATUS>
+			<STMTRS>
+				<CURDEF>USD</CURDEF>
+				<BANKACCTFROM>
+					<BANKID>121202211</BANKID>
+					<ACCTID>12345</ACCTID>
+					<ACCTTYPE>CHECKING</ACCTTYPE>
+				</BANKACCTFROM>
+				<BANKTRANLIST>
+					<DTSTART>19700101</DTSTART>
+					<DTEND>20220905</DTEND>
+					<STMTTRN>
+						<TRNTYPE>CREDIT</TRNTYPE>
+						<DTPOSTED>20220817000000</DTPOSTED>
+						<TRNAMT>20.00</TRNAMT>
+						<FITID>d68ff9d88e633aaf2421a50077f848fe</FITID>
+						<CHECKNUM></CHECKNUM>
+						<NAME>Deposit Mobile Banking</NAME>
+						<MEMO>Deposit Mobile Banking</MEMO>
+					</STMTTRN>
+					<STMTTRN>
+						<TRNTYPE>DEBIT</TRNTYPE>
+						<DTPOSTED>20220814000000</DTPOSTED>
+						<TRNAMT>-103.00</TRNAMT>
+						<FITID>c38117af9e65d7f8b751956b1f1e4173</FITID>
+						<CHECKNUM></CHECKNUM>
+						<NAME>BMO HARRIS BANK</NAME>
+						<MEMO>BMO HARRIS BANK</MEMO>
+					</STMTTRN>
+					<STMTTRN>
+						<TRNTYPE>DEBIT</TRNTYPE>
+						<DTPOSTED>20220809000000</DTPOSTED>
+						<TRNAMT>-75.00</TRNAMT>
+						<FITID>558</FITID>
+						<CHECKNUM>558</CHECKNUM>
+						<NAME>Check Paid #558</NAME>
+						<MEMO>Check Paid #558</MEMO>
+					</STMTTRN>
+					<STMTTRN>
+						<TRNTYPE>DEBIT</TRNTYPE>
+						<DTPOSTED>20220804000000</DTPOSTED>
+						<TRNAMT>-57.27</TRNAMT>
+						<FITID>a4ce828223f505323e618e1976ecc466</FITID>
+						<CHECKNUM></CHECKNUM>
+						<NAME>PAYPAL INST XFER 220803~ Tran: ACHDW</NAME>
+						<MEMO>PAYPAL INST XFER 220803~ Tran: ACHDW</MEMO>
+					</STMTTRN>
+				</BANKTRANLIST>
+			</STMTRS>
+		</STMTTRNRS>
+	</BANKMSGSRSV1>
+</OFX>


Test patch:
diff --git a/data/test/schwab-checking.csv b/data/test/schwab-checking.csv
new file mode 100644
index 0000000..3e1fee4
--- /dev/null
+++ b/data/test/schwab-checking.csv
@@ -0,0 +1,8 @@
+"Transactions  for Checking account ...123 as of 08/20/2022 04:04:41 PM ET"
+"Date","Type","Check #","Description","Withdrawal (-)","Deposit (+)","RunningBalance"
+"Pending Transactions are not reflected within this sort criterion."
+"Posted Transactions"
+"08/17/2022","DEPOSIT","","Deposit Mobile Banking","","$20.00","$878.47"
+"08/14/2022","ATM","","BMO HARRIS BANK","$103.00","","$858.47"
+"08/09/2022","CHECK","558","Check Paid #558","$75.00","","$961.47"
+"08/04/2022","ACH","","PAYPAL INST XFER 220803~ Tran: ACHDW","$57.27","","$1,036.47"
diff --git a/tests/test.py b/tests/test.py
index cef45c2..715d071 100755
--- a/tests/test.py
+++ b/tests/test.py
@@ -157,6 +157,11 @@ def gen_test(raw):
             "ingesp.csv",
             "ingesp.ofx",
         ),
+        (
+            ["-o", "-m schwabchecking", "-e 20220905", SERVER_DATE],
+            "schwab-checking.csv",
+            "schwab-checking.ofx",
+        ),
     ]
 
     main(csv2ofx, gen_test(PRE_TESTS))