<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/109913/">https://git.reviewboard.kde.org/r/109913/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ship It!</p></pre>
<br />
<p>- Christian David</p>
<br />
<p>On April 8th, 2013, 8:19 nachm. CEST, Łukasz Maszczyński wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KMymoney.</div>
<div>By Łukasz Maszczyński.</div>
<p style="color: grey;"><i>Updated April 8, 2013, 8:19 nachm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kmymoney
</div>
<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;">This review is a follow-up of discussion started in review 108489.
There was an open point whether payee should be included in match criteria for detecting a duplicate transaction on import. Cristian's comment on this:
"If the amount and the post date are checked you could also add the payee check although I'm not sure about the first two checks either. Scoping the bankId within an account seems OK though (I don't have a strong argument for this, it's just a feeling)."
I see two options:
1. (the one I prefer) Extend match criteria to include payee check (so that the 'account', 'post date', 'amount' and 'payee' would be checked)
2. Get rid of extra checks and trust the bankId of imported transaction only - this would require removing 'post date' and 'amount' checks, possibly 'account' check as well (to be decided)
This review implements option 1, since I believe the extra checks are sanity checks after all (I cannot imagine that two supposedly identical transactions would have different parameters, e.g. amounts).
</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;">make test</pre>
</td>
</tr>
</table>
<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/matchfindertest.h <span style="color: grey">(28edbb6664474b8a873a0744bd48f42ed15526ed)</span></li>
<li>kmymoney/converter/matchfindertest.cpp <span style="color: grey">(879b3c26781cf2b7177febb3b8816dc6abac7de7)</span></li>
<li>kmymoney/converter/transactionmatchfinder.cpp <span style="color: grey">(63619e183617e017343c26ffee044f21127a5cac)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/109913/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>