Calendar:Architecture: Difference between revisions

** A more concrete example might be the alarmService, which only cares about changes to objects with alarm information.  If a user changes the start-time, the service will need to retrieve the object to learn if it had an alarm on it, and then determine whether that alarm is affected by the change. '''Re: If we understand your concern correctly, you are talking about how to find out differences between the old and the new item.  That's why we have just added a proposal about a different [[#Notifications|Notification]].''' '''jminta says:''' My general concern isn't just notifications (although this section helps a lot), but rather that a set of modified attributes doesn't always tell the whole story.  In simple terms: The day-view always needs to know start *and* end in order to make a decision on whether it should update.  The alarmService needs to know start *and* alarmOffset.  Transactions oftentimes will only provide 1 of these pieces of information, and it isn't clear how to obtain the other.

** deleteProperty("X-PROP1"). '''Re: The granularity of changes are the attributes of an item.  If ical properties have been changed in a transaction, all depending attributes have to be marked as ''modified''. If you reinitiliaze an item using the <code>icalString</code> attribute, the whole item, i.e. all of its attributes are marked as being ''modified''.''' '''jminta says:''' I don't think I was clear here.  My issue is that if a calITransaction is empty, you can't simply call deleteProperty("FOO") on it (like you could on a clone), because it doesn't have the property.  Furthermore, how would you distinguish between a property that has been deleted and a property whose value is simply blank?

** changing an event into a task - would the transaction implement calIToDo or calIEvent?  This requires that certain code areas that only care about one of these still need to watch other transactions. '''Re: Why would someone ever want to switch an event into a task or vice versa?''' '''jminta says:''' it's actually a pretty common request, bug 277731.

** adding an attendee - If a transaction contains a single attendee, how can a listener determine whether this is a modifcation of a previous attendee or an addition of a new one? '''Re: As stated above, we currently think it is sufficient, that the granularity is about attributes of an item. In this case, you would receive two different sets of attendees and have to diff out by yourself; well, as it is now.''' '''jminta says:''' Right now you don't have to diff them out, you can just ignore the old set and use the new one, since it's assured to be complete.  Diffing them out is much harder.

** Problematic(?) case: 'Modifications of all future occurrences' has recieved some attention lately.  If I want to perform this type of change, it seems I would need a ticket from the parentItem?  The semantics seem rather vague here regarding from whom I should ask for the ticket. '''Re: From our understanding of the ical standard, 4.2.13 says that this has to be specified as a property parameter of the <code>RECURRENCE-ID</code>. So the obvious approach would be to add this feature to proxy items.''' '''jminta says:''' maybe we need to leave recurrence for another discussion :-/

* It is my opinion that the second cloning in modifyItem is a bug that we can probably fix by enforcing that only mutable items be passed in to modifyItem.  (I don't think anyone ever even tries to pass an an immutable one.)  In that case, we have the same issue in both models, one XPCOM object is required to be created for each change. '''Re: Since cloning items is a deep copy (remember to clone all exceptions etc.), this is not true. And moreover, the new approach does not require ''any'' cloning. It just creates a cheap transaction object.''' '''jminta says:''' OK, this makes some sense.

* The listener's currently passed in for making modifications, at least in theory, ought to be told when errors occur and what kind of error it was.  This is advantageous, in my opinion, and if this new model were to be adopted, I would like to see commitTransaction accept a listener argument as well. '''Re: Agreed, has been omitted for brevity.''' '''jminta says:''' OK, as long as it's there in the final form.

* This model puts a lot of stress on the event-dialog.  It seems to need to figure out exactly what changed, rather than being able to simply rebuild the object from scratch and re-submit it. '''Re: Where is the difference to the current model? Currently we are creating and writing a new event reading out the whole dialog, and with the new proposal, there would be no code change; just using a transaction object. But if we like, we need only commit changes to UI elements that have actually changed. Being efficient for the first time.''' '''jminta says:''' My point was that I don't see how you can *not* only submit the changed fields.  Figuring out which fields changed isn't trivial.

* This model needs to be '''absolutely certain''' that it has iron-clad enforcement of the rule that it's impossible to change an item other than by a transaction.  Otherwise, I'm very scared of how easily a novice extension writer could completely break this model by following the intuitive idea of coding <code>item.title = "new title"</code>. '''Re: Items will be immutable in the new approach. So this is no problem.''' '''jminta says:''' Good.  Just be certain.

* Finally, <code>calITransaction</code> will run into problems in truly implementing <code>calIItemBase</code>.  Certain properties of <code>calIItemBase</code> depend on certain other properties being present.  In the atomic model, this is not assured.  The most critical example of this is <code>icalString</code>.  Currently, the ics provider can be certain that it has all the information about an event if it calls icalString.  However, calling icalString on a transaction will likely throw an error.  Providers will need to be very smart in order to assure that they have all of the changes truly included in a transaction. '''Re: There seems to be a misunderstanding. A transaction object is not supposed to be passed around, it usually lives only for short time, minimizing the risk to fail upon <code>commitTransaction()</code> (becoming inconsistent). After a transaction object has been committed, it must not be used anymore, leading to an error.''' '''jminta says:''' My point was merely that you can't truly implement an interface if you don't support everything that interface does, ie. <code>icalString</code> for <code>calIItemBase</code> or <code>duration</code> for <code>calIEvent</code>.  If you implement an interface, consumers ought to be able to count on being able to use the properties of that interface.