Create and implement a shiny module for Archiver class

[Polkas]

linked to #80

We want to create a shiny module for the Archiver class and implement it in the vignettes.
The issue assume the Archiver class have to be adjusted when any additional functionalities are needed.

Remember it has to be applied for Previewer and single button functionality at the same time.
The best if the button functionality (like add card module) could be simply used in the Previewer directly.
I imagine something similar to Add Card module.
In future there will be possible many ways for storing the data please take it into account, now we will work with zip file with JSON and static files (png and Rds).

[Polkas]

@donyunardi in my opinion one of the priorities.
We have to decide if we want to remove the Archiver from the code base as It is not used now or finally implement the Archiver shiny module.

[donyunardi]

@lcd2yyz
Can I hear your opinion on the suggested feature from user/business perspective?
In short, this feature will allow user to upload their reporter setup and customize it further.

[lcd2yyz]

It's a neat little feature for sure, but I don't feel it would add as much value as upload/restore filter settings, or ultimately, the encoding panel settings. Once user add a card to the reporter, the analysis settings and contents are not editable anymore. Also this feature should not be needed, if we can tackle the saving app state at some point (wishful thinking).

[Polkas]

Hey @lcd2yyz and @donyunardi :)
There is already a PR for that #177
I list all the things that still have to be done there.
Do you want me to continue the work? I can work on that in my free time.

[lcd2yyz]

Great to hear from you @Polkas! Thanks for the offer and showing your passion towards teal framework! We'd certainly love to see you continue to contribute to the product and collaborate on new features/ideas.
I think there are already a couple of feedback and feature enhancement ideas from the draft PR that would be helpful to look into. While coredev team won't have the capacity to review PR during this increment (to mid-March) due to the focus on CRAN releases, if you have the time to pick up where it's left off and propose how you envision to take this feature further, I'm hopeful we can find a good collaboration model in future increments!

[Polkas]

Thank you for a quick reply. I am delighted to hear you are open to collaboration. I will work in my free time so It will take me a few weeks. Thus, the NEST team review will not be needed in the close time. I understand you have complete control over the feature shape and design and I am ready for discussions and adjustments. I will try to make a few alternative solutions for my proposition, or even 2 different PRs.
Have a great week all:)

[Polkas]

@pawelru @lcd2yyz @donyunardi

I found something exciting.
I recall some team discussions from the past, and I came up with a simple yet powerful solution. I remember we wanted to have something simple, general, and extensible.
Another advantage of this solution is that we do NOT have to update dozens of teal modules to implement it.
I will be glad to get your opinion on that.

Screen.Recording.2024-02-21.at.23.07.53.mov

Details:
The same zip for the rendered report can be used to load the reporter back.
It is possible as the JSON file, which represents the reporter, is added to the zip file with rendered, e.g., HTML report.

PR with the raw code for the raised solution. Still a lot of work ahead;p
#251

[pawelru]

Hey @Polkas. I liked this. I have one comment though before looking into the code, I feel that the "upload" functionality belongs more into the reporter manager screen reporter previewer as opposed to the individual module - that's because you are working on the collection and not individual card.
You probably wanted to keep it close to the download functionality that is already present there so from that perspective I have to admit that this is consistent and makes sense. This makes me thinking that we should move everything except "add" to the manager screen. Ughh... I probably went beyond what you asked for but still these are my thoughts when I am seeing your demo.

[Polkas]

@pawelru, thanks for a quick replay. I am happy to hear you like the solution. Now, I will wait for others' opinions.

@pawelru, your concerns about simple reporter buttons visible for each teal module are meaningful; you do not want to overcomplicate the teal UI.
teal.reporter was designed to work with any shiny app, not only teal ones.
The Simple reporter (3 buttons now and with my proposition 4) is created to work even without a previewer. For teal modules, we can define a separate teal simple report module or simply add the Add Card module only. It will require to update all teal modules UI/SRV with one line of code, e.g. here.
We do not have to update teal.reporter to change the teal.modules reporter buttons.
I hope I have helped to clarify it a little bit.

[Polkas]

Hey @gogonzo and @donyunardi.
I will need to update teal, then I have to create a separate PR for teal.
The TealReportCard and TealSlicesBlock were moved to teal, and they must be updated for this request.
Please be aware of that.

[lcd2yyz]

Thanks for the update!
I previously made the same comment here #177 (comment) as @pawelru , the upload button would be more fitting in the Previewer tab.
Separately, in coredev team, we've recently been discussing mechanisms to ensure reproducibility for the filter snapshot manager in teal.slice, where there might be changes between the download vs upload sessions in source dataset, or analysis module config, or even package version used to create the app. I feel there might be some shared challenges with this feature in the reporter - how can we check and handle inconsistencies in such scenarios. @chlebowa has been leading that effort, might be good to connect.

[Polkas]

@lcd2yyz thank you for your comments.
I am happy to collaborate on connected issues. @chlebowa, please add me to an appropriate Slack group.
I recommend making this task as independent as possible as my current work has to be reviewed eventually. Later we can create new issues to improve specific parts.

@pawelru and @lcd2yyz, I think I found an excellent solution to make the simple reporter suited for teal without the need to influence the teal.reporter itself.
We can create a teal.reporter option "teal.reporter.simple_reporter_modules" which will take any of c("add", "download", "load", "reset") values.
We will make the simple reporter very flexible by that.
We can add the option for a teal.reporter in the .onLoad of teal. Then, we can update all teal modules without needing a code update and the teal.reporter will still be independent of teal. Example code for the teal onLoad function, options("teal.reporter.simple_reporter_modules" = c("add")).
The code snipped for UI, where SRV will need a similar one.

#' @rdname simple_reporter
#' @export
simple_reporter_ui <- function(
    id, 
    modules = getOption("teal.reporter.simple_reporter_modules", c("add", "download", "load", "reset"))
) {
...

[chlebowa]

@chlebowa, please add me to an appropriate Slack group.

The isn't any 😉

What happens is we create an application hash based on the data and modules here and stamp snapshots with it, and then, when a snapshot is uploaded, we compare the stamp to the current app's id.

It's not the be all and end all because we didn't figure out a way to properly incorporate the contents of remote data into the hash but it's quite simple.

[Polkas]

The update is ready for the review.
teal.reporter PR: #251
teal PR: insightsengineering/teal#1120

I followed a simple design, which was evaluated positively in this issue.
Please visit teal.reporter PR for more information

How it works, videos.

teal:

teal_load_reporter.mov

general shiny app:

general_load_reporter.mov

[Polkas]

I want to leave here an additional comment that my work is done as a collaboration of UCB company with Roche.

[gogonzo]

We are not planning to load the Reporter or any other individual teal-component from the file. teal restore will be supported only globally - once for all the components.

So far we enabled server-bookmarking which is able to restore previewer cards. To do so, just use shinyApp(...,, enableBookmarking = "server")
Feature is currently in a development stage available in teal@main and teal.reporter@main

[Polkas]

teal.reporter is independent of teal. So I am surprised the teal feature impact the teal.reporter. Somebody can use teal.reporter in their regular shiny app. I am open to a discussion. I introduced many improvements not connected with loading reporter too.

[Polkas]

please consider that webassembly is more and more popular then a server side solution like proposed teal one will be not working. Please read about shinylive https://shiny.posit.co/py/docs/shinylive.html

[gogonzo]

teal.reporter is independent of tea

Yes, this is why we are going to discuss this again. Currently teal.reporter is independent and is able to independently restore cards using shiny-bookmarking (see the app below)

code

library(shiny)
library(teal.reporter)
library(ggplot2)
library(rtables)
library(DT)
library(bslib)

# any of c("add", "download", "load", "reset")
options("teal.reporter.simple_reporter_modules" = c("add", "download", "load", "reset"))

ui <- fluidPage(
    # please, specify specific bootstrap version and theme
    theme = bs_theme(version = "4"),
    titlePanel(""),
    tabsetPanel(
        tabPanel(
            "main App",
            tags$br(),
            sidebarLayout(
                sidebarPanel(
                    uiOutput("encoding")
                ),
                mainPanel(
                  bookmarkButton(),
                    tabsetPanel(
                        id = "tabs",
                        tabPanel("Plot", plotOutput("dist_plot")),
                        tabPanel("Table", verbatimTextOutput("table")),
                        tabPanel("Table DataFrame", verbatimTextOutput("table2")),
                        tabPanel("Table DataTable", dataTableOutput("table3"))
                    )
                )
            )
        ),
        ### REPORTER
        tabPanel(
            "Previewer",
            reporter_previewer_ui("prev")
        )
        ###
    )
)
server <- function(input, output, session) {
    output$encoding <- renderUI({
        tagList(
            ### REPORTER
            teal.reporter::simple_reporter_ui("simple_reporter"),
            ###
            if (input$tabs == "Plot") {
                sliderInput(
                    "binwidth",
                    "binwidth",
                    min = 2,
                    max = 10,
                    value = 8
                )
            } else if (input$tabs %in% c("Table", "Table DataFrame", "Table DataTable")) {
                selectInput(
                    "stat",
                    label = "Statistic",
                    choices = c("mean", "median", "sd"),
                    "mean"
                )
            } else {
                NULL
            }
        )
    })
    plot <- reactive({
        req(input$binwidth)
        x <- mtcars$mpg
        ggplot(data = mtcars, aes(x = mpg)) +
            geom_histogram(binwidth = input$binwidth)
    })
    output$dist_plot <- renderPlot(plot())
    
    table <- reactive({
        req(input$stat)
        lyt <- basic_table() %>%
            split_rows_by("Month", label_pos = "visible") %>%
            analyze("Ozone", afun = eval(str2expression(input$stat)))
        build_table(lyt, airquality)
    })
    output$table <- renderPrint(table())
    
    table2 <- reactive({
        req(input$stat)
        data <- aggregate(
            airquality[, c("Ozone"), drop = FALSE], list(Month = airquality$Month), get(input$stat),
            na.rm = TRUE
        )
        colnames(data) <- c("Month", input$stat)
        data
    })
    output$table2 <- renderPrint(print.data.frame(table2()))
    output$table3 <- renderDataTable(table2())
    
    ### REPORTER
    reporter <- Reporter$new()
    card_fun <- function(card = ReportCard$new(), comment) {
        if (input$tabs == "Plot") {
            card$set_name("Plot Module")
            card$append_text("My plot", "header2")
            card$append_plot(plot())
            card$append_rcode(
                paste(
                    c(
                        "x <- mtcars$mpg",
                        "ggplot2::ggplot(data = mtcars, ggplot2::aes(x = mpg)) +",
                        paste0("ggplot2::geom_histogram(binwidth = ", input$binwidth, ")")
                    ),
                    collapse = "\n"
                ),
                echo = TRUE,
                eval = FALSE
            )
        } else if (input$tabs == "Table") {
            card$set_name("Table Module rtables")
            card$append_text("My rtables", "header2")
            card$append_table(table())
            card$append_rcode(
                paste(
                    c(
                        "lyt <- rtables::basic_table() %>%",
                        'rtables::split_rows_by("Month", label_pos = "visible") %>%',
                        paste0('rtables::analyze("Ozone", afun = ', input$stat, ")"),
                        "rtables::build_table(lyt, airquality)"
                    ),
                    collapse = "\n"
                ),
                echo = TRUE,
                eval = FALSE
            )
        } else if (input$tabs %in% c("Table DataFrame", "Table DataTable")) {
            card$set_name("Table Module DF")
            card$append_text("My Table DF", "header2")
            card$append_table(table2())
            # Here r code added as a regular verbatim text
            card$append_text(
                paste0(
                    c(
                        'data <- aggregate(airquality[, c("Ozone"), drop = FALSE], list(Month = airquality$Month), ',
                        input$stat,
                        ", na.rm = TRUE)\n",
                        'colnames(data) <- c("Month", ', paste0('"', input$stat, '"'), ")\n",
                        "data"
                    ),
                    collapse = ""
                ), "verbatim"
            )
        }
        if (!comment == "") {
            card$append_text("Comment", "header3")
            card$append_text(comment)
        }
        card
    }
    teal.reporter::simple_reporter_srv("simple_reporter", reporter = reporter, card_fun = card_fun)
    teal.reporter::reporter_previewer_srv("prev", reporter)
    ###
}

shinyApp(ui = ui, server = server, enableBookmarking = "server")

In the next step we will research how app user can restore the whole app state using fileupload in the running session (which should cover web-assembly case)

---

@Polkas We are having complex design discussions and we are happy to include you at some moment. Let's discuss this soon.

[Polkas]

Thank you for the invitation and the app example. I am very happy you want to invest more time in this topic.
It is not only about shinylive (webassembly); people can run apps interactively without deploying them. Users may want to sent the report to another developer.
The proposed by me solution is light and already in-built in the package, will always work, and can be optional. Remember, the file has already been downloaded, so we do not require additional action from end users.

[Polkas]

More than that, I discovered the proposed server solution works only by luck and only briefly. A card contains FileBlocks (like PictureBlock), and an image/plot is on the TEMP path.
If you reload the report the next day, these temp images/tables can already disappear:)
@pawelru @gogonzo

[Polkas]

The possible solution to what I raised ( the TEMP files problem in the proposed server proposition) is to rewrite onBookmark and onRestored to save the Report (JSON + files) representation in state$dir and later reload it. In this solution, you need most of my PR as you have to use the toJSON and fromJSON methods (I validated and fix many things). FYI, We do not need to render a Report.
However, I found even more problems with the Bookmarking ; the teal.reporter can be used WITHOUT the previewer, but Bookmark calls are only inside it. The module which has to be used always is the Add Card one.

[gogonzo]

However, I found even more problems with the Bookmarking ; the teal.reporter can be used WITHOUT the previewer, but Bookmark calls are only inside it. The module which has to be used always is the Add Card one.

Partially good point. Having just an "add button" is not enough because you need a process which access Reporter object (either download button or previewer module or any other module which access Reporter object). I disagree that teal_module generating card should have a download button to download the whole report (including other teal_module-s). Global actions should be performed globally (through previewer or other module exposed globally for all modules)

There are multiple scenarios we can discuss:

• single reportable module outside of teal. What inputs and outputs this module needs?
• a single reportable module in teal
• multiple reportable modules in teal
• each of above with and without previewer

[Polkas]

You are welcome. I was happy to identify for you the server proposition is faulty:)