> On Juli 20, 2016, 7:55 vorm., 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).
The changes look good to me, but since I am not an OFX user, I cannot comment
on the overall functionality.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128478/#review97637
-----------------------------------------------------------
On Okt. 25, 2016, 11:44 nachm., Jeff Lundblad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128478/
> -----------------------------------------------------------
>
> (Updated Okt. 25, 2016, 11:44 nachm.)
>
>
> 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
>
>
