Closes #1966 address derive_vars_joined bugs

NEWS.md

@@ -39,6 +39,8 @@ the `filter_add` using the next phase of the deprecation process. (#1950)
 
 - The list of package authors/contributors has been reformatted so that those who are actively maintaining the code base are now marked as *authors*, whereas those who made a significant contribution in the past are now down as *contributors*. All other acknowledgements have been moved to README section (#1941).
 
+- `derive_vars_joined()` had two bugs with regards to duplicates messaging and when `new_vars` was set to `NULL` that have now been addressed (#1966). 
+
 # admiral 0.11.1
 
 - Fix bug in `derive_param_tte()`. (#1962)

R/derive_joined.R

@@ -352,6 +352,18 @@ derive_vars_joined <- function(dataset,
 
   if (is.null(new_vars)) {
     new_vars <- chr2vars(colnames(dataset_add))
+    preexisting_vars <- chr2vars(colnames(dataset))
+    preexisting_vars_no_by_vars <- preexisting_vars[which(!(preexisting_vars %in% by_vars))]
+    if (any(new_vars %in% preexisting_vars_no_by_vars)) {
+      err_msg <- sprintf(
+        paste(
+          "The following columns in `dataset_add` have naming conflicts with `dataset`,\n",
+          "please make the appropriate modifications to `new_vars`, with respect to:\n%s"
+        ),
+        enumerate(vars2chr(new_vars[which(new_vars %in% preexisting_vars_no_by_vars)]))
+      )
+      abort(err_msg)
+    }
   }
 
   # number observations of the input dataset to get a unique key
@@ -371,7 +383,7 @@ derive_vars_joined <- function(dataset,
     filter_if(filter_add) %>%
     select(
       !!!by_vars,
-      !!!chr2vars(names(order)),
+      !!!replace_values_by_names(extract_vars(order)),
       !!!replace_values_by_names(join_vars),
       !!!intersect(unname(extract_vars(new_vars)), chr2vars(colnames(dataset_add)))
     )
@@ -394,7 +406,7 @@ derive_vars_joined <- function(dataset,
       data_return,
       by_vars = expr_c(by_vars_left, tmp_obs_nr),
       order = add_suffix_to_vars(
-        replace_values_by_names(order),
+        replace_values_by_names(extract_vars(order)),
         vars = common_vars,
         suffix = ".join"
       ),
@@ -410,6 +422,7 @@ derive_vars_joined <- function(dataset,
       by_vars = exprs(!!!by_vars_left, !!tmp_obs_nr),
       new_vars = add_suffix_to_vars(new_vars, vars = common_vars, suffix = ".join"),
       missing_values = missing_values,
+      check_type = check_type,
       duplicate_msg = paste(
         paste(
           "After applying `filter_join` the joined dataset contains more",

tests/testthat/test-derive_joined.R

@@ -242,3 +242,66 @@ test_that("derive_vars_joined Test 7: new_vars expressions using variables from
     keys = c("USUBJID", "AESEQ")
   )
 })
+## Test 8: NULL new_vars will still remove .join columns ----
+test_that("derive_vars_joined Test 8: NULL new_vars will still remove .join columns", {
+  myd <- data.frame(day = c(1, 2, 3), val = c(0, 17, 21))
+  expect_error(
+    derive_vars_joined(
+      myd,
+      dataset_add = myd,
+      order = exprs(day),
+      mode = "last",
+      filter_join = day < day.join
+    ),
+    regexp = paste(
+      "The following columns in `dataset_add` have naming conflicts with `dataset`"
+    )
+  )
+})
+## Test 9: fixing a bug from issue 1966 ----
+test_that("derive_vars_joined Test 9: fixing a bug from issue 1966", { # nolint
+  adlb_ast <- tribble(
+    ~ADT,         ~ASEQ,
+    "2002-01-01", 1,
+    "2002-02-02", 2,
+    "2002-02-02", 3
+  ) %>%
+    mutate(
+      STUDYID = "ABC",
+      USUBJID = "1",
+      ADT = ymd(ADT),
+      ADTM = as_datetime(ADT)
+    )
+
+  adlb_tbili_pbl <- tribble(
+    ~ADT,         ~ASEQ,
+    "2002-01-01", 4,
+    "2002-02-02", 5,
+    "2002-02-02", 6
+  ) %>%
+    mutate(
+      STUDYID = "ABC",
+      USUBJID = "1",
+      ADT = ymd(ADT),
+      ADTM = as_datetime(ADT)
+    )
+
+  adlb_joined <- derive_vars_joined(
+    adlb_ast,
+    dataset_add = adlb_tbili_pbl,
+    by_vars = exprs(STUDYID, USUBJID),
+    order = exprs(ADTM, ASEQ),
+    new_vars = exprs(TBILI_ADT = ADT),
+    filter_join = ADT <= ADT.join,
+    mode = "first"
+  )
+
+  expected <- adlb_ast %>%
+    mutate(TBILI_ADT = as.Date(c("2002-01-01", "2002-02-02", "2002-02-02"), "%Y-%m-%d"))
+
+  expect_dfs_equal(
+    base = expected,
+    compare = adlb_joined,
+    keys = c("ADT", "ASEQ", "STUDYID", "USUBJID", "ADTM", "TBILI_ADT")
+  )
+})