redstreet / beancount_reds_importers

Simple ingesting tools for Beancount (plain text, double entry accounting software). More importantly, a framework to allow you to easily write your own importers.
GNU General Public License v3.0
115 stars 39 forks source link

Schwab Transfers Are Zero #59

Closed doughepi closed 1 year ago

doughepi commented 1 year ago

Using the Schwab Brokerage CSV importer with the following entry seemingly generates a $0 transaction. Seems like a bug.

"Date","Action","Symbol","Description","Quantity","Price","Fees & Comm","Amount"
"01/09/2023","MoneyLink Deposit","","My Name","","","","$25.00"

Results in

2022-12-27 * "transfer" "transfer"
  Assets:Long-Term-Investments:Charles-Schwab:General-Investing:USD  0 USD

Based on my config

CONFIG = [
    schwab_csv_brokerage.Importer(
        {
            "main_account": "Assets:Long-Term-Investments:Charles-Schwab:General-Investing:{ticker}",
            "cash_account": "Assets:Long-Term-Investments:Charles-Schwab:General-Investing:{currency}",
            "account_number": "XXX",
            "dividends": "Income:Dividends:Schwab:General-Investing:{ticker}",
            "interest": "Income:Interest:Schwab:General-Investing:{ticker}",
            "cg": "Income:Capital-Gains:Schwab:General-Investing:{ticker}",
            "capgainsd_lt": "Income:Capital-Gains-Distributions:Long:Schwab:General-Investing:{ticker}",
            "capgainsd_st": "Income:Capital-Gains-Distributions:Short:Schwab:General-Investing:{ticker}",
            "fees": "Expenses:Brokerage-Fees:Schwab",
            'rounding_error' : 'Equity:Rounding-Errors',
            "currency": "USD",
            "fund_info": fund_info,
        }
    ),
]

I think this has to do with https://github.com/redstreet/beancount_reds_importers/blob/main/beancount_reds_importers/libtransactionbuilder/investments.py#LL271C17-L277C17 which treats Schwab MoneyLink Deposit and MoneyLink Transfer actions as reds_importer transfers, but they lack a unit. It should be using the total in this case.

Let me know! I'm not sure what the best fix would be, as I'm new to this library. I also noticed in the code that Journal actions are ignored. Should MoneyLink actions be ignored too? They're also just plain account deposits and withdrawals, just like Journal.

redstreet commented 1 year ago

Thanks for filing a detailed report!

I haven't personally used this importer in a while and thus can't run it quickly to give you a response, but I do see that I used "MoneyLink Transfer" in the past successfully. Something probably changed though, because your reasoning seems correct.

The root of the problem here is there are no unit tests for this importer. Which makes it vulnerable to these types of bugs. I'm happy to add that when I get a chance (might take a while). Once that's done, it should be easy to fix this issue while ensuring other functionality is not lost. I imagine the fix would be add a line similar to this line to grab the Amount column and put it into units for the MoneyLink.* rows.

Journal is ignored because from my history, they were all simply duplicates of other transactions. Let me know if you're seeing something else.

doughepi commented 1 year ago

Thank you for your response!

I must say, I find this library to be the best implementation of a Schwab CSV importer that I've come across. We can definitely work through these few bugs together 😄

I'd be more than happy to attempt to address this issue for my own use case! I'll submit a PR when I have the chance. It may take me a little while to familiarize myself with the source code. If you or someone else manages to fix it before I do, please feel free to let me know!

I can also take care of #60 in the same release.

edit: I'll also try my hand at some test cases! However, I find that Schwab has a billion-and-a-half actions for their brokerage entries, so my test cases might not be as comprehensive as they could be.

redstreet commented 1 year ago

Awesome---glad you find it helpful :).

I'll see if I can add the unit testing here in the next few days that'll help. If you are able to submit a few anonymized test csv rows for various transaction types just like you have at the top of this ticket, that'll help too.

doughepi commented 1 year ago

Sure! Here's some of each of the different types of actions that I've seen in my files.

"Transactions  for account General Investing ...XXX as of 05/03/2023 02:21:05 PM ET"
"Date","Action","Symbol","Description","Quantity","Price","Fees & Comm","Amount"
"04/27/2023","Buy","BND","VANGUARD TOTAL BOND MARKET ETF","45","$73.7789","","-$3320.05"
"04/27/2023","Journal","","TRANSFER FUNDS FROM SCHWAB BANK - XXX","","","","$7461.72"
"04/20/2023","Journal","","TRANSFER FUNDS TO SCHWAB BANK - XXX","","","","-$7461.72"
"04/18/2023","Sell","BND","VANGUARD TOTAL BOND MARKET ETF","10.065","$73.4049","$0.01","$738.81"
"04/17/2023 as of 04/15/2023","Bank Interest","","BANK INT 031623-041523 SCHWAB BANK","","","","$0.03"
"04/10/2023","Reinvest Shares","BND","VANGUARD TOTAL BOND MARKET ETF","0.0249","$73.8993","","-$1.84"
"04/06/2023","Reinvest Dividend","BND","VANGUARD TOTAL BOND MARKET ETF","","","","$1.84"
"02/01/2023","Qualified Dividend","GIS","GENERAL MILLS INC","","","","$0.54"
"01/17/2023","Cash Dividend","SWVXX","SCHWAB VALUE ADVANTAGE MONEY INV","","","","$0.98"
"01/09/2023","MoneyLink Deposit","","John Smith","","","","$25.00"
"12/15/2022","MoneyLink Transfer","","Tfr JPMORGAN CHASE BAN, NOT AVAILABLE","","","","$980.65"
"Transactions Total","","","","","","","-$1,574.04",
redstreet commented 1 year ago

I added unit tests (thanks for test csv!), and your case reproduces. I'll fix this soon.

redstreet commented 1 year ago

Fixed. Thanks again for the detailed bug report and the test cases!

You might also check out 2cbac8d.

doughepi commented 1 year ago

Fantastic! Thanks so much @redstreet! Your hard work is very appreciated :)

redstreet commented 1 year ago

Happy to help: :).

Whether or not Journal rows must be included is unresolved. Feel free to open a new ticket if you find they must be included.