After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 744927 - Don't allow removing event notifications
Don't allow removing event notifications
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: calendar
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on: 746027
Blocks:
 
 
Reported: 2015-02-22 04:03 UTC by Giovanni Campagna
Modified: 2015-03-13 15:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
calendar: Minor cleanup (2.71 KB, patch)
2015-03-12 16:00 UTC, Florian Müllner
committed Details | Review
calendar: Factor out a proper EventMessage class (4.28 KB, patch)
2015-03-12 16:00 UTC, Florian Müllner
committed Details | Review
calendar: Only allow closing events on the current day (887 bytes, patch)
2015-03-12 16:00 UTC, Florian Müllner
committed Details | Review
calendar-server: Give each event an unambiguous ID (3.22 KB, patch)
2015-03-13 14:53 UTC, Florian Müllner
committed Details | Review
calendar: Permanently hide dismissed events (3.45 KB, patch)
2015-03-13 14:53 UTC, Florian Müllner
committed Details | Review

Description Giovanni Campagna 2015-02-22 04:03:43 UTC
Because in practice they are not removed, they appear again if you click another date and then the one with the event.
Comment 1 Florian Müllner 2015-02-22 04:22:15 UTC
That's plan B - according to the design, events should be hidden in the shell when removed.
Comment 2 Allan Day 2015-02-23 11:56:26 UTC
The design anticipates two behaviours here:

 * Events in the "today" view (which also shows notifications) act as reminders - they are shown once and can be dismissed.

 * When browsing other days using the calendar, events should be permanent (since, as Giovanni points out, they do not get removed) - you shouldn't be able to dismiss them, either individually or as a group.
Comment 3 Florian Müllner 2015-02-23 12:58:13 UTC
(In reply to Allan Day from comment #2)
> The design anticipates two behaviours here:
> 
>  * Events in the "today" view (which also shows notifications) act as
> reminders - they are shown once and can be dismissed.

What does "dismissed" mean here? The underlying calendar does not make any distinction between "events" and "reminders", so the behavior would still just be as described in comment #0 - the list is cleared, but will reappear when the user returns to the main view.
Comment 4 Allan Day 2015-02-23 13:43:27 UTC
(In reply to Florian Müllner from comment #3)
...
> >  * Events in the "today" view (which also shows notifications) act as
> > reminders - they are shown once and can be dismissed.

To clarify, "shown once" wasn't really necessary in that sentence. The can be viewed multiple times, but are hidden once dismissed.

> What does "dismissed" mean here?

"Isn't shown in the shell UI again." ?
Comment 5 Florian Müllner 2015-03-12 15:59:57 UTC
(In reply to Allan Day from comment #2)
> The design anticipates two behaviours here:
> 
>  * Events in the "today" view (which also shows notifications) act as
> reminders - they are shown once and can be dismissed.

I have working patches for this (hide events permanently from the shell) locally, but am still waiting for some feedback/guidance for the calendar bits. However ...


>  * When browsing other days using the calendar, events should be permanent
> (since, as Giovanni points out, they do not get removed) - you shouldn't be
> able to dismiss them, either individually or as a group.

... this part is ready and independent from the rest, so attaching patches now (on top of bug 746027, which touches some related message list oddities)
Comment 6 Florian Müllner 2015-03-12 16:00:42 UTC
Created attachment 299211 [details] [review]
calendar: Minor cleanup

As the design calls for slightly different behavior for the current
day, move the _isToday() function out of MessageListSection to have
it available elsewhere as well ...
Comment 7 Florian Müllner 2015-03-12 16:00:50 UTC
Created attachment 299212 [details] [review]
calendar: Factor out a proper EventMessage class

While messages in the EventsSection are currently simple enough to
use the generic Message baseclass, the design calls for events to
only be dismissable on the current day. We will need a subclass to
implement this behavior cleanly, so add one.
Comment 8 Florian Müllner 2015-03-12 16:00:58 UTC
Created attachment 299213 [details] [review]
calendar: Only allow closing events on the current day

The design calls for differentiating between dismissable reminders
and permanent events, based on whether the selected date is "today"
or some other day.
Comment 9 Carlos Soriano 2015-03-12 16:11:36 UTC
Review of attachment 299211 [details] [review]:

OK
Comment 10 Carlos Soriano 2015-03-12 16:24:34 UTC
Review of attachment 299212 [details] [review]:

OK. You will extend this one for the "hide if today" case right?
Comment 11 Florian Müllner 2015-03-12 16:25:45 UTC
(In reply to Carlos Soriano from comment #10)
> OK. You will extend this one for the "hide if today" case right?

Yeah, see next patch :-)
Comment 12 Carlos Soriano 2015-03-12 16:26:35 UTC
Review of attachment 299213 [details] [review]:

AH! yes =)
Comment 13 Florian Müllner 2015-03-12 16:33:41 UTC
Attachment 299211 [details] pushed as d48d787 - calendar: Minor cleanup
Attachment 299212 [details] pushed as d1efc27 - calendar: Factor out a proper EventMessage class
Attachment 299213 [details] pushed as 01b51cd - calendar: Only allow closing events on the current day
Comment 14 Florian Müllner 2015-03-13 14:53:48 UTC
Created attachment 299328 [details] [review]
calendar-server: Give each event an unambiguous ID

Each event returned by GetEvents includes the (currently unused)
UID, which is not always enough to unambiguously identify an event
(different calendar sources, recurring events, ...).
As we will start using the property to record events that have been
dismissed and should be persistently hidden from the calendar, change
it to a truly unique ID.
Comment 15 Florian Müllner 2015-03-13 14:53:54 UTC
Created attachment 299329 [details] [review]
calendar: Permanently hide dismissed events

Currently dismissed events will simply reappear when browsing
back and forth between dates, which is clearly broken. Instead,
hide events that have been dismissed permanently. For now, we
simply store a list of ignored IDs ourselves, until we get API
in evolution-data-server to reliably store custom per-event
properties.
Comment 16 Carlos Soriano 2015-03-13 15:23:43 UTC
Review of attachment 299328 [details] [review]:

OK
Comment 17 Carlos Soriano 2015-03-13 15:23:52 UTC
Review of attachment 299329 [details] [review]:

OK
Comment 18 Florian Müllner 2015-03-13 15:25:28 UTC
Attachment 299328 [details] pushed as 8aeebbb - calendar-server: Give each event an unambiguous ID
Attachment 299329 [details] pushed as 777616d - calendar: Permanently hide dismissed events