Add 1 to wishlist for 4.0 Dear Devs I'd like to suggest a small upgrade to the 4.x series onwards. Currently setting txn's closing-txn flag is done via C, but querying accounts' balances with/without closing-txn flag is handled mainly via scheme (see xaccQueryAddClosingTransMatch in report-utilities.scm). Proposal: I suggest we upgrade xaccAccountRecomputeBalance to, in addition to cleared/reconciled/unreconciled balances, also accumulate accounts' balances ignoring closing transactions by querying xaccTransGetIsClosingTxn(split->parent). Also add appropriate xaccAccountGetBalanceAsOfDateIgnoringClosing function and maybe a couple more. Benefits: - fixing bug 797326 becomes straightforward - scheme code to query closing should be faster; will not need separate qof_query to calculate balances ignoring closing Caveat: - the scheme code also handles the (very) old closing-txns whereby they were not originally tagged with trans's trans_is_closing_str kvp flag; the proposal above will ignore any of these old closing-txns. The only way to identify them is via description full-text or regex search "Closing Entries" as is currently done via income-type reports. The proposal will remove this search facility. Alternatively if this mechanism must be retained, the closing-entries search parameters would need to be saved as a book kvp property instead, to be queried by xaccAccountRecomputeBalance as needed. Roadmap: - I can modify the C code as above; but not confident to code the book-kvp backward-compatibility string/regex search. - I can modify scheme to make use of the new API - I can also then fix 797326 to use new API
The original commit to insert txn's closing-txn slot seems to be https://github.com/Gnucash/gnucash/commit/3f70c990026655af94eab7f354716e613a76fe4b
^ Actually the original trans->kvp slot inserted in the following commit. So any closing txn older than 2008 would be at risk of getting ignored. https://github.com/Gnucash/gnucash/commit/4b28001459d93e078a1dd729d41302eac049ab0c
P.S. I don't think the above would severely impact performance because cstim's commit https://github.com/Gnucash/gnucash/commit/75b6d14455db5c96711becef4719e26697bd7f21#diff-9b3d4cb626af230a273feed97e299ecc ensures the closing kvp is actually cached.
A reasonable proposal. To avoid the pre-2008 closing transaction issues, perhaps we can add a scrub function ("Check & Repair") that sets the trans_is_closing_str flag for pre 2008 closing transactions. If users experience issues with the new code they can run Check & Repair once to have it fixed.
OK. Where did you find a regex match for pre-2008 closing transactions? get-trans-type-splits-interval in report-utils.scm can match a supplied regex as well as filtering for closing transactions, but it uses xaccQueryAddClosingMatch, not a regex. In fact the closing transaction description is user-supplied (dialog-book-close.c line 286) so I don't see how such a check would occur. One could pretty accurately detect a closing split: If the split reduces the balance to 0 and the other account is of type equity then I can't think of what it could be other than a closing split. The extra balance is needed only for types income and expense, no need to spend the time checking on other account types. The isClosingTxn_cached commit is https://github.com/Gnucash/gnucash/commit/eb9e45bc20531f936881d2433053716473b37828.
See gnc:account-get-trans-type-splits-interval: ... (let (... (matchstr (get-val 'str)) (case-sens (get-val 'cased)) (regexp (get-val 'regexp)) ... (xaccQueryAddDescriptionMatch query2 matchstr case-sens regexp QOF-COMPARE-CONTAINS QOF-QUERY-OR) ... which sends regexp (#t/#f) to xaccQueryAddStringMatch use_regexp argument. Can be demonstrated by running income-statement. --- Re: heuristics -- I'm not against, but has to be documented properly.
Ah, typical scheme code, buried under 3 layers of indirection to make it difficult to reason about. gnc:account-get-trans-type-splits-interval can accept arbitrary strings to match on and they can be treated either as literal matches or regexps. They don't necessarily have anything to do with closing transactions. html-acct-table.scm has a type for use with gnc:account-get-trans-type-splits-interval called closing-pattern. It just does a translated string match on "Closing Entries" in addition to checking xaccTransGetIsClosingTxn. The balsheet-pnl, equity statement, income statement, and trial balance reports all have options for letting the user set the match string and regex (search "closing-pattern" to find the details). That's more likely to be successful than the html-acct-table.scm approach since the string it's matching on is user defined. There's similar options for defining adjusting entries using the same gnc:account-get-trans-type-splits-interval, but of course there's no slot for adjustments. Your PR 576 doesn't do anything about the reports yet. You might want to leave it that way.
> Your PR 576 doesn't do anything about the reports yet. You might > want to leave it that way. Agree -- I can query closing-txn much more efficiently than using the new API in PR 576 because the new-API will always scan split-list from the start. I don't like html-acct-table.scm -- I have a loose long-term plan to replace its use with the add-multicolumn-acct-table from balsheet-pnl.scm instead, which IMHO is cleaner and more capable. But this may take years.
fixed in maint & master. added minimal changes to fix bug 797326
