Review Request 128478: Only import OFX requested date range

[Thomas Baumgart]

> 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?

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).


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128478/#review97637
-----------------------------------------------------------


On Juli 18, 2016, 7:44 nachm., Jeff Lundblad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128478/
> -----------------------------------------------------------
> 
> (Updated Juli 18, 2016, 7: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
> -----
> 
>   "a/kmymoney/kmymoney/converter/mymoneystatementreader-1b81833-left.cpp" 0b31377 
> 
> 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.
> 
> 
> Thanks,
> 
> Jeff Lundblad
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20160720/08b578de/attachment.html>