From 545e9c883d202539ea370d038020e9d391f671c6 Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 16:21:58 +0200
Subject: [PATCH 01/26] deprecate filters argument in module function

---
 R/modules.R | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/R/modules.R b/R/modules.R
index d3ef4f0fcf..b0dbb377ee 100644
--- a/R/modules.R
+++ b/R/modules.R
@@ -190,16 +190,24 @@ module <- function(label = "module",
                    ui = function(id, ...) {
                      tags$p(paste0("This module has no UI (id: ", id, " )"))
                    },
-                   filters = "all",
+                   filters,
+                   datanames = "all",
                    server_args = NULL,
                    ui_args = NULL) {
   checkmate::assert_string(label)
   checkmate::assert_function(server)
   checkmate::assert_function(ui)
-  checkmate::assert_character(filters, min.len = 1, null.ok = TRUE, any.missing = FALSE)
+  checkmate::assert_character(datanames, min.len = 1, null.ok = TRUE, any.missing = FALSE)
   checkmate::assert_list(server_args, null.ok = TRUE, names = "named")
   checkmate::assert_list(ui_args, null.ok = TRUE, names = "named")
 
+  if (!missing(filters)) {
+    checkmate::assert_character(filters, min.len = 1, null.ok = TRUE, any.missing = FALSE)
+    datanames <- filters
+    warning("The `filters` argument is deprecated and will be removed in the next release. ",
+            "Please use `datanames` instead. ")
+  }
+
   if (label == "global_filters") {
     stop("Label 'global_filters' is reserved in teal. Please change to something else.")
   }
@@ -254,7 +262,7 @@ module <- function(label = "module",
   structure(
     list(
       label = label,
-      server = server, ui = ui, filters = filters,
+      server = server, ui = ui, datanames = datanames,
       server_args = server_args, ui_args = ui_args
     ),
     class = "teal_module"

From c71f383ad3d8405949a8d1ad544b260b79f85040 Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 16:23:08 +0200
Subject: [PATCH 02/26] propagate argument change

---
 R/example_module.R            |  4 ++--
 R/init.R                      | 10 +++++-----
 R/module_nested_tabs.R        |  2 +-
 R/module_tabs_with_filters.R  |  2 +-
 R/modules_debugging.R         |  2 +-
 R/reporter_previewer_module.R |  2 +-
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/R/example_module.R b/R/example_module.R
index b42782fe95..21c252f770 100644
--- a/R/example_module.R
+++ b/R/example_module.R
@@ -15,7 +15,7 @@
 #'   shinyApp(app$ui, app$server)
 #' }
 #' @export
-example_module <- function(label = "example teal module", filters = "all") {
+example_module <- function(label = "example teal module", datanames = "all") {
   checkmate::assert_string(label)
   module(
     label,
@@ -32,6 +32,6 @@ example_module <- function(label = "example teal module", filters = "all") {
         encoding = selectInput(ns("dataname"), "Choose a dataset", choices = names(data))
       )
     },
-    filters = filters
+    datanames = datanames
   )
 }
diff --git a/R/init.R b/R/init.R
index 745ddfbb38..ea98d7254e 100644
--- a/R/init.R
+++ b/R/init.R
@@ -70,7 +70,7 @@
 #'       label = "data source",
 #'       server = function(input, output, session, data) {},
 #'       ui = function(id, ...) div(p("information about data source")),
-#'       filters = "all"
+#'       datanames = "all"
 #'     ),
 #'     example_module(label = "example teal module"),
 #'     module(
@@ -84,7 +84,7 @@
 #'         ns <- NS(id)
 #'         plotOutput(ns("hist"))
 #'       },
-#'       filters = "new_iris"
+#'       datanames = "new_iris"
 #'     )
 #'   ),
 #'   title = "App title",
@@ -144,10 +144,10 @@ init <- function(data,
       modules$children <- sapply(modules$children, resolve_modules_datanames, simplify = FALSE)
       modules
     } else {
-      modules$filters <- if (identical(modules$filters, "all")) {
+      modules$datanames <- if (identical(modules$datanames, "all")) {
         datanames
-      } else if (is.character(modules$filters)) {
-        datanames_adjusted <- intersect(modules$filters, datanames)
+      } else if (is.character(modules$datanames)) {
+        datanames_adjusted <- intersect(modules$datanames, datanames)
         include_parent_datanames(dataname = datanames_adjusted, join_keys = join_keys)
       }
       modules
diff --git a/R/module_nested_tabs.R b/R/module_nested_tabs.R
index fcdaa4cabc..3627cfd91c 100644
--- a/R/module_nested_tabs.R
+++ b/R/module_nested_tabs.R
@@ -292,7 +292,7 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
 #' @keywords internal
 .datasets_to_data <- function(module, datasets, trigger_data = reactiveVal(1L)) {
   checkmate::assert_class(trigger_data, "reactiveVal")
-  datanames <- if (is.null(module$filter)) datasets$datanames() else module$filter
+  datanames <- if (is.null(module$datanames)) datasets$datanames() else module$datanames
 
   # list of reactive filtered data
   data <- sapply(
diff --git a/R/module_tabs_with_filters.R b/R/module_tabs_with_filters.R
index 56b49b01e9..d4e17e7cc0 100644
--- a/R/module_tabs_with_filters.R
+++ b/R/module_tabs_with_filters.R
@@ -164,7 +164,7 @@ srv_tabs_with_filters <- function(id, datasets, modules, reporter = teal.reporte
     )
 
     if (!is_module_specific) {
-      active_datanames <- reactive(active_module()$filters)
+      active_datanames <- reactive(active_module()$datanames)
       singleton <- unlist(datasets)[[1]]
       singleton$srv_filter_panel("filter_panel", active_datanames = active_datanames)
 
diff --git a/R/modules_debugging.R b/R/modules_debugging.R
index 4213eba88e..6260f3908c 100644
--- a/R/modules_debugging.R
+++ b/R/modules_debugging.R
@@ -40,6 +40,6 @@ filter_calls_module <- function(label = "Filter Calls Module") { # nolint
         verbatimTextOutput(ns("filter_calls"))
       )
     },
-    filters = "all"
+    datanames = "all"
   )
 }
diff --git a/R/reporter_previewer_module.R b/R/reporter_previewer_module.R
index 239ae70ec2..fd2411f76a 100644
--- a/R/reporter_previewer_module.R
+++ b/R/reporter_previewer_module.R
@@ -30,7 +30,7 @@ reporter_previewer_module <- function(label = "Report previewer", server_args =
   module <- module(
     label = label,
     server = srv, ui = ui,
-    server_args = server_args, ui_args = list(), filters = NULL
+    server_args = server_args, ui_args = list(), datanames = NULL
   )
   class(module) <- c("teal_module_previewer", class(module))
   module

From f8f37a5551f4ade6f45e21a06c75de07d8e8d52f Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 16:25:46 +0200
Subject: [PATCH 03/26] hide filter panel for datanames = NULL

---
 R/modules.R | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/R/modules.R b/R/modules.R
index b0dbb377ee..d7a2f66c5e 100644
--- a/R/modules.R
+++ b/R/modules.R
@@ -229,6 +229,11 @@ module <- function(label = "module",
     )
   }
 
+  if (!is.element("data", server_formals)) {
+    message(sprintf("module \"%s\" server function takes no data so \"datanames\" will be ignored", label))
+    datanames <- NULL
+  }
+
   srv_extra_args <- setdiff(names(server_args), server_formals)
   if (length(srv_extra_args) > 0 && !"..." %in% server_formals) {
     stop(

From ab42e1efac3dad6107d938a5da4d726d31524ade Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 16:36:46 +0200
Subject: [PATCH 04/26] complete propagation

---
 R/module_nested_tabs.R | 6 +++---
 R/module_teal.R        | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/R/module_nested_tabs.R b/R/module_nested_tabs.R
index 3627cfd91c..910f34c841 100644
--- a/R/module_nested_tabs.R
+++ b/R/module_nested_tabs.R
@@ -123,7 +123,7 @@ ui_nested_tabs.teal_module <- function(id, modules, datasets, depth = 0L, is_mod
     )
   )
 
-  if (!is.null(modules$filter) && is_module_specific) {
+  if (!is.null(modules$datanames) && is_module_specific) {
     fluidRow(
       column(width = 9, teal_ui, class = "teal_primary_col"),
       column(
@@ -210,8 +210,8 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
   logger::log_trace("srv_nested_tabs.teal_module initializing the module: { deparse1(modules$label) }.")
   moduleServer(id = id, module = function(input, output, session) {
     modules$server_args <- teal.transform::resolve_delayed(modules$server_args, datasets)
-    if (!is.null(modules$filter) && is_module_specific) {
-      datasets$srv_filter_panel("module_filter_panel", active_datanames = reactive(modules$filter))
+    if (!is.null(modules$datanames) && is_module_specific) {
+      datasets$srv_filter_panel("module_filter_panel", active_datanames = reactive(modules$datanames))
     }
 
     # Create two triggers to limit reactivity between filter-panel and modules.
diff --git a/R/module_teal.R b/R/module_teal.R
index 93f9c19f48..13197c0212 100644
--- a/R/module_teal.R
+++ b/R/module_teal.R
@@ -179,9 +179,9 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
           names(datasets) <- labels
           datasets
         } else if (isTRUE(attr(filter, "module_specific"))) {
-          # we should create FilteredData even if modules$filter is null
+          # 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$filter)) raw_data()$get_datanames() else modules$filter
+          datanames <- if (is.null(modules$datanames)) raw_data()$get_datanames() else modules$datanames
           data_objects <- sapply(
             datanames,
             simplify = FALSE,

From 3ce070d57518ba4939f90c973f2bf5652cee52dd Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 16:43:28 +0200
Subject: [PATCH 05/26] amend documentation

---
 R/modules.R                                |  9 +++++----
 man/example_module.Rd                      |  6 +++---
 man/init.Rd                                |  4 ++--
 man/module.Rd                              |  9 ++++++---
 man/modules.Rd                             |  4 ++--
 vignettes/adding-support-for-reporting.Rmd | 10 +++++-----
 vignettes/creating-custom-modules.Rmd      |  4 ++--
 vignettes/filter-panel.Rmd                 |  6 +++---
 8 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/R/modules.R b/R/modules.R
index d7a2f66c5e..2df84bdf51 100644
--- a/R/modules.R
+++ b/R/modules.R
@@ -41,7 +41,7 @@
 #'           ns <- NS(id)
 #'           tagList(dataTableOutput(ns("data")))
 #'         },
-#'         filters = "all"
+#'         datanames = "all"
 #'       )
 #'     ),
 #'     module(
@@ -58,7 +58,7 @@
 #'         ns <- NS(id)
 #'         tagList(textOutput(ns("text")))
 #'       },
-#'       filters = NULL
+#'       datanames = NULL
 #'     )
 #'   )
 #' )
@@ -145,10 +145,11 @@ is_arg_used <- function(modules, arg) {
 #'  - `data` (optional)  module will receive list of reactive (filtered) data specified in the `filters` argument.
 #'  - `datasets` (optional)  module will receive `FilteredData`. (See `[teal.slice::FilteredData]`).
 #'  - `...` (optional) `ui_args` elements will be passed to the module named argument or to the `...`.
-#' @param filters (`character`) A vector with `datanames` that are relevant for the item. The
+#' @param filters (`character`) Deprecated. Use `datanames` instead.
+#' @param datanames (`character`) A vector with `datanames` that are relevant for the item. The
 #'   filter panel will automatically update the shown filters to include only
 #'   filters in the listed datasets. `NULL` will hide the filter panel,
-#'   and the keyword `'all'` will show the filters of all datasets. `filters` determines also
+#'   and the keyword `'all'` will show filters of all datasets. `datanames` also determines
 #'   a subset of datasets which are appended to the `data` argument in `server` function.
 #' @param server_args (named `list`) with additional arguments passed on to the
 #'   `server` function.
diff --git a/man/example_module.Rd b/man/example_module.Rd
index 8869f1394a..37db37a1c8 100644
--- a/man/example_module.Rd
+++ b/man/example_module.Rd
@@ -4,16 +4,16 @@
 \alias{example_module}
 \title{An example \code{teal} module}
 \usage{
-example_module(label = "example teal module", filters = "all")
+example_module(label = "example teal module", datanames = "all")
 }
 \arguments{
 \item{label}{(\code{character(1)}) Label shown in the navigation item for the module. Any label possible except
 \code{"global_filters"} - read more in \code{mapping} argument of \link{teal_slices}.}
 
-\item{filters}{(\code{character}) A vector with \code{datanames} that are relevant for the item. The
+\item{datanames}{(\code{character}) A vector with \code{datanames} that are relevant for the item. The
 filter panel will automatically update the shown filters to include only
 filters in the listed datasets. \code{NULL} will hide the filter panel,
-and the keyword \code{'all'} will show the filters of all datasets. \code{filters} determines also
+and the keyword \code{'all'} will show filters of all datasets. \code{datanames} also determines
 a subset of datasets which are appended to the \code{data} argument in \code{server} function.}
 }
 \value{
diff --git a/man/init.Rd b/man/init.Rd
index 2c28801aed..234f60697f 100644
--- a/man/init.Rd
+++ b/man/init.Rd
@@ -83,7 +83,7 @@ app <- init(
       label = "data source",
       server = function(input, output, session, data) {},
       ui = function(id, ...) div(p("information about data source")),
-      filters = "all"
+      datanames = "all"
     ),
     example_module(label = "example teal module"),
     module(
@@ -97,7 +97,7 @@ app <- init(
         ns <- NS(id)
         plotOutput(ns("hist"))
       },
-      filters = "new_iris"
+      datanames = "new_iris"
     )
   ),
   title = "App title",
diff --git a/man/module.Rd b/man/module.Rd
index f091f28d49..d7267630cd 100644
--- a/man/module.Rd
+++ b/man/module.Rd
@@ -17,7 +17,8 @@ module(
      tags$p(paste0("This module has no UI (id: ", id, " )"))
 
     },
-  filters = "all",
+  filters,
+  datanames = "all",
   server_args = NULL,
   ui_args = NULL
 )
@@ -49,10 +50,12 @@ the \code{filters} argument.
 \item \code{...} (optional) \code{ui_args} elements will be passed to the module named argument or to the \code{...}.
 }}
 
-\item{filters}{(\code{character}) A vector with \code{datanames} that are relevant for the item. The
+\item{filters}{(\code{character}) Deprecated. Use \code{datanames} instead.}
+
+\item{datanames}{(\code{character}) A vector with \code{datanames} that are relevant for the item. The
 filter panel will automatically update the shown filters to include only
 filters in the listed datasets. \code{NULL} will hide the filter panel,
-and the keyword \code{'all'} will show the filters of all datasets. \code{filters} determines also
+and the keyword \code{'all'} will show filters of all datasets. \code{datanames} also determines
 a subset of datasets which are appended to the \code{data} argument in \code{server} function.}
 
 \item{server_args}{(named \code{list}) with additional arguments passed on to the
diff --git a/man/modules.Rd b/man/modules.Rd
index 9308058005..c86a83be06 100644
--- a/man/modules.Rd
+++ b/man/modules.Rd
@@ -65,7 +65,7 @@ app <- init(
           ns <- NS(id)
           tagList(dataTableOutput(ns("data")))
         },
-        filters = "all"
+        datanames = "all"
       )
     ),
     module(
@@ -82,7 +82,7 @@ app <- init(
         ns <- NS(id)
         tagList(textOutput(ns("text")))
       },
-      filters = NULL
+      datanames = NULL
     )
   )
 )
diff --git a/vignettes/adding-support-for-reporting.Rmd b/vignettes/adding-support-for-reporting.Rmd
index 551419e58c..d3131f1f5b 100644
--- a/vignettes/adding-support-for-reporting.Rmd
+++ b/vignettes/adding-support-for-reporting.Rmd
@@ -48,7 +48,7 @@ teal_example_module <- function(label = "example teal module") {
         encoding = selectInput(ns("dataname"), "Choose a dataset", choices = names(data))
       )
     },
-    filters = "all"
+    datanames = "all"
   )
 }
 ```
@@ -91,7 +91,7 @@ example_module_with_reporting <- function(label = "example teal module") {
         encoding = selectInput(ns("dataname"), "Choose a dataset", choices = names(data))
       )
     },
-    filters = "all"
+    datanames = "all"
   )
 }
 ```
@@ -145,7 +145,7 @@ example_module_with_reporting <- function(label = "example teal module") {
         encoding = selectInput(ns("dataname"), "Choose a dataset", choices = names(data))
       )
     },
-    filters = "all"
+    datanames = "all"
   )
 }
 ```
@@ -205,7 +205,7 @@ example_module_with_reporting <- function(label = "example teal module") {
         encoding = selectInput(ns("dataname"), "Choose a dataset", choices = names(data))
       )
     },
-    filters = "all"
+    datanames = "all"
   )
 }
 ```
@@ -328,7 +328,7 @@ example_reporter_module <- function(label = "Example") {
         )
       )
     },
-    filters = "all"
+    datanames = "all"
   )
 }
 
diff --git a/vignettes/creating-custom-modules.Rmd b/vignettes/creating-custom-modules.Rmd
index a5768e1c53..6d5e9fea93 100644
--- a/vignettes/creating-custom-modules.Rmd
+++ b/vignettes/creating-custom-modules.Rmd
@@ -35,7 +35,7 @@ example_module <- function(label = "example teal module") {
         encoding = selectInput(ns("dataname"), "Choose a dataset", choices = names(data))
       )
     },
-    filters = "all"
+    datanames = "all"
   )
 }
 ```
@@ -170,7 +170,7 @@ tm_histogram_example <- function(label, histogram_var) {
     ui = ui_histogram_example,
     ui_args = list(histogram_var = histogram_var),
     server_args = list(histogram_var = histogram_var),
-    filters = "all"
+    datanames = "all"
   )
 }
 ```
diff --git a/vignettes/filter-panel.Rmd b/vignettes/filter-panel.Rmd
index 345a2bed5c..6c20beba5f 100644
--- a/vignettes/filter-panel.Rmd
+++ b/vignettes/filter-panel.Rmd
@@ -57,9 +57,9 @@ app <- init(
   ),
   modules = modules(
     example_module(label = "all datasets"),
-    example_module(label = "IRIS only", filters = "IRIS"),
-    example_module(label = "CARS only", filters = "CARS"),
-    example_module(label = "no filter panel", filters = NULL)
+    example_module(label = "IRIS only", datanames = "IRIS"),
+    example_module(label = "CARS only", datanames = "CARS"),
+    example_module(label = "no filter panel", datanames = NULL)
   )
 )
 if (interactive()) {

From 7533f87e784e1ee89183275dabf0dfadee6ae390 Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 16:45:36 +0200
Subject: [PATCH 06/26] rename variable in snapshot manager module

---
 R/module_snapshot_manager.R | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/R/module_snapshot_manager.R b/R/module_snapshot_manager.R
index 32373b93f7..f70d4ec24e 100644
--- a/R/module_snapshot_manager.R
+++ b/R/module_snapshot_manager.R
@@ -152,13 +152,13 @@ snapshot_manager_srv <- function(id, slices_global, mapping_matrix, filtered_dat
       snapshot_state <- as.teal_slices(snapshot)
       mapping_unfolded <- unfold_mapping(attr(snapshot_state, "mapping"), names(filtered_data_list))
       mapply(
-        function(filtered_data, filters) {
+        function(filtered_data, filter_ids) {
           filtered_data$clear_filter_states(force = TRUE)
-          slices <- Filter(function(x) x$id %in% filters, snapshot_state)
+          slices <- Filter(function(x) x$id %in% filter_ids, snapshot_state)
           filtered_data$set_filter_state(slices)
         },
         filtered_data = filtered_data_list,
-        filters = mapping_unfolded
+        filter_ids = mapping_unfolded
       )
       slices_global(snapshot_state)
       removeModal()
@@ -186,13 +186,13 @@ snapshot_manager_srv <- function(id, slices_global, mapping_matrix, filtered_dat
             snapshot_state <- as.teal_slices(snapshot)
             mapping_unfolded <- unfold_mapping(attr(snapshot_state, "mapping"), names(filtered_data_list))
             mapply(
-              function(filtered_data, filters) {
+              function(filtered_data, filter_ids) {
                 filtered_data$clear_filter_states(force = TRUE)
-                slices <- Filter(function(x) x$id %in% filters, snapshot_state)
+                slices <- Filter(function(x) x$id %in% filter_ids, snapshot_state)
                 filtered_data$set_filter_state(slices)
               },
               filtered_data = filtered_data_list,
-              filters = mapping_unfolded
+              filter_ids = mapping_unfolded
             )
             slices_global(snapshot_state)
             removeModal()

From 779c326d4bb0237a4b768dd1dec2ea555e818451 Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 16:55:00 +0200
Subject: [PATCH 07/26] sapply to lapply

---
 R/init.R               |  2 +-
 R/module_nested_tabs.R | 39 +++++++++++++++------------------------
 R/module_teal.R        | 22 ++++++++++------------
 3 files changed, 26 insertions(+), 37 deletions(-)

diff --git a/R/init.R b/R/init.R
index ea98d7254e..a9a2d6c78d 100644
--- a/R/init.R
+++ b/R/init.R
@@ -141,7 +141,7 @@ init <- function(data,
   join_keys <- data$get_join_keys()
   resolve_modules_datanames <- function(modules) {
     if (inherits(modules, "teal_modules")) {
-      modules$children <- sapply(modules$children, resolve_modules_datanames, simplify = FALSE)
+      modules$children <- lapply(modules$children, resolve_modules_datanames)
       modules
     } else {
       modules$datanames <- if (identical(modules$datanames, "all")) {
diff --git a/R/module_nested_tabs.R b/R/module_nested_tabs.R
index 910f34c841..b4c5e62530 100644
--- a/R/module_nested_tabs.R
+++ b/R/module_nested_tabs.R
@@ -172,18 +172,15 @@ srv_nested_tabs.teal_modules <- function(id, datasets, modules, is_module_specif
     logger::log_trace("srv_nested_tabs.teal_modules initializing the module { deparse1(modules$label) }.")
 
     labels <- vapply(modules$children, `[[`, character(1), "label")
-    modules_reactive <- sapply(
-      names(modules$children),
-      function(module_id) {
-        srv_nested_tabs(
-          id = module_id,
-          datasets = datasets[[labels[module_id]]],
-          modules = modules$children[[module_id]],
-          is_module_specific = is_module_specific,
-          reporter = reporter
-        )
-      }
-    )
+    modules_reactive <- lapply(names(modules$children), function(module_id) {
+      srv_nested_tabs(
+        id = module_id,
+        datasets = datasets[[labels[module_id]]],
+        modules = modules$children[[module_id]],
+        is_module_specific = is_module_specific,
+        reporter = reporter
+      )
+    })
 
     # when not ready input$active_tab would return NULL - this would fail next reactive
     input_validated <- eventReactive(input$active_tab, input$active_tab, ignoreNULL = TRUE)
@@ -293,13 +290,10 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
 .datasets_to_data <- function(module, datasets, trigger_data = reactiveVal(1L)) {
   checkmate::assert_class(trigger_data, "reactiveVal")
   datanames <- if (is.null(module$datanames)) datasets$datanames() else module$datanames
+  datanames <- structure(datanames, names = datanames)
 
   # list of reactive filtered data
-  data <- sapply(
-    USE.NAMES = TRUE,
-    X = datanames,
-    FUN = function(x) eventReactive(trigger_data(), datasets$get_data(x, filtered = TRUE))
-  )
+  data <- lapply(datanames, function(x) eventReactive(trigger_data(), datasets$get_data(x, filtered = TRUE)))
 
   hashes <- calculate_hashes(datanames, datasets)
   metadata <- lapply(datanames, datasets$get_metadata)
@@ -330,11 +324,8 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
 #' @keywords internal
 #'
 calculate_hashes <- function(datanames, datasets) {
-  sapply(
-    datanames,
-    simplify = FALSE,
-    function(x) {
-      rlang::hash(datasets$get_data(x, filtered = FALSE))
-    }
-  )
+  if (is.null(names(datanames))) {
+    datanames <- structure(datanames, names = datanames)
+  }
+  lapply(datanames, function(x) rlang::hash(datasets$get_data(x, filtered = FALSE)))
 }
diff --git a/R/module_teal.R b/R/module_teal.R
index 13197c0212..45a316da02 100644
--- a/R/module_teal.R
+++ b/R/module_teal.R
@@ -182,18 +182,16 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
           # 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)) raw_data()$get_datanames() else modules$datanames
-          data_objects <- sapply(
-            datanames,
-            simplify = FALSE,
-            FUN = function(dataname) {
-              dataset <- raw_data()$get_dataset(dataname)
-              list(
-                dataset = dataset$get_raw_data(),
-                metadata = dataset$get_metadata(),
-                label = dataset$get_dataset_label()
-              )
-            }
-          )
+          datanames <- structure(datanames, names = datanames)
+
+          data_objects <- lapply(datanames, function(dataname) {
+            dataset <- raw_data()$get_dataset(dataname)
+            list(
+              dataset = dataset$get_raw_data(),
+              metadata = dataset$get_metadata(),
+              label = dataset$get_dataset_label()
+            )
+          })
           datasets_module <- teal.slice::init_filtered_data(
             data_objects,
             join_keys = raw_data()$get_join_keys(),

From 7fdf5ab340190e71fde4bf253a7c94139c200940 Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 16:55:24 +0200
Subject: [PATCH 08/26] propagate ardument change to unit tests

---
 tests/testthat/test-module_nested_tabs.R      | 24 ++++-----
 .../testthat/test-module_tabs_with_filters.R  |  4 +-
 tests/testthat/test-module_teal.R             |  4 +-
 tests/testthat/test-module_teal_with_splash.R |  2 +-
 tests/testthat/test-modules.R                 | 52 +++++++++----------
 5 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/tests/testthat/test-module_nested_tabs.R b/tests/testthat/test-module_nested_tabs.R
index fc8c560cd8..f297cd3c76 100644
--- a/tests/testthat/test-module_nested_tabs.R
+++ b/tests/testthat/test-module_nested_tabs.R
@@ -6,25 +6,25 @@ test_module1 <- module(
   label = "test1",
   ui = function(id, ...) NULL,
   server = function(id) moduleServer(id, function(input, output, session) message("1")),
-  filters = NULL
+  datanames = NULL
 )
 test_module2 <- module(
   label = "test2",
   ui = function(id) NULL,
   server = function(id) moduleServer(id, function(input, output, session) message("2")),
-  filters = NULL
+  datanames = NULL
 )
 test_module3 <- module(
   label = "test3",
   ui = function(id) NULL,
   server = function(id) moduleServer(id, function(input, output, session) message("3")),
-  filters = NULL
+  datanames = NULL
 )
 test_module4 <- module(
   label = "test4",
   ui = function(id) NULL,
   server = function(id) moduleServer(id, function(input, output, session) message("4")),
-  filters = NULL
+  datanames = NULL
 )
 
 testthat::test_that("srv_nested_tabs throws error if reporter is not inherited from class Reporter", {
@@ -188,7 +188,7 @@ testthat::test_that("srv_nested_tabs.teal_module does pass data if in the args e
     server = function(id, data, ...) {
       moduleServer(id, function(input, output, session) checkmate::assert_class(data, "tdata"))
     },
-    filters = NULL
+    datanames = NULL
   )
   testthat::expect_no_error(
     shiny::testServer(
@@ -207,7 +207,7 @@ testthat::test_that("srv_nested_tabs.teal_module does pass data if in the args e
 })
 
 testthat::test_that("srv_nested_tabs.teal_module passes data to the server module", {
-  module <- module(filters = NULL, server = function(id, data) {
+  module <- module(datanames = NULL, server = function(id, data) {
     moduleServer(id, function(input, output, session) checkmate::assert_list(data, "reactive"))
   })
 
@@ -268,7 +268,7 @@ testthat::test_that("srv_nested_tabs.teal_module passes server_args to the ...",
 })
 
 testthat::test_that("srv_nested_tabs.teal_module warns if both data and datasets are passed", {
-  module <- module(filters = NULL, label = "test module", server = function(id, datasets, data) {
+  module <- module(datanames = NULL, label = "test module", server = function(id, datasets, data) {
     moduleServer(id, function(input, output, session) NULL)
   })
 
@@ -386,7 +386,7 @@ testthat::test_that(".datasets_to_data accepts a reactiveVal as trigger_data inp
       teal.slice:::teal_slice(dataname = "d1", varname = "val", selected = c(1, 2))
     )
   )
-  module <- list(filter = c("d1", "d2"))
+  module <- list(datanames = c("d1", "d2"))
   trigger_data <- reactiveVal(1L)
   testthat::expect_silent(shiny::isolate(.datasets_to_data(module, datasets, trigger_data)))
 })
@@ -398,7 +398,7 @@ testthat::test_that(".datasets_to_data throws error if trigger_data is not a rea
       teal.slice:::teal_slice(dataname = "d1", varname = "val", selected = c(1, 2))
     )
   )
-  module <- list(filter = "all")
+  module <- list(datanames = "all")
   trigger_data <- 1
   testthat::expect_error(
     shiny::isolate(.datasets_to_data(module, datasets, trigger_data)),
@@ -413,7 +413,7 @@ testthat::test_that(".datasets_to_data returns data which is filtered", {
       teal.slice:::teal_slice(dataname = "d1", varname = "val", selected = c(1, 2))
     )
   )
-  module <- list(filter = c("d1", "d2"))
+  module <- list(datanames = c("d1", "d2"))
   trigger_data <- reactiveVal(1L)
   data <- shiny::isolate(.datasets_to_data(module, datasets, trigger_data))
 
@@ -426,7 +426,7 @@ testthat::test_that(".datasets_to_data returns data which is filtered", {
 
 testthat::test_that(".datasets_to_data returns only data requested by modules$filter", {
   datasets <- get_example_filtered_data()
-  module <- list(filter = "d1")
+  module <- list(datanames = "d1")
   trigger_data <- reactiveVal(1L)
   data <- .datasets_to_data(module, datasets, trigger_data)
   testthat::expect_equal(shiny::isolate(names(data)), "d1")
@@ -434,7 +434,7 @@ testthat::test_that(".datasets_to_data returns only data requested by modules$fi
 
 testthat::test_that(".datasets_to_data returns tdata object", {
   datasets <- get_example_filtered_data()
-  module <- list(filter = c("d1", "d2"))
+  module <- list(datanames = c("d1", "d2"))
   trigger_data <- reactiveVal(1L)
   data <- .datasets_to_data(module, datasets, trigger_data)
 
diff --git a/tests/testthat/test-module_tabs_with_filters.R b/tests/testthat/test-module_tabs_with_filters.R
index b65c0a56f8..74ccffb4bd 100644
--- a/tests/testthat/test-module_tabs_with_filters.R
+++ b/tests/testthat/test-module_tabs_with_filters.R
@@ -7,11 +7,11 @@ filtered_data <- teal.slice::init_filtered_data(
 
 test_module1 <- module(
   label = "iris tab",
-  filters = "iris"
+  datanames = "iris"
 )
 test_module2 <- module(
   label = "mtcars tab",
-  filters = "mtcars"
+  datanames = "mtcars"
 )
 
 testthat::test_that("srv_tabs_with_filters throws error if reporter is not of class Reporter", {
diff --git a/tests/testthat/test-module_teal.R b/tests/testthat/test-module_teal.R
index b6562d0f23..fddc04b2c7 100644
--- a/tests/testthat/test-module_teal.R
+++ b/tests/testthat/test-module_teal.R
@@ -4,11 +4,11 @@ data <- teal_data(iris_ds, mtcars_ds)
 
 test_module1 <- module(
   label = "iris_tab",
-  filters = "iris"
+  datanames = "iris"
 )
 test_module2 <- module(
   label = "mtcars_tab",
-  filters = "mtcars"
+  datanames = "mtcars"
 )
 
 testthat::test_that("srv_teal fails when raw_data is not reactive", {
diff --git a/tests/testthat/test-module_teal_with_splash.R b/tests/testthat/test-module_teal_with_splash.R
index e77e889b12..f96d13633a 100644
--- a/tests/testthat/test-module_teal_with_splash.R
+++ b/tests/testthat/test-module_teal_with_splash.R
@@ -4,7 +4,7 @@ data <- teal_data(iris_ds, mtcars_ds)
 
 test_module1 <- module(
   label = "iris_tab",
-  filters = "iris"
+  datanames = "iris"
 )
 
 testthat::test_that("srv_teal_with_splash creates reactiveVal returning data input", {
diff --git a/tests/testthat/test-modules.R b/tests/testthat/test-modules.R
index 88b9b9f8fa..96646b954f 100644
--- a/tests/testthat/test-modules.R
+++ b/tests/testthat/test-modules.R
@@ -20,7 +20,7 @@ ui_fun2 <- function(id, datasets) {
 }
 
 testthat::test_that("Calling module() does not throw", {
-  testthat::expect_error(module(), NA)
+  testthat::expect_no_error(suppressMessages(module()))
 })
 
 testthat::test_that("module requires label argument to be a string different than 'global_filters'", {
@@ -120,12 +120,12 @@ testthat::test_that("module expects all ui_args being a ui arguments or passed t
   )
 })
 
-testthat::test_that("module requires filters argument to be a character or NULL", {
-  testthat::expect_error(module(filters = "all"), NA)
-  testthat::expect_error(module(filters = ""), NA)
-  testthat::expect_error(module(filters = NULL), NA)
-  testthat::expect_error(module(filters = NA_character_), "Contains missing values")
-  testthat::expect_error(module(server = function(id, data) NULL, filters = NULL), NA)
+testthat::test_that("module requires datanames argument to be a character or NULL", {
+  testthat::expect_error(module(datanames = "all"), NA)
+  testthat::expect_error(module(datanames = ""), NA)
+  testthat::expect_error(module(datanames = NULL), NA)
+  testthat::expect_error(module(datanames = NA_character_), "Contains missing values")
+  testthat::expect_error(module(server = function(id, data) NULL, datanames = NULL), NA)
 })
 
 testthat::test_that("module() returns list of class 'teal_module' containing input objects", {
@@ -133,16 +133,16 @@ testthat::test_that("module() returns list of class 'teal_module' containing inp
     label = "aaa1",
     server = call_module_server_fun,
     ui = ui_fun1,
-    filters = "all",
+    datanames = "all",
     server_args = NULL,
     ui_args = NULL
   )
   testthat::expect_s3_class(test_module, "teal_module")
-  testthat::expect_named(test_module, c("label", "server", "ui", "filters", "server_args", "ui_args"))
+  testthat::expect_named(test_module, c("label", "server", "ui", "datanames", "server_args", "ui_args"))
   testthat::expect_identical(test_module$label, "aaa1")
   testthat::expect_identical(test_module$server, call_module_server_fun)
   testthat::expect_identical(test_module$ui, ui_fun1)
-  testthat::expect_identical(test_module$filters, "all")
+  testthat::expect_identical(test_module$datanames, NULL)
   testthat::expect_identical(test_module$server_args, NULL)
   testthat::expect_identical(test_module$ui_args, NULL)
 })
@@ -157,7 +157,7 @@ testthat::test_that("modules requires label argument to be a string ", {
     label = "label",
     server = module_server_fun,
     ui = ui_fun1,
-    filters = ""
+    datanames = ""
   )
 
   testthat::expect_error(modules(label = "label", test_module), NA)
@@ -173,7 +173,7 @@ testthat::test_that("modules accept teal_module in ...", {
     label = "label",
     server = module_server_fun,
     ui = ui_fun1,
-    filters = ""
+    datanames = ""
   )
 
   testthat::expect_error(modules(label = "label", test_module), NA)
@@ -184,7 +184,7 @@ testthat::test_that("modules accept multiple teal_module objects in ...", {
     label = "label",
     server = module_server_fun,
     ui = ui_fun1,
-    filters = ""
+    datanames = ""
   )
 
   testthat::expect_error(modules(label = "label", test_module, test_module), NA)
@@ -195,7 +195,7 @@ testthat::test_that("modules accept multiple teal_module and teal_modules object
     label = "label",
     server = module_server_fun,
     ui = ui_fun1,
-    filters = ""
+    datanames = ""
   )
   test_modules <- modules(label = "label", test_module)
 
@@ -221,7 +221,7 @@ testthat::test_that("modules returns teal_modules object with label and children
     label = "label",
     server = module_server_fun,
     ui = ui_fun1,
-    filters = ""
+    datanames = ""
   )
   out <- modules(label = "label2", test_module)
   testthat::expect_s3_class(out, "teal_modules")
@@ -233,7 +233,7 @@ testthat::test_that("modules returns children as list with list named after labe
     label = "module",
     server = module_server_fun,
     ui = ui_fun1,
-    filters = ""
+    datanames = ""
   )
   test_modules <- modules(label = "modules", test_module)
   out <- modules(label = "tabs", test_module, test_modules)$children
@@ -248,7 +248,7 @@ testthat::test_that("modules returns useful error message if label argument not
     label = "module",
     server = module_server_fun,
     ui = ui_fun1,
-    filters = ""
+    datanames = ""
   )
   testthat::expect_error(modules("module", test_module), "The only character argument to modules\\(\\) must be 'label'")
 })
@@ -259,7 +259,7 @@ testthat::test_that("modules returns children as list with unique names if label
     label = "module",
     server = module_server_fun,
     ui = ui_fun1,
-    filters = ""
+    datanames = ""
   )
   test_modules <- modules(label = "module", test_module)
   out <- modules(label = "tabs", test_module, test_modules)$children
@@ -276,7 +276,7 @@ testthat::test_that("modules_depth accepts depth as integer", {
         label = "label",
         server = module_server_fun,
         ui = ui_fun1,
-        filters = ""
+        datanames = ""
       ),
       depth = 3L
     ),
@@ -289,7 +289,7 @@ testthat::test_that("modules_depth accepts depth as integer", {
         label = "label",
         server = module_server_fun,
         ui = ui_fun1,
-        filters = ""
+        datanames = ""
       ),
       depth = "1"
     ),
@@ -304,7 +304,7 @@ testthat::test_that("modules_depth returns depth=0 by default", {
         label = "label",
         server = module_server_fun,
         ui = ui_fun1,
-        filters = ""
+        datanames = ""
       )
     ),
     0L
@@ -318,7 +318,7 @@ testthat::test_that("modules_depth accepts modules to be teal_module or teal_mod
         label = "label",
         server = module_server_fun,
         ui = ui_fun1,
-        filters = ""
+        datanames = ""
       )
     ),
     NA
@@ -331,7 +331,7 @@ testthat::test_that("modules_depth accepts modules to be teal_module or teal_mod
           label = "label",
           server = module_server_fun,
           ui = ui_fun1,
-          filters = ""
+          datanames = ""
         )
       )
     ),
@@ -346,7 +346,7 @@ testthat::test_that("modules_depth returns depth same as input for teal_module",
         label = "label",
         server = module_server_fun,
         ui = ui_fun1,
-        filters = ""
+        datanames = ""
       )
     ),
     0L
@@ -362,7 +362,7 @@ testthat::test_that("modules_depth increases depth by 1 for each teal_modules",
           label = "label",
           server = module_server_fun,
           ui = ui_fun1,
-          filters = ""
+          datanames = ""
         )
       ),
       depth = 1L
@@ -380,7 +380,7 @@ testthat::test_that("modules_depth increases depth by 1 for each teal_modules",
             label = "label",
             server = module_server_fun,
             ui = ui_fun1,
-            filters = ""
+            datanames = ""
           )
         )
       ),

From 73d0e43513d11f264f7f9f072324cf801253156c Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 18:01:35 +0200
Subject: [PATCH 09/26] bug fix

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

diff --git a/R/module_nested_tabs.R b/R/module_nested_tabs.R
index b4c5e62530..5027386474 100644
--- a/R/module_nested_tabs.R
+++ b/R/module_nested_tabs.R
@@ -181,6 +181,7 @@ srv_nested_tabs.teal_modules <- function(id, datasets, modules, is_module_specif
         reporter = reporter
       )
     })
+    names(modules_reactive) <- names(modules$children)
 
     # when not ready input$active_tab would return NULL - this would fail next reactive
     input_validated <- eventReactive(input$active_tab, input$active_tab, ignoreNULL = TRUE)
@@ -205,6 +206,7 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
                                         reporter = teal.reporter::Reporter$new()) {
   checkmate::assert_class(datasets, class = "FilteredData")
   logger::log_trace("srv_nested_tabs.teal_module initializing the module: { deparse1(modules$label) }.")
+
   moduleServer(id = id, module = function(input, output, session) {
     modules$server_args <- teal.transform::resolve_delayed(modules$server_args, datasets)
     if (!is.null(modules$datanames) && is_module_specific) {
@@ -289,6 +291,7 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
 #' @keywords internal
 .datasets_to_data <- function(module, datasets, trigger_data = reactiveVal(1L)) {
   checkmate::assert_class(trigger_data, "reactiveVal")
+
   datanames <- if (is.null(module$datanames)) datasets$datanames() else module$datanames
   datanames <- structure(datanames, names = datanames)
 

From ae1f830362666685e104d62b9a78709e07753908 Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 18:01:57 +0200
Subject: [PATCH 10/26] adjust unit tests to ignoring datanames argument when
 no data used

---
 tests/testthat/test-module_tabs_with_filters.R | 8 ++++++++
 tests/testthat/test-modules.R                  | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tests/testthat/test-module_tabs_with_filters.R b/tests/testthat/test-module_tabs_with_filters.R
index 74ccffb4bd..a82042cc81 100644
--- a/tests/testthat/test-module_tabs_with_filters.R
+++ b/tests/testthat/test-module_tabs_with_filters.R
@@ -7,10 +7,18 @@ filtered_data <- teal.slice::init_filtered_data(
 
 test_module1 <- module(
   label = "iris tab",
+  server = function(id, data, ...) {
+    moduleServer(id, function(input, output, session) {
+    })
+  },
   datanames = "iris"
 )
 test_module2 <- module(
   label = "mtcars tab",
+  server = function(id, data, ...) {
+    moduleServer(id, function(input, output, session) {
+    })
+  },
   datanames = "mtcars"
 )
 
diff --git a/tests/testthat/test-modules.R b/tests/testthat/test-modules.R
index 96646b954f..f4d72f4500 100644
--- a/tests/testthat/test-modules.R
+++ b/tests/testthat/test-modules.R
@@ -5,7 +5,7 @@ adsl_dataset <- teal.data::cdisc_dataset(
   parent = character(0), keys = teal.data::get_cdisc_keys("ADSL")
 )
 
-call_module_server_fun <- function(input, output, session, datasets) {
+call_module_server_fun <- function(input, output, session, data, datasets) {
 }
 
 module_server_fun <- function(id, datasets) {
@@ -142,7 +142,7 @@ testthat::test_that("module() returns list of class 'teal_module' containing inp
   testthat::expect_identical(test_module$label, "aaa1")
   testthat::expect_identical(test_module$server, call_module_server_fun)
   testthat::expect_identical(test_module$ui, ui_fun1)
-  testthat::expect_identical(test_module$datanames, NULL)
+  testthat::expect_identical(test_module$datanames, "all")
   testthat::expect_identical(test_module$server_args, NULL)
   testthat::expect_identical(test_module$ui_args, NULL)
 })

From 4e0a7f8343786604dbcacd7f5f36037dc4446eea Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 18:12:19 +0200
Subject: [PATCH 11/26] use expect_no_error

---
 tests/testthat/test-modules.R | 87 ++++++++++++++---------------------
 1 file changed, 34 insertions(+), 53 deletions(-)

diff --git a/tests/testthat/test-modules.R b/tests/testthat/test-modules.R
index f4d72f4500..dc2fd11c49 100644
--- a/tests/testthat/test-modules.R
+++ b/tests/testthat/test-modules.R
@@ -24,7 +24,7 @@ testthat::test_that("Calling module() does not throw", {
 })
 
 testthat::test_that("module requires label argument to be a string different than 'global_filters'", {
-  testthat::expect_error(module(label = "label"), NA)
+  testthat::expect_no_error(module(label = "label"))
 
   testthat::expect_error(module(label = NULL), "Assertion on 'label' failed.+'NULL'")
 
@@ -36,17 +36,11 @@ testthat::test_that("module requires label argument to be a string different tha
 })
 
 testthat::test_that("module expects server being a shiny server module with any argument", {
-  testthat::expect_error(module(
-    server = function(id) NULL
-  ), NA)
+  testthat::expect_no_error(module(server = function(id) NULL))
 
-  testthat::expect_error(module(
-    server = function(id, any_argument) NULL,
-  ), NA)
+  testthat::expect_no_error(module(server = function(id, any_argument) NULL))
 
-  testthat::expect_error(module(
-    server = function(input, output, session, any_argument) NULL,
-  ), NA)
+  testthat::expect_no_error(module(server = function(input, output, session, any_argument) NULL))
 
 
   testthat::expect_error(
@@ -61,23 +55,17 @@ testthat::test_that("module expects server being a shiny server module with any
 })
 
 testthat::test_that("module requires server_args argument to be a list", {
-  testthat::expect_error(module(server = function(id, a) NULL, server_args = list(a = 1)), NA)
-  testthat::expect_error(module(server_args = list()), NA)
-  testthat::expect_error(module(server_args = NULL), NA)
+  testthat::expect_no_error(module(server = function(id, a) NULL, server_args = list(a = 1)))
+  testthat::expect_no_error(module(server_args = list()))
+  testthat::expect_no_error(module(server_args = NULL))
   testthat::expect_error(module(server_args = ""), "Assertion on 'server_args' failed.+'list'")
   testthat::expect_error(module(server_args = list(1, 2, 3)), "Must have names")
 })
 
 testthat::test_that("module expects all server_args being a server arguments or passed through `...`", {
-  testthat::expect_error(module(
-    server = function(id, arg1) NULL,
-    server_args = list(arg1 = NULL)
-  ), NA)
+  testthat::expect_no_error(module(server = function(id, arg1) NULL, server_args = list(arg1 = NULL)))
 
-  testthat::expect_error(module(
-    server = function(id, ...) NULL,
-    server_args = list(arg1 = NULL)
-  ), NA)
+  testthat::expect_no_error(module(server = function(id, ...) NULL, server_args = list(arg1 = NULL)))
 
   testthat::expect_error(
     module(server = function(id) NULL, server_args = list(arg1 = NULL)),
@@ -86,16 +74,16 @@ testthat::test_that("module expects all server_args being a server arguments or
 })
 
 testthat::test_that("module requires ui_args argument to be a list", {
-  testthat::expect_error(module(ui = function(id, a) NULL, ui_args = list(a = 1)), NA)
-  testthat::expect_error(module(ui_args = list()), NA)
-  testthat::expect_error(module(ui_args = NULL), NA)
+  testthat::expect_no_error(module(ui = function(id, a) NULL, ui_args = list(a = 1)))
+  testthat::expect_no_error(module(ui_args = list()))
+  testthat::expect_no_error(module(ui_args = NULL))
   testthat::expect_error(module(ui_args = ""), "Assertion on 'ui_args' failed.+'list'")
   testthat::expect_error(module(ui_args = list(1, 2, 3)), "Must have names")
 })
 
 testthat::test_that("module expects ui being a shiny ui module with any argument", {
-  testthat::expect_error(module(ui = function(id) NULL), NA)
-  testthat::expect_error(module(ui = function(id, any_argument) NULL), NA)
+  testthat::expect_no_error(module(ui = function(id) NULL))
+  testthat::expect_no_error(module(ui = function(id, any_argument) NULL))
   testthat::expect_error(
     module(ui = function(any_argument) NULL),
     "`ui` argument requires a function with following arguments"
@@ -103,16 +91,9 @@ testthat::test_that("module expects ui being a shiny ui module with any argument
 })
 
 testthat::test_that("module expects all ui_args being a ui arguments or passed through `...`", {
-  testthat::expect_error(module(
-    ui = function(id, arg1) NULL,
-    ui_args = list(arg1 = NULL)
-  ), NA)
-
-  testthat::expect_error(module(
-    ui = function(id, ...) NULL,
-    ui_args = list(arg1 = NULL)
-  ), NA)
+  testthat::expect_no_error(module(ui = function(id, arg1) NULL, ui_args = list(arg1 = NULL)))
 
+  testthat::expect_no_error(module(ui = function(id, ...) NULL, ui_args = list(arg1 = NULL)))
 
   testthat::expect_error(
     module(ui = function(id) NULL, ui_args = list(arg1 = NULL)),
@@ -121,11 +102,11 @@ testthat::test_that("module expects all ui_args being a ui arguments or passed t
 })
 
 testthat::test_that("module requires datanames argument to be a character or NULL", {
-  testthat::expect_error(module(datanames = "all"), NA)
-  testthat::expect_error(module(datanames = ""), NA)
-  testthat::expect_error(module(datanames = NULL), NA)
+  testthat::expect_no_error(module(datanames = "all"))
+  testthat::expect_no_error(module(datanames = ""))
+  testthat::expect_no_error(module(datanames = NULL))
   testthat::expect_error(module(datanames = NA_character_), "Contains missing values")
-  testthat::expect_error(module(server = function(id, data) NULL, datanames = NULL), NA)
+  testthat::expect_no_error(module(server = function(id, data) NULL, datanames = NULL))
 })
 
 testthat::test_that("module() returns list of class 'teal_module' containing input objects", {
@@ -160,7 +141,7 @@ testthat::test_that("modules requires label argument to be a string ", {
     datanames = ""
   )
 
-  testthat::expect_error(modules(label = "label", test_module), NA)
+  testthat::expect_no_error(modules(label = "label", test_module))
   testthat::expect_error(modules(label = NULL, test_module), "Assertion on 'label' failed.+'NULL'")
   testthat::expect_error(
     modules(label = c("label", "label"), test_module),
@@ -176,7 +157,7 @@ testthat::test_that("modules accept teal_module in ...", {
     datanames = ""
   )
 
-  testthat::expect_error(modules(label = "label", test_module), NA)
+  testthat::expect_no_error(modules(label = "label", test_module))
 })
 
 testthat::test_that("modules accept multiple teal_module objects in ...", {
@@ -187,7 +168,7 @@ testthat::test_that("modules accept multiple teal_module objects in ...", {
     datanames = ""
   )
 
-  testthat::expect_error(modules(label = "label", test_module, test_module), NA)
+  testthat::expect_no_error(modules(label = "label", test_module, test_module))
 })
 
 testthat::test_that("modules accept multiple teal_module and teal_modules objects in ...", {
@@ -199,7 +180,7 @@ testthat::test_that("modules accept multiple teal_module and teal_modules object
   )
   test_modules <- modules(label = "label", test_module)
 
-  testthat::expect_error(modules(label = "label", test_module, test_modules), NA)
+  testthat::expect_no_error(modules(label = "label", test_module, test_modules))
 })
 
 testthat::test_that("modules does not accept objects other than teal_module(s) in ...", {
@@ -250,7 +231,10 @@ testthat::test_that("modules returns useful error message if label argument not
     ui = ui_fun1,
     datanames = ""
   )
-  testthat::expect_error(modules("module", test_module), "The only character argument to modules\\(\\) must be 'label'")
+  testthat::expect_error(
+    modules("module", test_module),
+    "The only character argument to modules\\(\\) must be 'label'"
+  )
 })
 
 
@@ -270,7 +254,7 @@ testthat::test_that("modules returns children as list with unique names if label
 
 
 testthat::test_that("modules_depth accepts depth as integer", {
-  testthat::expect_error(
+  testthat::expect_no_error(
     modules_depth(
       module(
         label = "label",
@@ -279,8 +263,7 @@ testthat::test_that("modules_depth accepts depth as integer", {
         datanames = ""
       ),
       depth = 3L
-    ),
-    NA
+    )
   )
 
   testthat::expect_error(
@@ -312,7 +295,7 @@ testthat::test_that("modules_depth returns depth=0 by default", {
 })
 
 testthat::test_that("modules_depth accepts modules to be teal_module or teal_modules", {
-  testthat::expect_error(
+  testthat::expect_no_error(
     modules_depth(
       module(
         label = "label",
@@ -320,10 +303,9 @@ testthat::test_that("modules_depth accepts modules to be teal_module or teal_mod
         ui = ui_fun1,
         datanames = ""
       )
-    ),
-    NA
+    )
   )
-  testthat::expect_error(
+  testthat::expect_no_error(
     modules_depth(
       modules(
         label = "tabs",
@@ -334,8 +316,7 @@ testthat::test_that("modules_depth accepts modules to be teal_module or teal_mod
           datanames = ""
         )
       )
-    ),
-    NA
+    )
   )
 })
 

From 0558002eb0ead4332dc244f5be2c58e29d5f3a1d Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Wed, 26 Jul 2023 18:22:14 +0200
Subject: [PATCH 12/26] by the way validation fix

---
 R/validations.R | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/R/validations.R b/R/validations.R
index 117eebb639..81e13554eb 100644
--- a/R/validations.R
+++ b/R/validations.R
@@ -284,16 +284,14 @@ validate_no_intersection <- function(x, y, msg) {
 #' }
 validate_has_variable <- function(data, varname, msg) {
   if (length(varname) != 0) {
-    has_vars <- all(varname %in% names(data))
-    has_all <- all(has_vars)
+    has_vars <- varname %in% names(data)
 
-    if (!has_all) {
+    if (!all(has_vars)) {
       if (missing(msg)) {
-        dataname <- deparse(substitute(data))
-        msg <- paste(
-          dataname, "does not have the required variables:",
-          paste(varname[!has_vars], collapse = ", "),
-          "."
+        msg <- sprintf(
+          "%s does not have the required variables: %s.",
+          deparse(substitute(data)),
+          toString(varname[!has_vars])
         )
       }
       validate(need(FALSE, msg))

From 4ba62ab8335ab573f2bead3ca26d6c9608bc539f Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Thu, 27 Jul 2023 10:23:58 +0200
Subject: [PATCH 13/26] improve argument checks and unit tests

---
 R/module_nested_tabs.R                   |  9 +++-
 tests/testthat/test-module_nested_tabs.R | 57 ++++++++++++++----------
 2 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/R/module_nested_tabs.R b/R/module_nested_tabs.R
index 5027386474..8282d5b34e 100644
--- a/R/module_nested_tabs.R
+++ b/R/module_nested_tabs.R
@@ -48,7 +48,8 @@
 #' }
 #' @keywords internal
 ui_nested_tabs <- function(id, modules, datasets, depth = 0L, is_module_specific = FALSE) {
-  checkmate::assert_int(depth)
+  checkmate::assert_multi_class(modules, c("teal_modules", "teal_module"))
+  checkmate::assert_count(depth)
   UseMethod("ui_nested_tabs", modules)
 }
 
@@ -150,6 +151,7 @@ ui_nested_tabs.teal_module <- function(id, modules, datasets, depth = 0L, is_mod
 #' @keywords internal
 srv_nested_tabs <- function(id, datasets, modules, is_module_specific = FALSE,
                             reporter = teal.reporter::Reporter$new()) {
+  checkmate::assert_multi_class(modules, c("teal_modules", "teal_module"))
   checkmate::assert_class(reporter, "Reporter")
   UseMethod("srv_nested_tabs", modules)
 }
@@ -168,6 +170,7 @@ srv_nested_tabs.default <- function(id, datasets, modules, is_module_specific =
 srv_nested_tabs.teal_modules <- function(id, datasets, modules, is_module_specific = FALSE,
                                          reporter = teal.reporter::Reporter$new()) {
   checkmate::assert_list(datasets, types = c("list", "FilteredData"))
+
   moduleServer(id = id, module = function(input, output, session) {
     logger::log_trace("srv_nested_tabs.teal_modules initializing the module { deparse1(modules$label) }.")
 
@@ -204,7 +207,7 @@ srv_nested_tabs.teal_modules <- function(id, datasets, modules, is_module_specif
 #' @keywords internal
 srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specific = TRUE,
                                         reporter = teal.reporter::Reporter$new()) {
-  checkmate::assert_class(datasets, class = "FilteredData")
+  checkmate::assert_class(datasets, "FilteredData")
   logger::log_trace("srv_nested_tabs.teal_module initializing the module: { deparse1(modules$label) }.")
 
   moduleServer(id = id, module = function(input, output, session) {
@@ -290,6 +293,8 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
 #'
 #' @keywords internal
 .datasets_to_data <- function(module, datasets, trigger_data = reactiveVal(1L)) {
+  checkmate::assert_class(module, "teal_module")
+  checkmate::assert_class(datasets, "FilteredData")
   checkmate::assert_class(trigger_data, "reactiveVal")
 
   datanames <- if (is.null(module$datanames)) datasets$datanames() else module$datanames
diff --git a/tests/testthat/test-module_nested_tabs.R b/tests/testthat/test-module_nested_tabs.R
index f297cd3c76..9494acad4a 100644
--- a/tests/testthat/test-module_nested_tabs.R
+++ b/tests/testthat/test-module_nested_tabs.R
@@ -26,6 +26,34 @@ test_module4 <- module(
   server = function(id) moduleServer(id, function(input, output, session) message("4")),
   datanames = NULL
 )
+test_module_wdata <- function(datanames) {
+  module(
+    label = "with_data",
+    ui = function(id) NULL,
+    server = function(id, data) moduleServer(id, function(input, output, session) message("module with data")),
+    datanames = datanames
+  )
+}
+
+get_example_filtered_data <- function() {
+  d1 <- data.frame(id = 1:5, pk = c(2, 3, 2, 1, 4), val = 1:5)
+  d2 <- data.frame(id = 1:5, value = 1:5)
+
+  cc <- teal.data:::CodeClass$new()
+  cc$set_code("d1 <- data.frame(id = 1:5, pk = c(2,3,2,1,4), val = 1:5)", "d1")
+  cc$set_code("d2 <- data.frame(id = 1:5, value = 1:5)", "d2")
+
+  teal.slice::init_filtered_data(
+    x = list(
+      d1 = list(dataset = d1, metadata = list("A" = 1)),
+      d2 = list(dataset = d2)
+    ),
+    join_keys = teal.data::join_keys(teal.data::join_key("d1", "d2", c("pk" = "id"))),
+    code = cc,
+    check = TRUE
+  )
+}
+
 
 testthat::test_that("srv_nested_tabs throws error if reporter is not inherited from class Reporter", {
   testthat::expect_error(
@@ -360,25 +388,6 @@ testthat::test_that("srv_nested_tabs.teal_module passes filter_panel_api to the
 })
 
 
-get_example_filtered_data <- function() {
-  d1 <- data.frame(id = 1:5, pk = c(2, 3, 2, 1, 4), val = 1:5)
-  d2 <- data.frame(id = 1:5, value = 1:5)
-
-  cc <- teal.data:::CodeClass$new()
-  cc$set_code("d1 <- data.frame(id = 1:5, pk = c(2,3,2,1,4), val = 1:5)", "d1")
-  cc$set_code("d2 <- data.frame(id = 1:5, value = 1:5)", "d2")
-
-  teal.slice::init_filtered_data(
-    x = list(
-      d1 = list(dataset = d1, metadata = list("A" = 1)),
-      d2 = list(dataset = d2)
-    ),
-    join_keys = teal.data::join_keys(teal.data::join_key("d1", "d2", c("pk" = "id"))),
-    code = cc,
-    check = TRUE
-  )
-}
-
 testthat::test_that(".datasets_to_data accepts a reactiveVal as trigger_data input", {
   datasets <- get_example_filtered_data()
   datasets$set_filter_state(
@@ -386,7 +395,7 @@ testthat::test_that(".datasets_to_data accepts a reactiveVal as trigger_data inp
       teal.slice:::teal_slice(dataname = "d1", varname = "val", selected = c(1, 2))
     )
   )
-  module <- list(datanames = c("d1", "d2"))
+  module <- test_module_wdata(datanames = c("d1", "d2"))
   trigger_data <- reactiveVal(1L)
   testthat::expect_silent(shiny::isolate(.datasets_to_data(module, datasets, trigger_data)))
 })
@@ -398,7 +407,7 @@ testthat::test_that(".datasets_to_data throws error if trigger_data is not a rea
       teal.slice:::teal_slice(dataname = "d1", varname = "val", selected = c(1, 2))
     )
   )
-  module <- list(datanames = "all")
+  module <- test_module_wdata(datanames = "all")
   trigger_data <- 1
   testthat::expect_error(
     shiny::isolate(.datasets_to_data(module, datasets, trigger_data)),
@@ -413,7 +422,7 @@ testthat::test_that(".datasets_to_data returns data which is filtered", {
       teal.slice:::teal_slice(dataname = "d1", varname = "val", selected = c(1, 2))
     )
   )
-  module <- list(datanames = c("d1", "d2"))
+  module <- test_module_wdata(datanames = c("d1", "d2"))
   trigger_data <- reactiveVal(1L)
   data <- shiny::isolate(.datasets_to_data(module, datasets, trigger_data))
 
@@ -426,7 +435,7 @@ testthat::test_that(".datasets_to_data returns data which is filtered", {
 
 testthat::test_that(".datasets_to_data returns only data requested by modules$filter", {
   datasets <- get_example_filtered_data()
-  module <- list(datanames = "d1")
+  module <- test_module_wdata(datanames = "d1")
   trigger_data <- reactiveVal(1L)
   data <- .datasets_to_data(module, datasets, trigger_data)
   testthat::expect_equal(shiny::isolate(names(data)), "d1")
@@ -434,7 +443,7 @@ testthat::test_that(".datasets_to_data returns only data requested by modules$fi
 
 testthat::test_that(".datasets_to_data returns tdata object", {
   datasets <- get_example_filtered_data()
-  module <- list(datanames = c("d1", "d2"))
+  module <- test_module_wdata(datanames = c("d1", "d2"))
   trigger_data <- reactiveVal(1L)
   data <- .datasets_to_data(module, datasets, trigger_data)
 

From 3e4762de81bbb2341a95c6c72bb25f95e879353a Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Fri, 28 Jul 2023 12:40:40 +0200
Subject: [PATCH 14/26] roll back sapply changes

---
 R/init.R               |  2 +-
 R/module_nested_tabs.R | 35 +++++++++++++++++++----------------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/R/init.R b/R/init.R
index a9a2d6c78d..ea98d7254e 100644
--- a/R/init.R
+++ b/R/init.R
@@ -141,7 +141,7 @@ init <- function(data,
   join_keys <- data$get_join_keys()
   resolve_modules_datanames <- function(modules) {
     if (inherits(modules, "teal_modules")) {
-      modules$children <- lapply(modules$children, resolve_modules_datanames)
+      modules$children <- sapply(modules$children, resolve_modules_datanames, simplify = FALSE)
       modules
     } else {
       modules$datanames <- if (identical(modules$datanames, "all")) {
diff --git a/R/module_nested_tabs.R b/R/module_nested_tabs.R
index 367e778344..443587b07e 100644
--- a/R/module_nested_tabs.R
+++ b/R/module_nested_tabs.R
@@ -173,16 +173,19 @@ srv_nested_tabs.teal_modules <- function(id, datasets, modules, is_module_specif
     logger::log_trace("srv_nested_tabs.teal_modules initializing the module { deparse1(modules$label) }.")
 
     labels <- vapply(modules$children, `[[`, character(1), "label")
-    modules_reactive <- lapply(names(modules$children), function(module_id) {
-      srv_nested_tabs(
-        id = module_id,
-        datasets = datasets[[labels[module_id]]],
-        modules = modules$children[[module_id]],
-        is_module_specific = is_module_specific,
-        reporter = reporter
-      )
-    })
-    names(modules_reactive) <- names(modules$children)
+    modules_reactive <- sapply(
+      names(modules$children),
+      function(module_id) {
+        srv_nested_tabs(
+          id = module_id,
+          datasets = datasets[[labels[module_id]]],
+          modules = modules$children[[module_id]],
+          is_module_specific = is_module_specific,
+          reporter = reporter
+        )
+      },
+      simplify = FALSE
+    )
 
     # when not ready input$active_tab would return NULL - this would fail next reactive
     input_validated <- eventReactive(input$active_tab, input$active_tab, ignoreNULL = TRUE)
@@ -295,10 +298,13 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
   checkmate::assert_class(trigger_data, "reactiveVal")
 
   datanames <- if (is.null(module$datanames)) datasets$datanames() else module$datanames
-  datanames <- structure(datanames, names = datanames)
 
   # list of reactive filtered data
-  data <- lapply(datanames, function(x) eventReactive(trigger_data(), datasets$get_data(x, filtered = TRUE)))
+  data <- sapply(
+    datanames,
+    function(x) eventReactive(trigger_data(), datasets$get_data(x, filtered = TRUE)),
+    simplify = FALSE
+  )
 
   hashes <- calculate_hashes(datanames, datasets)
   metadata <- lapply(datanames, datasets$get_metadata)
@@ -329,8 +335,5 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
 #' @keywords internal
 #'
 calculate_hashes <- function(datanames, datasets) {
-  if (is.null(names(datanames))) {
-    datanames <- structure(datanames, names = datanames)
-  }
-  lapply(datanames, function(x) rlang::hash(datasets$get_data(x, filtered = FALSE)))
+  sapply(datanames, function(x) rlang::hash(datasets$get_data(x, filtered = FALSE)), simplify = FALSE)
 }

From 1b88a252a8c9a1c8ad01ab5dcb88ee0f0686b85d Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Fri, 28 Jul 2023 12:43:37 +0200
Subject: [PATCH 15/26] roll back more sapply changes

---
 R/module_teal.R | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/R/module_teal.R b/R/module_teal.R
index 0b2d786600..481b1b727e 100644
--- a/R/module_teal.R
+++ b/R/module_teal.R
@@ -185,16 +185,17 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
           # 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)) raw_data()$get_datanames() else modules$datanames
-          datanames <- structure(datanames, names = datanames)
-
-          data_objects <- lapply(datanames, function(dataname) {
-            dataset <- raw_data()$get_dataset(dataname)
-            list(
-              dataset = dataset$get_raw_data(),
-              metadata = dataset$get_metadata(),
-              label = dataset$get_dataset_label()
-            )
-          })
+          data_objects <- sapply(
+            datanames,
+            function(dataname) {
+              dataset <- raw_data()$get_dataset(dataname)
+              list(
+                dataset = dataset$get_raw_data(),
+                metadata = dataset$get_metadata(),
+                label = dataset$get_dataset_label()
+              )
+            },
+            simplify = FALSE)
           datasets_module <- teal.slice::init_filtered_data(
             data_objects,
             join_keys = raw_data()$get_join_keys(),

From ee08f1a7f2db8fe449da3a5d46e402538c8c2197 Mon Sep 17 00:00:00 2001
From: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Date: Mon, 31 Jul 2023 13:10:29 +0000
Subject: [PATCH 16/26] [skip actions] Restyle files

---
 R/module_teal.R | 3 ++-
 R/modules.R     | 6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/R/module_teal.R b/R/module_teal.R
index 481b1b727e..de8279a911 100644
--- a/R/module_teal.R
+++ b/R/module_teal.R
@@ -195,7 +195,8 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
                 label = dataset$get_dataset_label()
               )
             },
-            simplify = FALSE)
+            simplify = FALSE
+          )
           datasets_module <- teal.slice::init_filtered_data(
             data_objects,
             join_keys = raw_data()$get_join_keys(),
diff --git a/R/modules.R b/R/modules.R
index 2df84bdf51..2c80384e7d 100644
--- a/R/modules.R
+++ b/R/modules.R
@@ -205,8 +205,10 @@ module <- function(label = "module",
   if (!missing(filters)) {
     checkmate::assert_character(filters, min.len = 1, null.ok = TRUE, any.missing = FALSE)
     datanames <- filters
-    warning("The `filters` argument is deprecated and will be removed in the next release. ",
-            "Please use `datanames` instead. ")
+    warning(
+      "The `filters` argument is deprecated and will be removed in the next release. ",
+      "Please use `datanames` instead. "
+    )
   }
 
   if (label == "global_filters") {

From 404000b654a355741124028b40673a12402dc7b4 Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Mon, 31 Jul 2023 15:19:14 +0200
Subject: [PATCH 17/26] spelling

---
 vignettes/adding-support-for-reporting.Rmd | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vignettes/adding-support-for-reporting.Rmd b/vignettes/adding-support-for-reporting.Rmd
index 4a554118eb..f8b11369b4 100644
--- a/vignettes/adding-support-for-reporting.Rmd
+++ b/vignettes/adding-support-for-reporting.Rmd
@@ -22,7 +22,7 @@ The responsibilities of a module developer include:
 - Adding support for reporting to their module.
 - Specifying the outputs that constitute a snapshot of their module.
 
-The entire lifecycle of objects involved in creating the report and configuring the module to preview the report is handled by `teal`.
+The entire life cycle of objects involved in creating the report and configuring the module to preview the report is handled by `teal`.
 
 ## Custom module
 

From 6052b8a51b8435b99e1c7342a0d1f48c6bd38baf Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Mon, 31 Jul 2023 15:26:50 +0200
Subject: [PATCH 18/26] trigger


From e0755239a5d165196a32919b64248c56cedf7d5b Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Mon, 31 Jul 2023 15:36:00 +0200
Subject: [PATCH 19/26] make use of tightened filter in
 set_available_teal_slices

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

diff --git a/R/module_filter_manager.R b/R/module_filter_manager.R
index c044237c5b..9d3f73bd39 100644
--- a/R/module_filter_manager.R
+++ b/R/module_filter_manager.R
@@ -196,11 +196,8 @@ filter_manager_srv <- function(id, filtered_data_list, filter) {
 #'
 filter_manager_module_srv <- function(id, module_fd, slices_global) {
   moduleServer(id, function(input, output, session) {
-    # Only operate on slices that refer to data sets present in this module.
-    available_slices <- reactive({
-      Filter(function(slice) slice$dataname %in% module_fd$datanames(), slices_global())
-    })
-    module_fd$set_available_teal_slices(available_slices)
+    # Only operate on slices that refer to data sets present in this module and their allowed variables.
+    module_fd$set_available_teal_slices(reactive(slices_global()))
 
     # Track filter state of this module.
     slices_module <- reactive(module_fd$get_filter_state())

From ae5b966b03cc9d5e2dab4b101082c36d5984acb9 Mon Sep 17 00:00:00 2001
From: Aleksander Chlebowski <aleksander.chlebowski@contractors.roche.com>
Date: Mon, 31 Jul 2023 15:39:23 +0200
Subject: [PATCH 20/26] revert last commit

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

diff --git a/R/module_filter_manager.R b/R/module_filter_manager.R
index 9d3f73bd39..c044237c5b 100644
--- a/R/module_filter_manager.R
+++ b/R/module_filter_manager.R
@@ -196,8 +196,11 @@ filter_manager_srv <- function(id, filtered_data_list, filter) {
 #'
 filter_manager_module_srv <- function(id, module_fd, slices_global) {
   moduleServer(id, function(input, output, session) {
-    # Only operate on slices that refer to data sets present in this module and their allowed variables.
-    module_fd$set_available_teal_slices(reactive(slices_global()))
+    # Only operate on slices that refer to data sets present in this module.
+    available_slices <- reactive({
+      Filter(function(slice) slice$dataname %in% module_fd$datanames(), slices_global())
+    })
+    module_fd$set_available_teal_slices(available_slices)
 
     # Track filter state of this module.
     slices_module <- reactive(module_fd$get_filter_state())

From 3d1affa51d97fccdbb1d1204c818091a334283eb Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 1 Aug 2023 14:02:45 +0200
Subject: [PATCH 21/26] warning is appended to `warnings()` and not explicitly
 printed.

---
 R/modules.R | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/R/modules.R b/R/modules.R
index 2c80384e7d..a80ac984c8 100644
--- a/R/modules.R
+++ b/R/modules.R
@@ -205,10 +205,10 @@ module <- function(label = "module",
   if (!missing(filters)) {
     checkmate::assert_character(filters, min.len = 1, null.ok = TRUE, any.missing = FALSE)
     datanames <- filters
-    warning(
-      "The `filters` argument is deprecated and will be removed in the next release. ",
-      "Please use `datanames` instead. "
-    )
+    msg <-
+      "The `filters` argument is deprecated and will be removed in the next release. Please use `datanames` instead."
+    logger::log_warn(msg)
+    warning(msg)
   }
 
   if (label == "global_filters") {

From 9af4ba67323a8f70e3fc5fa8f414611f1fa58c1f Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 1 Aug 2023 14:02:52 +0200
Subject: [PATCH 22/26] filling a gap

---
 R/dummy_functions.R | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/R/dummy_functions.R b/R/dummy_functions.R
index c742a2d70b..cf0c0116d4 100644
--- a/R/dummy_functions.R
+++ b/R/dummy_functions.R
@@ -93,7 +93,7 @@ example_datasets <- function() { # nolint
 #'   shinyApp(app$ui, app$server)
 #' }
 #' @export
-example_module <- function(label = "example teal module", filters = "all") {
+example_module <- function(label = "example teal module", datanames = "all") {
   checkmate::assert_string(label)
   module(
     label,
@@ -110,7 +110,7 @@ example_module <- function(label = "example teal module", filters = "all") {
         encoding = selectInput(ns("dataname"), "Choose a dataset", choices = names(data))
       )
     },
-    filters = filters
+    datanames = datanames
   )
 }
 
@@ -130,13 +130,13 @@ example_modules <- function(datanames = c("ADSL", "ADTTE")) {
       label = "d2",
       modules(
         label = "d3",
-        example_module(label = "aaa1", filters = datanames),
-        example_module(label = "aaa2", filters = datanames),
-        example_module(label = "aaa3", filters = datanames)
+        example_module(label = "aaa1", datanames = datanames),
+        example_module(label = "aaa2", datanames = datanames),
+        example_module(label = "aaa3", datanames = datanames)
       ),
-      example_module(label = "bbb", filters = datanames)
+      example_module(label = "bbb", datanames = datanames)
     ),
-    example_module(label = "ccc", filters = datanames)
+    example_module(label = "ccc", datanames = datanames)
   )
   return(mods)
 }

From 2acdeb6aca77f271f7744733192dba5b6a3f6150 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 1 Aug 2023 14:19:33 +0200
Subject: [PATCH 23/26] reload docs

---
 man/example_module.Rd | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/man/example_module.Rd b/man/example_module.Rd
index 7225197f9c..9a0c88862b 100644
--- a/man/example_module.Rd
+++ b/man/example_module.Rd
@@ -4,13 +4,17 @@
 \alias{example_module}
 \title{An example \code{teal} module}
 \usage{
-example_module(label = "example teal module", filters = "all")
+example_module(label = "example teal module", datanames = "all")
 }
 \arguments{
 \item{label}{(\code{character(1)}) Label shown in the navigation item for the module. Any label possible except
 \code{"global_filters"} - read more in \code{mapping} argument of \link{teal_slices}.}
 
-\item{filters}{(\code{character}) Deprecated. Use \code{datanames} instead.}
+\item{datanames}{(\code{character}) A vector with \code{datanames} that are relevant for the item. The
+filter panel will automatically update the shown filters to include only
+filters in the listed datasets. \code{NULL} will hide the filter panel,
+and the keyword \code{'all'} will show filters of all datasets. \code{datanames} also determines
+a subset of datasets which are appended to the \code{data} argument in \code{server} function.}
 }
 \value{
 A \code{teal} module which can be included in the \code{modules} argument to \code{\link[=init]{init()}}.

From 381e7da3c091d67cfe0db5382d7d1caba2460846 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 1 Aug 2023 14:22:18 +0200
Subject: [PATCH 24/26] update NEWS

---
 NEWS.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS.md b/NEWS.md
index e642796337..493adad67d 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -10,6 +10,7 @@
 ### Breaking changes
 
 * Specifying `filter` argument in `teal::init` requires `teal_slices` object now. Details in documentation of `teal::init`.
+* Soft deprecated `filters` argument in `module` and replaced it with `datanames`. Details in documentation of `teal::module`
 
 ### Miscellaneous
 

From 7af7b05cc07497c137edf511c5fe1951e0adf207 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 1 Aug 2023 14:29:57 +0200
Subject: [PATCH 25/26] correct test description

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

diff --git a/tests/testthat/test-module_nested_tabs.R b/tests/testthat/test-module_nested_tabs.R
index 9494acad4a..74aea0ecd8 100644
--- a/tests/testthat/test-module_nested_tabs.R
+++ b/tests/testthat/test-module_nested_tabs.R
@@ -433,7 +433,7 @@ testthat::test_that(".datasets_to_data returns data which is filtered", {
 })
 
 
-testthat::test_that(".datasets_to_data returns only data requested by modules$filter", {
+testthat::test_that(".datasets_to_data returns only data requested by modules$datanames", {
   datasets <- get_example_filtered_data()
   module <- test_module_wdata(datanames = "d1")
   trigger_data <- reactiveVal(1L)

From 572297c7fea4feb8301819c571282d47adfcdf68 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 1 Aug 2023 15:14:55 +0200
Subject: [PATCH 26/26] debugger!

---
 inst/js/sidebar.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/inst/js/sidebar.js b/inst/js/sidebar.js
index 9672f16c14..c3f8dddab8 100644
--- a/inst/js/sidebar.js
+++ b/inst/js/sidebar.js
@@ -5,7 +5,6 @@ const hideSidebar = () => {
   $(".teal_primary_col").attr("class", "teal_primary_col col-sm-12").resize();
 };
 const showSidebar = () => {
-  debugger;
   $(".teal_primary_col").attr("class", "teal_primary_col col-sm-9").resize();
   $(".teal_secondary_col").delay(600).fadeIn(50);
 };