diff --git a/.lintr b/.lintr index 34473d2738..0a0bb22f32 100644 --- a/.lintr +++ b/.lintr @@ -1,5 +1,6 @@ linters: linters_with_defaults( line_length_linter = line_length_linter(120), cyclocomp_linter = NULL, - object_usage_linter = NULL + object_usage_linter = NULL, + indentation_linter = NULL ) diff --git a/NEWS.md b/NEWS.md index 986272c1a5..4a4ce8451b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,8 @@ * Enhanced a `module` validation checks so that it won't throw messages about `data` argument unnecessarily. * Removed `Report previewer` module from mapping matrix display in filter manager. * Added internal functions for storing and restoring of `teal_slices` objects. +* Filter state snapshots can now be uploaded from file. See `?snapshot`. +* Added argument to `teal_slices` and made modifications to `init` to enable tagging `teal_slices` with an app id to safely upload snapshots from disk. # teal 0.14.0 diff --git a/R/init.R b/R/init.R index 4fdb147951..449db91f10 100644 --- a/R/init.R +++ b/R/init.R @@ -164,6 +164,17 @@ init <- function(data, # convert teal.slice::teal_slices to teal::teal_slices filter <- as.teal_slices(as.list(filter)) + # Calculate app hash to ensure snapshot compatibility. See ?snapshot. Raw data must be extracted from environments. + hashables <- mget(c("data", "modules")) + hashables$data <- sapply(hashables$data$get_datanames(), function(dn) { + if (hashables$data$is_pulled()) { + hashables$data$get_dataset(dn)$get_raw_data() + } else { + hashables$data$get_code(dn) + } + }, simplify = FALSE) + attr(filter, "app_id") <- rlang::hash(hashables) + # check teal_slices for (i in seq_along(filter)) { dataname_i <- shiny::isolate(filter[[i]]$dataname) diff --git a/R/module_filter_manager.R b/R/module_filter_manager.R index 8339efbf1c..69a31fdc12 100644 --- a/R/module_filter_manager.R +++ b/R/module_filter_manager.R @@ -137,7 +137,7 @@ filter_manager_srv <- function(id, filtered_data_list, filter) { } # Create mapping fo filters to modules in matrix form (presented as data.frame). - # Modules get NAs for filteres that cannot be set for them. + # Modules get NAs for filters that cannot be set for them. mapping_matrix <- reactive({ state_ids_global <- vapply(slices_global(), `[[`, character(1L), "id") mapping_smooth <- lapply(filtered_data_list, function(x) { diff --git a/R/module_snapshot_manager.R b/R/module_snapshot_manager.R index d2e05db923..a337e5f5ce 100644 --- a/R/module_snapshot_manager.R +++ b/R/module_snapshot_manager.R @@ -4,11 +4,13 @@ #' #' This module introduces snapshots: stored descriptions of the filter state of the entire application. #' Snapshots allow the user to save the current filter state of the application for later use in the session, -#' as well as to save it to file in order to share it with an app developer or other users. +#' as well as to save it to file in order to share it with an app developer or other users, +#' who in turn can upload it to their own session. #' #' The snapshot manager is accessed through the filter manager, with the cog icon in the top right corner. -#' At the beginning of a session it presents two icons: a camera and an circular arrow. -#' Clicking the camera captures a snapshot and clicking the arrow resets initial application state. +#' At the beginning of a session it presents three icons: a camera, an upload, and an circular arrow. +#' Clicking the camera captures a snapshot, clicking the upload adds a snapshot from a file +#' and applies the filter states therein, and clicking the arrow resets initial application state. #' As snapshots are added, they will show up as rows in a table and each will have a select button and a save button. #' #' @section Server logic: @@ -49,6 +51,19 @@ #' To save a snapshot, the snapshot is retrieved and reassembled just like for restoring, #' and then saved to file with [`slices_store`]. #' +#' When a snapshot is uploaded, it will first be added to storage just like a newly created one, +#' and then used to restore app state much like a snapshot taken from storage. +#' Upon clicking the upload icon the user will be prompted for a file to upload +#' and may choose to name the new snapshot. The name defaults to the name of the file (the extension is dropped) +#' and normal naming rules apply. Loading the file yields a `teal_slices` object, +#' which is disassembled for storage and used directly for restoring app state. +#' +#' @section Transferring snapshots: +#' Snapshots uploaded from disk should only be used in the same application they come from. +#' To ensure this is the case, `init` stamps `teal_slices` with an app id that is stored in the `app_id` attribute of +#' a `teal_slices` object. When a snapshot is restored from file, its `app_id` is compared to that +#' of the current app state and only if the match is the snapshot admitted to the session. +#' #' @param id (`character(1)`) `shiny` module id #' @param slices_global (`reactiveVal`) that contains a `teal_slices` object #' containing all `teal_slice`s existing in the app, both active and inactive @@ -75,6 +90,7 @@ snapshot_manager_ui <- function(id) { class = "snapshot_table_row", span(tags$b("Snapshot manager")), actionLink(ns("snapshot_add"), label = NULL, icon = icon("camera"), title = "add snapshot"), + actionLink(ns("snapshot_load"), label = NULL, icon = icon("upload"), title = "upload snapshot"), actionLink(ns("snapshot_reset"), label = NULL, icon = icon("undo"), title = "reset initial state"), NULL ), @@ -96,7 +112,7 @@ snapshot_manager_srv <- function(id, slices_global, mapping_matrix, filtered_dat moduleServer(id, function(input, output, session) { ns <- session$ns - # Store global filter states. + # Store global filter states ---- filter <- isolate(slices_global()) snapshot_history <- reactiveVal({ list( @@ -104,7 +120,8 @@ snapshot_manager_srv <- function(id, slices_global, mapping_matrix, filtered_dat ) }) - # Snapshot current application state - name snaphsot. + # Snapshot current application state ---- + # Name snaphsot. observeEvent(input$snapshot_add, { showModal( modalDialog( @@ -117,7 +134,7 @@ snapshot_manager_srv <- function(id, slices_global, mapping_matrix, filtered_dat ) ) }) - # Snapshot current application state - store snaphsot. + # Store snaphsot. observeEvent(input$snapshot_name_accept, { snapshot_name <- trimws(input$snapshot_name) if (identical(snapshot_name, "")) { @@ -131,7 +148,7 @@ snapshot_manager_srv <- function(id, slices_global, mapping_matrix, filtered_dat "This name is in conflict with other snapshot names. Please choose a different one.", type = "message" ) - updateTextInput(inputId = "snapshot_name", value = , placeholder = "Meaningful, unique name") + updateTextInput(inputId = "snapshot_name", value = "", placeholder = "Meaningful, unique name") } else { snapshot <- as.list(slices_global(), recursive = TRUE) attr(snapshot, "mapping") <- matrix_to_mapping(mapping_matrix()) @@ -144,7 +161,76 @@ snapshot_manager_srv <- function(id, slices_global, mapping_matrix, filtered_dat } }) - # Restore initial state. + # Upload a snapshot file ---- + # Select file. + observeEvent(input$snapshot_load, { + showModal( + modalDialog( + fileInput(ns("snapshot_file"), "Choose snapshot file", accept = ".json", width = "100%"), + textInput( + ns("snapshot_name"), + "Name the snapshot (optional)", + width = "100%", + placeholder = "Meaningful, unique name" + ), + footer = tagList( + actionButton(ns("snaphot_file_accept"), "Accept", icon = icon("thumbs-up")), + modalButton(label = "Cancel", icon = icon("thumbs-down")) + ) + ) + ) + }) + # Store new snapshot to list and restore filter states. + observeEvent(input$snaphot_file_accept, { + snapshot_name <- trimws(input$snapshot_name) + if (identical(snapshot_name, "")) { + snapshot_name <- tools::file_path_sans_ext(input$snapshot_file$name) + } + if (is.element(make.names(snapshot_name), make.names(names(snapshot_history())))) { + showNotification( + "This name is in conflict with other snapshot names. Please choose a different one.", + type = "message" + ) + updateTextInput(inputId = "snapshot_name", value = "", placeholder = "Meaningful, unique name") + } else { + # Restore snapshot and verify app compatibility. + snapshot_state <- try(slices_restore(input$snapshot_file$datapath)) + if (!inherits(snapshot_state, "modules_teal_slices")) { + showNotification( + "File appears to be corrupt.", + type = "error" + ) + } else if (!identical(attr(snapshot_state, "app_id"), attr(slices_global(), "app_id"))) { + showNotification( + "This snapshot file is not compatible with the app and cannot be loaded.", + type = "warning" + ) + } else { + # Add to snapshot history. + snapshot <- as.list(snapshot_state, recursive = TRUE) + snapshot_update <- c(snapshot_history(), list(snapshot)) + names(snapshot_update)[length(snapshot_update)] <- snapshot_name + snapshot_history(snapshot_update) + ### Begin simplified restore procedure. ### + mapping_unfolded <- unfold_mapping(attr(snapshot_state, "mapping"), names(filtered_data_list)) + mapply( + function(filtered_data, filter_ids) { + filtered_data$clear_filter_states(force = TRUE) + slices <- Filter(function(x) x$id %in% filter_ids, snapshot_state) + filtered_data$set_filter_state(slices) + }, + filtered_data = filtered_data_list, + filter_ids = mapping_unfolded + ) + slices_global(snapshot_state) + removeModal() + ### End simplified restore procedure. ### + } + } + }) + # Apply newly added snapshot. + + # Restore initial state ---- observeEvent(input$snapshot_reset, { s <- "Initial application state" ### Begin restore procedure. ### @@ -165,6 +251,7 @@ snapshot_manager_srv <- function(id, slices_global, mapping_matrix, filtered_dat ### End restore procedure. ### }) + # Build snapshot table ---- # Create UI elements and server logic for the snapshot table. # Observers must be tracked to avoid duplication and excess reactivity. # Remaining elements are tracked likewise for consistency and a slight speed margin. diff --git a/R/teal_slices.R b/R/teal_slices.R index 2562b0976f..ee26c82e4a 100644 --- a/R/teal_slices.R +++ b/R/teal_slices.R @@ -18,6 +18,10 @@ #' If missing, all filters will be applied to all modules. #' If empty list, all filters will be available to all modules but will start inactive. #' If `module_specific` is `FALSE`, only `global_filters` will be active on start. +#' @param app_id (`character(1)`)\cr +#' For internal use only, do not set manually. +#' Added by `init` so that a `teal_slices` can be matched to the app in which it was used. +#' Used for verifying snapshots uploaded from file. See `snapshot`. #' #' @param x (`list`) of lists to convert to `teal_slices` #' @@ -56,11 +60,13 @@ teal_slices <- function(..., count_type = NULL, allow_add = TRUE, module_specific = FALSE, - mapping) { + mapping, + app_id = NULL) { shiny::isolate({ checkmate::assert_flag(allow_add) checkmate::assert_flag(module_specific) if (!missing(mapping)) checkmate::assert_list(mapping, types = c("character", "NULL"), names = "named") + checkmate::assert_string(app_id, null.ok = TRUE) slices <- list(...) all_slice_id <- vapply(slices, `[[`, character(1L), "id") @@ -90,6 +96,7 @@ teal_slices <- function(..., ) attr(tss, "mapping") <- mapping attr(tss, "module_specific") <- module_specific + attr(tss, "app_id") <- app_id class(tss) <- c("modules_teal_slices", class(tss)) tss }) diff --git a/man/snapshot_manager_module.Rd b/man/snapshot_manager_module.Rd index ec883c2c07..af51d469be 100644 --- a/man/snapshot_manager_module.Rd +++ b/man/snapshot_manager_module.Rd @@ -33,11 +33,13 @@ Capture and restore snapshots of the global (app) filter state. \details{ This module introduces snapshots: stored descriptions of the filter state of the entire application. Snapshots allow the user to save the current filter state of the application for later use in the session, -as well as to save it to file in order to share it with an app developer or other users. +as well as to save it to file in order to share it with an app developer or other users, +who in turn can upload it to their own session. The snapshot manager is accessed through the filter manager, with the cog icon in the top right corner. -At the beginning of a session it presents two icons: a camera and an circular arrow. -Clicking the camera captures a snapshot and clicking the arrow resets initial application state. +At the beginning of a session it presents three icons: a camera, an upload, and an circular arrow. +Clicking the camera captures a snapshot, clicking the upload adds a snapshot from a file +and applies the filter states therein, and clicking the arrow resets initial application state. As snapshots are added, they will show up as rows in a table and each will have a select button and a save button. } \section{Server logic}{ @@ -80,6 +82,21 @@ The snapshot is then set as the current content of \code{slices_global}. To save a snapshot, the snapshot is retrieved and reassembled just like for restoring, and then saved to file with \code{\link{slices_store}}. + +When a snapshot is uploaded, it will first be added to storage just like a newly created one, +and then used to restore app state much like a snapshot taken from storage. +Upon clicking the upload icon the user will be prompted for a file to upload +and may choose to name the new snapshot. The name defaults to the name of the file (the extension is dropped) +and normal naming rules apply. Loading the file yields a \code{teal_slices} object, +which is disassembled for storage and used directly for restoring app state. +} + +\section{Transferring snapshots}{ + +Snapshots uploaded from disk should only be used in the same application they come from. +To ensure this is the case, \code{init} stamps \code{teal_slices} with an app id that is stored in the \code{app_id} attribute of +a \code{teal_slices} object. When a snapshot is restored from file, its \code{app_id} is compared to that +of the current app state and only if the match is the snapshot admitted to the session. } \author{ diff --git a/man/teal_slices.Rd b/man/teal_slices.Rd index 6d5117b058..cfa730ab79 100644 --- a/man/teal_slices.Rd +++ b/man/teal_slices.Rd @@ -13,7 +13,8 @@ teal_slices( count_type = NULL, allow_add = TRUE, module_specific = FALSE, - mapping + mapping, + app_id = NULL ) as.teal_slices(x) @@ -58,6 +59,11 @@ If missing, all filters will be applied to all modules. If empty list, all filters will be available to all modules but will start inactive. If \code{module_specific} is \code{FALSE}, only \code{global_filters} will be active on start.} +\item{app_id}{(\code{character(1)})\cr +For internal use only, do not set manually. +Added by \code{init} so that a \code{teal_slices} can be matched to the app in which it was used. +Used for verifying snapshots uploaded from file. See \code{snapshot}.} + \item{x}{(\code{list}) of lists to convert to \code{teal_slices}} } \description{ diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R index 834da959d0..e3c0fbbc33 100644 --- a/tests/testthat/test-init.R +++ b/tests/testthat/test-init.R @@ -9,8 +9,8 @@ mods <- teal:::example_modules() testthat::test_that("init data accepts TealData objects", { teal_data_object <- teal.data::teal_data(dataset_1) cdisc_data_object <- teal.data::cdisc_data(adsl_dataset) - testthat::expect_error(init(data = teal_data_object, modules = mods), NA) - testthat::expect_error(init(data = cdisc_data_object, modules = mods), NA) + testthat::expect_no_error(init(data = teal_data_object, modules = mods)) + testthat::expect_no_error(init(data = cdisc_data_object, modules = mods)) }) testthat::test_that("init data throws an error with input other than accepted input", { @@ -27,16 +27,15 @@ testthat::test_that("init data throws an error with input other than accepted in }) testthat::test_that("init data accepts a single TealDataset/CDISCTealDataset", { - testthat::expect_error(init(data = teal.data::dataset("iris", head(iris)), modules = mods), NA) - testthat::expect_error( + testthat::expect_no_error(init(data = teal.data::dataset("iris", head(iris)), modules = mods)) + testthat::expect_no_error( init( data = teal.data::cdisc_dataset("ADSL", adsl_df, parent = character(0), keys = teal.data::get_cdisc_keys("ADSL")), modules = mods - ), - NA + ) ) - testthat::expect_error(init(data = dataset_1, modules = mods), NA) - testthat::expect_error(init(data = adsl_dataset, modules = mods), NA) + testthat::expect_no_error(init(data = dataset_1, modules = mods)) + testthat::expect_no_error(init(data = adsl_dataset, modules = mods)) }) testthat::test_that("init data accepts a list of single TealDataset/CDISCTealDataset without renaming", { @@ -45,104 +44,112 @@ testthat::test_that("init data accepts a list of single TealDataset/CDISCTealDat teal.data::cdisc_dataset("ADSL", adsl_df, parent = character(0), keys = teal.data::get_cdisc_keys("ADSL")) ) - testthat::expect_error(init(data = list(teal.data::dataset("iris", head(iris))), modules = mods), NA) - testthat::expect_error(init( - data = list( - teal.data::cdisc_dataset("ADSL", adsl_df, parent = character(0), keys = teal.data::get_cdisc_keys("ADSL")) - ), - modules = mods - ), NA) - testthat::expect_error(init(data = dataset_list, modules = mods), NA) - testthat::expect_error(init(data = cdisc_dataset_list, modules = mods), NA) + testthat::expect_no_error(init(data = list(teal.data::dataset("iris", head(iris))), modules = mods)) + testthat::expect_no_error( + init( + data = list( + teal.data::cdisc_dataset("ADSL", adsl_df, parent = character(0), keys = teal.data::get_cdisc_keys("ADSL")) + ), + modules = mods + ) + ) + testthat::expect_no_error(init(data = dataset_list, modules = mods)) + testthat::expect_no_error(init(data = cdisc_dataset_list, modules = mods)) }) testthat::test_that("init data accepts a single dataframe", { - testthat::expect_error(init(data = adsl_df, modules = mods), NA) + testthat::expect_no_error(init(data = adsl_df, modules = mods)) }) testthat::test_that("init data accepts a list of single dataframe without renaming", { - testthat::expect_error(init(data = list(adsl_df), modules = mods), NA) + testthat::expect_no_error(init(data = list(adsl_df), modules = mods)) }) testthat::test_that("init data accepts a list of single dataframe with renaming", { adsl_list <- list(data1 = adsl_df) - testthat::expect_error(init(data = list(data1 = adsl_df), modules = mods), NA) - testthat::expect_error(init(data = adsl_list, modules = mods), NA) + testthat::expect_no_error(init(data = list(data1 = adsl_df), modules = mods)) + testthat::expect_no_error(init(data = adsl_list, modules = mods)) }) testthat::test_that("init data accepts a list of a TealDataset and a dataframe without renaming", { - testthat::expect_error(init(data = list(dataset_1, adsl_df), modules = mods), NA) + testthat::expect_no_error(init(data = list(dataset_1, adsl_df), modules = mods)) }) testthat::test_that("init data accepts a single MultiAssayExperiment object", { utils::data(miniACC, package = "MultiAssayExperiment") - testthat::expect_error(init(data = miniACC, modules = mods), NA) + testthat::expect_no_error(init(data = miniACC, modules = mods)) }) testthat::test_that("init data accepts a list of a single MultiAssayExperiment object without renaming", { utils::data(miniACC, package = "MultiAssayExperiment") - testthat::expect_error(init(data = list(miniACC), modules = mods), NA) + testthat::expect_no_error(init(data = list(miniACC), modules = mods)) }) testthat::test_that("init data accepts a list of a single MultiAssayExperiment object with renaming", { utils::data(miniACC, package = "MultiAssayExperiment") - testthat::expect_error(init(data = list(x = miniACC), modules = mods), NA) + testthat::expect_no_error(init(data = list(x = miniACC), modules = mods)) }) testthat::test_that("init data acceptsa mixed list of MultiAssayExperiment object and data.frame", { utils::data(miniACC, package = "MultiAssayExperiment") - testthat::expect_error(init(data = list(x = miniACC, y = head(iris)), modules = mods), NA) + testthat::expect_no_error(init(data = list(x = miniACC, y = head(iris)), modules = mods)) }) testthat::test_that("init data accepts a list of a TealDataset and a dataframe with renaming", { - testthat::expect_error(init( - data = list( - data1 = teal.data::dataset("iris", head(iris)), - data2 = as.data.frame(as.list(setNames(nm = teal.data::get_cdisc_keys("ADSL")))) - ), - modules = mods - ), NA) - testthat::expect_error(init(data = list(data1 = dataset_1, data2 = adsl_df), modules = mods), NA) + testthat::expect_no_error( + init( + data = list( + data1 = teal.data::dataset("iris", head(iris)), + data2 = as.data.frame(as.list(setNames(nm = teal.data::get_cdisc_keys("ADSL")))) + ), + modules = mods + ) + ) + testthat::expect_no_error(init(data = list(data1 = dataset_1, data2 = adsl_df), modules = mods)) }) testthat::test_that("init data accepts a list of mixed TealDataset and dataframe with mixed renaming", { - testthat::expect_error(init(data = list(data1 = teal.data::dataset("iris", head(iris)), adsl_df), modules = mods), NA) - testthat::expect_error(init(data = list(dataset_1, data2 = adsl_df), modules = mods), NA) + testthat::expect_no_error(init(data = list(data1 = teal.data::dataset("iris", head(iris)), adsl_df), modules = mods)) + testthat::expect_no_error(init(data = list(dataset_1, data2 = adsl_df), modules = mods)) }) testthat::test_that("init data accepts TealDatasetConnector object", { dsc1 <- teal.data::dataset_connector("iris", teal.data::callable_function(function() head(iris))) - testthat::expect_error(init(data = dsc1, modules = mods), NA) - testthat::expect_error(init( - data = teal.data::dataset_connector("iris", teal.data::callable_function(function() head(iris))), - modules = mods - ), NA) + testthat::expect_no_error(init(data = dsc1, modules = mods)) + testthat::expect_no_error( + init( + data = teal.data::dataset_connector("iris", teal.data::callable_function(function() head(iris))), + modules = mods + ) + ) }) testthat::test_that("init data accepts a list of TealDatasetConnector object", { dsc1 <- list(teal.data::dataset_connector("iris", teal.data::callable_function(function() head(iris)))) - testthat::expect_error(init(data = dsc1, modules = mods), NA) - testthat::expect_error( - init(data = list( - teal.data::dataset_connector("iris", teal.data::callable_function(function() head(iris))) - ), modules = mods), - NA + testthat::expect_no_error(init(data = dsc1, modules = mods)) + testthat::expect_no_error( + init( + data = list( + teal.data::dataset_connector("iris", teal.data::callable_function(function() head(iris))) + ), + modules = mods + ) ) }) testthat::test_that("init modules accepts a teal_modules object", { mods <- modules(example_module(), example_module()) - testthat::expect_error(init(data = iris, modules = mods), NA) + testthat::expect_no_error(init(data = iris, modules = mods)) }) testthat::test_that("init modules accepts a list of teal_module elements", { mods <- list(example_module(), example_module()) - testthat::expect_error(init(data = iris, modules = mods), NA) + testthat::expect_no_error(init(data = iris, modules = mods)) }) testthat::test_that("init modules accepts a teal_module object", { mods <- example_module() - testthat::expect_error(init(data = iris, modules = mods), NA) + testthat::expect_no_error(init(data = iris, modules = mods)) }) testthat::test_that("init filter accepts named list or `teal_slices`", {