[Kmymoney-devel] Review Request 110212: Add Merge Payee Option

[Thomas Baumgart]

> On May 3, 2013, 1:53 p.m., Thomas Baumgart wrote:
> > I wonder, why we need a merge payee feature if the delete payee feature has basically the same functionality. Maybe, you can provide some insights why you developed this patch in the first place. Or am I missing something completely here?
> 
> Felipe Tonello wrote:
>     That's true, but for the user stand point of view delete to merge doesn't make sense.
>     I added this feature because it is useful for me, perhaps for other users as well. Sometimes I have to merge Payees because I wrote them wrong or because they are the same and I didn't know.
>     
>     Maybe we can just change delete to delete/merge. But still, for in terms of UX I prefer having different buttons for each functionality.

Agreed, but can we share the code of both somehow? After all, we have a list of payees that need to be deleted and the transactions referenced by them to be changed to a single payee. The UI might be different, but the works are the same, right? I think it would definitely a plus to have a single place to maintain the logic instead of two. What do you think? (Just saw that you mentioned that yourself in the FIXME section). Would a MyMoneyFile::mergeAndDeletePayees(MyMoneyPayee, QList<MyMoneyPayee>) be of help?


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110212/#review31936
-----------------------------------------------------------


On April 27, 2013, 5:20 a.m., Felipe Tonello wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110212/
> -----------------------------------------------------------
> 
> (Updated April 27, 2013, 5:20 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Description
> -------
> 
> Patch 1: Implements the actual merge operation.
> 
> Patches 2 and 3: Implements KAction and context menu option[2] and a button in
> payees view[3].
> 
> Patch 4: This patch might be controversial because I believe the functionality
> provided by this patch was not implemented because of some reason that it's
> not documented.
> 
> TODO: improve user experience (UX) by:
>   - Use a dialog similar to the KPayeeReassignDlg. Probably just making it more generic.
>   - Improve slotPayeeNew() to not to ask if user wants to create new payee or not.
> 
> FIXME: Too much duplicated code from slotPayeeDelete() and others. The way that the
>        MyMoney* Objects are designed requires too much repeated work, so I believe using
>        generic helper functions might be a good idea.
> 
> 
> Diffs
> -----
> 
>   kmymoney/kmymoney.h 22515f4e842a48645ea82ee50d3bd735927b87d5 
>   kmymoney/kmymoney.cpp 063f8b5a0357e39de2482a93abbd4cb33f869e19 
>   kmymoney/kmymoneyui.rc f35364158b77af1ee066d779b59ded7578f5c8f2 
>   kmymoney/views/kpayeesview.h 1fb9633f96355e0cd0da231afc8384bc48f01ca6 
>   kmymoney/views/kpayeesview.cpp 97a3f93ab034707db30fb3085c79b06e7ca8f1ca 
>   kmymoney/views/kpayeesviewdecl.ui 8d9c0083780c5cfb31dc3a237aa1deac1c708cca 
> 
> Diff: http://git.reviewboard.kde.org/r/110212/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Felipe Tonello
> 
>

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