Sunbird Alerts cannot be dismissed

[Adam Tauno Williams]

<http://bugzilla.opengroupware.org/bugzilla/show_bug.cgi?id=1876>

When an alarm is dismissed Sunbird/Lightning the alarm is sent back to
ZideStore like:

BEGIN:VALARM
TRIGGER;VALUE=DURATION:-P1W
X-MOZ-LASTACK:20080702T211432Z
DESCRIPTION:Mozilla Alarm: hfhfdhd
ACTION:DISPLAY
END:VALARM

Note the 'extended' attribute "X-MOZ-LASTACK".    This is *not* encoded
into
the evoReminder,  then ZideStore dutifully returns -

BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:Mozilla Alarm: hfhfdhd
TRIGGER;VALUE=DURATION;RELATED=START:-P1W
END:VALARM

The encoding is performed by the "alarmsToCSV" method of the
"SxAppointmentMessageParser"; this only captures the core attributes.
The iCalObject looks like it always ignores 'extended' (X-*) attributes
so I assume the value is lost by the time the data gets to
"alarmsToCSV"?

--<quote iCalObject>--------------
- (void)takeValue:(id)_value forXKey:(id)_key {
  /* extended keys are ignored by default */
}

- (void)handleTakeValue:(id)_value forUnboundKey:(NSString *)_key {
  if ([_key hasPrefix:@"X-"]) {
    [self takeValue:_value forXKey:_key];
  }
  else {
    NSLog(@"0x%08x[%@]: ignoring attempt to set unbound key '%@'",
          self, NSStringFromClass([self class]), _key);
  }
}
--</quote>-------------------------

Also the Logic command appointment::get-ical
(Logic/LSScheduler/LSGetICalForAppointmentsCommand.m) does the reverse
of SxAppointmentMessageParser's alarmsToCSV and likewise only processes
specific attributes.

<aside>Wouldn't an "appointment::from-ical" command be better than
parsing to iCal in a distant class (SxAppointmentMessageParser) and
rendering to iCal in a Logic class?</aside>

So I'm wondering how to work around the issue;  it renders the calendar
client effectively locked if an alarm is triggered.   One brute-force
solution would be to introduce a default the simply disables the
evoReminder field - alarms either not stored or not delivered to the
client.  Otherwise I can't see a solution since [it seems] all X-*
fields are lost in parsing [in SOPE].

The only way [I can see] to preserve the value is to patch SOPE
(attached) to add the X-MOZ-LASTACK to the iCalAlarm object.  I don't
know if that will be acceptable to other SOPE consumers?  With this
patch the data is available for ZideStore (I've tested that) and the
alarm code would just have to be modified to include this value if
present.

[X-MOZ-LASTACK.patch]

---

Index: sope-ical/NGiCal/NGiCal.xmap
===================================================================
--- sope-ical/NGiCal/NGiCal.xmap (revision 1622)
+++ sope-ical/NGiCal/NGiCal.xmap (working copy)
@@ -63,6 +63,7 @@
 
       attributes = {
         rrule = recurrenceRule;
+        "X-MOZ-LASTACK" = lastACK;
       };
     };
     
@@ -273,7 +274,11 @@
     "X-LIC-LOCATION" = {
       class = NSString;
     };
-    
+
+    "X-MOZ-LASTACK" = {
+      class = NSString;
+    };
+
     "X-WR-TIMEZONE" = {
       rejectWithContent = YES;
     };
Index: sope-ical/NGiCal/iCalAlarm.h
===================================================================
--- sope-ical/NGiCal/iCalAlarm.h (revision 1622)
+++ sope-ical/NGiCal/iCalAlarm.h (working copy)
@@ -33,6 +33,7 @@
   NSString *action;
   id       attach;
   NSString *recurrenceRule;
+  NSString *lastACK;
 }
 
 /* accessors */
@@ -41,6 +42,7 @@
 - (id)attach;
 - (NSString *)comment;
 - (NSString *)action;
+- (NSString *)lastACK;
 - (void)setRecurrenceRule:(NSString *)_recurrenceRule;
 - (NSString *)recurrenceRule;
 
Index: sope-ical/NGiCal/iCalAlarm.m
===================================================================
--- sope-ical/NGiCal/iCalAlarm.m (revision 1622)
+++ sope-ical/NGiCal/iCalAlarm.m (working copy)
@@ -29,6 +29,7 @@
   [self->comment        release];
   [self->action         release];
   [self->attach         release];
+  [self->lastACK        release];
   [self->recurrenceRule release];
   [super dealloc];
 }
@@ -44,6 +45,7 @@
   new->comment        = [self->comment copyWithZone:_zone];
   new->action         = [self->action copyWithZone:_zone];
   new->attach         = [self->attach copyWithZone:_zone];
+  new->lastACK        = [self->lastACK copyWithZone:_zone];
   new->recurrenceRule = [self->recurrenceRule copyWithZone:_zone];
   
   return new;
@@ -79,6 +81,13 @@
   return self->action;
 }
 
+- (void)setLastACK:(NSString *)_value {
+  ASSIGNCOPY(self->lastACK, _value);
+}
+- (NSString *)lastACK {
+  return self->lastACK;
+}
+
 - (void)setRecurrenceRule:(NSString *)_recurrenceRule {
   ASSIGN(self->recurrenceRule, _recurrenceRule);
 }
@@ -107,4 +116,12 @@
   return ms;
 }
 
+/* KVC */
+
+- (void)takeValue:(id)_value forXKey:(id)_key {
+  if ([_key isEqualToString:@"X-MOZ-LASTACK"])
+    [self setLastACK:_value];
+}
+
+
 @end /* iCalAlarm */

---

[Helge Hess]

On 03.07.2008, at 21:44, Adam Tauno Williams wrote:
> When an alarm is dismissed Sunbird/Lightning the alarm is sent back to
> ZideStore like:
>
> BEGIN:VALARM
> TRIGGER;VALUE=DURATION:-P1W
> X-MOZ-LASTACK:20080702T211432Z

I would tend to say that this is an issue in Lightning. If it uses X-  
attributes in CalDAV, it must be able to deal with the possibility of  
loosing such fields. This is not OGo specific at all.

In short: Sunbird would probably be better off storing ACKs in a  
separate store (possibly in addition to the X-MOZ-LASTACK which would  
still be useful in some cases).

My personal opinion is that iCal alarms (and especially ACKs!) never  
belong on the server but always on the client. Eg what about shared  
calendars? The ACK is at least person specific. Does that mean that if  
a user ACKs an event in Lightning, other attendees won't see the  
alarm?! Sounds like.

Anyways, I have no issue with adding support for storing the X-MOZ-
LASTACK field in that CSV field.

Greets,
   Helge
--
Helge Hess
http://helgehess.eu/
--
OpenGroupware.org Developer
developer@...
http://mail.opengroupware.org/mailman/listinfo/developer

[Adam Tauno Williams]

On Thu, 2008-07-03 at 22:41 +0200, Helge Hess wrote:
> On 03.07.2008, at 21:44, Adam Tauno Williams wrote:
> > When an alarm is dismissed Sunbird/Lightning the alarm is sent back to
> > ZideStore like:
> > BEGIN:VALARM
> > TRIGGER;VALUE=DURATION:-P1W
> > X-MOZ-LASTACK:20080702T211432Z
> I would tend to say that this is an issue in Lightning. If it uses X-  
> attributes in CalDAV, it must be able to deal with the possibility of  
> loosing such fields. This is not OGo specific at all.

Agree.  But I can't fix Sunbird/Lightning;  and I'd like to see it
"work" for simple use-cases (mostly private calendars) as the user would
naturally expect.  More complicated scenarios [as always] will need to
educate/train their users.

> In short: Sunbird would probably be better off storing ACKs in a  
> separate store (possibly in addition to the X-MOZ-LASTACK which would  
> still be useful in some cases).

Agree.

> My personal opinion is that iCal alarms (and especially ACKs!) never  
> belong on the server but always on the client. Eg what about shared  
> calendars? The ACK is at least person specific. Does that mean that if  
> a user ACKs an event in Lightning, other attendees won't see the  
> alarm?!

Agree, alarms in general don't seem very multi-user friendly or aware.
It also seems like *supporting* alarms on any server, let alone OGo,
(sending e-mail, notices, etc...) would be extremely complicated.

> Anyways, I have no issue with adding support for storing the X-MOZ-
> LASTACK field in that CSV field.

Does that mean you'll accept the patch to SOPE or an equivalent?

As an aside:  Evolution, interestingly, seems to notice the alarm(s) but
not do anything about them - no popups, etc...

--
OpenGroupware.org Developer
developer@...
http://mail.opengroupware.org/mailman/listinfo/developer

[Helge Hess]

On 03.07.2008, at 21:44, Adam Tauno Williams wrote:
> The only way [I can see] to preserve the value is to patch SOPE
> (attached) to add the X-MOZ-LASTACK to the iCalAlarm object.


I added that patch. (except for the KVC thing which is superflous).

Thanks,
   Helge
--
Helge Hess
http://helgehess.eu/
--
OpenGroupware.org Developer
developer@...
http://mail.opengroupware.org/mailman/listinfo/developer

[Adam Tauno Williams-2]

On Fri, 2008-07-04 at 17:37 +0200, Helge Hess wrote:
> On 03.07.2008, at 21:44, Adam Tauno Williams wrote:
> > The only way [I can see] to preserve the value is to patch SOPE
> > (attached) to add the X-MOZ-LASTACK to the iCalAlarm object.
> I added that patch. (except for the KVC thing which is superflous).

Are you sure about that?  Without the KVC bit I get -

PUT /zidestore/dav/adam/Overview/11170.ics HTTP/1.1
.....
X-MICROSOFT-CDO-IMPORTANCE:0
X-MICROSOFT-CDO-BUSYSTATUS:BUSY
X-MICROSOFT-CDO-INSTTYPE:0
X-MICROSOFT-CDO-ALLDAYEVENT:FALS
BEGIN:VALARM
TRIGGER;VALUE=DURATION:-P1W
X-MOZ-LASTACK:20080705T005551Z
DESCRIPTION:Mozilla Alarm: hfhfdhd
ACTION:DISPLAY
END:VALARM
END:VEVENT
END:VCALENDAR
...
Jul 05 00:55:51 ogo-zidestore-1.5 [14120]:
<<0x0x847db0c[SxAppointment]>>D etag '11170:24' matches: 11170:24
Jul 05 00:55:51 ogo-zidestore-1.5 [14120]:
<0x0x86f14dc[SxAppointmentMessageParser]> X-MOZ-LASTACK =
Jul 05 00:55:51 ogo-zidestore-1.5 [14120]:
<0x0x86f14dc[SxAppointmentMessageParser]> ALARMS = 'DISPLAY','Mozilla
Alarm: hfhfdhd','DURATION','-P1W',,
...

There is no value in lastACK.    With the KVC thing I get -

PUT /zidestore/dav/adam/Overview/11170.ics HTTP/1.1
....
X-MICROSOFT-CDO-IMPORTANCE:0
X-MICROSOFT-CDO-BUSYSTATUS:BUSY
X-MICROSOFT-CDO-INSTTYPE:0
X-MICROSOFT-CDO-ALLDAYEVENT:FALSE
BEGIN:VALARM
TRIGGER;VALUE=DURATION:-P1W
X-MOZ-LASTACK:20080705T010537Z
DESCRIPTION:Mozilla Alarm: hfhfdhd
ACTION:DISPLAY
END:VALARM
END:VEVENT
END:VCALENDAR
...
Jul 05 01:05:38 ogo-zidestore-1.5 [14966]:
<<0x0x8729d74[SxAppointment]>>D etag '11170:25' matches: 11170:25
Jul 05 01:05:38 ogo-zidestore-1.5 [14966]:
<0x0x8469b2c[SxAppointmentMessageParser]> X-MOZ-LASTACK =
20080705T010537Z
Jul 05 01:05:38 ogo-zidestore-1.5 [14966]:
<0x0x8469b2c[SxAppointmentMessageParser]> ALARMS = 'DISPLAY','Mozilla
Alarm: hfhfdhd','DURATION','-P1W',,
...

I'm pretty sure overload in iCalObject is causing this.

--<quote>---------------------
- (void)takeValue:(id)_value forXKey:(id)_key {
   /* extended keys are ignored by default */
}

- (void)handleTakeValue:(id)_value forUnboundKey:(NSString *)_key {
   if ([_key hasPrefix:@"X-"]) {
     [self takeValue:_value forXKey:_key];
   }
   else {
     NSLog(@"0x%08x[%@]: ignoring attempt to set unbound key '%@'",
           self, NSStringFromClass([self class]), _key);
   }
}
--</quote>--------------------

--
OpenGroupware.org Developer
developer@...
http://mail.opengroupware.org/mailman/listinfo/developer

[Helge Hess]

On 05.07.2008, at 04:37, Adam Tauno Williams wrote:
> On Fri, 2008-07-04 at 17:37 +0200, Helge Hess wrote:
>> On 03.07.2008, at 21:44, Adam Tauno Williams wrote:
>> > The only way [I can see] to preserve the value is to patch SOPE
>> > (attached) to add the X-MOZ-LASTACK to the iCalAlarm object.
>> I added that patch. (except for the KVC thing which is superflous).
>
> Are you sure about that?  Without the KVC bit I get -
...

Hm, so it does not work. The question is why, I don't see a good reason.

> I'm pretty sure overload in iCalObject is causing this.
>
> --<quote>---------------------
> - (void)takeValue:(id)_value forXKey:(id)_key {
>  /* extended keys are ignored by default */
> }
>
> - (void)handleTakeValue:(id)_value forUnboundKey:(NSString *)_key {
>  if ([_key hasPrefix:@"X-"]) {
>    [self takeValue:_value forXKey:_key];
>  }
>  else {
>    NSLog(@"0x%08x[%@]: ignoring attempt to set unbound key '%@'",
>          self, NSStringFromClass([self class]), _key);
>  }
> }
> --</quote>--------------------

Hm, but this should only get triggered for *unbound* keys. Apparently  
the "X-" string is passed into the KVC system, instead of the  
'lastACK' which you bound in NGiCal.xmap. Weird.

Anyways, I'm going to add the 'forXKey' thing.

Thanks,
   Helge
--
Helge Hess
http://helgehess.eu/
--
OpenGroupware.org Developer
developer@...
http://mail.opengroupware.org/mailman/listinfo/developer