Review Request: Add basic drag&drop support for groups model

[David Edmundson]

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

Ship it!


Few comments that should be addressed before actually shipping. Damn good work otherwise.

If you make the changes, post here again if you want a re-review. Otherwise I'm ok for it to be shipped.


models/groups-model.cpp
<http://git.reviewboard.kde.org/r/102543/#comment5526>

    braces round here. (and the others)



models/groups-model.cpp
<http://git.reviewboard.kde.org/r/102543/#comment5531>

    When do you use this?
    
    I assume you were going to build a rowChanged() piece of code, but this isn't needed as it should change /after/ the groups get changed in TP.



models/groups-model.cpp
<http://git.reviewboard.kde.org/r/102543/#comment5530>

    What's this for?
    



models/groups-model.cpp
<http://git.reviewboard.kde.org/r/102543/#comment5527>

    I'm not entirely sure I like the way we're storing this. Effectively we have a stringlist which goes
    
    contact
    account
    contact
    account
    
    given we're just talking about a few people accepting this drag data it might make more sense to store it as "account/contact" as a single string so it's a bit clearer on how to unpack our data.
    I don't know.



models/groups-model.cpp
<http://git.reviewboard.kde.org/r/102543/#comment5528>

    This needs fixing, well spotted. 
    
    Worst case we could add an extra boolean role in the groups model for "this is the ungrouped contacts group"?
    
    I'm happy for this to be a separate patch.



models/groups-model.cpp
<http://git.reviewboard.kde.org/r/102543/#comment5529>

    connect this to generalOperationFinished at least.
    
    We have lots of times when adding a user to a group fails when the server doesn't allow it, we need to report it.


- David


On Sept. 6, 2011, 3:37 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102543/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2011, 3:37 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> It's what it says. You can drag contacts around your groups. When you drag it from 'Ungrouped', the contact is actually not copied but always moved (why would you want to have it in Ungrouped anyway?).
> 
> 
> This addresses bug 279657.
>     http://bugs.kde.org/show_bug.cgi?id=279657
> 
> 
> Diffs
> -----
> 
>   main-widget.cpp b477500 
>   models/groups-model.h f838897 
>   models/groups-model.cpp 1ee874a 
> 
> Diff: http://git.reviewboard.kde.org/r/102543/diff
> 
> 
> Testing
> -------
> 
> Yes.
> 
> 
> Thanks,
> 
> Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20110906/549f8385/attachment-0001.html>