fix: minor improvements to time zone parsing (#400)

Author: toph-allen

NEWS.md

@@ -2,7 +2,10 @@
 
 ## Enhancements and fixes
 
-- `get_groups()` now paginates through all results when a `prefix` is provided, if the Connect server API version supports pagination. (#328)
+- `get_groups()` now paginates through all results when a `prefix` is provided,
+  if the Connect server API version supports pagination. (#328)
+- Timestamps from the Connect server are now displayed in your local time zone,
+  rather than in UTC. (#400)
 
 # connectapi 0.7.0
 

R/parse.R

@@ -154,35 +154,21 @@ coerce_datetime <- function(x, to, ...) {
 # - "2020-01-01T00:02:03-01:00"
 # nolint end
 parse_connect_rfc3339 <- function(x) {
-  # Convert any timestamps with offsets to a format recognized by `strptime`.
+  # Convert timestamps with offsets to a format recognized by `strptime`.
   x <- gsub("([+-]\\d\\d):(\\d\\d)$", "\\1\\2", x)
+  x <- gsub("Z$", "+0000", x)
 
-  # `purrr::map2_vec()` converts to POSIXct automatically, but we need
-  # `as.POSIXct()` in there to account vectors of length 1, which it seems are
-  # not converted.
-  #
-  # Parse with an inner call to `strptime()`; convert the resulting `POSIXlt`
-  # object to `POSIXct`.
+  # Parse with an inner call to `strptime()`, which returns a POSIXlt object,
+  # and convert that to `POSIXct`.
   #
   # We must specify `tz` in the inner call to correctly compute date math.
-  # Specifying `tz` when parsing just changes the time zone without doing any
-  # date math!
+  # Specifying `tz` when in the outer call just changes the time zone without
+  # doing any date math!
   #
-  # > xlt
-  # [1] "2024-08-29 16:36:33 EDT"
-  # > tzone(xlt)
-  # [1] "America/New_York"
-  # > as.POSIXct(xlt, tz = "UTC")
-  # [1] "2024-08-29 16:36:33 UTC"
-  purrr::map_vec(x, function(.x) {
-    # Times with and without offsets require different formats.
-    format_string <- ifelse(
-      grepl("Z$", .x),
-      "%Y-%m-%dT%H:%M:%OSZ",
-      "%Y-%m-%dT%H:%M:%OS%z"
-    )
-    as.POSIXct(strptime(.x, format = format_string, tz = "UTC"))
-  })
+  # > xlt [1] "2024-08-29 16:36:33 EDT" tzone(xlt) [1] "America/New_York"
+  # as.POSIXct(xlt, tz = "UTC") [1] "2024-08-29 16:36:33 UTC"
+  format_string <- "%Y-%m-%dT%H:%M:%OS%z"
+  as.POSIXct(x, format = format_string, tz = Sys.timezone())
 }
 
 vec_cast.POSIXct.double <- # nolint: object_name_linter

tests/testthat/test-parse.R

@@ -47,8 +47,6 @@ test_that("coerce_datetime fills the void", {
 })
 
 test_that("parse_connect_rfc3339() parses timestamps with offsets as expected", {
-  withr::defer(Sys.setenv(TZ = Sys.getenv("TZ")))
-
   x_mixed <- c(
     "2023-08-22T14:13:14Z",
     "2020-01-01T01:02:03Z",
@@ -75,32 +73,47 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected",
 
   single_offset <- "2023-08-22T15:13:14+01:00"
 
+  withr::local_envvar(TZ = "America/New_York")
   expected <- as.POSIXct(strptime(
     c(
       "2023-08-22T14:13:14+0000",
       "2020-01-01T01:02:03+0000"
     ),
     format = "%Y-%m-%dT%H:%M:%S%z",
-    tz = "UTC"
+    tz = Sys.timezone()
   ))
-
-  Sys.setenv(TZ = "America/New_York")
   expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2))
   expect_identical(parse_connect_rfc3339(x_zero_offset), expected)
   expect_identical(parse_connect_rfc3339(x_plus_one), expected)
   expect_identical(parse_connect_rfc3339(x_minus_one), expected)
   expect_identical(parse_connect_rfc3339(single_zero_offset), expected[1])
   expect_identical(parse_connect_rfc3339(single_offset), expected[1])
 
-  Sys.setenv(TZ = "UTC")
+  withr::local_envvar(TZ = "UTC")
+  expected <- as.POSIXct(strptime(
+    c(
+      "2023-08-22T14:13:14+0000",
+      "2020-01-01T01:02:03+0000"
+    ),
+    format = "%Y-%m-%dT%H:%M:%S%z",
+    tz = Sys.timezone()
+  ))
   expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2))
   expect_identical(parse_connect_rfc3339(x_zero_offset), expected)
   expect_identical(parse_connect_rfc3339(x_plus_one), expected)
   expect_identical(parse_connect_rfc3339(x_minus_one), expected)
   expect_identical(parse_connect_rfc3339(single_zero_offset), expected[1])
   expect_identical(parse_connect_rfc3339(single_offset), expected[1])
 
-  Sys.setenv(TZ = "Asia/Tokyo")
+  withr::local_envvar(TZ = "Asia/Tokyo")
+  expected <- as.POSIXct(strptime(
+    c(
+      "2023-08-22T14:13:14+0000",
+      "2020-01-01T01:02:03+0000"
+    ),
+    format = "%Y-%m-%dT%H:%M:%S%z",
+    tz = Sys.timezone()
+  ))
   expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2))
   expect_identical(parse_connect_rfc3339(x_zero_offset), expected)
   expect_identical(parse_connect_rfc3339(x_plus_one), expected)
@@ -109,7 +122,9 @@ test_that("parse_connect_rfc3339() parses timestamps with offsets as expected",
   expect_identical(parse_connect_rfc3339(single_offset), expected[1])
 })
 
+
 test_that("parse_connect_rfc3339() handles fractional seconds", {
+  withr::local_envvar(TZ = "UTC")
   expected <- as.POSIXct(strptime(
     c(
       "2024-12-06T19:09:29.948016766+0000",
@@ -125,8 +140,6 @@ test_that("parse_connect_rfc3339() handles fractional seconds", {
 })
 
 test_that("make_timestamp produces expected output", {
-  withr::defer(Sys.setenv(TZ = Sys.getenv("TZ")))
-
   x_mixed <- c(
     "2023-08-22T14:13:14Z",
     "2020-01-01T01:02:03Z",
@@ -158,7 +171,7 @@ test_that("make_timestamp produces expected output", {
     "2020-01-01T01:02:03Z"
   )
 
-  Sys.setenv(TZ = "America/New_York")
+  withr::local_envvar(TZ = "America/New_York")
   expect_equal(
     make_timestamp(coerce_datetime(x_mixed, NA_datetime_)),
     rep(outcome, 2)
@@ -185,7 +198,7 @@ test_that("make_timestamp produces expected output", {
   )
   expect_equal(make_timestamp(outcome), outcome)
 
-  Sys.setenv(TZ = "UTC")
+  withr::local_envvar(TZ = "UTC")
   expect_equal(
     make_timestamp(coerce_datetime(x_mixed, NA_datetime_)),
     rep(outcome, 2)
@@ -212,7 +225,7 @@ test_that("make_timestamp produces expected output", {
   )
   expect_equal(make_timestamp(outcome), outcome)
 
-  Sys.setenv(TZ = "Asia/Tokyo")
+  withr::local_envvar(TZ = "Asia/Tokyo")
   expect_equal(
     make_timestamp(coerce_datetime(x_mixed, NA_datetime_)),
     rep(outcome, 2)