From cb95d7e3a8133bff3dd4f3141a76974e6459d346 Mon Sep 17 00:00:00 2001 From: go_gonzo Date: Wed, 1 Nov 2023 06:55:13 +0100 Subject: [PATCH 1/9] tm_teal_data module --- R/data-data-utils.R | 75 ++++++++++++++++ R/data-module.R | 175 ++++++++++++++++++++++++++++++++++++ R/init.R | 41 +++------ R/module_teal_with_splash.R | 22 ++++- R/utils.R | 18 ++++ R/zzz.R | 3 + 6 files changed, 301 insertions(+), 33 deletions(-) create mode 100644 R/data-data-utils.R create mode 100644 R/data-module.R diff --git a/R/data-data-utils.R b/R/data-data-utils.R new file mode 100644 index 0000000000..a46d140fcd --- /dev/null +++ b/R/data-data-utils.R @@ -0,0 +1,75 @@ +#' Function runs the `code`, masks the `code` and creates `teal_data` object. +#' @param input (`list`) containing inputs to be used in the `code` +#' @inheritParams tm_teal_data +#' +#' @return `teal_data` object +#' +#' @export +eval_and_mask <- function(data, + code, + input = list(), + input_mask = list()) { + checkmate::assert_list(input) + if (inherits(input, "reactivevalues")) { + input <- shiny::reactiveValuesToList(input) + } + + # evaluate code and substitute input + data <- teal.code::eval_code(data, .mask_code(code, args = input)) + + if (identical(ls(data@env), character(0))) { + warning( + "Evaluation of `ddl` code haven't created any objects.\n", + "Please make sure that the code is syntactically correct and creates necessary data." + ) + } + + if (!missing(input_mask)) { + # mask dynamic inputs with mask + input <- utils::modifyList(input, input_mask) + + # replace last code entry with masked code + # format_expression needed to convert expression into character(1) + # question: warnings and errors are not masked, is it ok? + data@code[length(code)] <- format_expression(.mask_code(code, args = input)) + } + + # todo: should it be here or in datanames(data)? + if (length(datanames(data)) == 0) { + datanames(data) <- ls(data@env) + } + + data +} + +#' substitute inputs in the code +#' +#' Function replaces symbols in the provided code prefixed with `input$` or `input[["` +#' by values of the `args` argument. +#' +#' @param code (`language`) code to substitute +#' @param args (`list`) named list or arguments +.mask_code <- function(code, args) { + code <- if (identical(as.list(code)[[1L]], as.symbol("{"))) { + as.list(code)[-1L] + } else { + code + } + + code_strings <- vapply(code, deparse1, character(1L)) + # Replace input$ with .() + code_strings <- gsub("input\\$(\\w+\\.?\\w*)", "\\.(\\1)", code_strings) + code_strings <- gsub("(input\\$)(`[^`]+`)", "\\.(\\2)", code_strings) + + # Replace input[[ with .() + code_strings <- gsub("(input\\[\\[\")(\\w+\\.?\\w*)(\"\\]\\])", "\\.(\\2\\)", code_strings) + code_strings <- gsub("(input\\[\\[\")(\\w+\\-\\w+)\"\\]\\]", ".(`\\2`)", code_strings) + + # Use bquote to obtain code with input values and masking values. + # todo: make sure it produces only one entry in qenv@code!!! + as.expression( + lapply(code_strings, function(x) { + do.call(bquote, list(str2lang(x), list2env(args))) + }) + ) +} diff --git a/R/data-module.R b/R/data-module.R new file mode 100644 index 0000000000..9df0361f83 --- /dev/null +++ b/R/data-module.R @@ -0,0 +1,175 @@ +#' DDL object +#' +#' Object to execute custom DDL code in the shiny session. +#' +#' @section Creating reproducible data: +#' `ddl` object can be used to create reproducible data in the shiny session. `ddl$server` function +#' can execute any R code and return [`teal.data::teal_data-class`]. For reproducibility purposes, +#' we recommend to initialize empty `teal_data` object and evaluate necessary code with `eval_code` or `within`. +#' ```r +#' function(id, ...) { +#' moduleServer(id, function(input, output, session) { +#' eventReactive(input$submit, { +#' data <- teal_data() |> within({ +#' # code to be run when app user presses submit +#' }) +#' }) +#' }) +#' } +#' ``` +#' Obtained data is passed further in the `teal` app with `code` which can be used to recreate the objects. +#' +#' @section Code masking: +#' `ddl` object can be used in a way that evaluated code is different than the code +#' returned in `teal_data` object. Typically occurs when app user is asked to input a +#' password and we'd like to skip this input in the reproducible code. Possibly, users password +#' could be substituted with `askpass::askpass()` call, so the returned code is still executable but secure. +#' `ddl` developer must understand that this is a security risk and should be handled with care. +#' To make sure that the code is reproducible, `ddl` object should be used with `input_mask` argument. +#' `teal` provides convenience function [ddl_run()] which handles evaluation of the code, masking +#' and creating `teal_data` object. Such `server` function could look like this: +#' +#' ``` +#' server = function(id, ...) { +#' moduleServer(id, function(input, output, session) { +#' reactive({ +#' ddl_run(input = input, ...) +#' }) +#' }) +#' } +#' ``` +#' +#' If `ddl` developer values more control, then might be interested in using `...` explicitly, +#' and create `teal_data` object manually. +#' +#' @param ui (`shiny.tag`)\cr +#' `shiny` user-interface module containing inputs whose `id` correspond to the arguments in the `code`. +#' +#' @param server (`function`)\cr +#' `shiny` server module [`teal.data::teal_data-class`] possibly wrapped in a [reactive()]. +#' `server` function should have `id` and `...` as formals. Where: +#' - `id` is a `shiny` module id, and +#' - `...` passes arguments from the `ddl` object (`code`, `input_mask`, `datanames`, `join_keys`). +#' See section `Code masking`. +#' +#' @param expr (optional `expression`)\cr +#' Syntactically valid R expression to be executed in the shiny session. +#' Shouldn't be specified when `code` is specified. +#' +#' @param code (optional `character` or `language`)\cr +#' Object containing (defused) syntactically valid R expression to be executed in the shiny session. +#' Shouldn't be specified when `expr` is specified. +#' +#' @param input_mask (optional `named list`)\cr +#' arguments to be substituted in the `code`. These (named) list elements are going to replace +#' symbols in the code prefixed with `input$` or `input[["`. Typically `input_mask` is used +#' to mask username or password with `list(password = quote(askpass::askpass()))`. +#' See section `code masking` for more details. +#' +#' @param datanames (optional `character`)\cr +#' Names of the datasets created by evaluation of the `code`. By default, `datanames` +#' are obtained from the `join_keys` or from results of the `code` evaluation. +#' If `code` evaluation creates objects which are not considered as datasets, they +#' should be omitted from `datanames` to avoid errors. +#' +#' @inheritParams teal.data::teal_data +#' +#' @export +tm_teal_data <- function(label = "data", + expr, + code, + input_mask = list(), + ui = submit_button_ui, # or empty div? + server = submit_button_server) { + checkmate::assert_list(input_mask) + checkmate::check_function(ui, args = "id") + checkmate::check_function(server, args = c("id", "...")) # todo: data should be passed to this module + + out <- module( + label = label, + ui = ui, + server = server, + server_args = list(input_mask = input_mask) + # datanames = "all" by default + ) + class(out) <- c(class(out), "teal_module_data") + + if (!missing(expr) || !missing(code)) { + # this is intended to be used with input mask + # but in the same time we can't forbid user to use it + # without input_mask. Some users might prefer to use ddl_run + # to automaticaly handle their code. + # Q: can NEST bear responsibility for reproducibility of the masked code? + if (!missing(expr)) { + code <- substitute(expr) + } + if (is.character(code)) { + code <- parse(text = code) + } + out$server_args$code <- code + } + + out +} + + +#' Run code and mask inputs +#' +#' Delayed Data Loading module with login and password input. +#' +#' @name submit_button_module +#' +#' +#' @param id (`character`) `shiny` module id. +#' @param ... (`list`) arguments passed to `ddl_run` function. +#' @return `shiny` module +NULL + +#' @rdname submit_button_module +#' @export +submit_button_ui <- function(id) { + ns <- NS(id) + actionButton(inputId = ns("submit"), label = "Submit") +} + +#' @rdname submit_button_module +#' @export +submit_button_server <- function(id, ...) { + moduleServer(id, function(input, output, session) { + tdata <- eventReactive(input$submit, { + ddl_run(input = input, ...) + }) + + # would need to make sure we handle reactivity correctly here as teal::init expects not reactive teal_data... + return(tdata) + }) +} + + +# methods from teal.data ---- +# to be removed soon + +#' Get data names from `ddl` +#' @rdname get_dataname +#' @param x (`ddl`) object +#' @export +get_dataname.ddl <- function(x) { + attr(x, "datanames") +} + +#' @rdname get_join_keys +#' @export +get_join_keys.ddl <- function(data) { + attr(data, "join_keys") +} + +# todo: to remove before merge ------------- +#' @export +open_conn <- function(username, password) { + if (password != "pass") stop("Invalid credentials. 'pass' is the password") else TRUE +} +#' @export +close_conn <- function(conn) { + message("closed") + return(NULL) +} diff --git a/R/init.R b/R/init.R index b697e2829b..20014eff3e 100644 --- a/R/init.R +++ b/R/init.R @@ -118,6 +118,7 @@ init <- function(data, if (!inherits(data, c("TealData", "teal_data"))) { data <- teal.data::to_relational_data(data = data) } + checkmate::assert_multi_class(data, c("TealData", "teal_data")) checkmate::assert_multi_class(modules, c("teal_module", "list", "teal_modules")) checkmate::assert_string(title, null.ok = TRUE) @@ -142,26 +143,10 @@ init <- function(data, if (length(landing) > 1L) stop("Only one `landing_popup_module` can be used.") modules <- drop_module(modules, "teal_module_landing") - # resolve modules datanames - datanames <- teal.data::get_dataname(data) - join_keys <- teal.data::get_join_keys(data) - modules <- resolve_modules_datanames(modules = modules, datanames = datanames, join_keys = join_keys) - - if (!inherits(filter, "teal_slices")) { - checkmate::assert_subset(names(filter), choices = datanames) - # list_to_teal_slices is lifted from teal.slice package, see zzz.R - # This is a temporary measure and will be removed two release cycles from now (now meaning 0.13.0). - filter <- list_to_teal_slices(filter) - } - # 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 <- if (inherits(hashables$data, "teal_data")) { as.list(hashables$data@env) - } else if (inherits(hashables$data, "ddl")) { - attr(hashables$data, "code") } else if (hashables$data$is_pulled()) { sapply(get_dataname(hashables$data), simplify = FALSE, function(dn) { hashables$data$get_dataset(dn)$get_raw_data() @@ -172,20 +157,14 @@ init <- function(data, attr(filter, "app_id") <- rlang::hash(hashables) - # check teal_slices - for (i in seq_along(filter)) { - dataname_i <- shiny::isolate(filter[[i]]$dataname) - if (!dataname_i %in% datanames) { - stop( - sprintf( - "filter[[%s]] has a different dataname than available in a 'data':\n %s not in %s", - i, - dataname_i, - toString(datanames) - ) - ) - } - } + # convert teal.slice::teal_slices to teal::teal_slices + filter <- as.teal_slices(as.list(filter)) + + # resolve modules datanames + datanames <- teal.data::get_dataname(data) + join_keys <- teal.data::get_join_keys(data) + modules <- resolve_modules_datanames(modules = modules, datanames = datanames, join_keys = join_keys) + assert_filter_datanames(filter, datanames) if (isTRUE(attr(filter, "module_specific"))) { module_names <- unlist(c(module_labels(modules), "global_filters")) @@ -218,7 +197,7 @@ init <- function(data, # the `ui` and `server` with `id = character(0)` and calling the server function directly # rather than through `callModule` res <- list( - ui = ui_teal_with_splash(id = id, data = data, title = title, header = header, footer = footer), + ui = ui_teal_with_splash(id = id, data = data, modules = modules, title = title, header = header, footer = footer), server = function(input, output, session) { if (length(landing) == 1L) { landing_module <- landing[[1L]] diff --git a/R/module_teal_with_splash.R b/R/module_teal_with_splash.R index 01348381b2..76d9e6d6b2 100644 --- a/R/module_teal_with_splash.R +++ b/R/module_teal_with_splash.R @@ -19,17 +19,23 @@ #' @export ui_teal_with_splash <- function(id, data, + modules, title, header = tags$p("Add Title Here"), footer = tags$p("Add Footer Here")) { checkmate::assert_multi_class(data, c("TealDataAbstract", "teal_data")) ns <- NS(id) + data_module <- extract_module(modules, "teal_module_data")[[1]] + # Startup splash screen for delayed loading # We use delayed loading in all cases, even when the data does not need to be fetched. # This has the benefit that when filtering the data takes a lot of time initially, the # Shiny app does not time out. - splash_ui <- if (inherits(data, "teal_data")) { + + splash_ui <- if (length(data_module)) { + data_module$ui(ns("data")) + } else if (inherits(data, "teal_data")) { div() } else if (inherits(data, "TealDataAbstract") && teal.data::is_pulled(data)) { div() @@ -64,9 +70,21 @@ srv_teal_with_splash <- function(id, data, modules, filter = teal_slices()) { shinyjs::showLog() } + data_module <- extract_module(modules, "teal_module_data") + if (length(data_module) > 1L) stop("Only one `teal_module_data` can be used.") + modules <- drop_module(modules, "teal_module_data") + # raw_data contains teal_data object # either passed to teal::init or returned from ddl - raw_data <- if (inherits(data, "teal_data")) { + raw_data <- if (length(data_module)) { + do.call( + data_module[[1]]$server, + c( + list(id = "data", data = data), + data_module[[1]]$server_args + ) + ) + } else if (inherits(data, "teal_data")) { reactiveVal(data) } else if (inherits(data, "TealDataAbstract") && teal.data::is_pulled(data)) { new_data <- do.call( diff --git a/R/utils.R b/R/utils.R index a4a31f2a5d..7f67ca5a2f 100644 --- a/R/utils.R +++ b/R/utils.R @@ -140,3 +140,21 @@ resolve_modules_datanames <- function(modules, datanames, join_keys) { modules } } + + +assert_filter_datanames <- function(filter, datanames) { + # check teal_slices against datanames + for (i in seq_along(filter)) { + dataname_i <- shiny::isolate(filter[[i]]$dataname) + if (!dataname_i %in% datanames) { + stop( + sprintf( + "filter[[%s]] has a different dataname than available in a 'data':\n %s not in %s", + i, + dataname_i, + toString(datanames) + ) + ) + } + } +} diff --git a/R/zzz.R b/R/zzz.R index fbc9c756d9..ce6fdb281c 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -32,3 +32,6 @@ setdiff_teal_slices <- getFromNamespace("setdiff_teal_slices", "teal.slice") coalesce_r <- getFromNamespace("coalesce_r", "teal.slice") # all *Block objects are private in teal.reporter RcodeBlock <- getFromNamespace("RcodeBlock", "teal.reporter") # nolint + + +format_expression <- getFromNamespace("format_expression", "teal.code") From c577d21c64ef477e6e027766275b53c52dbdc6b4 Mon Sep 17 00:00:00 2001 From: go_gonzo Date: Wed, 1 Nov 2023 07:04:02 +0100 Subject: [PATCH 2/9] merging two reactives into one --- R/module_teal.R | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/R/module_teal.R b/R/module_teal.R index 413e47d349..e53765be39 100644 --- a/R/module_teal.R +++ b/R/module_teal.R @@ -161,8 +161,21 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) { ) env <- environment() - datasets_reactive <- eventReactive(raw_data(), { + + reporter <- teal.reporter::Reporter$new() + if (is_arg_used(modules, "reporter") && length(extract_module(modules, "teal_module_previewer")) == 0) { + modules <- append_module(modules, reporter_previewer_module()) + } + + # Replace splash / welcome screen once data is loaded ---- + # ignoreNULL to not trigger at the beginning when data is NULL + # just handle it once because data obtained through delayed loading should + # usually not change afterwards + # if restored from bookmarked state, `filter` is ignored + observeEvent(raw_data(), { + logger::log_trace("srv_teal@5 setting main ui after data was pulled") env$progress <- shiny::Progress$new(session) + on.exit(env$progress$close()) env$progress$set(0.25, message = "Setting data") # create a list of data following structure of the nested modules list structure. @@ -201,26 +214,8 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) { } datasets <- module_datasets(modules) - logger::log_trace("srv_teal@4 Raw Data transferred to FilteredData.") - datasets - }) - - reporter <- teal.reporter::Reporter$new() - if (is_arg_used(modules, "reporter") && length(extract_module(modules, "teal_module_previewer")) == 0) { - modules <- append_module(modules, reporter_previewer_module()) - } - - # Replace splash / welcome screen once data is loaded ---- - # ignoreNULL to not trigger at the beginning when data is NULL - # just handle it once because data obtained through delayed loading should - # usually not change afterwards - # if restored from bookmarked state, `filter` is ignored - observeEvent(datasets_reactive(), ignoreNULL = TRUE, once = TRUE, { - logger::log_trace("srv_teal@5 setting main ui after data was pulled") - env$progress$set(0.5, message = "Setting up main UI") - on.exit(env$progress$close()) # main_ui_container contains splash screen first and we remove it and replace it by the real UI - + env$progress$set(0.5, message = "Setting up main UI") removeUI(sprintf("#%s:first-child", session$ns("main_ui_container"))) insertUI( selector = paste0("#", session$ns("main_ui_container")), @@ -230,7 +225,7 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) { ui = div(ui_tabs_with_filters( session$ns("main_ui"), modules = modules, - datasets = datasets_reactive(), + datasets = datasets, filter = filter )), # needed so that the UI inputs are available and can be immediately updated, otherwise, updating may not @@ -242,7 +237,7 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) { # registered once (calling server functions twice would trigger observers twice each time) active_module <- srv_tabs_with_filters( id = "main_ui", - datasets = datasets_reactive(), + datasets = datasets, modules = modules, reporter = reporter, filter = filter From bb59ffe2fa2d58a0fe85737d2f45b94ccc87e977 Mon Sep 17 00:00:00 2001 From: go_gonzo Date: Wed, 1 Nov 2023 07:30:39 +0100 Subject: [PATCH 3/9] moving data assertions to srv_teal --- R/data-data-utils.R | 2 +- R/init.R | 10 +++++----- R/module_teal.R | 8 ++++++++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/R/data-data-utils.R b/R/data-data-utils.R index a46d140fcd..200ec2f4f2 100644 --- a/R/data-data-utils.R +++ b/R/data-data-utils.R @@ -31,7 +31,7 @@ eval_and_mask <- function(data, # replace last code entry with masked code # format_expression needed to convert expression into character(1) # question: warnings and errors are not masked, is it ok? - data@code[length(code)] <- format_expression(.mask_code(code, args = input)) + data@code[length(data@code)] <- format_expression(.mask_code(code, args = input)) } # todo: should it be here or in datanames(data)? diff --git a/R/init.R b/R/init.R index 20014eff3e..c5c3aabd8f 100644 --- a/R/init.R +++ b/R/init.R @@ -160,11 +160,11 @@ init <- function(data, # convert teal.slice::teal_slices to teal::teal_slices filter <- as.teal_slices(as.list(filter)) - # resolve modules datanames - datanames <- teal.data::get_dataname(data) - join_keys <- teal.data::get_join_keys(data) - modules <- resolve_modules_datanames(modules = modules, datanames = datanames, join_keys = join_keys) - assert_filter_datanames(filter, datanames) + # resolve modules datanames (can't be here anymore as we might not have datanames in data) + # datanames <- teal.data::get_dataname(data) + # join_keys <- teal.data::get_join_keys(data) + # modules <- resolve_modules_datanames(modules = modules, datanames = datanames, join_keys = join_keys) + # assert_filter_datanames(filter, datanames) if (isTRUE(attr(filter, "module_specific"))) { module_names <- unlist(c(module_labels(modules), "global_filters")) diff --git a/R/module_teal.R b/R/module_teal.R index e53765be39..1f899e42f4 100644 --- a/R/module_teal.R +++ b/R/module_teal.R @@ -178,6 +178,14 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) { on.exit(env$progress$close()) env$progress$set(0.25, message = "Setting data") + # after loading data we can finaly get datanames and join_keys + # we need to resolve module$datanames to replace "all" to all datasets and include parent datanames + # we need to check whether filters are set for existing datanames + datanames <- teal.data::get_dataname(raw_data()) + join_keys <- teal.data::get_join_keys(raw_data()) + modules <- resolve_modules_datanames(modules = modules, datanames = datanames, join_keys = join_keys) + assert_filter_datanames(filter, datanames) + # create a list of data following structure of the nested modules list structure. # Because it's easier to unpack modules and datasets when they follow the same nested structure. datasets_singleton <- teal_data_to_filtered_data(raw_data()) From b84327bbf87d45c2c4b28a00d35ede3368a1d495 Mon Sep 17 00:00:00 2001 From: "27856297+dependabot-preview[bot]@users.noreply.github.com" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 1 Nov 2023 06:47:53 +0000 Subject: [PATCH 4/9] [skip actions] Roxygen Man Pages Auto Update --- DESCRIPTION | 2 + man/dot-mask_code.Rd | 17 ++++++ man/eval_and_mask.Rd | 27 ++++++++++ man/get_dataname.Rd | 14 +++++ man/get_join_keys.Rd | 7 ++- man/module_nested_tabs.Rd | 6 +++ man/module_tabs_with_filters.Rd | 6 +++ man/module_teal.Rd | 6 +++ man/submit_button_module.Rd | 23 ++++++++ man/tm_teal_data.Rd | 95 +++++++++++++++++++++++++++++++++ man/ui_teal_with_splash.Rd | 7 +++ 11 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 man/dot-mask_code.Rd create mode 100644 man/eval_and_mask.Rd create mode 100644 man/get_dataname.Rd create mode 100644 man/submit_button_module.Rd create mode 100644 man/tm_teal_data.Rd diff --git a/DESCRIPTION b/DESCRIPTION index 2591116aee..d0af2a49ba 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -71,6 +71,8 @@ LazyData: true Roxygen: list(markdown = TRUE) RoxygenNote: 7.2.3 Collate: + 'data-data-utils.R' + 'data-module.R' 'dummy_functions.R' 'get_rcode_utils.R' 'include_css_js.R' diff --git a/man/dot-mask_code.Rd b/man/dot-mask_code.Rd new file mode 100644 index 0000000000..4a53b0642b --- /dev/null +++ b/man/dot-mask_code.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/data-data-utils.R +\name{.mask_code} +\alias{.mask_code} +\title{substitute inputs in the code} +\usage{ +.mask_code(code, args) +} +\arguments{ +\item{code}{(\code{language}) code to substitute} + +\item{args}{(\code{list}) named list or arguments} +} +\description{ +Function replaces symbols in the provided code prefixed with \verb{input$} or \verb{input[["} +by values of the \code{args} argument. +} diff --git a/man/eval_and_mask.Rd b/man/eval_and_mask.Rd new file mode 100644 index 0000000000..3c3006d13c --- /dev/null +++ b/man/eval_and_mask.Rd @@ -0,0 +1,27 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/data-data-utils.R +\name{eval_and_mask} +\alias{eval_and_mask} +\title{Function runs the \code{code}, masks the \code{code} and creates \code{teal_data} object.} +\usage{ +eval_and_mask(data, code, input = list(), input_mask = list()) +} +\arguments{ +\item{code}{(optional \code{character} or \code{language})\cr +Object containing (defused) syntactically valid R expression to be executed in the shiny session. +Shouldn't be specified when \code{expr} is specified.} + +\item{input}{(\code{list}) containing inputs to be used in the \code{code}} + +\item{input_mask}{(optional \verb{named list})\cr +arguments to be substituted in the \code{code}. These (named) list elements are going to replace +symbols in the code prefixed with \verb{input$} or \verb{input[["}. Typically \code{input_mask} is used +to mask username or password with \code{list(password = quote(askpass::askpass()))}. +See section \verb{code masking} for more details.} +} +\value{ +\code{teal_data} object +} +\description{ +Function runs the \code{code}, masks the \code{code} and creates \code{teal_data} object. +} diff --git a/man/get_dataname.Rd b/man/get_dataname.Rd new file mode 100644 index 0000000000..fa091f15fe --- /dev/null +++ b/man/get_dataname.Rd @@ -0,0 +1,14 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/data-module.R +\name{get_dataname.ddl} +\alias{get_dataname.ddl} +\title{Get data names from \code{ddl}} +\usage{ +\method{get_dataname}{ddl}(x) +} +\arguments{ +\item{x}{(\code{ddl}) object} +} +\description{ +Get data names from \code{ddl} +} diff --git a/man/get_join_keys.Rd b/man/get_join_keys.Rd index 977a4b3145..f45b02ceee 100644 --- a/man/get_join_keys.Rd +++ b/man/get_join_keys.Rd @@ -1,9 +1,12 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/tdata.R -\name{get_join_keys.tdata} +% Please edit documentation in R/data-module.R, R/tdata.R +\name{get_join_keys.ddl} +\alias{get_join_keys.ddl} \alias{get_join_keys.tdata} \title{Extract \code{JoinKeys} from \code{tdata}} \usage{ +\method{get_join_keys}{ddl}(data) + \method{get_join_keys}{tdata}(data) } \arguments{ diff --git a/man/module_nested_tabs.Rd b/man/module_nested_tabs.Rd index b21eeaa139..e1f8aa9c44 100644 --- a/man/module_nested_tabs.Rd +++ b/man/module_nested_tabs.Rd @@ -56,6 +56,12 @@ srv_nested_tabs( \item{id}{(\code{character(1)})\cr module id} +\item{modules}{(\code{list}, \code{teal_modules} or \code{teal_module})\cr +nested list of \code{teal_modules} or \code{teal_module} objects or a single +\code{teal_modules} or \code{teal_module} object. These are the specific output modules which +will be displayed in the teal application. See \code{\link[=modules]{modules()}} and \code{\link[=module]{module()}} for +more details.} + \item{datasets}{(\verb{named list} of \code{FilteredData})\cr object to store filter state and filtered datasets, shared across modules. For more details see \code{\link[teal.slice:FilteredData]{teal.slice::FilteredData}}. Structure of the list must be the same as structure diff --git a/man/module_tabs_with_filters.Rd b/man/module_tabs_with_filters.Rd index 1be1c16d21..224cb51068 100644 --- a/man/module_tabs_with_filters.Rd +++ b/man/module_tabs_with_filters.Rd @@ -20,6 +20,12 @@ srv_tabs_with_filters( \item{id}{(\code{character(1)})\cr module id} +\item{modules}{(\code{list}, \code{teal_modules} or \code{teal_module})\cr +nested list of \code{teal_modules} or \code{teal_module} objects or a single +\code{teal_modules} or \code{teal_module} object. These are the specific output modules which +will be displayed in the teal application. See \code{\link[=modules]{modules()}} and \code{\link[=module]{module()}} for +more details.} + \item{datasets}{(\verb{named list} of \code{FilteredData})\cr object to store filter state and filtered datasets, shared across modules. For more details see \code{\link[teal.slice:FilteredData]{teal.slice::FilteredData}}. Structure of the list must be the same as structure diff --git a/man/module_teal.Rd b/man/module_teal.Rd index 10c1c8654c..6b66f26c10 100644 --- a/man/module_teal.Rd +++ b/man/module_teal.Rd @@ -35,6 +35,12 @@ argument) will be placed in the app's \code{ui} function so code which needs to \item{footer}{(\code{shiny.tag} or \code{character})\cr the footer of the app} +\item{modules}{(\code{list}, \code{teal_modules} or \code{teal_module})\cr +nested list of \code{teal_modules} or \code{teal_module} objects or a single +\code{teal_modules} or \code{teal_module} object. These are the specific output modules which +will be displayed in the teal application. See \code{\link[=modules]{modules()}} and \code{\link[=module]{module()}} for +more details.} + \item{raw_data}{(\code{reactive})\cr returns the \code{teal_data}, only evaluated once, \code{NULL} value is ignored} } diff --git a/man/submit_button_module.Rd b/man/submit_button_module.Rd new file mode 100644 index 0000000000..45ba8f52af --- /dev/null +++ b/man/submit_button_module.Rd @@ -0,0 +1,23 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/data-module.R +\name{submit_button_module} +\alias{submit_button_module} +\alias{submit_button_ui} +\alias{submit_button_server} +\title{Run code and mask inputs} +\usage{ +submit_button_ui(id) + +submit_button_server(id, ...) +} +\arguments{ +\item{id}{(\code{character}) \code{shiny} module id.} + +\item{...}{(\code{list}) arguments passed to \code{ddl_run} function.} +} +\value{ +\code{shiny} module +} +\description{ +Delayed Data Loading module with login and password input. +} diff --git a/man/tm_teal_data.Rd b/man/tm_teal_data.Rd new file mode 100644 index 0000000000..ed6a143533 --- /dev/null +++ b/man/tm_teal_data.Rd @@ -0,0 +1,95 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/data-module.R +\name{tm_teal_data} +\alias{tm_teal_data} +\title{DDL object} +\usage{ +tm_teal_data( + label = "data", + expr, + code, + input_mask = list(), + ui = submit_button_ui, + server = submit_button_server +) +} +\arguments{ +\item{expr}{(optional \code{expression})\cr +Syntactically valid R expression to be executed in the shiny session. +Shouldn't be specified when \code{code} is specified.} + +\item{code}{(optional \code{character} or \code{language})\cr +Object containing (defused) syntactically valid R expression to be executed in the shiny session. +Shouldn't be specified when \code{expr} is specified.} + +\item{input_mask}{(optional \verb{named list})\cr +arguments to be substituted in the \code{code}. These (named) list elements are going to replace +symbols in the code prefixed with \verb{input$} or \verb{input[["}. Typically \code{input_mask} is used +to mask username or password with \code{list(password = quote(askpass::askpass()))}. +See section \verb{code masking} for more details.} + +\item{ui}{(\code{shiny.tag})\cr +\code{shiny} user-interface module containing inputs whose \code{id} correspond to the arguments in the \code{code}.} + +\item{server}{(\code{function})\cr +\code{shiny} server module \code{\link[teal.data:teal_data-class]{teal.data::teal_data}} possibly wrapped in a \code{\link[=reactive]{reactive()}}. +\code{server} function should have \code{id} and \code{...} as formals. Where: +\itemize{ +\item \code{id} is a \code{shiny} module id, and +\item \code{...} passes arguments from the \code{ddl} object (\code{code}, \code{input_mask}, \code{datanames}, \code{join_keys}). +See section \verb{Code masking}. +}} + +\item{datanames}{(optional \code{character})\cr +Names of the datasets created by evaluation of the \code{code}. By default, \code{datanames} +are obtained from the \code{join_keys} or from results of the \code{code} evaluation. +If \code{code} evaluation creates objects which are not considered as datasets, they +should be omitted from \code{datanames} to avoid errors.} +} +\description{ +Object to execute custom DDL code in the shiny session. +} +\section{Creating reproducible data}{ + +\code{ddl} object can be used to create reproducible data in the shiny session. \code{ddl$server} function +can execute any R code and return \code{\link[teal.data:teal_data-class]{teal.data::teal_data}}. For reproducibility purposes, +we recommend to initialize empty \code{teal_data} object and evaluate necessary code with \code{eval_code} or \code{within}. + +\if{html}{\out{
}}\preformatted{function(id, ...) \{ + moduleServer(id, function(input, output, session) \{ + eventReactive(input$submit, \{ + data <- teal_data() |> within(\{ + # code to be run when app user presses submit + \}) + \}) + \}) +\} +}\if{html}{\out{
}} + +Obtained data is passed further in the \code{teal} app with \code{code} which can be used to recreate the objects. +} + +\section{Code masking}{ + +\code{ddl} object can be used in a way that evaluated code is different than the code +returned in \code{teal_data} object. Typically occurs when app user is asked to input a +password and we'd like to skip this input in the reproducible code. Possibly, users password +could be substituted with \code{askpass::askpass()} call, so the returned code is still executable but secure. +\code{ddl} developer must understand that this is a security risk and should be handled with care. +To make sure that the code is reproducible, \code{ddl} object should be used with \code{input_mask} argument. +\code{teal} provides convenience function \code{\link[=ddl_run]{ddl_run()}} which handles evaluation of the code, masking +and creating \code{teal_data} object. Such \code{server} function could look like this: + +\if{html}{\out{
}}\preformatted{server = function(id, ...) \{ + moduleServer(id, function(input, output, session) \{ + reactive(\{ + ddl_run(input = input, ...) + \}) + \}) +\} +}\if{html}{\out{
}} + +If \code{ddl} developer values more control, then might be interested in using \code{...} explicitly, +and create \code{teal_data} object manually. +} + diff --git a/man/ui_teal_with_splash.Rd b/man/ui_teal_with_splash.Rd index 0ece4d3027..098901459f 100644 --- a/man/ui_teal_with_splash.Rd +++ b/man/ui_teal_with_splash.Rd @@ -7,6 +7,7 @@ ui_teal_with_splash( id, data, + modules, title, header = tags$p("Add Title Here"), footer = tags$p("Add Footer Here") @@ -26,6 +27,12 @@ NOTE: teal does not guarantee reproducibility of the code when names of the list do not match the original object names. To ensure reproducibility please use \code{\link[teal.data:teal_data]{teal.data::teal_data()}} or \code{\link[teal.data:cdisc_data]{teal.data::cdisc_data()}} with \code{check = TRUE} enabled.} +\item{modules}{(\code{list}, \code{teal_modules} or \code{teal_module})\cr +nested list of \code{teal_modules} or \code{teal_module} objects or a single +\code{teal_modules} or \code{teal_module} object. These are the specific output modules which +will be displayed in the teal application. See \code{\link[=modules]{modules()}} and \code{\link[=module]{module()}} for +more details.} + \item{title}{(\code{NULL} or \code{character})\cr The browser window title (defaults to the host URL of the page).} From 2cc954fd97f6a15e77e0d657e7d344572a6f98af Mon Sep 17 00:00:00 2001 From: go_gonzo Date: Wed, 1 Nov 2023 11:04:01 +0100 Subject: [PATCH 5/9] reactive datanames check --- R/data-data-utils.R | 3 +++ R/init.R | 4 +-- R/module_nested_tabs.R | 6 ++++- R/module_tabs_with_filters.R | 8 +++++- R/module_teal.R | 21 +++++++--------- R/module_teal_with_splash.R | 27 ++++++++++++++++++++- R/utils.R | 47 +++++++++++++++++++++++++++++------- 7 files changed, 90 insertions(+), 26 deletions(-) diff --git a/R/data-data-utils.R b/R/data-data-utils.R index 200ec2f4f2..bf5cbb1ccd 100644 --- a/R/data-data-utils.R +++ b/R/data-data-utils.R @@ -16,6 +16,9 @@ eval_and_mask <- function(data, # evaluate code and substitute input data <- teal.code::eval_code(data, .mask_code(code, args = input)) + if (inherits(data, "qenv.error")) { + return(data) + } if (identical(ls(data@env), character(0))) { warning( diff --git a/R/init.R b/R/init.R index c5c3aabd8f..333485cf4d 100644 --- a/R/init.R +++ b/R/init.R @@ -106,7 +106,7 @@ #' shinyApp(app$ui, app$server) #' } #' -init <- function(data, +init <- function(data = teal_data(), modules, title = NULL, filter = teal_slices(), @@ -164,7 +164,7 @@ init <- function(data, # datanames <- teal.data::get_dataname(data) # join_keys <- teal.data::get_join_keys(data) # modules <- resolve_modules_datanames(modules = modules, datanames = datanames, join_keys = join_keys) - # assert_filter_datanames(filter, datanames) + # check_filter_datanames(filter, datanames) if (isTRUE(attr(filter, "module_specific"))) { module_names <- unlist(c(module_labels(modules), "global_filters")) diff --git a/R/module_nested_tabs.R b/R/module_nested_tabs.R index 0a012f32dd..a84950f03f 100644 --- a/R/module_nested_tabs.R +++ b/R/module_nested_tabs.R @@ -297,7 +297,11 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi checkmate::assert_class(datasets, "FilteredData") checkmate::assert_class(trigger_data, "reactiveVal") - datanames <- if (is.null(module$datanames)) datasets$datanames() else module$datanames + datanames <- if (is.null(module$datanames) || identical(module$datanames, "all")) { + datasets$datanames() + } else { + module$datanames # todo: include parents! + } # list of reactive filtered data data <- sapply( diff --git a/R/module_tabs_with_filters.R b/R/module_tabs_with_filters.R index 13fd6d5ebe..7f2fca1406 100644 --- a/R/module_tabs_with_filters.R +++ b/R/module_tabs_with_filters.R @@ -122,7 +122,13 @@ srv_tabs_with_filters <- function(id, ) if (!is_module_specific) { - active_datanames <- reactive(active_module()$datanames) + active_datanames <- reactive({ + if (identical(active_module()$datanames, "all")) { + singleton$datanames() + } else { + active_module()$datanames + } + }) singleton <- unlist(datasets)[[1]] singleton$srv_filter_panel("filter_panel", active_datanames = active_datanames) diff --git a/R/module_teal.R b/R/module_teal.R index 1f899e42f4..7c531ef053 100644 --- a/R/module_teal.R +++ b/R/module_teal.R @@ -160,8 +160,6 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) { } ) - env <- environment() - reporter <- teal.reporter::Reporter$new() if (is_arg_used(modules, "reporter") && length(extract_module(modules, "teal_module_previewer")) == 0) { modules <- append_module(modules, reporter_previewer_module()) @@ -172,26 +170,20 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) { # just handle it once because data obtained through delayed loading should # usually not change afterwards # if restored from bookmarked state, `filter` is ignored + env <- environment() observeEvent(raw_data(), { logger::log_trace("srv_teal@5 setting main ui after data was pulled") env$progress <- shiny::Progress$new(session) on.exit(env$progress$close()) env$progress$set(0.25, message = "Setting data") - # after loading data we can finaly get datanames and join_keys - # we need to resolve module$datanames to replace "all" to all datasets and include parent datanames - # we need to check whether filters are set for existing datanames - datanames <- teal.data::get_dataname(raw_data()) - join_keys <- teal.data::get_join_keys(raw_data()) - modules <- resolve_modules_datanames(modules = modules, datanames = datanames, join_keys = join_keys) - assert_filter_datanames(filter, datanames) - # create a list of data following structure of the nested modules list structure. # Because it's easier to unpack modules and datasets when they follow the same nested structure. datasets_singleton <- teal_data_to_filtered_data(raw_data()) # Singleton starts with only global filters active. filter_global <- Filter(function(x) x$id %in% attr(filter, "mapping")$global_filters, filter) datasets_singleton$set_filter_state(filter_global) + module_datasets <- function(modules) { if (inherits(modules, "teal_modules")) { datasets <- lapply(modules$children, module_datasets) @@ -201,11 +193,16 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) { } else if (isTRUE(attr(filter, "module_specific"))) { # we should create FilteredData even if modules$datanames is null # null controls a display of filter panel but data should be still passed - datanames <- if (is.null(modules$datanames)) teal.data::get_dataname(raw_data()) else modules$datanames - # todo: subset tdata object to datanames + datanames <- if (is.null(modules$datanames) || modules$datanames == "all") { + include_parent_datanames(raw_data()@datanames, raw_data()@join_keys) # todo: use methods instead + } else { + modules$datanames + } + # todo: subset teal_data to datanames datasets_module <- teal_data_to_filtered_data(raw_data()) # set initial filters + # - filtering filters for this module slices <- Filter(x = filter, f = function(x) { x$id %in% unique(unlist(attr(filter, "mapping")[c(modules$label, "global_filters")])) && x$dataname %in% datanames diff --git a/R/module_teal_with_splash.R b/R/module_teal_with_splash.R index 76d9e6d6b2..755da3e9a5 100644 --- a/R/module_teal_with_splash.R +++ b/R/module_teal_with_splash.R @@ -77,13 +77,38 @@ srv_teal_with_splash <- function(id, data, modules, filter = teal_slices()) { # raw_data contains teal_data object # either passed to teal::init or returned from ddl raw_data <- if (length(data_module)) { - do.call( + ddl_out <- do.call( data_module[[1]]$server, c( list(id = "data", data = data), data_module[[1]]$server_args ) ) + reactive({ + data <- ddl_out() + if (inherits(data, "qenv.error")) { + # + showNotification(sprintf("Error: %s", data$message)) + return(NULL) + } + + is_modules_ok <- check_modules_datanames(modules, teal.data::datanames(data)) + is_filter_ok <- check_filter_datanames(filter, teal.data::datanames(data)) + + if (!isTRUE(is_modules_ok)) { + showNotification(is_modules_ok) + # NULL won't trigger observe which waits for raw_data() + # we will need to consider validate process for filtered data and modules! + return(NULL) + } + if (!isTRUE(is_filter_ok)) { + showNotification(is_filter_ok) + # we allow app to continue if applied filters are outside + # of possible data range + } + + data + }) } else if (inherits(data, "teal_data")) { reactiveVal(data) } else if (inherits(data, "TealDataAbstract") && teal.data::is_pulled(data)) { diff --git a/R/utils.R b/R/utils.R index 7f67ca5a2f..bed5374994 100644 --- a/R/utils.R +++ b/R/utils.R @@ -141,20 +141,49 @@ resolve_modules_datanames <- function(modules, datanames, join_keys) { } } +check_modules_datanames <- function(modules, datanames) { + recursive_check_datanames <- function(modules, datanames) { + # check teal_modules against datanames + if (inherits(modules, "teal_modules")) { + sapply(modules$children, function(module) recursive_check_datanames(module, datanames = datanames)) + } else { + if (!modules$datanames %in% c("all", datanames)) { + sprintf( + "- Module %s has a different dataname than available in a 'data': %s not in %s", + modules$label, + toString(datanames), + toString(datanames) + ) + } + } + } + check_datanames <- unlist(recursive_check_datanames(modules, datanames)) + if (length(check_datanames)) { + paste(check_datanames, collapse = "\n") + } else { + TRUE + } +} + -assert_filter_datanames <- function(filter, datanames) { +check_filter_datanames <- function(filters, datanames) { # check teal_slices against datanames - for (i in seq_along(filter)) { - dataname_i <- shiny::isolate(filter[[i]]$dataname) - if (!dataname_i %in% datanames) { - stop( + out <- sapply( + filters, function(filter) { + dataname <- shiny::isolate(filter$dataname) + if (!dataname %in% datanames) { sprintf( - "filter[[%s]] has a different dataname than available in a 'data':\n %s not in %s", - i, - dataname_i, + "- Filter %s has a different dataname than available in a 'data':\n %s not in %s", + filter$label, + dataname, toString(datanames) ) - ) + } } + ) + if (length(unlist(out))) { + paste(out, collapse = "\n") + } else { + TRUE } } From b083a115b219b7eae98da680062767578df392a8 Mon Sep 17 00:00:00 2001 From: "27856297+dependabot-preview[bot]@users.noreply.github.com" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 1 Nov 2023 10:09:12 +0000 Subject: [PATCH 6/9] [skip actions] Roxygen Man Pages Auto Update --- man/init.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/init.Rd b/man/init.Rd index 07bbec2deb..fef6b25b07 100644 --- a/man/init.Rd +++ b/man/init.Rd @@ -5,7 +5,7 @@ \title{Create the Server and UI Function For the Shiny App} \usage{ init( - data, + data = teal_data(), modules, title = NULL, filter = teal_slices(), From a31ffa4526f1517f36ffc90c1178a4faad2a15b0 Mon Sep 17 00:00:00 2001 From: go_gonzo Date: Wed, 1 Nov 2023 11:46:02 +0100 Subject: [PATCH 7/9] fix checking of datanames of static teal_data --- R/data-module.R | 10 +++---- R/module_teal_with_splash.R | 54 +++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/R/data-module.R b/R/data-module.R index 9df0361f83..5e71533553 100644 --- a/R/data-module.R +++ b/R/data-module.R @@ -26,14 +26,14 @@ #' could be substituted with `askpass::askpass()` call, so the returned code is still executable but secure. #' `ddl` developer must understand that this is a security risk and should be handled with care. #' To make sure that the code is reproducible, `ddl` object should be used with `input_mask` argument. -#' `teal` provides convenience function [ddl_run()] which handles evaluation of the code, masking +#' `teal` provides convenience function [eval_and_mask()] which handles evaluation of the code, masking #' and creating `teal_data` object. Such `server` function could look like this: #' #' ``` #' server = function(id, ...) { #' moduleServer(id, function(input, output, session) { #' reactive({ -#' ddl_run(input = input, ...) +#' eval_and_mask(input = input, ...) #' }) #' }) #' } @@ -97,7 +97,7 @@ tm_teal_data <- function(label = "data", if (!missing(expr) || !missing(code)) { # this is intended to be used with input mask # but in the same time we can't forbid user to use it - # without input_mask. Some users might prefer to use ddl_run + # without input_mask. Some users might prefer to use `eval_and_mask` # to automaticaly handle their code. # Q: can NEST bear responsibility for reproducibility of the masked code? if (!missing(expr)) { @@ -121,7 +121,7 @@ tm_teal_data <- function(label = "data", #' #' #' @param id (`character`) `shiny` module id. -#' @param ... (`list`) arguments passed to `ddl_run` function. +#' @param ... (`list`) arguments passed to [eval_and_mask()]. #' @return `shiny` module NULL @@ -137,7 +137,7 @@ submit_button_ui <- function(id) { submit_button_server <- function(id, ...) { moduleServer(id, function(input, output, session) { tdata <- eventReactive(input$submit, { - ddl_run(input = input, ...) + eval_and_mask(input = input, ...) }) # would need to make sure we handle reactivity correctly here as teal::init expects not reactive teal_data... diff --git a/R/module_teal_with_splash.R b/R/module_teal_with_splash.R index 755da3e9a5..e4f9e89709 100644 --- a/R/module_teal_with_splash.R +++ b/R/module_teal_with_splash.R @@ -84,31 +84,6 @@ srv_teal_with_splash <- function(id, data, modules, filter = teal_slices()) { data_module[[1]]$server_args ) ) - reactive({ - data <- ddl_out() - if (inherits(data, "qenv.error")) { - # - showNotification(sprintf("Error: %s", data$message)) - return(NULL) - } - - is_modules_ok <- check_modules_datanames(modules, teal.data::datanames(data)) - is_filter_ok <- check_filter_datanames(filter, teal.data::datanames(data)) - - if (!isTRUE(is_modules_ok)) { - showNotification(is_modules_ok) - # NULL won't trigger observe which waits for raw_data() - # we will need to consider validate process for filtered data and modules! - return(NULL) - } - if (!isTRUE(is_filter_ok)) { - showNotification(is_filter_ok) - # we allow app to continue if applied filters are outside - # of possible data range - } - - data - }) } else if (inherits(data, "teal_data")) { reactiveVal(data) } else if (inherits(data, "TealDataAbstract") && teal.data::is_pulled(data)) { @@ -145,7 +120,34 @@ srv_teal_with_splash <- function(id, data, modules, filter = teal_slices()) { raw_data } - res <- srv_teal(id = "teal", modules = modules, raw_data = raw_data, filter = filter) + raw_data_checked <- reactive({ + data <- raw_data() + if (inherits(data, "qenv.error")) { + # + showNotification(sprintf("Error: %s", data$message)) + return(NULL) + } + + is_modules_ok <- check_modules_datanames(modules, teal.data::datanames(data)) + is_filter_ok <- check_filter_datanames(filter, teal.data::datanames(data)) + + if (!isTRUE(is_modules_ok)) { + showNotification(is_modules_ok) + # NULL won't trigger observe which waits for raw_data() + # we will need to consider validate process for filtered data and modules! + return(NULL) + } + if (!isTRUE(is_filter_ok)) { + showNotification(is_filter_ok) + # we allow app to continue if applied filters are outside + # of possible data range + } + + data + }) + + + res <- srv_teal(id = "teal", modules = modules, raw_data = raw_data_checked, filter = filter) logger::log_trace("srv_teal_with_splash initialized module with data { toString(get_dataname(data))}.") return(res) }) From 8bca4ddfc7fe6a70d65baa6d205fb393cc879b20 Mon Sep 17 00:00:00 2001 From: "27856297+dependabot-preview[bot]@users.noreply.github.com" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 1 Nov 2023 10:51:03 +0000 Subject: [PATCH 8/9] [skip actions] Roxygen Man Pages Auto Update --- man/submit_button_module.Rd | 2 +- man/tm_teal_data.Rd | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/man/submit_button_module.Rd b/man/submit_button_module.Rd index 45ba8f52af..23f08633b9 100644 --- a/man/submit_button_module.Rd +++ b/man/submit_button_module.Rd @@ -13,7 +13,7 @@ submit_button_server(id, ...) \arguments{ \item{id}{(\code{character}) \code{shiny} module id.} -\item{...}{(\code{list}) arguments passed to \code{ddl_run} function.} +\item{...}{(\code{list}) arguments passed to \code{\link[=eval_and_mask]{eval_and_mask()}}.} } \value{ \code{shiny} module diff --git a/man/tm_teal_data.Rd b/man/tm_teal_data.Rd index ed6a143533..b2a466b4b7 100644 --- a/man/tm_teal_data.Rd +++ b/man/tm_teal_data.Rd @@ -77,13 +77,13 @@ password and we'd like to skip this input in the reproducible code. Possibly, us could be substituted with \code{askpass::askpass()} call, so the returned code is still executable but secure. \code{ddl} developer must understand that this is a security risk and should be handled with care. To make sure that the code is reproducible, \code{ddl} object should be used with \code{input_mask} argument. -\code{teal} provides convenience function \code{\link[=ddl_run]{ddl_run()}} which handles evaluation of the code, masking +\code{teal} provides convenience function \code{\link[=eval_and_mask]{eval_and_mask()}} which handles evaluation of the code, masking and creating \code{teal_data} object. Such \code{server} function could look like this: \if{html}{\out{
}}\preformatted{server = function(id, ...) \{ moduleServer(id, function(input, output, session) \{ reactive(\{ - ddl_run(input = input, ...) + eval_and_mask(input = input, ...) \}) \}) \} From 0fbfdabd6d2dd3258073c4cecefa0b5660a60f23 Mon Sep 17 00:00:00 2001 From: go_gonzo Date: Wed, 1 Nov 2023 13:14:37 +0100 Subject: [PATCH 9/9] move teal_module_data to data before splitting ddl and shiny modules --- R/init.R | 6 ++++-- R/module_teal_with_splash.R | 22 ++++++++-------------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/R/init.R b/R/init.R index 333485cf4d..94cbbfdf39 100644 --- a/R/init.R +++ b/R/init.R @@ -115,11 +115,11 @@ init <- function(data = teal_data(), id = character(0)) { logger::log_trace("init initializing teal app with: data ({ class(data)[1] }).") - if (!inherits(data, c("TealData", "teal_data"))) { + if (!inherits(data, c("TealData", "teal_data", "teal_module_data"))) { data <- teal.data::to_relational_data(data = data) } - checkmate::assert_multi_class(data, c("TealData", "teal_data")) + checkmate::assert_multi_class(data, c("TealData", "teal_data", "teal_module_data")) checkmate::assert_multi_class(modules, c("teal_module", "list", "teal_modules")) checkmate::assert_string(title, null.ok = TRUE) checkmate::assert( @@ -147,6 +147,8 @@ init <- function(data = teal_data(), hashables <- mget(c("data", "modules")) hashables$data <- if (inherits(hashables$data, "teal_data")) { as.list(hashables$data@env) + } else if (inherits(data, "teal_module_data")) { + # what? } else if (hashables$data$is_pulled()) { sapply(get_dataname(hashables$data), simplify = FALSE, function(dn) { hashables$data$get_dataset(dn)$get_raw_data() diff --git a/R/module_teal_with_splash.R b/R/module_teal_with_splash.R index e4f9e89709..1dc8fbda0b 100644 --- a/R/module_teal_with_splash.R +++ b/R/module_teal_with_splash.R @@ -23,18 +23,16 @@ ui_teal_with_splash <- function(id, title, header = tags$p("Add Title Here"), footer = tags$p("Add Footer Here")) { - checkmate::assert_multi_class(data, c("TealDataAbstract", "teal_data")) + checkmate::assert_multi_class(data, c("TealDataAbstract", "teal_data", "teal_module_data")) ns <- NS(id) - data_module <- extract_module(modules, "teal_module_data")[[1]] - # Startup splash screen for delayed loading # We use delayed loading in all cases, even when the data does not need to be fetched. # This has the benefit that when filtering the data takes a lot of time initially, the # Shiny app does not time out. - splash_ui <- if (length(data_module)) { - data_module$ui(ns("data")) + splash_ui <- if (inherits(data, "teal_module_data")) { + data$ui(ns("data")) } else if (inherits(data, "teal_data")) { div() } else if (inherits(data, "TealDataAbstract") && teal.data::is_pulled(data)) { @@ -62,7 +60,7 @@ ui_teal_with_splash <- function(id, #' If data is not loaded yet, `reactive` returns `NULL`. #' @export srv_teal_with_splash <- function(id, data, modules, filter = teal_slices()) { - checkmate::assert_multi_class(data, c("TealDataAbstract", "teal_data")) + checkmate::assert_multi_class(data, c("TealDataAbstract", "teal_data", "teal_module_data")) moduleServer(id, function(input, output, session) { logger::log_trace("srv_teal_with_splash initializing module with data { toString(get_dataname(data))}.") @@ -70,18 +68,14 @@ srv_teal_with_splash <- function(id, data, modules, filter = teal_slices()) { shinyjs::showLog() } - data_module <- extract_module(modules, "teal_module_data") - if (length(data_module) > 1L) stop("Only one `teal_module_data` can be used.") - modules <- drop_module(modules, "teal_module_data") - # raw_data contains teal_data object # either passed to teal::init or returned from ddl - raw_data <- if (length(data_module)) { + raw_data <- if (inherits(data, "teal_module_data")) { ddl_out <- do.call( - data_module[[1]]$server, + data$server, c( - list(id = "data", data = data), - data_module[[1]]$server_args + list(id = "data"), + data$server_args ) ) } else if (inherits(data, "teal_data")) {