Bug 328127

Summary: mymoneystatementreader.cpp matching problem
Product: kmymoney Reporter: allan <agander93>
Component: generalAssignee: KMyMoney Devel Mailing List <kmymoney-devel>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: git (master)   
Target Milestone: ---   
Platform: Mint (Ubuntu based)   
OS: Linux   
Latest Commit: Version Fixed In:

Description allan 2013-11-26 22:50:00 UTC
Many thanks to Chris Tucker for producing a patch for this problem, and his description -
"
OFX import doesn't match securities with different names but same symbol.  For example, I direct-connect to download OFX and pick up a security:

<SECURITY symbol="VOE" id="922908512" name="VANGUARD INDEX FDS MCAP VL IDXVIP"/>

The security exists already as VOE with name "Vanguard Index FDS Mid-Cap Value Index (VOE)". 

mymoneystatementreader.cpp finds the existing security by ticker but the transaction import fails to find the security by name.  The following patch to mymoneystatementreader.cpp solves the problem:

*** mymoneystatementreader.cpp.orig     2013-11-24 17:27:39.959812598 -0800
--- mymoneystatementreader.cpp  2013-11-24 17:27:44.436812841 -0800
***************
*** 630,636 ****
            QList<MyMoneySecurity> list = MyMoneyFile::instance()->securityList();
            QList<MyMoneySecurity>::ConstIterator it = list.constBegin();
            while (it != list.constEnd() && security.id().isEmpty()) {
!             if (statementTransactionUnderImport.m_strSecurity.toLower() == (*it).tradingSymbol().toLower()
                  || statementTransactionUnderImport.m_strSecurity.toLower() == (*it).name().toLower()) {
                security = *it;
              }
--- 630,636 ----
            QList<MyMoneySecurity> list = MyMoneyFile::instance()->securityList();
            QList<MyMoneySecurity>::ConstIterator it = list.constBegin();
            while (it != list.constEnd() && security.id().isEmpty()) {
!             if (statementTransactionUnderImport.m_strSymbol.toLower() == (*it).tradingSymbol().toLower()
                  || statementTransactionUnderImport.m_strSecurity.toLower() == (*it).name().toLower()) {
                security = *it;
              }
"
While investigating this problem, using the CSV importer, I encountered some other issues - to follow.



Reproducible: Always

Steps to Reproduce:
1.Create an investment transaction with a new symbol.
2.Import another investment transaction using the same symbol, but a non-identical name.
3.The two should get matched, but the second was being rejected because the symbol match was failing.
Actual Results:  
As above

Expected Results:  
The two transactions should get matched.
Comment 1 allan 2013-11-27 00:39:14 UTC
Not having an OFX source, I tested using the CSV importer, expecting that this was a reasonable approach because the provided patch was to code common to all import sources.  However, whereas the user received a message box, indicating that the transaction would be dropped because no security was found, this did not happen with CSV.  Instead, a new security was created.
This was traced to MyMoneyStatementReader::processSecurityEntry line 497 -
"} else if ((*it).tradingSymbol() == sec_in.m_strSymbol)" which was not finding a match because the two symbols were of different case, and '.toLower()' needed to be added to both sides.  The CSV importer now produced the same message box as in OFX import.
The immediate cause of this was that, at the point were the patch had been installed, the symbol there was empty.
Finally, this was traced to void InvestProcessing::investCsvImport(MyMoneyStatement& st), where tr.m_strSymbol = (*it_s).m_strSymbol needed to be added.

I'll be adding a patch for the full fix.
Comment 2 allan 2013-12-18 21:00:36 UTC
The patch to fix this problem was committed in Git commit 22107d5c17f7a7c55b2abe0e09efc09c05e419a8 by Allan Anderson.
Committed on 09/12/2013 at 21:58.
Pushed by allananderson into branch 'master'.

M  +8    -3    kmymoney/converter/mymoneystatementreader.cpp
M  +1    -0    kmymoney/plugins/csvimport/investprocessing.cpp

http://commits.kde.org/kmymoney/22107d5c17f7a7c55b2abe0e09efc09c05e419a8

BUG:327806 - Investment transactions dropped without notification during import.
Comment 3 allan 2013-12-27 23:26:29 UTC
Git commit 8c1f883fc302c639a5ee326ea226b2daf5a7a105 by Allan Anderson.
Committed on 18/12/2013 at 21:12.
Pushed by allananderson into branch 'master'.
Related: bug 328776
without assigning a brokerage or checking account.
Also, improve wording of heading label of symboltabledlg.ui.

M  +6    -5    kmymoney/converter/mymoneystatementreader.cpp
M  +9    -4    kmymoney/plugins/csvimport/investprocessing.cpp
M  +1    -0    kmymoney/plugins/csvimport/investprocessing.h
M  +5    -3    kmymoney/plugins/csvimport/symboltabledlg.ui

http://commits.kde.org/kmymoney/8c1f883fc302c639a5ee326ea226b2daf5a7a105