<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://git.reviewboard.kde.org/r/110022/">http://git.reviewboard.kde.org/r/110022/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 21st, 2013, 3:47 p.m. UTC, <b>Thomas Baumgart</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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://git.reviewboard.kde.org/r/110022/diff/1/?file=138922#file138922line89" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/models/accountsmodel.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; ">class AccountsModel::Private</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">74</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">acc</span><span class="p">.</span><span class="n">accountList</span><span class="p">().</span><span class="n">count</span><span class="p">()</span> <span class="o">></span> <span class="mi">0</span><span class="p">)</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">89</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p"><span class="hl">(</span>(</span><span class="n">acc</span><span class="p">.</span><span class="n">accountList</span><span class="p">().</span><span class="n">count</span><span class="p">()</span> <span class="o">></span> <span class="mi">0</span><span class="p">)</span> <span class="o"><span class="hl">&&</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="o"><span class="hl">!</span></span><span class="n"><span class="hl">duplicateCategory</span></span><span class="p"><span class="hl">))</span></span><span class="hl"> </span><span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">.. and here the check for the flag to be false. How could it ever get true here?
Looking at it, I would simply remove all the flag business and simply return if the account is already there (as you do).</pre>
</blockquote>
<p>On April 25th, 2013, 8:12 p.m. UTC, <b>Allan Anderson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Agreed and done. All tests OK. I think what I'd done was to add the return later, without looking at the consequence. A lesson to be learned.</pre>
</blockquote>
<p>On May 14th, 2013, 12:31 p.m. UTC, <b>Allan Anderson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is anyone able to say if this is now OK to ship, please?</pre>
</blockquote>
<p>On May 14th, 2013, 12:39 p.m. UTC, <b>Cristian Oneț</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You did not update the patch with the flag removed so I can't say. I still didn't have time to look into this but without that flag the patch should look more 'elegant' :).</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No, I was holding off for further changes from yourself. I have had the flag removed here for a while, but I'll do some triple checking, and further testing and then do a full update of the patch.</pre>
<br />
<p>- Allan</p>
<br />
<p>On April 15th, 2013, 11:53 a.m. UTC, Allan Anderson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.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 April 15, 2013, 11:53 a.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;">BUG:317655 - Re-importing a categories list QIF file results in duplicated categories. Also, when creating sub-categories, some may appear twice or more times in Categories view.
These problems showed up with a single users categories list QIF file. The re-importing problem is fixed by the
/kmymoney/converter/mymoneyqifreader.cpp change.
@@ -906,7 +906,7 @@ void MyMoneyQifReader::processCategoryEntry(void)
}
// check if we can find the account already in the file
- MyMoneyAccount acc = kmymoney->findAccount(account, MyMoneyAccount());
+ MyMoneyAccount acc = kmymoney->findAccount(account, parentAccount);
Even without any re-importing, there was a further problem, where third-level(plus) sub-categories get duplicated. With the following simple file -
"!Type:Cat
NUtilities:Telephone:Cell:A
DCell Phone
E
^"
the "N" line is dealt with recursively, and initially, all the categories get created. However, what happens then is that the second part is dealt with, and the next lower level is created again, etc.
Initially, during this recursive process, the categories have not yet been added to the KMM file, so I add them to a new addedCategoriesList, which is scanned on each pass, to avoid this duplication. Because the process is recursive, I made the list static, to ensure the higher levels are retained between passes when descending the tree.
The problem is not confined to importing. If that "N" line category tree is created manually, in one entry, or during .kmy file loading, the third and lower levels get duplicated/triplicated.
The problem manifests itself only in the Categories view. Within the rest of KMM, the categories are created and maintained correctly.</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;">Numerous test case QIF files created and imported. Also, as the problem affected my live data file, that, too, was used for testing.</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="http://bugs.kde.org/show_bug.cgi?id=317655">317655</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>kmymoney/converter/mymoneyqifreader.cpp <span style="color: grey">(f42b12b)</span></li>
<li>kmymoney/models/accountsmodel.h <span style="color: grey">(38f9f2c)</span></li>
<li>kmymoney/models/accountsmodel.cpp <span style="color: grey">(3679314)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110022/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>