Remove overlaping tests and remove confusion with fields name

ECCC-lavoie-ecotox · Jun 18, 2024 · 7cdc5d9 · 7cdc5d9
1 parent 5051608
commit 7cdc5d9
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 42 deletions.
diff --git a/R/db_check_misc.R b/R/db_check_misc.R
@@ -12,9 +12,9 @@
 #' 
 check_fields_exist <- function(con, tbl, fields){
     columns <- get_tbl_info(con, tbl = tbl) |> dplyr::pull(name)
-    if(!all(names(fields) %in% columns)){
-        unknown_fields <- names(fields)[which(!names(fields) %in% columns)] |>
-            glue::glue_collapse(", ", last = "and")
+    if(!all(fields %in% columns)){
+        unknown_fields <- fields[which(!fields %in% columns)] |>
+            glue::glue_collapse(", ", last = " and ")
         cli::cli_abort("Fields { unknown_fields } is/are not present in table { tbl }")       
     }
 }
@@ -34,7 +34,7 @@ check_fields_exist <- function(con, tbl, fields){
 check_fields_pkeys <- function(con, tbl, fields){
     pkeys <- get_tbl_pkeys(con, tbl = tbl)
 
-    if(!all(pkeys %in% names(fields))){
+    if(!all(pkeys %in% fields)){
         missing_pkeys <- pkeys[which(!pkeys %in% fields)] |>
             glue::glue_collapse(", ", last = "and")
         cli::cli_abort("Primary key(s) { missing_pkeys } is/are missing")
@@ -56,7 +56,7 @@ check_fields_pkeys <- function(con, tbl, fields){
 check_fields_notnulls <- function(con, tbl, fields){
     notnulls <- get_tbl_notnulls(con, tbl = tbl)
 
-    if(!all(notnulls %in% names(fields))){
+    if(!all(notnulls %in% fields)){
         missing_fields <- notnulls[which(!notnulls %in% fields)] |>
             glue::glue_collapse(", ", last = "and")
         cli::cli_abort("{ missing_fields } cannot be null(s)")

diff --git a/R/db_tbl.R b/R/db_tbl.R
@@ -9,9 +9,9 @@
 add_entry_tbl <- function(con = NULL, tbl = NULL, ...){
     fields <- list(...)
 
-    check_fields_exist(con, tbl, fields)
-    check_fields_pkeys(con, tbl, fields)
-    check_fields_notnulls(con, tbl, fields)
+    check_fields_exist(con, tbl, names(fields))
+    check_fields_pkeys(con, tbl, names(fields))
+    check_fields_notnulls(con, tbl, names(fields))
 
     ddl <- glue::glue_sql("INSERT INTO { tbl } ({ names(fields)* }) VALUES ({ fields* });", .con = con)
     res <- DBI::dbSendStatement(con, ddl)
@@ -25,8 +25,8 @@ add_entry_tbl <- function(con = NULL, tbl = NULL, ...){
 modify_entry_tbl <- function(con = NULL, tbl = NULL, ...){
     fields <- list(...)
 
-    check_fields_exist(con, tbl, fields)
-    check_fields_pkeys(con, tbl, fields)
+    check_fields_exist(con, tbl, names(fields))
+    check_fields_pkeys(con, tbl, names(fields))
 
     pkeys_tbl <- get_tbl_pkeys(con, tbl)
     target_row <- do.call("search_tbl", list(con = con, tbl = tbl) |> append(fields[pkeys_tbl]))
@@ -65,8 +65,8 @@ modify_entry_tbl <- function(con = NULL, tbl = NULL, ...){
 delete_entry_tbl <- function(con = NULL, tbl = NULL, ...){
     fields <- list(...)
 
-    check_fields_exist(con, tbl, fields)
-    check_fields_pkeys(con, tbl, fields)
+    check_fields_exist(con, tbl, names(fields))
+    check_fields_pkeys(con, tbl, names(fields))
 
     pkeys_tbl <- get_tbl_pkeys(con, tbl)
     target_row <- do.call("search_tbl", list(con =  con, tbl = tbl) |> append(fields[pkeys_tbl]))

diff --git a/tests/testthat/test-info_misc.R → tests/testthat/test-check_misc.R b/tests/testthat/test-info_misc.R → tests/testthat/test-check_misc.R
@@ -23,6 +23,16 @@ test_that("get_tbl_notnulls() success", {
     withr::deferred_run()     
 })
 
+test_that("check_fields_exist() error", {
+    con <- test_db()
+    mockery::stub(check_fields_exist, "get_tbl_info", tbl_info_test)
+    testthat::expect_error(
+        check_fields_exist(con, "species", "test"), 
+        "Fields test is/are not present in table species", fixed = TRUE
+    )
+    withr::deferred_run()     
+})
+
 test_that("check_fields_notnulls() error", {
     con <- test_db()
     mockery::stub(check_fields_notnulls, "get_tbl_notnulls", "species")  

diff --git a/tests/testthat/test-db_tbl.R b/tests/testthat/test-db_tbl.R
@@ -3,6 +3,7 @@ context("Add, delete, update entries in table")
 test_that("add_entry_tbl() success", {
 
     con <- test_db()
+
     add_entry_tbl(
         con = con, 
         tbl = "species", 
@@ -20,36 +21,6 @@ test_that("add_entry_tbl() success", {
     withr::deferred_run()
 })
 
-
-test_that("add_entry_tbl() with missing pkeys", {
-    con <- test_db()
-
-    testthat::expect_error(
-        add_entry_tbl(
-            con = con, 
-            tbl = "species", 
-            genus = "Lupus",
-            species = "Lupus lupus"
-    ), "Primary key(s) species_id is/are missing", fixed = TRUE)
-
-    withr::deferred_run()
-})
-
-test_that("add_entry_tbl() with not null contraint ", {
-    con <- test_db()
-
-    testthat::expect_error(
-        add_entry_tbl(
-            con = con, 
-            tbl = "species", 
-            species_id = "TSN", 
-            genus = "Lupus"
-    ), "species cannot be null(s)", fixed = TRUE)
-
-    withr::deferred_run()
-})
-
-
 test_that("delete_entry_tbl() success", {
     con <- test_db(mockData = TRUE)