[Kde-pim] Re: Review Request: Include collections wich are watched in the ChangeRecorder in the EntityTreeModel

[Christian Mollekopf]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6330/
-----------------------------------------------------------

(Updated Jan. 23, 2011, 10:34 p.m.)


Review request for KDE PIM and Stephen Kelly.


Changes
-------

I went over the patch again and I believe it should be implemented appropriately now. I made the following changes:

-support for multiple monitored collections: 
Since monitored collections don't have to be toplevel, they are fetched after the toplevel collections have been fetched. Parents are fetched recursively.
As soon as more than one collection is monitored, Collection::root is taken as root of the node tree. While here could be cases where we could take another root, it would be more difficult to implement, and I don't see a big advantage.
-reset the model when the monitored resources change
-fetch toplevel items from m_rootCollection since it is not always collection::root
-fetch the items of the m_rootCollection right away when necessary

I also added some docs to some functions I touched, I'm not sure if it is good to have implementation details in the docs, but since it is a private class I don't think it is a big issue. 
I had a hard time finding my ways in the sourcode and would have been glad for some pointer =)

The model reacts fine to both adding and removing watched resources/collections. 
I also tried it with lazy population, which works fine as well.


Summary
-------

With this patch, the model includes also collections which are monitored in the monitor with ChangeRecorder::setCollectionMonitored() the same way it includes also watched resources.

Since I don't fully understand the whole code I just stuck to how the resource monitoring is implemented.

I had to add the fetching of the items in EntityTreeModelPrivate::topLevelCollectionsFetched(), otherwise only the items in subcollections would have been fetched. 
Since this should also have been a problem when watching resources, I reckon this was a bug.


Diffs (updated)
-----

  /trunk/KDE/kdepimlibs/akonadi/entitytreemodel.h 1215020 
  /trunk/KDE/kdepimlibs/akonadi/entitytreemodel_p.h 1215020 
  /trunk/KDE/kdepimlibs/akonadi/entitytreemodel_p.cpp 1215020 

Diff: http://svn.reviewboard.kde.org/r/6330/diff


Testing
-------

I'm using it in my own app, where I populate the model with items from a Akonotes resource collection and a Kcal resource collection.
It works, and I couldn't find any sideffects.


Thanks,

Christian

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/