Created attachment 373480 [details] Sample file Step 1: invoice customer $100 and create an AR for Invoice #1. Step 2: customer pays $300, apply it to the Invoice #1, there is a net credit to the customer of $200. Screenshot of customer report (beta): https://i.imgur.com/d91u5oX.png Step 3: look at receivable aging data, instead of showing an aged prepayment of the $200 credit, it shows $400, which seems to be [$-300] + [$100 * (-1)] = [$-400]. In other words, the sign got reversed on the Invoice #1, and got applied as a prepayment. Screenshot: https://i.imgur.com/SpVy9hK.png This seems a related-ish issue: https://bugs.gnucash.org/show_bug.cgi?id=797468
Thank you for reports. Will check later. I'd hope that you used a very recent nightly; these things were still being debugged 2 weeks ago; pesky bugs were appearing everywhere.
The nightly used was whatever was available for download on 2019-12-08, so fairly recent!
As expected, the suggested fix is a fairly surprising one. Please try fixing new-aging.scm (somewhere in Program Files (x86)\gnucash). Reasoning: The APAR splits are analysed, partitioned into accounts. In cases whereby 1 payment overpays the invoice, it creates a multisplit txn with 1 transfer split and 2 APAR splits - one to fulfil the invoice lot, the other one to set up the overpayment. The suggested change will ensure each payment is processed only once. into gnc:owner-splits->aging-list As usual further stress testing of the new reports would be very appreciated. For further tests: - 1 invoice -> 1 payment - 1 invoice -> 1 overpayment - 1 invoice -> 1 underpayment and subsequent payment - 1 invoice -> 1 underpayment and subsequent overpayment - 2 invoice -> 1 payment for both - ditto credit notes - ditto vendor bill modified gnucash/report/business-reports/new-aging.scm @@ -235,7 +235,7 @@ exist but have no suitable transactions.")) (else (setup-query query accounts report-date) - (let* ((splits (qof-query-run query)) + (let* ((splits (xaccQueryGetSplitsUniqueTrans query)) (accounts (sort-and-delete-duplicates (map xaccSplitGetAccount splits) gnc:account-path-less-p equal?))) (qof-query-destroy query)
NB I'm not 100% sure it's bullet proof yet, hence more testing would be useful... and if another bug found please attach datafile.
Ok reports are still not known 100% bulletproof but have pushed this fix (and another couple). The number of bugs in new-business-reports is asymptotically approaching zero.
I just built the current maint version (3eef88410) and still have some problems. It's an admittedly strange situation. The local power company completely messed up the billing for one of the accounts I deal with. It issued 4 bills for 4 months (which I put in the books) then deleted those 4 bills and issued 2 more for 2 months each covering the same 4 months. The bills are paid via ACH withdrawal from the bank account and the power company seemed to make random withdrawals at various times for 5 or 6 months. I think everything ended up even, but in the process of fixing this I created some very strange transactions. Your new report was very valuable in trying to sort this out, even if it's not yet perfect. It would have taken even more staring at the lot viewer without it. There is a payment transaction that has two splits linked to the same bill. This doesn't work right in the report. With detailed links it links the bill to the payment and lists the total amount of the two splits (which is ok). The payment is linked to the bill twice and each line shows the total amount (which is not ok). There are also some bills where the payment is linked to the bill but the bill is not linked to the payment. I think this involves prepayments that were later connected to the bill. I can send you the file that causes this if you want. It's for my condo association and is not particularly secret. However I don't want to post it here, I'd rather send it privately. Recreating this would be a lot of trouble, it took hours to resolve this mess.
@mta go ahead and send, I'll have a look at your datafile. CDB-Man knows I prefer screenshots, and even better if I can generate them myself :) The key is we need to decide beforehand what *should* the report show in these unusual txns. warlord did warn that users will keep pushing the boundaries. :-O
Created attachment 373482 [details] Faulty report I've attached the HTML for the report that is incorrect, is that good enough? There are two bills that are wrong IMO. Bill 674 shows two payments of $77.13. The associated payment shows twice in the report, once with an amount of $77.13 and once with blanks for the payment amount. This payment transaction has two splits in the lot for bill 674, one for $42.04 and one for %35.09. Neither of these amounts appears in the report. There is a third payment for this bill, %44.20 on October 11. This payment appears in the report and ls linked to the bill, but the bill is not linked to the payment. Bill 673 also has the second of these two problems. It has two payments: $45.92 on Aug. 25 and $22.25 on Sept. 24. Both of these are linked to the bill, but the bill only links to the $22.25 payment. Both of these payments are normal two split payments.
Try this patch on current maint 7cd6591c7. Instead of scanning transactions, we scan individual splits. It reports the multiple APAR splits separately. One step closer to ideal report output? ----- diff --git a/gnucash/report/business-reports/new-aging.scm b/gnucash/report/business-reports/new-aging.scm index a9bb0a045..5f0f8ba5a 100644 --- a/gnucash/report/business-reports/new-aging.scm +++ b/gnucash/report/business-reports/new-aging.scm @@ -235,7 +235,7 @@ exist but have no suitable transactions.")) (else (setup-query query accounts report-date) - (let* ((splits (xaccQueryGetSplitsUniqueTrans query)) + (let* ((splits (qof-query-run query)) (accounts (sort-and-delete-duplicates (map xaccSplitGetAccount splits) gnc:account-path-less-p equal?))) (qof-query-destroy query) diff --git a/gnucash/report/business-reports/new-owner-report.scm b/gnucash/report/business-reports/new-owner-report.scm index 0379f5ec6..5ab0fb865 100644 --- a/gnucash/report/business-reports/new-owner-report.scm +++ b/gnucash/report/business-reports/new-owner-report.scm @@ -480,7 +480,7 @@ (let* ((split (car splits)) (txn (xaccSplitGetParent split)) (date (xaccTransGetDate txn)) - (value (AP-negate (xaccTransGetAccountAmount txn acc))) + (value (AP-negate (xaccSplitGetAmount split))) (invoice (gncInvoiceGetInvoiceFromTxn txn))) (cond @@ -737,7 +737,7 @@ invoices and amounts."))))) (else (setup-query query owner accounts end-date (eqv? GNC-OWNER-JOB type)) - (let ((splits (xaccQueryGetSplitsUniqueTrans query))) + (let ((splits (qof-query-run query))) (qof-query-destroy query) (gnc:html-document-set-headline! diff --git a/gnucash/report/report-system/report-utilities.scm b/gnucash/report/report-system/report-utilities.scm index 5cb4d44e7..8510da8eb 100644 --- a/gnucash/report/report-system/report-utilities.scm +++ b/gnucash/report/report-system/report-utilities.scm @@ -1165,20 +1165,11 @@ flawed. see report-utilities.scm. please update reports.") (loop (1+ idx) (cdr bucket-dates)))) (lp (cdr splits)))) - ;; next split is a payment. analyse its sister APAR splits. any - ;; split whose lot has no invoice is an overpayment. - ((eqv? (xaccTransGetTxnType (xaccSplitGetParent (car splits))) - TXN-TYPE-PAYMENT) - (let* ((txn (xaccSplitGetParent (car splits))) - (splitlist (xaccTransGetAPARAcctSplitList txn #f)) - (payment (apply + (map xaccSplitGetAmount splitlist))) - (overpayment - (fold - (lambda (a b) - (if (null? (gncInvoiceGetInvoiceFromLot (xaccSplitGetLot a))) - (- b (xaccSplitGetAmount a)) - b)) - 0 splitlist))) + ;; next split is a prepayment. its lot has no invoice. + ((and (eqv? (xaccTransGetTxnType (xaccSplitGetParent (car splits))) + TXN-TYPE-PAYMENT) + (null? (gncInvoiceGetInvoiceFromLot (xaccSplitGetLot (car splits))))) + (let ((overpayment (xaccSplitGetAmount (car splits)))) (gnc:msg "next " (gnc:strify (car splits)) " payment " payment " overpayment " overpayment) (addbucket! (1- num-buckets) (if receivable? (- overpayment) overpayment)) -----
The second issue whereby the payment precedes the invoice/bill date, the gnc_lot_get_earliest_split actually returns the undesired split: it means to return the 'posting split' ie the split that opens the lot, but it returns the split that prepays the lot. I will remove gnc_lot_get_earliest_split use. So the above is a documentation bug for gnc_lot_get_earliest_split.
A more comprehensive patch. 1) process splits rather than txns. 2) don't use gnc-lot-get-earliest-split -- will fail. @mta Please try and let me know if it produces satisfactory output. I'd think it now does. modified gnucash/report/business-reports/new-aging.scm @@ -235,7 +235,7 @@ exist but have no suitable transactions.")) (else (setup-query query accounts report-date) - (let* ((splits (xaccQueryGetSplitsUniqueTrans query)) + (let* ((splits (qof-query-run query)) (accounts (sort-and-delete-duplicates (map xaccSplitGetAccount splits) gnc:account-path-less-p equal?))) (qof-query-destroy query) modified gnucash/report/business-reports/new-owner-report.scm @@ -309,8 +309,7 @@ (define (make-invoice->payments-table invoice) (define lot (gncInvoiceGetPostedLot invoice)) - (let lp ((invoice-splits (delete (gnc-lot-get-earliest-split lot) - (gnc-lot-get-split-list lot))) + (let lp ((invoice-splits (gnc-lot-get-split-list lot)) (result '())) (match invoice-splits ;; finished. test for underpayment and add outstanding balance @@ -480,7 +479,7 @@ (let* ((split (car splits)) (txn (xaccSplitGetParent split)) (date (xaccTransGetDate txn)) - (value (AP-negate (xaccTransGetAccountAmount txn acc))) + (value (AP-negate (xaccSplitGetAmount split))) (invoice (gncInvoiceGetInvoiceFromTxn txn))) (cond @@ -737,7 +736,7 @@ invoices and amounts."))))) (else (setup-query query owner accounts end-date (eqv? GNC-OWNER-JOB type)) - (let ((splits (xaccQueryGetSplitsUniqueTrans query))) + (let ((splits (qof-query-run query))) (qof-query-destroy query) (gnc:html-document-set-headline! modified gnucash/report/report-system/report-utilities.scm @@ -1165,20 +1165,11 @@ flawed. see report-utilities.scm. please update reports.") (loop (1+ idx) (cdr bucket-dates)))) (lp (cdr splits)))) - ;; next split is a payment. analyse its sister APAR splits. any - ;; split whose lot has no invoice is an overpayment. - ((eqv? (xaccTransGetTxnType (xaccSplitGetParent (car splits))) - TXN-TYPE-PAYMENT) - (let* ((txn (xaccSplitGetParent (car splits))) - (splitlist (xaccTransGetAPARAcctSplitList txn #f)) - (payment (apply + (map xaccSplitGetAmount splitlist))) - (overpayment - (fold - (lambda (a b) - (if (null? (gncInvoiceGetInvoiceFromLot (xaccSplitGetLot a))) - (- b (xaccSplitGetAmount a)) - b)) - 0 splitlist))) + ;; next split is a prepayment. its lot has no invoice. + ((and (eqv? (xaccTransGetTxnType (xaccSplitGetParent (car splits))) + TXN-TYPE-PAYMENT) + (null? (gncInvoiceGetInvoiceFromLot (xaccSplitGetLot (car splits))))) + (let ((overpayment (xaccSplitGetAmount (car splits)))) (gnc:msg "next " (gnc:strify (car splits)) " payment " payment " overpayment " overpayment) (addbucket! (1- num-buckets) (if receivable? (- overpayment) overpayment))
... and another patch to prevent duplicate invoices in payment->invoices links modified gnucash/report/business-reports/new-owner-report.scm @@ -359,7 +359,9 @@ invoices) (lp rest overpayment - (cons invoice invoices)))))))) + (if (member invoice invoices) + invoices + (cons invoice invoices))))))))) (define (make-payment->invoices-list txn) (list
Created attachment 373483 [details] output after above two patches
P.S. I'm still not 100% sure it's better to show 1 APAR split per row in this report. After all, the customer/vendor report shows activity from 'our' side, and if we create a payment to fulfill multiple invoices/bills, is it really important to show all invoices/bills in individual rows? In your book, on 3-December a $77.13 payment was processed to fulfill the same $121.33 bill with amounts $42.04 and $35.09 -- I'd think it'd be best to show $77.13 (same as the bank statement) rather than $42.04 and $35.09 (from the vendor statements).
For the record, the offending business txns are: - bill 673 for $68.17 paid by $45.92 on 25/08/19, and $22.25 on 24/09/19 - bill 674 for 121.33 paid by $44.20 on 11/10/10, and $77.13 on 03/12/19 -- the latter bank withdrawal is documented by the vendor as $42.04 and $35.09 Bill 673 was prepaid $22.25 and this prepayment was removed by deleting the gnc_lot_get_earliest_split and is easily fixed. Bill 674 needs to show the corresponding payments. My view is that the $77.13 appears in *my* bank statement and is what I want to see as the partial payment when auditing bill payments. Whether the vendor classes it as 2 partial amounts is irrelevant to me. Even better, if the bill was settled from a foreign currency I think I'll want to see the foreign currency amount instead of the partial payment. Back to your book, I'm still not sure why the 03/12/19 was recorded as 3-split with 2 splits into APAR account if both APAR splits pay the same bill, and are in the same lot. If you combine both splits, the report will be all fine. (In reply to Mike Alexander from comment #6) > There is a payment transaction that has two splits linked to the same bill. > This doesn't work right in the report. With detailed links it links the > bill to the payment and lists the total amount of the two splits (which is > ok). > The payment is linked to the bill twice and each line shows the total > amount (which is not ok). This was a design decision in commit 0b8bfe66. I felt a payment needed to show the full invoice amount. After all for a $100 payment partially covering a $250 invoice, what is the benefit of showing $100 in the linked bill/invoice amount? It simply duplicates the left-hand amount of $100. > > There are also some bills where the payment is linked to the bill but the > bill is not linked to the payment. I think this involves prepayments that > were later connected to the bill. > > I can send you the file that causes this if you want. It's for my condo > association and is not particularly secret. However I don't want to post it > here, I'd rather send it privately. Recreating this would be a lot of > trouble, it took hours to resolve this mess.
Created attachment 373484 [details] patch APAR txns->splits This is the isolated patch to process APAR splits instead of APAR txns, to be applied onto 924fee2f3 maint. I am not convinced this is the right approach though as described in https://bugs.gnucash.org/show_bug.cgi?id=797521#c14
I applied the patches in comments 11 and 12 and seem to get the same output as the attachment in comment 13. This is a definite improvement, but I still have a couple of questions. What goes in the "Amount" column of the report (the last column)? If the line type is "Bill" and there is a payment for the bill, the Amount seems to be the total amount from the payment transaction. That gives the misleading result that bill 674 has three payments totaling $198.46 even though the billed amount is $121.33. I don't really care whether you show a line for each split in this payment or a single line for the payment, but the amount needs to be consistent with whichever you choose. As it is you show a line for each split, but give the total for the payment transaction. if the type of the line is "Payment" then the amount column seems to be the total amount on the bill which is confusing since it isn't really related to the payment itself. As to how to handle transactions with multiple splits paying the same bill, I don't think it matters whether you add a line for each split or one for the transaction so long as the amounts shown agree with whichever you choose. What happens if you have a payment that pays two or more different bills (a more comon case)? You really want the payment for each bill to appear separately so you know which bills have been paid. That means you want a line per split even though the bank statement may show both payments as one transaction. It might be best, and easiest, to always show a line per split and not make a special case for two splits in the same lot (so long as the amount is correct). As to how this happened, I really don't know. I spent a long time trying to figure out what DTE had done and make the books agree. It didn't help that I couldn't really change payment transactions much since the bank statements had already been reconciled before I realized that DTE had reissued those bills. I'm not sure what I did to cause this strange transaction to appear. However, it would be nice if the rrport can handle it, and I think it's very close to doing so.
FWIW for standard business txns I think the report is already satisfactory. The difficult part is what should we report on unusual transactions. (In reply to Mike Alexander from comment #17) > What goes in the "Amount" column of the report (the last column)? If the > line type is "Bill" and there is a payment for the bill, the Amount seems to > be the total amount from the payment transaction. The Amount column is designed (IMHO) best to show the matched invoice/payment. i.e. a invoice->payments should show the original payment as it appears in the Transfer Account; it means if the Transfer Account is in a different currency it'll pick up the other currency. This allows me to state "your 1/10/19 invoice for $25 was paid by my bank transfer dated 1/12/19 for $100". Correspondingly payment->invoice will show the corresponding invoice total. This lets me know "My payment $100 on 1/12/19 was used to pay 1/10/19 invoice $25 and 1/11/19 invoice $75." > That gives the misleading > result that bill 674 has three payments totaling $198.46 even though the > billed amount is $121.33. I don't really care whether you show a line for > each split in this payment or a single line for the payment, but the amount > needs to be consistent with whichever you choose. As it is you show a line > for each split, but give the total for the payment transaction. This is the case whereby an invoice/bill has multiple payments. I designed the payment (links) rows to have 1 payment-per-row, and in this unusual case, 2 payments were identical ($77.13). So I think a reasonable change is to remove duplicate payments. I can push this change imminently. > if the type of the line is "Payment" then the amount column seems to be the > total amount on the bill which is confusing since it isn't really related to > the payment itself. As above -- the payment->invoices links show details pertaining to the invoice/bill. > As to how to handle transactions with multiple splits paying the same bill, > I don't think it matters whether you add a line for each split or one for > the transaction so long as the amounts shown agree with whichever you > choose. What happens if you have a payment that pays two or more different > bills (a more comon case)? You really want the payment for each bill to > appear separately so you know which bills have been paid. That means you > want a line per split even though the bank statement may show both payments > as one transaction. It might be best, and easiest, to always show a line > per split and not make a special case for two splits in the same lot (so > long as the amount is correct). My preference is still 1 row per transaction rather than 1 row per split. In my view, for your 03/12/19 payment: 03/12/19 - Payment - Electricity for 602 - $77.13 - 06/11/19 "00674" $121.33 This describes that $77.13 left your bank account *once*, to partially pay the $121.33. Your bank does *not* record $42.04 nor $35.09 which IMHO is a "vendor problem". Your preference would lead to the $77.13 row split into $42.04 and $35.09, and the payment->links details would need to be duplicated because they both partially pay the $121.33 bill. The more common scenario, a payment fulfills 2 bills/invoices but overpays, is shown *by my design* as follows. Isn't this nice? 03/12/19 - Payment - For 2 bills - $110.00 - 06/10/19 "Bill 1" $25.00 07/10/19 "Bill 2" $75.00 Overpayment $10.00 Or for the awkward user, they have 2 invoices for $75 and $25, and 2 payments for $60 and $35, the output confidently tells me what paid for what. 01/01/19 - Invoice 1 - - $75.00 - 01/05/19 Payment 1 $60.00 - 01/06/19 Payment 2 $35.00 03/01/19 - Invoice 2 - - $25.00 - 01/06/19 Payment 2 $35.00 Outstanding $5.00 01/05/19 - Payment 1 - $60.00 - 01/01/19 Invoice 1 $75.00 01/06/19 - Payment 2 - $35.00 - 01/01/19 Invoice 1 $75.00 03/01/19 Invoice 2 $25.00 > As to how this happened, I really don't know. I spent a long time trying to > figure out what DTE had done and make the books agree. It didn't help that > I couldn't really change payment transactions much since the bank statements > had already been reconciled before I realized that DTE had reissued those > bills. I'm not sure what I did to cause this strange transaction to appear. > However, it would be nice if the rrport can handle it, and I think it's very > close to doing so. I'll add a last patch -- I think fixes most issues. Let me know ASAP and it'll be ready for 3.8.
Created attachment 373485 [details] last patch. fixes invoice->payments. multiple changes. from invoice, finds the payment splits. -filter payment splits only - much safer than gnc_lot_get_earliest_split -dedupes them -one payment row per line -sort by date
Thanks for the explanation. Now that I understand what you're doing it makes sense. I applied the patch in comment 19 and the report looks fine now. Thanks for your work on this.
Note a small caveat regarding the debit/credit amount. It does not 100% represent my 'bank statement amount' -- for invoices/bills paid in a different currency, it'll display the amount converted to APAR currency, because we need the converted amounts for debit/credit totals and to update the running balance. Moreover I'm not 100% sure of output for gjanssens' weird txn whereby AP bill is paid by an AR credit. I suspect the output will be ok though. Otherwise now this report is ready I am now happy to use it personally :)
(In reply to Christopher Lam from comment #21) > Moreover I'm not 100% sure of output for gjanssens' weird txn whereby AP > bill is paid by an AR credit. I suspect the output will be ok though. > You can safely ignore this case. It's not supported ATM and was only in my test book to verify if it was possible or not. Such a manually crafted Frankenstein transaction will upset more parts of gnucash than just your report. > Otherwise now this report is ready I am now happy to use it personally :) Great!
Created attachment 373491 [details] moar patch. Ok. Unfortunately my desired report which should have the following characteristics is incompatible with lot-link transactions. * one line per invoice posting txn * one line per payment txn Currently (92509761a) the report would fail for lot-link txns. This is because invoices paid via a credit-note does *not* have xaccTransGetPaymentAcctSplitlist splits, therefore the report fails spectacularly. The fix to allow lot-link txns to be reported decently would mean that invoice->payment will show *all* APAR splits, therefore mta's book would show $42.04 and $35.09 as 03/12/19 payments for the $121.33 bill; these payments would not match the 03/12/19 payment of $77.13. So this patch would, for invoice->payments, report all APAR splits. It would be sensible to then allow the main table to report on all APAR splits too, reusing #c11 and #c12 patches. What does everyone think?
I haven't tried this patch yet, but I think showing a line per split is fine. I think I could even argue that it's better but i don't really care much. This case is rare and bank reconciliation isn't affected by this report much anyway.
Created attachment 373494 [details] patch and sample reports All, I'm currently in an area with very limited internet connectivity. I have experimented with all sorts of combinations of invoice->payment splits. I still think that a *transaction* based report is a much better one than a *split* based report. The reasoning is: * a transaction based report will have 1 main-report (left-hand-side) with 1 line per invoice posting, and 1 line per payment activity. A payment activity is e.g. "I pay $200 to supplier for 3 bills, and overpaid as well". A transaction-based report will show "$200 paid, with links to $20+$30+$50+$100". A split-based report will show 1 line per split, e.g. $20 for bill 1, $30 for bill 2, $50 for bill 3, and prepaid $100 to vendor. These smaller amounts are not present in my bank statement, and IMHO very confusing. * a payment involving lot links i.e. whereby an invoice is "paid" using a credit-note, is currently broken. I have spent the past few days considering how to report them correctly, and so far my only conclusion is that lot-link txns have no useful textual information and are best reported as "Offset Documents". Payments which involve lot links *can* be reported properly. e.g. an invoice $194 is partially paid by $99 credit-note and $95 payment will show "payment of $95, and offset documents of $95". See latest patch and sample reports! I would wager these are now satisfactory.
I tried some unusual books with linked lots, but creates imperfect report. I think I'm just pushing it a bit too far. (1) inv $100, CN $80, linked with $20 outstanding from CN (2) CN $80, inv $100, linked with $20 outstanding from invoice (3) link these two $20 outstanding together. I've realised that to get 100% bulletproof is a wild goose chase. Unless further objections I'll seek to merge soon.
Created attachment 373495 [details] Sample output posted by Christopher Lam to gnucash-devel this morning
Created attachment 373496 [details] Sample output on my local build Based on https://github.com/Gnucash/gnucash/commit/f583bc6d869a38adafd748743c3d6e8c01e61df0 I still have the following observations 1. On my report the references don't render properly. They are the same strings as the type column. This does render fine on Chris' sample though so I don't know what's going on here. 2. In the document offsetting situation the details columns are not as on Chris' output. Mine puts the CN or bill number in the (details) type column, whereas Chris' output puts it in the Details column. 3. Also in the offsetting situation the linked documents are not html links pointing at the respective documents.
I do like you split up the details column in two. This is more consistent with the left hand side of the report. A few more observations while testing this some more with my local build: * clicking on any payment link opens the respective bank or cash account, but won't select the payment split/transaction itself. Looking at the generated links in detail I see they are like this: gnc-register:split-guid=9890b36a099951a50fda44bf04a196a9# This seems to be a general regression. None of the other reports still do this properly on current maint. * for consistency you may want to swap the reference and type accounts on the left hand side or the type and details columns on the right-hand side. That way reports will consistently have Bill Bill-ID ...... Credit Note CN-ID .... or Bill-ID Bill ...... CN-ID Credit Note .... * Currently your report will add hyperlinks to the type column on the left-hand side and to document-id/amounts on the right hand side. It would be more intuitive to me if hyperlinks are always on Bill-ID/CN-ID and payment amounts. * On the details side, for a payment it currently shows the word payment twice: once in the type column and once in the details column. Once is probably enough :)
(In reply to Mike Alexander from comment #8) > Created attachment 373482 [details] > Faulty report > > I've attached the HTML for the report that is incorrect, is that good enough? > > There are two bills that are wrong IMO. > > Bill 674 shows two payments of $77.13. The associated payment shows twice > in the report, once with an amount of $77.13 and once with blanks for the > payment amount. This payment transaction has two splits in the lot for bill > 674, one for $42.04 and one for %35.09. Neither of these amounts appears in > the report. I suspect this is something a check and repair would solve. It should detect multiple splits in the same transaction linked to the same lot and merge those together. I can't think of any reason why they should not be. Can you test this on a copy of your book to check if that works properly (or you can send my a copy of your book in private as well and I'll do the test myself) ?
(In reply to Geert Janssens from comment #28) > 1. On my report the references don't render properly. They are the same > strings as the type column. This does render fine on Chris' sample though so > I don't know what's going on here. No idea either. > 2. In the document offsetting situation the details columns are not as on > Chris' output. Mine puts the CN or bill number in the (details) type column, > whereas Chris' output puts it in the Details column. > > 3. Also in the offsetting situation the linked documents are not html links > pointing at the respective documents. Agree this is not consistent with other behaviour, will offer patch soon. (In reply to Geert Janssens from comment #29) > * clicking on any payment link opens the respective bank or cash account, > but won't select the payment split/transaction itself. > Looking at the generated links in detail I see they are like this: > gnc-register:split-guid=9890b36a099951a50fda44bf04a196a9# > This seems to be a general regression. None of the other reports still do > this properly on current maint. This calls gnc:split-anchor-text and shouldn't insert a # anywhere. > * for consistency you may want to swap the reference and type accounts on > the left hand side or the type and details columns on the right-hand side. > That way reports will consistently have > Bill Bill-ID ...... Credit Note CN-ID .... > or > Bill-ID Bill ...... CN-ID Credit Note .... I think it'll make sense to swap on right-hand-side. > * Currently your report will add hyperlinks to the type column on the > left-hand side and to document-id/amounts on the right hand side. It would > be more intuitive to me if hyperlinks are always on Bill-ID/CN-ID and > payment amounts. I had the general idea that hyperlinks to business documents would be from a non-monetary cell, and hyperlinks to register would be from a monetary cell. But this is not obvious! > * On the details side, for a payment it currently shows the word payment > twice: once in the type column and once in the details column. Once is > probably enough :) Can't see this myself either.
(In reply to Geert Janssens from comment #28) > Created attachment 373496 [details] > Sample output on my local build > > Based on > https://github.com/Gnucash/gnucash/commit/ > f583bc6d869a38adafd748743c3d6e8c01e61df0 I still have the following > observations > > 1. On my report the references don't render properly. They are the same > strings as the type column. This does render fine on Chris' sample though so > I don't know what's going on here. > > 2. In the document offsetting situation the details columns are not as on > Chris' output. Mine puts the CN or bill number in the (details) type column, > whereas Chris' output puts it in the Details column. > > 3. Also in the offsetting situation the linked documents are not html links > pointing at the respective documents. Just to be sure I wasn't seeing older build interference, I started with a clean build and install directory. The above observations still stand with this clean build.
Created attachment 373497 [details] latest patch patch and demo. - doesn't swap type/details; maintains date/type/description/amount order - r.h.s. hyperlinks belong on the Type column - hyperlinks to invoice/bill/creditnote now point to the document editor I still don't know why gjanssens' build inserts wrong order. Maybe something with the book split-action property?
Your patch doesn't apply to current maint. The patch claims to apply a diff between commits 6feddfde4 and 0d1383d1d but there is no commit 6feddfde4 on maint on github. What's in your repo after commit f583bc6d869 ?
(In reply to Geert Janssens from comment #29) > * clicking on any payment link opens the respective bank or cash account, > but won't select the payment split/transaction itself. > Looking at the generated links in detail I see they are like this: > gnc-register:split-guid=9890b36a099951a50fda44bf04a196a9# > This seems to be a general regression. None of the other reports still do > this properly on current maint. > Ok, this was just noise. The register to jump to had a filter applied and the split to select was hidden... Thoughts for a separate enhancement request, but this can further be ignored here.
(In reply to Christopher Lam from comment #33) > Created attachment 373497 [details] > latest patch > > patch and demo. > - doesn't swap type/details; maintains date/type/description/amount order > - r.h.s. hyperlinks belong on the Type column > - hyperlinks to invoice/bill/creditnote now point to the document editor > > I still don't know why gjanssens' build inserts wrong order. Maybe something > with the book split-action property? Ah, exactly. My book has the num-for-split-action property set. Removing this fixes the references and the double mention of Payment. This does bring me to another point though: this is a business report. IMO that means it should use business data as much as possible. In the reference field you apparently are using the "num" field's info where I was expecting you to use the invoice id extracted from the invoice object. That's more useful information than a transaction reference for invoices. Technically it should be the same, except the num-for-split-action doesn't seem to work well with the business features. Probably worth another bug.
Created attachment 373498 [details] Report output with Chris's current maint (9dd06b88450) With num-for-split disabled and chris' current maint applied, the report looks as attached. I have my reservations on title change from Details to Description onthe right-hand side . To me that info is really what the reference field is for the left-hand side. For bills and invoices it shows the document id (as does the reference field on the lhs). For payments it shows the transaction number (again as does the reference filed on the lhs). Further it now applies hyperlinks to the words "Bill", "Payment",... While more consistent than your original strategy, I was actually with you on adding hyperlinks to amount fields when linking to a transaction and to non-amount fields when linking to documents. Except your original strategy was not consistent on this either. On the left-hand side it always made/makes links on the "type" column. I would expect it to make links on the reference column for bills, invoices and credit notes and on the debit/credit column for payments. For bills, invoices and credit notes the reference field holds the document's ID number, which makes perfect sense as a hyperlink anchor to that document. The rhs can follow lhs. Note on my report the rhs doesn't show document references for documents related to lot links. I hinted on the mailing list these can be retrieved from the respective lots they link. I'd go as far as consistently using the invoice reference from lots whenever available so not only for lot link transactions but for all transactions involving invoice objects. That will circumvent the split-action bug at least for the document related data. I'm not sure where your description data comes from. On my system the lhs description column is just empty and the rhs one shows the reference data, where your report seems to find description data.
Thank you this is the intended output. My comments below are *not* to defend the design decisions, but rather forms the basis of how I've arrived at the current iteration 9dd06b88450, and we'll use it to discuss the next iteration. (In reply to Geert Janssens from comment #37) > Created attachment 373498 [details] > Report output with Chris's current maint (9dd06b88450) > > With num-for-split disabled and chris' current maint applied, the report > looks as attached. > > I have my reservations on title change from Details to Description onthe > right-hand side . To me that info is really what the reference field is for > the left-hand side. For bills and invoices it shows the document id (as does > the reference field on the lhs). For payments it shows the transaction > number (again as does the reference filed on the lhs). An owner has many text fields -- number/company-name/name/address/notes. An invoice also has many text fields - ID(may be autogenerated)/notes/billing-ID. An invoice posting-txn also has text field - description. IIUC the invoice-posting-transaction *description* (i.e. the invoice transaction->splits xaccSplitGetMemo) is used to populate the Description column in the old owner-report, and I merely copied it to the right-hand-side. I suspect that in your book, you manually inserted the invoice ID into the invoice-posting-txn description field. We can definitely choose another field to describe an invoice. But there's too many of them! Which one is the most appropriate one? I have no idea. > Further it now applies hyperlinks to the words "Bill", "Payment",... While > more consistent than your original strategy, I was actually with you on > adding hyperlinks to amount fields when linking to a transaction and to > non-amount fields when linking to documents. > Except your original strategy was not consistent on this either. > On the left-hand side it always made/makes links on the "type" column. I > would expect it to make links on the reference column for bills, invoices > and credit notes and on the debit/credit column for payments. For bills, > invoices and credit notes the reference field holds the document's ID > number, which makes perfect sense as a hyperlink anchor to that document. > The rhs can follow lhs. The left-hand-side merely copied old owner-report behaviour. Should we change *all* so that monetary-cell hyperlinks to register and document-type-cell hyperlinks to document-editor? > > Note on my report the rhs doesn't show document references for documents > related to lot links. I hinted on the mailing list these can be retrieved > from the respective lots they link. I'd go as far as consistently using the > invoice reference from lots whenever available so not only for lot link > transactions but for all transactions involving invoice objects. That will > circumvent the split-action bug at least for the document related data. (split-action is a headache I'd gladly live without) > I'm not sure where your description data comes from. On my system the lhs > description column is just empty and the rhs one shows the reference data, > where your report seems to find description data. as above, with my book split-action being FALSE it comes from the posting-txn Description.
Ok, I see some of the decisions were copied from the old report which is fair. I'll admit I have been looking at your report in isolation and my remarks are based on what it does, regardless of where it came from. As you are seriously revamping the report, I didn't think of looking back. My apologies by the way for joining the discussion this late. I only now have a few cycles to dedicate to it. (In reply to Christopher Lam from comment #38) > (In reply to Geert Janssens from comment #37) > > I have my reservations on title change from Details to Description on the > > right-hand side . To me that info is really what the reference field is for > > the left-hand side. For bills and invoices it shows the document id (as does > > the reference field on the lhs). For payments it shows the transaction > > number (again as does the reference field on the lhs). > > > An owner has many text fields -- number/company-name/name/address/notes. An > invoice also has many text fields - ID(may be > autogenerated)/notes/billing-ID. An invoice posting-txn also has text field > - description. > > IIUC the invoice-posting-transaction *description* (i.e. the invoice > transaction->splits xaccSplitGetMemo) is used to populate the Description > column in the old owner-report, and I merely copied it to the > right-hand-side. > > I suspect that in your book, you manually inserted the invoice ID into the > invoice-posting-txn description field. > > We can definitely choose another field to describe an invoice. But there's > too many of them! Which one is the most appropriate one? I have no idea. > As for the description - it is an optional string entered by the user when posting an invoice. I had forgotten about that and clearly never use it myself. It's a bit of an idiosyncrasy to have the user enter a "Description" and then store it as a "Memo", but that's how it is currently... Personally the key I typically use to quickly locate invoices is a combination of date and invoice number (aside from owner name of course but on an owner report all invoices belong to the same owner). Those are the most formal fields to refer to a document. The report currently gets the invoice number from the invoice transaction's num field, but this has a few limitations a. it's not populated for a lot link transaction b. it's susceptible to split-action (which I am not too fond of either) That's why I would propose to get it from the lot's invoice directly. For people using the description field it is probably also a useful key to add to the report. (It's slightly inconsistent as this adds transaction only information to a line about a document. But alas, the "invoice description/memo" is not stored as part of an invoice. It's a transaction only data element.) As the lhs of the report suggests, this is a separate field from the reference field. Perhaps it's worth doing this same separation on the rhs ? And then show/hide the fields in pairs. If the user unticks "Description", hide in in both lhs and rhs of the report. Same for "Reference" and "Type" fields. > > Further it now applies hyperlinks to the words "Bill", "Payment",... While > > more consistent than your original strategy, I was actually with you on > > adding hyperlinks to amount fields when linking to a transaction and to > > non-amount fields when linking to documents. > > > Except your original strategy was not consistent on this either. > > On the left-hand side it always made/makes links on the "type" column. I > > would expect it to make links on the reference column for bills, invoices > > and credit notes and on the debit/credit column for payments. For bills, > > invoices and credit notes the reference field holds the document's ID > > number, which makes perfect sense as a hyperlink anchor to that document. > > The rhs can follow lhs. > > The left-hand-side merely copied old owner-report behaviour. Should we > change *all* so that monetary-cell hyperlinks to register and > document-type-cell hyperlinks to document-editor? > If you want to pursue your logic, then yes. It works well for me, as then the choice of anchor holds semantic information about where the link will go. In addition you can have a link both on reference and amount on the same line if you want - one will link to the document the other to the post transaction. > (split-action is a headache I'd gladly live without) +1 > > I'm not sure where your description data comes from. On my system the lhs > > description column is just empty and the rhs one shows the reference data, > > where your report seems to find description data. > > as above, with my book split-action being FALSE it comes from the > posting-txn Description. Lhs it comes from the posting-split memo field, rhs it's from the posting-txn num field. Hence my suggestion rhs is not really description, it's reference. And while looking at the report I got another thought about the visual representation: when a lhs document has multiple payments in detailed mode these payments are presented one per line. All but the first line begin with a whitespace consisting of a horizontal span of cells. This is slightly confusing as that large block of white space intuitively suggests a break, like a next section of the report to begin after it. It's not really a next section though, what follows is just the next lhs data row. To me it would make more sense to group the lhs cells vertically in this case. So for a lhs data element with say 3 rhs elements to go with it, all of the lhs cells would vertically span 3 rows. I have no idea whether this can be done with a reasonable amount of work or not in the current table code.
Next iteration. This comment is based on the state of the report in https://github.com/christopherlam/gnucash/commit/98274aa28 The parent commit (https://github.com/christopherlam/gnucash/commit/f72d48e19bc6fb) changes the rhs columns to represent a Reference and a Type, dropping the Description column. I get a feeling there is some confusion on what each column represents. The Reference column represents the document id (for bills, invoices,...) or the transaction number (for payments). The Description field represents the payment split's memo field. These are separate bits of information and can both be presented on the report, but in their own column. So both lhs and rhs I would provide these 3 colummns: Reference, Type and Description. And let the user control visibility with via the Display options. Personally I would offer a single checkbox per pair of columns. So the "Description" checkbox shows or hides both Description columns at once. And the same for the Reference and Type checkboxes. Next, to me the document ID is the most specific bit of information representing the documents, so I would use that as the hyperlink anchor rather than the more generic type strings. That also solves the redundant hyperlink anchors for payments. You could argue the same for the transaction number - that number is more specific than the generic payment string. So making the transaction number a hyperlink would be my preference. Disadvantage there is not all transactions have a number. Not too much of a problem as there are still hyperlinks on the amounts in that case. Or if preferred you could replace empty reference fields with a generic marker that can serve as hyperlink anchor, like a "*" or whatever. I also noted there's "Overpayment" (singular) and "Prepayments" (plural). Is that intentional ?
Ok (In reply to Geert Janssens from comment #40) > I get a feeling there is some confusion on what each column represents. The > Reference column represents the document id (for bills, invoices,...) or the > transaction number (for payments). The Description field represents the > payment split's memo field. These are separate bits of information and can > both be presented on the report, but in their own column. Agree very confusing. I've amended in my current maint 6bc03a58c to query gncInvoiceGetID for ref, and xaccSplitGetMemo for desc. > So both lhs and rhs I would provide these 3 colummns: Reference, Type and > Description. And let the user control visibility with via the Display > options. Personally I would offer a single checkbox per pair of columns. So > the "Description" checkbox shows or hides both Description columns at once. > And the same for the Reference and Type checkboxes. > > Next, to me the document ID is the most specific bit of information > representing the documents, so I would use that as the hyperlink anchor > rather than the more generic type strings. That also solves the redundant > hyperlink anchors for payments. You could argue the same for the transaction > number - that number is more specific than the generic payment string. So > making the transaction number a hyperlink would be my preference. > Disadvantage there is not all transactions have a number. Not too much of a > problem as there are still hyperlinks on the amounts in that case. Or if > preferred you could replace empty reference fields with a generic marker > that can serve as hyperlink anchor, like a "*" or whatever. I think these are now done. > I also noted there's "Overpayment" (singular) and "Prepayments" (plural). Is > that intentional ? I actually don't know how best to describe these things.
Ok a few commit message amendments, my maint will always have the latest :)
Created attachment 373499 [details] iteration so far most feedback considered. i'm still not sure whether to use split->memo or anything else for description.
Created attachment 373500 [details] iteration so far (bis) Further work. - mta's book has peculiarity that some of his bill posting transactions have 2 splits to EXPENSE balancing the AP split. These 2 splits have different memos, whereas the AP split has no memo. We can use BillID as reference but description is either blank (matching AP split), or a combination of all split->memos that I have illustrated in this sample report. - hyperlinks now stem from Bill/Customer ID to document editor, and monetary-amount to register as gjanssens discussed above. - use gncInvoiceGetID instead of gnc-get-num-action and friends for a more reliable reference lookup. Advantage - more official, less susceptible to split-action-for-num bugs. Disadvantage - loses ability to use split/transaction properties which some users may have become accustomed to. - invoices/payments with multi-row links now use rowspan instead of repeating blank line. neater. I'm quite happy myself that we can now discuss cosmetics rather than exploring how to retrieve owner activity data.
This looks rather nice IMO. (In reply to Christopher Lam from comment #44) > - mta's book has peculiarity that some of his bill posting transactions have > 2 splits to EXPENSE balancing the AP split. That's not a peculiarity per se. It's one of the business features. When you post an invoice you can choose to "Accumulate splits" or not. If you don't accumulate, all bill entries will be converted into individual splits. If you accumulate, the entries will be grouped per account. > These 2 splits have different > memos, whereas the AP split has no memo. We can use BillID as reference but > description is either blank (matching AP split), or a combination of all > split->memos that I have illustrated in this sample report. > I don't think it would be wise to combine the individual split memos. IMO it will only work well in a very limited number of cases, namely when the bills have only very few entries *and* these entries are not accumulated. That info is also only one click away via the bill hyperlink (entries view) or via the bill's amount hyperlink (transaction view with individual splits). > > - use gncInvoiceGetID instead of gnc-get-num-action and friends for a more > reliable reference lookup. Advantage - more official, less susceptible to > split-action-for-num bugs. Disadvantage - loses ability to use > split/transaction properties which some users may have become accustomed to. > It doesn't really loose that ability. Invoice post transactions are read only. User's can't do anything to change the transaction number or split action fields on these transactions. They are completely controlled by the business features. For payments you continue to use the transaction number field. These transactions can be modified by users, so it's good to keep on presenting the information in there. So this continues to work as before for users that rely on manipulations of the num/action fields for payments. > - invoices/payments with multi-row links now use rowspan instead of > repeating blank line. neater. > Very nice indeed. > I'm quite happy myself that we can now discuss cosmetics rather than > exploring how to retrieve owner activity data. Same here. It's shaping up to be a very nice report. I'm impressed really.
(In reply to Geert Janssens from comment #45) > (In reply to Christopher Lam from comment #44) > > - use gncInvoiceGetID instead of gnc-get-num-action and friends for a more > > reliable reference lookup. Advantage - more official, less susceptible to > > split-action-for-num bugs. Disadvantage - loses ability to use > > split/transaction properties which some users may have become accustomed to. > > > It doesn't really loose that ability. Invoice post transactions are read > only. User's can't do anything to change the transaction number or split > action fields on these transactions. They are completely controlled by the > business features. > > For payments you continue to use the transaction number field. These > transactions can be modified by users, so it's good to keep on presenting > the information in there. So this continues to work as before for users that > rely on manipulations of the num/action fields for payments. > I should add for completeness that the canonical way of entering a payment is via the Payment dialog. That dialog allows the user to add a "Num" and a "Memo". Those are the ones being used to create the payment transaction and split. I believe it makes most sense to try to display those in the report. Which is what you get when you use the transaction number (or split-num-for-action thingy) and the payment split's memo field. Which is how you implemented it. I'm sure users will eventually use all other transaction fields in some form or another. That doesn't mean your report has to show all of that info. It's primary focus is displaying a transaction history for a business relation. So it makes sense to focus on the data provided by the business features in some way. If more details are needed, your report makes it easy to drill down to actual documents and transactions.
(In reply to Geert Janssens from comment #30) Sorry for the delay, we went to Quebec City for Christmas. First I want to say that I like the way the report works now. I have only one small suggestion. Invoice references appear in the left section once for each invoice and in the right section once for each payment to the invoice. In the right section the description column for the invoice contains a description, but in the left section the description is usually blank. It would be nice if both "description" columns contained the information shown in the right section. With regard to check and repair fixing the funny transactions in my file, I'm afraid I have bad news. Running it on the bank account did nothing and ran quickly (which isn't surprising). Running it on the A/P account took a long time (after an hour I went to dinner) and then crashed. Before it crashed it wrote 9820 lines in the .log file associated with the .gnucash file. It also wrote 293 lines to the debug file, but the last line seems to have been written long before the crash. I can't find a crash log file anywhere and the only thing on the console after the crash was /Users/mta/bin/gnucash: line 14: 20818 Killed: 9 ${GNC_TOP_DIR}/bin/gnucash $GNC_PARAM Do you want me to send you a copy of the book? Or file a new bug?
(In reply to Mike Alexander from comment #47) > First I want to say that I like the way the report works now. I have only > one small suggestion. Invoice references appear in the left section once > for each invoice and in the right section once for each payment to the > invoice. In the right section the description column for the invoice > contains a description, but in the left section the description is usually > blank. It would be nice if both "description" columns contained the > information shown in the right section. As of 3.8, the textual fields are derived as follows: LHS reference: - payment/link - from APAR split's gnc_get_num_action - invoice - from APAR split's invoice's gncInvoiceGetID LHS desc: - APAR split's xaccSplitGetMemo RHS reference: - payment - from transfer split's gnc_get_num_action - link - from invoice/CN posting split's gncInvoiceGetID RHS desc: - payment - from transfer split's memo - link - from invoice/CN posting split's memo mta I suspect you want LHS's desc to match RHS desc - I think this is not straightforward; a LHS txn may match several several RHS objects and currently the report is likely to pick one of them "arbitrarily" to populate the LHS desc field. This is why I put forward the experiment to match all "matching splits' memos" in the screenshot in https://bugs.gnucash.org/show_bug.cgi?id=797521#c44 -- I also highlight "arbitrary" because it's not really arbitrary; it picks up the *first* split in a multisplit APAR transaction. As a test I suggest to modify a PAYMENT txn so that the APAR split and the Transfer acc split have different strings for the memo... which one should we consider the canonical one to use for desc?
Created attachment 373502 [details] screenshot for #c48 report output - simple invoice + payment; payment txns modified to have different memos.
Created attachment 373503 [details] trial: make LHS split->desc consistent mta: trial patch on 3.8, forces all split->desc to show the Transfer split memo. if a invoice-posting / payment transaction should have >1 transfer split, this would be an unusual transaction and would return the first transfer split memo only.
Chris, I'm not sure I read Mike's remark the same as you do. From what I see he's specifically referring to invoice descriptions. (Mike, correct me if I'm interpreting this wrongly). Also your lists of what data goes into lhs or rhs descriptions seems to be incomplete ? rhs you document payments and links, but what happens when rhs is an invoice ? I presume it also takes the invoice's post split memo. And I'm confused how - for invoices - APAR xaccSplitGetMemo can be different from invoice posting split memo ? (In reply to Christopher Lam from comment #48) > mta I suspect you want LHS's desc to match RHS desc - I think this is not > straightforward; a LHS txn may match several several RHS objects and > currently the report is likely to pick one of them "arbitrarily" to populate > the LHS desc field. The way I understand this, it should not? Particularly for invoices, there can be only one APAR split and it's memo is the post description. That should be consistent left and right. I'll assume this discussion is mainly focusing on payments. > This is why I put forward the experiment to match all > "matching splits' memos" in the screenshot in > https://bugs.gnucash.org/show_bug.cgi?id=797521#c44 -- I also highlight > "arbitrary" because it's not really arbitrary; it picks up the *first* split > in a multisplit APAR transaction. > > As a test I suggest to modify a PAYMENT txn so that the APAR split and the > Transfer acc split have different strings for the memo... which one should > we consider the canonical one to use for desc? What amount are you displaying for payments ? The APAR split amount or the transfer amount ? Your memo should be from the same split.
(In reply to Christopher Lam from comment #50) > Created attachment 373503 [details] > trial: make LHS split->desc consistent > > mta: trial patch on 3.8, forces all split->desc to show the Transfer split > memo. > > if a invoice-posting / payment transaction should have >1 transfer split, > this would be an unusual transaction and would return the first transfer > split memo only. No, transactions with more than one transfer split are not uncommon at all. You can't rely on that assumption. If an invoice has entries to more than one account the posted transaction for this invoice will have more than one transfer split. If you have tax tables, your posted invoice transaction will have multiple transfer splits. It's equally common to have payments with more than one transfer split. In the payment dialog, select two invoices to pay at once and your payment transaction will have two transfer splits, unless you consider all APAR splits as non-transfer. Your patch at least doesn't. These are all very common scenarios and the report should handle them fine. Can we step back a little and summarize the higher level concepts to avoid further confusion ? Here is how I see it so far: LHS 1. will list all invoices, payments and links in chronological order. 2. I presume you get those by running over all APAR splits for a given vendor/customer. 3. if the APAR split is from an invoice transaction, extract the invoice data and report that. The data can be extracted from either the invoice lot or the invoice transaction. 4. if the APAR split is from a payment transaction, the way to extract the necessary details can be more complicated as users can freely modify payment splits outside of the payment window. The payment window as it currently is will only ever create payments with one non-APAR split. So after all this thinking out loud, I come to almost the same conclusion as you that there is a common trait in "normal" payment splits, it's not that there's only one transfer split though, it's that there's normally only one non-APAR split. However, again, as users can freely tweak this transaction it's difficult to rely on this. My own test book has several payment transactions which contain both a bank and a cash account split. 5. So for payment splits, I would consider all of the non-APAR splits together; that is, amount is sum of all these non-APAR split amounts, description is concat of all (non-empty) non-APAR split memo fields. It *should* be uncommon to have many non-APAR splits in a payment transaction, so the resulting description should in general remain relatively short. RHS 1. for each LHS, determine the related offsetting items 2. I presume you start from the lot the lhs split is in and work from there. 3. That lot can contain multiple other APAR splits referring to other invoices (both positive or negative), or payment transactions. 4 I assume the user would be most interested in knowing the exact offsetting amounts here, rather than the payment or invoice totals. 5 Because of this assumption the APAR splits hold the amounts we're most interested in. 6. So use this APAR split data (being memo and amount) to present on the rhs. In case of partial payments, this may mean the amount rhs is not the total amount of the rhs item. For example a $200 credit note may offset a $150 bill. So lhs you can find a $150 bill and a $200 credit note. rhs, next to the bill there's $150 credit note (as that's all of the credit note that was used to pay the bill). I realize this point 6 is not what happens currently. However I would propose this anyway as without it, it's very difficult to figure out the exact amounts of "paying" items (rhs) are really offsetting the "to be paid" items (lhs). I have written this part without actually reading your current code and I'm interested in hearing your views on this. Where you have made different decisions and why.
A *very* short reply to explain why no.6 isn't happening as it stands (In reply to Geert Janssens from comment #52) > 6. So use this APAR split data (being memo and amount) to present on the > rhs. In case of partial payments, this may mean the amount rhs is not the > total amount of the rhs item. For example a $200 credit note may offset a > $150 bill. So lhs you can find a $150 bill and a $200 credit note. rhs, next > to the bill there's $150 credit note (as that's all of the credit note that > was used to pay the bill). > > I realize this point 6 is not what happens currently. However I would > propose this anyway as without it, it's very difficult to figure out the > exact amounts of "paying" items (rhs) are really offsetting the "to be paid" > items (lhs). Because (in my view only) RHS should show the balancing document/payment. If I paid a $100 invoice using a $150 payment, the RHS showing "Payment" "$150" to me is much more recognisable than having RHS showing Payment $100. The partial RHS $100 amount would seem redundant to the LHS invoice-amount; the full RHS $150 amount would trigger a much better memory of a payment. Likewise a LHS payment of (say) $1000 to pay invoices of $250, $400 and $600 would want to show the invoices as their full amounts, rather than their contributory amounts $250, $400 and $350; there's no invoice of $350. I considered the invoice amounts as part of their identity. (IMO) I'd tell a vendor, "On 22 December I paid $100 to cover your bill of $75.00 dated 1-December and partially your bill of $45.00 dated 15-December. I am sorry can't pay the remaining $20 yet." The vendor would not recognise having issued a bill of $25. --- With the other issues I'll review in due course and consider a patch when I can.
I can understand your view. However again I suspect you're only looking at the most simple use case: one invoice is paid with (part of) one payment. So in your example you have LHS Invoice 1, amount $100 RHS Payment 1, amount $150 Let's expand this and imagine this invoice was paid with parts of two payments: one payment was $150, and I used $60 of it for this invoice, the rest was assigned somewhere else. The second payment was $90, and I used $40 to complete the invoice's payment. With your chosen strategy you'd now get LHS Invoice 1, amount $100 RHS Payment 1, amount $150 Payment 2, amount $90 This line doesn't tell me anything about how much of either payment was used to pay Invoice 1. Obviously somewhere else in the report the two payments will appear LHS as well. Imagine the remainder of the $150 payment was used to partially pay another invoice, this time one for $110, the remainder of that invoice is also paid with part of payment 3, which is $200. So this is how Payment 1 would appear with it's balancing actions in your case: LHS Payment 1, amount $150 RHS Invoice 1, amount $100 Invoice 2, amount $110 Again this line doesn't help me in determining how much of Invoice 1 was paid by Payment 1. If payments get more involved and fractured, the data I'm looking for is very hard to extract from the current report. On the other hand, if RHS only shows the assigned amounts this is straight forward: LHS Invoice 1, amount $100 RHS Payment 1, amount $60 Payment 2, amount $40 and LHS Payment 1, amount $150 RHS Invoice 1, amount $60 Invoice 2, amount $90 Now as to the point you make about your vendor being interested in total payment amounts only, I agree. However that info is also still in your report: LHS still is a chronological overview of total amounts. So if you want to discuss Payment 1 with the vendor for whatever reason you can use RHS date and reference to quickly look up the payment LHS for the full amounts. It is one step extra for your use case, but you don't have to leave the report for this. My use case is impossible with the current report. Now so much for the basics. There are probably ways to improve on this still. I can think of two: 1. expand rhs with an additional amount column. One would be the amount effectively assigned, the other would be the payment/invoice/link total. This is fairly easy to implement. The challenge is to come up with names that don't make things more confusing. 2. use javascript or advanced css to highlight all legs of one payment/invoice/link when hovering over one. So in the example above, when you hover over Payment 1 LHS, that would light up, together with the two appearances RHS. Same when you hover over one of the legs RHS, the other RHS leg and the LHS leg should light up. That can help as a visual aid to quickly find related links.
I agree with Geert's comments on this. He brought up a number of use cases I hadn't thought of. For the record, he is correct in C#51 when he says "I'm not sure I read Mike's remark the same as you do. From what I see he's specifically referring to invoice descriptions. (Mike, correct me if I'm interpreting this wrongly)." I also tried your simple patch mentioned in C#50 (although further discussion has rendered it somewhat obsolete) and it changes the report in the way I suggested.
(In reply to Mike Alexander from comment #47) > (In reply to Geert Janssens from comment #30) > With regard to check and repair fixing the funny transactions in my file, > I'm afraid I have bad news. Running it on the bank account did nothing and > ran quickly (which isn't surprising). Running it on the A/P account took a > long time (after an hour I went to dinner) and then crashed. Before it > crashed it wrote 9820 lines in the .log file associated with the .gnucash > file. It also wrote 293 lines to the debug file, but the last line seems to > have been written long before the crash. I can't find a crash log file > anywhere and the only thing on the console after the crash was > > /Users/mta/bin/gnucash: line 14: 20818 Killed: 9 > ${GNC_TOP_DIR}/bin/gnucash $GNC_PARAM > > Do you want me to send you a copy of the book? Or file a new bug? Please do send me your book in private. As it does have some uncommon APAR transactions, it makes for a good stress test of the business scrub functions. Right before the 3.8 release John pushed a commit that eliminates a number of asserts in the capital gains scrubbing code. Was that commit already in your build ?
I'll send the file. I did not have John's commit d51b4d7 in the build that got the crash.
Created attachment 373504 [details] concat unique tfr split memos Candidate patch. * LHS split->desc is modified so that it finds all Transfer (i.e. non-APAR) splits. Their non-identical memos are collected, and concat with ", ". I think this is a reasonable approach: (a) for INVOICE txns, *all* memos are identical, and protected from modification. This means invoice posting txns, the LHS Description column will be fine. This works well even if entries use tax tables. (b) for PAYMENT txns, *all* non-APAR memos will be collected. The unusual case of, e.g. a payment txn to one or more invoices is modified to include another non-APAR split will show on LHS Desc, both memos concatenated if they are different. * An invoice->RHS payments will collect all payments. If INV-01 has 2 separate payment txns, and the 2nd payment has 2 non-APAR splits, then its RHS will have 3 rows to describe the 3 non-APAR splits in their original currency. I think this is nice. * showing full vs partial contributory amounts not currently implemented yet. Will take some hacking. If we want to show contributory amounts then it *must* be shown in APAR currency rather than transfer currency.
oops the patch may not apply cleanly because it depends on another unrelated commit. my github maint will always have latest WIP.
Created attachment 373517 [details] candidate patch Candidate patch. I can merge this and await further feedback; there's plenty of time until 3.9... INV/PMT txns both derive DESCRIPTION from invoice-posting / payment txn's ASSET/LIABILITY splits->memos This seems to produce consistent results in all circumstances. If a payment txn is modified so that it is funded from >1 asset/liability split, it will concat unique transfer splits memos. eg. payment transaction Asset:USDBank -$100 memo "100 USD" Asset:GBPBank -$100 memo "70 GBP" A/Payable +$200 memo "orig-memo" The Payment Description will be "100 USD, 70 GBP"
I've pushed commit to fix Invoice->Desc and Payment->Desc in both RHS & LHS. Note LHS&RHS use slightly different approaches because LHS payments are merged together whereas RHS payments are shown separately. This approach seems to produce useful Desc in all of my tests. Further testing would be welcome.
Descriptions look good on my system. I might prefer concatenating multiple descriptions using a line break rather than a comment, but that's just personal preference. Feel free to follow your own judgement here. (In reply to Christopher Lam from comment #58) > * showing full vs partial contributory amounts not currently implemented > yet. Will take some hacking. If we want to show contributory amounts then it > *must* be shown in APAR currency rather than transfer currency. Yes, that's the only way to do it.
(In reply to Mike Alexander from comment #57) > I'll send the file. I did not have John's commit d51b4d7 in the build that > got the crash. Mike, your datafile is a nugget for performance testing! With it I've confirmed that the (sort-and-delete-duplicates ...) approach is a good one for finding owners with business activity from all splits->owners. sort-and-delete-duplicates: new-aging.scm runs in 2.7s delete-duplicates: new-aging.scm runs in 9.8s