GNOME Bugzilla – Bug 452496
Dirtying a split does not dirty the parent txn or book
Last modified: 2018-06-29 21:41:07 UTC
Reproduce: 1) open attachment 2 [details]) open account a 3) change withdrawal of the txn to 220, commit txn ; or 3) press split button, change memo of a split, commit txn the txn is not marked dirty. The same holds for the book.
Created attachment 90919 [details] Example file
I just verified that in 2.0.5 and in 1.8.12, the book is correctly marked dirty. However, this bug is confirmed in trunk@r15678 and r15316. If I get a chance, I'll look deeper into this tonight. Since this could result in data loss, I'm bumping severity. Oh and BTW, I don't think changing the split is _supposed_ to dirty the transaction, but it is supposed to dirty the book.
So, it seems that this bug was introduced in r14614. I'm trying to figure out why, but it's not making much sense to me right now. I'm concerned that the change in r14614 revealed some bug somewhere else.
The other thing that's rather strange about this is that I can't imagine how the presence or absence of the code in r14614 can affect the dirtying behavior in the register. This code is not (supposed to be) subtle. If the code is really needed, I can think of all sorts of bad things that would happen if it was removed - but not this. Either I really don't understand this code, or there's something very subtle and probably incorrect lurking somewhere.
More observations: 1) Transactions that have been created in the register (as opposed to just loaded from file) don't exhibit this behavior. 2) Forcing the splits to be dirtied (with qof_instance_set_dirty()) an extra time inside add_transaction_local() "fixes" the bug.
As near as I can make out... the code before r14614 was leaving _every_ Transaction in the dirtied state upon loading. (I'm not entirely clear on why this didn't also make the collections dirty, but it must not have, since the book was not considered dirty.) Then, when the register would change a Split, the the Transaction would get committed. The split setter didn't dirty the transaction (and wasn't supposed to) , but since the transaction had already been left dirty (even though the editlevel was 0) it dirtied the transaction collection, and hence, the book. However, after 14614, the transactions were left in a clean state after loading. But new transactions were created in a dirty state. Therefore, when just-loaded transactions were committed after changing a split, they looked clean, so they didn't dirty the book. However, that commit process _did_ dirty the transaction. So, subsequent edits of splits in that transaction _would_ dirty the book, as would edits to splits in new transactions. This is probably why nobody noticed this bug for 1 year. Incidentally, I think the real bug is in r13958. Or rather, the fix in r13958 is an incorrect fix for that problem. Still investigating...
Created attachment 91004 [details] [review] Dirty collection instead of transaction when committing a split. I think this patch might fix the bug, but I haven't tested very thoroughly. Also, I haven't looked at this code much for about a year, so please give this a test before committing.
Committed to trunk @ 16399.
Oh thanks, Chris & Josh! I have forgotten that one completely ;-)
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=452496. Please update any external references or bookmarks.