[GNC-dev] Online ID matching for OFX import
jralls at ceridwen.us
Sun Jan 5 23:27:02 EST 2020
> On Jan 5, 2020, at 7:44 PM, Mike Alexander <mta at umich.edu> wrote:
> On 5 Jan 2020, at 19:46, John Ralls wrote:
> Complexity is not a valid reason to reject a solution to a complex problem unless there's a simpler solution that solves it *just as well*. Hackish solutions--and I'm just as guilty on that front as I wrote the strncmp hack--are far more likely to trip us up again and again every time someone else pops up with another use case. Piling up hacks creates technical debt: we're already going through the account list twice because of an another hack for a libofx peculiarity. We need to work out a correct general solution that doesn't pile up more technical debt. We have quite enough of that already.
> Whether the user can do something about it or not it saves us work on a support email or bug report later if we at least log that we couldn't match because the online account number has multiple matches.
> You're right about the list, the allocations would be expensive and pointless since we can't do anything with it. All we need is an Account* for the first partial match and either an int to count the partials (for the error message) or a gboolean to record that we had more than one partial.
> We can promote Daniel Ditman's create_online_id to import-utilities and add stripping the space there then use it everywhere we're creating an online id and avoid that pass as well. I've started on this already, I'll have a PR tomorrow sometime.
> So what do you want me to do? It seems to me that a simple 90% solution would be for gnc_import_select_account to refuse to return an account whose commodity didn't match the one given to it (if any). The case that led me to this idea was where there is an account with a partial match in the wrong commodity but no account with a full match. If you do the full match if possible and partial match if not solution it becomes hard to get it to create an account for the full match if a partial match account already exists. This will happen in my books if I add a new security to my TIAA/CREF accounts. The parent account will be a partial match for the wrong commodity, but there won't be a full match and I want it to let me create or select one.
> I can either change gnc_import_select_account to never return an account that doesn't match the commodity it's given or add a parameter to it telling it whether to make this test. I looked at the calls to it and I don't think making this test unconditionally will cause problems. Many of the calls don't pass a commodity so it won't matter anyway. For that matter, if we compare the commodity it might be ok to just do the partial match and not look for a full match instead. The description of gnc_import_select_account says that it warns if the found account's commodity doesn't match, but this is not correct. It only uses that commodity when creating a new account.
> Just punting and returning the first match, partial or full, isn't really an option since it will make it difficult to import my OFX files. Worse, when I do select an account it will use the commodity's CUSIP as the online ID and this is not unique since the same commodity can (and does) exist in multiple parent accounts. This will lead to random matches to the wrong account on future imports.
> The bottom line is that I think we should have it return the first account with the correct commodity and a partial match to the online ID>
I want you to code review and test the PR when I finish. ;-)
The online_id comes from from the import source, AQBanking in this particular case--and we must be more careful because the code in question applies to all of the importers except QIF--and the import source doesn't generally contain any commodity information, so basing the match on that is a non-starter.
Returning the first match is what we do now. I get that that was the wrong approach.
As for GnuCash creating a new account when you've got a new TIAA security, does that actually work? For that matter, are you able to connect to your TIAA master account and import transactions for all of the securities you have, all in one go? I thought that GnuCash didn't support that, never mind creating a new security account if it fails to find a match. Mind, this isn't the same as the matcher letting you create a new account when you double-click on a yellow line, this is during the matcher setup when it decides what asset/liability account the transactions belong to.
More information about the gnucash-devel