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 703899 - Moving a meeting does not ask for confirmation
Moving a meeting does not ask for confirmation
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: 703515 704369
 
 
Reported: 2013-07-09 19:20 UTC by Jan-Michael Brummer
Modified: 2013-07-17 11:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Bug #703899 - Moving a meeting does not ask for confirmation (6.68 KB, patch)
2013-07-12 12:53 UTC, Fabiano Fidêncio
none Details | Review
Bug #703899 - Moving a meeting does not ask for confirmation (19.28 KB, patch)
2013-07-15 14:43 UTC, Fabiano Fidêncio
reviewed Details | Review
Bug #703899 - Moving a meeting does not ask for confirmation (18.11 KB, patch)
2013-07-17 01:38 UTC, Fabiano Fidêncio
none Details | Review
Bug #703899 - Moving a meeting does not ask for confirmation (17.99 KB, patch)
2013-07-17 01:49 UTC, Fabiano Fidêncio
committed Details | Review

Description Jan-Michael Brummer 2013-07-09 19:20:13 UTC
Moving a meeting in the calendar view (accidentally) to another slot immediately sends out mails to all participants WITHOUT asking for confirmation. As the google mail account ask me for a confirmation and ews does not, this is a serious bug to me.
Comment 1 Milan Crha 2013-07-10 13:37:36 UTC
Thanks for a bug report. This is done on evolution's side, in its calendar UI. But only for the backends which do not deal with the invitations internally, on the server (the Exchange server sends change notifications, not evolution itself, I believe). The change on evolution's side might be to ask whether user wants to change the event time, and if not, then it should be left unchanged. Unfortunately, this is not the case when you click "Do not send" on other backends.
Comment 2 Jan-Michael Brummer 2013-07-10 16:27:01 UTC
I don't know if i got this right. When i move a EWS meeting to another time/day  slot evolution immediately sends out the update meeting to all participants (i can see the mail in the sent folder). Is it EWS or Evolution which must be fixed in this case? Who must display a confirmation dialog?
Comment 3 David Woodhouse 2013-07-10 16:34:32 UTC
You (or your cat/baby/etc.) can do that with just a stray click and drag, can't you? We should *definitely* be requiring some kind of confirmation for it, even if emails aren't being sent.
Comment 4 Fabiano Fidêncio 2013-07-10 16:41:49 UTC
(In reply to comment #3)

Yeah, yeah! I'm working on it, David.
We can ask for a confirmation before really move the event to another time/date.
Comment 5 Milan Crha 2013-07-11 11:13:13 UTC
(In reply to comment #2)
> I don't know if i got this right. When i move a EWS meeting to another time/day
>  slot evolution immediately sends out the update meeting to all participants (i
> can see the mail in the sent folder). Is it EWS or Evolution which must be
> fixed in this case? Who must display a confirmation dialog?

I'm pretty sure the invitation update is sent by the Exchange server, thus another related place is:
https://git.gnome.org/browse/evolution-ews/tree/src/calendar/e-cal-backend-ews.c?h=gnome-3-8#n2449

The thing is that there is currently no way to tell the backend that it should not send meeting update emails. There is no API for the modify_object method for it, thus the max to add some X-EVOLUTION property into the component which will indicate that the update should not be send, which will *each* backend strip off on modify and even evolution itself will make sure that it (always?) sets the flag or removes it, regardless the event is or is not a meeting (or only for backends which advertise being able to send meeting notifications on their own).
And do not forget of the end of the comment #1.

Now it gets complicated, right? :)
Comment 6 Fabiano Fidêncio 2013-07-12 12:53:54 UTC
Created attachment 249000 [details] [review]
Bug #703899 - Moving a meeting does not ask for confirmation

Now the user is asked for a conformation for every drag/resize
operation in the calendar view and it is valid for every backend.

Note:
I'm not adding new strings, just reusing the old ones in the popup,
so this patch can be easily backported to gnome-3.8
Comment 7 Fabiano Fidêncio 2013-07-15 14:43:55 UTC
Created attachment 249206 [details] [review]
Bug #703899 - Moving a meeting does not ask for confirmation

Notes:
- I'm not adding new strings, just reusing the old ones in the popup,
so this patch can be easily backported to gnome-3.8
- To work properly still depends of fixes on:
  - https://bugzilla.gnome.org/show_bug.cgi?id=704220
  - https://bugzilla.gnome.org/show_bug.cgi?id=704256
Comment 8 Fabiano Fidêncio 2013-07-15 15:47:02 UTC
Milan, you can have some problems testing the patch if you are in this case:
https://bugzilla.gnome.org/show_bug.cgi?id=704264

I'm assuming it's not my fault :-)
Comment 9 Milan Crha 2013-07-16 04:20:36 UTC
Review of attachment 249206 [details] [review]:

The patch looks good, except of few things:
- I am asked for "send meeting notification" when I drag&drop the meeting's edge. I almost always send meeting updates when drag&dropping in the day view and not new invitations
- the "x" on the window with Cancel/Do not send/Send acts as "Do not send", not as "Cancel"
- were you able to get a prompt for save_dragged_or_resized_component_dialog()? You cannot get there, because the event cannot be modified if a user is not an organizer, which means you always get a question for send_dragged_or_resized_component_dialog(). I'm not asked when I resize an appointment, which is correct.
- queue draw on change cancel on the day view, please, as I got a UI draw glitch on cancel of the recurring event change type question. I believe this was there before your change, please extend your fix

::: calendar/gui/dialogs/save-comp.c
@@ +82,3 @@
+	switch (vtype) {
+		case E_CAL_COMPONENT_EVENT:
+			attendees = e_cal_component_has_attendees (comp);

this check might be done outside of the function, because otherwise the function does more than is written above (though you might just drop the function (and its text in the .error file) entirely [see the main patch comment]).
Comment 10 Fabiano Fidêncio 2013-07-17 01:37:19 UTC
(In reply to comment #9)
> - queue draw on change cancel on the day view, please, as I got a UI draw
> glitch on cancel of the recurring event change type question. I believe this
> was there before your change, please extend your fix

I tried hard to reproduce this issue, but I can't. So, I did a completely blind shot to try to fix it.
I'm updating the patch and I hope it fits according with your comments and, please, let me know if you can reproduce it again.
Comment 11 Fabiano Fidêncio 2013-07-17 01:38:14 UTC
Created attachment 249338 [details] [review]
Bug #703899 - Moving a meeting does not ask for confirmation
Comment 12 Fabiano Fidêncio 2013-07-17 01:49:23 UTC
Created attachment 249339 [details] [review]
Bug #703899 - Moving a meeting does not ask for confirmation
Comment 13 Milan Crha 2013-07-17 07:15:56 UTC
Review of attachment 249339 [details] [review]:

The patch works fine, thanks. Just fix the two nitpicks mentioned here and commit to master and stable. Thanks.

::: calendar/calendar.error.xml
@@ +159,3 @@
+    <button _label="Do _not Send" response="GTK_RESPONSE_NO"/>
+    <button _label="_Send" response="GTK_RESPONSE_YES"/>
+  </error>

This error is unused, please drop it

::: calendar/gui/dialogs/send-comp.c
@@ +272,3 @@
+	case E_CAL_COMPONENT_EVENT:
+		id = !save_schedules ? "calendar:prompt-send-updated-meeting-info-dragged-or-resized" :
+				       "calendar:prompt-save-meeting-dragged-or-resized";

Reverse the if, to get rid of !save_schedules. Using negatives makes code harder to read (or easy to overlook the negation, in this case).
Comment 14 Milan Crha 2013-07-17 07:17:15 UTC
I also filled a follow-up bug #704369.