307556 - Undo/redo 'move' should not delete events before adding them

Status: RESOLVED WONTFIX

(Calendar :: Sunbird Only, defect)

Description

[Joey Minta]

Bug 220655 made it possible to move events from one calendar to the other. 
However, the undo/redo transaction 'move' needed to delete the event before
adding it to the new calendar.  This does open the possibility for dataloss to
occur, if addItem fails, (e.g. the new calendar is unreachable).  This should be
corrected.

Comment 1

[Matthew (lilmatt) Willis]

Reassigning all automatically assigned bugs from Mostafa to nobody@m.o

Bugspam filter: TorontoMostafaMove

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

The real fix for this is architectural: since someone may be subscribed to multiple people's calendars with different views of the same event we shouldn't be using UID as our internal primary key, we need a separate internal-only ID for that which is guaranteed to be unique across all composite calendars.  A bug should be spun off for that.

For now, though, we _think_ this shouldn't be too hard to fix just by generating a temporary UID, so we're going to mark it blocking0.3+.  If it turns out to be overly difficult or overly risky, we may wish to reconsider.

Comment 3

[Matthew (lilmatt) Willis]

(In reply to comment #2)
> ...we shouldn't be using UID as our internal primary key, we need a separate
> internal-only ID for that which is guaranteed to be unique across all
> composite calendars.

How about concatenating a UID for the calendar with the one for the event, and using that, rather than creating an altogether new UID?

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Sipaq pointed out in IRC that this is really pretty difficult to trigger: not only  do you have to want to change calendars for an event (which requires that you're using multiple calendars), you also have to undo such an action, _and_ your computer or app has to either crash or lose network connectivity at exactly the moment you're doing this.  I think even without taking this patch, we can confidently say that "losing data is hard" with this release, which is really all we can say anyway.  Removing from the blocker list.

Comment 5

[Michiel van Leeuwen (email: mvl+moz@)]

There are a few errors in the reasoning:
redo is also called for the first action, not only for redo. (http://lxr.mozilla.org/seamonkey/source/calendar/base/content/calendar-item-editing.js#139)
An easy way to trigger this bug is trying to move an item to a read-only calendar.
I get this error:

[JavaScript Error: "[Exception... "'<error>' when calling method: [calICalendar::addItem]"  nsresult: "0x804a0002 (<unknown>)"  location: "JS frame :: chrome://calendar/content/calendar-item-editing.js :: anonymous :: line 324"  data: no]" {file: "chrome://calendar/content/calendar-item-editing.js" line: 324}]

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

bug 352865 will reduce the risk of dataloss, by disabling the option to move to a readonly calendar. That might be enough to make this bug not block 0.3 anymore.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

I agree with comment 6.  Marking that one as blocking, since it has a reviewed patch ready for checking, and this one as non-blocking.

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Joey, is this still valid for the latest 0.8pre or trunk nightlies?

Comment 9

[Joey Minta]

Yes. http://mxr.mozilla.org/seamonkey/source/calendar/base/src/calTransactionManager.js#179

Comment 10

[[:MakeMyDay]]

Sunbird is discontinued and not actively developed anymore.