Fetching Calendly Events

[cansavvy]

Summary

In this PR, we fetch Calendly events listings!
It also involves some retrieving next pages so I built some pagination handlers. I might try to make this a more universal function for the other APIs that have pagination to handle. But one thing at a time.

Usage

The culmination of this PR is that you should be able to do something like this:

authorize("calendly")
user <- get_calendly_user()
events <- list_calendly_events(user = user$resource$uri)

And the events object is a list that has events that have ever been scheduled for the user.

[cansavvy]

I've now added a main function to handle GET requests. It is called by these other Calendly GET functions.

[howardbaik]

Looking good! Both functions get_calendly_user() and list_calendly_events() work fairly well, if the arguments are fixed.

I should note that I haven't been able to test list_calendly_events() for more than 100 events, since my Calendly account only has 4 events.

[howardbaik]

Suggested change

	get_calendly_user <- function(api_key) {
	get_calendly_user <- function(api_key = NULL) {

The function needs a default of NULL for the example on line 56 to work.

[cansavvy]

Added NULLs 👍

[howardbaik]

Suggested change

	if (is.null(api_key)) {
	# Get auth token
	token <- get_token(app_name = "calendly")
	} else {
	token <- api_key
	}
	if (is.null(api_key)) {
	# Get auth token
	token <- get_token(app_name = "calendly")
	message("Using user-supplied token stored using authorize(\"calendly\")")
	} else {
	token <- api_key
	}

To improve the API design, I suggest adding a message telling the user that we will be using the user-supplied token from authorize("calendly"). Kudos to Hadley's R Packages Workshop.

[cansavvy]

Excellent idea. Yes this whole package needs a lot more error handling and messaging improvements.

[cansavvy]

Added messages to warn users 👍

[howardbaik]

Confused with the argument api_key as we are technically grabbing the personal access token. What about using token as an argument instead?

Suggested change

	#' @param api_key You can provide the API key directly or this function will attempt to grab an API key that was stored using the `authorize("calendly")` function
	#' @param token You can provide the token directly or this function will attempt to grab the token that was stored using the `authorize("calendly")` function

[cansavvy]

I have been inconsistent in this. At some times it's an api key and sometimes it's a token. I've been realizing this as I've been writing but I've not yet fixed it. Because you've noticed I'll make sure to fix it now lol

[cansavvy]

Made it more consistent that we always refer to it as a "token" instead of an "api_key"

[howardbaik]

I think you forgot to add api_key as an argument:

Suggested change

	list_calendly_events <- function(user, count = 100) {
	list_calendly_events <- function(api_key, user, count = 100) {

[cansavvy]

Added this missing argument.

[cansavvy]

Toward the end of getting this package pushed through, I'm not going to wait for @howardbaek to re-review to merge this since his first review didn't have major qualms and I addressed his minor qualms.