Bug 385857 - Wrong Currency conversion rate in Reports
Summary: Wrong Currency conversion rate in Reports
Status: REOPENED
Alias: None
Product: kmymoney
Classification: Applications
Component: reports (show other bugs)
Version: 4.8.0
Platform: Microsoft Windows Microsoft Windows
: NOR normal
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-17 13:58 UTC by Tamer
Modified: 2022-12-03 09:17 UTC (History)
2 users (show)

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


Attachments
test case (26.53 KB, text/xml)
2017-10-17 21:33 UTC, Ralf Habacker
Details
[kmymoney4] [Bug 385857] Wrong Currency conversion rate in Reports (552.29 KB, application/pdf)
2017-10-18 08:35 UTC, Tamer
Details
test case (26.79 KB, text/xml)
2017-10-18 11:00 UTC, Ralf Habacker
Details
kmy file showing the case (19.22 KB, application/gzip)
2017-10-19 07:27 UTC, Tamer
Details
Updated Screen Shots (618.37 KB, application/pdf)
2017-10-19 07:35 UTC, Tamer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tamer 2017-10-17 13:58:20 UTC
Dear Sirs,
Problem Statement: Reports convert currencies in wrong rates in case rate changes of the different currencies in use.
 
Below are the reproducing steps:
1. Make the application base currency as AED (United Arab Emirates Dirhams).
2. Create a cash account in AED: name= "Cash in AED"
3. Create an income category "Salary" in EGP (Egyptian Pounds).
4. Set the conversion rate between AED and EGP as: 1 AED = 2 EGP effective  1 Jan 2017.
5. Make a deposit transaction of 1,000 AED into "Cash in AED" with category "Salary" with entry date as 1 March 2017.
--> At this stage the application will convert 1,000 AED to 2,000 EGP.
6. Set the conversion rate to be 1 AED = 5 EGP effective 1 Oct 2017.
7. Configure the Income and Expenses Report to have the option "Convert values to base currency" checked.
--> the report will show the deposited 1,000 AED as 400 AED despite it is deposited into the an account with AED currency.
It took the converted 2,000 EGP and converted them into AED with the latest rate (1 AED= 5 EGP) which is wrong because they money is still in AED and never converted in reality. or either to consider the rate at which the transaction was inserted which was (1 AED= 2 EGP). 

The current report gives wrong information and accordingly we can take wrong decisions.
Comment 1 Ralf Habacker 2017-10-17 21:33:41 UTC
Created attachment 108417 [details]
test case

I tried to reproduce this issue with kmymoney version 4.8.1 on Windows and the appended test case, but could not reproduce this issue. Loading the test case with 4.8.0 also does not show the issue.
Comment 2 Tamer 2017-10-18 08:35:19 UTC
Created attachment 108428 [details]
[kmymoney4] [Bug 385857] Wrong Currency conversion rate in Reports

Reproducing steps with screen shots
Comment 3 Tamer 2017-10-18 08:37:04 UTC
Thanks Ralf for your prompt feedback,
I have uploaded the file "[kmymoney4] [Bug 385857] Wrong Currency conversion rate in Reports.pdf" which contains the reproducing steps with screen shots to demonstrate the case in real life example.

Please let me know if you need further information.
Comment 4 Ralf Habacker 2017-10-18 11:00:35 UTC
Created attachment 108429 [details]
test case

test case updated according to the additional informations

This test case shows the mentioned sum 172260,75 AED in the sum column of the category tab. Removing the price 4,833 from 10/16/17 shows 344457,36 AED which is calculated by using the price from 11/13/16. This indicates that kmymoney shows always the latest available price in the sum column of the category tab.

Opening the report "Income and expense (convert to base currency)" always shows 344457,36 AED (regardless if the price 4,833 from 10/16/17 is present or not). This indicates that the report seems to use the last price in the reports date range.

I'm still not able to see the mentioned value 172260,75 AED in the report you are mentioned with KMymoney 4.8.1 on Windows
Comment 5 Tamer 2017-10-18 13:58:30 UTC
Hi Ralf,
I created a new project to simulate the case and what you mentioned is correct. the report shows the correct value.

The bug looks to appear only with old projects and files.
Please note that I am using this application since 2014 and I used different versions with the same file. Maybe something wrong in the database stored in the file or a scenario not covered in the new version.

What I have shared is real snapshots of my records and definitely there is a problem which cannot be reproduced by simulating new projects.  

I am not sure if I there is any utility that can decrypt the data so i can share my file with you for analysis. if so, please let me know, or I can share my screen with you to validate my argument your self, or we can close this bug if there no other option.

Thanks for your efforts.
Comment 6 Ralf Habacker 2017-10-19 07:14:17 UTC
(In reply to Tamer from comment #5)
> Hi Ralf,
> I am not sure if I there is any utility that can decrypt the data so i can
> share my file with you for analysis. if so, please let me know
see https://docs.kde.org/stable4/en/extragear-office/kmymoney/details.formats.anonymous.html and please append such generated file to this bug.
Comment 7 Tamer 2017-10-19 07:27:01 UTC
Created attachment 108451 [details]
kmy file showing the case

I attached the kmy file in which the bug appears.
You can see the wrong summation in the report "Income and Expenses 2016" in Month of Nov.
The report is showing 170,263.37 AED instead of 344,450 AED
Comment 8 Tamer 2017-10-19 07:35:28 UTC
Created attachment 108452 [details]
Updated Screen Shots

Adding two screen shots to show the list of entered transactions and showing the report that has the wrong values.
Comment 9 Ralf Habacker 2017-10-19 08:30:42 UTC
(In reply to Tamer from comment #7)
> Created attachment 108451 [details]
> kmy file showing the case
> 
> I attached the kmy file in which the bug appears.
> You can see the wrong summation in the report "Income and Expenses 2016" in
> Month of Nov.
> The report is showing 170,263.37 AED instead of 344,450 AED

Opening the mentioned reports show the following used conversion factors (see PivotTable::convertToBaseCurrency()) 

conversionfactor QDate("2016-01-01") 2.13174
conversionfactor QDate("2016-02-01") 2.13174
conversionfactor QDate("2016-03-01") 2.13311
conversionfactor QDate("2016-04-01") 2.4148
conversionfactor QDate("2016-05-01") 2.4181
conversionfactor QDate("2016-06-01") 2.4197
conversionfactor QDate("2016-07-01") 2.42131
conversionfactor QDate("2016-08-01") 2.4178
conversionfactor QDate("2016-09-01") 3.3783
conversionfactor QDate("2016-10-01") 3.3774
conversionfactor QDate("2016-11-01") 2.4172
conversionfactor QDate("2016-12-01") 4.92611
Comment 10 Ralf Habacker 2017-10-19 10:14:59 UTC
(In reply to Ralf Habacker from comment #9)
> (In reply to Tamer from comment #7)
> > Created attachment 108451 [details]
> > kmy file showing the case
> > 
> > I attached the kmy file in which the bug appears.
> > You can see the wrong summation in the report "Income and Expenses 2016" in
> > Month of Nov.
> > The report is showing 170,263.37 AED instead of 344,450 AED
> 
> Opening the mentioned reports show the following used conversion factors
> (see PivotTable::convertToBaseCurrency()) 

adding more debug information returns for monthly report interval

Debug:date QDate("Mi Nov 30 2016") conversionfactor 4.89 oldvalue 832588 new value 170263 

and for weekly report interval

Debug:date QDate("Do Nov 17 2016") conversionfactor 2.4167 oldvalue 832588 new value 344514 

and for daily report interval

Debug:date QDate("Do Nov 10 2016") conversionfactor 2.4175 oldvalue 107458 new value 44450 
Debug:date QDate("So Nov 13 2016") conversionfactor 2.4171 oldvalue 725130 new value 300000 

for QDate("Mi Nov 30 2016") a price is present and has value 4.89
for QDate("Do Nov 17 2016") a price is also present and has value 2.4167
for QDate("Do Nov 10 2016") a price is present and has value 2.4175
for QDate("So Nov 13 2016") a price is present and has value 2.4171
Comment 11 Ralf Habacker 2017-10-21 10:16:08 UTC
The root cause for this bug is that in PivotTable::init() the cell values are collected in the accounts currency first. Then the sum is converted to the file currency using the date from the report cell, which may not be the same date as the transaction date the single values are coming from.

To have a correct sum each value from a transaction need to be converted to the base currency before adding to the report cell and not to convert the sum.
Comment 12 wojnilowicz 2017-10-21 12:24:11 UTC
(In reply to Ralf Habacker from comment #11)
> The root cause for this bug is that in PivotTable::init() the cell values
> are collected in the accounts currency first. Then the sum is converted to
> the file currency using the date from the report cell, which may not be the
> same date as the transaction date the single values are coming from.
> 
> To have a correct sum each value from a transaction need to be converted to
> the base currency before adding to the report cell and not to convert the
> sum.

I'm not following this bug very dilligently, but before modifying 4.8.1 version code paths in PivotTable::init() and then applying it on 5.0 branch, see if code paths in 5.0 (there were some noticeable changes there) show the same error and maybe backport if not.
Comment 13 Ralf Habacker 2017-10-23 08:29:56 UTC
(In reply to NSLW from comment #12)
> (In reply to Ralf Habacker from comment #11)
> > The root cause for this bug is that in PivotTable::init() the cell values
> > are collected in the accounts currency first. Then the sum is converted to
> > the file currency using the date from the report cell, which may not be the
> > same date as the transaction date the single values are coming from.
> > 
> > To have a correct sum each value from a transaction need to be converted to
> > the base currency before adding to the report cell and not to convert the
> > sum.
> 
> I'm not following this bug very dilligently, but before modifying 4.8.1
> version code paths in PivotTable::init() and then applying it on 5.0 branch,
> see if code paths in 5.0 (there were some noticeable changes there) show the
> same error 
This has been reported in bug 385900 - git master show different but also wrong values
>and maybe backport if not.
not sure yet if this is possible - I guess this depends on how much the branch is diverged in that area. I tried backporting already with online price update which was not possible because the newly introduced regex classes are not available for qt4. :-(
Comment 14 Ralf Habacker 2017-10-26 22:19:05 UTC
While working on this stuff I used the pivottable test app to see if something has been changed by the fix mentioned in comment 11 and I got differences in testMultipleCurrencies()

The basic test data are 

  MyMoneyMoney moJpyPrice(0.010, 100);
  MyMoneyMoney moJpyPrice2(0.011, 100);
  MyMoneyMoney moJpyPrice3(0.014, 100);
  MyMoneyMoney moJpyPrice4(0.0395, 100);
  makePrice("JPY", QDate(2004, 1, 1), MyMoneyMoney(moJpyPrice));
  makePrice("JPY", QDate(2004, 5, 1), MyMoneyMoney(moJpyPrice2));
  makePrice("JPY", QDate(2004, 6, 30), MyMoneyMoney(moJpyPrice3));
  makePrice("JPY", QDate(2004, 7, 15), MyMoneyMoney(moJpyPrice4));

  MyMoneyMoney moJpyTransaction(100.0, 100);

  related transactions 
  ... t1(QDate(2004, 2, 20), .., moJpyTransaction, acJpyChecking, acJpyCash, "JPY");
  ... t2(QDate(2004, 3, 20), .., moJpyTransaction, acJpyChecking, acJpyCash, "JPY");
  ... t3(QDate(2004, 4, 20), .., moJpyTransaction, acJpyChecking, acJpyCash, "JPY");

old

Account	Asset	Canadian Checking	Checking Account	Japanese Checking	Total Asset	Liability	Credit Card	Total Liability	Grand Total
Jan		0.00	0.00	0.00	0.00		0.00	0.00	0.00
Feb		-75.00	0.00	-1.00	-76.00		0.00	0.00	-76.00
Mar		-150.00	0.00	-2.00	-152.00		0.00	0.00	-152.00
Apr		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
May		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Jun		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Jul		-225.00	0.00	-12.00	-237.00		0.00	0.00	-237.00 [1]
Aug		-225.00	0.00	-12.00	-237.00		0.00	0.00	-237.00
Sep		-225.00	0.00	-12.00	-237.00		0.00	0.00	-237.00
Oct		-225.00	0.00	-12.00	-237.00		0.00	0.00	-237.00
Nov		-225.00	0.00	-12.00	-237.00		0.00	0.00	-237.00
Dec		-225.00	0.00	-12.00	-237.00		0.00	0.00	-237.00

[1] caused by price moJpyPrice4

new

Account	Asset	Canadian Checking	Checking Account	Japanese Checking	Total Asset	Liability	Credit Card	Total Liability	Grand Total
Jan		0.00	0.00	0.00	0.00		0.00	0.00	0.00
Feb		-75.00	0.00	-1.00	-76.00		0.00	0.00	-76.00
Mar		-150.00	0.00	-2.00	-152.00		0.00	0.00	-152.00
Apr		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
May		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Jun		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Jul		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Aug		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Sep		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Oct		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Nov		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00
Dec		-225.00	0.00	-3.00	-228.00		0.00	0.00	-228.00

this shows that price changes after Apr 20 does not affect "Japanese Checking".
Comment 15 Ralf Habacker 2017-11-06 11:50:37 UTC
Review request for bug fix:  https://phabricator.kde.org/D8678
Comment 16 Ralf Habacker 2017-11-14 16:20:09 UTC
Git commit c829ec79e0649c4b923a16f4154caa38ba72beeb by Ralf Habacker.
Committed on 14/11/2017 at 16:19.
Pushed by habacker into branch '4.8'.

Fix 'Wrong Currency conversion rate in Reports'

The root cause was that the conversion of reports value to the base
currency is done on grid entry date and not on transaction date.
The price on grid entry date may differ from the price on the
transaction date and results in unexpected sums.
FIXED-IN:4.8.2

Test Plan: - generated with appended test case

Reviewers: #kmymoney, wojnilowicz

Reviewed By: wojnilowicz

Subscribers: wojnilowicz

Differential Revision: https://phabricator.kde.org/D8678

M  +25   -50   kmymoney/reports/pivottable.cpp
M  +6    -4    kmymoney/reports/pivottable.h
M  +1    -1    kmymoney/reports/pivottabletest.cpp

https://commits.kde.org/kmymoney/c829ec79e0649c4b923a16f4154caa38ba72beeb
Comment 17 Ralf Habacker 2018-01-08 17:40:25 UTC
Git commit eed47fd1687c159ad4e02e84782398b7ecaef059 by Ralf Habacker.
Committed on 04/01/2018 at 09:21.
Pushed by habacker into branch '4.8'.

Fix 'Git commit c829ec79 broke investment price chart'

The fix is to revert commit "Fix 'Wrong Currency conversion
rate in Reports'" (c829ec79e0649c4b923a16f4154caa38ba72beeb)
required for bug 385857 for now.
Related: bug 387040
FIXED-IN:4.8.2

M  +50   -25   kmymoney/reports/pivottable.cpp
M  +4    -6    kmymoney/reports/pivottable.h
M  +1    -1    kmymoney/reports/pivottabletest.cpp

https://commits.kde.org/kmymoney/eed47fd1687c159ad4e02e84782398b7ecaef059
Comment 18 Ralf Habacker 2018-01-08 17:43:20 UTC
It turned out that the fix for this bug introduced bug 387040, so I reverted it

Until this bug is fixed by using another approach, the workaround is to only use daily report intervals.