[Kmymoney-devel] Review Request 122746: Add a stage to the consistency check to fix transaction post dates.

[Cristian Oneț]

> On April 6, 2015, 6:44 a.m., Thomas Baumgart wrote:
> >

Thomas, if you have the feeling that moving the transaction's date is not the right way feel free to make that staement :). I'm not sure either about this patch but it's a way forward for the reported bug, I agree that if the transaction dates are correct and the account dates are wrong this could cause a lot of work for the user. So we might as well just drop this patch.


> On April 6, 2015, 6:44 a.m., Thomas Baumgart wrote:
> > kmymoney/dialogs/transactioneditor.cpp, line 678
> > <https://git.reviewboard.kde.org/r/122746/diff/5/?file=356770#file356770line678>
> >
> >     Why do you change 'Sell' to 'Add'? AFAIR, adding and removing shares does not involve a price. Please check by using the UI.

I've changed this because the action() of the split will never have the value "Sell" AFAICS. Take a look at mymoneysplit.cpp for a list of possible actions. Judging by the intended action the list should be "Buy", "Add", "Reinvest". I agree though that I should have mentioned the reason behind this action.


> On April 6, 2015, 6:44 a.m., Thomas Baumgart wrote:
> > kmymoney/mymoney/mymoneyfile.cpp, line 2138
> > <https://git.reviewboard.kde.org/r/122746/diff/5/?file=356773#file356773line2138>
> >
> >     Same as above about Add vs. Sell.

See above.


> On April 6, 2015, 6:44 a.m., Thomas Baumgart wrote:
> > kmymoney/kmymoney.cpp, line 6981
> > <https://git.reviewboard.kde.org/r/122746/diff/5/?file=356772#file356772line6981>
> >
> >     In case MyMoneyFile::consistencyCheck() throws an exception, the assignment to m_consistencyCheckResult does not happen. Now appending a single line leaves the result with just that single line (m_consistencyCheckResult.size() == 1).
> >     
> >     I suggest to override alwaysDisplayResult with 'true' in this case to make sure that the user gets a notification about the problem. Since alwaysDisplayResult is false for those cases when the consistency check is run as part of any File/Save operation it is even more important to make sure that the user gets the information.

This makes sense, I'll update the patch.


> On April 6, 2015, 6:44 a.m., Thomas Baumgart wrote:
> > kmymoney/mymoney/mymoneyfile.cpp, line 2134
> > <https://git.reviewboard.kde.org/r/122746/diff/5/?file=356773#file356773line2134>
> >
> >     The logic does not implement a move but a copy of the price information since the old price information is not removed. Can we even move the price information or do we break other things in doing so?

You're right it's a copy and I would leave it as is and fix the comment. If we wouldn't copy the price information the consistency check's fixup would not be complete and copying the price information when moving the transaction makes sense.


- Cristian


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


On March 27, 2015, 6:29 a.m., Cristian Oneț wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122746/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 6:29 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 313793
>     http://bugs.kde.org/show_bug.cgi?id=313793
> 
> 
> Repository: kmymoney
> 
> 
> Description
> -------
> 
> If the transaction is moved to another post date the price information
> is moved along with it for investments transactions. This fixes the
> reported issue, saving the file once will fix all price issues.
> 
> Also add the possibility to save the consistency check log to a file
> or copy it into the clipboard.
> 
> Make sure that the user enters a valid transaction date by checking
> that the transaction date is after the opening date of each and every
> account involved in the transaction. Until now we only checked that
> the transaction date is after the opening date of the account in which
> is being entered.
> 
> BUG: 313793
> REVIEW: 122746
> 
> 
> Diffs
> -----
> 
>   kmymoney/dialogs/transactioneditor.cpp a215e3f8a1eaf2f7ccf1d73f29190f2ad86ff282 
>   kmymoney/kmymoney.h 92ab4b28d52c237222a2fb1106398ce0208215aa 
>   kmymoney/kmymoney.cpp c132983b63536e094375532082ea87c54461235c 
>   kmymoney/mymoney/mymoneyfile.cpp 8c1be4301a89b65f428edf2b8f0ab8b6e3dac51e 
> 
> Diff: https://git.reviewboard.kde.org/r/122746/diff/
> 
> 
> Testing
> -------
> 
> Opened the file attached to the report and observe that the consistency check fixes all of the issues.
> 
> 
> Thanks,
> 
> Cristian Oneț
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20150406/6a45bff4/attachment-0001.html>