caldav: add support for more props on MKCOL and PROPFIND #150
Conversation
|
@emersion please review |
DeepDiver1975
force-pushed
the
feat/create-calendar-props
branch
from
36d2981
to
c2ad716
Compare
DeepDiver1975
changed the title
|
@emersion how can we get this moving? THX |
caldav/server_test.go
Outdated
| "fmt" | ||
| "github.com/emersion/go-webdav/internal" | ||
| "github.com/stretchr/testify/assert" |
There was a problem hiding this comment.
I'd prefer to not add the testify dependency.
There was a problem hiding this comment.
Testify allows to write a bit more compact test code.
Which improves readability.
There was a problem hiding this comment.
It's a whole new library to learn. I prefer to write tests like the Go standard library does. Also, compact ≠ readable (quite the contrary in general).
| calendarName = xml.Name{namespace, "calendar"} | ||
| calendarDataName = xml.Name{namespace, "calendar-data"} | ||
| calendarColorName = xml.Name{ | ||
| Space: "http://apple.com/ns/ical/", |
There was a problem hiding this comment.
The standard way to set the color of a calendar is: https://datatracker.ietf.org/doc/html/rfc7986#section-5.9
This is a non-standard, Apple-specfic property. I'm not sure I want to support it.
There was a problem hiding this comment.
I never saw this one in real life. But happy to add this as well.
There was a problem hiding this comment.
Just adding my 2 cents: I have indeed also observed the proliferation of this pseudo-standard (apple.com/ns/ical) into many open-source clients (e.g. DavX5 sets it on MKCOL). On the other hand, from my experience, after initial calendar creation, this is treated as a pure client-side property by pretty much all clients (i.e. it never gets updated again), so not sure how necessary it really is...
There was a problem hiding this comment.
so not sure how necessary it really is...
agreed - this is for sure a pretty low feature.
Never the less if the color is stored server side it will be "synced" once opening the calendar with another client which has some benefits of recognizing calendars.
e.g. on the owncloud 10 codebase (php+sabredav) colors are the same on all clients (owncloud calendar app in web, DAVx and iOS)
There was a problem hiding this comment.
Hm, actually it doesn't seem like there is any standard way to fetch a VCALENDAR for the calendar itself in CalDAV. So RFC 7986 is only useful to set the calendar color when exporting a whole calendar as .ics.
Oh well, let's just do like everybody else then…
caldav/elements.go
Outdated
| DisplayName string `xml:"set>prop>displayname"` | ||
| Description string `xml:"set>prop>calendar-description"` | ||
| CalendarColor string `xml:"set>prop>calendar-color"` | ||
| CalemdarTimeZone string `xml:"set>prop>calendar-timezone"` |
|
Some more general notes:
|
DeepDiver1975
force-pushed
the
feat/create-calendar-props
branch
from
89545b3
to
46d5e86
Compare
DeepDiver1975
changed the title
DeepDiver1975
force-pushed
the
feat/create-calendar-props
branch
from
46d5e86
to
96422a0
Compare
DeepDiver1975
force-pushed
the
feat/create-calendar-props
branch
from
96422a0
to
ab37619
Compare
|
@emersion @bitfehler please review again - did not yet change the color prop name ... as explained above this is the property I see in real life .... 🤷 |
| ResourceType internal.ResourceType `xml:"set>prop>resourcetype"` | ||
| DisplayName string `xml:"set>prop>displayname"` | ||
| Description string `xml:"set>prop>calendar-description"` | ||
| CalendarColor string `xml:"set>prop>calendar-color"` |
There was a problem hiding this comment.
We should probably specify the http://apple.com/ns/ical/ namespace for this prop?
| MaxResourceSize int64 | ||
| SupportedComponentSet []string | ||
| Timezone string |
There was a problem hiding this comment.
This should probably be a parsed *ical.Calendar?
We should probably check that the parsed value contains exactly one VTIMEZONE component?
Todo: