Bug 797249 - Cutting home account causes transaction to disappear #517
Conversation
|
Can one still REPLACE the anchor-split with a different account? I think that is a very important feature to keep. For example, if I put a transaction onto the wrong credit card, I think it's very important to be able to go into that credit card account and change the anchor split to the correct credit card account without having to jump to the other account. |
|
To be consistent the "delete" toolbar button should also be disabled in this case, and then we can get rid of the dialog. |
|
Yes you can still replace the anchor split with a different one as before, you just can not delete or cut it and the toolbar 'delete' button does follow the menu settings. |
The General Journal isn't associated with an account so what's the anchor split? |
|
I certainly prefer pro-active behavior (that is prevent a user from doing the wrong thing) over reactive behavior (warn the user when s/he does something wrong). In this case it could be debated whether it's really the case of doing something wrong or just potentially unexpected. However I go with your decision. I'm not sure though why you would disable the "Paste Split" action. Will a paste split replace the currently selected split, or just add a split to the current transaction ? |
| gboolean disable = FALSE; | ||
|
|
||
| if (split == gnc_split_register_get_current_trans_split (reg, NULL)) | ||
| disable = TRUE; |
There was a problem hiding this comment.
How will this logic behave if the current transaction has more than one split in the current account register ?
There was a problem hiding this comment.
If the transaction has more than one split in the current account register you are able to delete one of them but not the other based on the order loaded in the register from the split list used to create the register.
There was a problem hiding this comment.
That's what I suspected and what I think is suboptimal. I think if there is more than one split in the current register the user should either be prevented from deleting any of those splits or should be allowed to delete any except of the last one (which may not be the anchoring split).
Remember our goal is to avoid the transaction from unexpectedly disappearing from the current register if the user deletes/cuts a split.
Oh but wait, if a transaction has more than one split in a register it will appear twice in that register, right ? So each of these two appearances has its own anchor split and removing the anchor split would suddenly drop that instance of the transaction from the register view... Darn complicated.
Regardless I did test this and if you don't know why the buttons are greyed out it's indeed pretty confusing you can delete one and not the other split.
I was thinking of dynamically updating tooltips for the menu items and toolbar buttons. That way a disabled button could have a tooltip that explains why it's disabled (like "Can't delete the anchoring split of a transaction). But currently tooltips for greyed out menu items are not shown in the status bar. Do you think we can have this or is this a Gtk limitation ?
There was a problem hiding this comment.
A transaction with two splits in the current account appears twice in Basic View, but one obviously can't edit the splits. In split view the transaction displays only once with all of its splits.
There was a problem hiding this comment.
I tested the Auto-split ledger, not the Transaction journal mode. Transaction journal mode behaves as you describe indeed.
Unfortunately that means the transaction journal mode does expose the poor behavior I tried to explain in my previous comment. Auto-split ledger is confusing in a different way as it may seem arbitrary which split you can delete in which instance of the same transaction.
I fear this to be a recipe for an pile of user support questions on what we developers perceive as perfectly logical from a technical point of view but will be non-obvious to the end-user... At least assuming many users actually use the cut/copy/delete split functionality.
There was a problem hiding this comment.
We could keep using the message box and block method instead of disabling the actions. That at least explains what's going on.
There was a problem hiding this comment.
Maybe then a combination of the two, grey out the options for when the transaction is read only / void and then add the warning to the cut / paste operation and also remove it from the General Journal.
From what I ascertain, what is reported as the anchor split is the first split for that transaction in the split list used to load the General Journal and when using the current version you can happily 'Cut' the reported anchor split and then amend the blank split so blocking the 'Cut' and 'Delete' should only apply to a normal register. |
Using an unmodified version, if you copy a split from 'Savings Account', go to the 'Checking Account' and paste on the anchor split, you get a warning about overwriting an existing split and if you proceed the transaction disappears form the 'Checking Account' and is now in the 'Savings Account' |
|
Ok, I wasn't aware of this. And I propose to change that behaviour as well. To my mind it makes more sense a split is added to a transaction than the current split is overwritten (luckily there's a warning). |
I do not think this needs changing as there is a warning, there is nothing stopping you from pasting on to the blank split which will probably create an Imbalance split but that is normal until you make the necessary changes. |
Oh, my suggestion was not "because it's not possible", it's "because I think the current default is backwards" :) I understood you can paste onto the blank split, I just think it makes more sense to add a pasted split rather than have it replace an existing one. But that's just opinion and not directly related to this PR so don't block it for that reason. |
|
I have changed this to be a combination of the two... The second part is to add the warning to the cut option for anchor splits and reconciled ones. This could possibly be combined with delete splits if the messages were changed to a form of 'Removal'. Prevent pasting on an anchor split with message really for the same reasons as cutting/deleting. The change for cancelling a split paste seems the best option I can think off, the down side is that if the transaction is already being edited you can not paste but I thought that was better than not being able to cancel it. The last fix is one I have been looking for ages, making the blank split read only when the transaction is read only. I await your feedback. |
|
Does any body have thoughts on the new changes? |
|
@Bob-IT The narrative seems on target. |
|
I have changed the warning message for the cut operations to use 'remove' with the intention to combine the delete and cut code in master so both will have the same message based on 'remove', titles and buttons will still show cut or delete as appropriate. I will push this to maint on Tuesday if no one has any concerns. |
transactions are marked read only
In the General Journal there is no anchor split so allow all splits to be deleted.
Add some test for when cutting splits from transactions that prevent the cutting of the anchor split and warn when the split is a reconciled one or when cutting the transaction that it contains reconciled splits.
Present a dialogue advising that you can not paste a split on top of the anchoring split.
Currently when you paste a split you can not cancel the changes as they are already committed. By opening the transaction for editing before the split paste the cancel option is now available.
If the transactions are read only or voided and selected for editing a blank split is added that allows you to change the transaction so in these cases make the blank split read only.
I had a look at this one and I started off thinking about adding the warning dialogue but then thought why not disable the menu options when not appropriate which is probably the correct option.
So this commit does that, disables the 'Cut' option when the cursor is on the anchor split along with 'Delete' and 'Paste'. Also added tests for the transaction being read only with appropriate options being disabled.
The only thing I am concerned with is if having some options disabled on some splits will raise questions from users.