Bug 420593 - A sum of multiple rows selected is incorrect for securities with fraction > 100
Summary: A sum of multiple rows selected is incorrect for securities with fraction > 100
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: general (show other bugs)
Version: git (master)
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-25 21:48 UTC by Dawid Wróbel
Modified: 2020-06-05 04:07 UTC (History)
1 user (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dawid Wróbel 2020-04-25 21:48:22 UTC
SUMMARY
We assume a fraction of 100 for accounts created through "New account" wizard. The ledger view then uses said fraction to display a sum if more than 1 row is selected. If any of the securities are traded with a fraction > 100, the calculated sum will be off.


STEPS TO REPRODUCE
1. Add securities with fraction > 100 to a brokerage account
2. Add some transactions
3. Select more than 1 transaction

OBSERVED RESULT
The sum is rounded to 2 decimal places and is inaccurate.
 
EXPECTED RESULT
The sum should not be rounded up and instead displayed using a all the decimal places required.
Comment 1 Dawid Wróbel 2020-04-25 22:01:39 UTC
Git commit c37191ada147cf18f1c74dab70c02a425b67104a by Dawid Wróbel.
Committed on 25/04/2020 at 22:01.
Pushed by wrobelda into branch 'master'.

Display transactions sum using all decimal places

For a sum of multiple rows selected in the global ledger view, use the
automatically determined minimum required precision to avoid truncating
the value

M  +1    -1    kmymoney/views/kgloballedgerview.cpp

https://commits.kde.org/kmymoney/c37191ada147cf18f1c74dab70c02a425b67104a
Comment 2 Jack 2020-04-25 22:19:59 UTC
Maybe I don't understand something here, but for an investment transaction, the split for the brokerage account is always in the currency of the brokerage account, no matter how many decimal places are used for the number of shares transacted.  At least for every investment account I have had, the amount of money used for any transaction (buy, sell, dividend) is always stated in whole cents (2 decimal places for $US) no matter the the fraction used for number of shares and price per share.  I'd really like to see an example that demonstrates the problem you see.
Comment 3 Dawid Wróbel 2020-04-25 22:27:53 UTC
Jack, this is not about the value – if you select more than one transaction, the sum displayed is of the quantity of the shares. 

Hope that explains it.
Comment 4 Jack 2020-04-26 17:05:49 UTC
Is this in the investment account or the brokerage account?  Comment 1 starts with adding securities to a brokerage account, which doesn't actually make sense.

In the investment account, I don't think it makes any sense to show a sum of anything, unless the selected transactions are all for the same security.  In that case, why not use use precision of the security, instead of just figuring what the transactions need.  If the transactions are for different securities, I would prefer not to show any total since it would be like adding the amounts in different currencies.  With currencies, at least you could show the total after conversion to the base currency of the account.
Comment 5 Dawid Wróbel 2020-04-26 19:02:35 UTC
Jack, yes, point 1. was supposed to say "investment account", not "brokerage".

> In the investment account, I don't think it makes any sense to show a sum of anything

This functionality was already in place and I find it useful. 

> unless the selected transactions are all for the same security. 

I think we can safely assume the users are smart enough to understand which securities they select and what does the sum mean in this case. Besides, I don't think you're right – I myself specifically had to sum the shares of different securities after my brokerage automatically liquidated B class of shares and repurchased C class. Either way, it's a different issue, unrelated to this bug fix. 

> In that case, why not use use precision of the security, instead of just figuring what the transactions need. 

We don't need to figure out the precision at all when summing the values. The precision only matters when converting the floating point value to a string for displaying and the MyMoneyMoney::formatMoney() function can handle that automatically – which my code change took advantage of.
Comment 6 Thomas Baumgart 2020-05-01 05:46:09 UTC
Git commit 7d860a6b0f17b7903bbe14ce93f798a25067f150 by Thomas Baumgart.
Committed on 01/05/2020 at 05:40.
Pushed by tbaumgart into branch '5.0'.

Display transactions sum using all decimal places

For a sum of multiple rows selected in the global ledger view, use the
automatically determined minimum required precision to avoid truncating
the value
FIXED-IN: 5.0.9
(cherry picked from commit c37191ada147cf18f1c74dab70c02a425b67104a)

M  +1    -1    kmymoney/views/kgloballedgerview.cpp

https://commits.kde.org/kmymoney/7d860a6b0f17b7903bbe14ce93f798a25067f150