diff --git a/DESCRIPTION b/DESCRIPTION index a324972..8c4f954 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -16,6 +16,7 @@ Imports: curl, dplyr (>= 1.1.4), httr2, + lifecycle, magrittr, rlang, rstudioapi, @@ -34,4 +35,4 @@ VignetteBuilder: Config/testthat/edition: 3 Encoding: UTF-8 Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.2 +RoxygenNote: 7.3.3 diff --git a/R/assets.R b/R/assets.R index 865fa33..b573d40 100644 --- a/R/assets.R +++ b/R/assets.R @@ -7,14 +7,8 @@ # nolint end #' #' @param workspace_uuid UUID of workspace -#' @param type List of asset types to return. Default returns all types; -#' "document", "folder" and "link". -#' -#' List must be empty (default, returns all asset types), or length 1 (e.g. -#' `list("document")`). This is a temporary measure while a bug in the -#' underlying API is outstanding (see -#' [objr#53](https://github.com/ScotGovAnalysis/objr/issues/53)). -#' +#' @param type Asset type to filter results by. Either "document", "folder" or +#' "link". Default returns all asset types. #' @inheritParams objr #' @inheritParams workspaces #' @@ -25,35 +19,38 @@ #' @export assets <- function(workspace_uuid, - type = list(), + type = NULL, page = NULL, size = NULL, use_proxy = FALSE) { - check_list(type) - - if (length(type) > 1) { - cli::cli_abort(c( - "x" = "{.arg type} must be a list of length 0 or 1, not {length(type)}.", - "i" = paste( - "There is currently a bug in the underlying API preventing", - "users from selecting more than one asset type. See", - "{.href [objr#53](https://github.com/ScotGovAnalysis/objr/issues/53)}", - "for more information." - ), - "i" = paste("To return all assets, use `type = list()` (default)."), - "i" = paste("To return one asset type only, e.g. documents, use", - "`type = list(\"document\")`. See `?assets` for all", - "options.") - )) + if (rlang::is_list(type)) { + lifecycle::deprecate_warn( + when = "0.2.0", + what = "assets(type = 'must be a character string')", + details = "All asset types are returned.", + always = TRUE + ) + type <- NULL } - type <- paste(toupper(type), collapse = "|") + if (!is.null(type)) { + if (!rlang::is_string(type)) { + cli::cli_abort( + "`type` must be a string, length 1." + ) + } + if (!type %in% c("document", "folder", "link")) { + cli::cli_abort( + "`type` must be one of {.str document}, {.str folder} or {.str link}." + ) + } + } response <- objr( endpoint = "assets", url_query = list(workspaceUuid = workspace_uuid, - type = type, + type = toupper(type), page = page, size = size), use_proxy = use_proxy diff --git a/R/utils.R b/R/utils.R index 03db689..be920e4 100644 --- a/R/utils.R +++ b/R/utils.R @@ -183,9 +183,7 @@ random_uuid <- function(seed = NULL) { #' #' @return `uuid`, invisibly. #' -#' @details See the [Getting Started vignette] -#' (https://scotgovanalysis.github.io/objr/ -#' articles/objr.html#universally-unique-identifiers) for valid UUID format. +#' @details See the [Getting Started vignette](https://scotgovanalysis.github.io/objr/articles/objr.html#universally-unique-identifiers) for valid UUID format. # nolint: line_length_linter. #' #' @noRd diff --git a/man/assets.Rd b/man/assets.Rd index 7b04c02..5326a17 100644 --- a/man/assets.Rd +++ b/man/assets.Rd @@ -6,7 +6,7 @@ \usage{ assets( workspace_uuid, - type = list(), + type = NULL, page = NULL, size = NULL, use_proxy = FALSE @@ -15,13 +15,8 @@ assets( \arguments{ \item{workspace_uuid}{UUID of workspace} -\item{type}{List of asset types to return. Default returns all types; -"document", "folder" and "link". - -List must be empty (default, returns all asset types), or length 1 (e.g. -\code{list("document")}). This is a temporary measure while a bug in the -underlying API is outstanding (see -\href{https://github.com/ScotGovAnalysis/objr/issues/53}{objr#53}).} +\item{type}{Asset type to filter results by. Either "document", "folder" or +"link". Default returns all asset types.} \item{page}{Page number of responses to return (0..N). Default is 0.} diff --git a/tests/testthat/api/assets-90958f.R b/tests/testthat/api/assets-891c20.R similarity index 100% rename from tests/testthat/api/assets-90958f.R rename to tests/testthat/api/assets-891c20.R diff --git a/tests/testthat/test-assets.R b/tests/testthat/test-assets.R index 0eadd87..5cdeac8 100644 --- a/tests/testthat/test-assets.R +++ b/tests/testthat/test-assets.R @@ -1,8 +1,10 @@ # assets ---- -test_that("Error if `type` length > 1", { +test_that("Error if `type` invalid", { expect_error(assets(workspace_uuid = "test_workspace", - type = list("document", "link"))) + type = c("document", "link"))) + expect_error(assets(workspace_uuid = "test_workspace", + type = "x")) }) without_internet({ @@ -10,21 +12,29 @@ without_internet({ test_that("Valid request created", { expect_GET( - assets(workspace_uuid = "test_workspace", type = list()), + assets(workspace_uuid = "test_workspace"), paste0("https://secure.objectiveconnect.co.uk/publicapi/1/assets?", - "workspaceUuid=test_workspace&type=") + "workspaceUuid=test_workspace") ) expect_GET( assets(workspace_uuid = "test_workspace", - type = list("folder")), + type = "folder"), paste0("https://secure.objectiveconnect.co.uk/publicapi/1/assets?", "workspaceUuid=test_workspace&type=FOLDER") ) - expect_error( - assets(workspace_uuid = "test_workspace", - type = "folder") + }) + + test_that("Deprecated `type` list format handled correctly", { + + expect_GET( + suppressWarnings( + assets(workspace_uuid = "test_workspace", + type = list("folder")) + ), + paste0("https://secure.objectiveconnect.co.uk/publicapi/1/assets?", + "workspaceUuid=test_workspace") ) }) @@ -43,15 +53,13 @@ with_mock_api({ }) test_that("Results filtered by type", { - expect_equal( unique( assets(workspace_uuid = "test_workspace_uuid", - type = list("folder"))$asset_type + type = "folder")$asset_type ), "FOLDER" ) - }) })