Review Request 129739: Revise investment performance report

Thomas Baumgart tbaumgart at kde.org
Sun Jan 8 16:21:00 UTC 2017



> On Jan. 8, 2017, 11:19 vorm., Thomas Baumgart wrote:
> > kmymoney/reports/querytable.cpp, line 1025
> > <https://git.reviewboard.kde.org/r/129739/diff/2/?file=488800#file488800line1025>
> >
> >     If we need the dissectTransaction here also, we should refactor it to KMyMoneyUtils. This also helps to remove the include of the editor header file.
> 
> Łukasz Wojniłowicz wrote:
>     Do you mean to carry it over from investtransactioneditor.h to kmymoneyutils.h?
> 
> Łukasz Wojniłowicz wrote:
>     Is it OK now?

Yes, but why do I not see the addition in KMyMoneyUtils here in the patch? I just saw that there also is a CsvUtils::dissectTransaction. Maybe, we can even eliminate that at the same time. Maybe with a different RR if you want.


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129739/#review101859
-----------------------------------------------------------


On Jan. 8, 2017, 5:14 nachm., Łukasz Wojniłowicz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129739/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2017, 5:14 nachm.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> -------
> 
> 1) "amount" (last column) has been replaced by "ending balance" because
> it's calculated specially for this report by
> QueryTable::constructPerformanceRow,
> 2) "return on investment" column is being calculated also when "ending
> balance" is zero, because zero ending balance doesn't invalidate ROI
> 3) "buys" and "sells" column take "fee amounts" into account during ROI
> calculation, because fees are inseparable from investments,
> 4) "buys" column has minus sign and "sells" column has plus sing,
> because the same notatnion is used by KMM importers.
> 
> 
> Diffs
> -----
> 
>   kmymoney/reports/listtable.cpp 72b605f 
>   kmymoney/reports/querytable.cpp e44f74c 
> 
> Diff: https://git.reviewboard.kde.org/r/129739/diff/
> 
> 
> Testing
> -------
> 
> Reports on stock securities with dividends, completely, partially and not sold at all.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20170108/d7c604bc/attachment.html>


More information about the KMyMoney-devel mailing list