Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure to handle a PROPFIND to / #126

Merged
merged 2 commits into from
Jul 6, 2023

Conversation

cheif
Copy link
Contributor

@cheif cheif commented Jul 5, 2023

It seems like the Reminders app in iOS/macOS does this request as the first thing when setting up an account, so it seems reasonable to handle it for us.

This just returns the most basic current-user-principal now, but that should hopefully be enough to continue the process.

It seems like the Reminders app in `iOS`/`macOS` does this request as
the first thing when setting up an account, so it seems reasonable to
handle it for us. This just returns the most basic
current-user-principal now, but that should hopefully be enough to
continue the process.
@emersion
Copy link
Owner

emersion commented Jul 6, 2023

It sounds reasonable to serve a default user principal when the library user doesn't handle that case.

  • Can we use webdav.ServePrincipal? This would reduce code duplication.
  • Can we do the same for CardDAV?

@emersion
Copy link
Owner

emersion commented Jul 6, 2023

Hm, actually, no, this is the root, not the user principal… Nevermind about using webdav.ServePrincipal.

@cheif
Copy link
Contributor Author

cheif commented Jul 6, 2023

Yeah, I'm not competent enough in *Dav to know what the most reasonable approach is here.

I just assume that it's proper to respond to the <current-user-principal> request no matter what the path is.

@emersion
Copy link
Owner

emersion commented Jul 6, 2023

Okay. Can we copy-paste propFindUserPrincipal over to a new propFindRoot function which only returns the current user principal prop?

Also I think the CurrentUserPrincipal call can be dropped, its result is unused.

@cheif
Copy link
Contributor Author

cheif commented Jul 6, 2023

@emersion Done! 😄

Copy link
Owner

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@emersion emersion merged commit 7dd6490 into emersion:master Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants