[GNC-dev] New balsheet (and P&L report), API considerations, and (slow?) KVP in Account.cpp

[John Ralls]

Chris,

OK, I think that’s a good choice.

As a general design principle I’d like to see the reports use QofQuery as much as possible--and extend QofQuery as necessary--because that makes it easier to reimplement as database queries.

Regards,
John Ralls


> On Jun 24, 2018, at 2:50 AM, Christopher Lam <christopher.lck at gmail.com> wrote:
> 
> I think I'll forego a noclosing_balance upgrade in C.
> 
> According to https://bugzilla.gnome.org/show_bug.cgi?id=570042 <https://bugzilla.gnome.org/show_bug.cgi?id=570042> the "Closing Entries" transactions did not always receive a special KVP-based flag until the commit onhttps://github.com/Gnucash/gnucash/commit/4b2800145 <https://github.com/Gnucash/gnucash/commit/4b2800145> on datafiles generated from 27-11-2008 and later.
> So, to find and exclude these closing entries, we can use the following strategies:
> xaccTransGetIsClosingTxn, will query the KVP flag
> Run a QofQuery and add xaccQueryAddClosingTransMatch (also queries the KVP flag)
> Free-text search "Closing Entries" to seek these old closing entries
> So a P&L report will necessarily need to do both filtering for KVP and free-text search.
> 
> The multicolumn report will be different enough from the old balance-sheet and income-statement that I think I should spin it off into a different report altogether, and the others will be sunsetted. Hopefully this new report will be broadly acceptable because the old reports have a *lot* of supporting unintelligible old code, especially to handle closing-entries as above.
> 
> C
> 
> On 24/06/18 00:51, Christopher Lam wrote:
>> Hi John, the split->noclosing_balance is updated in xaccAccountRecomputeBalance. Will continue copypasta coding until it works!
>> 
>> On Sat, 23 Jun 2018, 23:56 John Ralls <jralls at ceridwen.fremont.ca.us <mailto:jralls at ceridwen.fremont.ca.us>> wrote:
>> 
>> 
>> > On Jun 22, 2018, at 9:42 PM, Christopher Lam <christopher.lck at gmail.com <mailto:christopher.lck at gmail.com>> wrote:
>> > 
>> > Hi All
>> > 
>> > I'm working through balance-sheet.scm and overhauling this report.
>> > 
>> > At the same time, I can see that balance-sheet.scm and income-statement.scm can be merged together.
>> > 
>> > After all
>> > 
>> > * balance sheet = asset/liability/equity balance at date X,
>> > * income statement = difference in income/expense balances at date X and Y
>> > 
>> > The issue I have is that the balance sheet can call xaccAccountGetBalanceAsOfDate(acc,date) directly to nicely retrieve the balance, whereas income statement cannot directly call it because it also includes closing transactions.
>> > 
>> > My proposal is to augment xaccAccountGetBalanceAsOfDate to accept a 3rd boolean argument i.e. xaccAccountGetBalanceAsOfDate(acc,date,ignoreclosing) which will selectively produce the balance, ignore closing transactions.
>> > 
>> > This is partly done in the last commit of https://github.com/christopherlam/gnucash/tree/maint-create-API-call-for-noclosing-balances <https://github.com/christopherlam/gnucash/tree/maint-create-API-call-for-noclosing-balances> - it will augment API everywhere, and also modify Account.cpp, especially xaccAccountRecomputeBalance which will call xaccTransGetIsClosingTxn(xaccSplitGetParent(split)) for each split in the account to determine closing status and cache the balances for each split. See draft on https://github.com/christopherlam/gnucash/tree/maint-create-API-call-for-noclosing-balances <https://github.com/christopherlam/gnucash/tree/maint-create-API-call-for-noclosing-balances> - this currently fails because I'm not good at C.
>> > 
>> > *My query is whether it will be too expensive to call xaccTransGetIsClosingTxn for each split in Account.cpp to produce the split->noclosing_balance, which will be crucial to calculate income between two dates.*
>> > 
>> > *Currently this 'closing-txn' filter is done in income-statement.scm via a "Closing Entries" string/regex filter and xaccTransGetIsClosingTxn calls, which IMHO is not ideal either.
>> > *
>> > 
>> > Any help welcome!
>> > 
>> > P.S. if this last commit is removed, the https://github.com/christopherlam/gnucash/tree/maint-create-API-call-for-noclosing-balances <https://github.com/christopherlam/gnucash/tree/maint-create-API-call-for-noclosing-balances> branch contains work-so-far for updated balance-sheet. Screenshot https://screenshots.firefox.com/eelmGQrGCtcP6bqC/null <https://screenshots.firefox.com/eelmGQrGCtcP6bqC/null> - see the multilevel subtotals were previously horizonal, are now vertical; original-currency amounts, and multiple-date balance sheets.
>> > 
>> > _
>> 
>> Chris,
>> 
>> Only profiling will tell how much of a performance hit this creates.
>> 
>> I don’t see where in your commit you’re computing the split’s no-closing balance. Perhaps that’s why it doesn’t work?
>> 
>> Regards,
>> John Ralls
>