> On Nov. 29, 2012, 6:52 p.m., Cristian Oneț wrote: > > There still seem to be some problems with the patch here are the steps to > > reproduce the (using the same ofx file that I've sent you): > > 1. Before importing the file prepare a transaction so that one of the > > imported transactions are matched > > 2. Import the ofx file > > 3. Observe the match - don't accept it just save the kmymoney file (I don't > > know if this and the next step is important) > > 4. Close and reopen the file > > 5. Import the ofx file again > > > > Expected result: > > The matched transaction is detected as a duplicate and not imported. > > > > Actual result: > > The matched transaction is imported again. > > > > Note that I performed steps 1-3 with the previous buggy version of the > > patch so this could be a no-issue if only this patch is used. Anyway I'll > > send you my testfile by mail.
That was an issue, indeed. I hastily simplified the method TransactionMatchFinder::findMatchingSplit() which finds a match for a split-under-import, so that both duplicate matching and regular matching works on a similar set of conditions, while the original version didn't really work this way. I think there are some inconsistencies in handling duplicates vs. regular matches that need to be addressed - I will do that in a separate review after this one is finished. Anyway I fixed this case and also some other(s) in rev. 6. - Łukasz ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107137/#review22774 ----------------------------------------------------------- On Dec. 5, 2012, 8:28 p.m., Łukasz Maszczyński wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107137/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2012, 8:28 p.m.) > > > Review request for KMymoney. > > > Description > ------- > > 1. please note that dependency on Boost is no longer optional (see changes in > CMakeLists.txt) > > 2. Architectural changes > Until now, the method MyMoneyStatementReader::processTransactionEntry() > handled the whole process of importing a transaction - that is: handling the > securities, matching and creating payees, and - at the very end of the method > - adding the transaction to the ledger. > The last step (adding transaction to ledger) is the main target of this > refactoring. Its algorithm was as follows: > 1. find a matching transaction (either existing or scheduled) - using > TransactionMatcher::findMatch() > 2. If an "existing transaction match" is found - handle it (in the old > code it's the block starting with a comment "// it matched a simple > transaction. that's the easy case") > 3. Else if a "scheduled transaction match" is found - handle it ("// a > match has been found in a pending schedule"...) > > Code "mapping" is as follows: > - step 2 (above) is extracted to handleMatchingOfExistingTransaction() > - step 3 (above) is extracted to handleMatchingOfScheduledTransaction() > - TransactionMatcher::findMatch() is extracted to > TransactionMatchFinder::findMatch() (note: there are two pure-virtual > functions that are implemented in ExistingTransactionMatchFinder, > ScheduledTransactionMatchFinder classes) > - TransactionMatcher::checkTransaction() is extracted to > TransactionMatchFinder::findMatchingSplit() > > 3. Memory management changes > Raw pointers are no longer used, as these are typically error-prone. > Pointers were replaced either with object instances, or boost::optional is > used if applicable (e.g. see members of TransactionMatchFinder class). > > 4. dynamic_casts removed (were used on pointers returned by > TransactionMatcher::findMatch(), no longer needed - see > TransactionMatchFinder::getMatchedTransaction() and getMatchedSchedule() ) > > 5. variable/method names - I did my best to keep those meaningful: e.g. > "importedTransaction" instead of "t") > > > Diffs > ----- > > CMakeLists.txt 93af070 > kmymoney/converter/mymoneystatementreader.h 758ff00 > kmymoney/converter/mymoneystatementreader.cpp 42c4841 > kmymoney/dialogs/CMakeLists.txt 9a8d782 > kmymoney/dialogs/existingtransactionmatchfinder.h PRE-CREATION > kmymoney/dialogs/existingtransactionmatchfinder.cpp PRE-CREATION > kmymoney/dialogs/scheduledtransactionmatchfinder.h PRE-CREATION > kmymoney/dialogs/scheduledtransactionmatchfinder.cpp PRE-CREATION > kmymoney/dialogs/transactionmatcher.h d09a4cd > kmymoney/dialogs/transactionmatcher.cpp c380877 > kmymoney/dialogs/transactionmatchfinder.h PRE-CREATION > kmymoney/dialogs/transactionmatchfinder.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/107137/diff/ > > > Testing > ------- > > make test > > > Thanks, > > Łukasz Maszczyński > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
