Severe bug in reports in master branch

[Łukasz Wojniłowicz]

Dnia sobota, 24 czerwca 2017 08:52:43 CEST Thomas Baumgart pisze:
> Hi,
> 
> On Mittwoch, 21. Juni 2017 12:49:09 CEST Łukasz Wojniłowicz wrote:
> > Hi Cristian,
> > 
> > Please test latest master branch because I think your issues are fixed.
> > 
> > Referring to order of transactions in report after transaction
> > modification, it was sorting, that was to blame.
> > I changed it intentionally while implementing multicurrency totals, but it
> > broke your use case, so I returned old sorting.
> > First tests show that my use case isn't broken and yours work too, but I
> > would expect troubles with multicurrency totals anyways somewhere in
> > future.
> > 
> > During the research, I found a bug (not critical) that transaction IDs
> > written to .kmy file aren't being red from the file. Transaction IDs are
> > assigned through function during KMM start and stay so till application
> > quits. The assignment is according to date. When one changes date, the
> > assigned ID won't change till KMM starts anew. The assigned ID is being
> > written to .kmy file, but won't be red, because KMM will assign new IDs at
> > start.
> 
> Well, sort of. For XML based files, this ID is reassigned during file load.
> This is not a bug, but based on a design decision. to keep transaction ids
> small in amount and regain those that were deleted. It should not hurt at
> all to keep the values that are stored in the file. This can be changed in
> MyMoneyXmlContentHandler::endElement() if it helps to solve your issues.
> One thing is critical: you should not base application logic on any of the
> IDs of KMyMoney. At one point, we might change them to something completely
> different which is not in any particular order (GUID). I did not use them
> as debugging and following is much harder and hence used the system as it
> is today. I know, that in some places (ID assignment in general) the
> current code relies on the layout of the IDs (one letter, n digits) but
> that is only in a very few places in the engine code (mymoney subdirectory)
> and not in the application.
> 
> There is an additional piece of information which is used for accessing a
> transaction and sorting per post-date and is obtained via
> MyMoneyTransaction::uniqueSortKey() when the transaction is added to the
> ledger in MyMoneySeqAccessMgr::addTransaction() or modified (since parts of
> the sortkey may change). When loading from file, the creation of that key
> happens in MyMoneyXmlContentHandler::endElement() where the transaction
> loaded. If a transaction does not have an id (it is empty) it is part of a
> schedule and the re-assignment won't happen.
> 
> This is sort key is used during the lifetime of the application and not
> stored on file.
> 
> So in fact, you have the following indirection:
> 
>   m_transactionKeys[transactionid] = sortkey;
>   m_transactionList[sortkey] = transaction
> 
> This way, iterating over m_transactionList is always in post-date order as
> the sortkey is based on MyMoneyTransaction::postDate()  (see
> MyMoneyTransaction::uniqueSortKey()).

I observed that and not qualified it as harmful, but thank you for detailed 
description.
Nonetheless I believe, that we shouldn't write misleading IDs in .kmy file. It 
takes some time to deduce that and there were no BIG signs that it's just like 
that.

> > That was the reason why your reports showed correctly after restart.
> 
> I am not sure if I completely understand what your multi currency report
> problem has to do with transaction IDs.

Sorting of list table is done on basis of IDs (among others) and I gave it 
precedence over date. That was wrong move but is fixed now.