[Kde-pim] Review Request: 1/4 Proper implementation of virtual collections

[Volker Krause]

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

Ship it!


Looks good overall, and can go in with the changes mentioned below.

However, I think this misses one more thing to actually work: Considering the virtual flag during fetching. See itemqueryhelper.cpp, that currently only checks for the resource being virtual, not the collection.


server/src/handler/create.cpp
<http://git.reviewboard.kde.org/r/106546/#comment15637>

    The C-style cast ("(bool)") shouldn't be necessary here. If it is, better use != 0 here.



server/src/handler/modify.cpp
<http://git.reviewboard.kde.org/r/106546/#comment15638>

    This entire block looks useless, shouldn't we just not send a VIRTUAL attribute in MODIFY? If at all, I'd just throw an unconditional error in this case.



server/src/storage/collectionqueryhelper.h
<http://git.reviewboard.kde.org/r/106546/#comment15640>

    With the modify change removed, this method also becomes unnecessary.



server/src/storage/dbupdate.xml
<http://git.reviewboard.kde.org/r/106546/#comment15639>

    ResourceTable already has a "isVirtual" column, might be better to use that for updating.


- Volker Krause


On Sept. 23, 2012, 9:16 p.m., Dan Vratil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106546/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2012, 9:16 p.m.)
> 
> 
> Review request for Akonadi.
> 
> 
> Description
> -------
> 
> Akonadi-part of proper implementation of virtual collections. 
> 
> To make things easier, virtual collections can only be made on creation. You cannot convert non-virtual collection to virtual or virtual to non-virtual. I also added a fail if you try to link items to non-virtual collections (because that is not supposed to work). There are no restrictions on parenting - virtual collection can be parent of a non-virtual collection and vice versa.
> 
> The flag is stored in isVirtual column in CollectionTable in Akonadi DB Scheme. Using dbupdate.xml the flag is automatically set to 1 for all collections from akonadi_search_resource and akonadi_nepomuktag_resource during update. These resources are the only that can have virtual collections atm (because it's hardcoded), so there should not be any other problems with the update.
> 
> 
> Diffs
> -----
> 
>   server/src/handler/create.cpp 1a8be48 
>   server/src/handler/link.cpp 5cf6573 
>   server/src/handler/list.h 7f4c108 
>   server/src/handler/modify.cpp 52a5b86 
>   server/src/handler/searchpersistent.cpp 5b78f54 
>   server/src/handlerhelper.cpp 8631aa7 
>   server/src/storage/akonadidb.xml 501dfd8 
>   server/src/storage/collectionqueryhelper.h 49fdb40 
>   server/src/storage/collectionqueryhelper.cpp e6edf6d 
>   server/src/storage/dbupdate.xml f9952fe 
> 
> Diff: http://git.reviewboard.kde.org/r/106546/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vratil
> 
>

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