471973 - Make use of alarm interface in backend code

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v1

This patch takes care of using calIAlarm everywhere where we have previously been using alarmOffset/alarmRelated. No UI is changed here and no support for multiple/abosulte alarms is guaranteed, but this is another step into that direction.

I've sent this patch to Andreas for further testing. Please do comment on the code.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

Oh btw, I have no problem changing the interface iid for calIItemBase here, I just forgot about it :-)

Comment 2

[Daniel Boelzle [:dbo]]

Comment on attachment 355220 [details] [diff] [review]
Fix - v1

Overall the changes look good to me, though I haven't tested them; r=dbo

b/calendar>+    let alarms = item.getAlarms({})
>+                     .filter(function(x) x.related != x.ALARM_RELATED_ABSOLUTE);
I didn't know that defining function like this is valid :)

>diff --git a/calendar/base/src/calItemBase.js b/calendar/base/src/calItemBase.js
>+        for (let alarmComp in cal.ical.subcomponentIterator(icalcomp, "VALARM")) {
>+            let alarm = cal.createAlarm();
>+            try {
>+                alarm.icalComponent = alarmComp;
>+                if (alarm.related != Components.interfaces.calIAlarm.ALARM_RELATED_ABSOLUTE) {
>+                    // TODO ALARMSUPPORT unconditionally add alarm when we
>+                    // support multiple alarms.
>+                    this.addAlarm(alarm);
>                 }
Does this mean we yet don't roundtrip multiple resp. absolute alarms?

>+    addAlarm: function cIB_addAlarm(aAlarm) {
>+        try {
>+            // Trigger the icalComponent getter to make sure the alarm is valid.
>+            aAlarm.icalComponent;
>+        } catch (e) {
>+            throw Components.results.NS_ERROR_INVALID_ARG;
>+        }
I don't think we should serialize the VALARM on every addAlarm() when reading/stuffing events objects. I agree it's good to check the alarm object though. Maybe you could add a private addAlarm_() that does not check.

Please use let instead of var in various for loops.

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

> Does this mean we yet don't roundtrip multiple resp. absolute alarms?
No, we also haven't been doing this before afaik. This is reserved for the next step.


> I don't think we should serialize the VALARM on every addAlarm() when
> reading/stuffing events objects. I agree it's good to check the alarm object
> though. Maybe you could add a private addAlarm_() that does not check.
Done, added a second parameter.


Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/a41dd9c8e51c>

-> FIXED

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".

Comment 5

[Philipp Kewisch [:Fallen][📱📆]]

These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".