Re: [Jmap] Review: draft-ietf-jmap-calendars-03

[Neil Jenkins <[e-mail]>]

On Mon, 13 Jul 2020, at 23:22, Bron Gondwana wrote:
> *Why do this rather than defining properties on the Account object directly and then adding an Account/query mechanism *which allows you to filter to accounts which support "urn:ietf:params:jmap:calendars" as part of the query parameters?

For several reasons, including:
 1. You may have access to more than one account containing calendar principals. These are separate namespaces and need to remain so. Making it all part of one thing would break this, making it much harder to transparently proxy different backends via a single JMAP endpoint.
 2. You may not have access to the account (you are only allowed to call getAvailability); so you can't represent the principal as an account.
 3. The standard get/changes etc. methods have consistency guarantees tied to an account. This would be undefined, or require redefining if you needed to do it for querying accounts, which makes the spec more complex. These guarantees may not be possible to make when the set of accounts actually comes from multiple different backends.

> *2. Calendar Principals*
> 
> All the object properties appear to be required with no default value.  Should they have default values given (generaly null) in the spec?  Particularly where null is a valid value, it appears that would be a sensible default.

Since you can't create a principal via this spec, it's unclear to me what a default would mean for these properties.

> *3. Calendars***
> 
> Likewise, should color be optional (default #000000 or whatever).

I'll make it nullable and that can be the default.

> Speaking of color - I believe that the CSS color specs should be normative references, since you can't build a compliant implementation without them.

I'm not sure I can do this in the markdown source; this will probably need fixing up at the end when I do final edits in the XML. I'll make sure we do this though.

> Likewise isSubscribed should probably default to true if not specified, since a server which doesn't care about subscriptions would just return the calendars that you can see..  

It's a bit more nuanced than that. The spec says:

*This SHOULD default to false for Calendars in shared accounts the user has access to and true for any new Calendars created by the user themself.*

> And probably ditto for everything else that doesn't have a default.

I've added in defaults for all other calendar properties where it makes sense.

>    o  *timeZone*: "String|null" The time zone to use for events without
      a time zone when the server needs to resolve them into absolute
      time, e.g., for reminders, queries, or availability calculation.
      The value MUST be a time zone id from the IANA Time Zone Database.
      If "null", the timeZone of the account's associated
      CalendarPrincipal will be used.  Clients SHOULD use this as the
      default for new events in this calendar if set.
> 
> The IANA Time Zone Database should be in the references at the end of the document.

Same as with the CSS ref; I've put a link in and will make sure it's a normative reference in the final edit.

> *3.1. Per-user properties***
> 
> We should address read-only accounts here, where users may not be able to store anything on the server.

I've taken out this section because it's covered in more detail in the Calendar/set description (the user must be subscribed to the calendar in order to change any of these properties for themself, and the server may reject them subscribing themself even if they can otherwise see and access it, thus enforcing a read-only mode).

> *4.5.  CalendarShareNotification/set*

   This is a standard "/changes" method as described in [RFC8620],
   Section 5.3.
> 
> This looks like a copy-paste error from above.

Thanks, fixed.

> It looks from the lack of create/update that "undo" will not work when dismissing notifications!

Correct. I considered this when defining it and I think it's OK; I've yet to see a client that has undo for dismissing a notification like this, and it makes server implementation considerably easier (you don't want clients being able to create bogus notifications; if they can create anything it would have to be something identical to the one they dismissed and that gets icky).

> *5. Calendar Events*
> 
> You know my opinion on this!  I think events should be able to be present in mutliple Calendars, so calendarIds becomes a map of { "id" : true, "id2": true } instead of a single value.
> 
> Of course that doesn't map well with copying stuff out of the "defaults" fields of the calendar into the event when it's created

Nothing is copied on create. Inheritance from the calendar is live. e.g. If you set the event to useDefaultAlerts then change the default alerts on the calendar it's in, the new defaults now apply to the event. Inheriting this from multiple calendars would just be a mess (same for time zone for floating events etc.)

> *5.8.  CalendarEvent/set*
> 
>    o  *sendSchedulingMessages*: "Boolean" (default: true) If true then
      any changes to scheduled events will be sent to all the
      participants (if the user is an owner of the event) or back to the
      owners (otherwise).  If false, the changes only affect this
      calendar and no scheduling messages will be sent.
> 
> Speaking of which - I believe the default should be false here.  Clients can easily be built to automatically add it, and the wording needs to say that it's not an error to set this even if none of the events are scheduling events... but things should default to less impactful and the client should ask for impacts - and sending scheduling messages is an impact.  The ICALENDAR default is the wrong one, and we should fix that here.

Sure, fine by me.

> *9. IANA Considerations*
> 
> There are a few fields which have been specified as an enumeration, e.g. Calendar/role, and "Per User Properties" in JSEvent.  Should there be registries created for these?

Yes, I think there should.

Neil.