Redesign of teal.reporter

[m7pr]

Companion to

• teal.modules.general/report_redesign_poc@main
• teal.reporter/redesign@main

Code examples of dealing with reporter

devtools::load_all("../teal.reporter")
devtools::load_all(".")

# DISABLE teal.reporter::Reporter globally
app <- init(
  data = within(teal_data(), {iris <- iris}),
  modules = example_module(label = "example teal module"),
  reporter = NULL
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

# predefined teal.reporter::Reporter with inital card

reporter <- teal.reporter::Reporter$new()
doc1 <- teal.reporter::report_document("## Header 2 text", "Regular text")
reporter$append_cards(setNames(list(doc1), "Welcome card"))

app <- init(
  data = within(teal_data(), {iris <- iris}),
  modules = example_module(label = "example teal module"),
  reporter = reporter
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

# predefined teal.reporter::Reporter
# that has a template that is always used to prepend all cards

reporter <- teal.reporter::Reporter$new()
template_fun <- function(document) {
  disclaimer <- teal.reporter::report_document("Here comes disclaimer text")
  c(disclaimer, document)
}
reporter$set_template(template_fun)

app <- init(
  data = within(teal_data(), {iris <- iris}),
  modules = example_module(label = "example teal module"),
  reporter = reporter
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

# remove reporter ADD BUTTON for a single module

app <- init(
  data = within(teal_data(), {iris <- iris}),
  modules = modules(
    example_module(label = "module with reporter"),
    example_module(label = "module without reporter") |> nullify_teal_module_report_card()
    # the same nullify_teal_module_report_card = disable_teal_module_report
    #example_module(label = "module without reporter") |> disable_teal_module_report()
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

[m7pr]

@gogonzo modify_reactive_output as agreed is not the best name for the function, especially since we consider the output to not be reactive

         if (is.reactive(original_outputs[[output_name]])) {
            reactive({
              res <- original_outputs[[output_name]]()
              output_funs[[output_name]](res)
            })
          } else {
            res <- original_outputs[[output_name]]
            output_funs[[output_name]](res)
          }

Some potential ideas on the names are below. Given that the function operates on the outputs of a Shiny module's server, I'm also thinking about including 'output' or 'module_output'

• modify_server_output
• mutate_module_output
• remap_module_output (like purrr:map)
• chain_module_output
• wrap_server

In general it could be some verb like wrap/edit and then the object name like output/server_output

When it comes to reusing some names from base R or from tidyverse

• local (evaluates an expression in a local environment, potentially modifying variables within that local scope - module's server outputs being defined in a specific scope)
• transform (base function)
• modify (purrr)
• mutate (dplyr)
• update (base function, used to modify and re-fit model objects, update.lm / update.packages)

however I don't think there is a base equivalent to the fact that we are editing an output of a function (server) that is a part of a list (module)

[m7pr]

Making it a regular PR so we can get CI/CD running

[github-actions]

Unit Tests Summary

  1 files   26 suites   3m 27s ⏱️
274 tests 219 ✅ 48 💤 4 ❌ 3 🔥
470 runs  415 ✅ 48 💤 4 ❌ 3 🔥

For more details on these failures and errors, see this check.

Results for commit e90a5d5.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
module_session_info	💚	$19.65$	$-1.61$	$0$	$0$	$0$	$0$
module_teal	💔	$159.27$	$+26.14$	$0$	$0$	$+4$	$+3$

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
module_session_info	💚	$19.63$	$-1.61$	creation_process_is_invoked_for_teal.lockfile.mode_enabled_and_snapshot_is_copied_to_teal_app.lock_and_removed_after_session_ended
module_teal	💔	$0.36$	$+1.54$	does_not_receive_report_previewer_when_none_of_the_modules_contain_reporter_argument
module_teal	💔	$3.59$	$+1.08$	sets_filters_from_mapping_mod_to_module_s_FilteredData_when_module_specific
shinytest2-reporter	💀	$0.02$	$-0.02$	e2e_adding_a_report_card_in_a_module_adds_it_in_the_report_previewer_tab
shinytest2-reporter	👶		$+0.01$	e2e_adding_a_report_card_with_global_button_adds_it_in_the_report_previewer_tab
shinytest2-reporter	👶		$+0.01$	e2e_reporter_card_can_be_customized
shinytest2-reporter	👶		$+0.01$	e2e_reporter_previewer_module_has_download_load_and_reset_buttons
shinytest2-reporter	💀	$0.01$	$-0.01$	e2e_reporter_tab_is_created_when_a_module_has_reporter
shinytest2-reporter	👶		$+0.02$	e2e_reporter_tab_is_created_when_a_module_has_reporter_report_fun

Results for commit 826e632

♻️ This comment has been updated with latest results.

[averissimo]

Some comments to discuss with @m7pr

[averissimo]

Could we move this to a shiny module to simplify and keep it contained?

Something like

    # replacing UI above
    ui_teal_reporter(ns("reporter_wrapper")),
    # replacing all of this chunk in srv
    srv_teal_reporter("reporter_wrapper", reporter, module_out)

ui_teal_reporter <- function(id) uiOutput(NS(id, "reporter_add_container"))

srv_teal_reporter <- function(id, reporter, module_out) {
  if (is.null(reporter)) return(FALSE) # early exit
  moduleServer(id, function(input, output, session) {
    reporter_card_out <- reactive({
      if (is.list(module_out())) {
        module_out()$report_card()
      }
    })

    output$reporter_add_container <- renderUI({
      req(reporter_card_out())
      tags$div(
        class = "teal add-reporter-container",
        teal.reporter::add_card_button_ui(session$ns("reporter_add"))
      )
    })

    add_document_button_srv("reporter_add", reporter = reporter, r_card_fun = reporter_card_out)
    TRUE
  })
}

[m7pr]

maybe during the redesign of nested tabs? or you think it could be applied even now? sounds like a nice cleanup

[averissimo]

Even now, I merged it in a47a24a

[averissimo]

One less thing to do with nested tabs :-) as it can then reuse this module

[averissimo]

Found a 🐞 bug: Can only add 1 card

How to reproduce:

• Load example shiny app
• Add card with any title (e.g. "title 1")
• Add second card with a different title (e.g. "title 2")
• Open the reporter tab
• Only 1 card is present as screenshot below shows

[m7pr]

Thanks @averissimo for the thorough review! Those are all really useful updates to the current flow :)!

[m7pr]

@averissimo the bug that you reported I was aware of

Found a 🐞 bug: Can only add 1 card
How to reproduce:

Load example shiny app
Add card with any title (e.g. "title 1")
Add second card with a different title (e.g. "title 2")
Open the reporter tab
Only 1 card is present as screenshot below shows

This is because you are adding a second card, that has the same content as the first card (they have just different names, but the content is the same). There is some reactiveVal that keeps the state of the last added card, and if a new card is added with the same name, it prevents adding this card.

If you add a new card inside the same module, and you at least specify a comment, then the content of the second card will be different and this will be possible to be added.