> On July 20, 2016, 5:55 a.m., Thomas Baumgart wrote:
> > Your patch in general seems OK if you only have OFX accounts. It will
> > interfere though with other online transaction downloads e.g. HBCI.
> >
> > 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
> > **startDate** into **bool OfxImporterPlugin::import(const QString&
> > filename)** and the filter logic at the end of **int
> > OfxImporterPlugin::ofxTransactionCallback(struct OfxTransactionData data,
> > void * pv)** where unhandled transaction types are already eliminated.
>
> Jack Ostroff wrote:
> 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.
>
> Thomas Baumgart wrote:
> 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 **startDate** is moved to **bool
> OfxImporterPlugin::updateAccount(const MyMoneyAccount& acc, bool
> moreAccounts)** 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 **bool
> KBankingPlugin::updateAccount(const MyMoneyAccount& acc, bool moreAccounts)**.
>
> Jack Ostroff wrote:
> 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?
>
> Thomas Baumgart wrote:
> 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.
> ```C++
> // get last statement request date from application account object
> // and start from a few days before if the date is valid
> QDate lastUpdate =
> QDate::fromString(acc.value("lastImportedTransactionDate"), Qt::ISODate);
> if (lastUpdate.isValid())
> lastUpdate = lastUpdate.addDays(-3);
> ```
> 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).
>
> Jeff Lundblad wrote:
> Moved the logic to ofximporterplugin. Date filter only applies in
> updateAccount(), so "File->Import->OFX" files are not affected.
>
> 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).
>
> Thomas Baumgart wrote:
> The changes look good to me, but since I am not an OFX user, I cannot
> comment on the overall functionality.
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.
- Jack
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128478/#review97637
-----------------------------------------------------------
On Oct. 25, 2016, 9:44 p.m., Jeff Lundblad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128478/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2016, 9:44 p.m.)
>
>
> Review request for KMymoney.
>
>
> Bugs: 365818
> http://bugs.kde.org/show_bug.cgi?id=365818
>
>
> Repository: kmymoney
>
>
> Description
> -------
>
> Only import transactions that are newer than or equal to the "Start date of
> import" in the account's online settings
>
>
> Diffs
> -----
>
> kmymoney/plugins/ofximport/ofximporterplugin.cpp 46842ee
>
> Diff: https://git.reviewboard.kde.org/r/128478/diff/
>
>
> Testing
> -------
>
> 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.
>
>
> File Attachments
> ----------------
>
> ofx-update-date.patch
>
> https://git.reviewboard.kde.org/media/uploaded/files/2016/10/25/7838204d-f10b-41cd-8774-044e17b9e98e__ofx-update-date.patch
>
>
> Thanks,
>
> Jeff Lundblad
>
>
