From 7c585040f1d59a52677c829d2b3269611ab1066a Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 13 Nov 2024 13:02:27 -0500
Subject: [PATCH 01/36] Migrate to `review_records` from `review_row`

---
 R/fct_SQLite.R        | 67 ++++++++++++++-----------------------------
 R/mod_review_forms.R  | 38 +++++++++++-------------
 man/db_get_review.Rd  | 21 ++------------
 man/db_save_review.Rd | 19 ++----------
 4 files changed, 43 insertions(+), 102 deletions(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index 85f4d63e..e861d9d6 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -302,50 +302,29 @@ db_upsert <- function(con, data, idx_cols) {
 #' @param db_path Character vector. Path to the database.
 #' @param tables Character vector. Names of the tables within the database to
 #'   save the review in.
-#' @param common_vars A character vector containing the common key variables.
-#' @param review_by A character vector, containing the key variables to perform
-#'   the review on. For example, the review can be performed on form level
-#'   (writing the same review to all items in a form), or on item level, with a
-#'   different review per item.
 #'
 #' @return Review information will be written in the database. No local objects
 #'   will be returned.
 #' @export
 #' 
 db_save_review <- function(
-    rv_row,
+    rv_records,
     db_path,
-    tables = c("all_review_data"),
-    common_vars = c("subject_id", "event_name", "item_group", 
-                    "form_repeat", "item_name"),
-    review_by = c("subject_id", "item_group")
+    tables = c("all_review_data")
 ){
-  stopifnot(is.data.frame(rv_row))
-  if(nrow(rv_row) != 1){
-    warning("multiple rows detected to save in database. Only the first row will be selected.")
-    rv_row <- rv_row[1, ]
+  stopifnot(is.data.frame(rv_records))
+  if (any(duplicated(rv_records[["id"]]))) {
+    warning("duplicate records detected to save in database. Only the first will be selected.")
+    rv_records[!duplicated(rv_records[["id"]]),]
   }
-  
+
   cols_to_change <- c("reviewed", "comment", "reviewer", "timestamp", "status")
   db_con <- get_db_connection(db_path)
-  new_review_state <- rv_row$reviewed
-  cat("copy row ids into database\n ")
-  dplyr::copy_to(db_con, rv_row[review_by], "row_ids")
-  new_review_rows <-  dplyr::tbl(db_con, "all_review_data") |> 
-    dplyr::inner_join(dplyr::tbl(db_con, "row_ids"), by = review_by) |> 
-    # Filter below prevents unnecessarily overwriting the review status in forms   
-    # with mixed reviewed status (due to an edit by the investigators). 
-    dplyr::collect()
-  if(nrow(new_review_rows) == 0){return(
+  if(nrow(rv_records) == 0){return(
     warning("Review state unaltered. No review will be saved.")
   )}
-  new_review_rows <- new_review_rows |> 
-    dplyr::select(-dplyr::all_of(cols_to_change)) |> 
-    # If there are multiple edits, make sure to only select the latest editdatetime for all items:
-    # dplyr::slice_max(edit_date_time, by = dplyr::all_of(common_vars)) |> 
-    dplyr::bind_cols(rv_row[cols_to_change]) # bind_cols does not work in a db connection.
   cat("write updated review data to database\n")
-  dplyr::copy_to(db_con, new_review_rows, "row_updates")
+  dplyr::copy_to(db_con, rv_records, "row_updates")
   rs <- DBI::dbSendStatement(db_con, paste(
     "UPDATE",
     tables,
@@ -455,10 +434,8 @@ db_get_query <- function(
 #' with the given subject id (`subject`) and `form`.
 #'
 #' @param db_path Character vector. Needs to be a valid path to a database.
-#' @param subject Character vector with the subject identifier to select from
+#' @param ids Integer vector with the unique identifier to select from
 #'   the database.
-#' @param form Character vector with the form identifier to select from the
-#'   database.
 #'
 #' @inheritParams db_slice_rows
 #' @return A data frame.
@@ -485,23 +462,21 @@ db_get_query <- function(
 #' 
 db_get_review <- function(
     db_path, 
-    subject = review_row$subject_id, 
-    form = review_row$item_group,
-    db_table = "all_review_data",
-    slice_vars = c("timestamp", "edit_date_time"),
-    group_vars = c("subject_id", "event_name", "item_group",
-                   "form_repeat", "item_name")
+    ids = review_row$id,
+    db_table = "all_review_data"
 ){
   stopifnot(file.exists(db_path))
-  stopifnot(is.character(subject))
-  stopifnot(is.character(form))
+  stopifnot(is.integer(ids))
   db_temp_connect(db_path, {
-    sql <- "SELECT * FROM ?db_table WHERE subject_id = ?id AND item_group = ?group;"
-    query <- DBI::sqlInterpolate(con, sql, db_table = db_table[1], 
-                                 id = subject[1], group = form[1])
-    DBI::dbGetQuery(con, query) |> 
-      db_slice_rows(slice_vars = slice_vars, group_vars = group_vars) |> 
+    sql <- "SELECT * FROM ?db_table WHERE id = $id"
+    query <- DBI::sqlInterpolate(con, sql, db_table = db_table[1])
+    rs <- DBI::dbSendQuery(con, query)
+    DBI::dbBind(rs, list(id = ids))
+    df <- 
+      DBI::dbFetch(rs) |> 
       dplyr::as_tibble()
+    DBI::dbClearResult(rs)
+    df
   })
 }
 
diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 5f8bf9a8..0b89ab67 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -110,14 +110,10 @@ mod_review_forms_server <- function(
     ns <- session$ns
     
     review_data_active <- reactive({
-      df <- r$review_data |>
+      r$review_data |>
         dplyr::filter(subject_id == r$subject_id, 
                       item_group == active_form()) |> 
-        dplyr::distinct(subject_id, item_group, edit_date_time, reviewed, comment, status)
-      #!! below selects the latest edit_date_time; usually only one row will remain by then since there are no items displayed here.
-      if(nrow(df)== 0) return(df)
-      df |> 
-        dplyr::filter(edit_date_time == max(as.POSIXct(edit_date_time)))
+        dplyr::select(id, dplyr::all_of(idx_cols), edit_date_time, reviewed, comment, status)
     }) 
     
     observeEvent(c(active_form(), r$subject_id), {
@@ -231,8 +227,8 @@ mod_review_forms_server <- function(
       review_save_error(FALSE)
       golem::cat_dev("Save review status reviewed:", input$form_reviewed, "\n")
       
-      review_row <- review_data_active() |> 
-        dplyr::distinct(subject_id, item_group) |> 
+      review_records <- review_data_active() |> 
+        dplyr::distinct(id) |>
         dplyr::mutate(
           reviewed    = if(input$form_reviewed) "Yes" else "No",
           comment     = ifelse(is.null(input$review_comment), "", input$review_comment),
@@ -242,34 +238,34 @@ mod_review_forms_server <- function(
         ) 
       
       golem::cat_dev("review row to add:\n")
-      golem::print_dev(review_row)
+      golem::print_dev(review_records)
       
       cat("write review progress to database\n")
       db_save_review(
-        review_row, 
+        review_records, 
         db_path = db_path,
         # More tables can be added here if needed, to track process of 
         # individual reviewers in individual tables:
         tables = "all_review_data" 
       )
       
-      review_row_db <- db_get_review(
-        db_path, subject = review_row$subject_id, form = review_row$item_group
-        )
-      review_row_db <- unique(review_row_db[names(review_row)])
-      if(identical(review_row_db, review_row)){
+      review_records_db <- db_get_review(
+        db_path, ids = review_records$id
+        ) |> 
+        dplyr::select(dplyr::all_of(names(review_records)))
+      if(identical(review_records_db, review_records)){
         cat("Update review data and status in app\n")
         r$review_data <- r$review_data |> 
-          dplyr::rows_update(review_row, by = c("subject_id", "item_group"))
+          dplyr::rows_update(review_records, by = "id")
       }
       
-      review_row_memory <- review_row |> 
-        dplyr::left_join(r$review_data, by = names(review_row)) 
-      review_row_memory <- unique(review_row_memory[names(review_row)])
+      review_row_memory <- review_records |> 
+        dplyr::left_join(r$review_data, by = "id", suffix = c("", ".y")) |> 
+        dplyr::select(dplyr::all_of(names(review_records)))
       
       review_save_error(any(
-        !identical(review_row_db, review_row),
-        !identical(review_row_memory, review_row_db)
+        !identical(review_records_db, review_records),
+        !identical(review_row_memory, review_records_db)
       ))
       
       if(review_save_error()){
diff --git a/man/db_get_review.Rd b/man/db_get_review.Rd
index a36d15ec..d91ecc5b 100644
--- a/man/db_get_review.Rd
+++ b/man/db_get_review.Rd
@@ -4,33 +4,16 @@
 \alias{db_get_review}
 \title{Retrieve review}
 \usage{
-db_get_review(
-  db_path,
-  subject = review_row$subject_id,
-  form = review_row$item_group,
-  db_table = "all_review_data",
-  slice_vars = c("timestamp", "edit_date_time"),
-  group_vars = c("subject_id", "event_name", "item_group", "form_repeat", "item_name")
-)
+db_get_review(db_path, ids = review_row$id, db_table = "all_review_data")
 }
 \arguments{
 \item{db_path}{Character vector. Needs to be a valid path to a database.}
 
-\item{subject}{Character vector with the subject identifier to select from
+\item{ids}{Integer vector with the unique identifier to select from
 the database.}
 
-\item{form}{Character vector with the form identifier to select from the
-database.}
-
 \item{db_table}{Character string. Name of the table to collect. Will only be
 used if \code{data} is a character string to a database.}
-
-\item{slice_vars}{Character vector. Names of the variables that will be used
-to slice the data frame. Note that the order matters: Slicing will occur
-for each variable in this vector successively,}
-
-\item{group_vars}{Character vector. Variable names of the variables to
-perform the grouping on.}
 }
 \value{
 A data frame.
diff --git a/man/db_save_review.Rd b/man/db_save_review.Rd
index dbbbed19..1852e27c 100644
--- a/man/db_save_review.Rd
+++ b/man/db_save_review.Rd
@@ -4,29 +4,16 @@
 \alias{db_save_review}
 \title{Save review in database}
 \usage{
-db_save_review(
-  rv_row,
-  db_path,
-  tables = c("all_review_data"),
-  common_vars = c("subject_id", "event_name", "item_group", "form_repeat", "item_name"),
-  review_by = c("subject_id", "item_group")
-)
+db_save_review(rv_records, db_path, tables = c("all_review_data"))
 }
 \arguments{
-\item{rv_row}{A data frame containing the row of the data that needs to be
-checked.}
-
 \item{db_path}{Character vector. Path to the database.}
 
 \item{tables}{Character vector. Names of the tables within the database to
 save the review in.}
 
-\item{common_vars}{A character vector containing the common key variables.}
-
-\item{review_by}{A character vector, containing the key variables to perform
-the review on. For example, the review can be performed on form level
-(writing the same review to all items in a form), or on item level, with a
-different review per item.}
+\item{rv_row}{A data frame containing the row of the data that needs to be
+checked.}
 }
 \value{
 Review information will be written in the database. No local objects

From b271add9802a39203f510e231583185361408168 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 13 Nov 2024 15:06:35 -0500
Subject: [PATCH 02/36] Update test-fct_SQLite.R

---
 tests/testthat/test-fct_SQLite.R | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/tests/testthat/test-fct_SQLite.R b/tests/testthat/test-fct_SQLite.R
index 9acc738b..11fd48cc 100644
--- a/tests/testthat/test-fct_SQLite.R
+++ b/tests/testthat/test-fct_SQLite.R
@@ -265,11 +265,9 @@ describe(
       db_add_log(con)
       
       db_save_review(
-        cbind(df, new_review), 
+        cbind(id = 1, df, new_review), 
         temp_path, 
-        tables = c("all_review_data"),
-        common_vars = c("key_col1", "item_group", "item_name"), 
-        review_by = c("key_col1", "item_group")
+        tables = c("all_review_data")
       )
       expect_equal(
         dplyr::collect(dplyr::tbl(con, "all_review_data")), 
@@ -306,6 +304,7 @@ describe(
         status = "old"
       )
       review_row <- data.frame(
+        id = 1:2,
         key_col1 = "9999",
         item_group = "Group 1"
       ) |> 
@@ -318,9 +317,7 @@ describe(
       db_save_review(
         review_row, 
         temp_path, 
-        tables = c("all_review_data"),
-        common_vars = c("key_col1", "item_group", "item_name"), 
-        review_by = c("key_col1", "item_group")
+        tables = c("all_review_data")
       )
       expect_true(is.data.frame(dplyr::collect(dplyr::tbl(con, "all_review_data"))))
       results <- dplyr::collect(dplyr::tbl(con, "all_review_data"))
@@ -341,18 +338,16 @@ describe(
     
     
     
-    it("warns with multiple rows in the review_row object", {
+    it("warns with duplicate records in the review_row object", {
       temp_path <- withr::local_tempfile(fileext = ".sqlite")
       con <- get_db_connection(temp_path)
       
       db_add_primary_key(con, "all_review_data", cbind(df, old_review))
       db_add_log(con)
       db_save_review(
-        rbind(cbind(df, new_review), cbind(df, new_review)), 
+        rbind(cbind(id = 1:2, df, new_review), cbind(id = 1:2, df, new_review)), 
         temp_path, 
-        tables = "all_review_data",
-        common_vars = c("key_col1", "item_group", "item_name"), 
-        review_by = c("key_col1", "item_group")
+        tables = "all_review_data"
       ) |> expect_warning()
     })
   }
@@ -442,16 +437,12 @@ describe("db_get_review can collect latest review data from a database", {
   db_add_log(con)
 
   it("Can collect the desired data.", {
-    output <- db_get_review(temp_path, subject = "Test_name", form = "Test_group")
+    output <- db_get_review(temp_path, ids = 1L)
     expect_equal(output[,-1], review_data)
   })
   
-  it("Collects an empty data frame if the requested subject or form are not found", {
-    output <- db_get_review(temp_path, subject = "Non-existent", 
-                                         form = "Test_group")
-    expect_equal(output[,-1], review_data[0,])
-    output <- db_get_review(temp_path, subject = "Test_name", 
-                                        form = "Non-existent")
+  it("Collects an empty data frame if the requested ids are not found", {
+    output <- db_get_review(temp_path, ids = 2L)
     expect_equal(output[,-1], review_data[0,])
   })
 

From 39d8da1d372f2b8c858955b3681caa922158152e Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 13 Nov 2024 15:18:24 -0500
Subject: [PATCH 03/36] Update test-mod_review_forms.R

---
 tests/testthat/test-mod_review_forms.R | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/testthat/test-mod_review_forms.R b/tests/testthat/test-mod_review_forms.R
index 3601fdfc..d6268751 100644
--- a/tests/testthat/test-mod_review_forms.R
+++ b/tests/testthat/test-mod_review_forms.R
@@ -260,8 +260,8 @@ describe(
           session$setInputs(form_reviewed = FALSE)
           expect_equal(
             review_data_active(),
-            dplyr::filter(r$review_data, subject_id == "885", item_group == "Adverse events") |>
-              dplyr::select(subject_id, item_group, edit_date_time, reviewed, comment, status)
+            dplyr::filter(r$review_data, subject_id == "885", item_group == "Adverse events") |> 
+              dplyr::select(id, dplyr::all_of(idx_cols), edit_date_time, reviewed, comment, status)
           )
           expect_equal(review_data_active()$item_group, "Adverse events")
           expect_equal(nrow(review_data_active()), 1)
@@ -420,7 +420,7 @@ describe(
         
         # review status and reviewer is saved as expected
         saved_review_row <- db_get_review(
-          temp_path, subject = "885", form = "Adverse events"
+          temp_path, ids = 1L
         )
         expect_equal(saved_review_row$status, "new")
         expect_equal(saved_review_row$reviewer, "")

From 43931ed987a37dc0833422b9d585fc59449f1a7a Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 13 Nov 2024 16:45:05 -0500
Subject: [PATCH 04/36] Update test-mod_main_sidebar.R

---
 tests/testthat/test-mod_main_sidebar.R | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/testthat/test-mod_main_sidebar.R b/tests/testthat/test-mod_main_sidebar.R
index 1b5931a1..ef63410d 100644
--- a/tests/testthat/test-mod_main_sidebar.R
+++ b/tests/testthat/test-mod_main_sidebar.R
@@ -55,7 +55,8 @@ describe(
           dplyr::mutate(
             reviewed = "No",
             status = "new",
-            comment = ""
+            comment = "",
+            id = dplyr::row_number()
           )
         vars <- get_meta_vars(appdata, metadata)
         apptables <- lapply(
@@ -123,8 +124,12 @@ describe(
             r = reactiveValues(
               create_query = 0, 
               review_data = data.frame(
+                "id" = 1L,
                 "subject_id" = "NLD_06_755", 
+                "event_name" = "Any visit",
                 "item_group" = "Adverse events",
+                "form_repeat" = 1L,
+                "item_name" = "AE item",
                 "edit_date_time" = "2023-01-01",
                 "reviewed" = "",
                 "comment" = "",

From d311ab1b93eb3183638be4b99559d38ecec855f3 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 13 Nov 2024 16:54:00 -0500
Subject: [PATCH 05/36] Fix R CMD checks

---
 R/fct_SQLite.R        | 2 +-
 R/global.R            | 3 ++-
 man/db_save_review.Rd | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index e861d9d6..b09ae045 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -297,7 +297,7 @@ db_upsert <- function(con, data, idx_cols) {
 #' New rows with the new/updated review data will be added to the applicable
 #' database tables.
 #'
-#' @param rv_row A data frame containing the row of the data that needs to be
+#' @param rv_records A data frame containing the rows of data that needs to be
 #'   checked.
 #' @param db_path Character vector. Path to the database.
 #' @param tables Character vector. Names of the tables within the database to
diff --git a/R/global.R b/R/global.R
index 68e661f1..645cd6f1 100644
--- a/R/global.R
+++ b/R/global.R
@@ -111,7 +111,8 @@ utils::globalVariables(
     "vis_day",
     "event_id",
     "region",
-    "suffix_names"
+    "suffix_names",
+    "form_type"
   )
 )
 
diff --git a/man/db_save_review.Rd b/man/db_save_review.Rd
index 1852e27c..a176183a 100644
--- a/man/db_save_review.Rd
+++ b/man/db_save_review.Rd
@@ -7,13 +7,13 @@
 db_save_review(rv_records, db_path, tables = c("all_review_data"))
 }
 \arguments{
+\item{rv_records}{A data frame containing the rows of data that needs to be
+checked.}
+
 \item{db_path}{Character vector. Path to the database.}
 
 \item{tables}{Character vector. Names of the tables within the database to
 save the review in.}
-
-\item{rv_row}{A data frame containing the row of the data that needs to be
-checked.}
 }
 \value{
 Review information will be written in the database. No local objects

From ccd9bb1fbc1c402b1f471aaf2c377c0bacffedf5 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 13 Nov 2024 16:59:57 -0500
Subject: [PATCH 06/36] Fix `db_get_review()` example

---
 R/fct_SQLite.R | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index b09ae045..5a4e6fcb 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -447,7 +447,8 @@ db_get_query <- function(
 #'   temp_path <- withr::local_tempfile(fileext = ".sqlite")
 #'   con <- get_db_connection(temp_path)
 #'   review_data <- data.frame(
-#'   subject_id = "Test_name",
+#'    subject_id = "Test_name",
+#'    id = 1L,
 #'    event_name = "Visit 1",
 #'    item_group = "Test_group",
 #'    form_repeat = 1,
@@ -457,7 +458,7 @@ db_get_query <- function(
 #'   ) |>
 #'    dplyr::as_tibble()
 #'   DBI::dbWriteTable(con, "all_review_data", review_data)
-#'   db_get_review(temp_path, subject = "Test_name", form = "Test_group")
+#'   db_get_review(temp_path, ids = 1L)
 #' })
 #' 
 db_get_review <- function(

From 0743880fd50d2f8b06449fcc26ee4975a25bf573 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 13 Nov 2024 17:08:43 -0500
Subject: [PATCH 07/36] Helps if you update documentation

---
 man/db_get_review.Rd | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/man/db_get_review.Rd b/man/db_get_review.Rd
index d91ecc5b..e0396f0b 100644
--- a/man/db_get_review.Rd
+++ b/man/db_get_review.Rd
@@ -28,7 +28,8 @@ local({
   temp_path <- withr::local_tempfile(fileext = ".sqlite")
   con <- get_db_connection(temp_path)
   review_data <- data.frame(
-  subject_id = "Test_name",
+   subject_id = "Test_name",
+   id = 1L,
    event_name = "Visit 1",
    item_group = "Test_group",
    form_repeat = 1,
@@ -38,7 +39,7 @@ local({
   ) |>
    dplyr::as_tibble()
   DBI::dbWriteTable(con, "all_review_data", review_data)
-  db_get_review(temp_path, subject = "Test_name", form = "Test_group")
+  db_get_review(temp_path, ids = 1L)
 })
 
 }

From e5e8acbf9230acd07b274e7a8be1b4e6712bd3f8 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 14 Nov 2024 09:00:52 -0500
Subject: [PATCH 08/36] Only UPDATE reviews when review status is changed

---
 R/fct_SQLite.R | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index 5a4e6fcb..57b2ce10 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -333,7 +333,9 @@ db_save_review <- function(
     "FROM",
     "row_updates",
     "WHERE",
-    sprintf("%s.id = row_updates.id", tables)
+    sprintf("%s.id = row_updates.id", tables),
+    "AND",
+    sprintf("%s.reviewed <> row_updates.reviewed", tables)
   ))
   DBI::dbClearResult(rs)
   cat("finished writing to the tables:", tables, "\n")

From d863300eacafe7531f9f5bd473ee504ac0be7aa3 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Tue, 19 Nov 2024 15:00:56 -0500
Subject: [PATCH 09/36] Keep current structure for enabling reviews

---
 R/mod_review_forms.R                   | 10 +++++-----
 tests/testthat/test-mod_review_forms.R |  8 ++++----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 0b89ab67..f5d5d19b 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -132,8 +132,8 @@ mod_review_forms_server <- function(
         # it will give a warning. This would be rare since it would mean a datapoint with the same edit date-time was reviewed but another one was not. 
         # probably better to use defensive coding here to ensure the app does not crash in that case. However we need to define which review status we need to select
         # in this case get the reviewed = "No"
-        review_status  <- unique(review_data_active()$reviewed)
-        review_comment <- unique(review_data_active()$comment)
+        review_status <- with(review_data_active(), reviewed[edit_date_time == max(as.POSIXct(edit_date_time))]) |> unique()
+        review_comment <- with(review_data_active(), comment[edit_date_time == max(as.POSIXct(edit_date_time))]) |> unique()
         if(length(review_status) != 1) warning("multiple variables in review_status, namely: ", 
                                                review_status, "Verify data.")
       }
@@ -187,8 +187,8 @@ mod_review_forms_server <- function(
         ) 
       if(!enable_any_review()) return(FALSE)
       any(c(
-        unique(review_data_active()$reviewed) == "No"  & input$form_reviewed, 
-        unique(review_data_active()$reviewed) == "Yes" & !input$form_reviewed
+        unique(with(review_data_active(), reviewed[edit_date_time == max(as.POSIXct(edit_date_time))])) == "No"  & input$form_reviewed, 
+        unique(with(review_data_active(), reviewed[edit_date_time == max(as.POSIXct(edit_date_time))])) == "Yes" & !input$form_reviewed
       ))
     })
     
@@ -305,7 +305,7 @@ mod_review_forms_server <- function(
         "No user name found. Cannot save review"
       ))
       validate(need(
-        !review_data_active()$reviewed == "Yes",
+        !unique(with(review_data_active(), reviewed[edit_date_time == max(as.POSIXct(edit_date_time))])) == "Yes",
         "Form already reviewed"
       ))
       validate(need(input$form_reviewed, "Requires review"))
diff --git a/tests/testthat/test-mod_review_forms.R b/tests/testthat/test-mod_review_forms.R
index 98161f6d..0cb4d434 100644
--- a/tests/testthat/test-mod_review_forms.R
+++ b/tests/testthat/test-mod_review_forms.R
@@ -136,7 +136,7 @@ describe(
               )
             
             updated_rows_db <- db_get_review(
-              db_path, subject = "885", form = "Adverse events"
+              db_path, ids = 1:2
             )
             
             expect_equal(updated_rows_db$comment, c("test review", "test review"))
@@ -236,7 +236,7 @@ describe(
         expect_true(app$get_js("document.getElementById('test-review_comment').disabled;"))
         
         # review status and reviewer is saved as expected
-        saved_review_row <- db_get_review(temp_path, subject = "885", form = "Adverse events")
+        saved_review_row <- db_get_review(temp_path, ids = 1:2)
         expect_equal(saved_review_row$status, c("old", "old"))
         expect_equal(saved_review_row$reviewer, c("Reviewer 1", "test_name (Medical Monitor)"))
       }
@@ -289,8 +289,8 @@ describe(
             dplyr::filter(r$review_data, subject_id == "885", item_group == "Adverse events") |> 
               dplyr::select(id, dplyr::all_of(idx_cols), edit_date_time, reviewed, comment, status)
           )
-          expect_equal(review_data_active()$item_group, "Adverse events")
-          expect_equal(nrow(review_data_active()), 1)
+          expect_equal(review_data_active()$item_group, c("Adverse events", "Adverse events"))
+          expect_equal(nrow(review_data_active()), 2)
           expect_error(output[["save_review_error"]], "Requires review")
         })
       }

From a23b46218b49b402335ea39395ff7023f26717aa Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 20 Nov 2024 14:26:42 -0500
Subject: [PATCH 10/36] Generalize `db_get_review()`

---
 R/fct_SQLite.R                         | 35 ++++++++++++++------------
 R/mod_review_forms.R                   |  2 +-
 man/db_get_review.Rd                   | 26 ++++++++++---------
 tests/testthat/test-fct_SQLite.R       |  7 ++++--
 tests/testthat/test-mod_review_forms.R |  4 +--
 5 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index 2ce461f5..cd7035ee 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -459,10 +459,12 @@ db_get_query <- function(
 #' with the given subject id (`subject`) and `form`.
 #'
 #' @param db_path Character vector. Needs to be a valid path to a database.
-#' @param ids Integer vector with the unique identifier to select from
-#'   the database.
+#' @param ... Named arguments specifying which records to retrieve, see
+#'   examples. Note that `...` will be processed with `data.frame()` since
+#'   parameters must have equal length.
+#' @param db_table Character string. Name of the table to collect. Will only be
+#'   used if `data` is a character string to a database.
 #'
-#' @inheritParams db_slice_rows
 #' @return A data frame.
 #' @export
 #'
@@ -472,32 +474,33 @@ db_get_query <- function(
 #'   temp_path <- withr::local_tempfile(fileext = ".sqlite")
 #'   con <- get_db_connection(temp_path)
 #'   review_data <- data.frame(
-#'    subject_id = "Test_name",
-#'    id = 1L,
-#'    event_name = "Visit 1",
-#'    item_group = "Test_group",
-#'    form_repeat = 1,
-#'    item_name = "Test_item",
-#'    edit_date_time = "2023-11-05 01:26:00",
-#'    timestamp = "2024-02-05 01:01:01"
+#'    subject_id = c("Test_name", "Test_name2"),
+#'    id = 1:2,
+#'    event_name = c("Visit 1", "Visit 1"),
+#'    item_group = c("Test_group", "Test_group2"),
+#'    form_repeat = c(1, 1),
+#'    item_name = c("Test_item", "Test_item2"),
+#'    edit_date_time = rep("2023-11-05 01:26:00", 2),
+#'    timestamp = rep("2024-02-05 01:01:01", 2)
 #'   ) |>
 #'    dplyr::as_tibble()
 #'   DBI::dbWriteTable(con, "all_review_data", review_data)
-#'   db_get_review(temp_path, ids = 1L)
+#'   db_get_review(temp_path, id = 1L)
+#'   db_get_review(temp_path, subject_id = "Test_name2")
 #' })
 #' 
 db_get_review <- function(
     db_path, 
-    ids = review_row$id,
+    ...,
     db_table = "all_review_data"
 ){
   stopifnot(file.exists(db_path))
-  stopifnot(is.integer(ids))
   db_temp_connect(db_path, {
-    sql <- "SELECT * FROM ?db_table WHERE id = $id"
+    fields <- ...names()
+    sql <- paste("SELECT * FROM ?db_table WHERE", paste0(fields, " = $", fields, collapse = " AND "))
     query <- DBI::sqlInterpolate(con, sql, db_table = db_table[1])
     rs <- DBI::dbSendQuery(con, query)
-    DBI::dbBind(rs, list(id = ids))
+    DBI::dbBind(rs, params = data.frame(...))
     df <- 
       DBI::dbFetch(rs) |> 
       dplyr::as_tibble()
diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 75c78dc0..47415bad 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -250,7 +250,7 @@ mod_review_forms_server <- function(
       )
       
       updated_rows_db <- db_get_review(
-        db_path, ids = review_records$id
+        db_path, id = review_records$id
         ) |> 
         dplyr::select(dplyr::all_of(names(review_records)))
       
diff --git a/man/db_get_review.Rd b/man/db_get_review.Rd
index e0396f0b..17456098 100644
--- a/man/db_get_review.Rd
+++ b/man/db_get_review.Rd
@@ -4,13 +4,14 @@
 \alias{db_get_review}
 \title{Retrieve review}
 \usage{
-db_get_review(db_path, ids = review_row$id, db_table = "all_review_data")
+db_get_review(db_path, ..., db_table = "all_review_data")
 }
 \arguments{
 \item{db_path}{Character vector. Needs to be a valid path to a database.}
 
-\item{ids}{Integer vector with the unique identifier to select from
-the database.}
+\item{...}{Named arguments specifying which records to retrieve, see
+examples. Note that \code{...} will be processed with \code{data.frame()} since
+parameters must have equal length.}
 
 \item{db_table}{Character string. Name of the table to collect. Will only be
 used if \code{data} is a character string to a database.}
@@ -28,18 +29,19 @@ local({
   temp_path <- withr::local_tempfile(fileext = ".sqlite")
   con <- get_db_connection(temp_path)
   review_data <- data.frame(
-   subject_id = "Test_name",
-   id = 1L,
-   event_name = "Visit 1",
-   item_group = "Test_group",
-   form_repeat = 1,
-   item_name = "Test_item",
-   edit_date_time = "2023-11-05 01:26:00",
-   timestamp = "2024-02-05 01:01:01"
+   subject_id = c("Test_name", "Test_name2"),
+   id = 1:2,
+   event_name = c("Visit 1", "Visit 1"),
+   item_group = c("Test_group", "Test_group2"),
+   form_repeat = c(1, 1),
+   item_name = c("Test_item", "Test_item2"),
+   edit_date_time = rep("2023-11-05 01:26:00", 2),
+   timestamp = rep("2024-02-05 01:01:01", 2)
   ) |>
    dplyr::as_tibble()
   DBI::dbWriteTable(con, "all_review_data", review_data)
-  db_get_review(temp_path, ids = 1L)
+  db_get_review(temp_path, id = 1L)
+  db_get_review(temp_path, subject_id = "Test_name2")
 })
 
 }
diff --git a/tests/testthat/test-fct_SQLite.R b/tests/testthat/test-fct_SQLite.R
index 99952dce..a2084ec8 100644
--- a/tests/testthat/test-fct_SQLite.R
+++ b/tests/testthat/test-fct_SQLite.R
@@ -437,12 +437,15 @@ describe("db_get_review can collect latest review data from a database", {
   db_add_log(con, "id")
 
   it("Can collect the desired data.", {
-    output <- db_get_review(temp_path, ids = 1L)
+    output <- db_get_review(temp_path, subject_id = "Test_name", item_group = "Test_group")
     expect_equal(output[,-1], review_data)
   })
   
   it("Collects an empty data frame if the requested ids are not found", {
-    output <- db_get_review(temp_path, ids = 2L)
+    output <- db_get_review(temp_path, subject_id = "Non-existent", 
+                            item_group = "Test_group")
+    output <- db_get_review(temp_path, subject_id = "Test_name", 
+                            item_group = "Non-existent")
     expect_equal(output[,-1], review_data[0,])
   })
 
diff --git a/tests/testthat/test-mod_review_forms.R b/tests/testthat/test-mod_review_forms.R
index 68636846..8dabbc6a 100644
--- a/tests/testthat/test-mod_review_forms.R
+++ b/tests/testthat/test-mod_review_forms.R
@@ -130,7 +130,7 @@ describe(
               )
             
             updated_rows_db <- db_get_review(
-              db_path, ids = 1:2
+              db_path, subject_id = "885", item_group = "Adverse events"
             )
             
             expect_equal(updated_rows_db$comment, c("test review", "test review"))
@@ -230,7 +230,7 @@ describe(
         expect_true(app$get_js("document.getElementById('test-review_comment').disabled;"))
         
         # review status and reviewer is saved as expected
-        saved_review_row <- db_get_review(temp_path, ids = 1:2)
+        saved_review_row <- db_get_review(temp_path, subject_id = "885", item_group = "Adverse events")
         expect_equal(saved_review_row$status, c("old", "old"))
         expect_equal(saved_review_row$reviewer, c("Reviewer 1", "test_name (Medical Monitor)"))
       }

From 6990cf1f64309ff53089728a30da473eddcc8d91 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Wed, 20 Nov 2024 14:47:30 -0500
Subject: [PATCH 11/36] Update version and NEWS

---
 DESCRIPTION                      | 2 +-
 NEWS.md                          | 1 +
 inst/golem-config.yml            | 2 +-
 tests/testthat/test-fct_SQLite.R | 3 ++-
 4 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index b751b562..975c9e76 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: clinsight
 Title: ClinSight
-Version: 0.1.0.9009
+Version: 0.1.1.9010
 Authors@R: c(
     person("Leonard Daniël", "Samson", , "lsamson@gcp-service.com", role = c("cre", "aut"),
            comment = c(ORCID = "0000-0002-6252-7639")),
diff --git a/NEWS.md b/NEWS.md
index 80847cd4..a22c00b8 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -9,6 +9,7 @@
 - Added form type as a class to be used in `create_table()` to display tables.
 - Add a logging table to the DB for reviews.
 - Simplify pulling data from DB for reviews.
+- Review data by records IDs instead of subject & form
 
 ## Bug fixes
 
diff --git a/inst/golem-config.yml b/inst/golem-config.yml
index 9bb6516e..8cbba892 100644
--- a/inst/golem-config.yml
+++ b/inst/golem-config.yml
@@ -1,6 +1,6 @@
 default:
   golem_name: clinsight
-  golem_version: 0.1.0.9009
+  golem_version: 0.1.1.9010
   app_prod: no
   user_identification: test_user
   study_data: !expr clinsight::clinsightful_data
diff --git a/tests/testthat/test-fct_SQLite.R b/tests/testthat/test-fct_SQLite.R
index a2084ec8..35bd86d1 100644
--- a/tests/testthat/test-fct_SQLite.R
+++ b/tests/testthat/test-fct_SQLite.R
@@ -441,9 +441,10 @@ describe("db_get_review can collect latest review data from a database", {
     expect_equal(output[,-1], review_data)
   })
   
-  it("Collects an empty data frame if the requested ids are not found", {
+  it("Collects an empty data frame if the requested subject or form are not found", {
     output <- db_get_review(temp_path, subject_id = "Non-existent", 
                             item_group = "Test_group")
+    expect_equal(output[,-1], review_data[0,])
     output <- db_get_review(temp_path, subject_id = "Test_name", 
                             item_group = "Non-existent")
     expect_equal(output[,-1], review_data[0,])

From 3db42749600259b2700438126946779513ad73ec Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 08:42:49 -0500
Subject: [PATCH 12/36] Update scenario description for two rows

---
 tests/testthat/test-mod_review_forms.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-mod_review_forms.R b/tests/testthat/test-mod_review_forms.R
index 8dabbc6a..c77909cc 100644
--- a/tests/testthat/test-mod_review_forms.R
+++ b/tests/testthat/test-mod_review_forms.R
@@ -252,7 +252,7 @@ describe(
         and [active_form] set to 'Adverse events',
         and [active_tab] set to 'Common forms',
         and [form_reviewed] set to FALSE,
-        I expect that the data frame [active_review_data] contains one row with 
+        I expect that the data frame [active_review_data] contains two rows with 
         data of participant '885',
         and with the [item_group] set to 'Adverse events',
         and that a message will be displayed containing the text 'Requires review'", 

From 1c675a98a7c765645b4c14e6dcceb0f217dc0f01 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 08:44:59 -0500
Subject: [PATCH 13/36] Update language from row to records

---
 R/mod_review_forms.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 47415bad..ed392a57 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -237,7 +237,7 @@ mod_review_forms_server <- function(
           status      = if(input$form_reviewed) "old" else "new"
         ) 
       
-      golem::cat_dev("review row to add:\n")
+      golem::cat_dev("review records to add:\n")
       golem::print_dev(review_records)
       
       cat("write review progress to database\n")

From c177a300f277efcb2aa62efdded9274a18a21fd0 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 08:48:07 -0500
Subject: [PATCH 14/36] Grab un-duplicated `rv_records`

---
 R/fct_SQLite.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index cd7035ee..f1f8c755 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -337,7 +337,7 @@ db_save_review <- function(
   stopifnot(is.data.frame(rv_records))
   if (any(duplicated(rv_records[["id"]]))) {
     warning("duplicate records detected to save in database. Only the first will be selected.")
-    rv_records[!duplicated(rv_records[["id"]]),]
+    rv_records <- rv_records[!duplicated(rv_records[["id"]]),]
   }
 
   cols_to_change <- c("reviewed", "comment", "reviewer", "timestamp", "status")

From d2fa0d65df5aae4431ee52f0b22a5b250714a81b Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 08:50:22 -0500
Subject: [PATCH 15/36] Remove unnecessary `dplyr::select()`

---
 R/mod_review_forms.R | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index ed392a57..36a87dcb 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -112,8 +112,7 @@ mod_review_forms_server <- function(
     review_data_active <- reactive({
       r$review_data |>
         dplyr::filter(subject_id == r$subject_id, 
-                      item_group == active_form()) |> 
-        dplyr::select(id, dplyr::all_of(idx_cols), edit_date_time, reviewed, comment, status)
+                      item_group == active_form())
     }) 
     
     observeEvent(c(active_form(), r$subject_id), {

From de247774e86cce34f6f1a02151f3fb698e540a83 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 09:00:13 -0500
Subject: [PATCH 16/36] Remove unneeded `dplyr::distinct()`

---
 R/mod_review_forms.R | 1 -
 1 file changed, 1 deletion(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 36a87dcb..333959fd 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -227,7 +227,6 @@ mod_review_forms_server <- function(
       golem::cat_dev("Save review status reviewed:", input$form_reviewed, "\n")
       
       review_records <- review_data_active() |> 
-        dplyr::distinct(id) |>
         dplyr::mutate(
           reviewed    = if(input$form_reviewed) "Yes" else "No",
           comment     = ifelse(is.null(input$review_comment), "", input$review_comment),

From d372afa1295202a59f13b3bde44060f9f988371a Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 09:05:05 -0500
Subject: [PATCH 17/36] Return `db_get_review()` when `...` is empty

---
 R/fct_SQLite.R | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index f1f8c755..a26707e8 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -497,10 +497,20 @@ db_get_review <- function(
   stopifnot(file.exists(db_path))
   db_temp_connect(db_path, {
     fields <- ...names()
-    sql <- paste("SELECT * FROM ?db_table WHERE", paste0(fields, " = $", fields, collapse = " AND "))
+    if (is.null(fields)) {
+      if (...length() > 0)
+        warning("Unnamed arguments passed in `...`. Returning full data table.")
+      else
+        warning("No arguments passed in `...`. Returning full data table.")
+      conditionals <- "true"
+    } else {
+      conditionals <- paste0(fields, " = $", fields, collapse = " AND ")
+    }
+    sql <- paste("SELECT * FROM ?db_table WHERE", conditionals)
     query <- DBI::sqlInterpolate(con, sql, db_table = db_table[1])
     rs <- DBI::dbSendQuery(con, query)
-    DBI::dbBind(rs, params = data.frame(...))
+    if (!is.null(fields))
+      DBI::dbBind(rs, params = data.frame(...))
     df <- 
       DBI::dbFetch(rs) |> 
       dplyr::as_tibble()

From f28f1747b571d239d6be403fa6776a567e1f35cb Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 09:53:30 -0500
Subject: [PATCH 18/36] Update test-mod_review_forms.R

---
 tests/testthat/test-mod_review_forms.R | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/testthat/test-mod_review_forms.R b/tests/testthat/test-mod_review_forms.R
index c77909cc..99969a50 100644
--- a/tests/testthat/test-mod_review_forms.R
+++ b/tests/testthat/test-mod_review_forms.R
@@ -280,8 +280,7 @@ describe(
           session$setInputs(form_reviewed = FALSE)
           expect_equal(
             review_data_active(),
-            dplyr::filter(r$review_data, subject_id == "885", item_group == "Adverse events") |> 
-              dplyr::select(id, dplyr::all_of(idx_cols), edit_date_time, reviewed, comment, status)
+            dplyr::filter(r$review_data, subject_id == "885", item_group == "Adverse events")
           )
           expect_equal(review_data_active()$item_group, c("Adverse events", "Adverse events"))
           expect_equal(nrow(review_data_active()), 2)

From 7e26f83330703d3b14625a05afaf4f360ce4d523 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 10:19:25 -0500
Subject: [PATCH 19/36] Update only one table in `db_save_review()`

---
 R/fct_SQLite.R                   | 13 +++++++------
 R/mod_review_forms.R             |  4 +---
 man/db_save_review.Rd            |  4 ++--
 tests/testthat/test-fct_SQLite.R |  6 +++---
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index a26707e8..f49ba438 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -322,7 +322,7 @@ db_upsert <- function(con, data, idx_cols) {
 #' @param rv_records A data frame containing the rows of data that needs to be
 #'   checked.
 #' @param db_path Character vector. Path to the database.
-#' @param tables Character vector. Names of the tables within the database to
+#' @param table Character vector. Names of the table within the database to
 #'   save the review in.
 #'
 #' @return Review information will be written in the database. No local objects
@@ -332,9 +332,10 @@ db_upsert <- function(con, data, idx_cols) {
 db_save_review <- function(
     rv_records,
     db_path,
-    tables = c("all_review_data")
+    table = "all_review_data"
 ){
   stopifnot(is.data.frame(rv_records))
+  stopifnot(is.character(table) && length(table) != 1)
   if (any(duplicated(rv_records[["id"]]))) {
     warning("duplicate records detected to save in database. Only the first will be selected.")
     rv_records <- rv_records[!duplicated(rv_records[["id"]]),]
@@ -350,18 +351,18 @@ db_save_review <- function(
   dplyr::copy_to(db_con, rv_records, "row_updates")
   rs <- DBI::dbSendStatement(db_con, paste(
     "UPDATE",
-    tables,
+    table,
     "SET",
     sprintf("%1$s = row_updates.%1$s", cols_to_change) |> paste(collapse = ", "),
     "FROM",
     "row_updates",
     "WHERE",
-    sprintf("%s.id = row_updates.id", tables),
+    sprintf("%s.id = row_updates.id", table),
     "AND",
-    sprintf("%s.reviewed <> row_updates.reviewed", tables)
+    sprintf("%s.reviewed <> row_updates.reviewed", table)
   ))
   DBI::dbClearResult(rs)
-  cat("finished writing to the tables:", tables, "\n")
+  cat("finished writing to the table:", table, "\n")
 }
 
 #' Append database table
diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 333959fd..cfd24e55 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -242,9 +242,7 @@ mod_review_forms_server <- function(
       db_save_review(
         review_records, 
         db_path = db_path,
-        # More tables can be added here if needed, to track process of 
-        # individual reviewers in individual tables:
-        tables = "all_review_data" 
+        table = "all_review_data" 
       )
       
       updated_rows_db <- db_get_review(
diff --git a/man/db_save_review.Rd b/man/db_save_review.Rd
index a176183a..50da6d98 100644
--- a/man/db_save_review.Rd
+++ b/man/db_save_review.Rd
@@ -4,7 +4,7 @@
 \alias{db_save_review}
 \title{Save review in database}
 \usage{
-db_save_review(rv_records, db_path, tables = c("all_review_data"))
+db_save_review(rv_records, db_path, table = "all_review_data")
 }
 \arguments{
 \item{rv_records}{A data frame containing the rows of data that needs to be
@@ -12,7 +12,7 @@ checked.}
 
 \item{db_path}{Character vector. Path to the database.}
 
-\item{tables}{Character vector. Names of the tables within the database to
+\item{table}{Character vector. Names of the table within the database to
 save the review in.}
 }
 \value{
diff --git a/tests/testthat/test-fct_SQLite.R b/tests/testthat/test-fct_SQLite.R
index 35bd86d1..fc0f799a 100644
--- a/tests/testthat/test-fct_SQLite.R
+++ b/tests/testthat/test-fct_SQLite.R
@@ -267,7 +267,7 @@ describe(
       db_save_review(
         cbind(id = 1, df, new_review), 
         temp_path, 
-        tables = c("all_review_data")
+        table = c("all_review_data")
       )
       expect_equal(
         dplyr::collect(dplyr::tbl(con, "all_review_data")), 
@@ -317,7 +317,7 @@ describe(
       db_save_review(
         review_row, 
         temp_path, 
-        tables = c("all_review_data")
+        table = c("all_review_data")
       )
       expect_true(is.data.frame(dplyr::collect(dplyr::tbl(con, "all_review_data"))))
       results <- dplyr::collect(dplyr::tbl(con, "all_review_data"))
@@ -347,7 +347,7 @@ describe(
       db_save_review(
         rbind(cbind(id = 1:2, df, new_review), cbind(id = 1:2, df, new_review)), 
         temp_path, 
-        tables = "all_review_data"
+        table = "all_review_data"
       ) |> expect_warning()
     })
   }

From ad3f05eb9f67fa37cdd6f902779c3066dbd19dbe Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Thu, 21 Nov 2024 10:40:40 -0500
Subject: [PATCH 20/36] Fix oopsies

---
 R/fct_SQLite.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index f49ba438..abbed12f 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -335,7 +335,7 @@ db_save_review <- function(
     table = "all_review_data"
 ){
   stopifnot(is.data.frame(rv_records))
-  stopifnot(is.character(table) && length(table) != 1)
+  stopifnot(is.character(table) && length(table) == 1)
   if (any(duplicated(rv_records[["id"]]))) {
     warning("duplicate records detected to save in database. Only the first will be selected.")
     rv_records <- rv_records[!duplicated(rv_records[["id"]]),]

From 439d14bae8d81f06ecb7b705c82cd9eb3b309a5e Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Fri, 22 Nov 2024 08:32:29 -0500
Subject: [PATCH 21/36] Add test for `db_get_review()` when no filters are
 specified

---
 tests/testthat/test-fct_SQLite.R | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/testthat/test-fct_SQLite.R b/tests/testthat/test-fct_SQLite.R
index fc0f799a..26552cdc 100644
--- a/tests/testthat/test-fct_SQLite.R
+++ b/tests/testthat/test-fct_SQLite.R
@@ -449,5 +449,10 @@ describe("db_get_review can collect latest review data from a database", {
                             item_group = "Non-existent")
     expect_equal(output[,-1], review_data[0,])
   })
+  
+  it("Throws a warning if no filters are specified and returns full table", {
+    expect_warning(output <- db_get_review(temp_path))
+    expect_equal(output[,-1], review_data)
+  })
 
 })

From 00793482297c18c504a9bca7ed7df72d09ea62a0 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Date: Fri, 22 Nov 2024 08:48:33 -0500
Subject: [PATCH 22/36] Fix `updated_items_memory`

---
 R/mod_review_forms.R | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index cfd24e55..ac31f7dd 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -226,7 +226,7 @@ mod_review_forms_server <- function(
       review_save_error(FALSE)
       golem::cat_dev("Save review status reviewed:", input$form_reviewed, "\n")
       
-      review_records <- review_data_active() |> 
+      review_records <- review_data_active()["id"] |> 
         dplyr::mutate(
           reviewed    = if(input$form_reviewed) "Yes" else "No",
           comment     = ifelse(is.null(input$review_comment), "", input$review_comment),
@@ -260,10 +260,11 @@ mod_review_forms_server <- function(
           dplyr::rows_update(review_records, by = "id")
       }
       
-      updated_items_memory <- review_records |> 
-        dplyr::left_join(r$review_data, by = "id", suffix = c("", ".y")) |> 
-        dplyr::select(dplyr::all_of(names(review_records))) |> 
-        dplyr::filter(timestamp == review_records$timestamp[1])
+      updated_items_memory <- r$review_data |> 
+        dplyr::filter(
+          id %in% review_records$id,
+          timestamp == review_records$timestamp[1]
+          )
       
       review_save_error(any(
         !isTRUE(all.equal(review_records_db, review_records, check.attributes = FALSE)),

From ca8399722da321b72d3d8751b65a5662bcb1bdd9 Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 14:04:37 +0100
Subject: [PATCH 23/36] Don't export db_get_review anymore

---
 NAMESPACE      |  1 -
 R/fct_SQLite.R | 23 +----------------------
 2 files changed, 1 insertion(+), 23 deletions(-)

diff --git a/NAMESPACE b/NAMESPACE
index 09118691..6db89e4c 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -27,7 +27,6 @@ export(datatable_custom)
 export(date_cols_to_char)
 export(db_create)
 export(db_get_query)
-export(db_get_review)
 export(db_save)
 export(db_save_review)
 export(db_slice_rows)
diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index abbed12f..829dacd5 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -467,28 +467,7 @@ db_get_query <- function(
 #'   used if `data` is a character string to a database.
 #'
 #' @return A data frame.
-#' @export
-#'
-#' @examples
-#'
-#' local({
-#'   temp_path <- withr::local_tempfile(fileext = ".sqlite")
-#'   con <- get_db_connection(temp_path)
-#'   review_data <- data.frame(
-#'    subject_id = c("Test_name", "Test_name2"),
-#'    id = 1:2,
-#'    event_name = c("Visit 1", "Visit 1"),
-#'    item_group = c("Test_group", "Test_group2"),
-#'    form_repeat = c(1, 1),
-#'    item_name = c("Test_item", "Test_item2"),
-#'    edit_date_time = rep("2023-11-05 01:26:00", 2),
-#'    timestamp = rep("2024-02-05 01:01:01", 2)
-#'   ) |>
-#'    dplyr::as_tibble()
-#'   DBI::dbWriteTable(con, "all_review_data", review_data)
-#'   db_get_review(temp_path, id = 1L)
-#'   db_get_review(temp_path, subject_id = "Test_name2")
-#' })
+#' @keywords internal
 #' 
 db_get_review <- function(
     db_path, 

From 0f680cd393d23c7973ba72f9c2b37881ab5fd96f Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 14:05:23 +0100
Subject: [PATCH 24/36] Update documentation

---
 R/fct_SQLite.R       |  7 +++++--
 man/db_get_review.Rd | 30 ++++++------------------------
 2 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index 829dacd5..a4fc2d7c 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -461,8 +461,11 @@ db_get_query <- function(
 #'
 #' @param db_path Character vector. Needs to be a valid path to a database.
 #' @param ... Named arguments specifying which records to retrieve, see
-#'   examples. Note that `...` will be processed with `data.frame()` since
-#'   parameters must have equal length.
+#'   examples. Note that `...` will be processed with `data.frame()` and thus
+#'   the arguments within `...` should be convertible to a data frame. This is
+#'   chosen so that filters of length one can be used with other filters since
+#'   they will be recycled (for example, when selecting multiple events of one
+#'   subject). 
 #' @param db_table Character string. Name of the table to collect. Will only be
 #'   used if `data` is a character string to a database.
 #'
diff --git a/man/db_get_review.Rd b/man/db_get_review.Rd
index 17456098..cdbaa6d3 100644
--- a/man/db_get_review.Rd
+++ b/man/db_get_review.Rd
@@ -10,8 +10,11 @@ db_get_review(db_path, ..., db_table = "all_review_data")
 \item{db_path}{Character vector. Needs to be a valid path to a database.}
 
 \item{...}{Named arguments specifying which records to retrieve, see
-examples. Note that \code{...} will be processed with \code{data.frame()} since
-parameters must have equal length.}
+examples. Note that \code{...} will be processed with \code{data.frame()} and thus
+the arguments within \code{...} should be convertible to a data frame. This is
+chosen so that filters of length one can be used with other filters since
+they will be recycled (for example, when selecting multiple events of one
+subject).}
 
 \item{db_table}{Character string. Name of the table to collect. Will only be
 used if \code{data} is a character string to a database.}
@@ -23,25 +26,4 @@ A data frame.
 Small helper function to retrieve the (latest) review data from the database
 with the given subject id (\code{subject}) and \code{form}.
 }
-\examples{
-
-local({
-  temp_path <- withr::local_tempfile(fileext = ".sqlite")
-  con <- get_db_connection(temp_path)
-  review_data <- data.frame(
-   subject_id = c("Test_name", "Test_name2"),
-   id = 1:2,
-   event_name = c("Visit 1", "Visit 1"),
-   item_group = c("Test_group", "Test_group2"),
-   form_repeat = c(1, 1),
-   item_name = c("Test_item", "Test_item2"),
-   edit_date_time = rep("2023-11-05 01:26:00", 2),
-   timestamp = rep("2024-02-05 01:01:01", 2)
-  ) |>
-   dplyr::as_tibble()
-  DBI::dbWriteTable(con, "all_review_data", review_data)
-  db_get_review(temp_path, id = 1L)
-  db_get_review(temp_path, subject_id = "Test_name2")
-})
-
-}
+\keyword{internal}

From a08c5106e2cd351c7d40ad1318cd7d2f673ab3a6 Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 14:07:41 +0100
Subject: [PATCH 25/36] Ensure db_get_review errors before sending a db query
 if `...` cannot be coerced to a data frame

---
 R/fct_SQLite.R | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/R/fct_SQLite.R b/R/fct_SQLite.R
index a4fc2d7c..5df41a3e 100644
--- a/R/fct_SQLite.R
+++ b/R/fct_SQLite.R
@@ -478,24 +478,25 @@ db_get_review <- function(
     db_table = "all_review_data"
 ){
   stopifnot(file.exists(db_path))
-  db_temp_connect(db_path, {
-    fields <- ...names()
-    if (is.null(fields)) {
-      if (...length() > 0)
-        warning("Unnamed arguments passed in `...`. Returning full data table.")
-      else
-        warning("No arguments passed in `...`. Returning full data table.")
-      conditionals <- "true"
+  fields <- ...names()
+  if (is.null(fields)) {
+    if (...length() > 0) {
+      warning("Unnamed arguments passed in `...`. Returning full data table.")
     } else {
-      conditionals <- paste0(fields, " = $", fields, collapse = " AND ")
+      warning("No arguments passed in `...`. Returning full data table.")
     }
+    conditionals <- "true"
+  } else {
+    conditionals <- paste0(fields, " = $", fields, collapse = " AND ")
+    parameters <- data.frame(...)
+  }
+  
+  db_temp_connect(db_path, {
     sql <- paste("SELECT * FROM ?db_table WHERE", conditionals)
     query <- DBI::sqlInterpolate(con, sql, db_table = db_table[1])
     rs <- DBI::dbSendQuery(con, query)
-    if (!is.null(fields))
-      DBI::dbBind(rs, params = data.frame(...))
-    df <- 
-      DBI::dbFetch(rs) |> 
+    if (!is.null(fields)) DBI::dbBind(rs, params = parameters)
+    df <- DBI::dbFetch(rs) |> 
       dplyr::as_tibble()
     DBI::dbClearResult(rs)
     df

From f7ba04a42ca0eb32e0ff4ca2f118a107aa225afb Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 14:08:05 +0100
Subject: [PATCH 26/36] Add additional tests

---
 tests/testthat/test-fct_SQLite.R | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tests/testthat/test-fct_SQLite.R b/tests/testthat/test-fct_SQLite.R
index 26552cdc..1539bffb 100644
--- a/tests/testthat/test-fct_SQLite.R
+++ b/tests/testthat/test-fct_SQLite.R
@@ -454,5 +454,24 @@ describe("db_get_review can collect latest review data from a database", {
     expect_warning(output <- db_get_review(temp_path))
     expect_equal(output[,-1], review_data)
   })
+  
+  it("Throws a warning if specified filters are unnamed and returns full table", {
+    expect_warning(
+      output <- db_get_review(temp_path, "Test_name"), 
+      "Unnamed arguments passed"
+      )
+    expect_equal(output[,-1], review_data)
+  })
+  
+  it("Errors if provided filters cannot be coerced to a data frame", {
+    expect_error(
+      db_get_review(
+        temp_path, 
+        event_name = c("Visit 1", "Visit 2"), 
+        subject_id = c("Test_name", "another name", "third name")
+        ), 
+      "arguments imply differing number of rows"
+    )
+  })
 
 })

From 205896f21bcb842d7a2c38feadeb7c622c9f8f72 Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 14:26:01 +0100
Subject: [PATCH 27/36] Use more consistent naming in the save review process

---
 R/mod_review_forms.R | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index ac31f7dd..c11c9248 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -260,7 +260,7 @@ mod_review_forms_server <- function(
           dplyr::rows_update(review_records, by = "id")
       }
       
-      updated_items_memory <- r$review_data |> 
+      updated_records_memory <- r$review_data |> 
         dplyr::filter(
           id %in% review_records$id,
           timestamp == review_records$timestamp[1]
@@ -268,7 +268,7 @@ mod_review_forms_server <- function(
       
       review_save_error(any(
         !isTRUE(all.equal(review_records_db, review_records, check.attributes = FALSE)),
-        !isTRUE(all.equal(updated_items_memory, review_records_db, check.attributes = FALSE))
+        !isTRUE(all.equal(updated_records_memory, review_records_db, check.attributes = FALSE))
       ))
       
       if(review_save_error()){

From b8f3f52ab07da74b04df65c21c63613559a28502 Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 14:41:03 +0100
Subject: [PATCH 28/36] Change a dplyr::filter call to base R filter for
 performance reasons

---
 R/mod_review_forms.R | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index c11c9248..9983e062 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -260,11 +260,10 @@ mod_review_forms_server <- function(
           dplyr::rows_update(review_records, by = "id")
       }
       
-      updated_records_memory <- r$review_data |> 
-        dplyr::filter(
-          id %in% review_records$id,
-          timestamp == review_records$timestamp[1]
-          )
+      updated_records_memory <- with(r$review_data, r$review_data[
+        id %in% review_records$id &
+        timestamp == review_records$timestamp[1],
+      ])
       
       review_save_error(any(
         !isTRUE(all.equal(review_records_db, review_records, check.attributes = FALSE)),

From 0154355b800cdddd78607999109526d846e784da Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 16:16:39 +0100
Subject: [PATCH 29/36] Ensure that updated_records_memory has the same columns
 as in the db

---
 R/mod_review_forms.R | 1 +
 1 file changed, 1 insertion(+)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 9983e062..24bfb0b5 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -263,6 +263,7 @@ mod_review_forms_server <- function(
       updated_records_memory <- with(r$review_data, r$review_data[
         id %in% review_records$id &
         timestamp == review_records$timestamp[1],
+        names(review_records_db)
       ])
       
       review_save_error(any(

From f0144a2353ba6c2d6d854b7ced27183cd2d752c7 Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 16:22:17 +0100
Subject: [PATCH 30/36] Ensure tests will fail if there is a review discrepancy

---
 tests/testthat/test-mod_review_forms.R | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/testthat/test-mod_review_forms.R b/tests/testthat/test-mod_review_forms.R
index 99969a50..4ef10412 100644
--- a/tests/testthat/test-mod_review_forms.R
+++ b/tests/testthat/test-mod_review_forms.R
@@ -54,7 +54,8 @@ describe(
              "and [form_reviewed] set to FALSE, ",
              "I expect that I can save a new review properly, ",
              "with the result saved in the application being the same as ", 
-             "the one saved in the database."),
+             "the one saved in the database, ",
+             "and no review error occurring"),
       {
         temp_path <- withr::local_tempfile(fileext = ".sqlite")
         file.copy(test_path("fixtures", "review_testdb.sqlite"), temp_path) 
@@ -97,6 +98,7 @@ describe(
             db_reviewdata <- db_get_table(db_path)
             db_reviewlogdata <- db_get_table(db_path, "all_review_data_log")
             
+            expect_false(review_save_error())
             # app data should be equal to DB data
             expect_equal(r$review_data, db_reviewdata)
             # review table should only have one row in the DB containing the new reviewed = "Yes"
@@ -145,6 +147,7 @@ describe(
                 dplyr::collect()
             })
             
+            expect_false(review_save_error())
             expect_equal(with(db_reviewdata, comment[subject_id == "885"]), c("test review", "test review"))
             expect_equal(with(db_reviewdata, reviewed[subject_id == "885"]), c("No", "No"))
             r_id <- with(db_reviewdata, id[subject_id == "885"])

From 5360760b96c0c9d53f4335191873ea43d9c245cb Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 16:23:23 +0100
Subject: [PATCH 31/36] Export review_save_error to ensure errors are captured
 when saving review in in a shinytest2 test

---
 R/mod_review_forms.R | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 24bfb0b5..e8491631 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -314,6 +314,9 @@ mod_review_forms_server <- function(
       validate(need(input$form_reviewed, "Requires review"))
     })
     
+    shiny::exportTestValues(
+      review_save_error = review_save_error()
+    )
   })
 }
 

From 88f5b0410f976b1b878d521d869ac3f9b524b3ca Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 16:42:23 +0100
Subject: [PATCH 32/36] Update snaps

---
 tests/testthat/_snaps/mod_review_forms.md                       | 2 +-
 .../_snaps/mod_review_forms/test-mod_review_forms-001.json      | 2 +-
 .../_snaps/mod_review_forms/test-mod_review_forms-002.json      | 2 +-
 .../_snaps/mod_review_forms/test-mod_review_forms-003.json      | 2 +-
 .../mod_review_forms/test-mod_review_forms_no_user-001.json     | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/testthat/_snaps/mod_review_forms.md b/tests/testthat/_snaps/mod_review_forms.md
index 646ff879..6990b8e8 100644
--- a/tests/testthat/_snaps/mod_review_forms.md
+++ b/tests/testthat/_snaps/mod_review_forms.md
@@ -1,4 +1,4 @@
-# mod_review_forms. Feature 2 | Save review of a form. As a user, I want to be able to save a review of a form in the database. After saving the review, all items of that form that are not yet reviewed should get a tag that the value was reviewed.: Scenario 1 - Save a review. Given test review data with at least an 'Adverse event' form with patient '885',and [user_name] set to 'test_name' and [user_role] to 'Medical Monitor'and [active_patient] set to '885', and [active_form] set to 'Adverse events', and [active_tab] set to 'Common forms', and [form_reviewed] set to FALSE, I expect that I can save a new review properly, with the result saved in the application being the same as the one saved in the database.
+# mod_review_forms. Feature 2 | Save review of a form. As a user, I want to be able to save a review of a form in the database. After saving the review, all items of that form that are not yet reviewed should get a tag that the value was reviewed.: Scenario 1 - Save a review. Given test review data with at least an 'Adverse event' form with patient '885',and [user_name] set to 'test_name' and [user_role] to 'Medical Monitor'and [active_patient] set to '885', and [active_form] set to 'Adverse events', and [active_tab] set to 'Common forms', and [form_reviewed] set to FALSE, I expect that I can save a new review properly, with the result saved in the application being the same as the one saved in the database, and no review error occurring
 
     Code
       dplyr::select(r$review_data, -timestamp)
diff --git a/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-001.json b/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-001.json
index 56fb012a..a606f20a 100644
--- a/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-001.json
+++ b/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-001.json
@@ -17,6 +17,6 @@
     }
   },
   "export": {
-
+    "test-review_save_error": false
   }
 }
diff --git a/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-002.json b/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-002.json
index 422f89c1..e5be6ef2 100644
--- a/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-002.json
+++ b/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-002.json
@@ -10,6 +10,6 @@
     "test-save_review_error": ""
   },
   "export": {
-
+    "test-review_save_error": false
   }
 }
diff --git a/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-003.json b/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-003.json
index 232d2112..82026230 100644
--- a/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-003.json
+++ b/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms-003.json
@@ -17,6 +17,6 @@
     }
   },
   "export": {
-
+    "test-review_save_error": false
   }
 }
diff --git a/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms_no_user-001.json b/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms_no_user-001.json
index dfed2054..8a246b82 100644
--- a/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms_no_user-001.json
+++ b/tests/testthat/_snaps/mod_review_forms/test-mod_review_forms_no_user-001.json
@@ -17,6 +17,6 @@
     }
   },
   "export": {
-
+    "test-review_save_error": false
   }
 }

From 23b6e91538578950965ce8313ae72038220a4394 Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Thu, 28 Nov 2024 17:01:21 +0100
Subject: [PATCH 33/36] Also update mod_main_sidebar snaps

---
 .../_snaps/mod_main_sidebar/test-mod_main_sidebar-001.json      | 2 +-
 .../mod_main_sidebar/test-mod_main_sidebar-scen2-001.json       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/testthat/_snaps/mod_main_sidebar/test-mod_main_sidebar-001.json b/tests/testthat/_snaps/mod_main_sidebar/test-mod_main_sidebar-001.json
index cf902724..0d7f0d6c 100644
--- a/tests/testthat/_snaps/mod_main_sidebar/test-mod_main_sidebar-001.json
+++ b/tests/testthat/_snaps/mod_main_sidebar/test-mod_main_sidebar-001.json
@@ -40,6 +40,6 @@
     "test-write_query-reviewer": "Author: Admin (Medical Monitor)"
   },
   "export": {
-
+    "test-review_forms_1-review_save_error": false
   }
 }
diff --git a/tests/testthat/_snaps/mod_main_sidebar/test-mod_main_sidebar-scen2-001.json b/tests/testthat/_snaps/mod_main_sidebar/test-mod_main_sidebar-scen2-001.json
index b0933a72..8e3bfc3b 100644
--- a/tests/testthat/_snaps/mod_main_sidebar/test-mod_main_sidebar-scen2-001.json
+++ b/tests/testthat/_snaps/mod_main_sidebar/test-mod_main_sidebar-scen2-001.json
@@ -15,6 +15,6 @@
     "test-synch_info-db_synch_info": "EDC Sync date: <br>Unknown<br><br>EDC latest data: Unknown"
   },
   "export": {
-
+    "test-review_forms_1-review_save_error": false
   }
 }

From 6ce58279058340f92a82d0aeeebe08a31efd089a Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Mon, 2 Dec 2024 11:48:27 +0100
Subject: [PATCH 34/36] Change to base R filter

---
 R/mod_review_forms.R | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index e8491631..cf14c23a 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -110,9 +110,9 @@ mod_review_forms_server <- function(
     ns <- session$ns
     
     review_data_active <- reactive({
-      r$review_data |>
-        dplyr::filter(subject_id == r$subject_id, 
-                      item_group == active_form())
+      with(r$review_data, r$review_data[
+        subject_id == r$subject_id & item_group == active_form(),
+        ])
     }) 
     
     observeEvent(c(active_form(), r$subject_id), {

From 0e8ecc70b50d28b96c6f79558dbf20a1bedfc2b7 Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Mon, 2 Dec 2024 11:50:00 +0100
Subject: [PATCH 35/36] Fix review saving by selecting correct rows to update
 within the application

---
 R/mod_review_forms.R | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index cf14c23a..3d7437c4 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -226,7 +226,12 @@ mod_review_forms_server <- function(
       review_save_error(FALSE)
       golem::cat_dev("Save review status reviewed:", input$form_reviewed, "\n")
       
-      review_records <- review_data_active()["id"] |> 
+      old_review_status <- if (!input$form_reviewed) "Yes" else "No"
+      review_records <- review_data_active()[
+        review_data_active()$reviewed == old_review_status,
+        "id", 
+        drop = FALSE
+        ] |> 
         dplyr::mutate(
           reviewed    = if(input$form_reviewed) "Yes" else "No",
           comment     = ifelse(is.null(input$review_comment), "", input$review_comment),
@@ -245,16 +250,11 @@ mod_review_forms_server <- function(
         table = "all_review_data" 
       )
       
-      updated_rows_db <- db_get_review(
+      review_records_db <- db_get_review(
         db_path, id = review_records$id
-        ) |> 
-        dplyr::select(dplyr::all_of(names(review_records)))
+      )[, names(review_records)] 
       
-      review_records_db <- updated_rows_db |> 
-        # Within a form, only items with a changed review state are updated and 
-        # contain the new (current) time stamp. 
-        dplyr::filter(timestamp == review_records$timestamp[1])
-      if(isTRUE(all.equal(review_records_db, review_records, check.attributes = FALSE))){
+      if (isTRUE(all.equal(review_records_db, review_records, check.attributes = FALSE))){
         cat("Update review data and status in app\n")
         r$review_data <- r$review_data |> 
           dplyr::rows_update(review_records, by = "id")

From d8e24d44526dbfcb0345d2ea665d11f53b0d5cd9 Mon Sep 17 00:00:00 2001
From: LDSamson <l.d.samson@gmail.com>
Date: Mon, 2 Dec 2024 11:56:59 +0100
Subject: [PATCH 36/36] Remove the (now) redundant filtering by timestamp for
 updated_records_memory

---
 R/mod_review_forms.R | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/R/mod_review_forms.R b/R/mod_review_forms.R
index 3d7437c4..2cea4353 100644
--- a/R/mod_review_forms.R
+++ b/R/mod_review_forms.R
@@ -260,11 +260,10 @@ mod_review_forms_server <- function(
           dplyr::rows_update(review_records, by = "id")
       }
       
-      updated_records_memory <- with(r$review_data, r$review_data[
-        id %in% review_records$id &
-        timestamp == review_records$timestamp[1],
+      updated_records_memory <- r$review_data[
+        r$review_data$id %in% review_records$id,
         names(review_records_db)
-      ])
+      ]
       
       review_save_error(any(
         !isTRUE(all.equal(review_records_db, review_records, check.attributes = FALSE)),