[Kmymoney-devel] A bug in Git master

[Chris Tucker]

During OFX import when direct downloading from the brokerage there isn't a popup box for matching securities, as there likely shouldn't be.  Since the OFX import should just pull in the transactions and try to match, this make sense.  I might be able to get a OFX transaction to test with if you need it but I will try explaining first so you can see the problem in the code.

My line numbers might be off a few since I have modified code.

I have a security in another brokerage account with the same ticker symbol but that is named differently.  Each broker has a slightly different name even though both are the same ETF.  The first broker has VOE with the name "Vanguard Index FDS Mid-Cap Value Index (VOE)" but the second one comes in from OFX with the name "VANGUARD INDEX FDS MCAP VL IDXVIP".  Ideally this would be the same security in KMM since the returns in reports should be combined.  We could have multiple securities but it would be messier on other screens and reports.

The problem is that the security is found during import when it is checked in MyMoneyStatementReader::processSecurityEntry() on line 498:

  while (it != list.constEnd() && security.id().isEmpty()) {
    if (sec_in.m_strSymbol.isEmpty()) {
      if ((*it).name() == sec_in.m_strName)
        security = *it;
    } else if ((*it).tradingSymbol() == sec_in.m_strSymbol)  <----- Security found here
      security = *it;
    ++it;
  }

So the reader does not create the security since it already found it during the security import loop.  But when it gets to transaction import in MyMoneyStatementReader::processTransactionEntry() then it doesn't search for securities by symbol and just matches the names.  Since the names do not match, it just fails and presents the messagebox at line 656.  According to the comments on lines 621-623, we rely on the security import loop to insert securities and not the transaction import loop:

        // The security should always be available, because the statement file
        // should separately list all the securities referred to in the file,
        // and when we found a security, we added it to the file.

So in our security loop on line 633 we compare the OFX security name to the iterator symbol and the OFX security name to the iterator name we don't find the correct security and hence the error.

My patch may have one problem, though, since I assumed the OFX security name should not be compared to the iterator symbol and changed the code to compare symbol -> symbol or name -> name.  Maybe there was a reason for the name -> symbol comparison and the code should read:

            if (statementTransactionUnderImport.m_strSecurity.toLower() == (*it).tradingSymbol().toLower()
                || statementTransactionUnderImport.m_strSymbol.toLower() == (*it).tradingSymbol().toLower()
                || statementTransactionUnderImport.m_strSecurity.toLower() == (*it).name().toLower()) {

In my testing I never found a reason to compare the OFX name to symbol.

--

Chris


On Monday, November 25, 2013 10:51:13 PM Allan wrote:
> On 25/11/13 01:49, Chris Tucker wrote:
> > Vanguard Index FDS Mid-Cap Value Index (VOE)
> 
> @ Thomas please, below.
> 
> Hi Chris
> 
> I've had a quick look at your patch, and, while I've not sat and thought 
> about it in detail, it does look OK.
> 
> However, my first thought was 'How do I test it, as I don't have OFX?' 
> Then, your patch is for  mymoneystatementreader.cpp, which is common to 
> all input methods, so I knocked up a simple CSV file.  First, I entered 
> into KMM, your original security, with a single Buy transaction.  Then, 
> I imported my CSV file containing a Dividend Reinvest for your imported 
> security.  The CSV i9mporter, prior to the actual import, displays the 
> data from the file, to give the user the chance to edit security names 
> to correspond to existing securities.  This recognises the existing 
> symbol and security, and doesn't show the new data.  I proceed, and 
> continue the import.  The import statement stats show one transaction 
> processed and one added, with no duplicates.  Looking now at the ledger 
> for the import account, both transactions appear with their own distinct 
> security names, and the investment view contains both securities with 
> their own separate names.  Back in the Ledger, starting a new 
> transaction gives a choice of either security, so everything seems OK, 
> unless you actually wanted the two securities to match, rather than be 
> independent.  You say "...the transaction import fails to find the 
> security by name.", and it can't because it's dealing with a different 
> name, but it actually finds it by symbol.
> 
> I have not used your proposed patch.  Forgive my asking, but I'm 
> assuming you tested your patch?  Did it give the result you wanted, and 
> to be clear, you do want the two to match?
> 
> @ Thomas please, below.
> The next point is of principal.  I've always taken it that a symbol 
> needs to be unique within KMM, and I can see possible problems having 
> two different securities with the same symbol.  Of course, this would 
> resolve if the two securities can actually match.
> 
> Allan
> 
>