From 2e98e5e14ae3f37cd021dd32775608648805b5e8 Mon Sep 17 00:00:00 2001
From: Sam Abbott <contact@samabbott.co.uk>
Date: Fri, 4 Feb 2022 17:25:31 +0000
Subject: [PATCH] make sure any changes to variable order doesn't impact output

---
 R/processing.R                          |  5 +++--
 tests/testthat/custom_tests/mock_data.R | 12 +++++++-----
 tests/testthat/test-processing.R        |  3 +++
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/R/processing.R b/R/processing.R
index 20ef9584..4fcedaee 100644
--- a/R/processing.R
+++ b/R/processing.R
@@ -244,7 +244,8 @@ process_internal <- function(clean_data, level, group_vars,
           group_vars_standard, "cases_total", "deaths_total", "recovered_total",
           "hosp_total", "tested_total"
         )
-      ))
+      )) %>%
+      ungroup()
   } else {
     dat <- dat %>%
       drop_na(.data$date) %>%
@@ -258,9 +259,9 @@ process_internal <- function(clean_data, level, group_vars,
         )),
         everything()
       ) %>%
+      ungroup() %>%
       arrange(.data$date, all_of(group_vars_standard[1]))
   }
-  dat <- ungroup(dat)
 
   if (localise) {
     old <- names(group_vars)
diff --git a/tests/testthat/custom_tests/mock_data.R b/tests/testthat/custom_tests/mock_data.R
index 062c28ec..0bfd68f0 100644
--- a/tests/testthat/custom_tests/mock_data.R
+++ b/tests/testthat/custom_tests/mock_data.R
@@ -207,12 +207,16 @@ get_expected_data_for_fill_empty_dates_with_na_test <- function() {
   # full data is data with all dates/regions + some NAs in the cases column
   expected_data <- data.frame(expand.grid(dates, regions))
   colnames(expected_data) <- c("date", "level_1_region")
+
   expected_data$date <- as.Date(expected_data$date)
   expected_data$level_1_region <- as.character(expected_data$level_1_region)
   expected_data <- expected_data %>%
     dplyr::arrange(date, level_1_region) %>%
     dplyr::left_join(region_codes, by = c("level_1_region" = "region"))
   expected_data$cases <- c(1:5, rep(NA, 4), 10:12)
+  expected_data <- dplyr::select(
+    expected_data, level_1_region, level_1_region_code, date, everything()
+  )
   return(dplyr::tibble(expected_data))
 }
 
@@ -235,10 +239,8 @@ get_expected_data_for_complete_cumulative_columns_test <- function() {
 
   # manually add cumulative cases to get expected data
   full_data_with_cum_cases_filled <- partial_data %>%
-    dplyr::group_by(level_1_region) %>%
-    covidregionaldata:::fill_empty_dates_with_na() %>%
-    dplyr::ungroup()
-
+    dplyr::group_by(level_1_region, level_1_region_code) %>%
+    covidregionaldata:::fill_empty_dates_with_na()
   full_data_with_cum_cases_filled <-
     dplyr::arrange(full_data_with_cum_cases_filled, level_1_region, date)
   full_data_with_cum_cases_filled <-
@@ -248,5 +250,5 @@ get_expected_data_for_complete_cumulative_columns_test <- function() {
    )
   colnames(full_data_with_cum_cases_filled)[5] <- "cases_total"
 
-  return(dplyr::tibble(full_data_with_cum_cases_filled))
+  return(full_data_with_cum_cases_filled)
 }
diff --git a/tests/testthat/test-processing.R b/tests/testthat/test-processing.R
index 92f6410a..c8df2f39 100644
--- a/tests/testthat/test-processing.R
+++ b/tests/testthat/test-processing.R
@@ -80,6 +80,9 @@ test_that("fill_empty_dates_with_na fills empty dates with NA", {
   expected_data <- get_expected_data_for_fill_empty_dates_with_na_test()
   # partial data deletes some rows (i.e. gets rid of some dates - all the ones
   # with NA in cases)
+  expected_data <- expected_data %>%
+    dplyr::group_by(, level_1_region, level_1_region_code)
+  
   partial_data <- expected_data[-c(6:9), ]
   expect_equal(fill_empty_dates_with_na(partial_data), expected_data)
   expected_data <- dplyr::mutate(