From d194db94eba0984a594ee79bdf5fa07552723827 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Thu, 24 Oct 2024 16:50:41 +0200
Subject: [PATCH 01/11] split check_module_datanames

---
 R/init.R                       |   2 +-
 R/module_teal_data.R           |   9 ++-
 R/utils.R                      | 114 +++++++++++++++++----------------
 man/check_modules_datanames.Rd |   9 ++-
 tests/testthat/test-init.R     |   2 +-
 5 files changed, 72 insertions(+), 64 deletions(-)

diff --git a/R/init.R b/R/init.R
index a62034eaf8..4acc70b54f 100644
--- a/R/init.R
+++ b/R/init.R
@@ -213,7 +213,7 @@ init <- function(data,
 
     is_modules_ok <- check_modules_datanames(modules, ls(teal.code::get_env(data)))
     if (!isTRUE(is_modules_ok) && length(unlist(extract_transformers(modules))) == 0) {
-      lapply(is_modules_ok$string, warning, call. = FALSE)
+      warning(is_modules_ok)
     }
 
     is_filter_ok <- check_filter_datanames(filter, ls(teal.code::get_env(data)))
diff --git a/R/module_teal_data.R b/R/module_teal_data.R
index 899ef14028..c95552d4e1 100644
--- a/R/module_teal_data.R
+++ b/R/module_teal_data.R
@@ -222,14 +222,13 @@ srv_check_shiny_warnings <- function(id, data, modules) {
   moduleServer(id, function(input, output, session) {
     output$message <- renderUI({
       if (inherits(data(), "teal_data")) {
-        is_modules_ok <- check_modules_datanames(modules = modules, datanames = ls(teal.code::get_env(data())))
+        is_modules_ok <- check_modules_datanames(
+          modules = modules, datanames = ls(teal.code::get_env(data())), as_html = TRUE
+        )
         if (!isTRUE(is_modules_ok)) {
           tags$div(
             class = "teal-output-warning",
-            is_modules_ok$html(
-              # Show modules prefix on message only in teal_data_module tab
-              grepl(sprintf("data-teal_data_module-%s", id), session$ns(NULL), fixed = TRUE)
-            )
+            is_modules_ok
           )
         }
       }
diff --git a/R/utils.R b/R/utils.R
index e5830bf0ca..9381877a33 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -127,71 +127,77 @@ report_card_template <- function(title, label, description = NULL, with_filter,
 #'
 #' @param modules (`teal_modules`) object
 #' @param datanames (`character`) names of datasets available in the `data` object
+#' @param as_html (`logical(1)`) whether message should be returned in form of formatted `html`.
 #'
-#' @return A `character(1)` containing error message or `TRUE` if validation passes.
+#' @return `TRUE` if validation passes, otherwise `character` or `shiny.tag.list`
 #' @keywords internal
-check_modules_datanames <- function(modules, datanames) {
-  checkmate::assert_multi_class(modules, c("teal_modules", "teal_module"))
+check_modules_datanames <- function(modules, datanames, as_html = FALSE) {
+  checkmate::assert_multi_class(modules, c("teal_module", "teal_modules"))
   checkmate::assert_character(datanames)
-
-  recursive_check_datanames <- function(modules, datanames) {
-    # check teal_modules against datanames
-    if (inherits(modules, "teal_modules")) {
-      result <- lapply(modules$children, function(module) recursive_check_datanames(module, datanames = datanames))
-      result <- result[vapply(result, Negate(is.null), logical(1L))]
-      if (length(result) == 0) {
-        return(NULL)
-      }
-      list(
-        string = do.call(c, as.list(unname(sapply(result, function(x) x$string)))),
-        html = function(with_module_name = TRUE) {
-          tagList(
-            lapply(
-              result,
-              function(x) x$html(with_module_name = with_module_name)
-            )
-          )
-        }
-      )
-    } else {
-      extra_datanames <- setdiff(modules$datanames, c("all", datanames))
-      if (length(extra_datanames)) {
-        list(
-          string = build_datanames_error_message(
-            modules$label,
-            datanames,
-            extra_datanames,
-            tags = list(
-              span = function(..., .noWS = NULL) { # nolint: object_name
-                trimws(paste(..., sep = ifelse(is.null(.noWS), " ", ""), collapse = " "))
-              },
-              code = function(x) toString(dQuote(x, q = FALSE))
-            ),
-            tagList = function(...) trimws(paste(...))
-          ),
-          # Build HTML representation of the error message with <pre> formatting
-          html = function(with_module_name = TRUE) {
+  check_datanames <- recursive_check_datanames(modules, datanames)
+  if (length(check_datanames)) {
+    if (as_html) {
+      shiny::tagList(
+        lapply(
+          check_datanames,
+          function(mod) {
             tagList(
               build_datanames_error_message(
-                if (with_module_name) modules$label,
-                datanames,
-                extra_datanames
+                label = if (inherits(modules, "teal_modules")) mod$label,
+                datanames = datanames,
+                missing_datanames = mod$missing_datanames
               ),
               tags$br(.noWS = "before")
             )
           }
         )
-      }
+      )
+    } else {
+      modules_msg <- sapply(
+        check_datanames,
+        function(mod) {
+          sprintf(
+            "Dataset(s) (%s) are missing for module %s.",
+            toString(dQuote(mod$missing_datanames, q = FALSE)),
+            toString(dQuote(mod$label, q = FALSE))
+          )
+        }
+      )
+      sprintf(
+        "%s\nDataset(s) available in data: %s",
+        paste(modules_msg, collapse = "\n"),
+        toString(dQuote(datanames, q = FALSE))
+      )
     }
-  }
-  check_datanames <- recursive_check_datanames(modules, datanames)
-  if (length(check_datanames)) {
-    check_datanames
   } else {
     TRUE
   }
 }
 
+#' @rdname check_modules_datanames
+recursive_check_datanames <- function(modules, datanames) {
+  res <- if (inherits(modules, "teal_modules")) {
+    unlist(
+      lapply(
+        modules$children,
+        function(modules) recursive_check_datanames(modules, datanames = datanames)
+      ),
+      recursive = FALSE,
+      use.names = FALSE
+    )
+  } else {
+    missing_datanames <- setdiff(modules$datanames, c("all", datanames))
+    if (length(missing_datanames)) {
+      list(list(
+        label = modules$label,
+        dataname = modules$datanames,
+        missing_datanames = missing_datanames
+      ))
+    }
+  }
+  res
+}
+
 #' Check `datanames` in filters
 #'
 #' This function checks whether `datanames` in filters correspond to those in `data`,
@@ -367,15 +373,13 @@ paste_datanames_character <- function(x,
 #' @noRd
 build_datanames_error_message <- function(label = NULL,
                                           datanames,
-                                          extra_datanames,
-                                          tags = list(span = shiny::tags$span, code = shiny::tags$code),
-                                          tagList = shiny::tagList) { # nolint: object_name.
+                                          missing_datanames) {
   tags$span(
-    tags$span(ifelse(length(extra_datanames) > 1, "Datasets", "Dataset")),
-    paste_datanames_character(extra_datanames, tags, tagList),
+    tags$span(ifelse(length(missing_datanames) > 1, "Datasets", "Dataset")),
+    paste_datanames_character(missing_datanames, tags, tagList),
     tags$span(
       paste0(
-        ifelse(length(extra_datanames) > 1, "are missing", "is missing"),
+        ifelse(length(missing_datanames) > 1, "are missing", "is missing"),
         ifelse(is.null(label), ".", sprintf(" for tab '%s'.", label))
       )
     ),
diff --git a/man/check_modules_datanames.Rd b/man/check_modules_datanames.Rd
index 7fef35aec0..6f6ab4d475 100644
--- a/man/check_modules_datanames.Rd
+++ b/man/check_modules_datanames.Rd
@@ -2,17 +2,22 @@
 % Please edit documentation in R/utils.R
 \name{check_modules_datanames}
 \alias{check_modules_datanames}
+\alias{recursive_check_datanames}
 \title{Check \code{datanames} in modules}
 \usage{
-check_modules_datanames(modules, datanames)
+check_modules_datanames(modules, datanames, as_html = FALSE)
+
+recursive_check_datanames(modules, datanames)
 }
 \arguments{
 \item{modules}{(\code{teal_modules}) object}
 
 \item{datanames}{(\code{character}) names of datasets available in the \code{data} object}
+
+\item{as_html}{(\code{logical(1)}) whether message should be returned in form of formatted \code{html}.}
 }
 \value{
-A \code{character(1)} containing error message or \code{TRUE} if validation passes.
+\code{TRUE} if validation passes, otherwise \code{character} or \code{shiny.tag.list}
 }
 \description{
 This function ensures specified \code{datanames} in modules match those in the data object,
diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R
index d0a022330c..55319d1c4b 100644
--- a/tests/testthat/test-init.R
+++ b/tests/testthat/test-init.R
@@ -64,7 +64,7 @@ testthat::test_that(
         data = teal.data::teal_data(mtcars = mtcars),
         modules = list(example_module(datanames = "iris"))
       ),
-      "Dataset \"iris\" is missing for tab 'example teal module'. Dataset available in data: \"mtcars\"."
+      '"Dataset(s) ("iris") are missing for module "example teal module".\nDataset(s) available in data: "mtcars"'
     )
   }
 )

From 431d9c69dd06027da39c1985a457a30fdd395288 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Fri, 25 Oct 2024 07:46:54 +0200
Subject: [PATCH 02/11] R CMD check and docs

---
 R/init.R                       |  2 +-
 R/utils.R                      | 12 +++++++-----
 man/check_modules_datanames.Rd |  8 +++++---
 tests/testthat/test-init.R     |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/R/init.R b/R/init.R
index 4acc70b54f..005bc448f9 100644
--- a/R/init.R
+++ b/R/init.R
@@ -213,7 +213,7 @@ init <- function(data,
 
     is_modules_ok <- check_modules_datanames(modules, ls(teal.code::get_env(data)))
     if (!isTRUE(is_modules_ok) && length(unlist(extract_transformers(modules))) == 0) {
-      warning(is_modules_ok)
+      warning(is_modules_ok, call. = FALSE)
     }
 
     is_filter_ok <- check_filter_datanames(filter, ls(teal.code::get_env(data)))
diff --git a/R/utils.R b/R/utils.R
index 9381877a33..09b5dbd249 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -123,13 +123,15 @@ report_card_template <- function(title, label, description = NULL, with_filter,
 #' Check `datanames` in modules
 #'
 #' This function ensures specified `datanames` in modules match those in the data object,
-#' returning error messages or `TRUE` for successful validation.
+#' returning error messages or `TRUE` for successful validation. Function can return error message
+#' in two forms `character(1)` for basic assertion usage and `shiny.tag.list` when message is
+#' displayed in the app.
 #'
 #' @param modules (`teal_modules`) object
 #' @param datanames (`character`) names of datasets available in the `data` object
-#' @param as_html (`logical(1)`) whether message should be returned in form of formatted `html`.
+#' @param as_html (`logical(1)`) whether message should be returned in form of formatted `html` (`shiny.tag.list`).
 #'
-#' @return `TRUE` if validation passes, otherwise `character` or `shiny.tag.list`
+#' @return `TRUE` if validation passes, otherwise `character(1)` or `shiny.tag.list`
 #' @keywords internal
 check_modules_datanames <- function(modules, datanames, as_html = FALSE) {
   checkmate::assert_multi_class(modules, c("teal_module", "teal_modules"))
@@ -157,14 +159,14 @@ check_modules_datanames <- function(modules, datanames, as_html = FALSE) {
         check_datanames,
         function(mod) {
           sprintf(
-            "Dataset(s) (%s) are missing for module %s.",
+            "Datasets %s are missing for module %s.",
             toString(dQuote(mod$missing_datanames, q = FALSE)),
             toString(dQuote(mod$label, q = FALSE))
           )
         }
       )
       sprintf(
-        "%s\nDataset(s) available in data: %s",
+        "%s Datasets available in data: %s",
         paste(modules_msg, collapse = "\n"),
         toString(dQuote(datanames, q = FALSE))
       )
diff --git a/man/check_modules_datanames.Rd b/man/check_modules_datanames.Rd
index 6f6ab4d475..a46412fde5 100644
--- a/man/check_modules_datanames.Rd
+++ b/man/check_modules_datanames.Rd
@@ -14,13 +14,15 @@ recursive_check_datanames(modules, datanames)
 
 \item{datanames}{(\code{character}) names of datasets available in the \code{data} object}
 
-\item{as_html}{(\code{logical(1)}) whether message should be returned in form of formatted \code{html}.}
+\item{as_html}{(\code{logical(1)}) whether message should be returned in form of formatted \code{html} (\code{shiny.tag.list}).}
 }
 \value{
-\code{TRUE} if validation passes, otherwise \code{character} or \code{shiny.tag.list}
+\code{TRUE} if validation passes, otherwise \code{character(1)} or \code{shiny.tag.list}
 }
 \description{
 This function ensures specified \code{datanames} in modules match those in the data object,
-returning error messages or \code{TRUE} for successful validation.
+returning error messages or \code{TRUE} for successful validation. Function can return error message
+in two forms \code{character(1)} for basic assertion usage and \code{shiny.tag.list} when message is
+displayed in the app.
 }
 \keyword{internal}
diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R
index 55319d1c4b..6094b198e0 100644
--- a/tests/testthat/test-init.R
+++ b/tests/testthat/test-init.R
@@ -64,7 +64,7 @@ testthat::test_that(
         data = teal.data::teal_data(mtcars = mtcars),
         modules = list(example_module(datanames = "iris"))
       ),
-      '"Dataset(s) ("iris") are missing for module "example teal module".\nDataset(s) available in data: "mtcars"'
+      'Datasets "iris" are missing for module "example teal module". Datasets available in data: "mtcars"'
     )
   }
 )

From 12bd7ff2ca350b69cf33ecfe15c784fb16442ae9 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Fri, 25 Oct 2024 10:22:21 +0200
Subject: [PATCH 03/11] fixes

---
 R/module_teal_data.R           |   4 +-
 R/utils.R                      | 185 +++++++++++++++------------------
 man/check_modules_datanames.Rd |  19 ++--
 3 files changed, 97 insertions(+), 111 deletions(-)

diff --git a/R/module_teal_data.R b/R/module_teal_data.R
index c95552d4e1..5cc970aa84 100644
--- a/R/module_teal_data.R
+++ b/R/module_teal_data.R
@@ -222,8 +222,8 @@ srv_check_shiny_warnings <- function(id, data, modules) {
   moduleServer(id, function(input, output, session) {
     output$message <- renderUI({
       if (inherits(data(), "teal_data")) {
-        is_modules_ok <- check_modules_datanames(
-          modules = modules, datanames = ls(teal.code::get_env(data())), as_html = TRUE
+        is_modules_ok <- check_modules_datanames_html(
+          modules = modules, datanames = ls(teal.code::get_env(data()))
         )
         if (!isTRUE(is_modules_ok)) {
           tags$div(
diff --git a/R/utils.R b/R/utils.R
index 09b5dbd249..be1ec803ef 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -122,70 +122,92 @@ report_card_template <- function(title, label, description = NULL, with_filter,
 
 #' Check `datanames` in modules
 #'
-#' This function ensures specified `datanames` in modules match those in the data object,
-#' returning error messages or `TRUE` for successful validation. Function can return error message
-#' in two forms `character(1)` for basic assertion usage and `shiny.tag.list` when message is
-#' displayed in the app.
+#' These functions check if specified `datanames` in modules match those in the data object,
+#' returning error messages or `TRUE` for successful validation. Two functions return error message
+#' in different forms:
+#' - `check_modules_datanames` returns `character(1)` for basic assertion usage
+#' - `check_modules_datanames_html` returns `shiny.tag.list` to display it in the app.
 #'
 #' @param modules (`teal_modules`) object
 #' @param datanames (`character`) names of datasets available in the `data` object
-#' @param as_html (`logical(1)`) whether message should be returned in form of formatted `html` (`shiny.tag.list`).
 #'
 #' @return `TRUE` if validation passes, otherwise `character(1)` or `shiny.tag.list`
 #' @keywords internal
-check_modules_datanames <- function(modules, datanames, as_html = FALSE) {
-  checkmate::assert_multi_class(modules, c("teal_module", "teal_modules"))
-  checkmate::assert_character(datanames)
-  check_datanames <- recursive_check_datanames(modules, datanames)
+check_modules_datanames <- function(modules, datanames) {
+  check_datanames <- check_modules_datanames_recursive(modules, datanames)
   if (length(check_datanames)) {
-    if (as_html) {
-      shiny::tagList(
-        lapply(
-          check_datanames,
-          function(mod) {
-            tagList(
-              build_datanames_error_message(
-                label = if (inherits(modules, "teal_modules")) mod$label,
-                datanames = datanames,
-                missing_datanames = mod$missing_datanames
-              ),
-              tags$br(.noWS = "before")
-            )
-          }
-        )
+    modules_msg <- sapply(check_datanames, function(mod) {
+      sprintf(
+        "Datasets %s are missing for module %s.",
+        toString(dQuote(mod$missing_datanames, q = FALSE)),
+        toString(dQuote(mod$label, q = FALSE))
       )
-    } else {
-      modules_msg <- sapply(
+    })
+    sprintf(
+      "%s Datasets available in data: %s",
+      paste(modules_msg, collapse = "\n"),
+      toString(dQuote(datanames, q = FALSE))
+    )
+  } else {
+    TRUE
+  }
+}
+
+#' @rdname check_modules_datanames
+check_modules_datanames_html <- function(modules,
+                                         datanames) {
+  check_datanames <- check_modules_datanames_recursive(modules, datanames)
+  show_module_info <- inherits(modules, "teal_modules")
+  if (length(check_datanames)) {
+    shiny::tagList(
+      lapply(
         check_datanames,
         function(mod) {
-          sprintf(
-            "Datasets %s are missing for module %s.",
-            toString(dQuote(mod$missing_datanames, q = FALSE)),
-            toString(dQuote(mod$label, q = FALSE))
+          tagList(
+            tags$span(
+              tags$span(`if`(length(mod$missing_datanames) > 1, "Datasets", "Dataset")),
+              to_html_code_list(mod$missing_datanames),
+              tags$span(
+                paste0(
+                  `if`(length(mod$missing_datanames) > 1, "are missing", "is missing"),
+                  `if`(show_module_info, sprintf(" for tab '%s'.", mod$label), ".")
+                )
+              ),
+              if (length(mod$datanames) >= 1) {
+                tagList(
+                  tags$span(`if`(length(mod$datanames) > 1, "Datasets", "Dataset")),
+                  tags$span("available in data:"),
+                  tagList(
+                    tags$span(
+                      to_html_code_list(mod$datanames),
+                      tags$span(".", .noWS = "outside"),
+                      .noWS = c("outside")
+                    )
+                  )
+                )
+              } else {
+                tags$span("No datasets are available in data.")
+              }
+            ),
+            tags$br(.noWS = "before")
           )
         }
       )
-      sprintf(
-        "%s Datasets available in data: %s",
-        paste(modules_msg, collapse = "\n"),
-        toString(dQuote(datanames, q = FALSE))
-      )
-    }
+    )
   } else {
     TRUE
   }
 }
 
-#' @rdname check_modules_datanames
-recursive_check_datanames <- function(modules, datanames) {
-  res <- if (inherits(modules, "teal_modules")) {
+#' Recursively checks modules and returns list for every datanames mismatch between module and data
+#' @noRd
+check_modules_datanames_recursive <- function(modules, datanames) {
+  checkmate::assert_multi_class(modules, c("teal_module", "teal_modules"))
+  checkmate::assert_character(datanames)
+  if (inherits(modules, "teal_modules")) {
     unlist(
-      lapply(
-        modules$children,
-        function(modules) recursive_check_datanames(modules, datanames = datanames)
-      ),
-      recursive = FALSE,
-      use.names = FALSE
+      lapply(modules$children, function(mod) check_modules_datanames_recursive(mod, datanames)),
+      recursive = FALSE
     )
   } else {
     missing_datanames <- setdiff(modules$datanames, c("all", datanames))
@@ -197,9 +219,26 @@ recursive_check_datanames <- function(modules, datanames) {
       ))
     }
   }
-  res
 }
 
+#' Convert character vector to html code separated with commas and "and"
+#' @noRd
+to_html_code_list <- function(x) { # nolint: object_name.
+  checkmate::assert_character(x)
+  do.call(
+    tagList,
+    lapply(seq_along(x), function(.ix) {
+      tagList(
+        tags$code(x[.ix]),
+        if (.ix != length(x)) {
+          tags$span(ifelse(.ix == length(x) - 1, " and ", ", "))
+        }
+      )
+    })
+  )
+}
+
+
 #' Check `datanames` in filters
 #'
 #' This function checks whether `datanames` in filters correspond to those in `data`,
@@ -348,57 +387,3 @@ strip_style <- function(string) {
     useBytes = TRUE
   )
 }
-
-#' Convert character list to human readable html with commas and "and"
-#' @noRd
-paste_datanames_character <- function(x,
-                                      tags = list(span = shiny::tags$span, code = shiny::tags$code),
-                                      tagList = shiny::tagList) { # nolint: object_name.
-  checkmate::assert_character(x)
-  do.call(
-    tagList,
-    lapply(seq_along(x), function(.ix) {
-      tagList(
-        tags$code(x[.ix]),
-        if (.ix != length(x)) {
-          tags$span(ifelse(.ix == length(x) - 1, " and ", ", "))
-        }
-      )
-    })
-  )
-}
-
-#' Build datanames error string for error message
-#'
-#' tags and tagList are overwritten in arguments allowing to create strings for
-#' logging purposes
-#' @noRd
-build_datanames_error_message <- function(label = NULL,
-                                          datanames,
-                                          missing_datanames) {
-  tags$span(
-    tags$span(ifelse(length(missing_datanames) > 1, "Datasets", "Dataset")),
-    paste_datanames_character(missing_datanames, tags, tagList),
-    tags$span(
-      paste0(
-        ifelse(length(missing_datanames) > 1, "are missing", "is missing"),
-        ifelse(is.null(label), ".", sprintf(" for tab '%s'.", label))
-      )
-    ),
-    if (length(datanames) >= 1) {
-      tagList(
-        tags$span(ifelse(length(datanames) > 1, "Datasets", "Dataset")),
-        tags$span("available in data:"),
-        tagList(
-          tags$span(
-            paste_datanames_character(datanames, tags, tagList),
-            tags$span(".", .noWS = "outside"),
-            .noWS = c("outside")
-          )
-        )
-      )
-    } else {
-      tags$span("No datasets are available in data.")
-    }
-  )
-}
diff --git a/man/check_modules_datanames.Rd b/man/check_modules_datanames.Rd
index a46412fde5..b01270eae2 100644
--- a/man/check_modules_datanames.Rd
+++ b/man/check_modules_datanames.Rd
@@ -2,27 +2,28 @@
 % Please edit documentation in R/utils.R
 \name{check_modules_datanames}
 \alias{check_modules_datanames}
-\alias{recursive_check_datanames}
+\alias{check_modules_datanames_html}
 \title{Check \code{datanames} in modules}
 \usage{
-check_modules_datanames(modules, datanames, as_html = FALSE)
+check_modules_datanames(modules, datanames)
 
-recursive_check_datanames(modules, datanames)
+check_modules_datanames_html(modules, datanames)
 }
 \arguments{
 \item{modules}{(\code{teal_modules}) object}
 
 \item{datanames}{(\code{character}) names of datasets available in the \code{data} object}
-
-\item{as_html}{(\code{logical(1)}) whether message should be returned in form of formatted \code{html} (\code{shiny.tag.list}).}
 }
 \value{
 \code{TRUE} if validation passes, otherwise \code{character(1)} or \code{shiny.tag.list}
 }
 \description{
-This function ensures specified \code{datanames} in modules match those in the data object,
-returning error messages or \code{TRUE} for successful validation. Function can return error message
-in two forms \code{character(1)} for basic assertion usage and \code{shiny.tag.list} when message is
-displayed in the app.
+These functions check if specified \code{datanames} in modules match those in the data object,
+returning error messages or \code{TRUE} for successful validation. Two functions return error message
+in different forms:
+\itemize{
+\item \code{check_modules_datanames} returns \code{character(1)} for basic assertion usage
+\item \code{check_modules_datanames_html} returns \code{shiny.tag.list} to display it in the app.
+}
 }
 \keyword{internal}

From 0356a5522c830beacd1c1ba523d88ee231de05d6 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Fri, 25 Oct 2024 10:26:43 +0200
Subject: [PATCH 04/11] improvements

---
 R/module_teal_data.R       | 5 +----
 R/utils.R                  | 3 ++-
 tests/testthat/test-init.R | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/R/module_teal_data.R b/R/module_teal_data.R
index 5cc970aa84..ebf95b115f 100644
--- a/R/module_teal_data.R
+++ b/R/module_teal_data.R
@@ -226,10 +226,7 @@ srv_check_shiny_warnings <- function(id, data, modules) {
           modules = modules, datanames = ls(teal.code::get_env(data()))
         )
         if (!isTRUE(is_modules_ok)) {
-          tags$div(
-            class = "teal-output-warning",
-            is_modules_ok
-          )
+          tags$div(is_modules_ok, class = "teal-output-warning")
         }
       }
     })
diff --git a/R/utils.R b/R/utils.R
index be1ec803ef..55f64ee492 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -138,7 +138,8 @@ check_modules_datanames <- function(modules, datanames) {
   if (length(check_datanames)) {
     modules_msg <- sapply(check_datanames, function(mod) {
       sprintf(
-        "Datasets %s are missing for module %s.",
+        "%s %s are missing for module %s.",
+        `if`(length(mod$missing_datanames) > 1, "Datasets", "Dataset"),
         toString(dQuote(mod$missing_datanames, q = FALSE)),
         toString(dQuote(mod$label, q = FALSE))
       )
diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R
index 6094b198e0..8a9fce036f 100644
--- a/tests/testthat/test-init.R
+++ b/tests/testthat/test-init.R
@@ -64,7 +64,7 @@ testthat::test_that(
         data = teal.data::teal_data(mtcars = mtcars),
         modules = list(example_module(datanames = "iris"))
       ),
-      'Datasets "iris" are missing for module "example teal module". Datasets available in data: "mtcars"'
+      'Dataset "iris" are missing for module "example teal module". Datasets available in data: "mtcars"'
     )
   }
 )

From 6ec4ce088b25bf07057f568b60520f85597dc869 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Fri, 25 Oct 2024 10:33:45 +0200
Subject: [PATCH 05/11] improve

---
 R/utils.R                  |  3 ++-
 tests/testthat/test-init.R | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/R/utils.R b/R/utils.R
index 55f64ee492..e164c30dfb 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -138,9 +138,10 @@ check_modules_datanames <- function(modules, datanames) {
   if (length(check_datanames)) {
     modules_msg <- sapply(check_datanames, function(mod) {
       sprintf(
-        "%s %s are missing for module %s.",
+        "%s %s %s missing for module %s.",
         `if`(length(mod$missing_datanames) > 1, "Datasets", "Dataset"),
         toString(dQuote(mod$missing_datanames, q = FALSE)),
+        `if`(length(mod$missing_datanames) > 1, "are", "is"),
         toString(dQuote(mod$label, q = FALSE))
       )
     })
diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R
index 8a9fce036f..abc57f5928 100644
--- a/tests/testthat/test-init.R
+++ b/tests/testthat/test-init.R
@@ -64,7 +64,20 @@ testthat::test_that(
         data = teal.data::teal_data(mtcars = mtcars),
         modules = list(example_module(datanames = "iris"))
       ),
-      'Dataset "iris" are missing for module "example teal module". Datasets available in data: "mtcars"'
+      "Dataset \"iris\" is missing for module \"example teal module\". Datasets available in data: \"mtcars\""
+    )
+  }
+)
+
+testthat::test_that(
+  "init throws warning when datanames in modules incompatible w/ datanames in data and there is no transformers",
+  {
+    testthat::expect_warning(
+      init(
+        data = teal.data::teal_data(mtcars = mtcars),
+        modules = list(example_module(datanames = c("a", "b")))
+      ),
+      "Datasets \"a\", \"b\" are missing for module \"example teal module\". Datasets available in data: \"mtcars\""
     )
   }
 )

From 1a86779c861cafff6bda5d539a5800131e4739ab Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Fri, 25 Oct 2024 10:59:41 +0200
Subject: [PATCH 06/11] lintr

---
 R/utils.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/utils.R b/R/utils.R
index e164c30dfb..e91e1874b7 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -203,7 +203,7 @@ check_modules_datanames_html <- function(modules,
 
 #' Recursively checks modules and returns list for every datanames mismatch between module and data
 #' @noRd
-check_modules_datanames_recursive <- function(modules, datanames) {
+check_modules_datanames_recursive <- function(modules, datanames) { # nolint: object_name_length
   checkmate::assert_multi_class(modules, c("teal_module", "teal_modules"))
   checkmate::assert_character(datanames)
   if (inherits(modules, "teal_modules")) {

From 12d923f443e36441a540cdaa9ab3178c764ebb12 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Mon, 28 Oct 2024 13:54:38 +0100
Subject: [PATCH 07/11] @averissimo

---
 R/utils.R | 75 +++++++++++++++++++++++++++----------------------------
 1 file changed, 37 insertions(+), 38 deletions(-)

diff --git a/R/utils.R b/R/utils.R
index e91e1874b7..21c83e77bd 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -139,9 +139,9 @@ check_modules_datanames <- function(modules, datanames) {
     modules_msg <- sapply(check_datanames, function(mod) {
       sprintf(
         "%s %s %s missing for module %s.",
-        `if`(length(mod$missing_datanames) > 1, "Datasets", "Dataset"),
+        if (length(mod$missing_datanames) > 1) "Datasets" else "Dataset",
         toString(dQuote(mod$missing_datanames, q = FALSE)),
-        `if`(length(mod$missing_datanames) > 1, "are", "is"),
+        if (length(mod$missing_datanames) > 1) "are" else "is",
         toString(dQuote(mod$label, q = FALSE))
       )
     })
@@ -160,45 +160,44 @@ check_modules_datanames_html <- function(modules,
                                          datanames) {
   check_datanames <- check_modules_datanames_recursive(modules, datanames)
   show_module_info <- inherits(modules, "teal_modules")
-  if (length(check_datanames)) {
-    shiny::tagList(
-      lapply(
-        check_datanames,
-        function(mod) {
-          tagList(
+  if (!length(check_datanames)) {
+    return(TRUE)
+  }
+  shiny::tagList(
+    lapply(
+      check_datanames,
+      function(mod) {
+        tagList(
+          tags$span(
+            tags$span(`if`(length(mod$missing_datanames) > 1, "Datasets", "Dataset")),
+            to_html_code_list(mod$missing_datanames),
             tags$span(
-              tags$span(`if`(length(mod$missing_datanames) > 1, "Datasets", "Dataset")),
-              to_html_code_list(mod$missing_datanames),
-              tags$span(
-                paste0(
-                  `if`(length(mod$missing_datanames) > 1, "are missing", "is missing"),
-                  `if`(show_module_info, sprintf(" for tab '%s'.", mod$label), ".")
-                )
-              ),
-              if (length(mod$datanames) >= 1) {
+              paste0(
+                `if`(length(mod$missing_datanames) > 1, "are missing", "is missing"),
+                `if`(show_module_info, sprintf(" for tab '%s'.", mod$label), ".")
+              )
+            ),
+            if (length(mod$datanames) >= 1) {
+              tagList(
+                tags$span(`if`(length(mod$datanames) > 1, "Datasets", "Dataset")),
+                tags$span("available in data:"),
                 tagList(
-                  tags$span(`if`(length(mod$datanames) > 1, "Datasets", "Dataset")),
-                  tags$span("available in data:"),
-                  tagList(
-                    tags$span(
-                      to_html_code_list(mod$datanames),
-                      tags$span(".", .noWS = "outside"),
-                      .noWS = c("outside")
-                    )
+                  tags$span(
+                    to_html_code_list(mod$datanames),
+                    tags$span(".", .noWS = "outside"),
+                    .noWS = c("outside")
                   )
                 )
-              } else {
-                tags$span("No datasets are available in data.")
-              }
-            ),
-            tags$br(.noWS = "before")
-          )
-        }
-      )
+              )
+            } else {
+              tags$span("No datasets are available in data.")
+            }
+          ),
+          tags$br(.noWS = "before")
+        )
+      }
     )
-  } else {
-    TRUE
-  }
+  )
 }
 
 #' Recursively checks modules and returns list for every datanames mismatch between module and data
@@ -208,7 +207,7 @@ check_modules_datanames_recursive <- function(modules, datanames) { # nolint: ob
   checkmate::assert_character(datanames)
   if (inherits(modules, "teal_modules")) {
     unlist(
-      lapply(modules$children, function(mod) check_modules_datanames_recursive(mod, datanames)),
+      lapply(modules$children, check_modules_datanames_recursive, datanames = datanames),
       recursive = FALSE
     )
   } else {
@@ -225,7 +224,7 @@ check_modules_datanames_recursive <- function(modules, datanames) { # nolint: ob
 
 #' Convert character vector to html code separated with commas and "and"
 #' @noRd
-to_html_code_list <- function(x) { # nolint: object_name.
+to_html_code_list <- function(x) {
   checkmate::assert_character(x)
   do.call(
     tagList,

From bd8041aee7d8a942a695fe91ac1391e584bb8f61 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 29 Oct 2024 06:00:11 +0100
Subject: [PATCH 08/11] review and tests

---
 R/module_teal.R                   |   1 +
 R/utils.R                         |  35 ++++----
 tests/testthat/test-module_teal.R | 131 ++++++++++++++++++++++++------
 3 files changed, 125 insertions(+), 42 deletions(-)

diff --git a/R/module_teal.R b/R/module_teal.R
index 8624636dd2..3e913c2340 100644
--- a/R/module_teal.R
+++ b/R/module_teal.R
@@ -210,6 +210,7 @@ srv_teal <- function(id, data, modules, filter = teal_slices()) {
     data_load_status <- reactive({
       if (inherits(data_pulled(), "teal_data")) {
         "ok"
+        # todo: should we hide warnings on top for a data?
       } else if (inherits(data, "teal_data_module")) {
         "teal_data_module failed"
       } else {
diff --git a/R/utils.R b/R/utils.R
index 21c83e77bd..f3054cb4cc 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -169,30 +169,30 @@ check_modules_datanames_html <- function(modules,
       function(mod) {
         tagList(
           tags$span(
-            tags$span(`if`(length(mod$missing_datanames) > 1, "Datasets", "Dataset")),
+            tags$span(if (length(mod$missing_datanames) == 1) "Dataset" else "Datasets"),
             to_html_code_list(mod$missing_datanames),
             tags$span(
               paste0(
-                `if`(length(mod$missing_datanames) > 1, "are missing", "is missing"),
-                `if`(show_module_info, sprintf(" for tab '%s'.", mod$label), ".")
+                if (length(mod$missing_datanames) > 1) "are missing" else "is missing",
+                if (show_module_info) sprintf(" for tab '%s'.", mod$label) else "."
               )
-            ),
-            if (length(mod$datanames) >= 1) {
+            )
+          ),
+          if (length(datanames) >= 1) {
+            tagList(
+              tags$span(if (length(datanames) == 1) "Dataset" else "Datasets"),
+              tags$span("available in data:"),
               tagList(
-                tags$span(`if`(length(mod$datanames) > 1, "Datasets", "Dataset")),
-                tags$span("available in data:"),
-                tagList(
-                  tags$span(
-                    to_html_code_list(mod$datanames),
-                    tags$span(".", .noWS = "outside"),
-                    .noWS = c("outside")
-                  )
+                tags$span(
+                  to_html_code_list(datanames),
+                  tags$span(".", .noWS = "outside"),
+                  .noWS = c("outside")
                 )
               )
-            } else {
-              tags$span("No datasets are available in data.")
-            }
-          ),
+            )
+          } else {
+            tags$span("No datasets are available in data.")
+          },
           tags$br(.noWS = "before")
         )
       }
@@ -215,7 +215,6 @@ check_modules_datanames_recursive <- function(modules, datanames) { # nolint: ob
     if (length(missing_datanames)) {
       list(list(
         label = modules$label,
-        dataname = modules$datanames,
         missing_datanames = missing_datanames
       ))
     }
diff --git a/tests/testthat/test-module_teal.R b/tests/testthat/test-module_teal.R
index ed01caaef4..8e91f7f2c8 100644
--- a/tests/testthat/test-module_teal.R
+++ b/tests/testthat/test-module_teal.R
@@ -551,32 +551,115 @@ testthat::describe("srv_teal teal_modules", {
     )
   })
 
-  testthat::it("throws warning when dataname is not available", {
-    testthat::skip_if_not_installed("rvest")
-    shiny::testServer(
-      app = srv_teal,
-      args = list(
-        id = "test",
-        data = teal_data(mtcars = mtcars),
-        modules = modules(
-          module("module_1", server = function(id, data) data, datanames = c("iris"))
-        )
-      ),
-      expr = {
-        session$setInputs(`teal_modules-active_tab` = "module_1")
+  testthat::describe("warnings on missing datanames", {
+    testthat::it("warns when dataname is not available", {
+      testthat::skip_if_not_installed("rvest")
+      shiny::testServer(
+        app = srv_teal,
+        args = list(
+          id = "test",
+          data = teal_data(iris = iris),
+          modules = modules(
+            module("module_1", server = function(id, data) data, datanames = c("iris", "missing"))
+          )
+        ),
+        expr = {
+          session$setInputs(`teal_modules-active_tab` = "module_1")
+          testthat::expect_equal(
+            trimws(
+              rvest::html_text2(
+                rvest::read_html(
+                  output[["teal_modules-module_1-validate_datanames-shiny_warnings-message"]]$html
+                )
+              )
+            ),
+            "Dataset missing is missing. Dataset available in data: iris."
+          )
+        }
+      )
+    })
 
-        testthat::expect_equal(
-          trimws(
-            rvest::html_text2(
-              rvest::read_html(
-                output[["teal_modules-module_1-validate_datanames-shiny_warnings-message"]]$html
+    testthat::it("warns when datanames are not available", {
+      testthat::skip_if_not_installed("rvest")
+      shiny::testServer(
+        app = srv_teal,
+        args = list(
+          id = "test",
+          data = teal_data(mtcars = mtcars, iris = iris),
+          modules = modules(
+            module("module_1", datanames = c("mtcars", "iris", "missing1", "missing2"))
+          )
+        ),
+        expr = {
+          session$setInputs(`teal_modules-active_tab` = "module_1")
+
+          testthat::expect_equal(
+            trimws(
+              rvest::html_text2(
+                rvest::read_html(
+                  output[["teal_modules-module_1-validate_datanames-shiny_warnings-message"]]$html
+                )
               )
-            )
-          ),
-          "Dataset iris is missing. No datasets are available in data."
-        )
-      }
-    )
+            ),
+            "Datasets missing1 and missing2 are missing. Datasets available in data: iris and mtcars."
+          )
+        }
+      )
+    })
+
+    testthat::it("warns about empty data when none of module$datanames is available (even if data is not empty)", {
+      testthat::skip_if_not_installed("rvest")
+      shiny::testServer(
+        app = srv_teal,
+        args = list(
+          id = "test",
+          data = teal_data(mtcars = mtcars),
+          modules = modules(
+            module("module_1", datanames = c("missing1", "missing2"))
+          )
+        ),
+        expr = {
+          session$setInputs(`teal_modules-active_tab` = "module_1")
+          testthat::expect_equal(
+            trimws(
+              rvest::html_text2(
+                rvest::read_html(
+                  output[["teal_modules-module_1-validate_datanames-shiny_warnings-message"]]$html
+                )
+              )
+            ),
+            "Datasets missing1 and missing2 are missing. No datasets are available in data."
+          )
+        }
+      )
+    })
+
+    testthat::it("warns about empty data when none of module$datanames is available", {
+      testthat::skip_if_not_installed("rvest")
+      shiny::testServer(
+        app = srv_teal,
+        args = list(
+          id = "test",
+          data = reactive(teal_data(mtcars = mtcars)),
+          modules = modules(
+            module("module_1", datanames = c("missing1", "missing2"))
+          )
+        ),
+        expr = {
+          session$setInputs(`teal_modules-active_tab` = "module_1")
+          testthat::expect_equal(
+            trimws(
+              rvest::html_text2(
+                rvest::read_html(
+                  output[["validate-shiny_warnings-message"]]$html
+                )
+              )
+            ),
+            "Datasets missing1 and missing2 are missing for tab 'module_1'. Dataset available in data: mtcars."
+          )
+        }
+      )
+    })
   })
 
   testthat::it("is called and receives data even if datanames in `teal_data` are not sufficient", {

From a0d2e5965e14977c90aa5618589f7a0a18bd1238 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 29 Oct 2024 06:23:19 +0100
Subject: [PATCH 09/11] reusing html

---
 R/utils.R                         | 25 +++++++------------------
 tests/testthat/test-init.R        |  4 ++--
 tests/testthat/test-module_teal.R |  2 +-
 3 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/R/utils.R b/R/utils.R
index f3054cb4cc..df00a84e42 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -134,24 +134,13 @@ report_card_template <- function(title, label, description = NULL, with_filter,
 #' @return `TRUE` if validation passes, otherwise `character(1)` or `shiny.tag.list`
 #' @keywords internal
 check_modules_datanames <- function(modules, datanames) {
-  check_datanames <- check_modules_datanames_recursive(modules, datanames)
-  if (length(check_datanames)) {
-    modules_msg <- sapply(check_datanames, function(mod) {
-      sprintf(
-        "%s %s %s missing for module %s.",
-        if (length(mod$missing_datanames) > 1) "Datasets" else "Dataset",
-        toString(dQuote(mod$missing_datanames, q = FALSE)),
-        if (length(mod$missing_datanames) > 1) "are" else "is",
-        toString(dQuote(mod$label, q = FALSE))
-      )
-    })
-    sprintf(
-      "%s Datasets available in data: %s",
-      paste(modules_msg, collapse = "\n"),
-      toString(dQuote(datanames, q = FALSE))
-    )
+  out <- check_modules_datanames_html(modules, datanames)
+  if (inherits(out, "shiny.tag.list")) {
+    out_with_ticks <- gsub("<code>|</code>", "`", toString(out))
+    out_text <- trimws(gsub("<[^<>]+>", "", toString(out_with_ticks)))
+    gsub("[[:space:]]+", " ", out_text)
   } else {
-    TRUE
+    out
   }
 }
 
@@ -174,7 +163,7 @@ check_modules_datanames_html <- function(modules,
             tags$span(
               paste0(
                 if (length(mod$missing_datanames) > 1) "are missing" else "is missing",
-                if (show_module_info) sprintf(" for tab '%s'.", mod$label) else "."
+                if (show_module_info) sprintf(" for module '%s'.", mod$label) else "."
               )
             )
           ),
diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R
index abc57f5928..1ebca65fa2 100644
--- a/tests/testthat/test-init.R
+++ b/tests/testthat/test-init.R
@@ -64,7 +64,7 @@ testthat::test_that(
         data = teal.data::teal_data(mtcars = mtcars),
         modules = list(example_module(datanames = "iris"))
       ),
-      "Dataset \"iris\" is missing for module \"example teal module\". Datasets available in data: \"mtcars\""
+      "Dataset `iris` is missing for module 'example teal module'. Dataset available in data: `mtcars`."
     )
   }
 )
@@ -77,7 +77,7 @@ testthat::test_that(
         data = teal.data::teal_data(mtcars = mtcars),
         modules = list(example_module(datanames = c("a", "b")))
       ),
-      "Datasets \"a\", \"b\" are missing for module \"example teal module\". Datasets available in data: \"mtcars\""
+      "Datasets `a` and `b` are missing for module 'example teal module'. Dataset available in data: `mtcars`."
     )
   }
 )
diff --git a/tests/testthat/test-module_teal.R b/tests/testthat/test-module_teal.R
index 8e91f7f2c8..2fa450ed60 100644
--- a/tests/testthat/test-module_teal.R
+++ b/tests/testthat/test-module_teal.R
@@ -655,7 +655,7 @@ testthat::describe("srv_teal teal_modules", {
                 )
               )
             ),
-            "Datasets missing1 and missing2 are missing for tab 'module_1'. Dataset available in data: mtcars."
+            "Datasets missing1 and missing2 are missing for module 'module_1'. Dataset available in data: mtcars."
           )
         }
       )

From 62bde91014b10a5673c2c4ba4c2e114cac62fe75 Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 29 Oct 2024 07:17:20 +0100
Subject: [PATCH 10/11] minor

---
 R/utils.R | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/R/utils.R b/R/utils.R
index df00a84e42..46cc909955 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -137,8 +137,8 @@ check_modules_datanames <- function(modules, datanames) {
   out <- check_modules_datanames_html(modules, datanames)
   if (inherits(out, "shiny.tag.list")) {
     out_with_ticks <- gsub("<code>|</code>", "`", toString(out))
-    out_text <- trimws(gsub("<[^<>]+>", "", toString(out_with_ticks)))
-    gsub("[[:space:]]+", " ", out_text)
+    out_text <- gsub("<[^<>]+>", "", toString(out_with_ticks))
+    trimws(gsub("[[:space:]]+", " ", out_text))
   } else {
     out
   }
@@ -148,7 +148,7 @@ check_modules_datanames <- function(modules, datanames) {
 check_modules_datanames_html <- function(modules,
                                          datanames) {
   check_datanames <- check_modules_datanames_recursive(modules, datanames)
-  show_module_info <- inherits(modules, "teal_modules")
+  show_module_info <- inherits(modules, "teal_modules") # used in two contexts - module and app
   if (!length(check_datanames)) {
     return(TRUE)
   }

From 302208d32c5fa8bead89221ca26e75bfecbe510a Mon Sep 17 00:00:00 2001
From: go_gonzo <dawid.kaledkowski@gmail.com>
Date: Tue, 29 Oct 2024 13:53:42 +0100
Subject: [PATCH 11/11] review suggestion

---
 R/utils.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/utils.R b/R/utils.R
index 46cc909955..0397774e7b 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -220,7 +220,7 @@ to_html_code_list <- function(x) {
       tagList(
         tags$code(x[.ix]),
         if (.ix != length(x)) {
-          tags$span(ifelse(.ix == length(x) - 1, " and ", ", "))
+          if (.ix == length(x) - 1) tags$span(" and ") else tags$span(", ", .noWS = "before")
         }
       )
     })