[Kde-pim] Review Request: CalendarResources::endChange() only saves when an incidence was added or modified, not when deleted

[Sergio Martins]

> On 2009-02-15 14:17:29, Volker Krause wrote:
> > Good catch!
> 
> Kevin Krammer wrote:
>     I am kind of missing (or not seeing) the place where the new variable is initialized
> 
> Sergio Martins wrote:
>     It is set to false in beginChange(), I was going to put it only in the constructor, but what happens if deleteEvent() returns an error? Will endChange() still get called (and clear the variable)?.
> 
> Kevin Krammer wrote:
>     In beginChange() is definitely necessary, but I think it is always good to initialize members to a known value.
>     Actually, at work we have a static code analyzer that complains about that. Calls it a Severe Violation :)

yeah, well, actually I commited with both the initialization in the constructor and in beginChange() :) just forgot to update the attached diff :)

Thanks


- Sergio


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


On 2009-02-15 14:01:58, Sergio Martins wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/96/
> -----------------------------------------------------------
> 
> (Updated 2009-02-15 14:01:58)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> When an incidence is added or modified CalendarResources::endChange() saves it, but when one is deleted it doesn't because it exits immediately with an error.
> 
>   ResourceCalendar *r = resource( incidence );
>   if ( !r ) {
>     return false;
>   }
> 
> It doesn't know which resource the incidence belongs to because deleteEvent() already removed it from the mapping.
> 
> Not only it doesn't save, it also doesn't clear the lock created by beginChange(), so the next change won't be saved either, even if it's a move or add, (it will only save when KOrganizer exits).
> 
> This fix will allow the "save after 5 second timeout" workaround to be removed from ResourceAkonadi because now save is called after a delete.
> 
> This review is related to Kevin's http://reviewboard.kde.org/r/94/ and probably should go in the same commit.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepimlibs/kcal/calendarresources.h 926606 
>   trunk/KDE/kdepimlibs/kcal/calendarresources.cpp 926606 
> 
> Diff: http://reviewboard.kde.org/r/96/diff
> 
> 
> Testing
> -------
> 
> Added, modified, deleted incidences.
> Unchecked/checked the resource to see if the changes were permanent.
> 
> 
> Thanks,
> 
> Sergio
> 
>

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