GNOME Bugzilla – Bug 727664
Patch: Customer/Owner Report with Sale and Tax amount (owner-report.scm)
Last modified: 2018-06-29 23:29:23 UTC
Created attachment 273626 [details] [review] Report with sale and tax columns I have created a patch to add support for "Sale" and "Tax" columns in report. (Scheme file = owner-report.scm) Columns are optional and set to false (#f) by default. Patch also changes formatting of "Period Totals", slightly.
Created attachment 273636 [details] [review] Report with sale and tax columns
Thank your for your contribution. The changes are good, but as with your other patch I would like you to split this up in two patches: - a first patch that fixes the Period Totals" - a second patch that builds on top of this and creates the Sale and Tax columns. My primary reason to ask for this is that your Period Totals improvement should go in the 2.6 series. I consider the current handling of the period totals a bug. The newly added columns are a new feature though and should only be applied to master. Lastly, there's a small issue to fix still as well: when you open a Vendor report and show the new Sale and Tax columns, you'll note that Sale and Tax values are negative while the invoice totals (in the credit column) are positive. So you will need to insert an additional sign reversal based on the owner type (vendor vs customer) or document type (bill vs invoice).
As with your other report, I'm fine with a pull request on Github (provided the commit messages are well formatted).
1st patch: https://github.com/amishxda/gnucash/commit/e168257371a0d7e481bf8c547885262012589d6f 2nd patch https://github.com/amishxda/gnucash/commit/70e1bcbe917e5ad263135c492119bb8fef91e1c7 About negative sale and tax value. I dont have any vendor in my Gnucash so could not see what you said: But in my 2nd patch, there is this line: (387 to 390) (if reverse? (begin (set! tax (gnc-numeric-neg tax)) (set! sale (gnc-numeric-neg sale)))) May be removing those will solve the problem.(Hopefully!) Also see bug #727647 filed by me (confirmed by you). Since this report now accesses invoices, once this is pushed to stable, people might start complaining about that bug.
Any update? Is this implemented?
Thanks for reminding me. I got sidetracked by other issues... I have pushed your period totals patch to maint and both to master as well. Regarding the sign issue: reverse? is the wrong test to use here. It checks whether the transaction has a positive or negative effect on the balance. Bills have a negative effect on the balance, invoices have a positive effect. However the report is designed to always show bill and invoice amounts as positive. So transactions linked to a bill should be reversed and transactions linked to an invoice shouldn't. That was the situation before credit notes were implemented. With credit notes, credit note amount should be shown as negative. But here the reverse? test is working on the wrong classification (bill vs invoice instead of bill/invoice vs credit note). There is another function that can be used to check if the document is a bill/invoice or a credit note: gncInvoiceGetIsCreditNote. I have submitted a follow-up commit to correct this (master only as that's where the sales and tax columns were added). And yet another one to use the proper type name for credit notes as well (both master and maint). That should sufficiently fix the owner report issues to be able to close this report. Bug 727647 is still open but can be tracked separately. Thank you for your contribution !
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=727664. Please update any external references or bookmarks.