Correct check amount when printed from subaccount

[mabrowning]

Previously, GnuCash would include only a single split's amount as the check amount. If a "bank" account has multiple subaccounts for budgeting reasons, and a transaction involves both "internal" (e.g. from one bank subaccount to another bank subaccount) and external transfers, then even when a "subaccount" register showing the top-level bank account is printed from, the check will have the amount of the first split rather than the total of all splits which are from subaccounts.

This change communicates the parent account (when viewed from a "subaccount" register) to the check printing dialog so that the check amount matches the amount displayed in the register.

e.g.

Before:

After:

[jralls]

This change doesn't add an option to the dialog, it just totals up the splits in the account's sub accounts if the register is opened with the sub accounts. That's fine, it's intuitively the right thing to do. Please change the commit message to say so.

The check's amount when printing a Find-results dialog in the presence of sub accounts is a bit random, you might take a look at that.

A companion PR to gnucash-docs to update the manual section 6.15 would be nice.

[jralls]

The computation should be a separate function so that draw_page_item calls amount = compute_amount(pcd);.

[jralls]

Don't loop on all of pcd->account's children, use

if (pcd->account == xaccSplitGetAccount(split) ||
    pcd->account == gnc_account_get_parent(xaccSplitGetAccount(split)))
    amount = gnc_numeric_add_fixed(amount, xaccSplitGetAmount(split));

[mabrowning]

I have like 5 levels of subaccounts... won't gnc_account_get_parent return only the direct parent?

[mabrowning]

Not pointing fingers, but I copied this code from the existing ledger display logic:

gnucash/gnucash/register/ledger-core/split-register-model.c

Lines 112 to 126 in 4da655d

	if (subaccounts)
	{
	/* Add up the splits that belong to the transaction if they are
	* from the lead account or one of the subaccounts. */
	account = xaccSplitGetAccount (secondary);
	for (child = children; child; child = child->next)
	{
	if (account == child->data)
	{
	balance = gnc_numeric_add_fixed (balance, xaccSplitGetAmount (secondary));
	break;
	}
	}
	}

[jralls]

So? There's lots of crappy code in GnuCash. That's no excuse for adding more.

[jralls]

I have like 5 levels of subaccounts... won't gnc_account_get_parent return only the direct parent?

Yup. But gnc_account_get_children also goes only one level. You'd need to use gnc_account_get_descendants or recurse on gnc_account_get_children, which is what get_descendants does under the hood. Either gets ugly so you might consider gnc_account_foreach_descendant with a test-and-accumulate function that checks each account in turn for splits in the transaction rather than the other way around. That way you're doing the more expensive loop only once instead of once per split.

[mabrowning]

Sorry, not being defensive! I'm happy to use whatever best practices you'd prefer. I had simply found some existing code that did what I needed and copied it. However, both do use gnc_account_get_descendants to get all children accounts recursively into a list...

you might consider gnc_account_foreach_descendant with a test-and-accumulate function that checks each account in turn for splits in the transaction rather than the other way around. That way you're doing the more expensive loop only once instead of once per split.

Done. Found a few other helpers so all manual loops are removed.

[mabrowning]

This change doesn't add an option to the dialog... Please change the commit message to say so.

Done. No more mention of options.

The check's amount when printing a Find-results dialog in the presence of sub accounts is a bit random, you might take a look at that.

I had never personally printed a check from Find results, but yeah it warns about printing a check with splits from multiple accounts, but then seems to print the amount of the first split in the transaction. I'm not sure how best to identify the appropriate "parent" account in this case.

For awhile I had a local change that found the first account underneath the account named Assets and used that as the parent account for all check printing, which worked from any register including "Find" results. However, that didn't seem viable to upstream, so the LD_SUBACCOUNT register hook seemed like an reasonable compromise.

I did consider adding an option to each account indicating if it is "check bearing" or something, and there is the "Account Type" with experimental support for "Checking Account" in the codebase, but that seemed like a wider change I wasn't ready to sign up for.

[jralls]

Much nicer. Thanks!