Git commit 50da0c1ac7d911c024e8d4b1b9cd0a925aba858f by Dawid Wróbel. Committed on 17/04/2020 at 00:00. Pushed by wrobelda into branch 'master'.
Add option to enable fixing of the buy/sell signs Some providers do not follow the OFX spec and do not assign the correct positive/negative signage to amount (TOTALs) and quantity (UNITs) values assosciated with BUY/SELL investment transactions (see OFX2.2, section 13.3.3). This change adds option to OFX import dialog and account's online settings to force the signage to follow the the spec. BUG: 420056 GUI: M +1 -0 kmymoney/plugins/ofx/import/dialogs/konlinebankingstatus.cpp M +19 -2 kmymoney/plugins/ofx/import/dialogs/konlinebankingstatusdecl.ui M +18 -1 kmymoney/plugins/ofx/import/importoption.ui M +96 -74 kmymoney/plugins/ofx/import/ofximporter.cpp https://commits.kde.org/kmymoney/50da0c1ac7d911c024e8d4b1b9cd0a925aba858f diff --git a/kmymoney/plugins/ofx/import/dialogs/konlinebankingstatus.cpp b/kmymoney/plugins/ofx/import/dialogs/konlinebankingstatus.cpp index 1fb1d1afd..e5efc56b4 100644 --- a/kmymoney/plugins/ofx/import/dialogs/konlinebankingstatus.cpp +++ b/kmymoney/plugins/ofx/import/dialogs/konlinebankingstatus.cpp @@ -108,6 +108,7 @@ KOnlineBankingStatus::KOnlineBankingStatus(const MyMoneyAccount& acc, QWidget *p m_timestampOffset->setTime(QTime::fromMSecsSinceStartOfDay(qAbs(offset)*60*1000)); m_invertAmount->setChecked(settings.value("kmmofx-invertamount").toLower() == QStringLiteral("yes")); + m_fixBuySellSignage->setChecked(settings.value("kmmofx-fixbuysellsignage").toLower() == QStringLiteral("yes")); QString key = OFX_PASSWORD_KEY(settings.value("url"), settings.value("uniqueId")); QString pwd; diff --git a/kmymoney/plugins/ofx/import/dialogs/konlinebankingstatusdecl.ui b/kmymoney/plugins/ofx/import/dialogs/konlinebankingstatusdecl.ui index 8c63bb82c..03d7b50d5 100644 --- a/kmymoney/plugins/ofx/import/dialogs/konlinebankingstatusdecl.ui +++ b/kmymoney/plugins/ofx/import/dialogs/konlinebankingstatusdecl.ui @@ -7,7 +7,7 @@ <x>0</x> <y>0</y> <width>638</width> - <height>392</height> + <height>435</height> </rect> </property> <layout class="QVBoxLayout" name="verticalLayout_5"> @@ -383,7 +383,7 @@ <property name="title"> <string>Import options</string> </property> - <layout class="QGridLayout" name="gridLayout_3" rowstretch="2,0,0,0"> + <layout class="QGridLayout" name="gridLayout_3" rowstretch="2,0,0,0,0"> <item row="0" column="1"> <widget class="QComboBox" name="m_preferredPayee"> <item> @@ -485,6 +485,23 @@ </property> </widget> </item> + <item row="4" column="0"> + <widget class="QLabel" name="label_4"> + <property name="text"> + <string>Fix sign of the investement transaction amount and quantity</string> + </property> + </widget> + </item> + <item row="4" column="1"> + <widget class="QCheckBox" name="m_fixBuySellSignage"> + <property name="toolTip"> + <string><html><head/><body><p>Check this if the investment transactions after importing have incorrect Buy/Sell type assigned, which may be caused by your institution not applying the correct negative/positive sign to the share amount or quantity value.</p></body></html></string> + </property> + <property name="text"> + <string/> + </property> + </widget> + </item> </layout> </widget> </item> diff --git a/kmymoney/plugins/ofx/import/importoption.ui b/kmymoney/plugins/ofx/import/importoption.ui index ce9484229..d59558788 100644 --- a/kmymoney/plugins/ofx/import/importoption.ui +++ b/kmymoney/plugins/ofx/import/importoption.ui @@ -7,7 +7,7 @@ <x>0</x> <y>0</y> <width>598</width> - <height>184</height> + <height>218</height> </rect> </property> <layout class="QVBoxLayout" name="verticalLayout"> @@ -130,6 +130,23 @@ </property> </widget> </item> + <item row="4" column="0"> + <widget class="QLabel" name="label_4"> + <property name="text"> + <string>Fix sign of the investement transaction amount and quantity</string> + </property> + </widget> + </item> + <item row="4" column="1"> + <widget class="QCheckBox" name="m_fixBuySellSignage"> + <property name="toolTip"> + <string><html><head/><body><p>Check this if the investment transactions after importing have incorrect Buy/Sell type assigned, which may be caused by your institution not applying the correct negative/positive sign to the share amount or quantity value.</p></body></html></string> + </property> + <property name="text"> + <string/> + </property> + </widget> + </item> </layout> </widget> </item> diff --git a/kmymoney/plugins/ofx/import/ofximporter.cpp b/kmymoney/plugins/ofx/import/ofximporter.cpp index f5a120c56..33d36c26f 100644 --- a/kmymoney/plugins/ofx/import/ofximporter.cpp +++ b/kmymoney/plugins/ofx/import/ofximporter.cpp @@ -74,7 +74,7 @@ typedef enum { class OFXImporter::Private { public: - Private() : m_valid(false), m_preferName(PreferId), m_uniqueIdSource(UniqueIdUnknown), m_walletIsOpen(false), m_invertAmount(false), m_statusDlg(0), m_wallet(0), + Private() : m_valid(false), m_preferName(PreferId), m_uniqueIdSource(UniqueIdUnknown), m_walletIsOpen(false), m_invertAmount(false), m_fixBuySellSignage(false), m_statusDlg(0), m_wallet(0), m_updateStartDate(QDate(1900,1,1)), m_timestampOffset(0) {} bool m_valid; @@ -86,6 +86,7 @@ public: UniqueTransactionIdSource m_uniqueIdSource; bool m_walletIsOpen; bool m_invertAmount; + bool m_fixBuySellSignage; QList<MyMoneyStatement> m_statementlist; QList<MyMoneyStatement::Security> m_securitylist; QString m_fatalerror; @@ -185,6 +186,7 @@ void OFXImporter::slotImportFile() d->m_uniqueIdSource = static_cast<UniqueTransactionIdSource>(option->m_uniqueIdSource->currentIndex()); d->m_timestampOffset = d->constructTimeOffset(option->m_timestampOffset, option->m_timestampOffsetSign); d->m_invertAmount = option->m_invertAmount->isChecked(); + d->m_fixBuySellSignage = option->m_fixBuySellSignage->isChecked(); if (url.isValid()) { const QString filename(url.toLocalFile()); @@ -359,8 +361,95 @@ int OFXImporter::ofxTransactionCallback(struct OfxTransactionData data, void * p } } + bool unhandledtype = false; + QString type; + + if (data.invtransactiontype_valid) { + switch (data.invtransactiontype) { + case OFX_BUYDEBT: + case OFX_BUYMF: + case OFX_BUYOPT: + case OFX_BUYOTHER: + case OFX_BUYSTOCK: + t.m_eAction = eMyMoney::Transaction::Action::Buy; + break; + case OFX_REINVEST: + t.m_eAction = eMyMoney::Transaction::Action::ReinvestDividend; + break; + case OFX_SELLDEBT: + case OFX_SELLMF: + case OFX_SELLOPT: + case OFX_SELLOTHER: + case OFX_SELLSTOCK: + t.m_eAction = eMyMoney::Transaction::Action::Sell; + break; + case OFX_INCOME: + t.m_eAction = eMyMoney::Transaction::Action::CashDividend; + // NOTE: With CashDividend, the amount of the dividend should + // be in data.amount. Since I've never seen an OFX file with + // cash dividends, this is an assumption on my part. (acejones) + break; + + // + // These types are all not handled. We will generate a warning for them. + // + case OFX_CLOSUREOPT: + unhandledtype = true; + type = QStringLiteral("CLOSUREOPT (Close a position for an option)"); + break; + case OFX_INVEXPENSE: + unhandledtype = true; + type = QStringLiteral("INVEXPENSE (Misc investment expense that is associated with a specific security)"); + break; + case OFX_JRNLFUND: + unhandledtype = true; + type = QStringLiteral("JRNLFUND (Journaling cash holdings between subaccounts within the same investment account)"); + break; + case OFX_MARGININTEREST: + unhandledtype = true; + type = QStringLiteral("MARGININTEREST (Margin interest expense)"); + break; + case OFX_RETOFCAP: + unhandledtype = true; + type = QStringLiteral("RETOFCAP (Return of capital)"); + break; + case OFX_SPLIT: + unhandledtype = true; + type = QStringLiteral("SPLIT (Stock or mutial fund split)"); + break; + case OFX_TRANSFER: + unhandledtype = true; + type = QStringLiteral("TRANSFER (Transfer holdings in and out of the investment account)"); + break; + default: + unhandledtype = true; + type = QString("UNKNOWN %1").arg(data.invtransactiontype); + break; + } + } else + t.m_eAction = eMyMoney::Transaction::Action::None; + + t.m_shares = MyMoneyMoney(); + if (data.units_valid) { + t.m_shares = MyMoneyMoney(data.units, 100000).reduce(); + } + + t.m_amount = MyMoneyMoney(); if (data.amount_valid) { t.m_amount = MyMoneyMoney(data.amount, 1000); + + if (pofx->d->m_fixBuySellSignage) { + if (t.m_eAction == eMyMoney::Transaction::Action::Buy + || t.m_eAction == eMyMoney::Transaction::Action::ReinvestDividend) { + t.m_amount = -t.m_amount.abs(); + t.m_shares = t.m_shares.abs(); + } + else if (t.m_eAction == eMyMoney::Transaction::Action::Sell) { + t.m_amount = t.m_amount.abs(); + t.m_shares = -t.m_shares.abs(); + } + } + if (pofx->d->m_invertAmount) { t.m_amount = -t.m_amount; } @@ -498,11 +587,6 @@ int OFXImporter::ofxTransactionCallback(struct OfxTransactionData data, void * p } } - t.m_shares = MyMoneyMoney(); - if (data.units_valid) { - t.m_shares = MyMoneyMoney(data.units, 100000).reduce(); - } - t.m_price = MyMoneyMoney(); if (data.unitprice_valid) { t.m_price = MyMoneyMoney(data.unitprice, 100000).reduce(); @@ -517,74 +601,6 @@ int OFXImporter::ofxTransactionCallback(struct OfxTransactionData data, void * p t.m_fees += MyMoneyMoney(data.commission, 1000).reduce(); } - bool unhandledtype = false; - QString type; - - if (data.invtransactiontype_valid) { - switch (data.invtransactiontype) { - case OFX_BUYDEBT: - case OFX_BUYMF: - case OFX_BUYOPT: - case OFX_BUYOTHER: - case OFX_BUYSTOCK: - t.m_eAction = eMyMoney::Transaction::Action::Buy; - break; - case OFX_REINVEST: - t.m_eAction = eMyMoney::Transaction::Action::ReinvestDividend; - break; - case OFX_SELLDEBT: - case OFX_SELLMF: - case OFX_SELLOPT: - case OFX_SELLOTHER: - case OFX_SELLSTOCK: - t.m_eAction = eMyMoney::Transaction::Action::Sell; - break; - case OFX_INCOME: - t.m_eAction = eMyMoney::Transaction::Action::CashDividend; - // NOTE: With CashDividend, the amount of the dividend should - // be in data.amount. Since I've never seen an OFX file with - // cash dividends, this is an assumption on my part. (acejones) - break; - - // - // These types are all not handled. We will generate a warning for them. - // - case OFX_CLOSUREOPT: - unhandledtype = true; - type = QStringLiteral("CLOSUREOPT (Close a position for an option)"); - break; - case OFX_INVEXPENSE: - unhandledtype = true; - type = QStringLiteral("INVEXPENSE (Misc investment expense that is associated with a specific security)"); - break; - case OFX_JRNLFUND: - unhandledtype = true; - type = QStringLiteral("JRNLFUND (Journaling cash holdings between subaccounts within the same investment account)"); - break; - case OFX_MARGININTEREST: - unhandledtype = true; - type = QStringLiteral("MARGININTEREST (Margin interest expense)"); - break; - case OFX_RETOFCAP: - unhandledtype = true; - type = QStringLiteral("RETOFCAP (Return of capital)"); - break; - case OFX_SPLIT: - unhandledtype = true; - type = QStringLiteral("SPLIT (Stock or mutial fund split)"); - break; - case OFX_TRANSFER: - unhandledtype = true; - type = QStringLiteral("TRANSFER (Transfer holdings in and out of the investment account)"); - break; - default: - unhandledtype = true; - type = QString("UNKNOWN %1").arg(data.invtransactiontype); - break; - } - } else - t.m_eAction = eMyMoney::Transaction::Action::None; - // In the case of investment transactions, the 'total' is supposed to the total amount // of the transaction. units * unitprice +/- commission. Easy, right? Sadly, it seems // some ofx creators do not follow this in all circumstances. Therefore, we have to double- @@ -857,6 +873,11 @@ MyMoneyKeyValueContainer OFXImporter::onlineBankingSettings(const MyMoneyKeyValu } else { kvp.deletePair(QStringLiteral("kmmofx-invertamount")); } + if (d->m_statusDlg->m_fixBuySellSignage->isChecked()) { + kvp.setValue(QStringLiteral("kmmofx-fixbuysellsignage"), QStringLiteral("yes")); + } else { + kvp.deletePair(QStringLiteral("kmmofx-fixbuysellsignage")); + } // get rid of pre 4.6 values kvp.deletePair(QStringLiteral("kmmofx-preferPayeeid")); } @@ -918,6 +939,7 @@ bool OFXImporter::updateAccount(const MyMoneyAccount& acc, bool moreAccounts) } d->m_invertAmount = settings.value("kmmofx-invertamount").toLower() == QStringLiteral("yes"); + d->m_fixBuySellSignage= settings.value("kmmofx-fixbuysellsignage").toLower() == QStringLiteral("yes"); } d->m_timestampOffset = settings.value("kmmofx-timestampOffset").toInt(); //kDebug(0) << "ofx plugin: account" << acc.name() << "earliest transaction date to process =" << qPrintable(d->m_updateStartDate.toString(Qt::ISODate));
