Bug 327806 - Investment transactions dropped without notification during import.
Summary: Investment transactions dropped without notification during import.
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: general (show other bugs)
Version: git (master)
Platform: Mint (Ubuntu based) Linux
: NOR normal
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-19 11:38 UTC by allan
Modified: 2013-12-09 22:18 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Investment transactions dropped without notification during import and mymoneystatementreader.cpp matching problem. (3.91 KB, patch)
2013-11-27 14:29 UTC, allan
Details

Note You need to log in before you can comment on or make changes to this bug.
Description allan 2013-11-19 11:38:21 UTC
I was importing investment transactions, but the statement stats showed none had been imported, although no error messages appeared.   The error occurred because I imported into the wrong account, which did not include that particular security.
There is the following at line 649  in mymoneystatementreader.cpp -
"else {
            // This should be rare.  A statement should have a security entry for any
            // of the securities referred to in the transactions.  The only way to get
            // here is if that's NOT the case.
            KMessageBox::information(0, i18n("This investment account does not contain the \"%1\" security.  Transactions involving this security will be ignored.", statementTransactionUnderImport.m_strSecurity), i18n("Security not found"), QString("MissingSecurity%1").arg(statementTransactionUnderImport.m_strSecurity.trimmed()));
            return;"

That code is executed but the message box does not appear.
This was during a CSV import, but I think I've seen the same on a QIF import also.


Reproducible: Always

Steps to Reproduce:
1. Import a file containing security statements.
2.When prompted, select an investment account that does not include the security in question.
3.The import finishes without that message box, and the log shows the progress without any indication of error.
Actual Results:  
As above

Expected Results:  
The error should produce the message box to inform the user of the error.
Comment 1 allan 2013-11-19 12:37:44 UTC
The imported file does contain the security name, and imports correctly into an account which contains that security.
When another file, formatted differently, and containing security symbols, is imported into an account that does not include those securities, then new securities are created.

So, it appears that the imports work correctly, except for the non-appearance of the message box.

Truncating the instruction to - 
"
KMessageBox::information(0, i18n("This investment account does not contain the \"%1\" security.  Transactions involving this security will be ignored.", statementTransactionUnderImport.m_strSecurity), i18n("Security not found"));
"
allows the message box to appear as expected.
Comment 2 allan 2013-11-19 18:07:03 UTC
It looks like this is a feature of the "dontAskAgainName" feature, and I don't think I care for it, at least in this situation, as it might be some considerable time from accepting the "dontAskAgainName" question, until the problem recurs, by which time it's been forgotten, and time is wasted chasing a non-existent problem.

I think I would prefer a question asking whether or not to abort the import, especially when there could be quite a number of similar transactions being imported, where the only option is a forced need to abort KMM.
Comment 3 allan 2013-11-27 13:35:26 UTC
@Thomas again, please.
I've replaced the 
" KMessageBox::information(0, i18n("This investment account does not contain the \"%1\" security.  Transactions involving this security will be ignored.", statementTransactionUnderImport.m_strSecurity), i18n("Security not found"), QString("MissingSecurity%1").arg(statementTransactionUnderImport.m_strSecurity.trimmed()));"
with 
"int ret = KMessageBox::warningContinueCancel(0, i18n("<center>This investment account does not contain the \"%1\" security.</center>"
                                                        "<center>Transactions involving this security will be ignored.</center>", statementTransactionUnderImport.m_strSecurity),
                                                        i18n("Security not found"), KStandardGuiItem::cont(), KStandardGuiItem::cancel());
            if (ret == KMessageBox::Cancel) {
              m_userAbort = true;" to be less likely to allow the user to fall into a trap, and this, from my point of view, seems much better, and was going to ask if it was OK to go ahead.

However, something a bit unexpected has happened.  Having installed Chris Tucker's patch and my additions - see Bug 328127 - mymoneystatementreader.cpp matching problem -
when I came back to this and tested again, instead of encountering the new messagebox when importing into the 'wrong' investment account, instead the imported transaction resulted in the creation of the new security in that investment account.

In a way, this could be useful in creating a new investment on the fly, although it might not be so good if it has done this in the 'wrong account', ie, the user made a mistake.  I see no warning on the console of what was happening, and I'm inclined to accept this 'new functionality', but to add a warning message box for the user to accept or cancel.

Any thoughts, please?
Comment 4 allan 2013-11-27 14:29:53 UTC
Created attachment 83794 [details]
Investment transactions dropped without notification during import and mymoneystatementreader.cpp matching problem.

Here's the combined patch for the two bugs.
Comment 5 allan 2013-12-09 22:10:18 UTC
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
Comment 6 allan 2013-12-09 22:18:33 UTC
I have not yet committed the patch submitted by Chris Tucker referred to above, but only the changes related to the use of KMessageBox::warningContinueCancel, plus the addition of adding tr.m_strSymbol = (*it_s).m_strSymbol to inInvestProcessing::investCsvImport(), discovered during testing.