[kmymoney/bug-420056] kmymoney/plugins/ofx/import: Add option to enable fixing of the buy/sell signs

Dawid Wróbel null at kde.org
Wed Apr 15 23:54:55 BST 2020


Git commit 8bd68cd8a2e0ebadedc04ff665d6704d6395d8a0 by Dawid Wróbel.
Committed on 15/04/2020 at 22:54.
Pushed by wrobelda into branch 'bug-420056'.

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/8bd68cd8a2e0ebadedc04ff665d6704d6395d8a0

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));



More information about the kde-doc-english mailing list