After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 754209 - Bills can be posted from "find bill" search results even if bill is already posted and results in extra $ posted to A/P
Bills can be posted from "find bill" search results even if bill is already p...
Status: RESOLVED FIXED
Product: GnuCash
Classification: Other
Component: Business
git-maint
Other All
: Normal critical
: ---
Assigned To: Mike Evans
gnucash-core-maint
Depends on:
Blocks:
 
 
Reported: 2015-08-27 22:49 UTC by info
Modified: 2018-06-29 23:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Prevent posting already posted invoices (1014 bytes, patch)
2016-03-15 16:04 UTC, Mike Evans
reviewed Details | Review

Description info 2015-08-27 22:49:32 UTC
The "Post" button in the "Find Bill" search results dialog box allows posting of bills that are already posted. The duplicate posts then show up in the General Ledger on the A/P account with the original bill amount and a transaction type "?". They are then impossible to delete from the interface (I thought about trying to delete in the XML but decided not to): the error message is "Cannot modify or delete this transaction. This transaction is marked read-only with the comment: 'Generated from an invoice. Try unposting the invoice.'" This error message persists even after unposting the original bill. 

From a UX perspective the best fix for this is for the "post" button to be unavailable when the selected transaction in the search results has already been posted. If that's not a good solution for any reason, the next best would be to remove the "post" button from the search results dialog box altogether--it's preferable to click "view/edit bill" and post from there if necessary than to be able to introduce these errors into the data.
Comment 1 Mike Evans 2016-03-15 16:04:01 UTC
Created attachment 324019 [details] [review]
Prevent posting already posted invoices

This is just a suggestion and would silently prevent posting of already posted invoices. 

As it stands it emits a PINFO statement only. I'm not sure whether an info dialog should pop up but this may be annoying if attempting to post multiple invoices.
It also runs after the dialog asking for date etc., which may also not be ideal.

Then again, should there even be a post button in the search dialog?
Comment 2 Mike Evans 2016-03-15 19:32:31 UTC
There probably should be a check in gncInvoice.c and/or dialog-invoice.c for is_already_posted.
Comment 3 Geert Janssens 2016-03-16 09:21:51 UTC
Mike,

I agree these should be a check in gncInvoice.c to prevent an invoice from being posted again if it's already posted. That should be our lowest level security check.

In addition I would make the gui more aware of this as well. Your patch is a first good attempt.

I would however communicate slightly earlier with the user and insert a test in multi_post_invoice_cb right after the test for an empty invoice list and before asking for a date.

I would insert another g_list_for_each call in which you test each invoice whether it's already posted. If any invoice is, pop up a message that you can't continue because at least one selected invoice was posted already.

P.S. I'm raising the severity of this bug, because it can be the source of data corruption. It generates bogus transactions the user can't delete any more. I have recently had a conversation on the list with at least one user who ran into this.

We may need to add a scrub function in check & repair as well to fix the damage already caused. If you don't feel like that, I may have a stab at that later when I find some time.
Comment 4 Geert Janssens 2016-03-16 09:23:52 UTC
By the way, there's also the function gncInvoiceIsPosted you can use instead of testing for a posted account. Makes the test slightly easier.
Comment 5 Mike Evans 2016-03-16 10:24:48 UTC
Cheers Geert I didn't see gncInvoiceIsPosted .

I hadn't thought about a scrub function, for the moment I'll just concentrate on preventing this happening.
Comment 6 Mike Evans 2016-03-16 10:27:51 UTC
Comment on attachment 324019 [details] [review]
Prevent posting already posted invoices

First draft, works but not ideal so marking as obsolete.
Comment 7 Mike Evans 2016-03-16 14:50:04 UTC
Pushed a fix to maint commit 8117a7c17f5c

Geert, do you want to file a new bug for a scrub function or keep this open (possibly re-titled)?
Comment 8 Geert Janssens 2016-03-19 13:24:50 UTC
I have just pushed some changes in our Check & Repair code that will detect the bad transactions as a result of the double post. Check & Repair will lift the read-only restriction from these transactions but will not delete them. It is left to the user to go in and delete them together with any possible balance correcting transactions the user had already put in place.
The help the user identify the bad transactions the split memo will be set to

> Please delete this transaction. Explanation at http://wiki.gnucash.org/wiki/Business_Features_Issues#Double_Posting
(to be translated for future gnucash versions in all languages we support).

The wiki page the memo refers to gives additional background (including a link to this bug report) and guides the user in cleaning up.

Thanks for reporting this bug and thanks Mike for fixing it in the first place.
Comment 9 John Ralls 2018-06-29 23:42:33 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=754209. Please update any external references or bookmarks.