<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/6705/">http://svn.reviewboard.kde.org/r/6705/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just had a quick glimpse on the diff, no real testing done here. This is what I came across.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6705/diff/1/?file=46425#file46425line1462" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/extragear/office/kmymoney/kmymoney/converter/mymoneyqifreader.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void MyMoneyQifReader::processInvestmentTransactionEntry(void)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1462</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span> <span class="k">else</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1462</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please keep coding style "} else {"</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6705/diff/1/?file=46425#file46425line1500" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/extragear/office/kmymoney/kmymoney/converter/mymoneyqifreader.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void MyMoneyQifReader::processInvestmentTransactionEntry(void)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1496</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">transferAccount</span><span class="p">(</span><span class="n">tmp</span><span class="p">);</span><span class="hl"> </span> <span class="c1">// make sure the account exists</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1500</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">transferAccount</span><span class="p">(</span><span class="n">tmp</span><span class="p">);</span><span class="cm"><span class="hl">/*tmp.clear(); */</span></span><span class="c1"><span class="hl">//</span> // make sure the account exists</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is this some debug leftover?</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6705/diff/1/?file=46425#file46425line1503" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/extragear/office/kmymoney/kmymoney/converter/mymoneyqifreader.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void MyMoneyQifReader::processInvestmentTransactionEntry(void)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1498</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">tr</span><span class="p">.</span><span class="n">m_strInterestCategory</span> <span class="o">=</span> <span class="n">tmp</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1503</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">tr</span><span class="p">.</span><span class="n">m_strInterestCategory</span> <span class="o">=</span> <span class="n">tmp</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</div>
<br />
<p>- Thomas</p>
<br />
<p>On June 7th, 2011, 7:44 p.m., Allan Anderson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kmymoney.</div>
<div>By Allan Anderson.</div>
<p style="color: grey;"><i>Updated June 7, 2011, 7:44 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Imported several QIF files having varying formats, both real-life and
constructed to contain mixtures of record types and category structure.
atype run.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=274185">274185</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/extragear/office/kmymoney/kmymoney/converter/mymoneyqifreader.cpp <span style="color: grey">(1235620)</span></li>
<li>/trunk/extragear/office/kmymoney/kmymoney/converter/mymoneystatementreader.cpp <span style="color: grey">(1235620)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6705/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>