Bug 276322 - Unwanted fields show in ledger for investment.
Summary: Unwanted fields show in ledger for investment.
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: general (show other bugs)
Version: SVN
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: allan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-23 12:46 UTC by allan
Modified: 2014-11-09 10:48 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.7.2
Sentry Crash Report:


Attachments
Patch to reinstate interest field in Sell activities. (4.61 KB, patch)
2013-12-02 14:13 UTC, allan
Details
Reinstate Interest category for Buy activities. Also, Fix misbehaviour of Interest widget (flickering) in several activity types. (5.37 KB, patch)
2014-01-15 13:08 UTC, allan
Details

Note You need to log in before you can comment on or make changes to this bug.
Description allan 2011-06-23 12:46:22 UTC
Version:           SVN trunk (using KDE 4.6.0) 
OS:                Linux

The problem shows best if the transaction form is enabled, but also is evident without the form.
If an existing cash dividend transaction is selected, the form shows widgets for Date, Amount, Total and Status, which is what is expected.
If that transaction is then opened for editing, additional widgets show for Security, Quantity and Price, which aren't expected for a cash dividend.
If, while the transaction is open for editing, 'Dividend' is reselected, the additional fields disappear.  Similar behaviour occurs for Buy transactions.


Reproducible: Always

Steps to Reproduce:
1) Enable the Ledger form.
2) Open a file containing Dividend transaction.
3) Select a Dividend transaction.
4) Only widgets for Date, Amount, Total and Status appear.
5) Open that transaction for editing.
6) Additional widgets show for Security, Quantity and Price, which aren't expected.
7) Reselect Dividend from the combo.
8) The additional fields disappear.


Actual Results:  
As above.

Expected Results:  
The additional widgets for Security, Quantity and Price should not appear.
Comment 1 allan 2011-06-26 10:34:52 UTC
I've been looking at this, but am stuck:-(

I traced where it is going wrong, but, to me, the code looks OK.  In investtransactioneditor.cpp,  InvestTransactionEditor::slotUpdateActivity(), circa line 835, the following code appears -

  dynwidgets << "asset-account" << "interest-amount" << "fee-amount" <<    "shares" << "price" << "total";

  for (it_s = dynwidgets.constBegin(); it_s != dynwidgets.constEnd(); ++it_s) {
    QWidget* w = haveWidget(*it_s);
    if (w) 
      w->hide();
  }
I can change the 'w->hide()' to w->setDisabled(true) and that seems to work, or I can insert a messagebox() before the 'for' statement, and then the edit boxes do get hidden as expected, but I haven't yet found a proper fix.

I don't know if this is a timing issue, but, whether or not, what needs to be done?  I'm clutching at straws here!  Thanks.
Comment 2 Thomas Baumgart 2011-06-26 16:27:38 UTC
One thing you could try is to replace

  w->hide();

with

  QTimer::singleShot(0, w, SLOT(hide()));

If that does not work, try if replacing 0 with a value of 1000 makes a difference.
Comment 3 allan 2011-06-26 17:22:51 UTC
(In reply to comment #2)
> One thing you could try is to replace
> 
>   w->hide();
> 
> with
> 
>   QTimer::singleShot(0, w, SLOT(hide()));
> 
> If that does not work, try if replacing 0 with a value of 1000 makes a
> difference.

Sanity returns!  Thanks for that.  I had remembered seeing this in the import area, looked at it and didn't pursue it.  Instead I tried a huge for loop and that made no difference.

The value '0' didn't help, so I tried 1000.  The problem was still there and I cancelled the edit, and just as it closed I saw the fields disappear!  So, I now have worked it down to 10.  Problem now is that all those widgets are closing, but I need to clean up my code before I go any further.
Comment 4 allan 2011-06-27 10:33:28 UTC
Making progress, but I've noticed something else.

When a Dividend transaction is opened for editing, the fee category field is active, but there is no label.  If a fee category is entered, the fee amount field becomes active - again no label, and if an amount is entered, the total reflects this change.  However, when the transaction edit is accepted, the fee amount has been lost.

I'm not clear on whether it is expected that a Dividend payment could have a fee, which sounds a bit unlikely, so I suspect that the fee account should not be active.  If I hide it, though, the split widget shifts to the left.

Before going any further, I think I need a second opinion, please, on what is right and what is wrong.
Comment 5 David Houlden 2011-06-27 11:02:15 UTC
I don't think a fee is applicable to a dividend payment and when entering a new dividend the gui doesn't show the fee field. A fee can however be applicable to a reinvest dividend transaction.
Comment 6 allan 2011-06-27 12:42:44 UTC
(In reply to comment #5)
> I don't think a fee is applicable to a dividend payment and when entering a new
> dividend the gui doesn't show the fee field. A fee can however be applicable to
> a reinvest dividend transaction.

That's two for 'no fee'.  Good.  The GUI is what I'm working on at the moment, and there are anomalies.  The fee amount is not shown, but the fee category selector is shown, and if an entry is made, the fee amount becomes active, but any value entered is not retained.
Comment 7 allan 2011-06-28 10:02:39 UTC
I've noticed various other anomalies in the various investment types, to do with unnecessary fields, missing labels and one where it's possible to enter a value for a fee without a category, and which doesn't get retained.

Also Bug 276315 may need to be revisited. The fix worked for Dividends, where no price is involved, but I think the odd currency/price wizard still shows for ReinvDiv/Buy transactions, where there is a price.
Comment 8 allan 2011-06-28 11:35:47 UTC
A query about investment interest splits.

A Buy transaction being edited shows the Interest split button, but ReInvDiv does not.  The Buy split seems to work as I expect, at least in limited testing.

In the code, the ReInvDiv split button gets hidden, which figures, but for others the following appears -

// FIXME once we can handle split interest, we need to uncomment the next line
// interest->splitButton()->show();


As the splits do seem to work, should that comment be removed, particularly as the button does in fact show.  Also, and perhaps a bit more importantly, why no ReInvDiv splits?
Comment 9 allan 2011-06-28 22:49:40 UTC
Sorry, but another query.  I can't think of any reason for a Buy transaction to have an interest amount field, especially bearing in mind that ReInvDiv doesn't have one.  I can understand them having an interest account field for a category.

Unless I hear to the contrary, I'm inclined to remove it.
Comment 10 allan 2011-07-09 17:53:32 UTC
I'm still finding the odd wrinkle :( The latest is strange and I haven't found a way to fix it.

If I create a valid Dividend transaction and save it, then decide to edit it and make it an Add shares instead, when that is saved, the transaction is flagged as having a missing assignment, of the dividend amount.  I haven't found a way to correct this, other than deleting the transaction and starting again.

The reason I was changing a Dividend to an Add was that I have been finding widget problems when editing an existing transaction.

Bearing in mind that the circumstances are somewhat artificial, is it acceptable to resort to deleting the transaction, or is there a better way?
Comment 11 allan 2011-08-09 20:01:16 UTC
(In reply to comment #10)
> I'm still finding the odd wrinkle :( The latest is strange and I haven't found
> a way to fix it.
> 
> If I create a valid Dividend transaction and save it, then decide to edit it
> and make it an Add shares instead, when that is saved, the transaction is
> flagged as having a missing assignment, of the dividend amount.  I haven't
> found a way to correct this, other than deleting the transaction and starting
> again.
> 
> The reason I was changing a Dividend to an Add was that I have been finding
> widget problems when editing an existing transaction.
> 
> Bearing in mind that the circumstances are somewhat artificial, is it
> acceptable to resort to deleting the transaction, or is there a better way?

Hoping that someone is interested, I'm coming back to this issue.

Whilst my example is not very likely, it is not totally unrealistic.  It also can affect other types than the 'add shares' I first noticed.

When I've been trying to tie up loose ends in the UI, I've tried to show only the necessary widgets.  In this case, however, when attempting to change to an 'add shares', or to 'reinvest', not all widgets are shown, so a clean edit is not possible, without a lot of jiggery-pokery.

Should the app be able to cope with such an issue, or is zapping the transaction an acceptable solution?
Comment 12 allan 2011-08-27 19:32:31 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > One thing you could try is to replace
> > 
> >   w->hide();
> > 
> > with
> > 
> >   QTimer::singleShot(0, w, SLOT(hide()));
> > 
> > If that does not work, try if replacing 0 with a value of 1000 makes a
> > difference.
> 
Eventually, I settled on a value of '1', which has worked nicely, and I had to use it quite a lot.

However, and unexpectedly, I think I found why it appeared necessary, when I had accepted it and wasn't looking any deeper.

After a break, when I was doing a tidy up, one of the widgets which I had got to work correctly, appeared again when it should have been hidden.  Immediately on opening a new investment transaction, when 'Buy shares' was displayed, but hadn't been selected, the 'Interest' widget was visible, when it shouldn't have been.  When 'Buy shares' was selected, the widget correctly was hidden.

I found that in InvestTransactionEditor::slotUpdateActivity(), 'haveWidget("interest-account")->parentWidget()->hide()' didn't hide the combobox.  When I removed the 'parentWidget()' element, I was able to revert the majority of the cases where I'd needed to use the QTimer.
Comment 13 allan 2011-08-27 19:34:08 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > One thing you could try is to replace
> > 
> >   w->hide();
> > 
> > with
> > 
> >   QTimer::singleShot(0, w, SLOT(hide()));
> > 
> > If that does not work, try if replacing 0 with a value of 1000 makes a
> > difference.
> 
Eventually, I settled on a value of '1', which has worked nicely, and I had to use it quite a lot.

However, and unexpectedly, I think I found why it appeared necessary, when I had accepted it and wasn't looking any deeper.

After a break, when I was doing a tidy up, one of the widgets which I had got to work correctly, appeared again when it should have been hidden.  Immediately on opening a new investment transaction, when 'Buy shares' was displayed, but hadn't been selected, the 'Interest' widget was visible, when it shouldn't have been.  When 'Buy shares' was selected, the widget correctly was hidden.

I found that in InvestTransactionEditor::slotUpdateActivity(), 'haveWidget("interest-account")->parentWidget()->hide()' didn't hide the combobox.  When I removed the 'parentWidget()' element, I was able to revert the majority of the cases where I'd needed to use the QTimer.
Comment 14 allan 2011-08-27 19:37:33 UTC
Please ignore the duplication, which resulted from a mid-air collision with myself.
Comment 15 allan 2011-09-17 09:52:44 UTC
SVN commit 1254077 by allananderson:

BUG:276322
REVIEW:6791

If an existing cash dividend transaction is selected, the form shows widgets
for Date, Amount, Total and Status, which is what is expected.
If that transaction is then opened for editing, additional widgets show for
Security, Quantity and Price, which aren't expected for a cash dividend.
If, while the transaction is open for editing, 'Dividend' is reselected, the
additional fields disappear.  Similar behaviour occurs for Buy transactions.

Also, in numerous places, the likes of - 
QWidget* w = haveWidget(*it_s);
    if (w) 
      w->hide();
were found to produce unreliable results.

Also, When a Dividend transaction is opened for editing, the fee category field is
active, but there is no label.  If a fee category is entered, the fee amount
field becomes active - again no label, and if an amount is entered, the total
reflects this change.  However, when the transaction edit is accepted, the fee
amount has been lost.

Also, fees have been removed from Dividends, and interest income has been removed from 
Buy and Sell, as these seem most unlikely combinations.

I've noticed various other anomalies in the various investment types, to do
with unnecessary fields, missing labels and one where it's possible to enter a
value for a fee without a category, and which doesn't get retained.

 M  +116 -54   dialogs/investactivities.cpp  
 M  +47 -26    dialogs/investtransactioneditor.cpp  
 M  +2 -2      widgets/transaction.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1254077
Comment 16 Cristian Oneț 2013-10-03 18:05:25 UTC
Git commit a82ddef366860e8cd63ca903f7d8e1192c38d65d by Cristian Oneț, on behalf of Allan Anderson.
Committed on 17/09/2011 at 09:52.
Pushed by conet into branch '4.6'.
REVIEW:6791

If an existing cash dividend transaction is selected, the form shows widgets
for Date, Amount, Total and Status, which is what is expected.
If that transaction is then opened for editing, additional widgets show for
Security, Quantity and Price, which aren't expected for a cash dividend.
If, while the transaction is open for editing, 'Dividend' is reselected, the
additional fields disappear.  Similar behaviour occurs for Buy transactions.

Also, in numerous places, the likes of -
QWidget* w = haveWidget(*it_s);
    if (w)
      w->hide();
were found to produce unreliable results.

Also, When a Dividend transaction is opened for editing, the fee category field is
active, but there is no label.  If a fee category is entered, the fee amount
field becomes active - again no label, and if an amount is entered, the total
reflects this change.  However, when the transaction edit is accepted, the fee
amount has been lost.

Also, fees have been removed from Dividends, and interest income has been removed from
Buy and Sell, as these seem most unlikely combinations.

I've noticed various other anomalies in the various investment types, to do
with unnecessary fields, missing labels and one where it's possible to enter a
value for a fee without a category, and which doesn't get retained.

svn path=/trunk/extragear/office/kmymoney/; revision=1254077
(cherry picked from commit 101225c4ec65fc7eb4c94d92ee38a32666013908)

M  +116  -54   kmymoney/dialogs/investactivities.cpp
M  +48   -27   kmymoney/dialogs/investtransactioneditor.cpp
M  +2    -2    kmymoney/widgets/transaction.cpp

http://commits.kde.org/kmymoney/a82ddef366860e8cd63ca903f7d8e1192c38d65d
Comment 17 allan 2013-11-27 22:46:53 UTC
[From the list -On 24/11/13 23:28, William H. Haller wrote:}
> Allan,
>
> I noticed in your comments on this commit that Interest income was
> removed from Buys and Sells. Buys I can see, but I have been using the
> field in Sells to have a place to record long and short term capital
> gains. Thought I'd pass this along as a reasonable use case - it is
> useful for tax purposes to be able to record your income on sales
> somewhere. I've been patching this back into the total for a sell price.
Comment 18 allan 2013-12-02 14:13:39 UTC
Created attachment 83878 [details]
Patch to reinstate interest field in Sell activities.

Will commit patch shortly.
Comment 19 allan 2013-12-04 18:03:36 UTC
Git commit 3edd6f3b1eb10aa075dc2ebc4c37be74f258722c by Allan Anderson.
Committed on 27/11/2013 at 22:41.
Pushed by allananderson into branch 'master'.

M  +12   -5    kmymoney/dialogs/investactivities.cpp
M  +4    -3    kmymoney/dialogs/investtransactioneditor.cpp

http://commits.kde.org/kmymoney/3edd6f3b1eb10aa075dc2ebc4c37be74f258722c
Comment 20 allan 2014-01-12 21:10:45 UTC
From the Users list -
"I was updating my investment 401K accounts for year end to file taxes and noticed that for BUY Shares, only the Fee category line appears now appears in the transaction entry area.  Previously, and I am not sure in what version, the Interest Category line was present in the input area as well.

This is important because I work for a company that allows me to buy stock each quarter at a discount / subsidy.  This is income to me on my W2.  The Fee category does not accept an income category in the line item, even when you split the line.  All of my previously entered purchases with the discount now have the interest line hidden.  I have to switch the transaction type to see the hidden line.

Why does this matter?  As I stated the discount is income and I believe should appear in the income section of income and cash flow statements.  Allocating this to an expense category that is a credit (negative for income) instead of debit does track the dollars, but not correctly.  I have to explain it to may accountant every year or do a journal entry to move the balance to the correct account which should not be required.  Either way, it is a hassle.

Can we get the interest category  added back to the investment entry screen?

This may be related to BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.  I believe that widget visibility should be at the lowest common denominator.  That is if there is any question, show it and let the user choose to enter a value, even if the programmer does not agree.

Thank you
-- 
*Joe W. Byers* "

My reply -
Ok, Joe. I hold my hand up here.  In fact, this goes back even further, to https://bugs.kde.org/show_bug.cgi?id=276322m, June 2011.  Here's the first paragraph, to give you the initial issue.

"The problem shows best if the transaction form is enabled, but also is evident without the form.
If an existing cash dividend transaction is selected, the form shows widgets for Date, Amount, Total and Status, which is what is expected.
If that transaction is then opened for editing, additional widgets show for Security, Quantity and Price, which aren't expected for a cash dividend.
If, while the transaction is open for editing, 'Dividend' is reselected, the additional fields disappear.  Similar behaviour occurs for Buy transactions."

This prompted a review of what action types needed what fields, partly bearing in mind the new user, and trying to avoid 'visual overload'.

More recently, interest was reinstated for Sell transactions, and now we have a use case for Buy transactions, too.  I'm happy to respond, but don't think to the extent of enabling everything on everything, though.

Allan
Comment 21 allan 2014-01-15 13:08:47 UTC
Created attachment 84654 [details]
Reinstate Interest category for Buy activities. Also, Fix misbehaviour of Interest widget (flickering) in several activity types.

Reinstate Interest category for Buy activities. Also, Fix misbehaviour of Interest widget (flickering) in several activity types.
Comment 22 allan 2014-01-15 13:10:59 UTC
All tests OK now.

If no objections, I'll commit in a week or so.
Comment 23 Cristian Oneț 2014-07-31 09:58:34 UTC
Fix '3edd6f3b1eb10aa075dc2ebc4c37be74f258722c' committed by Allan.
Comment 24 Mehdi Tibouchi 2014-11-09 02:43:24 UTC
(In reply to allan from comment #22)
> All tests OK now.
> 
> If no objections, I'll commit in a week or so.

Apparently, this patch reinstating the interest field in Buy activities has never been committed; it is still not present in git today. Please do so! The problem is very jarring in the situation mentioned by J.W. Byers above (being allowed to buy company stock at a discount). For now we have to manually patch KMyMoney 4.7.1 to recover our previous workflow.
Comment 25 Cristian Oneț 2014-11-09 10:40:19 UTC
Git commit 404d6279c74b8e93696f774fd04ac55d29307325 by Cristian Oneț.
Committed on 09/11/2014 at 10:27.
Pushed by conet into branch 'master'.

Fix the missing interest category for 'Buy' activities.

This fix is based on Allan's patch which is attached to the bug report
that I incorrectly thought to be fixed. I did some refactoring
because the readability of the investment activities was horrible. We
really should not have that kind of code in the application.

M  +29   -92   kmymoney/dialogs/investactivities.cpp
M  +1    -0    kmymoney/dialogs/investactivities.h
M  +12   -8    kmymoney/dialogs/investtransactioneditor.cpp

http://commits.kde.org/kmymoney/404d6279c74b8e93696f774fd04ac55d29307325
Comment 26 Cristian Oneț 2014-11-09 10:41:09 UTC
Git commit cd6b3c9a653096188097fba254360edda395a14f by Cristian Oneț.
Committed on 09/11/2014 at 10:27.
Pushed by conet into branch '4.7'.

Fix the missing interest category for 'Buy' activities.

This fix is based on Allan's patch which is attached to the bug report
that I incorrectly thought to be fixed. I did some refactoring
because the readability of the investment activities was horrible. We
really should not have that kind of code in the application.
(cherry picked from commit 404d6279c74b8e93696f774fd04ac55d29307325)

M  +29   -92   kmymoney/dialogs/investactivities.cpp
M  +1    -0    kmymoney/dialogs/investactivities.h
M  +12   -8    kmymoney/dialogs/investtransactioneditor.cpp

http://commits.kde.org/kmymoney/cd6b3c9a653096188097fba254360edda395a14f
Comment 27 allan 2014-11-09 10:48:53 UTC
(In reply to Cristian Oneț from comment #26)
> Git commit cd6b3c9a653096188097fba254360edda395a14f by Cristian Oneț.
> Committed on 09/11/2014 at 10:27.
> Pushed by conet into branch '4.7'.
> 
> Fix the missing interest category for 'Buy' activities.
> 
> This fix is based on Allan's patch which is attached to the bug report
> that I incorrectly thought to be fixed. 

Thanks, Cristian, I had just starting to look into this.  Initially, I too, thought
that it had been committed, because of your following commit.

> I did some refactoring
> because the readability of the investment activities was horrible. We
> really should not have that kind of code in the application.
> (cherry picked from commit 404d6279c74b8e93696f774fd04ac55d29307325)
> 
> M  +29   -92   kmymoney/dialogs/investactivities.cpp
> M  +1    -0    kmymoney/dialogs/investactivities.h
> M  +12   -8    kmymoney/dialogs/investtransactioneditor.cpp
> 
> http://commits.kde.org/kmymoney/cd6b3c9a653096188097fba254360edda395a14f