diff --git a/R/deploymentTarget.R b/R/deploymentTarget.R index ff79f5c2..661785b8 100644 --- a/R/deploymentTarget.R +++ b/R/deploymentTarget.R @@ -41,13 +41,13 @@ deploymentTarget <- function(recordPath = ".", } createDeploymentTarget( - appName, - appTitle, - appId, - envVars, - fullAccount$name, # first deploy must be to own account - fullAccount$name, - fullAccount$server + appName = appName, + appTitle = appTitle, + appId = appId, + envVars = envVars, + username = fullAccount$name, # first deploy must be to own account + account = fullAccount$name, + server = fullAccount$server ) } else if (nrow(appDeployments) == 1) { # If both appName and appId supplied, check that they're consistent. @@ -77,12 +77,12 @@ deploymentTargetForApp <- function(appId, application <- getApplication(accountDetails$name, accountDetails$server, appId) createDeploymentTarget( - application$name, - application$title %||% appTitle, - application$id, - application$owner_username, - accountDetails$name, - accountDetails$server + appName = application$name, + appTitle = application$title %||% appTitle, + appId = application$id, + username = application$owner_username, + account = accountDetails$name, + server = accountDetails$server ) } @@ -92,7 +92,8 @@ createDeploymentTarget <- function(appName, envVars, username, account, - server) { + server, + version = 1) { list( appName = appName, appTitle = appTitle %||% "", @@ -100,20 +101,22 @@ createDeploymentTarget <- function(appName, appId = appId, username = username, account = account, - server = server + server = server, + version = version ) } updateDeploymentTarget <- function(previous, appTitle = NULL, envVars = NULL) { createDeploymentTarget( - previous$name, - appTitle %||% previous$title, - previous$appId, - envVars %||% previous$envVars[[1]], + appName = previous$name, + appTitle = appTitle %||% previous$title, + appId = previous$appId, + envVars = envVars %||% previous$envVars[[1]], # if username not previously recorded, use current account - previous$username %||% previous$account, - previous$account, - previous$server + username = previous$username %||% previous$account, + account = previous$account, + server = previous$server, + version = previous$version ) } diff --git a/R/deployments.R b/R/deployments.R index 479b67df..bae841be 100644 --- a/R/deployments.R +++ b/R/deployments.R @@ -70,6 +70,7 @@ deployments <- function(appPath = ".", ok <- ok & okServer } + deployments$envVars[is.na(deployments$envVars)] <- "" if (is.character(deployments$envVars)) { deployments$envVars <- strsplit(deployments$envVars, ", ") } @@ -79,9 +80,11 @@ deployments <- function(appPath = ".", deploymentFields <- c( "name", "title", "username", "account", "server", "hostUrl", "appId", - "bundleId", "url", "envVars" + "bundleId", "url", "envVars", "version" ) +deploymentRecordVersion <- 1L + saveDeployment <- function(recordDir, target, application, @@ -97,6 +100,7 @@ saveDeployment <- function(recordDir, account = target$account, server = target$server, envVars = target$envVars, + version = target$version, hostUrl = hostUrl, appId = application$id, bundleId = bundleId, @@ -111,7 +115,7 @@ saveDeployment <- function(recordDir, addToDeploymentHistory(recordDir, deployment) } - invisible(NULL) + invisible(path) } deploymentRecord <- function(name, @@ -124,6 +128,7 @@ deploymentRecord <- function(name, appId = NULL, bundleId = NULL, url = NULL, + version = deploymentRecordVersion, metadata = list()) { check_character(envVars, allow_null = TRUE) @@ -134,11 +139,12 @@ deploymentRecord <- function(name, username = username, account = account, server = server, - envVars = paste0(envVars, collapse = ", "), + envVars = if (length(envVars) > 0) paste0(envVars, collapse = ", ") else NA, hostUrl = hostUrl %||% "", appId = appId %||% "", bundleId = bundleId %||% "", - url = url %||% "" + url = url %||% "", + version = version ) c(standard, metadata) } diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index 551c5f08..3d4ff3ba 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -83,6 +83,7 @@ addTestDeployment <- function(path, server = "example.com", url = paste0("https://", server, "/", username, "/", appId), hostUrl = NULL, + version = deploymentRecordVersion, metadata = list()) { saveDeployment( path, @@ -93,12 +94,12 @@ addTestDeployment <- function(path, envVars = envVars, account = account, username = username, - server = server + server = server, + version = version ), application = list(id = appId, url = url), hostUrl = hostUrl, metadata = metadata, addToHistory = FALSE ) - invisible() } diff --git a/tests/testthat/test-deploymentTarget.R b/tests/testthat/test-deploymentTarget.R index c6330c22..94393b6f 100644 --- a/tests/testthat/test-deploymentTarget.R +++ b/tests/testthat/test-deploymentTarget.R @@ -80,11 +80,18 @@ test_that("succeeds if there's a single existing deployment", { addTestAccount("ron") app_dir <- withr::local_tempdir() - addTestDeployment(app_dir, appName = "test", appId = "1", username = "ron") + addTestDeployment( + app_dir, + appName = "test", + appId = "1", + username = "ron", + version = "999" + ) target <- deploymentTarget(app_dir) expect_equal(target$appId, "1") expect_equal(target$username, "ron") + expect_equal(target$version, "999") target <- deploymentTarget(app_dir, appName = "test") expect_equal(target$appId, "1") diff --git a/tests/testthat/test-deployments.R b/tests/testthat/test-deployments.R index db008f61..87bb5c5b 100644 --- a/tests/testthat/test-deployments.R +++ b/tests/testthat/test-deployments.R @@ -69,6 +69,25 @@ test_that("can read/write metadata", { expect_equal(out$meta2, "two") }) +test_that("can read/write version", { + dir <- local_temp_app() + + addTestDeployment(dir, version = "999") + out <- deployments(dir, excludeOrphaned = FALSE) + expect_equal(out$version, "999") +}) + +test_that("can read/write missing version", { + # also tests we can read files written by previous versions of package + dir <- local_temp_app() + + path <- addTestDeployment(dir, version = NA) + out <- deployments(dir, excludeOrphaned = FALSE) + + expect_false("version" %in% rownames(read.dcf(path))) + expect_equal(out$version, NA) +}) + test_that("can read/write env vars", { app <- local_temp_app() addTestDeployment(app, "test1", envVars = c("TEST1", "TEST2")) @@ -78,12 +97,21 @@ test_that("can read/write env vars", { expect_equal(deps$envVars, list(c("TEST1", "TEST2"), character())) }) -test_that("can read env vars when none set", { +test_that("can read/write empty env vars", { + # also tests we can read files written by previous versions of package app <- local_temp_app() - addTestDeployment(app, "test1", envVars = NA_character_) + # With empty character vector + path <- addTestDeployment(app, "test1", envVars = character()) deps <- deployments(app, excludeOrphaned = FALSE) - expect_equal(deps$envVars, list(NA_character_)) + expect_false("envVars" %in% rownames(read.dcf(path))) + expect_equal(deps$envVars, list(character())) + + # Or with empty string + path <- addTestDeployment(app, "test1", envVars = "") + deps <- deployments(app, excludeOrphaned = FALSE) + expect_false("envVars" %in% rownames(read.dcf(path))) + expect_equal(deps$envVars, list(character())) }) test_that("can read/write env vars", { @@ -108,7 +136,7 @@ test_that("saveDeployment appends to global history", { appName = "my-app", appTitle = "", appId = 10, - envVars = NULL, + envVars = "abc", # ensure there's an envVars column in output account = "foo", username = "foo", server = "bar"