| Summary: | When importing transactions, we're matching against the other transactions also being imported | ||
|---|---|---|---|
| Product: | [Applications] kmymoney | Reporter: | Dawid Wróbel <me> |
| Component: | importer | Assignee: | KMyMoney Devel Mailing List <kmymoney-devel> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | NOR | ||
| Version: | git (master) | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | https://commits.kde.org/kmymoney/84d204b26fd449f501d4f7cb360adad7def73627 | Version Fixed In: | 5.1.0 |
| Sentry Crash Report: | |||
| Attachments: | Sample CSV file | ||
Actually, after reading the code further I stand corrected - isImported() is a member of MyMoneyTransaction, while CSV importer uses the MyMoneyStatement::Transaction.
The MyMoneyStatementReader::processTransactionEntry() actually correctly sets the transactionUnderImport.setImported(). The actual issue is with the conditional expression I previously referred to – the !isImported() condition should not be optional. Proposed resolution:
- if (result == TransactionMatchFinder::MatchDuplicate
- || !matchedTransaction.isImported()
- || result == TransactionMatchFinder::MatchPrecise) { // don't match with just imported transaction
+ if (!matchedTransaction.isImported() &&
+ (result == TransactionMatchFinder::MatchDuplicate || result == TransactionMatchFinder::MatchPrecise)) { // don't match with just imported transaction
Let
Git commit 295d003c977ee67e2af7d76a14a39869c599ac9e by Dawid Wróbel. Committed on 12/04/2020 at 03:53. Pushed by wrobelda into branch 'master'. When importing transactions, make sure not to match against the transactions already imported during the same session. M +2 -3 kmymoney/converter/mymoneystatementreader.cpp https://commits.kde.org/kmymoney/295d003c977ee67e2af7d76a14a39869c599ac9e Git commit 5bcf3cf9db2f689aefc7d9c3459ac0a496075c85 by Dawid Wróbel. Committed on 12/04/2020 at 06:00. Pushed by wrobelda into branch 'master'. Revert the fix which broke duplicate transaction detection between imports. This reverts commit 295d003c977ee67e2af7d76a14a39869c599ac9e. M +3 -2 kmymoney/converter/mymoneystatementreader.cpp https://commits.kde.org/kmymoney/5bcf3cf9db2f689aefc7d9c3459ac0a496075c85 Git commit 1655f78fd9bcbb3c23f2a5c8d8d4cf05a61823a0 by Thomas Baumgart. Committed on 12/04/2020 at 07:55. Pushed by tbaumgart into branch '5.0'. Make sure bankID is assigned when importing transactions The bankID member of a split is the ultimate identifier to detect duplicate transactions. Yet if empty it is not used and transactions of the same import session are falsely detected as duplicates. This change assigns a bankID in case it is empty based on the account id, the post date, the payee name, the memo text, and the amount which makes the bankID unique for the import session. FIXED-IN: 5.0.9 M +26 -17 kmymoney/converter/mymoneystatementreader.cpp https://commits.kde.org/kmymoney/1655f78fd9bcbb3c23f2a5c8d8d4cf05a61823a0 Git commit 84d204b26fd449f501d4f7cb360adad7def73627 by Dawid Wróbel. Committed on 15/04/2020 at 22:49. Pushed by wrobelda into branch 'bug-420056'. Add option to enable fixing of the buy/sell signs Some providers do not follow the OFX spec and do not assign the correct positive/negative signage to amount (TOTALs) and quantity (UNITs) values assosciated with BUY/SELL investment transactions (see OFX2.2, section 13.3.3). This change adds option to OFX import dialog and account's online settings to force the signage to follow the the spec. GUI: M +1 -0 kmymoney/plugins/ofx/import/dialogs/konlinebankingstatus.cpp M +19 -2 kmymoney/plugins/ofx/import/dialogs/konlinebankingstatusdecl.ui M +18 -1 kmymoney/plugins/ofx/import/importoption.ui M +96 -74 kmymoney/plugins/ofx/import/ofximporter.cpp https://commits.kde.org/kmymoney/84d204b26fd449f501d4f7cb360adad7def73627 |
Created attachment 127457 [details] Sample CSV file SUMMARY While importing investment transactions from CSV file today, I noticed that one of the transactions was missing. Specifically, two of the transactions were similar enough for the TransactionMatcher to drop one of them and reconcile the other. Despite having same quantities, prices and date, the transactions have different memo, so it's understandable that it considers them a loose match. However, those two shouldn't even be matched against each other, since both are being imported from external source. I can see in the mymoneystatementreader.cpp that this is indeed aimed to be handled properly by checking if a transaction isImported(): if (result == TransactionMatchFinder::MatchDuplicate || !matchedTransaction.isImported() || result == TransactionMatchFinder::MatchPrecise) { // don't match with just imported transaction However, the CSV importer doesn't set it for its transactions, unlike the QIF importer. Note that the latter is the *only* importer that has any usage of setImported() in its code, so I presume other importers may be affected as well. STEPS TO REPRODUCE 1. Import the CSV file attached OBSERVED RESULT Only one of the two transactions was imported. EXPECTED RESULT Both should be.