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

From: "Bron Gondwana" <[email-masked]>
Date: Mon, 13 Jul 2020 23:22:40 +1000

Ok, so I haven't read this thing yet! Figured I'd have a read through and note down things as they come up. This'll just be all the clarifications or things that I don't like because otherwise it would be a long email!

*1.1: Data Model
*

A CalendarPrincipal has a 1:1
   correspondence with an Account (see [RFC8620], Section 1.6.2) that
   supports the "urn:ietf:params:jmap:calendars" capability.

And then later CalendarPrincipal/query is defined as a way to scan through CalendarPrincipals efficiently. *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? This would be easier to extend for other data types that may be available from many accounts in the future.

*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.

*3. Calendars**
*

Likewise, should color be optional (default #000000 or whatever).

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

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. And probably ditto for everything else that doesn't have a default.

   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.

*3.1. Per-user properties**
*

We should address read-only accounts here, where users may not be able to store anything on the server.
*
*
*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.

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

*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, but I also don't like that compared to the client reading the defaults off the calendar and applying them itself - good old "be explicit in what you ask of the server" which is a continual tradeoff, but which I really like as a pattern because it reduces magicalness and action at a distance.

*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.

*5.8.1.  Patching**
*

Yeah, I guess that's the only way to do it! I'm unhappy about the patching story here in general and would like to discuss the "patch the model" vs "patch the representation" a bit more, but there's definitely edge cases to review either way. It's not so bad with recurrences because you can treat them as fully expanded for patching purposes, but it does get very messy with localisation!

*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?

...

This may not be everything yet, but these are the main points I found reading through!

Cheers,

Bron.

--
 Bron Gondwana, CEO, Fastmail Pty Ltd
 [email-masked]

---

From: "Neil Jenkins" <[email-masked]>
Date: Mon, 20 Jul 2020 14:18:47 +1000

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.

> 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…

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***

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.

> 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***
> 

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).

> 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).

> 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.)

> 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…

Sure, fine by me.

> 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.