Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review by records instead of subject_id & form_id "rv_row" #137

Merged
merged 39 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
7c58504
Migrate to `review_records` from `review_row`
jthompson-arcus Nov 13, 2024
b271add
Update test-fct_SQLite.R
jthompson-arcus Nov 13, 2024
39d8da1
Update test-mod_review_forms.R
jthompson-arcus Nov 13, 2024
43931ed
Update test-mod_main_sidebar.R
jthompson-arcus Nov 13, 2024
d311ab1
Fix R CMD checks
jthompson-arcus Nov 13, 2024
ccd9bb1
Fix `db_get_review()` example
jthompson-arcus Nov 13, 2024
0743880
Helps if you update documentation
jthompson-arcus Nov 13, 2024
e5e8acb
Only UPDATE reviews when review status is changed
jthompson-arcus Nov 14, 2024
2551970
Resolve merge conflicts with `jt-113-simplify_review_process`
jthompson-arcus Nov 19, 2024
d863300
Keep current structure for enabling reviews
jthompson-arcus Nov 19, 2024
6ad27a6
Resolve merge conflict with `dev`
jthompson-arcus Nov 20, 2024
a23b462
Generalize `db_get_review()`
jthompson-arcus Nov 20, 2024
6990cf1
Update version and NEWS
jthompson-arcus Nov 20, 2024
3db4274
Update scenario description for two rows
jthompson-arcus Nov 21, 2024
1c675a9
Update language from row to records
jthompson-arcus Nov 21, 2024
c177a30
Grab un-duplicated `rv_records`
jthompson-arcus Nov 21, 2024
d2fa0d6
Remove unnecessary `dplyr::select()`
jthompson-arcus Nov 21, 2024
de24777
Remove unneeded `dplyr::distinct()`
jthompson-arcus Nov 21, 2024
d372afa
Return `db_get_review()` when `...` is empty
jthompson-arcus Nov 21, 2024
f28f174
Update test-mod_review_forms.R
jthompson-arcus Nov 21, 2024
7e26f83
Update only one table in `db_save_review()`
jthompson-arcus Nov 21, 2024
ad3f05e
Fix oopsies
jthompson-arcus Nov 21, 2024
939566f
Fix merge conflict with `dev`
jthompson-arcus Nov 21, 2024
439d14b
Add test for `db_get_review()` when no filters are specified
jthompson-arcus Nov 22, 2024
0079348
Fix `updated_items_memory`
jthompson-arcus Nov 22, 2024
ca83997
Don't export db_get_review anymore
LDSamson Nov 28, 2024
0f680cd
Update documentation
LDSamson Nov 28, 2024
a08c510
Ensure db_get_review errors before sending a db query if `...` cannot…
LDSamson Nov 28, 2024
f7ba04a
Add additional tests
LDSamson Nov 28, 2024
205896f
Use more consistent naming in the save review process
LDSamson Nov 28, 2024
b8f3f52
Change a dplyr::filter call to base R filter for performance reasons
LDSamson Nov 28, 2024
0154355
Ensure that updated_records_memory has the same columns as in the db
LDSamson Nov 28, 2024
f0144a2
Ensure tests will fail if there is a review discrepancy
LDSamson Nov 28, 2024
5360760
Export review_save_error to ensure errors are captured when saving re…
LDSamson Nov 28, 2024
88f5b04
Update snaps
LDSamson Nov 28, 2024
23b6e91
Also update mod_main_sidebar snaps
LDSamson Nov 28, 2024
6ce5827
Change to base R filter
LDSamson Dec 2, 2024
0e8ecc7
Fix review saving by selecting correct rows to update within the appl…
LDSamson Dec 2, 2024
d8e24d4
Remove the (now) redundant filtering by timestamp for updated_records…
LDSamson Dec 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: clinsight
Title: ClinSight
Version: 0.1.0.9010
Version: 0.1.1.9011
Authors@R: c(
person("Leonard Daniël", "Samson", , "[email protected]", role = c("cre", "aut"),
comment = c(ORCID = "0000-0002-6252-7639")),
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
111 changes: 54 additions & 57 deletions R/fct_SQLite.R
Original file line number Diff line number Diff line change
Expand Up @@ -319,63 +319,50 @@
#' 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
#' @param table Character vector. Names of the table within the database to
#' save the review in.
#' @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"),
review_by = c("subject_id", "item_group")
table = "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))
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"]]),]
}

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::filter(reviewed != new_review_state) |>
dplyr::collect()
if(nrow(new_review_rows) == 0){return(
if(nrow(rv_records) == 0){return(

Check warning on line 346 in R/fct_SQLite.R

View check run for this annotation

Codecov / codecov/patch

R/fct_SQLite.R#L346

Added line #L346 was not covered by tests
warning("Review state unaltered. No review will be saved.")
)}
new_review_rows <- new_review_rows |>
dplyr::select(-dplyr::all_of(cols_to_change)) |>
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,
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", table)
))
DBI::dbClearResult(rs)
cat("finished writing to the tables:", tables, "\n")
cat("finished writing to the table:", table, "\n")
}

#' Append database table
Expand Down Expand Up @@ -473,12 +460,12 @@
#' 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
#' the database.
#' @param form Character vector with the form 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
#'
Expand All @@ -488,38 +475,48 @@
#' temp_path <- withr::local_tempfile(fileext = ".sqlite")
#' con <- get_db_connection(temp_path)
#' review_data <- data.frame(
#' subject_id = "Test_name",
#' 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, subject = "Test_name", form = "Test_group")
#' db_get_review(temp_path, id = 1L)
#' db_get_review(temp_path, subject_id = "Test_name2")
#' })
#'
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")
...,
db_table = "all_review_data"
){
stopifnot(file.exists(db_path))
stopifnot(is.character(subject))
stopifnot(is.character(form))
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) |>
fields <- ...names()
if (is.null(fields)) {
if (...length() > 0)
warning("Unnamed arguments passed in `...`. Returning full data table.")

Check warning on line 503 in R/fct_SQLite.R

View check run for this annotation

Codecov / codecov/patch

R/fct_SQLite.R#L502-L503

Added lines #L502 - L503 were not covered by tests
else
warning("No arguments passed in `...`. Returning full data table.")
conditionals <- "true"

Check warning on line 506 in R/fct_SQLite.R

View check run for this annotation

Codecov / codecov/patch

R/fct_SQLite.R#L505-L506

Added lines #L505 - L506 were not covered by tests
} else {
LDSamson marked this conversation as resolved.
Show resolved Hide resolved
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)
if (!is.null(fields))
DBI::dbBind(rs, params = data.frame(...))
LDSamson marked this conversation as resolved.
Show resolved Hide resolved
df <-
DBI::dbFetch(rs) |>
dplyr::as_tibble()
DBI::dbClearResult(rs)
df
})
}

Expand Down
67 changes: 27 additions & 40 deletions R/mod_review_forms.R
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,9 @@ 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)))
item_group == active_form())
})

observeEvent(c(active_form(), r$subject_id), {
Expand All @@ -136,8 +131,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()
LDSamson marked this conversation as resolved.
Show resolved Hide resolved
if(length(review_status) != 1) warning("multiple variables in review_status, namely: ",
review_status, "Verify data.")
}
Expand Down Expand Up @@ -191,8 +186,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
))
})

Expand Down Expand Up @@ -231,8 +226,7 @@ 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() |>
LDSamson marked this conversation as resolved.
Show resolved Hide resolved
dplyr::mutate(
reviewed = if(input$form_reviewed) "Yes" else "No",
comment = ifelse(is.null(input$review_comment), "", input$review_comment),
Expand All @@ -241,46 +235,39 @@ mod_review_forms_server <- function(
status = if(input$form_reviewed) "old" else "new"
)

golem::cat_dev("review row to add:\n")
golem::print_dev(review_row)
golem::cat_dev("review records to add:\n")
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"
table = "all_review_data"
)

# Contains multiple rows, one for each item.
updated_rows_db <- db_get_review(
db_path, subject = review_row$subject_id, form = review_row$item_group
)[c(names(review_row), "event_name", "item_name", "form_repeat")]
# Within a form, only items with a changed review state are updated and
# contain the new (current) time stamp.
updated_rows_db <- updated_rows_db[
updated_rows_db$timestamp == review_row$timestamp[1],
]
db_path, id = review_records$id
) |>
dplyr::select(dplyr::all_of(names(review_records)))

review_row_db <- unique(updated_rows_db[names(review_row)])
if(identical(review_row_db, review_row)){
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))){
LDSamson marked this conversation as resolved.
Show resolved Hide resolved
cat("Update review data and status in app\n")
r$review_data <- r$review_data |>
dplyr::rows_update(
updated_rows_db,
by = c("subject_id", "item_group", "event_name", "item_name", "form_repeat")
)
dplyr::rows_update(review_records, by = "id")
}

updated_items_memory <- sort(with(r$review_data, item_name[
reviewer == review_row$reviewer[1] & timestamp == review_row$timestamp[1]
]))
updated_items_db <- sort(updated_rows_db$item_name)
updated_items_memory <- review_records |>
dplyr::left_join(r$review_data, by = "id", suffix = c("", ".y")) |>
dplyr::select(dplyr::all_of(names(review_records))) |>
LDSamson marked this conversation as resolved.
Show resolved Hide resolved
dplyr::filter(timestamp == review_records$timestamp[1])

review_save_error(any(
!identical(review_row_db, review_row),
!identical(updated_items_db, updated_items_memory)
!isTRUE(all.equal(review_records_db, review_records, check.attributes = FALSE)),
!isTRUE(all.equal(updated_items_memory, review_records_db, check.attributes = FALSE))
))

if(review_save_error()){
Expand Down Expand Up @@ -320,7 +307,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"))
Expand Down
2 changes: 1 addition & 1 deletion inst/golem-config.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
default:
golem_name: clinsight
golem_version: 0.1.0.9010
golem_version: 0.1.1.9011
app_prod: no
user_identification: test_user
study_data: !expr clinsight::clinsightful_data
Expand Down
42 changes: 14 additions & 28 deletions man/db_get_review.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading