[Kmymoney-devel] Review Request 109913: transaction matching criteria change

[Łukasz Maszczyński]

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

(Updated Sie 9, 2014, 7:28 po południu)


Status
------

This change has been discarded.


Review request for KMymoney.


Repository: kmymoney


Description
-------

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


Diffs
-----

  kmymoney/converter/matchfindertest.h 28edbb6664474b8a873a0744bd48f42ed15526ed 
  kmymoney/converter/matchfindertest.cpp 879b3c26781cf2b7177febb3b8816dc6abac7de7 
  kmymoney/converter/transactionmatchfinder.cpp 63619e183617e017343c26ffee044f21127a5cac 

Diff: https://git.reviewboard.kde.org/r/109913/diff/


Testing
-------

make test


Thanks,

Łukasz Maszczyński

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