Bug 363069 - Wrong accounts are presented during import of investment statement
Summary: Wrong accounts are presented during import of investment statement
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: importer (show other bugs)
Version: git (master)
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-14 14:10 UTC by wojnilowicz
Modified: 2016-09-21 11:14 UTC (History)
2 users (show)

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


Attachments
Account selection (33.77 KB, image/png)
2016-05-14 14:11 UTC, wojnilowicz
Details
Account selection (43.23 KB, image/png)
2016-05-14 14:13 UTC, wojnilowicz
Details
Account selection proposal (27.72 KB, image/png)
2016-05-14 14:28 UTC, wojnilowicz
Details
Account selection proposal (30.91 KB, image/png)
2016-05-14 14:28 UTC, wojnilowicz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description wojnilowicz 2016-05-14 14:10:21 UTC
It's not a bug, it's proposal of improvement. During import of CSV investment statement my investment account isn't automatically detected so I get asked about "Account selection".  I should choose an investment account, but I can also choose e.g. savings account which isn't appropriate for securities to be imported. Such accounts should be filtered out and only right ones should be shown.

Reproducible: Always

Steps to Reproduce:
1. File->Import->CSV... 
2. choose Investing,
3. proceed with importing till question about "Account Selection"

Actual Results:  
All asset and liabilities accounts are presented.

Expected Results:  
Only investment accounts should be presented, as it's single right choice and other choices are wrong.
Comment 1 wojnilowicz 2016-05-14 14:11:02 UTC
Created attachment 98964 [details]
Account selection
Comment 2 wojnilowicz 2016-05-14 14:13:38 UTC
Created attachment 98965 [details]
Account selection
Comment 3 wojnilowicz 2016-05-14 14:28:08 UTC
Created attachment 98966 [details]
Account selection proposal
Comment 4 wojnilowicz 2016-05-14 14:28:56 UTC
Created attachment 98967 [details]
Account selection proposal
Comment 5 Jack 2016-05-14 15:22:16 UTC
I can't comment on the code itself, but in the initial bug report, you talk about csv import, but it looks to me like your proposed changes will apply to any import.  How are you checking that the import is for an investment account?  If there is actually something in the import, this is great.  If it is only because you have to specify that it is an investment import for the csv importer, then it should be restricted to that situation.  If the proposed changes actually take care of that, you can ignore my comment.
Comment 6 wojnilowicz 2016-05-14 16:02:50 UTC
(In reply to Jack from comment #5)
> I can't comment on the code itself, but in the initial bug report, you talk
> about csv import, but it looks to me like your proposed changes will apply
> to any import.  How are you checking that the import is for an investment
> account?  If there is actually something in the import, this is great.  If
> it is only because you have to specify that it is an investment import for
> the csv importer, then it should be restricted to that situation.  If the
> proposed changes actually take care of that, you can ignore my comment.

Yes, you're right. It will apply to any import because it was easy to extend this feature on all imports. 
Type of import is today determined right before "Account selection" so no problem to identify which kind of import we make. If you want to analyze code yourself, then look into mymoneystatementreader.cpp, function "import", place right before "selectOrCreateAccount".
Comment 7 wojnilowicz 2016-05-28 18:12:42 UTC
Git commit bfa7b5ac37d2203e4a88d9ac73b1ec43ce5211b4 by Łukasz Wojniłowicz.
Committed on 28/05/2016 at 17:59.
Pushed by wojnilowicz into branch 'master'.

Present only right accounts during import of statements

During import of investment statement user is asked to select account
into which he wants to import. He should choose investment account, but
he is also presented with all other sorts of accounts. This patch makes
sure that user is presented only with the right account types.
REVIEW: 127915

Signed-off-by: Łukasz Wojniłowicz <lukasz.wojnilowicz@gmail.com>

M  +14   -2    kmymoney/converter/mymoneystatementreader.cpp
M  +8    -0    kmymoney/dialogs/kaccountselectdlg.cpp
M  +11   -7    kmymoney/kmymoneyutils.h

http://commits.kde.org/kmymoney/bfa7b5ac37d2203e4a88d9ac73b1ec43ce5211b4
Comment 8 wojnilowicz 2016-05-28 18:29:38 UTC
Git commit 1ab18d6b13b20af947a3c43e9a67a1db1e97d430 by Łukasz Wojniłowicz.
Committed on 28/05/2016 at 18:27.
Pushed by wojnilowicz into branch 'frameworks'.

Present only right accounts during import of statements

During import of investment statement user is asked to select account
into which he wants to import. He should choose investment account, but
he is also presented with all other sorts of accounts. This patch makes
sure that user is presented only with the right account types.
REVIEW: 127915

Signed-off-by: Łukasz Wojniłowicz <lukasz.wojnilowicz@gmail.com>

M  +14   -2    kmymoney/converter/mymoneystatementreader.cpp
M  +8    -0    kmymoney/dialogs/kaccountselectdlg.cpp
M  +11   -7    kmymoney/kmymoneyutils.h

http://commits.kde.org/kmymoney/1ab18d6b13b20af947a3c43e9a67a1db1e97d430
Comment 9 Jakub Jamro 2016-09-21 11:14:43 UTC
Hi,

After this feature has been delivered to version 4.8, I am now no longer able to import QIF statements to a Savings, Credit Card or Cash accounts. While I am able to change some account types to current, it does not make sense for me to do so with my credit card account.

I'm not familiar with the code base and it does seem that the QIF !Type: header is being parsed and interpreted correctly, however this those not seem to be considered when selecting the statement type which then is used to restrict the accounts available in the account selection dialog.

Also even if this was considered, I currently have bank exports from my bank Santander in the UK, which use !Type:Oth L for their Current and Savings accounts downloads, meaning I would have to manually modify these downloads in order to be able to select the correct account.

I would propose that actually selecting the correct account type for a particular QIF is very unreliable as not all banks follow the spec correctly.