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 700804 - Manually change ordering of Transactions
Manually change ordering of Transactions
Status: RESOLVED FIXED
Product: GnuCash
Classification: Other
Component: Register
git-master
Other All
: Normal enhancement
: ---
Assigned To: Christian Stimming
Geert Janssens
Depends on:
Blocks:
 
 
Reported: 2013-05-21 19:16 UTC by Christian Stimming
Modified: 2018-06-29 23:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add up/down buttons for ordering of transactions (14.35 KB, patch)
2013-06-09 21:45 UTC, Christian Stimming
none Details | Review
Implements the up/down buttons in the register2 (18.32 KB, patch)
2013-06-13 20:29 UTC, Christian Stimming
needs-work Details | Review
second patch on top of the first one (6.32 KB, patch)
2013-06-14 20:50 UTC, Christian Stimming
needs-work Details | Review
Third patch on top of the first two (2.69 KB, patch)
2013-06-20 20:29 UTC, Christian Stimming
none Details | Review
One commit for all three patches above, based on trunk r23060 (20.22 KB, patch)
2013-06-20 20:49 UTC, Christian Stimming
none Details | Review

Description Christian Stimming 2013-05-21 19:16:13 UTC
See http://gnucash.uservoice.com/suggestions/1602507

It would be really helpful if GnuCash had the facility to move transactions before or after a particular transaction of the same day, i.e. to change the ordering manually by some buttons or similar. If time could be mentioned in seconds then we could do that to rearrange the transactions.
But easier would be to have Up and Down arrows to rearrange transactions by just selecting on the arrow and clicking on up or down.
If the moved transaction reaches the beginning or end of the set of the same day's transactions, the transaction date of that transaction could be incremented or decremented OR could stop right there OR could cycle from the bottom top or top bottom depending on which arrow button you were clicking on to move the transaction.
Comment 1 Geert Janssens 2013-05-25 12:46:43 UTC
With the new register code, it would even be possible to implement it using drag-and-drop. Some kind of anchor/handle would have to be added to each row that could be used to drag the whole row to another location.
Comment 2 Christian Stimming 2013-06-08 21:34:29 UTC
I'm about to implement this request and will attach a patch shortly.
Comment 3 Christian Stimming 2013-06-09 21:45:26 UTC
Created attachment 246369 [details] [review]
Add up/down buttons for ordering of transactions

The buttons are added in the register2. They work for moving txn up/down (but not splits). However, this applies only to txn that have the same date and the same num.

Questions: What should the buttons do when the neighboring txn have different date or, if the same date, different num? Just do nothing? Show a short status line text for some seconds?

Also, for the completion of the bounty, should register1 have those buttons as well? Dealing with the VirtualCellLocation there turned out to be a bit more involved than I thought. Surely the first priority is to have this feature available in the register2.
Comment 4 Elias K Gardner 2013-06-10 02:08:37 UTC
I think date should change automatically with some clear nondisruptive notification.
Comment 5 Christian Stimming 2013-06-10 19:42:37 UTC
I have a different idea: Better make the respective button for up/down inactive most of the time and active only in those cases where it would actually do something, i.e. when there are neighbouring transaction that have identical date and number.

But first I'd like to hear some feedback on whether the register2 is considered sufficient or whether this should be implemented in the register1 as well.
Comment 6 Christian Stimming 2013-06-13 20:29:28 UTC
Created attachment 246768 [details] [review]
Implements the up/down buttons in the register2

This provides up/down buttons for transactions in the register2. The buttons are active only when the ordering can indeed be changed, otherwise they are inactive.
Comment 7 Christian Stimming 2013-06-13 20:30:54 UTC
@Evaluators: I'd kindly like to request bounty program evaluation...
Comment 8 Geert Janssens 2013-06-14 16:36:49 UTC
I followed your progress, but didn't have time before now to evaluate it. Thanks for working on this.

I have pondered your question for some time whether the old register should be supported a well. The answer largely depends on how confident we are that the new register code will effectively replace the old code. I think this is very likely (though the new register code still needs quite some optimisation and tweaking). So as far as I'm concerned it's sufficient to implement this for the register2 code only.

Regarding the implementation: I have downloaded your patch and tested it. It mostly works as advertised and seems to be cleanly written. Thanks!

It doesn't implement drag-and-drop as I suggested but that's fine. Thinking about it more, drag-and-drop would not easily handle the constraint of only moving when the adjacent line has the same date/number. So the arrow implementation is probably simpler.

I do have these minor nits:
- in gnc-tree-control-split-reg.c:1426 you test if an attempt to move up actually did move. If not you set an info message and exit. A few lines down you attempt a move down, but don't test if the attempt was successful. Is this intentional ? It seems to me the current code won't clean up the allocated memory if the move down fails.
- There is a comment in the same code a little bit down that you "cowardly refuse to move" if the adjacent dates are not the same potentially due to a different sort order. That's a good safety net. I would however like to see this constraint visually as well. For example, if I currently select a transaction that is movable in normal sort order and then change the sort order the arrow button is still active, suggesting I can change this order. But when clicking this button nothing happens. This is confusing. So can you add some code that disables the up and down arrows when the register is using any sort order except on date ?
- Last remark is mostly a question: I notice that I can move reconciled transactions up/down without a warning. Is that ok ? Personally I don't want it to be easy to reorder reconciled transactions for my bank statements without getting a warning (which can potentially be disabled). But maybe you have a different view.
Comment 9 Christian Stimming 2013-06-14 20:50:23 UTC
Created attachment 246856 [details] [review]
second patch on top of the first one

Here's what I came up with as responses to your question:

- In a different sort order, the buttons just cannot "do the right thing". Hence,
I've disabled them in any sort order except the by-date ordering.

- For reconciled splits, user will be asked, which is what the other edit functions do as well. For frozen splits and book-closing txn, the buttons are disabled altogether.

- As for the move up with sanity check and move down and no check: That's a weird difference in the gtk_tree_path_prev() and path_next() function. Only the prev() function can fail, the other cannot (but the iter lookup later will fail). I've added a remark for that.
Comment 10 Christian Stimming 2013-06-14 20:51:16 UTC
Once the code is accepted, of course I can attach this as a single patch (or would commit this as one single commit.) Two patches only for Geert's convenience.
Comment 11 Geert Janssens 2013-06-15 09:26:31 UTC
(In reply to comment #9)
> Created an attachment (id=246856) [details] [review]
> second patch on top of the first one
> 
> Here's what I came up with as responses to your question:
> 
> - In a different sort order, the buttons just cannot "do the right thing".
> Hence,
> I've disabled them in any sort order except the by-date ordering.
> 
I like that much better, thank you. I came across one last unhandled situation:
starting from a register in standard by-date ordering, I clicked on a transaction split that could be moved up/down. As a result, the appropriate arrow got enabled. Next I changed the sort order to a not by-date ordering (say description based ordering). This doesn't disable the arrow button. Clicking on another transaction that could be moved in by-date ordering now (correctly) does disable both buttons. If you at this point change the sort order to by-date ordering, the buttons are not enabled again until you click another transaction.
So IMO the callback that gets triggered when the sort order is changed should also enable/disable the arrows appropriately.

> - For reconciled splits, user will be asked, which is what the other edit
> functions do as well. For frozen splits and book-closing txn, the buttons are
> disabled altogether.
> 
The warning when trying to move a reconciled split works well. Thank you. I don't know what kind of action in GnuCash would create a frozen split, so I'm not sure how to verify this. From reading your code such split will not be moved, but I'm not sure if the arrow button will be disabled as well when such a split is clicked on.

> - As for the move up with sanity check and move down and no check: That's a
> weird difference in the gtk_tree_path_prev() and path_next() function. Only the
> prev() function can fail, the other cannot (but the iter lookup later will
> fail). I've added a remark for that.

Ok, thank you for clearing that up.
Comment 12 Geert Janssens 2013-06-15 09:28:50 UTC
Comment on attachment 246856 [details] [review]
second patch on top of the first one

As per the Bounty Program wiki, I have set both patches to needs-work. The comments on the first patch have been dealt with in the second patch. I'm not sure if I should change the status of the first patch to accept-commit-now then or not ? Anyway, that's probably only an administrative detail...
Comment 13 Christian Stimming 2013-06-20 20:29:57 UTC
Created attachment 247386 [details] [review]
Third patch on top of the first two

With this patch, the up/down buttons are updated immediately after changing the sort order as well.

(Just give your comment in text. No need to set the patches to any specific state. I'll commit everything in one single commit once this is fully accepted, anyway.)
Comment 14 Christian Stimming 2013-06-20 20:33:49 UTC
(In reply to comment #11)
> > - For reconciled splits, user will be asked, which is what the other edit
> > functions do as well. For frozen splits and book-closing txn, the buttons are
> > disabled altogether.
> 
> The warning when trying to move a reconciled split works well. Thank you. I
> don't know what kind of action in GnuCash would create a frozen split, so I'm
> not sure how to verify this. From reading your code such split will not be
> moved, but I'm not sure if the arrow button will be disabled as well when such
> a split is clicked on.

The arrow buttons will be disabled, too, because the check for FREC and returning FALSE is done right before the "if (really_do_it)" clause. Hence, in the query mode of the function (where really_do_it == FALSE) it will already return FALSE, making the buttons disabled.

I'm also not sure whether any function is available in the UI to set a split to "frozen". But as it's one of the choices of the reconcile status, I thought I better deal with it here in a meaningful way.
Comment 15 Christian Stimming 2013-06-20 20:49:04 UTC
Created attachment 247387 [details] [review]
One commit for all three patches above, based on trunk r23060

(trunk r23060 is needed because I've moved part of the "Third patch" already in one refactoring commit that was unrelated to this feature)
Comment 16 Geert Janssens 2013-06-21 12:26:32 UTC
The last patch works like a charm. I consider this complete.

Thank you very much for this work !
Comment 17 Christian Stimming 2013-06-23 07:59:26 UTC
r23061, thanks everyone!
Comment 18 John Ralls 2018-06-29 23:16:19 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=700804. Please update any external references or bookmarks.