Match transactions differently
AbandonedPublic

Authored by wojnilowicz on Aug 12 2018, 6:46 AM.

Details

Reviewers
tbaumgart
Group Reviewers
KMyMoney
Summary

This patch allows to write and read information about matched transactions in a way suited well for SQL and XML storage.
Until now matched transactions worked only with XML storage.
In general the new way entails referring to matched transactions by their IDs instead of embedding transaction in a transaction.

Changes:

  1. honor transaction IDs in XML storage, just like it is in SQL storage

Those IDs were the only ones that have no significance while loading storage, because at each loading, new transaction IDs were assigned.
We should use them now if we want to refer to transactions by their IDs.

  1. new transaction attribute called origin

It tells if the transaction is: imported, typed, matching input (transaction that served for matching process), matching output (transaction that is a result of matching process). It's an alternative for hiding a matched transaction (previously by embedding it in another transaction) from balance calculation.

  1. no embedding of a matched transaction by a matched split

Transaction node as XML has been embedded in another transaction as a key-value pair. That's not well suited for SQL but also for easy undoing of operation. That's because matched split stored backup information not only about matched split but also about matched transaction in itself. That mix of backup values is not transparent, hard to develop further and error prone.
Now third transaction is created and the two that served for matching are hidden and the new transaction refers to the hidden ones by their IDs.

  1. information about storage version change in case of XML storage

User now gets an information window, that tells him that his storage gets updated. I think it's important to rise user's awareness in that situation instead of silently update his storage which always may go askew. SQL storage should also get this kind of information soon.

  1. xmlstorageupdater

Center place for all updates in XML storage. I think updater should update file and then give it to KMyMoney as the latest one. Later on it could be separate plugin or something like that. SQL storage should also get this kind of updater soon.

Below is a mockup to maybe show better what I want to have

Diff Detail

Repository
R261 KMyMoney
Lint
Lint Skipped
Unit
Unit Tests Skipped
wojnilowicz requested review of this revision.Aug 12 2018, 6:46 AM
wojnilowicz created this revision.
tbaumgart requested changes to this revision.Aug 13 2018, 10:41 AM

In general, the idea to change the storage of matching transactions is OK. It was more of a quick hack back then. But I think, we need to iron out a few things before we can add it to master. Maybe even postpone it after the next release, because it changes the file structure and is not backward compatible.

kmymoney/mymoney/mymoneysplit.cpp
33

This is a no-go per design. I know that your logic does not work without it, but none of the single objects (split, transaction, ...) should have knowledge nor access to the MyMoneyFile object. The logic should therefore move into the MyMoneyFile object.

kmymoney/mymoney/mymoneytransaction.cpp
35

Please see above.

kmymoney/plugins/xml/xmlstorage.cpp
218

These integers should be kept as #define's somewhere and must not be hardcoded

kmymoney/plugins/xml/xmlstorageupdater.cpp
218

What about the existing fixes? Don't they need to be called here in a loop before the new one can be executed?

220

no need of this comment here, multiple case statements in a row are no problem.

This revision now requires changes to proceed.Aug 13 2018, 10:41 AM

In general, the idea to change the storage of matching transactions is OK. It was more of a quick hack back then. But I think, we need to iron out a few things before we can add it to master. Maybe even postpone it after the next release, because it changes the file structure and is not backward compatible.

So let's make a release now and then move forward.

Does upgrade of XML storage works for you without issues?

Incorporated changes according to the review.

wojnilowicz marked 3 inline comments as done.Aug 13 2018, 4:41 PM
wojnilowicz added inline comments.
kmymoney/mymoney/mymoneysplit.cpp
33

That's not true. MyMoneySchedule uses MyMoneyFile.

kmymoney/plugins/xml/xmlstorage.cpp
218

I thought about it too but I wan't to do that later.

kmymoney/plugins/xml/xmlstorageupdater.cpp
218

Existing fixes don't touch a file but MyMoneyStorageMgr after the file is loaded, so IMO they are useless if MyMoneyStorageMgr changes. I thought about dropping them because:

  1. they are old,
  2. they are useless with our changing code.
wojnilowicz abandoned this revision.Mar 10 2019, 6:50 AM
wojnilowicz marked an inline comment as done.