Add some additional checks to row_filter

R/parse_row_filters.R

@@ -1,37 +1,35 @@
 #' Create JSON 'dict' from named list or vector
 #' @description Formats a list or named vector into a valid query
-#' @param row_filters list or named vectors matching fileds to values
+#' @param row_filters list or named vectors matching fields to values
 #' @return a json as a character string
 parse_row_filters <- function(row_filters) {
   # exit function if no filters
   if (is.null(row_filters)) {
     return(NULL)
   }
 
+  if (!inherits(row_filters, "list")) {
+    cli::cli_abort("{.arg row_filters} must be a {.cls list}, not a {.cls {class(row_filters)}}.")
+  }
+
   # check if any filters in list have length > 1
   too_many <- sapply(row_filters, length) > 1
 
   if (any(too_many)) {
     cli::cli_abort(c(
-      "Invalid input for {.var row_filters}",
-      i = "{names(row_filters)[which(too_many)]} in {.var row_filters} has too many values. ",
-      x = "The {.var row_filters} list must only contain vectors of length 1."
+      "Invalid input for {.arg row_filters}",
+      i = "The {.val {names(row_filters)[which(too_many)]}} filter{?s} {?has/have} too many values.",
+      x = "The {.arg row_filters} list must only contain vectors of length 1."
     ))
   }
 
-  # check if any items in the list/vector have the same name
-  # find number of unique names
-  n_u_row_filters <- length(unique(names(row_filters)))
-  # find total number of names
-  n_row_filters <- length(names(row_filters))
-  # if same, all names are unique
-  unique_names <- n_u_row_filters == n_row_filters
-
-  if (!unique_names) {
+  # check if any items in the list/vector are duplicates
+  duplicates <- duplicated(names(row_filters))
+  if (any(duplicates)) {
     cli::cli_abort(c(
-      "Invalid input for {.var row_filters}",
-      x = "One or more elements in {.var row_filters} have the same name.",
-      i = "Only one filter per field is currently supported by `get_resource`."
+      "Invalid input for {.arg row_filters}",
+      x = "The {.val {names(row_filters)[which(duplicates)]}} filter{?s} {?is/are} duplicated.",
+      i = "Only one filter per field is currently supported by {.fun get_resource}."
     ))
   }
 

tests/testthat/test-get_resource_dump.R

@@ -15,7 +15,7 @@ test_that("returns full data if rows is set to over 99999", {
     data <- get_resource(
       res_id = gp_list_apr_2021,
       rows = 9999999,
-      row_filters = c("GPPracticeName" = "The Blue Practice")
+      row_filters = list("GPPracticeName" = "The Blue Practice")
     ),
     regexp = "Can't request over 99,999 rows"
   )

tests/testthat/test-parse_row_filters.R

@@ -14,7 +14,7 @@ test_that("throws error for length > 1", {
 test_that("throws error for non-unique names", {
   expect_error(
     parse_row_filters(list(x = 1, x = 2)),
-    regexp = "Only one filter per field is currently supported by `get_resource`"
+    regexp = "Only one filter per field is currently supported"
   )
 })