Adapt behaviour of type argument (closes #53)

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

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

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
 

tests/testthat/test-assets.R

@@ -1,30 +1,40 @@
 # 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({
 
   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"
     )
-
   })
 
 })