[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [kmymoney/frameworks] kmymoney/mymoney: Reduce the precision of shares and values
From: Cristian_Oneț <onet.cristian () gmail ! com>
Date: 2016-06-13 13:30:13
Message-ID: E1bCRwT-0005T4-Pc () scm ! kde ! org
[Download RAW message or body]
Git commit dd5bfdd80e64ec30ac10b13c6a0dd7aa818bd1d2 by Cristian Oneț, 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 currency.
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/mymoneyfile.cpp
index d93f891..bf1e022 100644
--- a/kmymoney/mymoney/mymoneyfile.cpp
+++ b/kmymoney/mymoney/mymoneyfile.cpp
@@ -498,8 +498,7 @@ void MyMoneyFile::modifyTransaction(const MyMoneyTransaction& \
transaction) {
d->checkTransaction(Q_FUNC_INFO);
- const MyMoneyTransaction* t = &transaction;
- MyMoneyTransaction tCopy;
+ MyMoneyTransaction tCopy(transaction);
// now check the splits
bool loanAccountAffected = false;
@@ -519,7 +518,6 @@ void MyMoneyFile::modifyTransaction(const MyMoneyTransaction& \
transaction) // change transfer splits between asset/liability and loan accounts
// into amortization splits
if (loanAccountAffected) {
- tCopy = transaction;
QList<MyMoneySplit> list = transaction.splits();
for (it_s = list.constBegin(); it_s != list.constEnd(); ++it_s) {
if ((*it_s).action() == MyMoneySplit::ActionTransfer) {
@@ -529,7 +527,6 @@ void MyMoneyFile::modifyTransaction(const MyMoneyTransaction& \
transaction) MyMoneySplit s = (*it_s);
s.setAction(MyMoneySplit::ActionAmortization);
tCopy.modifySplit(s);
- t = &tCopy;
}
}
}
@@ -549,12 +546,15 @@ void MyMoneyFile::modifyTransaction(const MyMoneyTransaction& \
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 = t->splits().constBegin(); it_s != t->splits().constEnd(); ++it_s) {
- d->addCacheNotification((*it_s).accountId(), t->postDate());
+ for (it_s = tCopy.splits().constBegin(); it_s != tCopy.splits().constEnd(); \
++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::countTransactionsWithSpecificReconciliationState(const QString& return \
transactionList(filter).count(); }
+ /**
+ * Make sure that the splits value has the precision of the corresponding account
+ */
+void MyMoneyFile::fixSplitPrecision(MyMoneyTransaction& t) const
+{
+ QList<MyMoneySplit>::iterator its;
+ MyMoneySecurity transactionSecurity = security(t.commodity());
+ int transactionFraction = transactionSecurity.smallestAccountFraction();
+
+ for(its = t.splits().begin(); its != t.splits().end(); ++its) {
+ MyMoneyAccount acc = account((*its).accountId());
+ int fraction = acc.fraction();
+ if(fraction == -1) {
+ MyMoneySecurity sec = security(acc.currencyId());
+ fraction = acc.fraction(sec);
+ }
+ (*its).setShares((*its).shares().convertDenominator(fraction).canonicalize());
+ (*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 MyMoneySecurity& \
security) const;
+ /**
+ * Make sure that the splits value has the precision of the corresponding account
+ */
+ void fixSplitPrecision(MyMoneyTransaction& t) const;
+
private:
/// \internal d-pointer class.
class Private;
diff --git a/kmymoney/mymoney/mymoneyfiletest.cpp \
b/kmymoney/mymoney/mymoneyfiletest.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 stock
+ AddOneAccount();
+
+ MyMoneyAccount exp1;
+ exp1.setAccountType(MyMoneyAccount::Expense);
+ exp1.setName("Expense1");
+ exp1.setCurrencyId("EUR");
+
+ MyMoneyFileTransaction ft;
+ try {
+ MyMoneyAccount parent = m->expense();
+ m->addAccount(exp1, parent);
+ ft.commit();
+ } catch (const MyMoneyException &) {
+ QFAIL("Unexpected exception!");
+ }
+
+ testAddEquityAccount();
+
+ testBaseCurrency();
+ MyMoneySecurity stockSecurity(QLatin1String("Blubber"), \
QLatin1String("TestsockSecurity"), 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 = 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?id=345655
+ 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/25")));
+
+ 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("157689/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 = m->transaction(t.id());
+
+ QCOMPARE(t2.splitById(s3.id()).shares(), MyMoneyMoney(QLatin1String("649/1000")));
+ QCOMPARE(t2.splitById(s3.id()).value(), MyMoneyMoney(QLatin1String("10234/100")));
+ QCOMPARE(t2.splitById(s3.id()).price(), \
MyMoneyMoney(QLatin1String("157689/1000"))); + QCOMPARE(t2.splitSum(), \
MyMoneyMoney()); +}
diff --git a/kmymoney/mymoney/mymoneyfiletest.h b/kmymoney/mymoney/mymoneyfiletest.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 MyMoneyObject * \
const obj);
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic