Bug 541115 comment 2 describes a patch to this behavior.

FWIW, my vote would also be to allow the reply option for updated meetings (functionality does not exist currently), as well as for new meetings (functionality does exist currently).

Contrary to Bug 541115 comment 2, however, I would also advocate the (yet-to-be-added) option be _checked_. I believe it's safer to notify than not. However, I'd just be happy with having the option, regardless of its default.

I too have been mildly reprimanded recently for "ignoring" meeting update invitations. :-(

I think I have another use case that triggers the same sort of (undesired) behavior.

1. Invitation *recipient* opens Outlook (doesn't open any mail, just has Outlook running on some Windows machine)
2. *Organizer* uses sends a new meeting invite (attendee's status
defaults to "not replied" or equivalent)
3. Recipient's machine running Outlook adds the meeting to the Recipient's calendar *automatically*--user doesn't even need to open the invitation for this to happen. (This is at least what I *think* Outlook does, but somehow, it happens automatically, anyway.)
4. Recipient receives and opens the meeting invite in Evo, but since Outlook has automatically added it to the calendar, Evo doesn't show the "notify organizer" option.

I'm a bit fuzzy on the mechanics of step 3, but I know that somehow, the meeting gets added to my calendar, without my having selected Accept/Decline.

The same proposed fix would seem to address both the use case in comment 1, as well as the use case in this comment.

Created attachment 113879 [details] [review]
itip-formater.c: always show option to send confirmation

This patch was created by Patrick Ohly (bug 541115 comment 2)

(Patrick, I hope it wasn't a breach of etiquette to add you to the CC list.)

Patrick, the patch looks good, you can add a ChangeLog entry and commit to trunk.

@Jamie: adding me on CC is okay - I meant to watch this bug anyway.

@Milan: I will commit the patch, but first I would clarify one open question.

Should Evolution offer the "Notify the Organizer" option also for meeting invitations which were forwarded from inside a calendar (METHOD:PUBLISH instead of METHOD:REQUEST)?

If I understand Jamie correctly (bug #541115 comment #4), at least in some cases he got such forwarded invitations and couldn't notify  the organizer although he wanted to. Is that right?

I think Evolution should offer the option in all cases where the event is a meeting invitation and has a meeting organizer, regardless of how the event was sent. If we agree on that, then the patch needs further work. I can have a look at that.

(In reply to comment #5)
> If I understand Jamie correctly (bug #541115 comment #4), at least in some
> cases he got such forwarded invitations and couldn't notify  the organizer
> although he wanted to. Is that right?
> 
> I think Evolution should offer the option in all cases where the event is a
> meeting invitation and has a meeting organizer, regardless of how the event was
> sent. If we agree on that, then the patch needs further work. I can have a look
> at that.

I'm fine with this extension, no objection from my side.

e_cal_save_chedules static capability should be checked before showing rsvp as some servers handle sending mails automatically when the events are accepted.

> e_cal_save_chedules static capability should be checked before showing rsvp as
> some servers handle sending mails automatically when the events are accepted.

I don't understand the documentation for that call. "e_cal_get_save_schedules - Checks whether the calendar saves schedules. TRUE if it saves schedules, FALSE otherwise." The comment just repeats the function name, without explaining what "saving schedules" means and how it is related to notifications. Very useful.

Does it return "true" when the backend itself sends notifications or does it return "false" in that case?

(In reply to comment #5)
> If I understand Jamie correctly (bug #541115 comment #4), at least in some
> cases he got such forwarded invitations and couldn't notify  the organizer
> although he wanted to. Is that right?

I haven't experienced the forwarding case you've described, actually. (I'm not saying it doesn't exist, but I can't remember the last time I was forwarded an invitation.)

As far as I have noticed, the two cases that affect me are described in comment 0 and comment 2 (both of which preclude notifying the organizer, since the event is in the calendar already).

Chenthill, can you please comment on my questions in comment #8 and then reassign to me? Thanks!

Patrick, save_schedules is a static capability added in the backend. In servers such as groupwise the notifications would be handled by the server. The client need not send a separate notification to the organizer. save_schedules means the server would handle the notifications when the client saves the meetings. None of the static capabilities are currently documented atm which is bad :( I will document them in e-cal-utils.h. 

Sorry for the delay, just came back from GUADEC.

> save_schedules means
> the server would handle the notifications when the client saves the meetings.

"save_schedules == TRUE" means that the server handles the notification, right?

Created attachment 118033 [details]
test data: a normal meeting request and a forward meeting

Created attachment 118036 [details] [review]
improved meeting invitation handling

This patch does two things:
1. a meeting response can be sent even if the meeting was already imported earlier
2. a meeting response can be sent even if the meeting was forwarded, including tentative and decline replies

The second point is accomplished by treating events which contain attendees as meetings in addition to those which are sent with method=request (the old behavior).

Okay to commit on trunk?

definitely remove this junk, please
@@ -1863,6 +1888,11 @@
we discuss this in that bug, no need to making comment like this within other bugs...

the check for ICAL_ATTENDEE_PROPERTY, I noticed that for Google everything is a meeting, there is always at least one attendee, the owner of the calendar. I'm not sure how much does it matter, though.

Do you expect notification with any other non-meeting events? I cannot think of any. (I mean events, neither assigned tasks nor memos)

> definitely remove this junk, please
> @@ -1863,6 +1888,11 @@
> we discuss this in that bug, no need to making comment like this within other
> bugs...

I'll remove the comment because your patch will solve it (will test it soon). 

What is the right way to add "TODO" or "FIXME" comments to the code? The simplest one is to add them during maintenance of the code and then commit as part of the resulting patch, as I did.

I think source code comments and bug tracker entries should be used in parallel:
a bug tracker cannot replace source code comments because people reading the code usually won't be aware of them. Likewise source code comments cannot be used to discuss an issue.

But I digress...

> the check for ICAL_ATTENDEE_PROPERTY, I noticed that for Google everything is a
> meeting, there is always at least one attendee, the owner of the calendar. I'm
> not sure how much does it matter, though.

If someone forwarded such a non-meeting event, the recipient would now have the option to send a response. I don't think that this matters because ...

> Do you expect notification with any other non-meeting events? I cannot think 
> of any. (I mean events, neither assigned tasks nor memos)

... such forwarded events should indeed be rare. But I can imagine that this happens, for example to let a friend know about a certain event which is not a meeting.

(In reply to comment #16)
> What is the right way to add "TODO" or "FIXME" comments to the code? The
> simplest one is to add them during maintenance of the code and then commit as
> part of the resulting patch, as I did.

Do not take me wrong, I agree, just in this particular case we have live discussion in the other bug already, thus no need to mention it redundantly in the code in different patch.

> If someone forwarded such a non-meeting event, the recipient would now have the
> option to send a response. I don't think that this matters because ...
> 
> > Do you expect notification with any other non-meeting events? I cannot think 
> > of any. (I mean events, neither assigned tasks nor memos)
> 
> ... such forwarded events should indeed be rare. But I can imagine that this
> happens, for example to let a friend know about a certain event which is not a
> meeting.

OK, I see now.

Is it okay to commit the patch minus the comment about #346146?

I tried to compile and test the patch and with it I do not see any body of the meeting in the mail for invites I see before the patch. Something is wrong. My invite is a simple evolution meeting. I see the attachment icon with arrow down (indicating the body should follow), but nothing under that.

It worked for me. Please provide further information and attach the example for which it doesn't work.

When you revert the patch, recompile the itip-formatter plugin and restart Evolution, does it really work as expected?

Is the meeting stored in any calendar? If so, which one (local, remote, etc)?

I suppose you are using trunk?

Is format_itip_object() called and is the variable response_enabled set to true?

Created attachment 118359 [details]
test invite email

Created attachment 118360 [details]
what I see after patch

(In reply to comment #20)
> It worked for me. Please provide further information and attach the example for
> which it doesn't work.

Attached above. Import to local folder and see.

> When you revert the patch, recompile the itip-formatter plugin and restart
> Evolution, does it really work as expected?

Yes. No changes in my setup, only recompilation.

> Is the meeting stored in any calendar? If so, which one (local, remote, etc)?

It isn't. Without patch I see "select calendar" combo, thus should not be.

> I suppose you are using trunk?

Yes.

> Is format_itip_object() called and is the variable response_enabled set to
> true?

I do not know, let me know if have troubles to reproduce with this data, I'll dig more, but I'm doing other things at the moment.

Created attachment 118361 [details]
patch reverted

Created attachment 118382 [details] [review]
improved meeting invitation handling, fixed

The previous patch accidentally overwrote pitip->method in the following assignment:
 pitip->is_meeting = pitip->method = ICAL_METHOD_REQUEST ||
      icalcomponent_get_first_property (pitip->ical_comp, ICAL_ATTENDEE_PROPERTY) != NULL;

That should have been:
 pitip->is_meeting = pitip->method == ICAL_METHOD_REQUEST ||
      icalcomponent_get_first_property (pitip->ical_comp, CAL_ATTENDEE_PROPERTY) != NULL;

Not sure when I broke that, because it worked for me when I tested it. Sorry for that, and thanks to Milan for testing!

OK, this works better. What am I supposed to see when watching forwarded meetings?
I thought there will be also "Send reply to sender" check too, but no, it isn't.
Maybe it shouldn't be there, because Sender wouldn't be an Organizer, right?

Otherwise it seems to work fine, I only do not have the backend with the flag mentioned by chen in comment #11, thus I would prefer to let him to say the last word.

Patrick, you cheater, you left there that... @@ -1867,6 +1892,11 @@

> OK, this works better. What am I supposed to see when watching forwarded
> meetings?

You should see the same "Send reply to sender/Comment/Show time as free/Open Calendar/Decline/Tentative/Accept" options. I just checked, with the "forwarded invitation" from the "test data: a normal meeting request and a forward meeting" attachment I get these options.

> Maybe it shouldn't be there, because Sender wouldn't be an Organizer, right?

The reply should go to the ORGANIZER mentioned in the VEVENT.

Perhaps your forwarded meeting has no ATTENDEEs? That's what the code checks to turn of the options for normal events.

Can you attach your example?

> Patrick, you cheater, you left there that... @@ -1867,6 +1892,11 @@

Argh, sorry! I have removed it from my local copy now.

I just tried to create a meeting in evolution, and forward it from calendar view when right-clicking over it and chose "Forward as iCalendar", then I looked on this mail in regular folder. It was a meeting with two attendees, one same as organizer and the second one.

That's also how I created the "forwarded invitation" test case. Can you please attach your example so that I can look at the differences?

Does the "forwarded invitation" example work for you when you import it into a mailbox?

Created attachment 118435 [details]
forwarded one

hmm, so my Forward as ICalendar strips all the attendees, and then it makes trouble. I tried with your attachment and there it shows all things.

Your example looks almost like a normal non-meeting event. The only difference is the ORGANIZER. Perhaps the presence of the ORGANIZER property should be used to control whether "reply" is enabled?

Currently I check for ATTENDEE because that also seems to trigger the different behavior in the Evolution GUI. Checking for ORGANIZER makes more sense because without one, the event cannot be replied to.

I also thought that I had seen a non-meeting event with an ORGANIZER field, but I might have been wrong: creating one anew results in one without an ORGANIZER. Even so, if a VEVENT has an ORGANIZER even if it is not a meeting, then replying to it might make sense.

I'll rework the patch...

(In reply to comment #31)
> Your example looks almost like a normal non-meeting event. The only difference
> is the ORGANIZER. Perhaps the presence of the ORGANIZER property should be used
> to control whether "reply" is enabled?

I do not know, but believe or not, I forwarded regular meeting with exactly two attendees from trunk evolution, one was same as organizer, second was different. Something goes wrong there, probably (but true, question for other bug). I'm not sure whether there is something to rework (except of the offending chunk removal and ChangeLog entry addition) before we knew what's going on here.

> Currently I check for ATTENDEE because that also seems to trigger the different
> behavior in the Evolution GUI. Checking for ORGANIZER makes more sense because
> without one, the event cannot be replied to.
> 
> I also thought that I had seen a non-meeting event with an ORGANIZER field, but
> I might have been wrong: creating one anew results in one without an ORGANIZER.
> Even so, if a VEVENT has an ORGANIZER even if it is not a meeting, then
> replying to it might make sense.

This kind of events creates Google calendar. I mixed it in comment #15.

> > I also thought that I had seen a non-meeting event with an ORGANIZER field, but
> > I might have been wrong: creating one anew results in one without an ORGANIZER.
> > Even so, if a VEVENT has an ORGANIZER even if it is not a meeting, then
> > replying to it might make sense.
>
> This kind of events creates Google calendar. I mixed it in comment #15.

Evolution 2.23.92 also creates non-meeting events with ORGANIZER. This is where I have seen it before. Evolution 2.22.3 doesn't add an ORGANIZER.

So what's the preferred logic? How about this:

* if ORGANIZER: enable reply options, otherwise disable
* if ATTENDEEs -> assume meeting: send reply to ORGANIZER by default
* if no ATTENDEEs: don't send reply by default, but let user choose it

Created attachment 118514 [details] [review]
checks ORGANIZER, ATTENDEE and RSVP to determine whether a reply is possible/wanted

This implements the logic that I outlined in my previous comment. In addition I also added code which checks for RSVP=FALSE, so now it honors the meeting organizer's choice whether he wants replies.

The comment in the source explains the behavior and the rationale:

	/* a reply can only be sent if and only if there is an organizer */
	gboolean has_organizer;
	/*
	 * Usually replies are sent unless the user unchecks that option.
	 * There are some cases when the default is not to sent a reply
	 * (but the user can still chose to do so by checking the option):
	 * - the organizer explicitly set RSVP=FALSE for the current user
	 * - the event has no ATTENDEEs: that's the case for most non-meeting
	 *   events
	 *
	 * The last case is meant for forwarded non-meeting
	 * events. Traditionally Evolution hasn't offered to send a
	 * reply, therefore the updated implementation mimics that
	 * behavior.
	 *
	 * Unfortunately some software apparently strips all ATTENDEEs
	 * when forwarding a meeting; in that case sending a reply is
	 * also unchecked by default. So the check for ATTENDEEs is a
	 * tradeoff between sending unwanted replies in cases where
	 * that wasn't done in the past and not sending a possibly
	 * wanted reply where that wasn't possible in the past
	 * (because replies to forwarded events were not
	 * supported). Overall that should be an improvement, and the
	 * user can always override the default.
	 */
	gboolean no_reply_wanted;

	/*
	 * Treat meeting request (sent by organizer directly) and
	 * published evend (forwarded by organizer or attendee) alike:
	 * if the event has an organizer, then it can be replied to and
	 * we show the "accept/tentative/decline" choice.
	 * Otherwise only show "accept".
	 */
	itip_view_set_mode (ITIP_VIEW (info->view),
			    info->has_organizer ?
			    ITIP_VIEW_MODE_REQUEST :
			    ITIP_VIEW_MODE_PUBLISH);

Is it okay to commit? I would like to see this included in 2.24.

Just a little tiny, when you have a cancel notification in a mail, and it results in "Unable to find this meeting in any calendar", then "Open Calendar" and "Update" buttons are insensitive, but you still could "Send reply to sender" with a "Comment". It doesn't make much sense there.

Otherwise looks fine to me, even I prefer to let Chen decide, he knows the details around much better than me.

> Just a little tiny, when you have a cancel notification in a mail, and it
> results in "Unable to find this meeting in any calendar", then "Open Calendar"
> and "Update" buttons are insensitive, but you still could "Send reply to
> sender" with a "Comment". It doesn't make much sense there.

Agreed. I'll have a look at this after 2.24. Right now I'd rather not do further modifications of the patch because it is pretty well-tested as it is.

> Otherwise looks fine to me, even I prefer to let Chen decide, he knows the
> details around much better than me.

Chenthill?

Suman, Chenthill told me that he'll hand over patch review to you. Can you please give your okay to commit on the 2.24.x stable branch and trunk?

If you don't have the time to look at it, is the (very throrough, thanks!) review by Milan enough to proceed?

Have you considered this situation?

organizer A sends meeting request to attendees B and C -> B forwards meeting to D -> D responds -> A receives response from D but does not find D in the original list of attendees -> (problem is - whose response status does the organizer update?)

What do other clients do in such a scenario? AFAIR, Evolution adds D to the original list which seems like the right thing to do. If other clients don't behave similarly, do we really want to send responses to meeting forward notifications?

Otherwise, the patch looks good to me.. Please commit to STABLE and trunk with these changes: 
* add a ChangeLog entry
* avoid e_cal_get_save_schedules(), instead use e_cal_get_static_capability() [see bug #545263]
* this one's for readability: 
--- ICAL_RSVP_FALSE == icalparameter_get_rsvp (param) 
+++ icalparameter_get_rsvp (param) == ICAL_RSVP_FALSE 

--------

(In reply to comment #12)
> > save_schedules means
> > the server would handle the notifications when the client saves the meetings.
> 
> "save_schedules == TRUE" means that the server handles the notification, right?

yeah.. right. also see bug #545261

In reply to comment #31)
> I also thought that I had seen a non-meeting event with an ORGANIZER field, but
> I might have been wrong: creating one anew results in one without an ORGANIZER.
> Even so, if a VEVENT has an ORGANIZER even if it is not a meeting, then
> replying to it might make sense.

this should NEVER happen, if it does then its a bug [RFC 2445 states "ORGANIZER and ATTENDEE properties may appear only in group-scheduled entities" aka they got to have at least one attendee.]

I do not know why forwarding a meeting stripped all attendees for Milan. We have to check that. 

> Have you considered this situation?
>
> organizer A sends meeting request to attendees B and C -> B forwards meeting to
> D -> D responds -> A receives response from D but does not find D in the
> original list of attendees -> (problem is - whose response status does the
> organizer update?)

Yes. My assumption is that like Evolution, other clients also handle this situation by adding the sender of the unexpected reply by adding him/her as an attendee.

> Otherwise, the patch looks good to me.. Please commit to STABLE and trunk with these changes: 
> * add a ChangeLog entry

Done.

> * avoid e_cal_get_save_schedules(), instead use e_cal_get_static_capability()
> [see bug #545263]

Done.

As an aside: the existing capabilities a under-documented. e_cal_get_static_capability() is unusable unless one somehow learns about the predefined capability strings. I had to look at the implementation of e_cal_get_save_schedules() to find CAL_STATIC_CAPABILITY_SAVE_SCHEDULES.

> * this one's for readability: 
> --- ICAL_RSVP_FALSE == icalparameter_get_rsvp (param) 
> +++ icalparameter_get_rsvp (param) == ICAL_RSVP_FALSE 

Done.

Patch committed as revisions 36594 (trunk) and 36593 (2.24).

Created attachment 120342 [details] [review]
do not show reply-to-sender when viewing a reply

This addresses comment #36 by checking the type before enabling the option.

Suman, is it okay to also commit this additional improvement?

looks good.. please commit to STABLE and trunk. 

Committed to both branches in 36747.