[Kde-pim] Review Request: Enable determination of ETM populated status for collections and items

[David Jarvie]

> On July 20, 2012, 3:57 p.m., Stephen Kelly wrote:
> > Sorry for the long delay in reviewing!

I've taken some time to respond too, having been away on holiday.


> On July 20, 2012, 3:57 p.m., Stephen Kelly wrote:
> > akonadi/entitytreemodel.h, line 531
> > <http://git.reviewboard.kde.org/r/105457/diff/2/?file=71465#file71465line531>
> >
> >     Can you do without this method?
> >     
> >     New collections can come in at any time, so the 'initialization' being done doesn't really mean anything.

Knowing when the collection tree has been fetched is essential. Otherwise, testing for all collections having been populated at startup will always return true since there aren't any collections. Some KAlarm startup processing doesn't work unless it waits until the model has populated its collection tree.


> On July 20, 2012, 3:57 p.m., Stephen Kelly wrote:
> > akonadi/entitytreemodel.h, line 597
> > <http://git.reviewboard.kde.org/r/105457/diff/2/?file=71465#file71465line597>
> >
> >     Again, because the first time shouldn't really be significant, I don't like this one. There might be no collections at T = 0, and then 'new' collections would be added, which wouldn't be emitted in this signal, but should have the same effect on your application.

See the comment above. New collections being added later are fine, but I need to know when existing collections have been added.


> On July 20, 2012, 3:57 p.m., Stephen Kelly wrote:
> > akonadi/entitytreemodel.h, line 606
> > <http://git.reviewboard.kde.org/r/105457/diff/2/?file=71465#file71465line606>
> >
> >     Please use the dataChanged signal instead, and check the IsPopulatedRole role.

The slot to check this will be called a lot, so it seems more efficient to do it my original way. But I'll amend it if that's what you want.


> On July 20, 2012, 3:57 p.m., Stephen Kelly wrote:
> > akonadi/entitytreemodel.h, line 540
> > <http://git.reviewboard.kde.org/r/105457/diff/2/?file=71465#file71465line540>
> >
> >     I'd prefer you use
> >     
> >     model->data(idx, EntityTreeModel::IsPopulatedRole);
> >     
> >     and add the IsPopulatedRole as an enum value to the Roles enum.

OK


- David


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


On July 6, 2012, 12:18 p.m., David Jarvie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105457/
> -----------------------------------------------------------
> 
> (Updated July 6, 2012, 12:18 p.m.)
> 
> 
> Review request for KDEPIM-Libraries and Stephen Kelly.
> 
> 
> Description
> -------
> 
> Provide methods to determine whether the collection tree has been fetched at model initialisation, and whether individual collections have been populated. Also provide signals when the collection tree has been fetched, and when each collection has been populated.
> 
> This will enable applications to wait until the model is fully populated before doing things which rely on this.
> 
> 
> Diffs
> -----
> 
>   akonadi/entitytreemodel.h 3de59bc 
>   akonadi/entitytreemodel.cpp bd031d8 
>   akonadi/entitytreemodel_p.h 266cea9 
>   akonadi/entitytreemodel_p.cpp 59a79db 
> 
> Diff: http://git.reviewboard.kde.org/r/105457/diff/
> 
> 
> Testing
> -------
> 
> Used in KAlarm. All the signals are emitted, and the methods work - checked both by behaviour and by temporary debug statements.
> 
> 
> Thanks,
> 
> David Jarvie
> 
>

_______________________________________________
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/