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 703194 - Custom alarm message is REMINDER
Custom alarm message is REMINDER
Status: RESOLVED FIXED
Product: evolution-ews
Classification: Other
Component: Calendar
3.8.x
Other Linux
: Normal normal
: ---
Assigned To: Evolution EWS maintainer(s)
Evolution EWS maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-06-27 14:31 UTC by Milan Crha
Modified: 2013-07-02 02:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
EDS: Bug #703194 - Custom alarm message is REMINDER (1.01 KB, patch)
2013-07-01 01:56 UTC, Fabiano Fidêncio
committed Details | Review
Evo: Bug #703194 - Custom alarm message is REMINDER (1.27 KB, patch)
2013-07-01 01:57 UTC, Fabiano Fidêncio
committed Details | Review
Bug #703194 - Custom alarm message is REMINDER (2.02 KB, patch)
2013-07-01 01:57 UTC, Fabiano Fidêncio
needs-work Details | Review
Bug #703194 - Custom alarm message is REMINDER (2.05 KB, patch)
2013-07-01 10:01 UTC, Fabiano Fidêncio
committed Details | Review

Description Milan Crha 2013-06-27 14:31:11 UTC
Since bug #386113 evolution shows custom alarm messages, instead of component summary in reminders. The thing is that evolution-ews' events has set this custom message as REMINDER (in capitals for me). I didn't check any details why and what, but I suppose that evolution-ews doesn't set the custom message, thus the server fills it on its own, with the REMINDER text. Thus I see two bugs:
a) data loss - if user fills a custom alarm message for a popup, then it should
   be preserved (it seems to be lost)
b) if the server does set the default text, then evolution-ews should construct
   the component alarm with a summary text, rather than REMINDER (this can be
   tricky due to localization)

and the last thing to check:
c) what if the event is filled in OWA, can we properly recognize it
Comment 1 Fabiano Fidêncio 2013-06-28 01:40:50 UTC
(In reply to comment #0)
> Since bug #386113 evolution shows custom alarm messages, instead of component
> summary in reminders. The thing is that evolution-ews' events has set this
> custom message as REMINDER (in capitals for me). I didn't check any details why
> and what, but I suppose that evolution-ews doesn't set the custom message, thus
> the server fills it on its own, with the REMINDER text.

You're right, Milan.

> Thus I see two bugs:
> a) data loss - if user fills a custom alarm message for a popup, then it should
>    be preserved (it seems to be lost)

Actually, it's never set in the Server. And when we update the event, the "non-value" is replaced by "REMINDER".

> b) if the server does set the default text, then evolution-ews should construct
>    the component alarm with a summary text, rather than REMINDER (this can be
>    tricky due to localization)

Apparently server does. The interesting part is that there is no specific option for this in the OWA interface. We can easily do a ugly workaround and guarantee the i18n of the "REMINDER" word, in the worst case. 

> 
> and the last thing to check:
> c) what if the event is filled in OWA, can we properly recognize it

I've been looking for all properties that I think we are able to receive from the Server and I didn't find which one is setting this "REMINDER" :-(. If we find it, we could easily set whatever we want without problems. Some tip?
Comment 2 Milan Crha 2013-06-28 09:41:17 UTC
Ah, I see, there is basically no API for this on the EWS protocol side. In that case:
a) disable (or rather hide) the custom message entry/widgets in alarm dialog
   for ews
b) on event read, update alarms descriptions to event's summary, always,
   unconditionally, on the ews calendar backend side
Comment 3 Fabiano Fidêncio 2013-07-01 01:56:46 UTC
Created attachment 248109 [details] [review]
EDS: Bug #703194 - Custom alarm message is REMINDER
Comment 4 Fabiano Fidêncio 2013-07-01 01:57:11 UTC
Created attachment 248110 [details] [review]
Evo: Bug #703194 - Custom alarm message is REMINDER
Comment 5 Fabiano Fidêncio 2013-07-01 01:57:28 UTC
Created attachment 248111 [details] [review]
Bug #703194 - Custom alarm message is REMINDER
Comment 6 Milan Crha 2013-07-01 08:33:57 UTC
Review of attachment 248109 [details] [review]:

Looks fine, please commit.
Comment 7 Milan Crha 2013-07-01 08:34:41 UTC
Review of attachment 248110 [details] [review]:

Looks fine, please commit.
Comment 8 Milan Crha 2013-07-01 08:40:58 UTC
Review of attachment 248111 [details] [review]:

Please address these issues first.

::: src/calendar/e-cal-backend-ews.c
@@ +3502,3 @@
+			for (l = alarm_uids; l != NULL; l = l->next) {
+				ECalComponentAlarm *alarm;
+				ECalComponentText *text = g_new0 (ECalComponentText, 1);

No need to allocate the structure on heap, please see other similar places of its usage.

@@ +3508,3 @@
+				e_cal_component_alarm_set_description (alarm, text);
+				g_free (text);
+			}

All the above code has various memory leaks. Please check respective e_cal_component_... functions' documentation what to do with returned memory.
Comment 9 Fabiano Fidêncio 2013-07-01 10:01:31 UTC
Created attachment 248125 [details] [review]
Bug #703194 - Custom alarm message is REMINDER
Comment 10 Milan Crha 2013-07-01 16:15:44 UTC
Review of attachment 248125 [details] [review]:

Untested, but looks like it. Please fix the coding style thing and commit to master and stable. Thanks.

::: src/calendar/e-cal-backend-ews.c
@@ +3510,3 @@
+				e_cal_component_alarm_free (alarm);
+			}
+			cal_obj_uid_list_free(alarm_uids);

coding style: missing white space
Comment 11 Fabiano Fidêncio 2013-07-02 02:58:17 UTC
Pushed to master (> 3.9.3) and stable (> 3.8.3)