Accept dataset titles in get_dataset()

[sbarnesphs]

Closes #80

• add resolve_dataset_name(): resolves an exact dataset title to its name, warns on resolution, errors on ambiguous or unmatched titles
• call resolve_dataset_name() in get_dataset() prior to name validation
• document title support in get_dataset() roxygen (inherited by get_latest_resource())
• add mocked unit tests covering pass-through, resolution, ambiguity and no-match scenarios
• add integration test for get_dataset() title resolution
• change get_dataset() malformed-name test from Mal-formed-name to -bad-name-, as the former is now handled upstream by resolve_dataset_name()
• add NEWS entry for dataset title support

[Moohan]

I noticed that the documentation for dataset_name has changed in a number of places, this is because we 'inherit' the docs. I just want to make sure that the parameter will work the same in all the locations, as is now stated in the docs?

[Moohan]

I think these need doing first (in get_dataset, or a helper), as they should all throw errors, R/check_dataset_name.R partially does this but also does the regexp check.

I think the logic flow should be:

get_dataset(...) {

# Check input is valid - using error message currently in `check_dataset_name()`

# Check if it looks like a name using the regex in R/check_dataset_name.R `"^[a-z0-9][a-z0-9\\-]+?[a-z0-9]$"` (possibly could be simplified to "^[0-9][a-z0-9\\-]+?[a-z0-9]$" if names can't start with a number?)

# If it looks like a name, use the existing flow to execute the query, if not use your new title flow.

# If the name didn't match a real dataset, or your title search didn't find something, use a modified version of `suggest_dataset_name` to suggest both titles and names, guiding the user to use a name.

[Moohan]

We discussed turning this into a helper function, as it's also used pretty much identically in list_resources, that will be sufficient for this PR and we can look at potentially caching in the future.

[Moohan]

We could be super defensive and have a warning if length(results) == 10000 i.e. it looks like the results were probably truncated.

[Moohan]

Some additional tests for your perusal, courtesy of GitHub Copilot

# Additional test cases for resolve_dataset_name()
# These tests cover edge cases and improve branch coverage to ensure robustness

# Helper function (matches the one in main test file for consistency)
mock_package_search <- function(...) {
  list(
    result = list(
      results = list(...)
    )
  )
}

# ============================================================================
# Edge Case 1: Input with special regex characters (parentheses, brackets, etc.)
# This ensures fixed = TRUE in the substring search prevents regex interpretation
# ============================================================================

test_that("resolve_dataset_name() handles titles with special characters in substring search", {
  mock_content <- mock_package_search(
    list(
      title = "Dataset (beta) v2.0",
      name = "dataset-beta-v2"
    ),
    list(
      title = "Hospital [Public] Data",
      name = "hospital-public-data"
    ),
    list(
      title = "Normal Dataset Name",
      name = "normal-dataset-name"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      expect_identical(action, "package_search")
      expect_identical(query$q, "*:*")
      expect_identical(query$rows, 10000)
      mock_content
    }
  )

  # Search for "(beta)" substring should find it literally, not as regex
  err <- rlang::catch_cnd(
    resolve_dataset_name("(beta)"),
    classes = "error"
  )
  expect_s3_class(err, "rlang_error")
  expect_match(conditionMessage(err), "did not match any dataset title exactly")
  expect_match(conditionMessage(err), "dataset-beta-v2")
})

# ============================================================================
# Edge Case 2: Empty string input (should fail gracefully)
# ============================================================================

test_that("resolve_dataset_name() errors on empty string with no candidates", {
  mock_content <- mock_package_search(
    list(
      title = "GP Practice Populations",
      name = "gp-practice-populations"
    ),
    list(
      title = "Cancelled Operations",
      name = "cancelled-operations"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  err <- rlang::catch_cnd(
    resolve_dataset_name(""),
    classes = "error"
  )

  expect_s3_class(err, "rlang_error")
  expect_match(conditionMessage(err), "did not match any dataset title exactly")
  expect_match(conditionMessage(err), "Candidates: none")
})

# ============================================================================
# Edge Case 3: All digits input (passes name-like regex, doesn't trigger API)
# ============================================================================

test_that("resolve_dataset_name() passes all-digits input through unchanged", {
  testthat::local_mocked_bindings(
    phs_GET = function(...) {
      stop("phs_GET() should not be called for digit-only input")
    }
  )

  expect_identical(
    resolve_dataset_name("123456"),
    "123456"
  )
})

# ============================================================================
# Edge Case 4: All hyphens + digits (edge of name-like regex)
# ============================================================================

test_that("resolve_dataset_name() passes hyphen-digit combinations through unchanged", {
  testthat::local_mocked_bindings(
    phs_GET = function(...) {
      stop("phs_GET() should not be called for name-like input")
    }
  )

  expect_identical(
    resolve_dataset_name("123-456-789"),
    "123-456-789"
  )

  expect_identical(
    resolve_dataset_name("-"),
    "-"
  )

  expect_identical(
    resolve_dataset_name("a-b-c-1-2-3"),
    "a-b-c-1-2-3"
  )
})

# ============================================================================
# Edge Case 5: Case variations in ambiguous title matching
# ============================================================================

test_that("resolve_dataset_name() detects case-insensitive ambiguity", {
  mock_content <- mock_package_search(
    list(
      title = "Covid Data",
      name = "covid-data-historical"
    ),
    list(
      title = "COVID DATA",
      name = "covid-data-current"
    ),
    list(
      title = "COVID data",
      name = "covid-data-alternate"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  err <- rlang::catch_cnd(
    resolve_dataset_name("COVID Data"),
    classes = "error"
  )

  expect_s3_class(err, "rlang_error")
  expect_match(conditionMessage(err), "matched more than one dataset")
  # Verify all three candidates are shown
  expect_match(conditionMessage(err), "covid-data-historical")
  expect_match(conditionMessage(err), "covid-data-current")
  expect_match(conditionMessage(err), "covid-data-alternate")
})

# ============================================================================
# Edge Case 6: Whitespace-only input (passes single string check, should error)
# ============================================================================

test_that("resolve_dataset_name() errors on whitespace-only input", {
  mock_content <- mock_package_search(
    list(
      title = "GP Practice Populations",
      name = "gp-practice-populations"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  err <- rlang::catch_cnd(
    resolve_dataset_name("   "),
    classes = "error"
  )

  expect_s3_class(err, "rlang_error")
  expect_match(conditionMessage(err), "did not match any dataset title exactly")
})

# ============================================================================
# Edge Case 7: Multiple partial matches (substring search returns >1)
# Verifies error message includes all candidates
# ============================================================================

test_that("resolve_dataset_name() lists all substring candidates", {
  mock_content <- mock_package_search(
    list(
      title = "GP Practice Populations",
      name = "gp-practice-populations"
    ),
    list(
      title = "GP Practice Contact Details",
      name = "gp-practice-contact-details"
    ),
    list(
      title = "Hospital Practice Guidelines",
      name = "hospital-practice-guidelines"
    ),
    list(
      title = "Primary Practice Standards",
      name = "primary-practice-standards"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  err <- rlang::catch_cnd(
    resolve_dataset_name("Practice"),
    classes = "error"
  )

  expect_s3_class(err, "rlang_error")
  expect_match(conditionMessage(err), "did not match any dataset title exactly")
  # All three "Practice" matches should appear
  expect_match(conditionMessage(err), "gp-practice-populations")
  expect_match(conditionMessage(err), "gp-practice-contact-details")
  expect_match(conditionMessage(err), "hospital-practice-guidelines")
  expect_match(conditionMessage(err), "primary-practice-standards")
})

# ============================================================================
# Edge Case 8: Very long dataset name input
# Ensures string operations handle long inputs gracefully
# ============================================================================

test_that("resolve_dataset_name() handles very long input strings", {
  long_input <- paste(rep("very-long-name", 100), collapse = "-")

  testthat::local_mocked_bindings(
    phs_GET = function(...) {
      stop("phs_GET() should not be called for name-like input")
    }
  )

  # Long name-like input should pass through unchanged
  result <- resolve_dataset_name(long_input)
  expect_identical(result, long_input)
  expect_type(result, "character")
})

# ============================================================================
# Edge Case 9: Single character valid title (edge of heuristic)
# ============================================================================

test_that("resolve_dataset_name() passes single-character name-like input through", {
  testthat::local_mocked_bindings(
    phs_GET = function(...) {
      stop("phs_GET() should not be called for name-like input")
    }
  )

  expect_identical(
    resolve_dataset_name("a"),
    "a"
  )

  expect_identical(
    resolve_dataset_name("z"),
    "z"
  )

  expect_identical(
    resolve_dataset_name("0"),
    "0"
  )
})

# ============================================================================
# Edge Case 10: Uppercase input requiring API search (not name-like)
# Verifies the case-insensitive matching works for various input cases
# ============================================================================

test_that("resolve_dataset_name() resolves uppercase input", {
  mock_content <- mock_package_search(
    list(
      title = "GP Practice Populations",
      name = "gp-practice-populations"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  expect_warning(
    out <- resolve_dataset_name("GP PRACTICE POPULATIONS"),
    "resolved to name"
  )
  expect_identical(out, "gp-practice-populations")
})

# ============================================================================
# Edge Case 11: Input with internal uppercase (CamelCase, mixed case)
# ============================================================================

test_that("resolve_dataset_name() resolves mixed-case input", {
  mock_content <- mock_package_search(
    list(
      title = "Hospital Admissions Data",
      name = "hospital-admissions-data"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  expect_warning(
    out <- resolve_dataset_name("HoSpItAl AdMiSsIoNs DaTa"),
    "resolved to name"
  )
  expect_identical(out, "hospital-admissions-data")
})

# ============================================================================
# Edge Case 12: Leading/trailing whitespace in input
# ============================================================================

test_that("resolve_dataset_name() handles leading/trailing whitespace", {
  mock_content <- mock_package_search(
    list(
      title = "GP Practice Populations",
      name = "gp-practice-populations"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  # Note: This will NOT match due to whitespace difference
  # The function performs exact matching, so " GP Practice Populations "
  # will not match "GP Practice Populations"
  err <- rlang::catch_cnd(
    resolve_dataset_name("  GP Practice Populations  "),
    classes = "error"
  )

  expect_s3_class(err, "rlang_error")
  expect_match(conditionMessage(err), "did not match any dataset title exactly")
  # But substring should find it as a candidate
  expect_match(conditionMessage(err), "gp-practice-populations")
})

# ============================================================================
# Edge Case 13: Datasets with NULL results (empty API response)
# Verifies behavior when phs_GET returns no datasets
# ============================================================================

test_that("resolve_dataset_name() handles empty API results", {
  mock_content <- mock_package_search()  # Empty results list

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  err <- rlang::catch_cnd(
    resolve_dataset_name("Any Dataset"),
    classes = "error"
  )

  expect_s3_class(err, "rlang_error")
  expect_match(conditionMessage(err), "did not match any dataset title exactly")
  expect_match(conditionMessage(err), "Candidates: none")
})

# ============================================================================
# Edge Case 14: Single dataset with exact match (minimal dataset)
# Verifies function works correctly with minimal API state
# ============================================================================

test_that("resolve_dataset_name() resolves single dataset with exact match", {
  mock_content <- mock_package_search(
    list(
      title = "Only Dataset",
      name = "only-dataset"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      mock_content
    }
  )

  expect_warning(
    out <- resolve_dataset_name("Only Dataset"),
    "resolved to name"
  )
  expect_identical(out, "only-dataset")
})

# ============================================================================
# Edge Case 15: Name-like input with internal special characters that are valid
# e.g., "my_dataset" should NOT match regex (underscore is not in [a-z0-9-])
# ============================================================================

test_that("resolve_dataset_name() does NOT pass underscore-containing input as name-like", {
  mock_content <- mock_package_search(
    list(
      title = "My Dataset",
      name = "my-dataset"
    )
  )

  testthat::local_mocked_bindings(
    phs_GET = function(action, query, ...) {
      expect_identical(action, "package_search")
      mock_content
    }
  )

  # "my_dataset" does NOT match ^[a-z0-9-]+$, so API is called
  result <- resolve_dataset_name("my_dataset")
  # No exact match for "my_dataset", but "my-dataset" might substring match
  # depending on whether underscore prevents matching
  expect_type(result, "character")
})

[sbarnesphs]

Re the documentation, because @inheritParams only copies @param entries, I moved the documentation relating to title-support to the @details section.

[codecov]

Codecov Report

❌ Patch coverage is 98.46154% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
R/get_dataset.R	0.0%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.