If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Enable moving items (such as events) from one Calendar File to another

RESOLVED FIXED

Status

Calendar
General
--
enhancement
RESOLVED FIXED
14 years ago
10 years ago

People

(Reporter: Matija Polajnar, Assigned: Joey Minta)

Tracking

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20030925
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20030925

I'd like to be able to move events and tasks from one calendar file to another,
because I use multiple Calendar Files for purpose of grouping items.

Reproducible: Always

Steps to Reproduce:
1. Enter a new event or task. Calendar File can be chosen.
2. Modify event or task.
Actual Results:  
"Calendar File" combo box is disabled.

Expected Results:  
It would be nice that Mozilla Calendar would enable users to move items from one
file to another.

This entry has some corellation to my entry #220652, but as it is not just the
same, I decided to add a separate entry.

Comment 1

13 years ago
*** Bug 259472 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

12 years ago
*** Bug 298598 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 3

12 years ago
Making this bug a blocker for 0.3a1.  Currently, for me, if I switch the
calendar an event belongs to in the edit-event dialog, then hit 'OK', the event
just disappears.  I'm not sure where exactly this should be fixed ('modify'
command? event-dialog?), but losing events is definitely not cool.  Worst case,
we need to disable the menu-option again.
Blocks: 298936
(Assignee)

Comment 4

12 years ago
Created attachment 190907 [details] [diff] [review]
switch action order

This was easy once I looked at it more closely.  Due to the fact that the event
still has the same ID, you can't add it before you delete it without problems. 
Deleting first makes it work.
Assignee: mostafah → jminta
Status: NEW → ASSIGNED
Attachment #190907 - Flags: first-review?(mvl)
Comment on attachment 190907 [details] [diff] [review]
switch action order

r=mvl
Attachment #190907 - Flags: first-review?(mvl) → first-review+
patch checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

12 years ago
*** Bug 291243 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 8

12 years ago
*** Bug 209687 has been marked as a duplicate of this bug. ***

Comment 9

12 years ago
(In reply to comment #4)
> Due to the fact that the event
> still has the same ID, you can't add it before you delete it without problems. 
> Deleting first makes it work.

What are the problems?  

I'm concerned this may lead to dataloss.  Adding before deleting avoids
dataloss, and switching this order may lead to dataloss if there is an error.

1. Deleting from calendar A before adding to calendar B mean
there could be dataloss.  (For example, the calendar B might be remote
and may become unreachable.  If calendar is deleted from A and then addition to
B fails because of poweroutage or network failure, then item could be lost.)

* should add to calendar B before deleting from calendar A, so there is no 
dataloss, even if program crashes when accessing one of the calendars.

* the 'add' and 'delete' should be separate subtransactions, so when one throws
an exception, the other is undone.  (Note: these are undo/redo 'transactions',
not distributed database transactions.  See
http://www.mozilla.org/editor/txmgr/transaction-manager.html#Rollback)


2. In any case, same fixes should be made to undoTransaction, just a few lines
later.

http://lxr.mozilla.org/mozilla/source/calendar/resources/content/calendar.js#1498
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

12 years ago
(In reply to comment #9)
I'm happy to fix the undoTransaction part here.  However, I suggest filing a
separate bug on the fact that these should be undo/redo transactions, as that
will be a more substantial patch and rather unrelated to the problems described
in this bug.

The current problem with adding before deleting was discussed a bit on IRC.  The
providers only like to have one instance of an event-ID.  When you add and then
delete, the same event tends to get acted on.  (Try performing a 'move'
transaction before this patch.)  Deleting before adding prevents this from
happening.  This is the same problem that's holding up bug 300250, since
copy+paste results in the same 2-instances-of-the-same-ID problem.  We really
need a method for copying all properties of an event, except the ID.

However, while copy+paste should result in an event with a new ID, moving an
event from one calendar to another probably should not.  It's still the same
event, and should therefore have the same ID.  Therefore, I'd recommend keeping
the delete and then add order, but with a proper implementation of transactions,
to avoid the data loss.

Comment 11

12 years ago
why:

These are undo/redo 'transactions', not atomic transactions as in a database. 
Unlike atomic transactions, they do not undo if the program crashes, or the
computer fails (power outage).  

(In a database transaction, all the suboperations are performed/written only
tentatively, and don't become permanent until the transaction commits.  So if
the system crashes, when the database restarts it throws away the uncommitted
changes and reverts to the previous state.  That way the transaction is atomic:
it either performs all its operation, or none of them.)

The undo/redo 'transactions' have no 'commit' operation, so they don't inform
the provider when they are ready to commit, so they can't be atomic against
program/computer crashes.  Therefore the program should still add before delete
to avoid dataloss in case of program/computer crashes.

how:

It sounds like maybe the old provider and the new provider are the same
provider, and calendar is just a field.  

A. May be able to use a modify operation to change the calendar field, rather
than delete/add, if on same provider.  (Somewhat analogous to moving a file on
the same disk: the filesystem modifies the name or path, it doesn't rewrite the
file.)

B. Or add a new item with a different id, then delete the old item, then modify
the new item id to the old item id.  (One standard approach used to prevent
dataloss when saving files is to write the new file to a temporary name, and
then once that succeeds rename the new file to the old filename.)

Updated

12 years ago
QA Contact: gurganbl → general
Created attachment 194227 [details] [diff] [review]
fix transaction stuff

The problem with duplicated id's only happens when you have two storage
calendars in the same file (two 'local' calendars) Then the db gets confused
when two events have the same ID. I'm not sure if that is more a
storage-provider bug, but for now, we can workaround it.

This patch makes the move a nested transaction of change id, add, delete, so
should be more safe. (it doesn't leave the calendar in an unchanged state if
the computer crashes halfway, but at least the data isn't gone. With all the
other possible places for dataloss we have, i consider this good enough)
Attachment #194227 - Flags: first-review?(gekacheka)

Comment 13

12 years ago
Comment on attachment 194227 [details] [diff] [review]
fix transaction stuff

			       add    id2 to	    cal2
  delete id1 from      cal1
			       rename id2 to id1 in cal2

what patch implements:
  rename id1 to id2 in cal1
			       add    id1 to	    cal2
  delete id2 from      cal1

This is subtle.  As you noted, there is a disadvantage: it does not leave cal1
in the original state if there is a power failure or crash during the add.  Is
there an advantage of doing the rename operation first?
Also, how about changing the ID back, post-move?
Created attachment 194460 [details] [diff] [review]
better fix for transaction stuff

> Is there an advantage of doing the rename operation first?
The advantage was that i actaully thought of it. I didn't thought of doing it
the other way. The new patch does adding before modify or delete. Should be
safe. (a crash during undo will leave the calendar modified, but that is less
common)
Attachment #194227 - Attachment is obsolete: true
Attachment #194460 - Flags: first-review?(gekacheka)
No longer blocks: 298936

Comment 16

12 years ago
Comment on attachment 194460 [details] [diff] [review]
better fix for transaction stuff

Looks ok, but crashes during redo 
(on w2k, using nightly 20050901):

Local calendars:
0. default local calendar is "home"
1. create local calendar "home2" if necessary.
2. make sure "calendar" column is visible in event search list (unifinder).

3. create event "movetest" in calendar "home". ok.
   (event appears with calendar "home")
4. edit event "movetest", change calandar to "home2". ok.
   (event appears with calendar "home2")

5. menu edit | undo
   (BUG: calendar is still "home2".  Expected calendar to become "home".)
6. menu edit | redo
   (BUG: CRASH)

7. restart sunbird
   ("movetest" appears twice in unifinder, both in "home2".  
    Can't select second one?
    Deleting first deletes both.)

ICS Calendars (local file)
0. create local ICS file calendar /temp/ICSMoveTest1.ics
1. create local ICS file calendar /temp/ICSMoveTest2.ics
2. make sure "calendar" column is visible in event search list (unifinder).

3. create event "icsmove" in calendar "ICSMoveTest1". ok.
   (event appears with calendar "ICSMoveTest1")
4. edit event "icsmove", change calandar to "ICSMoveTest2". ok.
   (event appears with calendar "ICSMoveTest2")

5. menu edit | undo
   (Calendar becomes "ICSMoveTest1", better than local calendars.)
8. menu edit | redo
   (BUG: CRASH)

9. restart sunbird
   ("icsmove" appears once in unifinder, in "ICSMoveTest2".)

Can you find cause of these crashes?
Attachment #194227 - Flags: first-review?(gekacheka)
Comment on attachment 194460 [details] [diff] [review]
better fix for transaction stuff

Doing this right turns out to be a major pain.
- The ics (or better: memory) provider doesn't allow you to modify the id of an
event. Could make sense, but hurts us here
- To do undo right, you need to catch the errors that you get back in the
listener, which is async. The txnmgr can't really handle that.

So, i'm staying with the current code. That at least works. Yes, it might loose
an event if something crashes at the wrong moment. But that can happen anyway
when writing the ics file to disk. We should start with fixing that.
Attachment #194460 - Attachment is obsolete: true
Attachment #194460 - Flags: first-review?(gekacheka)
btw, i did found the reason for the crash: you shouldn't add a new transaction
in redoTransaction. The 'move' action shouldn't do anything. The txnmgr will
handle it, thanks to the child-transactions.
(Assignee)

Comment 19

12 years ago
Created attachment 194694 [details] [diff] [review]
switch undo move order

From gekacheka's original comments, the order of undo's move should also be
switched.  We still have a chance for dataloss, but this will work correctly
most of the time.
Attachment #194694 - Flags: first-review?(mvl)
Comment on attachment 194694 [details] [diff] [review]
switch undo move order

r=mvl

I suggest closing this bug, and filing a new one for the safer way to do the
move.
Attachment #194694 - Flags: first-review?(mvl) → first-review+
(Assignee)

Comment 21

12 years ago
Patch checked in.  Closing bug in response to comment #20.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED

Comment 22

12 years ago
This bug is closed, in order to file a new bug to do this safer. Where is that bug? This feature is still not available in TB 1.5 extension 20051201. 

Is it fixed in Sunbird? Will that come to the TB stable extension anytime soon?
(Assignee)

Comment 23

12 years ago
(In reply to comment #22)
> This bug is closed, in order to file a new bug to do this safer. Where is that
> bug? This feature is still not available in TB 1.5 extension 20051201. 
> 
> Is it fixed in Sunbird? Will that come to the TB stable extension anytime soon?
> 
This bug is fixed in Sunbird 0.3a1 and nightly builds.  The new bug for safer moving is bug 307556.  The 'stable' extension is not being updated, only the 'experimental' extension and Lightning will contain this feature.  (Hopefully the 'experimental' extension can eventually become stable enough to replace the stable version.)

Comment 24

10 years ago
Has the latest patch landed yet?

I don't think so and I think that caused bug 425260.
You need to log in before you can comment on or make changes to this bug.