Merge pull request #132 from dfe-analytical-services/development

Development
dfe-analytical-services · Aug 21, 2024 · a9570b1 · a9570b1
2 parents 863b853 + 362c8ba
commit a9570b1
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 33 deletions.
diff --git a/R/mainTests.r b/R/mainTests.r
@@ -1493,11 +1493,12 @@ ward_combinations <- function(data) {
 # region_combinations -------------------------------------
 # Checking that region_code and region_name combinations are valid
 # We know from geography_level_present (pre-check 2) that if regional rows exist both cols must be present
+# We know from region_col_present (main tests) that if one region col exists both cols must be present
 
 region_combinations <- function(data) {
-  if (!"region_code" %in% names(data)) {
+  if (!"region_code" %in% names(data) || !"region_name" %in% names(data)) {
     output <- list(
-      "message" = "region_code column is not present in this data file.",
+      "message" = "At least one of the region columns is not present in this data file.",
       "result" = "IGNORE"
     )
   } else {
@@ -1544,40 +1545,33 @@ region_combinations <- function(data) {
 }
 
 # country_combinations -------------------------------------
-# checking that country_code and country_name combinations are valid
-
+# Checking that country_code and country_name combinations are valid
+# We already know that both columns have to exist from data_mandatory_cols() (fileValidation)
 country_combinations <- function(data) {
-  if (!"country_code" %in% names(data)) {
+  invalid_values <- data %>%
+    select("country_code", "country_name") %>%
+    filter(country_code != gssNAvcode) %>%
+    unique() %>%
+    mutate(combo = paste(country_code, country_name)) %>%
+    pull(combo) %>%
+    .[!(. %in% expected_country_combinations)]
+
+  if (length(invalid_values) == 0) {
     output <- list(
-      "message" = "Country columns are not present in this data file.",
-      "result" = "IGNORE"
+      "message" = "All country_code and country_name combinations are valid.",
+      "result" = "PASS"
     )
   } else {
-    invalid_values <- data %>%
-      select("country_code", "country_name") %>%
-      filter(country_code != gssNAvcode) %>%
-      unique() %>%
-      mutate(combo = paste(country_code, country_name)) %>%
-      pull(combo) %>%
-      .[!(. %in% expected_country_combinations)]
-
-    if (length(invalid_values) == 0) {
+    if (length(invalid_values) == 1) {
       output <- list(
-        "message" = "All country_code and country_name combinations are valid.",
-        "result" = "PASS"
+        "message" = paste0("The following country_code / country_name combination is invalid: '", paste0(invalid_values), "'. <br> - We do not expect any combinations outside of the <a href='https://github.com/dfe-analytical-services/dfe-published-data-qa/blob/main/data/country.csv' target='_blank'>standard geographies lookup</a> (case sensitive), please check your name and code combinations against this lookup."),
+        "result" = "FAIL"
       )
     } else {
-      if (length(invalid_values) == 1) {
-        output <- list(
-          "message" = paste0("The following country_code / country_name combination is invalid: '", paste0(invalid_values), "'. <br> - We do not expect any combinations outside of the <a href='https://github.com/dfe-analytical-services/dfe-published-data-qa/blob/main/data/country.csv' target='_blank'>standard geographies lookup</a> (case sensitive), please check your name and code combinations against this lookup."),
-          "result" = "FAIL"
-        )
-      } else {
-        output <- list(
-          "message" = paste0("The following country_code / country_name combinations are invalid: '", paste0(invalid_values, collapse = "', '"), "'. <br> - We do not expect any combinations outside of the <a href='https://github.com/dfe-analytical-services/dfe-published-data-qa/blob/main/data/country.csv' target='_blank'>standard geographies lookup</a> (case sensitive), please check your name and code combinations against this lookup."),
-          "result" = "FAIL"
-        )
-      }
+      output <- list(
+        "message" = paste0("The following country_code / country_name combinations are invalid: '", paste0(invalid_values, collapse = "', '"), "'. <br> - We do not expect any combinations outside of the <a href='https://github.com/dfe-analytical-services/dfe-published-data-qa/blob/main/data/country.csv' target='_blank'>standard geographies lookup</a> (case sensitive), please check your name and code combinations against this lookup."),
+        "result" = "FAIL"
+      )
     }
   }
 

diff --git a/manifest.json b/manifest.json
@@ -3903,7 +3903,7 @@
       "checksum": "e5b9343c9a0e43d83e02b339946591d1"
     },
     ".Rprofile": {
-      "checksum": "ec8bbd4624ab1b090bdb8673b41d024a"
+      "checksum": "48d983e0bfe75553a940019bee72a138"
     },
     "azure-pipelines-dev.yml": {
       "checksum": "fc2ef843ff5024bcba25ff84eec05f4c"
@@ -3984,7 +3984,7 @@
       "checksum": "7248277e0e99b5aee4e66bf9e8c69963"
     },
     "R/mainTests.r": {
-      "checksum": "6dda83d9475c4095792d073c57fb73a8"
+      "checksum": "58b52819cc963c2cdcbe70024a0307b7"
     },
     "R/manual_scripts/debugging.R": {
       "checksum": "e56763afdf3659c7c47cfe0e3262e8cf"
@@ -4568,6 +4568,12 @@
     "tests/testthat/otherData/lad_within_la.meta.csv": {
       "checksum": "fb14aee7a7e0fb0671e5feb4c41af9bf"
     },
+    "tests/testthat/otherData/missing_region_name.csv": {
+      "checksum": "307a7ff183aa1eb50f567d2a3d7b6071"
+    },
+    "tests/testthat/otherData/missing_region_name.meta.csv": {
+      "checksum": "f7397c726426f2c1210b9062ddeb8ac5"
+    },
     "tests/testthat/otherData/multiple_stripped_filter_groups.csv": {
       "checksum": "93efa02917cba280b37a1b13083d2819"
     },
@@ -4899,7 +4905,7 @@
       "checksum": "41a423a4bc68e5ba6c20a11f06092471"
     },
     "tests/testthat/test-function-edge_cases.R": {
-      "checksum": "cc86859c0c8b5919e764d20fa8c72207"
+      "checksum": "f28bebce557004db15917ee71e2a4fab"
     },
     "tests/testthat/test-function-fileValidation.R": {
       "checksum": "302dd04b308471644c1a6574c3844c81"
@@ -4926,7 +4932,7 @@
       "checksum": "e1c273ef462a5f49a60df1e3a5e94fb6"
     },
     "tests/testthat/test-UI-03_additional_qa.R": {
-      "checksum": "4570dc372c5f2d83bb0039da58795e3e"
+      "checksum": "82a40f0765550189be42b19c4178ee66"
     },
     "ui.R": {
       "checksum": "52e49b75e9766a106c5653305d382d16"

diff --git a/tests/testthat/otherData/missing_region_name.csv b/tests/testthat/otherData/missing_region_name.csv
@@ -0,0 +1,4 @@
+time_period,time_identifier,geographic_level,country_code,country_name,region_code,region,old_la_code,new_la_code,la_name,school_urn,school_laestab,school_name,number_dragons
+2024,Reporting year,School,E92000001,England,E12000001,North East,390,E08000037,Gateshead,108320,3901000,Bensham Grove Nursery School,25
+2024,Reporting year,School,E92000001,England,E12000001,North East,390,E08000037,Gateshead,108321,3902008,Carr Hill Community Primary School,12
+2024,Reporting year,School,E92000001,England,E12000001,North East,390,E08000037,Gateshead,108323,3902012,Kelvin Grove Community Primary School,13
diff --git a/tests/testthat/otherData/missing_region_name.meta.csv b/tests/testthat/otherData/missing_region_name.meta.csv
@@ -0,0 +1,2 @@
+col_name,col_type,label,indicator_grouping,indicator_unit,indicator_dp,filter_hint,filter_grouping_column
+number_dragons,Indicator,Count of imaginary dragons,,,,,
diff --git a/tests/testthat/test-UI-03_additional_qa.R b/tests/testthat/test-UI-03_additional_qa.R
@@ -56,6 +56,8 @@ test_that("Additional QA tabs", {
   app$set_inputs(outlier_indicator_parameter = "num_schools")
   app$set_inputs(submit_outlier = "click")
 
+  app$wait_for_idle(50) # this test seems weirdly flaky for unknown reasons
+
   app$expect_values(output = "table_outlier_list")
 
   # 9. Outlier check doesn't run with same time periods

diff --git a/tests/testthat/test-function-edge_cases.R b/tests/testthat/test-function-edge_cases.R
@@ -208,3 +208,7 @@ test_that("blank_meta_label_notNA_still_fails", {
 test_that("Can handle incorrect provider cols", {
   expect_no_error(screeningOutput <- testOther("../../tests/testthat/otherData/provider_col_incorrect.csv"))
 })
+
+test_that("Can handle missing region_name", {
+  expect_no_error(screeningOutput <- testOther("../../tests/testthat/otherData/missing_region_name.csv"))
+})