[Kde-pim] Review Request: Fix saving and loading of suspended reminders in KOrganizer

[Frank Roscher]

> On Dec. 1, 2012, 11:45 p.m., Sergio Luis Martins wrote:
> > Awesome. Thanks for working on this, since noone is looking at korgac currently.
> > 
> > Could you have a quick look these bugs and see if any one should be closed due to your patch ?
> > 
> > https://bugs.kde.org/buglist.cgi?list_id=314786&short_desc=reminders%20reminder%20alarm&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&short_desc_type=anywords&product=korganizer
> > 
> > https://bugs.kde.org/buglist.cgi?list_id=314788&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&product=korgac
> > 
> >

Thank you for those lists, I had searched for open bugs at some point but somehow didn't find any corresponding to the problem.
Bugs 280321 is fixed by this patch.
Numbers 146336, 180413 and 302865 are improved but can't be closed. One of the reports contains two bugs, only one of which is fixed. The other reports are fixed for suspended reminders but still need work for non-suspended ones (so if I find the time I'll have a look at that after this patch is committed). I'll CCBUG these in a commit.
In addition, these two bugs are questionable:
https://bugs.kde.org/show_bug.cgi?id=199980
https://bugs.kde.org/show_bug.cgi?id=200986
I guess I would close them NEEDSINFO together with a comment suggesting retesting with KDE 4.10.


- Frank


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


On Dec. 1, 2012, 4:01 p.m., Frank Roscher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107546/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2012, 4:01 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> Shortly after fixing reminders that trigger while korgac is not running, I had to find out that suspended reminders work differently and are buggy, too.
> This patch should fix the following two (related) issues:
> 1. Suspended reminders are not loaded correctly at korgac startup because collections haven't been populated.
> 2. Suspended reminders are forgotten if they don't get triggered between two shutdowns of korgac. This happens because the code only saves them to a config file when they are explicitly (re-)suspended, but removes them from the file on startup.
> 
> The patch fixes this by changing the following:
> - It introduces a method deferredInit that gets called when the collections have been populated. The code for loading suspended reminders from the config file is moved here from the constructor. The first check for reminders is triggered from here as well.
> - The saved suspended reminders are not deleted from the config file anymore on startup. Instead, they are deleted from there only when they are dismissed by the user after having triggered. In order to adjust to this behaviour, slotSave of the alarm dialog does not append to the reminder list in the config file anymore, but replaces it instead (because the dialog is supposed to know about all suspended reminders anyway). 
> - A new method collectionsAvailable is introduced because we need to check the collection population status in two different places now; this is not new code. Two kDebug statements were removed here due to reduced usefulness in this more general context.
> 
> The patch is pretty straightforward and should be a clear improvement, but it is possible I'm missing some code that ties into this and needs to be adjusted as well.
> 
> 
> Diffs
> -----
> 
>   korganizer/korgac/alarmdialog.cpp d3b41aa 
>   korganizer/korgac/koalarmclient.h 9ec5316 
>   korganizer/korgac/koalarmclient.cpp 81480bd 
> 
> Diff: http://git.reviewboard.kde.org/r/107546/diff/
> 
> 
> Testing
> -------
> 
> - Suspending reminders across shutdown of korgac and akonadi
> - Suspended reminders that trigger again only after _two_ shutdowns of korgac and akonadi
> - Resuspending such reminders
> - Dismissal of reminders in all three scenarios described above.
> - Checked that config file does not get filled up with dead entries
> 
> 
> Thanks,
> 
> Frank Roscher
> 
>

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