[PATCH] Repair automatch_store_transactions

[Simon Ruggier]

On Sun, Feb 8, 2009 at 7:40 PM, David Reiser <dbreiser at earthlink.net> wrote:
>
> On Feb 8, 2009, at 6:07 PM, Simon Ruggier wrote:
>
>> On Sun, Feb 8, 2009 at 3:51 PM, David Reiser <dbreiser at earthlink.net>
>> wrote:
>>>
>>> This patch has some nasty side effects. It tries to change assigned
>>> target
>>> accounts on the fly within a single import. If I have several ATM
>>> deposits
>>> in one import, most of them will come up assigned to the last assignment
>>> from a prior import. When I change the first one to its proper account,
>>> your
>>> patch results in the rest of the deposits also being assigned to the
>>> account
>>> just entered. That's not so good, but not the end of the world (and
>>> definitely not what gnucash has done in the past).
>>
>> So basically, before, you'd have to be doing a new import for the
>> automatching to happen,
>
> That isn't what happens on my system, but my data file has been accumulating
> for so long, I'm not sure what was already there when the switch to the
> GTKTreeModel happened.

What happens on your system?

Also, note that I started using GnuCash after the switch happened. I'm
probably going to make a patch for the two things I mentioned before
whenever I get a chance in the next few weeks, I don't think it's
right that the patch I submitted breaks the workflow where the user
moves from top to bottom, setting destination accounts and verifying
the automatches for correctness at the same time. I also won't think
it's right if I make it really easy for the user to game their match
data by toggling the add checkbox repeatedly, so I'd have to implement
both changes.

One could make the loop only trigger automatches for transactions
after the one that was just set, but that's a bad idea, because it
assumes that the user is moving downwards only, and the user doesn't
know that we're assuming that, so they could get bitten if they jump
back up to the top of the dialog and change something without
rechecking all the automatches below it (of course, with this patch,
they're getting bitten with the loop now no matter what direction they
go in, for now).

>> Are you sure that the first deposit gets reassigned in the case where
>> it is a manual deposit? The matching code clearly attempts to avoid
>> matching transactions that were manually set (in import-backend.c at
>> gnc_import_TransInfo_refresh_destacc).
>
> Yes, I'm sure. I watched it happen in an import.

I'm at a loss to explain that, since the same flag that is used to
choose whether to put (manual) or (auto) next to the text in the
destination column is used in the decision to skip automatching a
given import transaction. As far as I know, the flag is only set to
false (i.e. auto) when the transaction is first added to the list, and
the loop in automatch_store_transactions doesn't change any
transactions where the flag is true. If the list shows "transfer to
(manual)" next to the destination account name, the automatching stuff
shouldn't be changing that transaction.

However, if you have a transaction that the match data isn't really
familiar with, and you select an account for it manually in the import
dialog, then hit cancel, and then reimport the same file, the import
code will (most likely) match the same transaction to the destination
account, but it will show (auto) instead of (manual). Could that be
what happened?