GNOME Bugzilla – Bug 748226
Dismissing appointments causes all reoccurring appointments to vanish
Last modified: 2016-12-11 10:27:33 UTC
DESCRIPTION: After dismissing appointments that have occurred in the past and are reoccurring appointments the panel removes all mention of them in the future or the past. EXPECTED BEHAVIOR: Past appointments either hide themselves, or crossing off an appointment today would not remove future appointments of the same type. Such in the way one could see what is upcoming in the day / week ahead at a quick glance or manually cross off appointments are they are finished through the day. Nice work on the new panel!
I can confirm this behavior. For now I found a workaround to bring back all dismissed events so their repetitions are shown again: - rm ~/.local/share/gnome-shell/ignored_events - reload gnome-shell via Alt+F2 r
(In reply to bevan from comment #1) > I can confirm this behavior. For now I found a workaround to bring back all > dismissed events so their repetitions are shown again: > > - rm ~/.local/share/gnome-shell/ignored_events > - reload gnome-shell via Alt+F2 r Thanks! Good for a temporary fix.
*** Bug 750408 has been marked as a duplicate of this bug. ***
I can confirm this on 3.18.2. Perhaps it should not be possible to ignore recurring events at all? If there must be some kind of "dismiss event" action, there really needs to be a way to bring an event back as well. I've accidentally clicked that close button far too many times...
Perhaps the file should get auto-cleaned every new day (or when the event is over)? This will reinstate ignored events when they reoccur. Not doing so may be very confusing to the users.
*** Bug 761153 has been marked as a duplicate of this bug. ***
Created attachment 331035 [details] [review] calendar-server: Minor optimization We use the same query string for all sources, so no need to allocate/free it on each loop iteration.
Created attachment 331036 [details] [review] calendar-server: Fetch default zone from client We are already setting the default zone on the client, no need to pass it around.
Created attachment 331037 [details] [review] calendar-server: Get recurrence ID from occurrences We use the triplet of source ID, UID and recurrence ID to create an ID to unambiguously identify an event, which we use to implement hiding dismissed events from the calendar. However we currently try to fetch the recurrence ID from the objects returned by e_cal_client_get_object_list_sync(), which are always the primary events with no recurrence ID. Instead, we need a recurrence ID associated with each occurrence.
Created attachment 331038 [details] [review] calendar-server: Use the actual recurrence ID of events Instead of querying the client for a list of objects and using e_cal_recur_generate_instances() to get occurrences for each of them, we can use e_cal_client_generate_instances_sync() which combines the functionality of both functions. This doesn't only save us some lines of code (yay!), but also gives us access to the real recurrence ID of an event, so we can get rid of the hack of faking one.
Review of attachment 331035 [details] [review]: sure
Review of attachment 331036 [details] [review]: seems right, but then why even have app->zone at all ?
(In reply to Rui Matos from comment #12) > seems right, but then why even have app->zone at all ? Mostly due to oversight, though it still saves a tiny bit of work when the timezone didn't change between calls to app_update_timezone(). I'm not sure how expensive zone = (location == NULL) ? icaltimezone_get_utc_timezone () : icaltimezone_get_builtin_timezone (location); would be, however in case we keep the current code I spotted a leak we should fix in that case (app takes ownership of location after a timezone change, but the string is leaked when the timezone didn't change)
Review of attachment 331037 [details] [review]: not 100% sure about the semantic change but other than the leak below, seems right ::: src/calendar-server/gnome-shell-calendar-server.c @@ +419,3 @@ occurrence->start_time = occurrence_start; occurrence->end_time = occurrence_end; + occurrence->rid = g_strdup (rid); this is never free'd now @@ +919,3 @@ + a->source_id, + a->uid, + o->rid ? o->rid : ""); seems like ->rid can't ever be NULL
Review of attachment 331038 [details] [review]: looks good
Created attachment 331086 [details] [review] calendar-server: Get recurrence ID from occurrences (In reply to Rui Matos from comment #14) > + occurrence->rid = g_strdup (rid); > > this is never free'd now Ugh, thanks for the catch! > + o->rid ? o->rid : ""); > > seems like ->rid can't ever be NULL Right, not in this patch. It can with the next patch, but it's more correct to use ->rid directly here and change it in the other patch, so changed.
Review of attachment 331086 [details] [review]: looks good
(In reply to Florian Müllner from comment #13) > (In reply to Rui Matos from comment #12) > > seems right, but then why even have app->zone at all ? > > Mostly due to oversight, though it still saves a tiny bit of work when the > timezone didn't change between calls to app_update_timezone(). > > I'm not sure how expensive > > zone = (location == NULL) ? icaltimezone_get_utc_timezone () > : icaltimezone_get_builtin_timezone (location); > not sure I follow. We don't use app->zone at all anymore. Where would that code be needed? > would be, however in case we keep the current code I spotted a leak we > should fix in that case (app takes ownership of location after a timezone > change, but the string is leaked when the timezone didn't change) indeed
(In reply to Rui Matos from comment #18) > We don't use app->zone at all anymore. Where would that code be needed? We do, to set the default zone of each client: https://git.gnome.org/browse/gnome-shell/tree/src/calendar-server/gnome-shell-calendar-server.c#n657
(In reply to Florian Müllner from comment #19) > (In reply to Rui Matos from comment #18) > > We don't use app->zone at all anymore. Where would that code be needed? > > We do, to set the default zone of each client: > > https://git.gnome.org/browse/gnome-shell/tree/src/calendar-server/gnome- > shell-calendar-server.c#n657 Oh right, I was under the impression the patch removed that line but then it wouldn't make much sense. I'd say let it be then
Created attachment 331098 [details] [review] calendar-server: Fix a memory leak App will take ownership of the location string when the timezone changes, but not when there was no change - free the memory in that case. OK, so let's plug the leak spotted earlier.
Review of attachment 331098 [details] [review]: right
Review of attachment 331036 [details] [review]: ++
Attachment 331035 [details] pushed as 8c51f00 - calendar-server: Minor optimization Attachment 331036 [details] pushed as b2d79b6 - calendar-server: Fetch default zone from client Attachment 331038 [details] pushed as 7e0e224 - calendar-server: Use the actual recurrence ID of events Attachment 331086 [details] pushed as 35825cf - calendar-server: Get recurrence ID from occurrences Attachment 331098 [details] pushed as f5e1dc8 - calendar-server: Fix a memory leak
*** Bug 769640 has been marked as a duplicate of this bug. ***
As a reply to 769640, after dismissing yesterday's event I've realized that this thing happened again. It seems here is happening that event summary and notification gets mixed up resulting in hiding summary if notification gets dismissed. So by hiding notification summary is gone too, not really intuitive IMHO. To sum it up, if I want to see summary of past events I still need to clear past dismissed event notifications by removing/moving ~/.local/share/gnome-shell/ignored_events?
*** Bug 762698 has been marked as a duplicate of this bug. ***
This doesn't look fixed for me either. I keep having to periodically delete /home/hussam/.local/share/gnome-shell/ignored_events for recurring calendar events to show.