Review Request: New GroupsModel

[Daniele Elmo Domenichelli]

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


This is not a real review, just a couple of things that I spotted...


KTp/Models/abstract-grouping-proxy-model.h
<http://git.reviewboard.kde.org/r/107377/#comment17089>

    Q_SLOTS



KTp/Models/abstract-grouping-proxy-model.h
<http://git.reviewboard.kde.org/r/107377/#comment17090>

    #ifndef was ABSTRACTGROUPINGPROXYMODEL_H



KTp/Models/abstract-grouping-proxy-model.cpp
<http://git.reviewboard.kde.org/r/107377/#comment17091>

    qobject_cast?


- Daniele Elmo Domenichelli


On Nov. 19, 2012, 6:10 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107377/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2012, 6:10 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Initial draft of a replacement for GroupsModel.
> The key differences are:
>  - It doesn't rely on ContactModel* items, just a general model so can be used with the kpeople model as well as ours
>  - It can work on a tree (grouping by the top level items in the tree) for working with the kpeople model
>  - It can group on any property in the model. Not just GroupName. This will allow the KPeople model to still group by account.
>  - As it is generic it can also be used to provide the grouping in the LogViewer(for example), simplifying the entity model code there.
> 
> The code is pretty complex so I would like people to comment if it makes sense.
> 
> It is expected one subclasses this model for a specific purpose. i.e to group by groupNames:
> 
> QSet<QString> GroupGroupingModel::groupsForIndex(const QModelIndex &sourceIndex) const
> {
>     QStringList groups = sourceIndex.data(AccountsModel::GroupsRole).value<QStringList>();
>     if (groups.isEmpty()) {
>         groups.append("_unsorted");
>     }
> 
>     return groups.toSet();
> }
> 
> QVariant GroupGroupingModel::dataForGroup(const QString &group, int role) const
> {
>     switch (role) {
>     case Qt::DisplayRole:
>         if (group == "_unsorted") {
>             return i18n("Unsorted");
>         } else {
>             return group;
>         }
>     case AccountsModel::IdRole:
>         return group;
>     }
>     return QVariant();
> }
> 
> 
> Also if anyone can think of a better class name for something that group nodes by group name that isn't GroupGroupingModel that would be much appreciated.
> 
> 
> Diffs
> -----
> 
>   KTp/Models/abstract-grouping-proxy-model.h PRE-CREATION 
>   KTp/Models/abstract-grouping-proxy-model.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/107377/diff/
> 
> 
> Testing
> -------
> 
> See it in action at: git at git.kde.org:scratch/davidedmundson/groupingmodel
> 
> Tested with ContactsModel->FlatModelProxy->This
> Tested grouping by GroupName, by account, by client type, and grouping by presence.
> 
> In the near future ContactsModel will be a list, so this makes more sense.
> 
> No crashes, everything appeared to work.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20121119/c3886933/attachment.html>