From kde-commits Mon Jun 13 13:30:13 2016 From: =?utf-8?q?Cristian_One=C8=9B?= Date: Mon, 13 Jun 2016 13:30:13 +0000 To: kde-commits Subject: [kmymoney/frameworks] kmymoney/mymoney: Reduce the precision of shares and values Message-Id: X-MARC-Message: https://marc.info/?l=kde-commits&m=146582464223090 Git commit dd5bfdd80e64ec30ac10b13c6a0dd7aa818bd1d2 by Cristian One=C8=9B, = on behalf of Thomas Baumgart. Committed on 13/06/2016 at 13:22. Pushed by conet into branch 'frameworks'. Reduce the precision of shares and values The problem described in bug 345655 is caused by a fractional part of the splits value that is larger than the fractional part of the accounts curren= cy. This leads to the effect, that amounts are displayed with correctly but the underlying amount used for calculations is not equal to the amount displayed. This will end up in KMyMoney presenting the user strange messages like 'Unassigned value of 0.00' which do not make sense. The difference is in reality between 0 and 0.01, e.g. 0.008. BUG: 345655 FIXED-IN: 4.8.0 REVIEW: 128061 M +32 -7 kmymoney/mymoney/mymoneyfile.cpp M +5 -0 kmymoney/mymoney/mymoneyfile.h M +127 -0 kmymoney/mymoney/mymoneyfiletest.cpp M +1 -0 kmymoney/mymoney/mymoneyfiletest.h http://commits.kde.org/kmymoney/dd5bfdd80e64ec30ac10b13c6a0dd7aa818bd1d2 diff --git a/kmymoney/mymoney/mymoneyfile.cpp b/kmymoney/mymoney/mymoneyfil= e.cpp index d93f891..bf1e022 100644 --- a/kmymoney/mymoney/mymoneyfile.cpp +++ b/kmymoney/mymoney/mymoneyfile.cpp @@ -498,8 +498,7 @@ void MyMoneyFile::modifyTransaction(const MyMoneyTransa= ction& transaction) { d->checkTransaction(Q_FUNC_INFO); = - const MyMoneyTransaction* t =3D &transaction; - MyMoneyTransaction tCopy; + MyMoneyTransaction tCopy(transaction); = // now check the splits bool loanAccountAffected =3D false; @@ -519,7 +518,6 @@ void MyMoneyFile::modifyTransaction(const MyMoneyTransa= ction& transaction) // change transfer splits between asset/liability and loan accounts // into amortization splits if (loanAccountAffected) { - tCopy =3D transaction; QList list =3D transaction.splits(); for (it_s =3D list.constBegin(); it_s !=3D list.constEnd(); ++it_s) { if ((*it_s).action() =3D=3D MyMoneySplit::ActionTransfer) { @@ -529,7 +527,6 @@ void MyMoneyFile::modifyTransaction(const MyMoneyTransa= ction& transaction) MyMoneySplit s =3D (*it_s); s.setAction(MyMoneySplit::ActionAmortization); tCopy.modifySplit(s); - t =3D &tCopy; } } } @@ -549,12 +546,15 @@ void MyMoneyFile::modifyTransaction(const MyMoneyTran= saction& transaction) //FIXME-ALEX Do I need to add d->addCacheNotification((*it_s).tagList(= )); ?? } = + // make sure the value is rounded to the accounts precision + fixSplitPrecision(tCopy); + // perform modification - d->m_storage->modifyTransaction(*t); + d->m_storage->modifyTransaction(tCopy); = // and mark all accounts that are referenced - for (it_s =3D t->splits().constBegin(); it_s !=3D t->splits().constEnd()= ; ++it_s) { - d->addCacheNotification((*it_s).accountId(), t->postDate()); + for (it_s =3D tCopy.splits().constBegin(); it_s !=3D tCopy.splits().cons= tEnd(); ++it_s) { + d->addCacheNotification((*it_s).accountId(), tCopy.postDate()); d->addCacheNotification((*it_s).payeeId()); //FIXME-ALEX Do I need to add d->addCacheNotification((*it_s).tagList(= )); ?? } @@ -1182,6 +1182,9 @@ void MyMoneyFile::addTransaction(MyMoneyTransaction& = transaction) transaction.setCommodity(baseCurrency().id()); } = + // make sure the value is rounded to the accounts precision + fixSplitPrecision(transaction); + // then add the transaction to the file global pool d->m_storage->addTransaction(transaction); = @@ -3172,6 +3175,28 @@ int MyMoneyFile::countTransactionsWithSpecificReconc= iliationState(const QString& return transactionList(filter).count(); } = + /** + * Make sure that the splits value has the precision of the correspondin= g account + */ +void MyMoneyFile::fixSplitPrecision(MyMoneyTransaction& t) const +{ + QList::iterator its; + MyMoneySecurity transactionSecurity =3D security(t.commodity()); + int transactionFraction =3D transactionSecurity.smallestAccountFraction(= ); + + for(its =3D t.splits().begin(); its !=3D t.splits().end(); ++its) { + MyMoneyAccount acc =3D account((*its).accountId()); + int fraction =3D acc.fraction(); + if(fraction =3D=3D -1) { + MyMoneySecurity sec =3D security(acc.currencyId()); + fraction =3D acc.fraction(sec); + } + (*its).setShares((*its).shares().convertDenominator(fraction).canonica= lize()); + (*its).setValue((*its).value().convertDenominator(transactionFraction)= .canonicalize()); + } +} + + MyMoneyFileTransaction::MyMoneyFileTransaction() : m_isNested(MyMoneyFile::instance()->hasTransaction()), m_needRollback(!m_isNested) diff --git a/kmymoney/mymoney/mymoneyfile.h b/kmymoney/mymoney/mymoneyfile.h index 0fd558b..a89c5a2 100644 --- a/kmymoney/mymoney/mymoneyfile.h +++ b/kmymoney/mymoney/mymoneyfile.h @@ -1625,6 +1625,11 @@ private: = const MyMoneyAccount openingBalanceAccount_internal(const MyMoneySecurit= y& security) const; = + /** + * Make sure that the splits value has the precision of the correspondin= g account + */ + void fixSplitPrecision(MyMoneyTransaction& t) const; + private: /// \internal d-pointer class. class Private; diff --git a/kmymoney/mymoney/mymoneyfiletest.cpp b/kmymoney/mymoney/mymone= yfiletest.cpp index 9903ed4..f685466 100644 --- a/kmymoney/mymoney/mymoneyfiletest.cpp +++ b/kmymoney/mymoney/mymoneyfiletest.cpp @@ -2537,3 +2537,130 @@ void MyMoneyFileTest::testClearedBalance() QFAIL("Unexpected exception!"); } } + +void MyMoneyFileTest::testAdjustedValues() +{ + // create a checking account, an expeense, an investment account and a s= tock + AddOneAccount(); + + MyMoneyAccount exp1; + exp1.setAccountType(MyMoneyAccount::Expense); + exp1.setName("Expense1"); + exp1.setCurrencyId("EUR"); + + MyMoneyFileTransaction ft; + try { + MyMoneyAccount parent =3D m->expense(); + m->addAccount(exp1, parent); + ft.commit(); + } catch (const MyMoneyException &) { + QFAIL("Unexpected exception!"); + } + + testAddEquityAccount(); + + testBaseCurrency(); + MyMoneySecurity stockSecurity(QLatin1String("Blubber"), QLatin1String("T= estsockSecurity"), QLatin1String("BLUB"), 1000, 1000, 1000); + stockSecurity.setTradingCurrency(QLatin1String("BLUB")); + // add the security + ft.restart(); + try { + m->addSecurity(stockSecurity); + ft.commit(); + } catch (const MyMoneyException &e) { + unexpectedException(e); + } + + MyMoneyAccount i =3D m->accountByName("Investment"); + MyMoneyAccount stock; + ft.restart(); + try { + stock.setName("Teststock"); + stock.setCurrencyId(stockSecurity.id()); + stock.setAccountType(MyMoneyAccount::Stock); + m->addAccount(stock, i); + ft.commit(); + } catch (const MyMoneyException &e) { + unexpectedException(e); + } + + // values taken from real example on https://bugs.kde.org/show_bug.cgi?i= d=3D345655 + MyMoneySplit s1, s2, s3; + s1.setAccountId(QLatin1String("A000001")); + s1.setShares(MyMoneyMoney(QLatin1String("-99901/1000"))); + s1.setValue(MyMoneyMoney(QLatin1String("-999/10"))); + + s2.setAccountId(exp1.id()); + s2.setShares(MyMoneyMoney(QLatin1String("-611/250"))); + s2.setValue(MyMoneyMoney(QLatin1String("-61/25"))); + + s3.setAccountId(stock.id()); + s3.setAction(MyMoneySplit::BuyShares); + s3.setShares(MyMoneyMoney(QLatin1String("64901/100000"))); + s3.setPrice(MyMoneyMoney(QLatin1String("157689/1000"))); + s3.setValue(MyMoneyMoney(QLatin1String("102340161/1000000"))); + + MyMoneyTransaction t; + t.setCommodity(QLatin1String("EUR")); + t.setPostDate(QDate::currentDate()); + t.addSplit(s1); + t.addSplit(s2); + t.addSplit(s3); + + // make sure the split sum is not zero + QVERIFY(!t.splitSum().isZero()); + + ft.restart(); + try { + m->addTransaction(t); + ft.commit(); + } catch (const MyMoneyException &) { + QFAIL("Unexpected exception!"); + } + + QCOMPARE(t.splitById(s1.id()).shares(), MyMoneyMoney(QLatin1String("-999= /10"))); + QCOMPARE(t.splitById(s1.id()).value(), MyMoneyMoney(QLatin1String("-999/= 10"))); + + QCOMPARE(t.splitById(s2.id()).shares(), MyMoneyMoney(QLatin1String("-61/= 25"))); + QCOMPARE(t.splitById(s2.id()).value(), MyMoneyMoney(QLatin1String("-61/2= 5"))); + + QCOMPARE(t.splitById(s3.id()).shares(), MyMoneyMoney(QLatin1String("649/= 1000"))); + QCOMPARE(t.splitById(s3.id()).value(), MyMoneyMoney(QLatin1String("10234= /100"))); + QCOMPARE(t.splitById(s3.id()).price(), MyMoneyMoney(QLatin1String("15768= 9/1000"))); + QCOMPARE(t.splitSum(), MyMoneyMoney()); + + // now reset and check if modify also works + s1.setShares(MyMoneyMoney(QLatin1String("-999/10"))); + s1.setValue(MyMoneyMoney(QLatin1String("-999/10"))); + + s2.setShares(MyMoneyMoney(QLatin1String("-61/25"))); + s2.setValue(MyMoneyMoney(QLatin1String("-61/25"))); + + s3.setShares(MyMoneyMoney(QLatin1String("649/1000"))); + s3.setPrice(MyMoneyMoney(QLatin1String("157689/1000"))); + s3.setValue(MyMoneyMoney(QLatin1String("102340161/1000000"))); + + t.modifySplit(s1); + t.modifySplit(s2); + t.modifySplit(s3); + + // make sure the split sum is not zero + QVERIFY(!t.splitSum().isZero()); + + ft.restart(); + try { + m->modifyTransaction(t); + ft.commit(); + } catch (const MyMoneyException &) { + QFAIL("Unexpected exception!"); + } + + // we need to get the transaction from the engine, as modifyTransaction = does + // not return the modified values + MyMoneyTransaction t2 =3D m->transaction(t.id()); + + QCOMPARE(t2.splitById(s3.id()).shares(), MyMoneyMoney(QLatin1String("649= /1000"))); + QCOMPARE(t2.splitById(s3.id()).value(), MyMoneyMoney(QLatin1String("1023= 4/100"))); + QCOMPARE(t2.splitById(s3.id()).price(), MyMoneyMoney(QLatin1String("1576= 89/1000"))); + QCOMPARE(t2.splitSum(), MyMoneyMoney()); +} diff --git a/kmymoney/mymoney/mymoneyfiletest.h b/kmymoney/mymoney/mymoneyf= iletest.h index 657ed39..f7e38ad 100644 --- a/kmymoney/mymoney/mymoneyfiletest.h +++ b/kmymoney/mymoney/mymoneyfiletest.h @@ -101,6 +101,7 @@ private slots: void testOnlineJobRollback(); void testModifyOnlineJob(); void testClearedBalance(); + void testAdjustedValues(); = private slots: void objectAdded(MyMoneyFile::notificationObjectT type, const MyMoneyObj= ect * const obj);