Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Graceful Handling of First-touch API Call (App List) #12

Merged
merged 10 commits into from
Jun 12, 2024
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ The LogAnalyzer open-source app is a simple, plug and play application developed

- `[app/logic/api_utils.R` - `get_app_list()]` Posit Connect differentiates apps with two `app_role` values: `owner` and `viewer`. You can toggle between these using the `config.yml` file. The set value is `owner`. If you want to use both together, you can simply set the value to a blank character `""`.

# FAQs

- I get `"Oops! Can't read apps from Posit Connect."` on the rightmost image?
- This may mean that the Posit Connect API's response did not send proper data.
- So far, one documented reason for this is that OAuth on Posit Connect instances may prevent the `/content` endpoint from sending app data.

# Credits

It was our collaboration with <img src="img/elkem_logo.png" alt="Elkem" width="50"/> which led to the creation of this app. The initial idea came from use-cases where we realised we wanted to track all the logs and be able to read them properly since Posit Connect was the de facto deployment environment. When we made this app, we realised there was potential in sharing this with the rest of the community and invite everyone to use it and add it. We appreciate and thank Elkem for their openness to share it with the world.
Expand Down
32 changes: 32 additions & 0 deletions app/logic/empty_state_utils.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
box::use(
shiny[
div,
img,
p,
renderUI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: prefer trailing comma

]
)

#' @description Function to generate an empty state UI
#' @param text Text to display in the empty state
#' @param image_path Path to the image to display in the empty state
#' @export
generate_empty_state_ui <- function(
text = "Select an application and a job to view logs",
image_path = "static/illustrations/empty_state.svg"
) {
renderUI({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: i think it's better to make a function that is not shiny-specific, i.e. remove renderUI from it. This way you can control what to do with the output of this function - use it in the server, or in the ui function.

div(
class = "empty-state-container",
p(
class = "empty-state-text",
text
),
img(
src = image_path,
class = "empty-state-image",
alt = text
)
)
})
}
44 changes: 22 additions & 22 deletions app/main.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ box::use(

box::use(
app/logic/api_utils[get_app_list],
app/logic/empty_state_utils[generate_empty_state_ui],
app/view/mod_app_table,
app/view/mod_header,
app/view/mod_job_list,
Expand Down Expand Up @@ -79,18 +80,12 @@ server <- function(id) {

mod_app_table$server(
"app_table",
app_list() %>%
select(
guid,
name,
r_version,
dashboard_url,
last_deployed_time
),
app_list(),
state
)

observeEvent(state$selected_app()$guid, {

if (isTruthy(state$selected_app()$guid)) {

output$job_list_pane <- renderUI({
Expand All @@ -101,12 +96,16 @@ server <- function(id) {
"job_list",
state
)

} else {

removeUI(ns("job_list_pane"))

}
}, ignoreInit = TRUE, ignoreNULL = TRUE)
}, ignoreInit = FALSE, ignoreNULL = FALSE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: ignoreInit = FALSE is the default value, no need to specify it.


observeEvent(state$selected_job()$key, {

if (isTruthy(state$selected_job()$key)) {

output$logs_pane <- renderUI({
Expand All @@ -118,21 +117,22 @@ server <- function(id) {
state
)
} else {
output$logs_pane <- renderUI({
div(
class = "empty-state-container",
p(
class = "empty-state-text",
"Select an application and a job to view logs"
),
img(
src = "static/empty_state.svg",
class = "empty-state-image",
alt = "Select an application and a job to view logs"
)

if (class(app_list()) != "data.frame") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: prefer `inherits(app_list(), "data.frame")
example:

x <- dplyr::as_tibble(mtcars)
class(x) # "tbl_df"     "tbl"        "data.frame"
class(x) != "data.frame" TRUE  TRUE FALSE
inherits(x, "data.frame") # TRUE

empty_state <- generate_empty_state_ui(
text = "Oops! Can't read apps from Posit Connect.",
image_path = "static/illustrations/missing_apps.svg"
)
})
} else {
empty_state <- generate_empty_state_ui(
text = "Select an application and a job to view logs.",
image_path = "static/illustrations/empty_state.svg"
)
}

output$logs_pane <- empty_state
}

}, ignoreInit = FALSE, ignoreNULL = FALSE)

})
Expand Down
2 changes: 1 addition & 1 deletion app/static/css/app.min.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

File renamed without changes
1 change: 1 addition & 0 deletions app/static/illustrations/missing_apps.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions app/styles/main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
font-family: "Maven Pro", sans-serif;
}

body {
overflow: hidden;
}

.vertical-line {
border-left: 1px $grey2-border solid;
height: 80%;
Expand Down
51 changes: 36 additions & 15 deletions app/view/mod_app_table.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ box::use(
colDef,
getReactableState,
reactable,
reactableLang,
reactableOutput,
renderReactable
],
shiny[
isTruthy,
moduleServer,
NS,
reactive
Expand Down Expand Up @@ -41,23 +43,37 @@ server <- function(id, app_list, state) {

output$app_table <- renderReactable({

processed_apps <- app_list %>%
mutate(
name = paste(
if (length(app_list) > 0 && class(app_list) == "data.frame") {
processed_apps <- app_list %>%
select(
guid,
name,
r_version,
dashboard_url,
last_deployed_time,
sep = "_-_"
)
) %>%
select(
-c(
r_version,
dashboard_url,
last_deployed_time
) %>%
mutate(
name = paste(
name,
r_version,
dashboard_url,
last_deployed_time,
sep = "_-_"
)
) %>%
select(
-c(
r_version,
dashboard_url,
last_deployed_time
)
)
} else {
processed_apps <- data.frame(
guid = character(),
name = character()
)
}

reactable(
data = processed_apps,
Expand All @@ -75,16 +91,21 @@ server <- function(id, app_list, state) {
process_app_data(app_data)
}
)
),
language = reactableLang(
noData = "No apps found."
)
)
})

state$selected_app <- reactive({
index <- getReactableState("app_table", "selected")
list(
"guid" = app_list[index, ]$guid,
"name" = app_list[index, ]$name
)
if (isTruthy(index) && length(app_list > 0)) {
list(
"guid" = app_list[index, ]$guid,
"name" = app_list[index, ]$name
)
}
})

})
Expand Down
Loading