Version: 0.9.3-CVS (using KDE 3.5.10) Compiler: gcc version 4.3.0 20080428 OS: Linux Installed from: Fedora RPMs Steps to reproduce (Using CVS build from 2009-03-18): 1/ Mark an account as closed. 2/ Go into ledger for account. 3/ Select a transaction. Expected Result 1/ Delete facility is not available. Actual Result 1/ Delete is enabled. If you continue with deleting process: 1/ Click on Delete button (or Delete from context menu). 2/ Answer Yes to confirmation dialog "Do you really want to delete this transaction?". 3/ An error dialog appears entitled "Sorry - KMyMoney" with message text of "Error" and "Details" and "OK" buttons. 4/ "Details" button gives: "Unable to delete transaction(s): Cannot remove transaction that references a closed account., thrown in mymoneyfile.cpp:398" Couple of side notes: 1/ Trying to delete a transaction that references a closed account from in an open account also gives the same error, which it should do, but it probably needs to be made more user friendly. 2/ Should Mark/Move Transaction also be disabled on closed accounts? Both work (that I can see) but should they?
Possible fix: Index: kmymoney2.cpp =================================================================== RCS file: /cvsroot/kmymoney2/kmymoney2/kmymoney2/kmymoney2.cpp,v retrieving revision 1.377 diff -p -u -d -r1.377 kmymoney2.cpp --- kmymoney2.cpp 8 Mar 2009 07:50:54 -0000 1.377 +++ kmymoney2.cpp 28 Mar 2009 13:32:51 -0000 @@ -5115,7 +5115,7 @@ void KMyMoney2App::slotUpdateActions(voi // can select at least a single transaction action("transaction_select_all")->setEnabled(true); if(!m_selectedTransactions.isEmpty()) { - action("transaction_delete")->setEnabled(m_selectedTransactions.count() != 0); + action("transaction_delete")->setEnabled((m_selectedTransactions.count() != 0) && !m_selectedAccount.isClosed()); if(!m_transactionEditor) { tooltip = i18n("Duplicate the current selected transactions");
Possible fix for disabling other menu items too (also fixes issue with mark as menu container being enabled when no transactions selected): Index: kmymoney2.cpp =================================================================== RCS file: /cvsroot/kmymoney2/kmymoney2/kmymoney2/kmymoney2.cpp,v retrieving revision 1.377 diff -p -u -d -r1.377 kmymoney2.cpp --- kmymoney2.cpp 8 Mar 2009 07:50:54 -0000 1.377 +++ kmymoney2.cpp 28 Mar 2009 14:28:58 -0000 @@ -5111,11 +5111,19 @@ void KMyMoney2App::slotUpdateActions(voi if(w) w->setEnabled(false); + w = factory()->container("transaction_mark_menu", this); + if(w) + w->setEnabled(false); + + w = factory()->container("transaction_context_mark_menu", this); + if(w) + w->setEnabled(false); + // FIXME for now it's always on, but we should only allow it, if we // can select at least a single transaction action("transaction_select_all")->setEnabled(true); if(!m_selectedTransactions.isEmpty()) { - action("transaction_delete")->setEnabled(m_selectedTransactions.count() != 0); + action("transaction_delete")->setEnabled((m_selectedTransactions.count() != 0) && !m_selectedAccount.isClosed()); if(!m_transactionEditor) { tooltip = i18n("Duplicate the current selected transactions"); @@ -5133,15 +5141,25 @@ void KMyMoney2App::slotUpdateActions(voi } action("transaction_edit")->setToolTip(tooltip); - w = factory()->container("transaction_move_menu", this); - if(w) - w->setEnabled(true); + if(!m_selectedAccount.isClosed()) { + w = factory()->container("transaction_move_menu", this); + if(w) + w->setEnabled(true); - // Allow marking the transaction if at least one is selected - action("transaction_mark_cleared")->setEnabled(true); - action("transaction_mark_reconciled")->setEnabled(true); - action("transaction_mark_notreconciled")->setEnabled(true); - action("transaction_mark_toggle")->setEnabled(true); + w = factory()->container("transaction_mark_menu", this); + if(w) + w->setEnabled(true); + + w = factory()->container("transaction_context_mark_menu", this); + if(w) + w->setEnabled(true); + + // Allow marking the transaction if at least one is selected + action("transaction_mark_cleared")->setEnabled(true); + action("transaction_mark_reconciled")->setEnabled(true); + action("transaction_mark_notreconciled")->setEnabled(true); + action("transaction_mark_toggle")->setEnabled(true); + } if(!m_accountGoto.isEmpty()) action("transaction_goto_account")->setEnabled(true); Index: kmymoney2ui.rc =================================================================== RCS file: /cvsroot/kmymoney2/kmymoney2/kmymoney2/kmymoney2ui.rc,v retrieving revision 1.81 diff -p -u -d -r1.81 kmymoney2ui.rc --- kmymoney2ui.rc 23 Feb 2009 19:41:25 -0000 1.81 +++ kmymoney2ui.rc 28 Mar 2009 14:28:58 -0000 @@ -74,7 +74,7 @@ <Action name="transaction_editsplits" /> <Action name="transaction_delete" /> <Action name="transaction_duplicate" /> - <Menu> + <Menu name="transaction_mark_menu"> <text>Mark transaction as...</text> <title>Mark transaction</title> <Action name="transaction_mark_notreconciled"/> @@ -212,7 +212,7 @@ <title>Select account</title> <ActionList name="transaction_move" /> </Menu> - <Menu> + <Menu name="transaction_context_mark_menu"> <text>Mark transaction as...</text> <title>Mark transaction</title> <Action name="transaction_mark_notreconciled"/>
Good catch. I just added a two methods to the MyMoneyFile object: referencesClosedAccount(MyMoneyTransaction) and referencesClosedAccount(MyMoneySplit) which could be used for those checks. Please update from CVS to get them. Of course, we now need to call them from within the application code. Your assumptions about the Move transaction mentioned are correct when the transaction should be moved out of the closed account. In other cases, this does not harm as the split referencing the closed account remains unchanged (value-wise). Since marking a transaction (even in the closed account) does not affect the closed account in any way, I don't see a problem with having it available. We just need to make sure, that the shares/value of a split referencing a closed account is not changed. This might become tricky in the split transaction editor though. If you want, please supply additional patches.
I wish I could add attachments on here, so sorry for the extra spam. This patch: * Disables delete button, delete context menuitem and delete transaction menuitem on closed accounts. * Disables "Move to" context menuitem on closed accounts. * Disables "Mark as" context menuitem and "Mark as" transaction menuitem on all accounts where no transactions are selected. I will look at using the new methods when looking at editing splits on open accounts that reference closed accounts and deleting transactions from open accounts that reference closed accounts - but that is probably outside scope of this bug so spun off into bug 188346. Index: kmymoney2.cpp =================================================================== RCS file: /cvsroot/kmymoney2/kmymoney2/kmymoney2/kmymoney2.cpp,v retrieving revision 1.378 diff -p -u -d -r1.378 kmymoney2.cpp --- kmymoney2.cpp 25 Mar 2009 15:17:00 -0000 1.378 +++ kmymoney2.cpp 28 Mar 2009 15:49:47 -0000 @@ -5109,11 +5109,19 @@ void KMyMoney2App::slotUpdateActions(voi if(w) w->setEnabled(false); + w = factory()->container("transaction_mark_menu", this); + if(w) + w->setEnabled(false); + + w = factory()->container("transaction_context_mark_menu", this); + if(w) + w->setEnabled(false); + // FIXME for now it's always on, but we should only allow it, if we // can select at least a single transaction action("transaction_select_all")->setEnabled(true); if(!m_selectedTransactions.isEmpty()) { - action("transaction_delete")->setEnabled(m_selectedTransactions.count() != 0); + action("transaction_delete")->setEnabled((m_selectedTransactions.count() != 0) && !m_selectedAccount.isClosed()); if(!m_transactionEditor) { tooltip = i18n("Duplicate the current selected transactions"); @@ -5131,10 +5139,20 @@ void KMyMoney2App::slotUpdateActions(voi } action("transaction_edit")->setToolTip(tooltip); - w = factory()->container("transaction_move_menu", this); + if(!m_selectedAccount.isClosed()) { + w = factory()->container("transaction_move_menu", this); + if(w) + w->setEnabled(true); + } + + w = factory()->container("transaction_mark_menu", this); if(w) w->setEnabled(true); + w = factory()->container("transaction_context_mark_menu", this); + if(w) + w->setEnabled(true); + // Allow marking the transaction if at least one is selected action("transaction_mark_cleared")->setEnabled(true); action("transaction_mark_reconciled")->setEnabled(true); Index: kmymoney2ui.rc =================================================================== RCS file: /cvsroot/kmymoney2/kmymoney2/kmymoney2/kmymoney2ui.rc,v retrieving revision 1.81 diff -p -u -d -r1.81 kmymoney2ui.rc --- kmymoney2ui.rc 23 Feb 2009 19:41:25 -0000 1.81 +++ kmymoney2ui.rc 28 Mar 2009 15:49:47 -0000 @@ -74,7 +74,7 @@ <Action name="transaction_editsplits" /> <Action name="transaction_delete" /> <Action name="transaction_duplicate" /> - <Menu> + <Menu name="transaction_mark_menu"> <text>Mark transaction as...</text> <title>Mark transaction</title> <Action name="transaction_mark_notreconciled"/> @@ -212,7 +212,7 @@ <title>Select account</title> <ActionList name="transaction_move" /> </Menu> - <Menu> + <Menu name="transaction_context_mark_menu"> <text>Mark transaction as...</text> <title>Mark transaction</title> <Action name="transaction_mark_notreconciled"/>
Fixed in CVS HEAD