568919 - Moving an event from a calendar to another may cause the loss or duplicity of the event.

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Dimas]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; es-ES; rv:1.9.0.19) Gecko/2010040116 Ubuntu/9.04 (jaunty) Firefox/3.0.19
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; es-ES; rv:1.9.1.11pre) Gecko/20100519 Lightning/1.0b2pre Shredder/3.0.6pre

If an error happens while moving an event from a calendar to another, the event can be lost (if the event couldn't be written to target calendar) or duplicated (if the event couldn't be removed from source calendar)

Reproducible: Always

Steps to Reproduce:
1. Create two calendars "Calendar1" and "Calendar2".
2. Create an event in "Calendar1"
3. Move that event to "Calendar2".
Actual Results:  
The event should be erased from "Calendar1" and written to "Calendar2" but these scenarios may arise:

Case 1
------
The event is successfuly created in Calendar2 but deletion from Calendar1 fails. Now both calendar contains the same event.

Case 2
------
The event is successfuly deleted from Calendar1 but insertion to Calendar2 fails, so the event is lost.

Expected Results:  
Case 1
------
Se produce un error al borrar de "Calendar1" y se crea correctamente en "Calendar2": el evento debería permanecer en "Calendar1" y no estar en "Calendar2". Quedarse como estaba al principio.
The operation must be atomic: if the event can't be deleted from Calendar1, then it shouldn't be created in Calendar2.

Case 2
------
The operation must be atomic: it the event cannot be written to Calendar2, it should not be erased from Calendar1.

Case 1 a minor annoyance but Case 2 is a serious error because there's data loss.

Comment 1

[Dimas]

Attachment: Patch for Bug 568919

This patch resolves the case 2, which I think is the most serious, because there's data loss.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 448064 [details] [diff] [review]
Patch for Bug 568919

>+                    var realListener = this;
I'd prefer calling this "self", you also don't need extra the variables but can use self instead.


>+                    var addListener = {};
Go ahead and inline the onOperationComplete instead of defining it afterwards.

>+
>+                    addListener.onOperationComplete = function cT_onOperationComplete(aCalendar,
>+                            aStatus,
>+                            aOperationType,
>+                            aId,
>+                            aDetail) {
>+                        cal.ERROR("Eliminamos del otro calendario");
Debug stuff needs to be removed.
>+                        realListener.onOperationComplete(aCalendar, aStatus, aOperationType, aId, aDetail);


Patch looks fine, r=philipp.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 448064 [details] [diff] [review]
Patch for Bug 568919

>+                    addListener.onOperationComplete = function cT_onOperationComplete(aCalendar,
>+                            aStatus,
>+                            aOperationType,
>+                            aId,
>+                            aDetail) {
>+                        cal.ERROR("Eliminamos del otro calendario");
>+                        realListener.onOperationComplete(aCalendar, aStatus, aOperationType, aId, aDetail);
>+                        oldItem.calendar.deleteItem(oldItem, realListener);
Actually you should also add if (Components.isSuccessCode(aStatus)) { ... }

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Backported to comm-1.9.2 <http://hg.mozilla.org/releases/comm-1.9.2/rev/031f946a64c5>

a=philipp