<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/128478/">https://git.reviewboard.kde.org/r/128478/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 20th, 2016, 5:55 a.m. UTC, <b>Thomas Baumgart</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Your patch in general seems OK if you only have OFX accounts. It will interfere though with other online transaction downloads e.g. HBCI.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Since the problem and the solution are very OFX specific, I suggest to move the logic into the OFX plugin. I would move the calculation of the <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">startDate</strong> into <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">bool OfxImporterPlugin::import(const QString& filename)</strong> and the filter logic at the end of <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">int OfxImporterPlugin::ofxTransactionCallback(struct OfxTransactionData data, void * pv)</strong> where unhandled transaction types are already eliminated.</p></pre>
</blockquote>
<p>On July 20th, 2016, 3:43 p.m. UTC, <b>Jack Ostroff</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thomas: do any of the other import types use the concept of a requested start date for transactions? (I know csv does not, and have very limited experience with anything except OFX.) Also, I think this only applies to OFX direct connect, not to OFX file import, in case that affects the implementation.</p></pre>
</blockquote>
<p>On July 20th, 2016, 4:56 p.m. UTC, <b>Thomas Baumgart</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In fact, KBanking supports this concept from day one and does so pretty well. The German bank servers obey the options in the request (I have not heard otherwise) whereas the Citibank server seems to be ignoring them completely. Your hint about import vs. online download is valid. One could catch this if the calculation for <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">startDate</strong> is moved to <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">bool OfxImporterPlugin::updateAccount(const MyMoneyAccount& acc, bool moreAccounts)</strong> which is only called for online updates AFAICS and leave it at 1.1.1900 for all other paths.
The KBanking logic btw. can be found in <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">bool KBankingPlugin::updateAccount(const MyMoneyAccount& acc, bool moreAccounts)</strong>.</p></pre>
</blockquote>
<p>On July 20th, 2016, 5:18 p.m. UTC, <b>Jack Ostroff</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Just because there is only one current example of a server not honoring the start date, and it happens to be OFX, would it hurt to have ALL online imports drop transactions prior to the requested start date?</p></pre>
</blockquote>
<p>On July 20th, 2016, 6:14 p.m. UTC, <b>Thomas Baumgart</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The KBanking plugin e.g. goes back two or three more days than the last download date. This is because sometimes transactions get delivered on a later date. Having a general filter would eliminate these transactions as well.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"><span></span> <span style="color: #408080; font-style: italic">// get last statement request date from application account object</span>
<span style="color: #408080; font-style: italic">// and start from a few days before if the date is valid</span>
QDate lastUpdate <span style="color: #666666">=</span> QDate<span style="color: #666666">::</span>fromString(acc.value(<span style="color: #BA2121">"lastImportedTransactionDate"</span>), Qt<span style="color: #666666">::</span>ISODate);
<span style="color: #008000; font-weight: bold">if</span> (lastUpdate.isValid())
lastUpdate <span style="color: #666666">=</span> lastUpdate.addDays(<span style="color: #666666">-3</span>);
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In general, I think, each solution to a specific problem should stay as close to the source of the problem. So this OFX specific thing should be handled in the OFX specific part. Plus you mention that it only applies to OFX download not the file import. There we have the first exception to the rule. The MyMoneyStatementReader does not differentiate between file import or online download (AFAIR).</p></pre>
</blockquote>
<p>On October 25th, 2016, 9:45 p.m. UTC, <b>Jeff Lundblad</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Moved the logic to ofximporterplugin. Date filter only applies in updateAccount(), so "File->Import->OFX" files are not affected.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Regarding the lastUpdate.addDays(-3), it has occurred to me before that having a "last update minus X days" in the "Import Details" choices would be nice. Like the current "Today minus X days". X=0 would provide current functionality. I think I occasionally miss investment transactions if I update "just right" between the trade date and the settlement date (I'm not real sure on this).</p></pre>
</blockquote>
<p>On October 26th, 2016, 6:55 a.m. UTC, <b>Thomas Baumgart</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The changes look good to me, but since I am not an OFX user, I cannot comment on the overall functionality.</p></pre>
</blockquote>
</blockquote>
<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;">If the impact of this change is simply to limit importing transactions to those after the requested start date, then this change will only affect downloads from institutions which do not honor that request. This reveiw was in response to the only report of that problem I recall ever hearing.</p></pre>
<br />
<p>- Jack</p>
<br />
<p>On October 25th, 2016, 9:44 p.m. UTC, Jeff Lundblad 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 Jeff Lundblad.</div>
<p style="color: grey;"><i>Updated Oct. 25, 2016, 9:44 p.m.</i></p>
<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=365818">365818</a>
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Only import transactions that are newer than or equal to the "Start date of import" in the account's online settings</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested on Citi credit card OFX downloads which nearly always download 2 years worth of transactions regardless of the date range that the OFX request requests.</p></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/plugins/ofximport/ofximporterplugin.cpp <span style="color: grey">(46842ee)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128478/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/10/25/7838204d-f10b-41cd-8774-044e17b9e98e__ofx-update-date.patch">ofx-update-date.patch</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>