Skip to content

Commit

Permalink
Merge pull request #44 from openpharma/jt-35-site_edge_case
Browse files Browse the repository at this point in the history
Properly handle zero regions selected in review configuration
  • Loading branch information
jthompson-arcus authored Jul 12, 2024
2 parents 0b897f0 + c119c57 commit 6cd1a5b
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 13 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: clinsight
Title: ClinSight
Version: 0.0.0.9005
Version: 0.0.0.9006
Authors@R: c(
person("Leonard Daniël", "Samson", , "[email protected]", role = c("cre", "aut")),
person("GCP-Service International Ltd.& Co. KG", role = "fnd")
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ to minimize the package dependencies of the production version.
## Bug fixes

- Fixed error of creating adverse events table with empty data frame input.
- Properly handled zero regions selected in review configuration and provided user feedback.
6 changes: 5 additions & 1 deletion R/mod_review_config.R
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ mod_review_config_server <- function(
observeEvent(input$config_review, showModal(review_modal()))

observeEvent(input$region_selection, {
req(input$region_selection, input$site_selection)
selected_sites <- with(sites, site_code[region %in% input$region_selection])
golem::cat_dev("update region selection to ", selected_sites, "\n")
shinyWidgets::updatePickerInput(
Expand All @@ -122,6 +121,11 @@ mod_review_config_server <- function(
choices = selected_sites,
selected = selected_sites
)
}, ignoreNULL = FALSE, ignoreInit = TRUE)

output$review_config_feedback <- renderText({
req(!isTruthy(input$region_selection) | !isTruthy(input$site_selection))
"You must select at least one site/region to review."
})

observeEvent(input$save_review_config, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@
]
},
"output": {

"test-review_config_feedback": {
"message": "",
"call": "NULL",
"type": [
"shiny.silent.error",
"validation"
]
}
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
{
"input": {
"test-config_review": 1,
"test-region_selection": "DEU",
"test-region_selection": null,
"test-save_review_config": 0,
"test-site_selection": [
"Site 01",
"Site 02"
]
"test-site_selection": null
},
"output": {

"test-review_config_feedback": "You must select at least one site/region to review."
}
}
Binary file modified tests/testthat/_snaps/mod_review_config/mod_review_config-002_.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
]
},
"output": {

"test-review_config_feedback": {
"message": "",
"call": "NULL",
"type": [
"shiny.silent.error",
"validation"
]
}
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 21 additions & 0 deletions tests/testthat/_snaps/mod_review_config/mod_review_config-004.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"input": {
"test-config_review": 1,
"test-region_selection": "DEU",
"test-save_review_config": 2,
"test-site_selection": [
"Site 01",
"Site 02"
]
},
"output": {
"test-review_config_feedback": {
"message": "",
"call": "NULL",
"type": [
"shiny.silent.error",
"validation"
]
}
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
19 changes: 16 additions & 3 deletions tests/testthat/test-mod_review_config.R
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,11 @@ describe(
and sites 'Site 01' and 'Site 02' belonging to region 'DEU',
and clicking on [settings],
I expect to see the modal to select regions and sites to review,
and given that I deselect regions 'NLD' and 'BEL',
I expect that only the sites 'Site 01' and 'Site 02' are still selected,
and given that I deselect all regions and click on [Save],
I expect that I will get the message 'You must select at least one site/region to review.',
and that the data within the app will not be updated with the empty selection,
and given that I select region 'DEU',
I expect that only the sites 'Site 01' and 'Site 02' will be selected,
and given that I click on [Save],
I expect that a confirmation will be shown with the text 'Review configuration applied successfully',
and that the data within the app only contains data of 'Site 01' and 'Site 02'. ",
Expand Down Expand Up @@ -146,6 +149,17 @@ describe(
withr::defer(app$stop())
app$click("test-config_review")
app$expect_values(input = TRUE, output = TRUE)
app$set_inputs(`test-region_selection` = "")
app$expect_values(input = TRUE, output = TRUE)
app$click("test-save_review_config")
filtered_data <- app$get_value(export = "filtered_data")
all_sites <- lapply(filtered_data, \(x){x[["site_code"]]}) |>
unlist() |>
unique()
expect_equal(
all_sites[order(all_sites)],
sort(vars$Sites$site_code)
)
app$set_inputs(`test-region_selection` = "DEU")
app$expect_values(input = TRUE, output = TRUE)
app$click("test-save_review_config")
Expand All @@ -162,4 +176,3 @@ describe(
)
}
)

0 comments on commit 6cd1a5b

Please sign in to comment.