[Kmymoney-devel] Re: Review Request: Fixes for problems when importing QIF files containing category/account sub-accounts

[Thomas Baumgart]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6705/#review10216
-----------------------------------------------------------


Just had a quick glimpse on the diff, no real testing done here. This is what I came across.


/trunk/extragear/office/kmymoney/kmymoney/converter/mymoneyqifreader.cpp
<http://svn.reviewboard.kde.org/r/6705/#comment11715>

    Please keep coding style "} else {"



/trunk/extragear/office/kmymoney/kmymoney/converter/mymoneyqifreader.cpp
<http://svn.reviewboard.kde.org/r/6705/#comment11716>

    Is this some debug leftover?



/trunk/extragear/office/kmymoney/kmymoney/converter/mymoneyqifreader.cpp
<http://svn.reviewboard.kde.org/r/6705/#comment11717>

    Don't you want this to be inside the 'else if' part? Otherwise it could happen, that tr.m_strBrokerageAccount == tr.m_strInterestCategory and that does not seem to be right.


- Thomas


On June 7, 2011, 7:44 p.m., Allan Anderson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6705/
> -----------------------------------------------------------
> 
> (Updated June 7, 2011, 7:44 p.m.)
> 
> 
> Review request for kmymoney.
> 
> 
> Summary
> -------
> 
> When importing a QIF investment file with Lcategory:sub-category, which
> indicates a sub-category for the transaction, the importation takes
> place without error. In the ledger view of the brokerage account all appears
> correct with the Category field or the Interest field displaying what appears
> to be Category:Sub-category.
> 
> However when inspecting the category list it is evident that only a single 
> new category has been created with the name of "Category:Sub-category" as 
> one word and the transaction has been allocated to this category with 
> nothing in the existing sub-category.
> 
> The exact same transaction can be done manually with no error or
> newly created categories. This bug only occurs in QIF import of investments.  
> The reason for this difference is that in imports, if the 'L' record is used 
> to nominate a category, then there is no means by which to specify a 
> transfer account.
> 
> Looking at the reasons for these problems, two routines were found to be 
> deficient.  In mymoneystatementreader.cpp, the routine
> d->interestId(t_in.m_strInterestCategory)) was found not to recognise a 
> category:sub-category structure already existing, and would create a new 
> category named like 'category:sub-category'.  When the categoryToAccount() 
> routine was substituted, this recognised and found the correct existing 
> sub-account, but did not create one if none existed.
> 
> Then, in the QIF file in question, transactions of type IntInc were involved,
> and, once the category structure was correctly recognised, the categories 
> created were created as income, when one of them should have been an expense.  
> As it happened, the statements in question included quantity and price values, 
> which KMM had decided were not relevant.  As the quantity record showed the 
> correct sign, changes were made to take notice of the quantity and price, 
> in order to allow a decision to be made on whether a transaction should be
> an income or an expense.  This should have no effect on others' files.
> 
> To assist with the handling of 'L' records which were indicating a category,
> changes were made to use any existing brokerage account to supply/receive 
> any monies.  If no brokerage account already existed, the record would be 
> left flagged as missing an assignment.
> 
> MyMoneyStatementReader::Private::nameToId was rewritten to handle category
> sub-accounts, recognising existing ones and otherwise creating them.
> 
> 
> This addresses bug 274185.
>     https://bugs.kde.org/show_bug.cgi?id=274185
> 
> 
> Diffs
> -----
> 
>   /trunk/extragear/office/kmymoney/kmymoney/converter/mymoneyqifreader.cpp 1235620 
>   /trunk/extragear/office/kmymoney/kmymoney/converter/mymoneystatementreader.cpp 1235620 
> 
> Diff: http://svn.reviewboard.kde.org/r/6705/diff
> 
> 
> Testing
> -------
> 
> Imported several QIF files having varying formats, both real-life and
> constructed to contain mixtures of record types and category structure.
> 
> atype run.
> 
> 
> Thanks,
> 
> Allan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kmymoney-devel/attachments/20110608/dfdb2fd7/attachment-0001.htm