Added search functionality to get_publications() #67
Conversation
| page_size = page_size, | ||
| page = page, | ||
| verbose = verbose | ||
| ) | ||
| ) |> | ||
| httr::content("text") |> | ||
| jsonlite::fromJSON() | ||
| if (!is.null(search)) { |
There was a problem hiding this comment.
Might be more general for the function as a whole rather than search, but could we add a nice message to return if there were no rows found? (Guessing it might be a lot more common if people are trying to search)
| @@ -10,6 +10,7 @@ | |||
| get_publications <- function( | |||
There was a problem hiding this comment.
Could we add a search example into the examples for roxygen2
| @@ -14,6 +14,7 @@ | |||
| #' | |||
| #' @param endpoint Name of endpoint, can be "get-publications", "get-data-catalogue", | |||
| #' "get-summary", "get-meta", "get-csv", "get-data" or "post-data" | |||
| #' @param search String for filtering publication list | |||
There was a problem hiding this comment.
Worth specifying that it filters by the publication title?
There was a problem hiding this comment.
Can we change the first test in this script to just check it returns more than 1 row now? Would be good to move away from it being exactly matching the live data as that will be hard to track over time.
Alternative, maybe we set it up against somewhere like test explicitly and then check that specific test data files we've added are in there or something?
| @@ -124,7 +126,12 @@ api_url <- function( | |||
| url <- paste0( | |||
| endpoint_base_version, | |||
| "publications?", | |||
| api_url_pages(page_size = page_size, page = page) | |||
| api_url_pages(page_size = page_size, page = page), | |||
There was a problem hiding this comment.
Should we handle the conversion of spaces?
| test_that("Search works as expected", { | ||
| expect_equal( | ||
| get_publications(search = "API") |> | ||
| dplyr::filter(!grepl("API", title)) |> | ||
| nrow(), | ||
| 0 | ||
| ) | ||
| }) |
There was a problem hiding this comment.
This test would also pass if the API call returned no rows (without testing the search at all), are we okay with that?
Brief overview of changes
Added the search parameter as an option in get_publications()
Why are these changes being made?
Missing functionality.
Detailed description of changes
Just changes in
get_publications()andapi_url()to add in the search parameter, plus some validation and testing.Additional information for reviewers
Issue ticket number/s and link
Closes #61