diff --git a/.github/workflows/check.yaml b/.github/workflows/check.yaml index 90e8823f08..0d4a2fd320 100644 --- a/.github/workflows/check.yaml +++ b/.github/workflows/check.yaml @@ -10,6 +10,7 @@ on: - ready_for_review branches: - main + - 1304-handle-data-inputs@main push: branches: - main diff --git a/R/module_nested_tabs.R b/R/module_nested_tabs.R index fae0714701..fbe1f86866 100644 --- a/R/module_nested_tabs.R +++ b/R/module_nested_tabs.R @@ -302,33 +302,21 @@ srv_teal_module.teal_module <- function(id, #' @keywords internal ui_validate_teal_data <- function(id) { ns <- NS(id) - uiOutput(ns("validation_error")) + div( + ui_check_class_teal_data(ns("class_teal_data")), + ui_is_empty_teal_data(ns("is_empty_teal_data")) + ) } #' @keywords internal srv_validate_teal_data <- function(id, data) { + checkmate::assert_string(id) moduleServer(id, function(input, output, session) { - output$validation_error <- renderUI({ - if (inherits(data(), "teal_data")) { - validate( - need( - !.is_empty_teal_data(data()), - "The module did not recieve any data" - ) - ) - } else { - validate( - need( - FALSE, - "The module did not recieve `teal_data`" - ) - ) - } - }) + srv_check_class_teal_data("check_class_teal_data", data) + srv_is_empty_teal_data("is_empty_teal_data", data, "Empty `teal_data` object.") }) } - # This function calls a module server function. .call_teal_module <- function(modules, datasets, filtered_teal_data, reporter) { # collect arguments to run teal_module diff --git a/R/module_teal.R b/R/module_teal.R index 2cd0f666bd..3adcfc5bd6 100644 --- a/R/module_teal.R +++ b/R/module_teal.R @@ -102,7 +102,7 @@ ui_teal <- function(id, shiny_busy_message_panel, tags$div( class = "teal_validated", - uiOutput(ns("shiny_error")) + ui_validate_qenv_error(ns("qenv_error")) ), tags$div( id = ns("tabpanel_wrapper"), @@ -198,23 +198,8 @@ srv_teal <- function(id, data, modules, filter = teal_slices()) { } }) - output$shiny_error <- renderUI({ - if (inherits(init_data(), "qenv.error")) { - validate( - need( - FALSE, - paste( - "Error when executing the `data` module:", - strip_style(paste(init_data()$message, collapse = "\n")), - "\nCheck your inputs or contact app developer if error persists.", - collapse = "\n" - ) - ) - ) - } + srv_validate_qenv_error("qenv_error", init_data) - NULL - }) datasets_rv <- if (!isTRUE(attr(filter, "module_specific"))) { eventReactive(data_rv(), { if (!inherits(data_rv(), "teal_data")) { diff --git a/R/module_teal_data.R b/R/module_teal_data.R index 6c73cb454b..07cae2ad62 100644 --- a/R/module_teal_data.R +++ b/R/module_teal_data.R @@ -61,22 +61,10 @@ srv_teal_data <- function(id, moduleServer(id, function(input, output, session) { logger::log_debug("srv_teal_data initializing.") - data_in <- reactive({ - if (inherits(data(), "teal_data")) { - if (.is_empty_teal_data(data())) { - validate( - need( - FALSE, - "Empty `teal_data` object." - ) - ) - } - } - data() - }) + srv_is_empty_teal_data("is_empty_teal_data", data, "The module did not receive any data") data_out <- if (is_arg_used(data_module$server, "data")) { - data_module$server(id = "data", data = data_in) + data_module$server(id = "data", data = data) } else { data_module$server(id = "data") } @@ -100,10 +88,13 @@ srv_teal_data <- function(id, #' @rdname module_teal_data ui_validate_reactive_teal_data <- function(id) { + ns <- NS(id) div( class = "teal_validated", - uiOutput(NS(id, "shiny_errors")), - uiOutput(NS(id, "shiny_warnings")) + ui_validate_silent_error(ns("silent_error")), + ui_validate_qenv_error(ns("qenv_error")), + ui_check_class_teal_data(ns("class_teal_data")), + ui_check_shiny_warnings(ns("shiny_warnings")) ) } @@ -117,16 +108,33 @@ srv_validate_reactive_teal_data <- function(id, # nolint: object_length checkmate::assert_flag(validate_shiny_silent_error) moduleServer(id, function(input, output, session) { - data_out_r <- reactive(tryCatch(data(), error = function(e) e)) + data_rv <- reactive(tryCatch(data(), error = function(e) e)) + + # there is an empty reactive cycle on init! + srv_validate_silent_error("silent_error", data_rv, validate_shiny_silent_error) + srv_validate_qenv_error("qenv_error", data_rv) + srv_check_class_teal_data("class_teal_data", data_rv) + srv_check_shiny_warnings("shiny_warnings", data_rv, modules) + + data_rv + }) +} - data_validated <- reactive({ - # custom module can return error - data_out <- data_out_r() +#' @keywords internal +ui_validate_silent_error <- function(id) { + ns <- NS(id) + uiOutput(ns("error")) +} - # there is an empty reactive cycle on init! - if (inherits(data_out, "shiny.silent.error") && identical(data_out$message, "")) { +#' @keywords internal +srv_validate_silent_error <- function(id, data, validate_shiny_silent_error) { + checkmate::assert_string(id) + checkmate::assert_flag(validate_shiny_silent_error) + moduleServer(id, function(input, output, session) { + output$error <- renderUI({ + if (inherits(data(), "shiny.silent.error") && identical(data()$message, "")) { if (!validate_shiny_silent_error) { - return(teal_data()) + return(NULL) } else { validate( need( @@ -140,40 +148,95 @@ srv_validate_reactive_teal_data <- function(id, # nolint: object_length ) } } + }) + }) +} - # to handle errors and qenv.error(s) - if (inherits(data_out, c("qenv.error"))) { +#' @keywords internal +ui_validate_qenv_error <- function(id) { + ns <- NS(id) + uiOutput(ns("error")) +} + +#' @keywords internal +srv_validate_qenv_error <- function(id, data) { + checkmate::assert_string(id) + moduleServer(id, function(input, output, session) { + output$error <- renderUI({ + if (inherits(data(), c("qenv.error"))) { validate( need( FALSE, paste( "Error when executing the `data` module:", - strip_style(paste(data_out$message, collapse = "\n")), + strip_style(paste(data()$message, collapse = "\n")), "\nCheck your inputs or contact app developer if error persists.", collapse = "\n" ) ) ) } + }) + }) +} +#' @keywords internal +ui_check_class_teal_data <- function(id) { + ns <- NS(id) + uiOutput(ns("check")) +} + +#' @keywords internal +srv_check_class_teal_data <- function(id, data) { + checkmate::assert_string(id) + moduleServer(id, function(input, output, session) { + output$check <- renderUI({ validate( need( - checkmate::test_class(data_out, "teal_data"), + checkmate::test_class(data(), "teal_data"), "Did not recieve a valid `teal_data` object. Cannot proceed further." ) ) - - data_out }) + }) +} + +#' @keywords internal +ui_is_empty_teal_data <- function(id) { + ns <- NS(id) + uiOutput(ns("is_empty")) +} - output$shiny_errors <- renderUI({ - data_validated() - NULL +#' @keywords internal +srv_is_empty_teal_data <- function(id, data, message) { + checkmate::assert_string(id) + moduleServer(id, function(input, output, session) { + output$is_empty <- renderUI({ + if (inherits(data(), "teal_data")) { + validate( + need( + !.is_empty_teal_data(data()), + message + ) + ) + } }) + }) +} - output$shiny_warnings <- renderUI({ - if (inherits(data_out_r(), "teal_data")) { - is_modules_ok <- check_modules_datanames(modules = modules, datanames = .teal_data_ls(data_validated())) +#' @keywords internal +ui_check_shiny_warnings <- function(id) { + ns <- NS(id) + uiOutput(NS(id, "warnings")) +} + +#' @keywords internal +srv_check_shiny_warnings <- function(id, data, modules) { + checkmate::assert_string(id) + moduleServer(id, function(input, output, session) { + output$warnings <- renderUI({ + if (inherits(data(), "teal_data")) { + is_modules_ok <- check_modules_datanames(modules = modules, datanames = .teal_data_ls(data())) if (!isTRUE(is_modules_ok)) { tags$div( class = "teal-output-warning", @@ -185,7 +248,5 @@ srv_validate_reactive_teal_data <- function(id, # nolint: object_length } } }) - - data_validated }) } diff --git a/tests/testthat/test-module_teal.R b/tests/testthat/test-module_teal.R index c9ed8e1f13..be70f766f3 100644 --- a/tests/testthat/test-module_teal.R +++ b/tests/testthat/test-module_teal.R @@ -511,7 +511,7 @@ testthat::describe("srv_teal teal_modules", { trimws( rvest::html_text2( rvest::read_html( - output[["teal_modules-module_1-validate_datanames-shiny_warnings"]]$html + output[["teal_modules-module_1-validate_datanames-shiny_warnings-warnings"]]$html ) ) ), diff --git a/tests/testthat/test-shinytest2-filter_panel.R b/tests/testthat/test-shinytest2-filter_panel.R index aa65eb567d..22be061ab7 100644 --- a/tests/testthat/test-shinytest2-filter_panel.R +++ b/tests/testthat/test-shinytest2-filter_panel.R @@ -1,7 +1,7 @@ testthat::skip_if_not_installed("shinytest2") testthat::skip_if_not_installed("rvest") -testthat::test_that("e2e: module content is updated when a data is filtered in filter panel", { +testthat::test_that("e2e: module content is updated when data is filtered in filter panel", { skip_if_too_deep(5) app <- TealAppDriver$new( data = simple_teal_data(),