-
Notifications
You must be signed in to change notification settings - Fork 26
Description
The search_content() function currently exposes the page_number and page_size arguments, the latter of which maxes out at 500. Functionally, this means that it's not usable without wrapping it in a function that fetches all pages.
Considering two approaches.
Option 1: Use the current page_offset function for pagination. page_offset passes the page_number query param as a function argument, which demands that the endpoint is wrapped in a function that implements it (like the current search_content function).
With the current search_content function, this looks like so:
search_all_content <- function(client, q = NULL, include = "owner,vanity_url") {
page_offset(client, req = search_content(client, q = q, page_number = 1, page_size = 500, include = include))
}I don't actually propose the outer function be named search_all_content — it should be named search_content, and the inner wrapper should have a different name. Previously, those inner wrapping funcs have been methods on the Connect R6 class. We've been moving away from that. Maybe we want to name the inner function directly for the API path, e.g. v1_search_content().
Option 2: Use a different approach to paginating.
If we don't want to require an inner function wrapping the endpoint, we would need a different approach for pagination logic. If the pagination function didn't increment the page_number argument to an R call, but instead incremented that parameter in a list, we could iterate directly on the Connect$GET() call to the endpoint. This would be a little more work.