450653 - dragging file onto Tasks or Calendar icons & pasting from clipboard changes UUIDs during importing

Status: NEW

(Calendar :: General, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

There are two semantic issues here:

* these events will no longer match up correctly if updates come along (eg via ITIP).  This makes it essentially dataloss; adding the dataloss keyword and blocking tb-integration+ for that reason.

* it's easy to import duplicates.  If undo works on import (does it?), this probably doesn't matter.

It seems entirely possible that importing events in an analogous way would have the same problem.  Adding qawanted to check that as well as the undo question.

As Daniel points out, we'll need some UI for what do in case duplicates are detected; adding uiwanted for that.

Comment 1

[Daniel Boelzle [:dbo]]

(In reply to comment #0)
> * these events will no longer match up correctly if updates come along (eg via
> ITIP).  This makes it essentially dataloss; adding the dataloss keyword and
> blocking tb-integration+ for that reason.
I slightly disagree (only) with the dataloss part. IMO this is not dataloss per se, because the current dialog states that you're creating a *new* event/task, thus I think this bug is more about adding functionality to import via drag/drop (our file import code keeps UIDs). Moreover, using the iTIP/iMIP buttons is recommended either way, and iTIP REQUEST messages are correctly handled.

Comment 2

[Günter]

(In reply to comment #1)

> I slightly disagree (only) with the dataloss part. IMO this is not dataloss per
> se, because the current dialog states that you're creating a *new* event/task,
> thus I think this bug is more about adding functionality to import via
> drag/drop (our file import code keeps UIDs). Moreover, using the iTIP/iMIP
> buttons is recommended either way, and iTIP REQUEST messages are correctly
> handled.
> 
Sorry, Daniel 'data loss' is DATA LOSS, no question! As Dan described the definition with UUID is clear! The argument "that you're creating a *new* event/task" is WRONG! If an user imports an event/todo with an UUID which is already stored in his stack, the import is NOT new!
Also your argument about different UI interfaces/importing methods fails here!
So not "adding functionality" but fixing a bug is necessary!

Comment 3

[Daniel Boelzle [:dbo]]

(In reply to comment #2)
> Sorry, Daniel 'data loss' is DATA LOSS, no question! As Dan described the
What's this supposed to mean?

> definition with UUID is clear! The argument "that you're creating a *new*
> event/task" is WRONG! If an user imports an event/todo with an UUID which is
> already stored in his stack, the import is NOT new!
Yes, that's what I wanted to express: file import already handles this correctly, because it has a clear semantic what it does.
I have doubts this is that easily adoptable to this bug. I currently see this conflicting with iTIP messages. Two scenarions:
* User simply wants to get an ics snippet into his calendar (preferrably UID stays as is) => IMO easy, probably just needs a confirmation in case an item gets overwritten.
* User drags an iTIP attachment onto the buttons: iTIP messages differ significantly and plain import is (except for REQUEST and PUBLISH) not what we want here. For example, a plain import/overwrite of a REPLY message would cause dataloss.

> Also your argument about different UI interfaces/importing methods fails here!
How do you want to handle UID without further UI? I doubt you just want to overwrite items without confirmation.

> So not "adding functionality" but fixing a bug is necessary!
I am still not convinced this is a plain bug, but we need to define what we really want to claim here. Staying defensive (especially w.r.t. to iTIP messages) is IMO not the worst option for the moment.

Comment 4

[Günter]

RFC very clearly defines the purpose, generation and use of the "UID":

http://tools.ietf.org/rfcmarkup?doc=draft-ietf-calsify-rfc2445bis-07#section-3.8.4.6
Unique Identifier

   Property Name:  UID

   Purpose:  This property defines the persistent, globally unique
      identifier for the calendar component.

	
   Description:  The "UID" itself MUST be a globally unique identifier.
      The generator of the identifier MUST guarantee that the identifier
      is unique. 
      ...
      This is the method for correlating scheduling messages with the
      referenced "VEVENT", "VTODO", or "VJOURNAL" calendar component.
      ....
      This property is an important method for group scheduling
      applications to match requests with later replies, modifications
      or deletion requests.  Calendaring and scheduling applications
      MUST generate this property in "VEVENT", "VTODO" and "VJOURNAL"
      calendar components to assure interoperability with other group
      scheduling applications.  This identifier is created by the
      calendar system that generates an iCalendar object.

From these points it's clear:

- an "VEVENT", "VTODO", or "VJOURNAL" calendar component having already an UID has to be handled to belong to an existing calendar component in a calendar .. if the UID is already part of it. If that's not the case, the component has to be handled as new.

It's totally irrelvant on which way the components get into the CUA. If the CUA doesn't find the UID stored, it has to be treated as NEW. If not, it's an UPDATE to that existent component. If there is a conflict, the CUA may / or may not ask the CU how to handle. A friendly CUA -- no question -- will ask.

That said, I assist Dan with his posting here: "adding the dataloss keyword and blocking tb-integration+"

Comment 5

[Stefan Sitter [:ssitter]]

I don't think there are any issues with the meaning of the "UID".

The issue raised here is that the mode toolbar icons action is currently defined and implemented as "create a new task/event/message from whatever is dropped". It is not defined as "import whatever is dropped".

Question is if the current behavior should be changed to "import whatever is dropped". In that case Bug 325449 needs to be fixed first.

Comment 6

[Daniel Boelzle [:dbo]]

(In reply to comment #4)
> It's totally irrelvant on which way the components get into the CUA. If the CUA
> doesn't find the UID stored, it has to be treated as NEW. If not, it's an
> UPDATE to that existent component. If there is a conflict, the CUA may / or may
> not ask the CU how to handle. A friendly CUA -- no question -- will ask.
This is wrong. An iTIP message may not only be an update, but can also contain updates to particular properties (e.g. PARTSTAT on REPLY) or even CANCEL items. You still seem to ignore my argument on iTIP messages. At least I see common sense that further UI is wanted. Thanks.

Comment 7

[Günter]

Daniel,
just stop fitting words! You just should accept what the RFC says!

"updates to particular properties (e.g. PARTSTAT on REPLY) or even CANCEL items" ARE updates to the component -- what posting #4 expresses. 

Comment 8

[Bas van den Bosch]

To clarify this bug, it sprung out from http://tinyurl.com/6oyo7o

I don't really understand the discussion, it's clear how the D&D works now and how it could (or should) work. ssitter is right, bug 325449 should be fixed first. Then we can change the behaviour of the D&D from creation-actions to import-actions. I think a lot of update-code is already written in bug 421886, we should attach the update-code to the importing and D&D. And of course we need additional UI for this, we should ask for replacing duplicates. Also, bug 449401 (seperate items of the same id across different calendars) should be regarded when working on the import-code I think.

Comment 9

[Daniel Boelzle [:dbo]]

Yes, Bas that's right and maybe what I over and over try to explain: The import feature we're supposed to head for actually needs to implement full iTIP, and thus needs to be massively enhanced compared to plain "take over the data with UID". That said, to me it's not just a bug, but actually a feature enhancement.

However, what I've asked initially is why this is dataloss. I still think the current feature incarnation doesn't cause dataloss and is sensible w.r.t. dropped iTIP messages.

@Günter: You shouldn't shout on every and each sentence, that doesn't help at all, but only leads to an aggressive atmosphere.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Seems like we should be able to base the behavior on whether or not the file is iTIP.  How about:

* for iTIP messages, we either error out or supply new IDs
* for non-iTIP messages we use the existing IDs

Comment 11

[Stefan Sitter [:ssitter]]

Looks like a duplicate of Bug 419334.

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

ssitter: agreed, but since there's been a bunch more discussion here, I'm going to mark that bug as a DUP of this one.