Merge pull request #141 from atorus-research/108-error-messages-for-spec_to_metacore

Closes #108 error message update for `spec_to_metacore()`

Author: LiamHobby

R/spec_builder.R

@@ -754,17 +754,27 @@ create_tbl <- function(doc, cols) {
     sheets_to_error <- mismatch_per_sheet %>%
       keep(names(.) %in% closest_sheets)
 
-    # Write out the error
-    sheets_to_error %>%
-      map2_chr(names(sheets_to_error), function(vars, sheet_name) {
-        paste0(
-          "Sheet '", sheet_name, "' is the closest match, but unable to match the following column(s)\n",
-          paste(names(vars), collapse = "\n")
-        )
-      }) %>%
-      paste0(collapse = "\n") %>%
-      paste0("Unable to identify a sheet with all columns.\n", .) %>%
-      (call. <- FALSE)
+    # 1. Check for "where" columns
+    has_where_col <- sheets_to_error %>%
+      map_lgl(~ any(stringr::str_detect(names(.x), stringr::regex("^where", ignore_case = TRUE)))) %>%
+      any()
+
+    # 2. Generate the formatted list of sheet matches
+    sheet_details <- sheets_to_error %>%
+      purrr::imap_chr(~ {
+        paste0("Sheet '", .y, "' is missing: ", paste(names(.x), collapse = ", "))
+      })
+
+    # 3. Use cli_abort with bullets to replace the standard Error: prefix
+    cli_abort(
+      c(
+        "x" = "Unable to identify a sheet with all columns.",
+        "i" = "Closest matches identified:",
+        "*" = sheet_details,
+        if (has_where_col) c("!" = "Tip: A 'where' column was detected. Check if {.arg where_sep_sheet} is set correctly.")
+      ),
+      call = NULL
+    )
   } else if (length(matches) == 1) {
     # Check names and write a better warning message if names don't work
     ds_nm <- matches[[1]] %>% names()

tests/testthat/test-reader.R

@@ -612,3 +612,36 @@ test_that("define_to_metacore(quiet) deprecation message is output when supplied
     spec <- define_to_metacore(metacore_example("ADaM_define_CDISC_pilot3.xml"), quiet = TRUE)
   )
 })
+
+test_that("Informative error when where_sep_sheet=TRUE but WhereClause sheet missing", {
+  # This should trigger the helpful error message about where_sep_sheet
+  expect_error(
+    spec_to_metacore(
+      "spec_no_val.xlsx", # Use relative path like the existing test
+      where_sep_sheet = TRUE, # This is the default, but being explicit
+      verbose = "silent"
+    ),
+    regexp = "where.*where_sep_sheet",
+    ignore.case = TRUE
+  )
+
+  # Verify the error message contains helpful context
+  err <- tryCatch(
+    spec_to_metacore(
+      "spec_no_val.xlsx",
+      where_sep_sheet = TRUE,
+      verbose = "silent"
+    ),
+    error = function(e) conditionMessage(e)
+  )
+
+  # Check that the error message mentions:
+  # 1. That columns couldn't be matched
+  expect_match(err, "Unable to identify a sheet|Could not find matching columns", ignore.case = TRUE)
+
+  # 2. Provides the helpful tip about where_sep_sheet
+  expect_match(err, "where_sep_sheet", ignore.case = TRUE)
+
+  # 3. Shows which sheet was closest
+  expect_match(err, "Sheet|Closest", ignore.case = TRUE)
+})