Branch 'kolab/integration/4.13.0' - 10 commits - korganizer/actionmanager.cpp korganizer/akonadicollectionview.cpp korganizer/akonadicollectionview.h korganizer/views

Christian Mollekopf mollekopf at kolabsys.com
Wed Sep 17 12:16:36 CEST 2014


 korganizer/actionmanager.cpp                                   |    9 
 korganizer/akonadicollectionview.cpp                           |  288 +++++++---
 korganizer/akonadicollectionview.h                             |    3 
 korganizer/views/collectionview/calendardelegate.cpp           |  145 +++--
 korganizer/views/collectionview/calendardelegate.h             |   11 
 korganizer/views/collectionview/controller.cpp                 |  168 +++--
 korganizer/views/collectionview/controller.h                   |   38 -
 korganizer/views/collectionview/reparentingmodel.cpp           |   69 +-
 korganizer/views/collectionview/reparentingmodel.h             |    2 
 korganizer/views/collectionview/tests/reparentingmodeltest.cpp |   70 ++
 10 files changed, 576 insertions(+), 227 deletions(-)

New commits:
commit cc6a60f69449e6a191f39c49ff70c8ec9a27872c
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Wed Sep 17 11:47:11 2014 +0200

    Recursive checkbox for person nodes.

diff --git a/korganizer/akonadicollectionview.cpp b/korganizer/akonadicollectionview.cpp
index 4258c73..3b0a808 100644
--- a/korganizer/akonadicollectionview.cpp
+++ b/korganizer/akonadicollectionview.cpp
@@ -434,6 +434,28 @@ protected:
 
         return QSortFilterProxyModel::data(index, role);
     }
+
+    void setChildren(const QModelIndex &sourceIndex, const QVariant &value, int role) const
+    {
+        if (!sourceIndex.isValid()) {
+            return;
+        }
+        for (int i = 0; i < sourceModel()->rowCount(sourceIndex); i++) {
+            const QModelIndex child = sourceModel()->index(i, 0, sourceIndex);
+            sourceModel()->setData(child, value, role);
+            setChildren(child, value, role);
+        }
+    }
+
+    virtual bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::EditRole)
+    {
+        if (role == Qt::CheckStateRole) {
+            if (sourceModel()->hasChildren(mapToSource(index)) && index.data(NodeTypeRole).toInt() == PersonNodeRole) {
+                setChildren(mapToSource(index), value, role);
+            }
+        }
+        return QSortFilterProxyModel::setData(index, value, role);
+    }
 };
 
 } // anonymous namespace


commit 5691990aa526e2fb3a096e61a2cfcfd0c70876d7
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Wed Sep 17 10:11:57 2014 +0200

    AkonadiCollectionView: Fixed sorting

diff --git a/korganizer/akonadicollectionview.cpp b/korganizer/akonadicollectionview.cpp
index 8f5bedf..4258c73 100644
--- a/korganizer/akonadicollectionview.cpp
+++ b/korganizer/akonadicollectionview.cpp
@@ -258,21 +258,28 @@ class SortProxyModel : public QSortFilterProxyModel
         setDynamicSortFilter(true);
     }
 
-     bool lessThan(const QModelIndex &left,
-                                       const QModelIndex &right) const
+    static int score(const QModelIndex &index)
     {
-        QVariant leftPerson = left.data(PersonRole);
-        QVariant rightPerson = right.data(PersonRole);
-        if (leftPerson.isValid() && !rightPerson.isValid()) {
-            return true;
+        int score = 0;
+        if (index.data(PersonRole).isValid()) {
+            score += 1;
         }
-        QString leftString = left.data().toString();
-        QString rightString = right.data().toString();
-        if (leftPerson.isValid() && rightPerson.isValid()) {
-            leftString = leftPerson.value<Person>().name;
-            rightString = rightPerson.value<Person>().name;
+        if (index.data(IsSearchResultRole).toBool()) {
+            score += 2;
         }
-        return QString::localeAwareCompare(leftString, rightString) < 0;
+        return score;
+    }
+
+    bool lessThan(const QModelIndex &left, const QModelIndex &right) const
+    {
+        const int leftScore = score(left);
+        const int rightScore = score(right);
+        // kDebug() << left.data().toString() << leftScore << " : " << right.data().toString() << rightScore;
+        if (leftScore != rightScore) {
+            return leftScore < rightScore;
+        }
+
+        return QString::localeAwareCompare(left.data().toString(), right.data().toString()) < 0;
     }
 };
 
@@ -489,22 +496,23 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
   CalendarDelegateModel *calendarDelegateModel = new CalendarDelegateModel(this);
   calendarDelegateModel->setSourceModel(enabledModel);
 
-  SortProxyModel *sortProxy = new SortProxyModel( this );
-  sortProxy->setSourceModel(calendarDelegateModel);
-
   //Hide collections that are not required
   CollectionFilter *collectionFilter = new CollectionFilter( this );
-  collectionFilter->setSourceModel( sortProxy );
+  collectionFilter->setSourceModel( calendarDelegateModel );
+
+  SortProxyModel *sortProxy = new SortProxyModel( this );
+  sortProxy->setSourceModel(collectionFilter);
 
   mCollectionView = new Akonadi::EntityTreeView( this );
   mCollectionView->header()->hide();
   mCollectionView->setRootIsDecorated( true );
+  // mCollectionView->setSorting( true );
   {
     StyledCalendarDelegate *delegate = new StyledCalendarDelegate(mCollectionView);
     connect(delegate, SIGNAL(action(QModelIndex, int)), this, SLOT(onAction(QModelIndex, int)));
     mCollectionView->setItemDelegate( delegate );
   }
-  mCollectionView->setModel( collectionFilter );
+  mCollectionView->setModel( sortProxy );
   connect( mCollectionView->selectionModel(),
            SIGNAL(selectionChanged(QItemSelection,QItemSelection)),
            SLOT(updateMenu()) );
@@ -524,6 +532,8 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
   connect( searchCol, SIGNAL(textChanged(QString)),
           filterTreeViewModel, SLOT(setFilterFixedString(QString)) );
 
+  SortProxyModel *searchSortProxy = new SortProxyModel( this );
+  searchSortProxy->setSourceModel(filterTreeViewModel);
 
   Akonadi::EntityTreeView *mSearchView = new Akonadi::EntityTreeView( this );
   mSearchView->header()->hide();
@@ -533,7 +543,7 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
     connect(delegate, SIGNAL(action(QModelIndex, int)), this, SLOT(onAction(QModelIndex, int)));
     mSearchView->setItemDelegate( delegate );
   }
-  mSearchView->setModel( filterTreeViewModel );
+  mSearchView->setModel( searchSortProxy );
   new NewNodeExpander(mSearchView, true, QString());
 
   mController = new Controller(userProxy, searchProxy, this);


commit 46fc075321fee28907b6663d5a75e6162fcd4dfb
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Wed Sep 17 00:19:49 2014 +0200

    Don't show remove button for person subfolders, properly enable person collections.

diff --git a/korganizer/akonadicollectionview.cpp b/korganizer/akonadicollectionview.cpp
index 61d8cda..8f5bedf 100644
--- a/korganizer/akonadicollectionview.cpp
+++ b/korganizer/akonadicollectionview.cpp
@@ -1006,7 +1006,7 @@ void AkonadiCollectionView::onAction(const QModelIndex &index, int a)
             } else {
                 const QVariant var = index.data(PersonRole);
                 if (var.isValid()) {
-                    mController->addPerson(var.value<Person>());
+                    mController->setCollectionState(Akonadi::Collection(var.value<Person>().rootCollection), Controller::Enabled, true);
                 }
             }
         }
diff --git a/korganizer/views/collectionview/calendardelegate.cpp b/korganizer/views/collectionview/calendardelegate.cpp
index ead89a4..ee29a3e 100644
--- a/korganizer/views/collectionview/calendardelegate.cpp
+++ b/korganizer/views/collectionview/calendardelegate.cpp
@@ -73,6 +73,18 @@ static QStyleOptionButton buttonOpt(const QStyleOptionViewItemV4 &opt, const QPi
     return option;
 }
 
+static bool isChildOfPersonCollection(const QModelIndex &index)
+{
+    QModelIndex parent = index.parent();
+    while (parent.isValid()) {
+        if (parent.data(NodeTypeRole).toInt() == PersonNodeRole) {
+            return true;
+        }
+        parent = parent.parent();
+    }
+    return false;
+}
+
 QList<StyledCalendarDelegate::Action> StyledCalendarDelegate::getActions(const QStyleOptionViewItem &option, const QModelIndex &index) const
 {
     const bool isSearchResult = index.data(IsSearchResultRole).toBool();
@@ -89,7 +101,10 @@ QList<StyledCalendarDelegate::Action> StyledCalendarDelegate::getActions(const Q
             if (enabled != Qt::Checked) {
                 buttons << Enable;
             }
-            buttons << RemoveFromList;
+            //The remove button should not be available for person subfolders
+            if (!isChildOfPersonCollection(index)) {
+                buttons << RemoveFromList;
+            }
         } else {
             if (enabled == Qt::Checked) {
                 buttons << Enable;


commit 0288de79bfa0804cdf1a2dca5f2929807b5c381e
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Tue Sep 16 17:50:57 2014 +0200

    ReparentingModel: Catch another corner case that ends up readding person nodes.

diff --git a/korganizer/views/collectionview/reparentingmodel.cpp b/korganizer/views/collectionview/reparentingmodel.cpp
index f402658..74363b3 100644
--- a/korganizer/views/collectionview/reparentingmodel.cpp
+++ b/korganizer/views/collectionview/reparentingmodel.cpp
@@ -234,6 +234,7 @@ void ReparentingModel::addNode(const ReparentingModel::Node::Ptr& node)
             return;
         }
     }
+    mNodesToAdd << node;
     qRegisterMetaType<Node::Ptr>("Node::Ptr");
     QMetaObject::invokeMethod(this, "doAddNode", Qt::QueuedConnection, QGenericReturnArgument(), Q_ARG(Node::Ptr, node));
 }
@@ -246,6 +247,20 @@ void ReparentingModel::doAddNode(const Node::Ptr &node)
             return;
         }
     }
+    //If a datachanged call triggered this through checkSourceIndex, right after a person node has been removed.
+    //We'd end-up re-inserting the node that has just been removed. Therefore removeNode can cancel the pending addNode
+    //call through mNodesToAdd.
+    bool addNodeAborted = true;
+    for (int i = 0; i < mNodesToAdd.size(); i++) {
+        if (*mNodesToAdd.at(i) == *node) {
+            mNodesToAdd.remove(i);
+            addNodeAborted = false;
+            break;
+        }
+    }
+    if (addNodeAborted) {
+        return;
+    }
 
     beginResetModel();
     mProxyNodes << node;
@@ -255,6 +270,12 @@ void ReparentingModel::doAddNode(const Node::Ptr &node)
 
 void ReparentingModel::removeNode(const ReparentingModel::Node& node)
 {
+    //If there is an addNode in progress for that node, abort it.
+    for (int i = 0; i < mNodesToAdd.size(); i++) {
+        if (*mNodesToAdd.at(i) == node) {
+            mNodesToAdd.remove(i);
+        }
+    }
     for (int i = 0; i < mProxyNodes.size(); i++) {
         if (*mProxyNodes.at(i) == node) {
             //TODO: this does not yet take care of un-reparenting reparented nodes.
@@ -466,13 +487,16 @@ void ReparentingModel::onSourceRowsAboutToBeRemoved(QModelIndex parent, int star
         Q_ASSERT(sourceIndex.isValid());
 
         const QModelIndex proxyIndex = mapFromSource(sourceIndex);
-        const Node *node = extractNode(proxyIndex);
-        Node *parentNode = node->parent;
-        Q_ASSERT(parentNode);
-        const int targetRow = node->row();
-        beginRemoveRows(index(parentNode), targetRow, targetRow);
-        parentNode->children.remove(targetRow); //deletes node
-        endRemoveRows();
+        //If the indexes have already been removed (e.g. by removeNode)this can indeed return an invalid index
+        if (proxyIndex.isValid()) {
+            const Node *node = extractNode(proxyIndex);
+            Node *parentNode = node->parent;
+            Q_ASSERT(parentNode);
+            const int targetRow = node->row();
+            beginRemoveRows(index(parentNode), targetRow, targetRow);
+            parentNode->children.remove(targetRow); //deletes node
+            endRemoveRows();
+        }
     }
     //Allows the node manager to remove nodes that are no longer relevant
     for (int row = start; row <= end; row++) {
diff --git a/korganizer/views/collectionview/reparentingmodel.h b/korganizer/views/collectionview/reparentingmodel.h
index ebcd8f6..2f85f04 100644
--- a/korganizer/views/collectionview/reparentingmodel.h
+++ b/korganizer/views/collectionview/reparentingmodel.h
@@ -135,6 +135,7 @@ private:
     Node mRootNode;
     QList<Node*> mSourceNodes;
     QVector<Node::Ptr> mProxyNodes;
+    QVector<Node::Ptr> mNodesToAdd;
     NodeManager::Ptr mNodeManager;
     // QModelIndexList mLayoutChangedProxyIndexes;
     // QList<QPersistentModelIndex> mLayoutChangedSourcePersistentModelIndexes;


commit daf90b859a79e9f8e05f1ce70979b33d10360712
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Tue Sep 16 00:08:26 2014 +0200

    Recursive actions for person nodes + some refactoring and cleanup.

diff --git a/korganizer/akonadicollectionview.cpp b/korganizer/akonadicollectionview.cpp
index fa2c425..61d8cda 100644
--- a/korganizer/akonadicollectionview.cpp
+++ b/korganizer/akonadicollectionview.cpp
@@ -255,6 +255,7 @@ class SortProxyModel : public QSortFilterProxyModel
     explicit SortProxyModel( QObject *parent=0 )
       : QSortFilterProxyModel( parent )
     {
+        setDynamicSortFilter(true);
     }
 
      bool lessThan(const QModelIndex &left,
@@ -349,6 +350,85 @@ class CollectionFilter : public QSortFilterProxyModel
     }
 };
 
+class EnabledModel : public QSortFilterProxyModel
+{
+public:
+    explicit EnabledModel(QObject *parent=0)
+      : QSortFilterProxyModel(parent)
+    {
+    }
+
+protected:
+
+    virtual QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const
+    {
+        if (role == EnabledRole) {
+            Akonadi::Collection col = index.data(Akonadi::EntityTreeModel::CollectionRole).value<Akonadi::Collection>();
+            if (col.shouldList(Akonadi::Collection::ListDisplay)) {
+                return Qt::Checked;
+            } else {
+                return Qt::Unchecked;
+            }
+        }
+        return QSortFilterProxyModel::data(index, role);
+    }
+};
+
+class CalendarDelegateModel : public QSortFilterProxyModel
+{
+public:
+    explicit CalendarDelegateModel(QObject *parent=0)
+      : QSortFilterProxyModel(parent)
+    {
+    }
+
+protected:
+    bool checkChildren(const QModelIndex &index, int role, const QVariant &value) const
+    {
+        const QModelIndex sourceIndex = mapToSource(index);
+        for (int i = 0; i < sourceModel()->rowCount(sourceIndex); i++) {
+            const QModelIndex child = sourceModel()->index(i, 0, sourceIndex);
+            if (child.data(role) != value) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    virtual QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const
+    {
+        if (role == Qt::CheckStateRole) {
+            if (sourceModel()->hasChildren(mapToSource(index)) && index.data(NodeTypeRole).toInt() == PersonNodeRole) {
+                bool allChecked = checkChildren(index, role, Qt::Checked);
+                bool allUnchecked = checkChildren(index, role, Qt::Unchecked);
+                if (allChecked) {
+                    return Qt::Checked;
+                } else if (allUnchecked) {
+                    return Qt::Unchecked;
+                } else {
+                    return Qt::PartiallyChecked;
+                }
+            }
+        }
+        if (role == EnabledRole) {
+            if (sourceModel()->hasChildren(mapToSource(index)) && index.data(NodeTypeRole).toInt() == PersonNodeRole) {
+                bool allChecked = checkChildren(index, role, Qt::Checked);
+                bool allUnchecked = checkChildren(index, role, Qt::Unchecked);
+                // kDebug() << "person node " << index.data().toString() << allChecked << allUnchecked;
+                if (allChecked) {
+                    return Qt::Checked;
+                } else if (allUnchecked) {
+                    return Qt::Unchecked;
+                } else {
+                    return Qt::PartiallyChecked;
+                }
+            }
+        }
+
+        return QSortFilterProxyModel::data(index, role);
+    }
+};
+
 } // anonymous namespace
 
 CalendarViewExtension *AkonadiCollectionViewFactory::create( QWidget *parent )
@@ -403,14 +483,17 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
   userProxy->setNodeManager(ReparentingModel::NodeManager::Ptr(new PersonNodeManager(*userProxy)));
   userProxy->setSourceModel(colorProxy);
 
+  EnabledModel *enabledModel = new EnabledModel(this);
+  enabledModel->setSourceModel(userProxy);
+
+  CalendarDelegateModel *calendarDelegateModel = new CalendarDelegateModel(this);
+  calendarDelegateModel->setSourceModel(enabledModel);
+
   SortProxyModel *sortProxy = new SortProxyModel( this );
-  // sortProxy->setObjectName( QLatin1String("Show calendar colors") );
-  sortProxy->setDynamicSortFilter( true );
-  sortProxy->setSourceModel(userProxy);
+  sortProxy->setSourceModel(calendarDelegateModel);
 
   //Hide collections that are not required
   CollectionFilter *collectionFilter = new CollectionFilter( this );
-  collectionFilter->setDynamicSortFilter( true );
   collectionFilter->setSourceModel( sortProxy );
 
   mCollectionView = new Akonadi::EntityTreeView( this );
@@ -867,15 +950,24 @@ void AkonadiCollectionView::edit_disable()
 {
     Akonadi::Collection col = mCollectionView->currentIndex().data(Akonadi::EntityTreeModel::CollectionRole).value<Akonadi::Collection>();
     if (col.isValid()) {
-        mController->setCollection(col, false, false);
+        mController->setCollectionState(col, Controller::Disabled);
+    }
+    const QVariant var = mCollectionView->currentIndex().data(PersonRole);
+    if (var.isValid()) {
+        mController->removePerson(var.value<Person>());
     }
 }
 
 void AkonadiCollectionView::edit_enable()
 {
     Akonadi::Collection col = mCollectionView->currentIndex().data(Akonadi::EntityTreeModel::CollectionRole).value<Akonadi::Collection>();
+    kDebug() << col.name();
     if (col.isValid()) {
-        mController->setCollection(col, true, false);
+        mController->setCollectionState(col, Controller::Enabled);
+    }
+    const QVariant var = mCollectionView->currentIndex().data(PersonRole);
+    if (var.isValid()) {
+        mController->addPerson(var.value<Person>());
     }
 }
 
@@ -886,31 +978,23 @@ void AkonadiCollectionView::onAction(const QModelIndex &index, int a)
         case StyledCalendarDelegate::AddToList: {
             const Akonadi::Collection col = index.data(CollectionRole).value<Akonadi::Collection>();
             if (col.isValid()) {
-                mController->setCollection(col, false, true);
+                mController->setCollectionState(col, Controller::Referenced);
             } else {
                 const QVariant var = index.data(PersonRole);
                 if (var.isValid()) {
-                    mController->setPersonEnabled(var.value<Person>(), true);
+                    mController->addPerson(var.value<Person>());
                 }
             }
         }
         break;
         case StyledCalendarDelegate::RemoveFromList: {
-            //Disable all child collections
-            const QAbstractItemModel *model = index.model();
-            for (int row = 0; row < model->rowCount(index); row++) {
-                const Akonadi::Collection col = CalendarSupport::collectionFromIndex(model->index(row, 0, index));
-                if (col.isValid()) {
-                    mController->setCollection(col, false, false);
-                }
-            }
             const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
             if (col.isValid()) {
-                mController->setCollection(col, false, false);
+                mController->setCollectionState(col, Controller::Disabled);
             } else {
                 const QVariant var = index.data(PersonRole);
                 if (var.isValid()) {
-                    mController->setPersonEnabled(var.value<Person>(), false);
+                    mController->removePerson(var.value<Person>());
                 }
             }
         }
@@ -918,7 +1002,12 @@ void AkonadiCollectionView::onAction(const QModelIndex &index, int a)
         case StyledCalendarDelegate::Enable: {
             const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
             if (col.isValid()) {
-                mController->setCollection(col, true, false);
+                mController->setCollectionState(col, Controller::Enabled);
+            } else {
+                const QVariant var = index.data(PersonRole);
+                if (var.isValid()) {
+                    mController->addPerson(var.value<Person>());
+                }
             }
         }
         break;
diff --git a/korganizer/views/collectionview/calendardelegate.cpp b/korganizer/views/collectionview/calendardelegate.cpp
index cf808cf..ead89a4 100644
--- a/korganizer/views/collectionview/calendardelegate.cpp
+++ b/korganizer/views/collectionview/calendardelegate.cpp
@@ -78,24 +78,20 @@ QList<StyledCalendarDelegate::Action> StyledCalendarDelegate::getActions(const Q
     const bool isSearchResult = index.data(IsSearchResultRole).toBool();
     const bool hover = option.state & QStyle::State_MouseOver;
     const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
-    const bool enabled = col.shouldList(Akonadi::Collection::ListDisplay);
-    const bool referenced = col.referenced();
+    Qt::CheckState enabled = static_cast<Qt::CheckState>(index.data(EnabledRole).toInt());
+    // kDebug() << index.data().toString() << enabled;
 
     QList<Action> buttons;
     if (isSearchResult) {
         buttons << AddToList;
     } else {
-        //Folders that have been pulled in due to a subfolder
-        // if (!enabled && !referenced) {
-        //     return QList<Action>() << RemoveFromList;
-        // }
         if (hover) {
-            if (!enabled) {
+            if (enabled != Qt::Checked) {
                 buttons << Enable;
             }
             buttons << RemoveFromList;
         } else {
-            if (enabled) {
+            if (enabled == Qt::Checked) {
                 buttons << Enable;
             }
         }
@@ -108,6 +104,10 @@ void StyledCalendarDelegate::paint( QPainter * painter, const QStyleOptionViewIt
     Q_ASSERT(index.isValid());
 
     const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
+    //We display the toolbuttons while hovering
+    const bool showButtons = option.state & QStyle::State_MouseOver;
+    // const bool enabled = col.shouldList(Akonadi::Collection::ListDisplay);
+    Qt::CheckState enabled = static_cast<Qt::CheckState>(index.data(EnabledRole).toInt());
 
     QStyleOptionViewItemV4 opt = option;
     initStyleOption(&opt, index);
@@ -121,6 +121,12 @@ void StyledCalendarDelegate::paint( QPainter * painter, const QStyleOptionViewIt
         int i = 1;
         Q_FOREACH (Action action, getActions(option, index)) {
             QStyleOptionButton buttonOption = buttonOpt(opt, mPixmap.value(action), i);
+            if (action == Enable && showButtons) {
+                buttonOption.state = QStyle::State_Active;
+            }
+            if (action == Enable && !showButtons && enabled == Qt::PartiallyChecked) {
+                buttonOption.state = QStyle::State_Active;
+            }
             s->drawControl(QStyle::CE_PushButton, &buttonOption, painter, 0);
             i++;
         }
diff --git a/korganizer/views/collectionview/controller.cpp b/korganizer/views/collectionview/controller.cpp
index c63d947..03f8949 100644
--- a/korganizer/views/collectionview/controller.cpp
+++ b/korganizer/views/collectionview/controller.cpp
@@ -88,6 +88,9 @@ QVariant CollectionNode::data(int role) const
     if (role == CollectionRole) {
         return QVariant::fromValue(mCollection);
     }
+    if (role == NodeTypeRole) {
+        return CollectionNodeRole;
+    }
     return QVariant();
 }
 
@@ -162,6 +165,9 @@ QVariant PersonNode::data(int role) const
     if (role == IsSearchResultRole) {
         return isSearchNode;
     }
+    if (role == NodeTypeRole) {
+        return PersonNodeRole;
+    }
     return QVariant();
 }
 
@@ -432,24 +438,6 @@ void Controller::setSearchString(const QString &searchString)
     mCollectionSearchJob->start();
 }
 
-void Controller::onPersonEnabled(bool enabled, const Person& person)
-{
-    setPersonEnabled(person, enabled);
-
-}
-
-void Controller::onPersonCollectionsFetched(KJob* job)
-{
-    if (job->error()) {
-        kWarning() << "Failed to fetch collections " << job->errorString();
-        return;
-    }
-    const bool enable = job->property("enable").toBool();
-    Q_FOREACH(const Akonadi::Collection &col, static_cast<Akonadi::CollectionFetchJob*>(job)->collections()) {
-        setCollectionReferenced(enable, col);
-    }
-}
-
 void Controller::onCollectionsFound(KJob* job)
 {
     if (job->error()) {
@@ -462,7 +450,7 @@ void Controller::onCollectionsFound(KJob* job)
         CollectionNode *collectionNode = new CollectionNode(*mSearchModel, col);
         collectionNode->isSearchNode = true;
         //toggled by the checkbox, results in collection getting monitored
-        connect(&collectionNode->emitter, SIGNAL(enabled(bool, Akonadi::Collection)), this, SLOT(onCollectionEnabled(bool, Akonadi::Collection)));
+        // connect(&collectionNode->emitter, SIGNAL(enabled(bool, Akonadi::Collection)), this, SLOT(onCollectionEnabled(bool, Akonadi::Collection)));
         mSearchModel->addNode(ReparentingModel::Node::Ptr(collectionNode));
     }
     mCollectionSearchJob = 0;
@@ -480,7 +468,7 @@ void Controller::onPersonsFound(KJob* job)
         PersonNode *personNode = new PersonNode(*mSearchModel, p);
         personNode->isSearchNode = true;
         //toggled by the checkbox, results in person getting added to main model
-        connect(&personNode->emitter, SIGNAL(enabled(bool, Person)), this, SLOT(onPersonEnabled(bool, Person)));
+        // connect(&personNode->emitter, SIGNAL(enabled(bool, Person)), this, SLOT(onPersonEnabled(bool, Person)));
         mSearchModel->addNode(ReparentingModel::Node::Ptr(personNode));
     }
     mPersonSearchJob = 0;
@@ -500,65 +488,67 @@ static Akonadi::EntityTreeModel *findEtm(QAbstractItemModel *model)
     return qobject_cast<Akonadi::EntityTreeModel*>(model);
 }
 
-void Controller::setCollectionReferenced(bool enabled, const Akonadi::Collection& collection)
-{
-    kDebug() << collection.displayName() << "do reference " << enabled;
-    kDebug() << "current " << collection.referenced();
-    Akonadi::EntityTreeModel *etm = findEtm(mPersonModel);
-    Q_ASSERT(etm);
-    etm->setCollectionReferenced(collection, enabled);
-}
-
-void Controller::setCollectionEnabled(bool enabled, const Akonadi::Collection& collection)
+void Controller::setCollectionState(const Akonadi::Collection &collection, CollectionState collectionState, bool recursive)
 {
-    kDebug() << collection.displayName() << "do enable " << enabled;
-    kDebug() << "current " << collection.enabled();
-
-    Akonadi::Collection modifiedCollection = collection;
-    modifiedCollection.setShouldList(Akonadi::Collection::ListDisplay, enabled);
-    new Akonadi::CollectionModifyJob(modifiedCollection);
-}
-
-void Controller::onCollectionEnabled(bool enabled, const Akonadi::Collection& collection)
-{
-    setCollectionReferenced(enabled, collection);
+    //We removed the children first, so the children in the tree are removed before the parents
+    if (recursive) {
+        //We have to include all mimetypes since mimetypes are not available yet (they will be synced once the collectoins are referenced)
+        Akonadi::CollectionFetchJob *fetchJob = new Akonadi::CollectionFetchJob(collection, Akonadi::CollectionFetchJob::Recursive, this);
+        fetchJob->setProperty("collectionState", static_cast<int>(collectionState));
+        fetchJob->fetchScope().setListFilter(Akonadi::CollectionFetchScope::NoFilter);
+        connect(fetchJob, SIGNAL(result(KJob*)), this, SLOT(onPersonCollectionsFetched(KJob*)));
+    }
+    {
+        Akonadi::CollectionFetchJob *fetchJob = new Akonadi::CollectionFetchJob(collection, Akonadi::CollectionFetchJob::Base, this);
+        fetchJob->setProperty("collectionState", static_cast<int>(collectionState));
+        fetchJob->fetchScope().setListFilter(Akonadi::CollectionFetchScope::NoFilter);
+        connect(fetchJob, SIGNAL(result(KJob*)), this, SLOT(onPersonCollectionsFetched(KJob*)));
+    }
 }
 
-void Controller::setCollection(const Akonadi::Collection &collection, bool enabled, bool referenced)
+void Controller::onPersonCollectionsFetched(KJob* job)
 {
+    if (job->error()) {
+        kWarning() << "Failed to fetch collections " << job->errorString();
+        return;
+    }
     Akonadi::EntityTreeModel *etm = findEtm(mPersonModel);
     if (!etm) {
         kWarning() << "Couldn't find etm";
         return;
     }
-    kDebug() << collection.displayName() << "do enable " << enabled;
-    Akonadi::Collection modifiedCollection = collection;
-    modifiedCollection.setShouldList(Akonadi::Collection::ListDisplay, enabled);
-    //HACK: We have no way of getting to the correct session as used by the etm,
-    //and two concurrent jobs end up overwriting the enabled state of each other.
-    etm->setCollectionReferenced(modifiedCollection, referenced);
+
+    const CollectionState collectionState = static_cast<CollectionState>(job->property("collectionState").toInt());
+    Q_FOREACH(const Akonadi::Collection &col, static_cast<Akonadi::CollectionFetchJob*>(job)->collections()) {
+        // kDebug() << col.displayName() << "do enable " << enabled;
+        Akonadi::Collection modifiedCollection = col;
+        if (collectionState == Enabled) {
+            modifiedCollection.setShouldList(Akonadi::Collection::ListDisplay, true);
+        }
+        if (collectionState == Disabled) {
+            modifiedCollection.setShouldList(Akonadi::Collection::ListDisplay, false);
+        }
+        //HACK: We have no way of getting to the correct session as used by the etm,
+        //and two concurrent jobs end up overwriting the enabled state of each other.
+        etm->setCollectionReferenced(modifiedCollection, collectionState == Referenced);
+    }
 }
 
-void Controller::setPersonEnabled(const Person &person, bool enabled)
+void Controller::addPerson(const Person &person)
 {
-    // kDebug() << person.name << enabled;
-    if (enabled) {
-        PersonNode *personNode = new PersonNode(*mPersonModel, person);
-        personNode->setChecked(true);
-        mPersonModel->addNode(ReparentingModel::Node::Ptr(personNode));
-    } else {
-        mPersonModel->removeNode(PersonNode(*mPersonModel, person));
-    }
+    kDebug() << person.name;
+    PersonNode *personNode = new PersonNode(*mPersonModel, person);
+    personNode->setChecked(true);
+    mPersonModel->addNode(ReparentingModel::Node::Ptr(personNode));
 
-    Akonadi::Collection rootCollection(person.rootCollection);
-    // kDebug() << rootCollection.id();
-    if (rootCollection.isValid()) {
-        //Reference the persons collections if available
-        //We have to include all mimetypes since mimetypes are not available yet (they will be synced once the collectoins are referenced)
-        Akonadi::CollectionFetchJob *fetchJob = new Akonadi::CollectionFetchJob(rootCollection, Akonadi::CollectionFetchJob::Recursive, this);
-        fetchJob->setProperty("enable", enabled);
-        fetchJob->fetchScope().setListFilter(Akonadi::CollectionFetchScope::NoFilter);
-        connect(fetchJob, SIGNAL(result(KJob*)), this, SLOT(onPersonCollectionsFetched(KJob*)));
-    }
+    setCollectionState(Akonadi::Collection(person.rootCollection), Referenced, true);
+}
+
+void Controller::removePerson(const Person &person)
+{
+    kDebug() << person.name;
+    mPersonModel->removeNode(PersonNode(*mPersonModel, person));
+
+    setCollectionState(Akonadi::Collection(person.rootCollection), Disabled, true);
 }
 
diff --git a/korganizer/views/collectionview/controller.h b/korganizer/views/collectionview/controller.h
index 981b840..a90a851 100644
--- a/korganizer/views/collectionview/controller.h
+++ b/korganizer/views/collectionview/controller.h
@@ -29,10 +29,18 @@
 #include <Akonadi/Collection>
 #include "reparentingmodel.h"
 
-enum DataRole {
+enum DataRoles {
     PersonRole = Akonadi::EntityTreeModel::UserRole + 1,
     IsSearchResultRole,
-    CollectionRole
+    CollectionRole,
+    NodeTypeRole,
+    EnabledRole
+};
+
+enum NodeTypeRoles {
+    SourceNodeRole,
+    PersonNodeRole,
+    CollectionNodeRole
 };
 
 struct Person
@@ -175,10 +183,15 @@ public:
      */
     void setEntityTreeModel(Akonadi::EntityTreeModel *etm);
 
-    void setCollectionReferenced(bool enabled, const Akonadi::Collection &collection);
-    void setCollectionEnabled(bool enabled, const Akonadi::Collection &collection);
-    void setCollection(const Akonadi::Collection &collection, bool enabled, bool referenced);
-    void setPersonEnabled(const Person &person, bool enabled);
+    enum CollectionState {
+        Disabled,
+        Referenced,
+        Enabled
+    };
+    void setCollectionState(const Akonadi::Collection &collection, CollectionState collectionState, bool recursive = false);
+
+    void addPerson(const Person &person);
+    void removePerson(const Person &person);
 
 Q_SIGNALS:
     void searchIsActive(bool);
@@ -189,9 +202,7 @@ public Q_SLOTS:
 private Q_SLOTS:
     void onCollectionsFound(KJob *job);
     void onPersonsFound(KJob *job);
-    void onPersonEnabled(bool enabled, const Person &person);
     void onPersonCollectionsFetched(KJob *job);
-    void onCollectionEnabled(bool enabled, const Akonadi::Collection &collection);
 
 private:
     ReparentingModel *mPersonModel;


commit 6e3eac7679c64937f7f1b407654c28dc8000fb30
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Sun Sep 14 23:19:53 2014 +0200

    AkonadiCollectionView: Reorder search model stack
    
    Make the filter model the toplevel to avoid issues in the searchProxy due to
    filtered collections (can affect deduplication). Further we use the collectionFilter
    as input to avoid getting unwanted results in the search view.

diff --git a/korganizer/akonadicollectionview.cpp b/korganizer/akonadicollectionview.cpp
index faf06f9..fa2c425 100644
--- a/korganizer/akonadicollectionview.cpp
+++ b/korganizer/akonadicollectionview.cpp
@@ -429,17 +429,18 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
 
 
   //Filter tree view.
+  ReparentingModel *searchProxy = new ReparentingModel( this );
+  searchProxy->setSourceModel(collectionFilter);
+  searchProxy->setObjectName(QLatin1String("searchProxy"));
+
   KRecursiveFilterProxyModel *filterTreeViewModel = new KRecursiveFilterProxyModel( this );
   filterTreeViewModel->setDynamicSortFilter( true );
-  filterTreeViewModel->setSourceModel( userProxy );
+  filterTreeViewModel->setSourceModel( searchProxy );
   filterTreeViewModel->setFilterCaseSensitivity( Qt::CaseInsensitive );
 //   filterTreeViewModel->setObjectName( "Recursive filtering, for the search bar" );
   connect( searchCol, SIGNAL(textChanged(QString)),
           filterTreeViewModel, SLOT(setFilterFixedString(QString)) );
 
-  ReparentingModel *searchProxy = new ReparentingModel( this );
-  searchProxy->setSourceModel(filterTreeViewModel);
-
 
   Akonadi::EntityTreeView *mSearchView = new Akonadi::EntityTreeView( this );
   mSearchView->header()->hide();
@@ -449,7 +450,7 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
     connect(delegate, SIGNAL(action(QModelIndex, int)), this, SLOT(onAction(QModelIndex, int)));
     mSearchView->setItemDelegate( delegate );
   }
-  mSearchView->setModel( searchProxy );
+  mSearchView->setModel( filterTreeViewModel );
   new NewNodeExpander(mSearchView, true, QString());
 
   mController = new Controller(userProxy, searchProxy, this);


commit 455f7f0d00c43a57d7b011c3b07cfcb17027565e
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Sun Sep 14 19:53:54 2014 +0200

    Moved actions to the appropriate place

diff --git a/korganizer/actionmanager.cpp b/korganizer/actionmanager.cpp
index 2732cf4..6e8e610 100644
--- a/korganizer/actionmanager.cpp
+++ b/korganizer/actionmanager.cpp
@@ -327,15 +327,6 @@ void ActionManager::initActions()
 
   /************************** EDIT MENU *********************************/
 
-  //Disable a calendar or remove a referenced calendar
-  QAction *disableAction = mACollection->addAction( QLatin1String("collection_disable"), mCollectionView, SLOT(edit_disable()) );
-  disableAction->setText( i18n( "Disable Calendar" ) );
-
-  //Enable (subscribe) to a calendar.
-  QAction *enableAction = mACollection->addAction( QLatin1String("collection_enable"), mCollectionView, SLOT(edit_enable()) );
-  enableAction->setText( i18n( "Enable Calendar" ) );
-  //TODO: hide option on enabled collections
-
   QAction *pasteAction;
   Akonadi::History *history = mCalendarView->history();
   if ( mIsPart ) {
diff --git a/korganizer/akonadicollectionview.cpp b/korganizer/akonadicollectionview.cpp
index 71a7fcf..faf06f9 100644
--- a/korganizer/akonadicollectionview.cpp
+++ b/korganizer/akonadicollectionview.cpp
@@ -63,6 +63,7 @@
 #include <QStyledItemDelegate>
 #include <QVBoxLayout>
 #include <QStackedWidget>
+#include <QAction>
 
 static Akonadi::EntityTreeModel *findEtm(QAbstractItemModel *model)
 {
@@ -552,6 +553,17 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
     mDefaultCalendar->setEnabled( false );
     xmlclient->actionCollection()->addAction( QString::fromLatin1( "set_standard_calendar" ),
                                               mDefaultCalendar );
+
+    //Disable a calendar or remove a referenced calendar
+    QAction *disableAction = xmlclient->actionCollection()->addAction( QLatin1String("collection_disable"), this, SLOT(edit_disable()) );
+    disableAction->setText( i18n( "Remove from list" ) );
+    disableAction->setIcon(KIconLoader().loadIcon(QLatin1String("list-remove"), KIconLoader::Small));
+
+    //Enable (subscribe) to a calendar.
+    mEnableAction = xmlclient->actionCollection()->addAction( QLatin1String("collection_enable"), this, SLOT(edit_enable()) );
+    mEnableAction->setText( i18n( "Add to list permanently" ) );
+    mEnableAction->setIcon(KIconLoader().loadIcon(QLatin1String("bookmarks"), KIconLoader::Small));
+
     connect( mDefaultCalendar, SIGNAL(triggered(bool)), this, SLOT(setDefaultCalendar()) );
   }
 }
@@ -674,6 +686,11 @@ void AkonadiCollectionView::updateMenu()
                                     collection.rights() & Akonadi::Collection::CanCreateItem );
       disableStuff = false;
     }
+    if ( collection.isValid() && collection.shouldList(Akonadi::Collection::ListDisplay) ) {
+        mEnableAction->setEnabled(false);
+    } else {
+        mEnableAction->setEnabled(true);
+    }
   }
   if ( disableStuff ) {
     mDisableColor->setEnabled( false );
diff --git a/korganizer/akonadicollectionview.h b/korganizer/akonadicollectionview.h
index 5e31b05..2be40a8 100644
--- a/korganizer/akonadicollectionview.h
+++ b/korganizer/akonadicollectionview.h
@@ -122,6 +122,7 @@ class AkonadiCollectionView : public CalendarViewExtension
     KAction *mAssignColor;
     KAction *mDisableColor;
     KAction *mDefaultCalendar;
+    QAction *mEnableAction;
     bool mNotSendAddRemoveSignal;
     bool mWasDefaultCalendar;
     bool mHasContextMenu;


commit 3a3bb9574ef78c5652f1063186e436e1eca42728
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Sun Sep 14 14:07:39 2014 +0200

    CalendarDelegate: cleanup, uniform size

diff --git a/korganizer/views/collectionview/calendardelegate.cpp b/korganizer/views/collectionview/calendardelegate.cpp
index 73a6f5f..cf808cf 100644
--- a/korganizer/views/collectionview/calendardelegate.cpp
+++ b/korganizer/views/collectionview/calendardelegate.cpp
@@ -63,14 +63,14 @@ static QStyle *style(const QStyleOptionViewItem &option)
 
 static QStyleOptionButton buttonOpt(const QStyleOptionViewItemV4 &opt, const QPixmap &pixmap, int pos = 1)
 {
-    QStyleOptionButton buttonOpt;
-    buttonOpt.icon = pixmap;
+    QStyleOptionButton option;
+    option.icon = pixmap;
     QRect r = opt.rect;
     const int h = r.height() - 4;
-    buttonOpt.rect = enableButtonRect(r, pos);
-    buttonOpt.state = QStyle::State_Active | QStyle::State_Enabled;
-    buttonOpt.iconSize = QSize(h, h);
-    return buttonOpt;
+    option.rect = enableButtonRect(r, pos);
+    option.state = QStyle::State_Active | QStyle::State_Enabled;
+    option.iconSize = QSize(h, h);
+    return option;
 }
 
 QList<StyledCalendarDelegate::Action> StyledCalendarDelegate::getActions(const QStyleOptionViewItem &option, const QModelIndex &index) const
@@ -130,15 +130,12 @@ void StyledCalendarDelegate::paint( QPainter * painter, const QStyleOptionViewIt
     if (opt.checkState){
         QColor color = KOHelper::resourceColor(col);
         if (color.isValid()){
-            QRect r = opt.rect;
-            const int h = r.height()- 4;
-            r.adjust(r.width()- h - 2, 2, - 2, -2);
             painter->save();
             painter->setRenderHint(QPainter::Antialiasing);
             QPen pen = painter->pen();
             pen.setColor(color);
             QPainterPath path;
-            path.addRoundedRect(r, 5, 5);
+            path.addRoundedRect(enableButtonRect(opt.rect, 0), 5, 5);
             color.setAlpha(200);
             painter->fillPath(path, color);
             painter->strokePath(path, pen);
@@ -197,6 +194,9 @@ bool StyledCalendarDelegate::editorEvent(QEvent *event,
 
 QSize StyledCalendarDelegate::sizeHint( const QStyleOptionViewItem &option, const QModelIndex &index ) const
 {
-    return QStyledItemDelegate::sizeHint(option, index);
+    QSize size = QStyledItemDelegate::sizeHint(option, index);
+    //Without this adjustment toplevel resource folders get a slightly greater height, which looks silly and breaks the toolbutton position.
+    size.setHeight(mPixmap.value(AddToList).height() + 4);
+    return size;
 }
 


commit 89677f80f25e57e1e17223748c6895b78b20296e
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Sun Sep 14 12:22:03 2014 +0200

    CalendarSelection: only show tool buttons on hover and display multiple tool buttons.

diff --git a/korganizer/akonadicollectionview.cpp b/korganizer/akonadicollectionview.cpp
index b7994db..71a7fcf 100644
--- a/korganizer/akonadicollectionview.cpp
+++ b/korganizer/akonadicollectionview.cpp
@@ -248,41 +248,6 @@ static bool hasCompatibleMimeTypes( const Akonadi::Collection &collection )
   return false;
 }
 
-class ColorDelegate : public QStyledItemDelegate
-{
-  public:
-    explicit ColorDelegate( QObject * parent = 0 ) : QStyledItemDelegate( parent )
-    {
-    }
-
-    void paint ( QPainter *painter, const QStyleOptionViewItem &option,
-                 const QModelIndex &index ) const
-    {
-      QStyledItemDelegate::paint( painter, option, index );
-      QStyleOptionViewItemV4 v4 = option;
-      initStyleOption( &v4, index );
-      if ( v4.checkState ) {
-        const Akonadi::Collection collection = CalendarSupport::collectionFromIndex( index );
-        QColor color = KOHelper::resourceColor( collection );
-        if ( color.isValid() ) {
-          QRect r = v4.rect;
-          const int h = r.height() - 4;
-          r.adjust( r.width() - h - 2, 2, - 2, -2 );
-          painter->save();
-          painter->setRenderHint( QPainter::Antialiasing );
-          QPen pen = painter->pen();
-          pen.setColor( color );
-          QPainterPath path;
-          path.addRoundedRect( r, 5, 5 );
-          color.setAlpha( 200 );
-          painter->fillPath( path, color );
-          painter->strokePath( path, pen );
-          painter->restore();
-        }
-      }
-    }
-};
-
 class SortProxyModel : public QSortFilterProxyModel
 {
   public:
@@ -294,8 +259,8 @@ class SortProxyModel : public QSortFilterProxyModel
      bool lessThan(const QModelIndex &left,
                                        const QModelIndex &right) const
     {
-        QVariant leftPerson = left.data(PersonNode::PersonRole);
-        QVariant rightPerson = right.data(PersonNode::PersonRole);
+        QVariant leftPerson = left.data(PersonRole);
+        QVariant rightPerson = right.data(PersonRole);
         if (leftPerson.isValid() && !rightPerson.isValid()) {
             return true;
         }
@@ -452,7 +417,7 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
   mCollectionView->setRootIsDecorated( true );
   {
     StyledCalendarDelegate *delegate = new StyledCalendarDelegate(mCollectionView);
-    connect(delegate, SIGNAL(enabled(QModelIndex, bool)), this, SLOT(onCalendarEnabled(QModelIndex, bool)));
+    connect(delegate, SIGNAL(action(QModelIndex, int)), this, SLOT(onAction(QModelIndex, int)));
     mCollectionView->setItemDelegate( delegate );
   }
   mCollectionView->setModel( collectionFilter );
@@ -478,7 +443,11 @@ AkonadiCollectionView::AkonadiCollectionView( CalendarView *view, bool hasContex
   Akonadi::EntityTreeView *mSearchView = new Akonadi::EntityTreeView( this );
   mSearchView->header()->hide();
   mSearchView->setRootIsDecorated( true );
-  mSearchView->setItemDelegate( new ColorDelegate( this ) );
+  {
+    StyledCalendarDelegate *delegate = new StyledCalendarDelegate(mCollectionView);
+    connect(delegate, SIGNAL(action(QModelIndex, int)), this, SLOT(onAction(QModelIndex, int)));
+    mSearchView->setItemDelegate( delegate );
+  }
   mSearchView->setModel( searchProxy );
   new NewNodeExpander(mSearchView, true, QString());
 
@@ -892,13 +861,23 @@ void AkonadiCollectionView::edit_enable()
     }
 }
 
-void AkonadiCollectionView::onCalendarEnabled(const QModelIndex &index, bool enabled)
-{
-    const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
-    if (col.isValid()) {
-        mController->setCollection(col, enabled, false);
-    } else {
-        if (!enabled) {
+void AkonadiCollectionView::onAction(const QModelIndex &index, int a)
+{
+    const StyledCalendarDelegate::Action action = static_cast<StyledCalendarDelegate::Action>(a);
+    switch (action) {
+        case StyledCalendarDelegate::AddToList: {
+            const Akonadi::Collection col = index.data(CollectionRole).value<Akonadi::Collection>();
+            if (col.isValid()) {
+                mController->setCollection(col, false, true);
+            } else {
+                const QVariant var = index.data(PersonRole);
+                if (var.isValid()) {
+                    mController->setPersonEnabled(var.value<Person>(), true);
+                }
+            }
+        }
+        break;
+        case StyledCalendarDelegate::RemoveFromList: {
             //Disable all child collections
             const QAbstractItemModel *model = index.model();
             for (int row = 0; row < model->rowCount(index); row++) {
@@ -907,12 +886,24 @@ void AkonadiCollectionView::onCalendarEnabled(const QModelIndex &index, bool ena
                     mController->setCollection(col, false, false);
                 }
             }
+            const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
+            if (col.isValid()) {
+                mController->setCollection(col, false, false);
+            } else {
+                const QVariant var = index.data(PersonRole);
+                if (var.isValid()) {
+                    mController->setPersonEnabled(var.value<Person>(), false);
+                }
+            }
         }
-        
-        const QVariant var = index.data(PersonNode::PersonRole);
-        if (var.isValid()) {
-            mController->setPersonDisabled(var.value<Person>());
+        break;
+        case StyledCalendarDelegate::Enable: {
+            const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
+            if (col.isValid()) {
+                mController->setCollection(col, true, false);
+            }
         }
+        break;
     }
 }
 
diff --git a/korganizer/akonadicollectionview.h b/korganizer/akonadicollectionview.h
index 8c68155..5e31b05 100644
--- a/korganizer/akonadicollectionview.h
+++ b/korganizer/akonadicollectionview.h
@@ -109,7 +109,7 @@ class AkonadiCollectionView : public CalendarViewExtension
     void disableColor();
     void setDefaultCalendar();
     void onSearchIsActive(bool);
-    void onCalendarEnabled(const QModelIndex &, bool);
+    void onAction(const QModelIndex &index, int action);
 
   private:
     Akonadi::EntityTreeModel *entityTreeModel() const;
diff --git a/korganizer/views/collectionview/calendardelegate.cpp b/korganizer/views/collectionview/calendardelegate.cpp
index a19e14f..73a6f5f 100644
--- a/korganizer/views/collectionview/calendardelegate.cpp
+++ b/korganizer/views/collectionview/calendardelegate.cpp
@@ -27,10 +27,14 @@
 
 #include <calendarsupport/utils.h>
 #include <kohelper.h>
+#include "controller.h"
 
 StyledCalendarDelegate::StyledCalendarDelegate(QObject * parent)
     : QStyledItemDelegate(parent)
 {
+    mPixmap.insert(Enable, KIconLoader().loadIcon(QLatin1String("bookmarks"), KIconLoader::Small));
+    mPixmap.insert(RemoveFromList, KIconLoader().loadIcon(QLatin1String("list-remove"), KIconLoader::Small));
+    mPixmap.insert(AddToList, KIconLoader().loadIcon(QLatin1String("list-add"), KIconLoader::Small));
 }
 
 StyledCalendarDelegate::~StyledCalendarDelegate()
@@ -38,12 +42,13 @@ StyledCalendarDelegate::~StyledCalendarDelegate()
 
 }
 
-static QRect enableButtonRect(const QRect &rect)
+static QRect enableButtonRect(const QRect &rect, int pos = 1)
 {
-    QRect r = rect;
-    const int h = r.height()- 4;
-    r.adjust(r.width()- h*2 - 2*2, 2, -2 - h - 2, -2);
-    return r;
+    //2px border on each side of the icon
+    static int border = 2;
+    const int side = rect.height() - (2 * border);
+    const int offset = side * pos + border * (pos + 1);
+    return rect.adjusted(rect.width() - (offset + side), border, -offset, -border);
 }
 
 static QStyle *style(const QStyleOptionViewItem &option)
@@ -54,38 +59,71 @@ static QStyle *style(const QStyleOptionViewItem &option)
     }
     QStyle *style = widget ? widget->style() : QApplication::style();
     return style;
+}
+
+static QStyleOptionButton buttonOpt(const QStyleOptionViewItemV4 &opt, const QPixmap &pixmap, int pos = 1)
+{
+    QStyleOptionButton buttonOpt;
+    buttonOpt.icon = pixmap;
+    QRect r = opt.rect;
+    const int h = r.height() - 4;
+    buttonOpt.rect = enableButtonRect(r, pos);
+    buttonOpt.state = QStyle::State_Active | QStyle::State_Enabled;
+    buttonOpt.iconSize = QSize(h, h);
+    return buttonOpt;
+}
+
+QList<StyledCalendarDelegate::Action> StyledCalendarDelegate::getActions(const QStyleOptionViewItem &option, const QModelIndex &index) const
+{
+    const bool isSearchResult = index.data(IsSearchResultRole).toBool();
+    const bool hover = option.state & QStyle::State_MouseOver;
+    const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
+    const bool enabled = col.shouldList(Akonadi::Collection::ListDisplay);
+    const bool referenced = col.referenced();
 
+    QList<Action> buttons;
+    if (isSearchResult) {
+        buttons << AddToList;
+    } else {
+        //Folders that have been pulled in due to a subfolder
+        // if (!enabled && !referenced) {
+        //     return QList<Action>() << RemoveFromList;
+        // }
+        if (hover) {
+            if (!enabled) {
+                buttons << Enable;
+            }
+            buttons << RemoveFromList;
+        } else {
+            if (enabled) {
+                buttons << Enable;
+            }
+        }
+    }
+    return buttons;
 }
 
 void StyledCalendarDelegate::paint( QPainter * painter, const QStyleOptionViewItem & option, const QModelIndex & index ) const
 {
     Q_ASSERT(index.isValid());
 
-    QStyledItemDelegate::paint( painter, option, index );
+    const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
+
     QStyleOptionViewItemV4 opt = option;
     initStyleOption(&opt, index);
+    QStyledItemDelegate::paint(painter, opt, index);
 
     QStyle *s = style(option);
 
-    const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
-
-    //Favorite button
+    //Buttons
     {
-        static QPixmap enablePixmap = KIconLoader().loadIcon(QLatin1String("bookmarks"), KIconLoader::Small);
-        static QPixmap disablePixmap = KIconLoader().loadIcon(QLatin1String("window-close"), KIconLoader::Small);
-        QStyleOptionButton buttonOpt;
-        if (!col.shouldList(Akonadi::Collection::ListDisplay)) {
-            buttonOpt.icon = enablePixmap;
-        } else {
-            buttonOpt.icon = disablePixmap;
+        QList<Action> buttons;
+        int i = 1;
+        Q_FOREACH (Action action, getActions(option, index)) {
+            QStyleOptionButton buttonOption = buttonOpt(opt, mPixmap.value(action), i);
+            s->drawControl(QStyle::CE_PushButton, &buttonOption, painter, 0);
+            i++;
         }
-        QRect r = opt.rect;
-        const int h = r.height()- 4;
-        buttonOpt.rect = enableButtonRect(r);
-        buttonOpt.state = QStyle::State_Active | QStyle::State_Enabled;
-        buttonOpt.iconSize = QSize(h, h);
-
-        s->drawControl(QStyle::CE_PushButton, &buttonOpt, painter, 0);
     }
 
     //Color indicator
@@ -117,15 +155,24 @@ bool StyledCalendarDelegate::editorEvent(QEvent *event,
     Q_ASSERT(event);
     Q_ASSERT(model);
 
+    int button = -1;
     // make sure that we have the right event type
     if ((event->type() == QEvent::MouseButtonRelease)
         || (event->type() == QEvent::MouseButtonDblClick)
         || (event->type() == QEvent::MouseButtonPress)) {
 
-        QRect buttonRect = enableButtonRect(option.rect);
-
         QMouseEvent *me = static_cast<QMouseEvent*>(event);
-        if (me->button() != Qt::LeftButton || !buttonRect.contains(me->pos())) {
+
+        if (enableButtonRect(option.rect, 1).contains(me->pos())) {
+            button = 1;
+        }
+        if (enableButtonRect(option.rect, 2).contains(me->pos())) {
+            button = 2;
+        }
+        if (enableButtonRect(option.rect, 3).contains(me->pos())) {
+            button = 3;
+        }
+        if (me->button() != Qt::LeftButton || button < 0) {
             return QStyledItemDelegate::editorEvent(event, model, option, index);
         }
 
@@ -137,14 +184,15 @@ bool StyledCalendarDelegate::editorEvent(QEvent *event,
         return QStyledItemDelegate::editorEvent(event, model, option, index);
     }
 
-    onEnableButtonClicked(index);
-    return true;
-}
+    Q_ASSERT(button > 0);
+    QStyleOptionViewItem opt = option;
+    opt.state |= QStyle::State_MouseOver;
 
-void StyledCalendarDelegate::onEnableButtonClicked(const QModelIndex &index)
-{
-    const Akonadi::Collection col = CalendarSupport::collectionFromIndex(index);
-    emit enabled(index, !col.enabled());
+    const Action a = getActions(opt, index).at(button - 1);
+    // kDebug() << "Button clicked: " << a;
+    emit action(index, a);
+
+    return true;
 }
 
 QSize StyledCalendarDelegate::sizeHint( const QStyleOptionViewItem &option, const QModelIndex &index ) const
diff --git a/korganizer/views/collectionview/calendardelegate.h b/korganizer/views/collectionview/calendardelegate.h
index a43bdd6..788a1d3 100644
--- a/korganizer/views/collectionview/calendardelegate.h
+++ b/korganizer/views/collectionview/calendardelegate.h
@@ -32,8 +32,14 @@ public:
     void paint( QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index ) const;
     QSize sizeHint( const QStyleOptionViewItem &option, const QModelIndex &index ) const;
 
+    enum Action {
+        AddToList,
+        RemoveFromList,
+        Enable
+    };
+
 Q_SIGNALS:
-    void enabled(const QModelIndex &, bool);
+    void action(const QModelIndex &, int);
 
 protected:
     bool editorEvent(QEvent *event,
@@ -42,7 +48,8 @@ protected:
                     const QModelIndex &index);
 
 private:
-    void onEnableButtonClicked(const QModelIndex &index);
+    QList<Action> getActions(const QStyleOptionViewItem & option, const QModelIndex &index) const;
+    QHash<Action, QPixmap> mPixmap;
 };
     
 #endif
diff --git a/korganizer/views/collectionview/controller.cpp b/korganizer/views/collectionview/controller.cpp
index 83fac64..c63d947 100644
--- a/korganizer/views/collectionview/controller.cpp
+++ b/korganizer/views/collectionview/controller.cpp
@@ -37,7 +37,8 @@
 CollectionNode::CollectionNode(ReparentingModel& personModel, const Akonadi::Collection& col)
 :   Node(personModel),
     mCollection(col),
-    mCheckState(Qt::Unchecked)
+    mCheckState(Qt::Unchecked),
+    isSearchNode(false)
 {
 }
 
@@ -73,11 +74,20 @@ QVariant CollectionNode::data(int role) const
         return QVariant();
     }
     if (role == Qt::CheckStateRole) {
+        if (isSearchNode) {
+            return QVariant();
+        }
         return mCheckState;
     }
     if (role == Qt::ToolTipRole) {
         return QString(QLatin1String("Collection: ") + mCollection.name() + QString::number(mCollection.id()));
     }
+    if (role == IsSearchResultRole) {
+        return isSearchNode;
+    }
+    if (role == CollectionRole) {
+        return QVariant::fromValue(mCollection);
+    }
     return QVariant();
 }
 
@@ -100,7 +110,8 @@ bool CollectionNode::isDuplicateOf(const QModelIndex& sourceIndex)
 PersonNode::PersonNode(ReparentingModel& personModel, const Person& person)
 :   Node(personModel),
     mPerson(person),
-    mCheckState(Qt::Unchecked)
+    mCheckState(Qt::Unchecked),
+    isSearchNode(false)
 {
 
 }
@@ -137,6 +148,9 @@ QVariant PersonNode::data(int role) const
         return KIcon(QLatin1String("meeting-participant"));
     }
     if (role == Qt::CheckStateRole) {
+        if (isSearchNode) {
+            return QVariant();
+        }
         return mCheckState;
     }
     if (role == Qt::ToolTipRole) {
@@ -145,6 +159,9 @@ QVariant PersonNode::data(int role) const
     if (role == PersonRole) {
         return QVariant::fromValue(mPerson);
     }
+    if (role == IsSearchResultRole) {
+        return isSearchNode;
+    }
     return QVariant();
 }
 
@@ -184,11 +201,11 @@ bool PersonNode::isDuplicateOf(const QModelIndex& sourceIndex)
 void PersonNodeManager::checkSourceIndex(const QModelIndex &sourceIndex)
 {
     const Akonadi::Collection col = sourceIndex.data(Akonadi::EntityTreeModel::CollectionRole).value<Akonadi::Collection>();
-    kDebug() << col.displayName() << col.enabled();
+    // kDebug() << col.displayName() << col.enabled();
     if (col.isValid()) {
         CollectionIdentificationAttribute *attr = col.attribute<CollectionIdentificationAttribute>();
         if (attr && attr->collectionNamespace() == "usertoplevel") {
-            kDebug() << "Found user folder, creating person node";
+            kDebug() << "Found user folder, creating person node " << col.displayName();
             Person person;
             person.name = col.displayName();
             person.rootCollection = col.id();
@@ -198,6 +215,23 @@ void PersonNodeManager::checkSourceIndex(const QModelIndex &sourceIndex)
     }
 }
 
+void PersonNodeManager::checkSourceIndexRemoval(const QModelIndex &sourceIndex)
+{
+    const Akonadi::Collection col = sourceIndex.data(Akonadi::EntityTreeModel::CollectionRole).value<Akonadi::Collection>();
+    // kDebug() << col.displayName() << col.enabled();
+    if (col.isValid()) {
+        CollectionIdentificationAttribute *attr = col.attribute<CollectionIdentificationAttribute>();
+        if (attr && attr->collectionNamespace() == "usertoplevel") {
+            kDebug() << "Found user folder, removing person node " << col.displayName();
+            Person person;
+            person.name = col.displayName();
+            person.rootCollection = col.id();
+            model.removeNode(PersonNode(model, person));
+        }
+    }
+}
+
+
 CollectionSearchJob::CollectionSearchJob(const QString& searchString, QObject* parent)
     : KJob(parent),
     mSearchString(searchString)
@@ -400,25 +434,7 @@ void Controller::setSearchString(const QString &searchString)
 
 void Controller::onPersonEnabled(bool enabled, const Person& person)
 {
-    // kDebug() << person.name << enabled;
-    if (enabled) {
-        PersonNode *personNode = new PersonNode(*mPersonModel, person);
-        personNode->setChecked(true);
-        mPersonModel->addNode(ReparentingModel::Node::Ptr(personNode));
-        Akonadi::Collection rootCollection(person.rootCollection);
-        if (rootCollection.isValid()) {
-            //Reference the persons collections if available
-            //We have to include all mimetypes since mimetypes are not available yet (they will be synced once the collectoins are referenced)
-            Akonadi::CollectionFetchJob *fetchJob = new Akonadi::CollectionFetchJob(rootCollection, Akonadi::CollectionFetchJob::Recursive, this);
-            fetchJob->setProperty("enable", enabled);
-            fetchJob->fetchScope().setListFilter(Akonadi::CollectionFetchScope::NoFilter);
-            connect(fetchJob, SIGNAL(result(KJob*)), this, SLOT(onPersonCollectionsFetched(KJob*)));
-        }
-    } else {
-        //If we accidentally added a person and want to remove it again
-        mPersonModel->removeNode(PersonNode(*mPersonModel, person));
-        //Dereference subcollections
-    }
+    setPersonEnabled(person, enabled);
 
 }
 
@@ -444,6 +460,7 @@ void Controller::onCollectionsFound(KJob* job)
     Q_ASSERT(mCollectionSearchJob == static_cast<CollectionSearchJob*>(job));
     Q_FOREACH(const Akonadi::Collection &col, mCollectionSearchJob->matchingCollections()) {
         CollectionNode *collectionNode = new CollectionNode(*mSearchModel, col);
+        collectionNode->isSearchNode = true;
         //toggled by the checkbox, results in collection getting monitored
         connect(&collectionNode->emitter, SIGNAL(enabled(bool, Akonadi::Collection)), this, SLOT(onCollectionEnabled(bool, Akonadi::Collection)));
         mSearchModel->addNode(ReparentingModel::Node::Ptr(collectionNode));
@@ -461,6 +478,7 @@ void Controller::onPersonsFound(KJob* job)
     Q_ASSERT(mPersonSearchJob == static_cast<PersonSearchJob*>(job));
     Q_FOREACH(const Person &p, mPersonSearchJob->matches()) {
         PersonNode *personNode = new PersonNode(*mSearchModel, p);
+        personNode->isSearchNode = true;
         //toggled by the checkbox, results in person getting added to main model
         connect(&personNode->emitter, SIGNAL(enabled(bool, Person)), this, SLOT(onPersonEnabled(bool, Person)));
         mSearchModel->addNode(ReparentingModel::Node::Ptr(personNode));
@@ -521,8 +539,26 @@ void Controller::setCollection(const Akonadi::Collection &collection, bool enabl
     etm->setCollectionReferenced(modifiedCollection, referenced);
 }
 
-void Controller::setPersonDisabled(const Person &person)
+void Controller::setPersonEnabled(const Person &person, bool enabled)
 {
-    mPersonModel->removeNode(PersonNode(*mPersonModel, person));
+    // kDebug() << person.name << enabled;
+    if (enabled) {
+        PersonNode *personNode = new PersonNode(*mPersonModel, person);
+        personNode->setChecked(true);
+        mPersonModel->addNode(ReparentingModel::Node::Ptr(personNode));
+    } else {
+        mPersonModel->removeNode(PersonNode(*mPersonModel, person));
+    }
+
+    Akonadi::Collection rootCollection(person.rootCollection);
+    // kDebug() << rootCollection.id();
+    if (rootCollection.isValid()) {
+        //Reference the persons collections if available
+        //We have to include all mimetypes since mimetypes are not available yet (they will be synced once the collectoins are referenced)
+        Akonadi::CollectionFetchJob *fetchJob = new Akonadi::CollectionFetchJob(rootCollection, Akonadi::CollectionFetchJob::Recursive, this);
+        fetchJob->setProperty("enable", enabled);
+        fetchJob->fetchScope().setListFilter(Akonadi::CollectionFetchScope::NoFilter);
+        connect(fetchJob, SIGNAL(result(KJob*)), this, SLOT(onPersonCollectionsFetched(KJob*)));
+    }
 }
 
diff --git a/korganizer/views/collectionview/controller.h b/korganizer/views/collectionview/controller.h
index a63747d..981b840 100644
--- a/korganizer/views/collectionview/controller.h
+++ b/korganizer/views/collectionview/controller.h
@@ -29,6 +29,12 @@
 #include <Akonadi/Collection>
 #include "reparentingmodel.h"
 
+enum DataRole {
+    PersonRole = Akonadi::EntityTreeModel::UserRole + 1,
+    IsSearchResultRole,
+    CollectionRole
+};
+
 struct Person
 {
     Person(): rootCollection(-1){};
@@ -69,10 +75,6 @@ public:
 class PersonNode : public ReparentingModel::Node
 {
 public:
-    enum DataRole {
-        PersonRole = Akonadi::EntityTreeModel::UserRole + 1
-    };
-
     PersonNode(ReparentingModel &personModel, const Person &person);
     virtual ~PersonNode();
     virtual bool operator==(const Node &) const;
@@ -82,6 +84,8 @@ public:
     virtual QVariant data(int role) const;
 
     Emitter emitter;
+    bool isSearchNode;
+
 private:
     virtual bool setData(const QVariant& variant, int role);
     virtual bool adopts(const QModelIndex& sourceIndex);
@@ -99,6 +103,7 @@ public:
     virtual bool operator==(const Node &) const;
 
     Emitter emitter;
+    bool isSearchNode;
 
 private:
     virtual QVariant data(int role) const;
@@ -114,6 +119,7 @@ public:
     PersonNodeManager(ReparentingModel &personModel) : ReparentingModel::NodeManager(personModel){};
 private:
     void checkSourceIndex(const QModelIndex &sourceIndex);
+    void checkSourceIndexRemoval(const QModelIndex &sourceIndex);
 };
 
 class CollectionSearchJob : public KJob
@@ -172,7 +178,7 @@ public:
     void setCollectionReferenced(bool enabled, const Akonadi::Collection &collection);
     void setCollectionEnabled(bool enabled, const Akonadi::Collection &collection);
     void setCollection(const Akonadi::Collection &collection, bool enabled, bool referenced);
-    void setPersonDisabled(const Person &person);
+    void setPersonEnabled(const Person &person, bool enabled);
 
 Q_SIGNALS:
     void searchIsActive(bool);
@@ -184,7 +190,6 @@ private Q_SLOTS:
     void onCollectionsFound(KJob *job);
     void onPersonsFound(KJob *job);
     void onPersonEnabled(bool enabled, const Person &person);
-    // void onPersonCollectionsReceived(Akonadi::Collection::List);
     void onPersonCollectionsFetched(KJob *job);
     void onCollectionEnabled(bool enabled, const Akonadi::Collection &collection);
 


commit 50bd5bc4435428c2ab39d5e41470705103875ea3
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date:   Fri Sep 12 16:58:44 2014 +0200

    ReparentingModel: allow the nodemanager to add/remove nodes based on sourcenodes.

diff --git a/korganizer/views/collectionview/reparentingmodel.cpp b/korganizer/views/collectionview/reparentingmodel.cpp
index 9ab2611..f402658 100644
--- a/korganizer/views/collectionview/reparentingmodel.cpp
+++ b/korganizer/views/collectionview/reparentingmodel.cpp
@@ -224,6 +224,16 @@ bool ReparentingModel::validateNode(const Node *node) const
 
 void ReparentingModel::addNode(const ReparentingModel::Node::Ptr& node)
 {
+    //We have to make this check before issuing the async method,
+    //otherwise we run into the problem that while a node is being removed,
+    //the async request could be triggered (due to a changed signal),
+    //resulting in the node getting readded immediately after it had been removed.
+    Q_FOREACH(const ReparentingModel::Node::Ptr &existing, mProxyNodes) {
+        if (*existing == *node) {
+            // kDebug() << "node is already existing";
+            return;
+        }
+    }
     qRegisterMetaType<Node::Ptr>("Node::Ptr");
     QMetaObject::invokeMethod(this, "doAddNode", Qt::QueuedConnection, QGenericReturnArgument(), Q_ARG(Node::Ptr, node));
 }
@@ -245,18 +255,20 @@ void ReparentingModel::doAddNode(const Node::Ptr &node)
 
 void ReparentingModel::removeNode(const ReparentingModel::Node& node)
 {
-    beginResetModel();
     for (int i = 0; i < mProxyNodes.size(); i++) {
         if (*mProxyNodes.at(i) == node) {
+            //TODO: this does not yet take care of un-reparenting reparented nodes.
+            const Node &n = *mProxyNodes.at(i);
+            Node *parentNode = n.parent;
+            beginRemoveRows(index(parentNode), n.row(), n.row());
+            parentNode->children.remove(n.row()); //deletes node
             mProxyNodes.remove(i);
+            endRemoveRows();
             break;
         }
     }
-    rebuildAll();
-    endResetModel();
 }
 
-
 void ReparentingModel::setNodes(const QList<Node::Ptr> &nodes)
 {
     Q_FOREACH(const ReparentingModel::Node::Ptr &node, nodes) {
@@ -462,6 +474,10 @@ void ReparentingModel::onSourceRowsAboutToBeRemoved(QModelIndex parent, int star
         parentNode->children.remove(targetRow); //deletes node
         endRemoveRows();
     }
+    //Allows the node manager to remove nodes that are no longer relevant
+    for (int row = start; row <= end; row++) {
+        mNodeManager->checkSourceIndexRemoval(sourceModel()->index(row, 0, parent));
+    }
 }
 
 void ReparentingModel::onSourceRowsRemoved(QModelIndex parent, int start, int end)
@@ -559,10 +575,10 @@ QModelIndex ReparentingModel::index(int row, int column, const QModelIndex& pare
         parentNode = &mRootNode;
     }
     //At least QAbstractItemView expects that we deal with this properly (see rowsAboutToBeRemoved "find the next visible and enabled item")
-    if (parentNode->children.size() <= row) {
+    //Also QAbstractItemModel::match does all kinds of weird shit including passing row=-1
+    if (parentNode->children.size() <= row || row < 0) {
         return QModelIndex();
     }
-    Q_ASSERT(parentNode->children.size() > row);
     Node *node = parentNode->children.at(row).data();
     Q_ASSERT(validateNode(node));
     return createIndex(row, column, node);
@@ -593,7 +609,7 @@ ReparentingModel::Node *ReparentingModel::getSourceNode(const QModelIndex &sourc
 
 QModelIndex ReparentingModel::mapFromSource(const QModelIndex& sourceIndex) const
 {
-    kDebug() << sourceIndex << sourceIndex.data().toString();
+    // kDebug() << sourceIndex << sourceIndex.data().toString();
     if (!sourceIndex.isValid()) {
         return QModelIndex();
     }
@@ -716,7 +732,6 @@ QModelIndex ReparentingModel::index(Node *node) const
 QModelIndex ReparentingModel::parent(const QModelIndex& child) const
 {
     // kDebug() << child << child.data().toString();
-    Q_ASSERT(child.isValid());
     if (!child.isValid()) {
         return QModelIndex();
     }
diff --git a/korganizer/views/collectionview/reparentingmodel.h b/korganizer/views/collectionview/reparentingmodel.h
index 43556e8..ebcd8f6 100644
--- a/korganizer/views/collectionview/reparentingmodel.h
+++ b/korganizer/views/collectionview/reparentingmodel.h
@@ -78,6 +78,7 @@ public:
 
         //Allows the implementation to create proxy nodes as necessary
         virtual void checkSourceIndex(const QModelIndex &sourceIndex){};
+        virtual void checkSourceIndexRemoval(const QModelIndex &sourceIndex){};
     };
 
 public:
diff --git a/korganizer/views/collectionview/tests/reparentingmodeltest.cpp b/korganizer/views/collectionview/tests/reparentingmodeltest.cpp
index c28abd3..818b03d 100644
--- a/korganizer/views/collectionview/tests/reparentingmodeltest.cpp
+++ b/korganizer/views/collectionview/tests/reparentingmodeltest.cpp
@@ -146,6 +146,8 @@ private Q_SLOTS:
     void testSourceDataChanged();
     void testSourceLayoutChanged();
     void testInvalidLayoutChanged();
+    void testAddRemoveNodeByNodeManager();
+    void testRemoveNodeByNodeManagerWithDataChanged();
 };
 
 void ReparentingModelTest::testPopulation()
@@ -269,7 +271,7 @@ void ReparentingModelTest::testAddRemoveProxyNode()
     QVERIFY(getIndex("row1", reparentingModel).isValid());
     QVERIFY(!getIndex("proxy1", reparentingModel).isValid());
 
-    QCOMPARE(spy.mSignals, QStringList() << QLatin1String("modelReset") << QLatin1String("modelReset"));
+    QCOMPARE(spy.mSignals, QStringList() << QLatin1String("modelReset") << QLatin1String("rowsRemoved"));
 }
 
 void ReparentingModelTest::testDeduplicate()
@@ -561,6 +563,72 @@ void ReparentingModelTest::testInvalidLayoutChanged()
     QVERIFY(!persistentIndex.isValid());
 }
 
+class DummyNodeManager : public ReparentingModel::NodeManager
+{
+public:
+    DummyNodeManager(ReparentingModel &m) : ReparentingModel::NodeManager(m){};
+private:
+    void checkSourceIndex(const QModelIndex &sourceIndex) {
+        if (sourceIndex.data().toString() == QLatin1String("personfolder")) {
+            model.addNode(ReparentingModel::Node::Ptr(new DummyNode(model, QLatin1String("personnode"))));
+        }
+    }
+
+    void checkSourceIndexRemoval(const QModelIndex &sourceIndex) {
+        if (sourceIndex.data().toString() == QLatin1String("personfolder")) {
+            model.removeNode(DummyNode(model, QLatin1String("personnode")));
+        }
+    }
+};
+
+void ReparentingModelTest::testAddRemoveNodeByNodeManager()
+{
+    QStandardItemModel sourceModel;
+    sourceModel.appendRow(new QStandardItem(QLatin1String("personfolder")));
+    ReparentingModel reparentingModel;
+    reparentingModel.setNodeManager(ReparentingModel::NodeManager::Ptr(new DummyNodeManager(reparentingModel)));
+    reparentingModel.setSourceModel(&sourceModel);
+
+    QTest::qWait(0);
+
+    QVERIFY(getIndex("personnode", reparentingModel).isValid());
+    QVERIFY(getIndex("personfolder", reparentingModel).isValid());
+
+    sourceModel.removeRows(0, 1, QModelIndex());
+
+    QTest::qWait(0);
+    QVERIFY(!getIndex("personnode", reparentingModel).isValid());
+    QVERIFY(!getIndex("personfolder", reparentingModel).isValid());
+}
+
+/*
+ * This tests a special case that is caused by the delayed doAddNode call,
+ * causing a removed node to be readded immediately if it's removed while
+ * a doAddNode call is pending (that can be triggered by dataChanged).
+ */
+void ReparentingModelTest::testRemoveNodeByNodeManagerWithDataChanged()
+{
+    QStandardItemModel sourceModel;
+    QStandardItem *item = new QStandardItem(QLatin1String("personfolder"));
+    sourceModel.appendRow(item);
+    ReparentingModel reparentingModel;
+    reparentingModel.setNodeManager(ReparentingModel::NodeManager::Ptr(new DummyNodeManager(reparentingModel)));
+    reparentingModel.setSourceModel(&sourceModel);
+
+    QTest::qWait(0);
+
+    QVERIFY(getIndex("personnode", reparentingModel).isValid());
+    QVERIFY(getIndex("personfolder", reparentingModel).isValid());
+
+    //Trigger data changed
+    item->setStatusTip(QLatin1String("sldkfjlfsj"));
+    sourceModel.removeRows(0, 1, QModelIndex());
+
+    QTest::qWait(0);
+    QVERIFY(!getIndex("personnode", reparentingModel).isValid());
+    QVERIFY(!getIndex("personfolder", reparentingModel).isValid());
+}
+
 
 QTEST_MAIN(ReparentingModelTest)
 




More information about the commits mailing list