From 604d9c3e0edc4b36117baa3009bf43829c37a53c Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Mon, 7 Oct 2024 14:08:51 +0100 Subject: [PATCH 01/39] Increment version number to 0.0.0.9006 --- DESCRIPTION | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 4420b33..64ea23e 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: objr Title: Wrapper for Objective APIs -Version: 0.0.0.9005 +Version: 0.0.0.9006 Authors@R: c( person("Scottish Government", , , "statistics.enquiries@gov.scot", role = "cph"), person("Alice", "Hannah", , "alice.hannah@gov.scot", role = c("aut", "cre")) @@ -8,7 +8,7 @@ Authors@R: c( Description: An R package wrapper for Objective APIs including Objective Connect. License: MIT + file LICENCE -URL:https://github.com/ScotGovAnalysis/objr, +URL: https://github.com/ScotGovAnalysis/objr, https://ScotGovAnalysis.github.io/objr/ BugReports: https://github.com/ScotGovAnalysis/objr/issues Imports: From 6908d685ffa090edca2e1503f28cf161029d6e85 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Mon, 7 Oct 2024 14:18:55 +0100 Subject: [PATCH 02/39] Add FAQ re compatibility with eRDM --- vignettes/faqs.Rmd | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vignettes/faqs.Rmd b/vignettes/faqs.Rmd index 1fd4f82..5e36f26 100644 --- a/vignettes/faqs.Rmd +++ b/vignettes/faqs.Rmd @@ -12,3 +12,8 @@ vignette: > A user is an individual using Objective Connect. A participant is a user in a particular workspace. For example, a person using Objective Connect will have a User UUID associated with them, and also a Participant UUID for each individual workspace they are a member of. A user can get their User UUID by running `my_user_id()`. A user can find their participant UUID for a particular workspace using `participants()`. + + +## Can I use the objr package to interact with eRDM? + +Unfortunately, no. Although both eRDM and Connect are [Objective](https://www.objective.co.uk/) products, their API's are considerably different. The objr package currently works for Objective Connect only, although it may be developed in the future to work with eRDM. From 8f610d3877930b55c4a99d97a1dee690ed9d4ad3 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Mon, 7 Oct 2024 14:37:26 +0100 Subject: [PATCH 03/39] Add .lintr to Rbuildignore --- .Rbuildignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.Rbuildignore b/.Rbuildignore index 9e7be91..a39eb84 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -5,3 +5,4 @@ ^_pkgdown\.yml$ ^docs$ ^pkgdown$ +^\.lintr$ From b1a386abc1a15f7836f9e52ed75bb2d8ccee0425 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Mon, 7 Oct 2024 14:50:17 +0100 Subject: [PATCH 04/39] Adjust nolint comments --- R/download.R | 4 +++- R/upload.R | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/R/download.R b/R/download.R index 8487136..f82e532 100644 --- a/R/download.R +++ b/R/download.R @@ -58,8 +58,10 @@ download_file <- function(document_uuid, #' To check what file type your document is (and thus what function additional #' arguments will be passed to), use \code{asset_info()}. #' +# nolint start: line_length_linter #' If there are other data file types you would like to download using this -#' function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. # nolint: line_length_linter +#' function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. +# nolint end #' #' @return For csv and xlsx files, a data frame. For rds files, an R object. #' diff --git a/R/upload.R b/R/upload.R index 9d13c7e..0be4f36 100644 --- a/R/upload.R +++ b/R/upload.R @@ -115,8 +115,10 @@ upload_file_version <- function(file, #' | rds | \code{readr::write_rds()} | #' | xlsx | \code{writexl::write_xlsx()} | #' +# nolint start: line_length_linter #' If there are other data file types you would like to upload using this -#' function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. # nolint: line_length_linter +#' function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. +# nolint end #' #' @return An httr2 [httr2::response()][response] (invisibly) #' @@ -169,8 +171,10 @@ write_data <- function(x, #' | rds | \code{readr::write_rds()} | #' | xlsx | \code{writexl::write_xlsx()} | #' +# nolint start: line_length_linter #' If there are other data file types you would like to upload using this -#' function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. # nolint: line_length_linter +#' function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. +# nolint end #' #' @return An httr2 [httr2::response()][response] (invisibly) #' From 26f1277f14ed4f84d6d922433b8346b3a303ce80 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Mon, 7 Oct 2024 15:41:14 +0100 Subject: [PATCH 05/39] Add context for 404 error (asset not found) --- R/objr.R | 7 +++++++ tests/testthat/test-objr.R | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/R/objr.R b/R/objr.R index 01fdd53..8e9d7cf 100644 --- a/R/objr.R +++ b/R/objr.R @@ -206,6 +206,13 @@ error <- function(response) { "See https://scotgovanalysis.github.io/objr/articles/two-factor.html" } + if (status == 404) { + extra <- c( + "Asset cannot be found.", + "Check asset UUID is correct." + ) + } + c(desc, extra) } diff --git a/tests/testthat/test-objr.R b/tests/testthat/test-objr.R index 9d2c531..a083c5b 100644 --- a/tests/testthat/test-objr.R +++ b/tests/testthat/test-objr.R @@ -113,6 +113,11 @@ test_that("Expect character value returned", { "character" ) + expect_type( + error(httr2::response(status_code = 404)), + "character" + ) + expect_type( error(httr2::response_json( status_code = 403, From 1341e2f9936e41c9b8e89eccb515ac18eaf0ddbd Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Mon, 7 Oct 2024 15:45:49 +0100 Subject: [PATCH 06/39] Add function to rename asset --- NAMESPACE | 1 + R/assets.R | 30 ++++++++++++++++++++++++++++++ man/rename_asset.Rd | 18 ++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 man/rename_asset.Rd diff --git a/NAMESPACE b/NAMESPACE index 59ea0ad..756983a 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -14,6 +14,7 @@ export(objr) export(participant_bypass_2fa) export(participants) export(read_data) +export(rename_asset) export(upload_file) export(upload_file_version) export(workspaces) diff --git a/R/assets.R b/R/assets.R index c4f8d65..d071cc9 100644 --- a/R/assets.R +++ b/R/assets.R @@ -93,6 +93,36 @@ delete_asset <- function(asset_uuid, } +#' Rename an asset +#' +#' @param asset_uuid UUID of asset +#' @param new_name Character. New name to give asset. +#' @inheritParams objr +#' +#' @export + +rename_asset <- function(asset_uuid, + new_name, + use_proxy = FALSE) { + + response <- objr( + endpoint = "assets", + method = "PUT", + accept = "*/*", + url_path = list(asset_uuid, "name"), + body = list(name = new_name), + use_proxy = use_proxy + ) + + if (response$status_code == 204) { + cli::cli_alert_success("Asset renamed to {.val new_name}: {asset_uuid}.") + } + + invisible(response) + +} + + na_if_null <- function(x) { if (is.null(x)) NA else x } diff --git a/man/rename_asset.Rd b/man/rename_asset.Rd new file mode 100644 index 0000000..0691338 --- /dev/null +++ b/man/rename_asset.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/assets.R +\name{rename_asset} +\alias{rename_asset} +\title{Rename an asset} +\usage{ +rename_asset(asset_uuid, new_name, use_proxy = FALSE) +} +\arguments{ +\item{asset_uuid}{UUID of asset} + +\item{new_name}{Character. New name to give asset.} + +\item{use_proxy}{Logical to indicate whether to use proxy} +} +\description{ +Rename an asset +} From 509e9807cc3c7f97a1319bdf36a7b4529dfe9912 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Mon, 7 Oct 2024 15:56:21 +0100 Subject: [PATCH 07/39] Test rename_asset --- .../assets/test_asset_uuid/name-94e381-PUT.R | 11 ++++++ tests/testthat/test-assets.R | 39 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 tests/testthat/api/assets/test_asset_uuid/name-94e381-PUT.R diff --git a/tests/testthat/api/assets/test_asset_uuid/name-94e381-PUT.R b/tests/testthat/api/assets/test_asset_uuid/name-94e381-PUT.R new file mode 100644 index 0000000..0b9f0b3 --- /dev/null +++ b/tests/testthat/api/assets/test_asset_uuid/name-94e381-PUT.R @@ -0,0 +1,11 @@ +structure(list(method = "PUT", url = "https://api/assets/test_asset_uuid/name", + status_code = 204L, headers = structure(list(Date = "Mon, 07 Oct 2024 14:50:38 GMT", + Connection = "keep-alive", `X-CONNECT-MDC` = "apiOuIhByfr", + `X-Frame-Options` = "deny", `X-XSS-Protection` = "1; mode=block", + `Cache-Control` = "no-cache, no-store", Expires = "0", + Pragma = "no-cache", `Strict-Transport-Security` = "max-age=31536000 ; includeSubDomains", + `Content-Security-Policy` = "script-src 'self' 'unsafe-inline'", + `X-Content-Type-Options` = "nosniff", `Referrer-Policy` = "strict-origin-when-cross-origin", + `Feature-Policy` = "vibrate 'none'; geolocation 'none'", + Authorization = "REDACTED", `Set-Cookie` = "REDACTED"), class = "httr2_headers"), + body = raw(0), cache = new.env(parent = emptyenv())), class = "httr2_response") diff --git a/tests/testthat/test-assets.R b/tests/testthat/test-assets.R index 9648388..f9713d2 100644 --- a/tests/testthat/test-assets.R +++ b/tests/testthat/test-assets.R @@ -118,3 +118,42 @@ with_mock_api({ }) }) + + + +# rename_asset ---- + +without_internet({ + + test_that("Valid request", { + + expect_PUT( + rename_asset(asset_uuid = "test_asset_uuid", + new_name = "test_new_name"), + paste0("https://secure.objectiveconnect.co.uk/publicapi/1/assets/", + "test_asset_uuid/name") + ) + + }) + +}) + +with_mock_api({ + + test_that("Function returns invisible", { + + expect_invisible( + suppressMessages(rename_asset(asset_uuid = "test_asset_uuid", + new_name = "test_new_name")) + ) + + }) + + test_that("Function returns success message", { + + expect_message(rename_asset(asset_uuid = "test_asset_uuid", + new_name = "test_new_name")) + + }) + +}) From 08294df1938b836582da93af4fc1a7d3d967a20a Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Mon, 7 Oct 2024 16:32:22 +0100 Subject: [PATCH 08/39] Add rename_asset to pkgdown reference --- _pkgdown.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/_pkgdown.yml b/_pkgdown.yml index d440611..a154d9a 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -61,6 +61,7 @@ reference: - assets - asset_info - create_folder + - rename_asset - delete_asset - subtitle: Download From f6396927fc5053e99d68bfa83ec019d0790d45e6 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 8 Oct 2024 16:49:55 +0100 Subject: [PATCH 09/39] Fix bug in var names from asset_info --- R/upload.R | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/R/upload.R b/R/upload.R index 0be4f36..2f37e7a 100644 --- a/R/upload.R +++ b/R/upload.R @@ -82,9 +82,10 @@ upload_file_version <- function(file, info <- asset_info(document_uuid) # nolint: object_usage_linter if (httr2::resp_status(response) == 204) { - cli::cli_alert_success( - "New version created: {paste(info$name, info$extension, sep = \".\")}." - ) + cli::cli_alert_success(paste( + "New version created:", + "{paste(info$asset_name, info$asset_ext, sep = \".\")}." + )) } invisible(response) @@ -189,8 +190,8 @@ write_data_version <- function(x, info <- asset_info(document_uuid) path <- write_temp(x, - file_name = info$name, - file_type = info$extension, + file_name = info$asset_name, + file_type = info$asset_ext, ...) # Delete temp file when exiting function From ca91878a60d70655ed712246d40e0cfe0414e35a Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 8 Oct 2024 16:58:18 +0100 Subject: [PATCH 10/39] Add function to get data frame of document versions --- NAMESPACE | 1 + R/assets.R | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ man/versions.Rd | 23 +++++++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 man/versions.Rd diff --git a/NAMESPACE b/NAMESPACE index 756983a..c2565c4 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -17,6 +17,7 @@ export(read_data) export(rename_asset) export(upload_file) export(upload_file_version) +export(versions) export(workspaces) export(write_data) export(write_data_version) diff --git a/R/assets.R b/R/assets.R index d071cc9..4791da2 100644 --- a/R/assets.R +++ b/R/assets.R @@ -63,6 +63,39 @@ asset_info <- function(asset_uuid, } +#' Get data frame of document versions +#' +#' @param document_uuid UUID of document (asset) +#' @param page Page number of responses to return (0..N). +#' @param size Number of results to be returned per page. +#' @inheritParams objr +#' +#' @return Data frame +#' +#' @export + +versions <- function(document_uuid, + page = NULL, + size = NULL, + use_proxy = FALSE) { + + response <- objr( + endpoint = "documentversions", + url_query = list(documentUuid = document_uuid, + page = page, + size = size), + use_proxy = use_proxy + ) + + content <- + httr2::resp_body_json(response)$content |> + lapply(\(x) data.frame(versions_info_list(x))) + + Reduce(dplyr::bind_rows, content) + +} + + #' Delete an asset #' #' @param asset_uuid UUID of asset @@ -145,3 +178,20 @@ asset_info_list <- function(x) { ) } + +versions_info_list <- function(x) { + + list( + asset_name = x[["asset"]]$name, + asset_ext = na_if_null(x$extension), + asset_uuid = x[["asset"]]$uuid, + version = x$version, + version_uuid = x$uuid, + created_date = na_if_null(convert_from_epoch(x$createdTime)), + created_by = paste(na_if_null(x[["createdBy"]]$givenName), + na_if_null(x[["createdBy"]]$familyName)), + workspace_name = x[["asset"]][["workspace"]]$name, + workspace_uuid = x[["asset"]][["workspace"]]$uuid + ) + +} diff --git a/man/versions.Rd b/man/versions.Rd new file mode 100644 index 0000000..b091b02 --- /dev/null +++ b/man/versions.Rd @@ -0,0 +1,23 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/assets.R +\name{versions} +\alias{versions} +\title{Get data frame of document versions} +\usage{ +versions(document_uuid, page = NULL, size = NULL, use_proxy = FALSE) +} +\arguments{ +\item{document_uuid}{UUID of document (asset)} + +\item{page}{Page number of responses to return (0..N).} + +\item{size}{Number of results to be returned per page.} + +\item{use_proxy}{Logical to indicate whether to use proxy} +} +\value{ +Data frame +} +\description{ +Get data frame of document versions +} From d4cfdd7739e32b5c3c288ecf9eb5fc86cae57ff7 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 8 Oct 2024 17:26:32 +0100 Subject: [PATCH 11/39] Test versions function --- tests/testthat/api/documentversions-f073ba.R | 13 +++++++++ tests/testthat/test-assets.R | 30 ++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 tests/testthat/api/documentversions-f073ba.R diff --git a/tests/testthat/api/documentversions-f073ba.R b/tests/testthat/api/documentversions-f073ba.R new file mode 100644 index 0000000..6ef2202 --- /dev/null +++ b/tests/testthat/api/documentversions-f073ba.R @@ -0,0 +1,13 @@ +structure(list(method = "GET", url = "https://api/documentversions?documentUuid=test_document_uuid", + status_code = 200L, headers = structure(list(Date = "Tue, 08 Oct 2024 16:01:54 GMT", + `Content-Type` = "application/json", `Transfer-Encoding` = "chunked", + Connection = "keep-alive", `X-CONNECT-MDC` = "apiOhBWBqzK", + `X-Frame-Options` = "deny", `X-XSS-Protection` = "1; mode=block", + `Cache-Control` = "no-cache, no-store", Expires = "0", + Pragma = "no-cache", `Strict-Transport-Security` = "max-age=31536000 ; includeSubDomains", + `Content-Security-Policy` = "script-src 'self' 'unsafe-inline'", + `X-Content-Type-Options` = "nosniff", `Referrer-Policy` = "strict-origin-when-cross-origin", + `Feature-Policy` = "vibrate 'none'; geolocation 'none'", + Authorization = "REDACTED", `Set-Cookie` = "REDACTED"), class = "httr2_headers"), + body = charToRaw("{\"content\":[{\"model\":\"DocumentVersion\",\"uuid\":\"test_version_uuid2\",\"version\":2,\"createdTime\":1728401404000,\"extension\":\"csv\",\"fileSize\":274,\"contentStatus\":\"AVAILABLE\",\"scanStatus\":\"COMPLETE\",\"annotationCount\":0,\"asset\":{\"model\":\"Asset\",\"uuid\":\"test_document_uuid\",\"name\":\"test_document_name\",\"workspaceUuid\":\"test_workspace_uuid\",\"type\":\"DOCUMENT\",\"annotationCount\":0,\"commentCount\":0,\"contentVersion\":2,\"createdTime\":1726043672000,\"modifiedTime\":1728401404000,\"extension\":\"csv\",\"fileSize\":274,\"isLocked\":false,\"isLockedInOffice\":false,\"modifiedByUuid\":\"test_modified_uuid\",\"previewStatus\":\"COMPLETED\",\"status\":\"COMPLETE\",\"mandatoryTagsPresent\":true,\"workspace\":{\"model\":\"Concise Workspace\",\"uuid\":\"test_workspace_uuid\",\"name\":\"test_workspace_name\",\"workgroupUuid\":\"test_workgroup_uuid\",\"ownerUuid\":\"test_owner_uuid\"},\"latestDocumentVersion\":{\"model\":\"Concise Document Version\",\"uuid\":\"test_version_uuid2\",\"assetUuid\":\"test_document_uuid\",\"version\":2},\"modifiedBy\":{\"model\":\"Concise User\",\"uuid\":\"test_modified_uuid\",\"email\":\"test_modified_email\",\"givenName\":\"test_modified_name1\",\"familyName\":\"test_modified_name2\",\"isMachineUser\":false}},\"createdBy\":{\"model\":\"Concise User\",\"uuid\":\"test_created_uuid\",\"email\":\"test_created_email\",\"givenName\":\"test_created_name1\",\"familyName\":\"test_created_name2\",\"isMachineUser\":false}},{\"model\":\"DocumentVersion\",\"uuid\":\"test_version_uuid1\",\"version\":1,\"createdTime\":1728401322000,\"extension\":\"csv\",\"fileSize\":274,\"contentStatus\":\"AVAILABLE\",\"scanStatus\":\"COMPLETE\",\"annotationCount\":0,\"asset\":{\"model\":\"Asset\",\"uuid\":\"test_document_uuid\",\"name\":\"test_document_name\",\"workspaceUuid\":\"test_workspace_uuid\",\"type\":\"DOCUMENT\",\"annotationCount\":0,\"commentCount\":0,\"contentVersion\":2,\"createdTime\":1726043672000,\"modifiedTime\":1728401404000,\"extension\":\"csv\",\"fileSize\":274,\"isLocked\":false,\"isLockedInOffice\":false,\"modifiedByUuid\":\"test_modified_uuid\",\"previewStatus\":\"COMPLETED\",\"status\":\"COMPLETE\",\"mandatoryTagsPresent\":true,\"workspace\":{\"model\":\"Concise Workspace\",\"uuid\":\"test_workspace_uuid\",\"name\":\"test_workspace_name\",\"workgroupUuid\":\"test_workgroup_uuid\",\"ownerUuid\":\"test_owner_uuid\"},\"latestDocumentVersion\":{\"model\":\"Concise Document Version\",\"uuid\":\"test_version_uuid2\",\"assetUuid\":\"test_asset_uuid\",\"version\":2},\"modifiedBy\":{\"model\":\"Concise User\",\"uuid\":\"test_modified_uuid\",\"email\":\"test_modified_email\",\"givenName\":\"test_modified_name1\",\"familyName\":\"test_modified_name2\",\"isMachineUser\":false}},\"createdBy\":{\"model\":\"Concise User\",\"uuid\":\"test_created_uuid\",\"email\":\"test_created_email\",\"givenName\":\"test_created_name1\",\"familyName\":\"test_created_name2\",\"isMachineUser\":false}}],\"metadata\":{\"totalElements\":2,\"totalPages\":1,\"page\":0,\"offset\":0}}"), + cache = new.env(parent = emptyenv())), class = "httr2_response") diff --git a/tests/testthat/test-assets.R b/tests/testthat/test-assets.R index f9713d2..5a9f607 100644 --- a/tests/testthat/test-assets.R +++ b/tests/testthat/test-assets.R @@ -85,6 +85,36 @@ with_mock_api({ }) +# versions ---- + +without_internet({ + + test_that("Valid request", { + + expect_GET( + versions(document_uuid = "test_document_uuid"), + paste0("https://secure.objectiveconnect.co.uk/publicapi/1/", + "documentversions?documentUuid=test_document_uuid") + ) + + }) + +}) + +with_mock_api({ + + test_that("Function returns dataframe", { + + expect_s3_class( + versions(document_uuid = "test_document_uuid"), + "data.frame" + ) + + }) + +}) + + # delete_asset ---- without_internet({ From 81937783716900adb980fb6aea6a161c104bc362 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 8 Oct 2024 17:34:40 +0100 Subject: [PATCH 12/39] Move versions fn to R/documents.R --- R/assets.R | 50 --------------------------------- R/documents.R | 49 ++++++++++++++++++++++++++++++++ man/versions.Rd | 2 +- tests/testthat/test-assets.R | 30 -------------------- tests/testthat/test-documents.R | 28 ++++++++++++++++++ 5 files changed, 78 insertions(+), 81 deletions(-) create mode 100644 R/documents.R create mode 100644 tests/testthat/test-documents.R diff --git a/R/assets.R b/R/assets.R index 4791da2..d071cc9 100644 --- a/R/assets.R +++ b/R/assets.R @@ -63,39 +63,6 @@ asset_info <- function(asset_uuid, } -#' Get data frame of document versions -#' -#' @param document_uuid UUID of document (asset) -#' @param page Page number of responses to return (0..N). -#' @param size Number of results to be returned per page. -#' @inheritParams objr -#' -#' @return Data frame -#' -#' @export - -versions <- function(document_uuid, - page = NULL, - size = NULL, - use_proxy = FALSE) { - - response <- objr( - endpoint = "documentversions", - url_query = list(documentUuid = document_uuid, - page = page, - size = size), - use_proxy = use_proxy - ) - - content <- - httr2::resp_body_json(response)$content |> - lapply(\(x) data.frame(versions_info_list(x))) - - Reduce(dplyr::bind_rows, content) - -} - - #' Delete an asset #' #' @param asset_uuid UUID of asset @@ -178,20 +145,3 @@ asset_info_list <- function(x) { ) } - -versions_info_list <- function(x) { - - list( - asset_name = x[["asset"]]$name, - asset_ext = na_if_null(x$extension), - asset_uuid = x[["asset"]]$uuid, - version = x$version, - version_uuid = x$uuid, - created_date = na_if_null(convert_from_epoch(x$createdTime)), - created_by = paste(na_if_null(x[["createdBy"]]$givenName), - na_if_null(x[["createdBy"]]$familyName)), - workspace_name = x[["asset"]][["workspace"]]$name, - workspace_uuid = x[["asset"]][["workspace"]]$uuid - ) - -} diff --git a/R/documents.R b/R/documents.R new file mode 100644 index 0000000..c63a490 --- /dev/null +++ b/R/documents.R @@ -0,0 +1,49 @@ +#' Get data frame of document versions +#' +#' @param document_uuid UUID of document (asset) +#' @param page Page number of responses to return (0..N). +#' @param size Number of results to be returned per page. +#' @inheritParams objr +#' +#' @return Data frame +#' +#' @export + +versions <- function(document_uuid, + page = NULL, + size = NULL, + use_proxy = FALSE) { + + response <- objr( + endpoint = "documentversions", + url_query = list(documentUuid = document_uuid, + page = page, + size = size), + use_proxy = use_proxy + ) + + content <- + httr2::resp_body_json(response)$content |> + lapply(\(x) data.frame(versions_info_list(x))) + + Reduce(dplyr::bind_rows, content) + +} + + +versions_info_list <- function(x) { + + list( + asset_name = x[["asset"]]$name, + asset_ext = na_if_null(x$extension), + asset_uuid = x[["asset"]]$uuid, + version = x$version, + version_uuid = x$uuid, + created_date = na_if_null(convert_from_epoch(x$createdTime)), + created_by = paste(na_if_null(x[["createdBy"]]$givenName), + na_if_null(x[["createdBy"]]$familyName)), + workspace_name = x[["asset"]][["workspace"]]$name, + workspace_uuid = x[["asset"]][["workspace"]]$uuid + ) + +} diff --git a/man/versions.Rd b/man/versions.Rd index b091b02..2946bb7 100644 --- a/man/versions.Rd +++ b/man/versions.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/assets.R +% Please edit documentation in R/documents.R \name{versions} \alias{versions} \title{Get data frame of document versions} diff --git a/tests/testthat/test-assets.R b/tests/testthat/test-assets.R index 5a9f607..f9713d2 100644 --- a/tests/testthat/test-assets.R +++ b/tests/testthat/test-assets.R @@ -85,36 +85,6 @@ with_mock_api({ }) -# versions ---- - -without_internet({ - - test_that("Valid request", { - - expect_GET( - versions(document_uuid = "test_document_uuid"), - paste0("https://secure.objectiveconnect.co.uk/publicapi/1/", - "documentversions?documentUuid=test_document_uuid") - ) - - }) - -}) - -with_mock_api({ - - test_that("Function returns dataframe", { - - expect_s3_class( - versions(document_uuid = "test_document_uuid"), - "data.frame" - ) - - }) - -}) - - # delete_asset ---- without_internet({ diff --git a/tests/testthat/test-documents.R b/tests/testthat/test-documents.R new file mode 100644 index 0000000..1ab63c7 --- /dev/null +++ b/tests/testthat/test-documents.R @@ -0,0 +1,28 @@ +# versions ---- + +without_internet({ + + test_that("Valid request", { + + expect_GET( + versions(document_uuid = "test_document_uuid"), + paste0("https://secure.objectiveconnect.co.uk/publicapi/1/", + "documentversions?documentUuid=test_document_uuid") + ) + + }) + +}) + +with_mock_api({ + + test_that("Function returns dataframe", { + + expect_s3_class( + versions(document_uuid = "test_document_uuid"), + "data.frame" + ) + + }) + +}) From 3dc313f2af1d6eb694550fe4b9c7337161bd83a3 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 8 Oct 2024 17:51:35 +0100 Subject: [PATCH 13/39] Add fn to rollback doc to previous version --- NAMESPACE | 1 + R/documents.R | 29 +++++++++++++++++++++++++++++ man/rollback_to_version.Rd | 18 ++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 man/rollback_to_version.Rd diff --git a/NAMESPACE b/NAMESPACE index c2565c4..c7a2e79 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -15,6 +15,7 @@ export(participant_bypass_2fa) export(participants) export(read_data) export(rename_asset) +export(rollback_to_version) export(upload_file) export(upload_file_version) export(versions) diff --git a/R/documents.R b/R/documents.R index c63a490..25926b7 100644 --- a/R/documents.R +++ b/R/documents.R @@ -31,6 +31,35 @@ versions <- function(document_uuid, } +#' Rollback a document to a previous version +#' +#' @param document_uuid UUID of document (asset) +#' @param version_uuid UUID of version to rollback to +#' @inheritParams objr +#' +#' @export + +rollback_to_version <- function(document_uuid, + version_uuid, + use_proxy = FALSE) { + + response <- objr( + endpoint = "documents", + method = "PUT", + url_path = list(document_uuid, "rollback"), + body = list(targetVersionUuid = version_uuid), + use_proxy = use_proxy + ) + + if (response$status_code == 204) { + cli::cli_alert_success("Document rollback successful.") + } + + invisible(response) + +} + + versions_info_list <- function(x) { list( diff --git a/man/rollback_to_version.Rd b/man/rollback_to_version.Rd new file mode 100644 index 0000000..1a63f90 --- /dev/null +++ b/man/rollback_to_version.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/documents.R +\name{rollback_to_version} +\alias{rollback_to_version} +\title{Rollback a document to a previous version} +\usage{ +rollback_to_version(document_uuid, version_uuid, use_proxy = FALSE) +} +\arguments{ +\item{document_uuid}{UUID of document (asset)} + +\item{version_uuid}{UUID of version to rollback to} + +\item{use_proxy}{Logical to indicate whether to use proxy} +} +\description{ +Rollback a document to a previous version +} From 27905e61f35d190315926e8940859b7e4b59e365 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 8 Oct 2024 18:08:04 +0100 Subject: [PATCH 14/39] Test rollback_to_version() --- _pkgdown.yml | 5 +++ .../test_document/rollback-2696d1-PUT.R | 11 ++++++ tests/testthat/test-documents.R | 39 +++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 tests/testthat/api/documents/test_document/rollback-2696d1-PUT.R diff --git a/_pkgdown.yml b/_pkgdown.yml index a154d9a..951c8c2 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -64,6 +64,11 @@ reference: - rename_asset - delete_asset +- subtitle: Document versions + contents: + - versions + - rollback_to_version + - subtitle: Download contents: - download_file diff --git a/tests/testthat/api/documents/test_document/rollback-2696d1-PUT.R b/tests/testthat/api/documents/test_document/rollback-2696d1-PUT.R new file mode 100644 index 0000000..f1a926e --- /dev/null +++ b/tests/testthat/api/documents/test_document/rollback-2696d1-PUT.R @@ -0,0 +1,11 @@ +structure(list(method = "PUT", url = "https://api/documents/test_document/rollback", + status_code = 204L, headers = structure(list(Date = "Tue, 08 Oct 2024 17:01:23 GMT", + Connection = "keep-alive", `X-CONNECT-MDC` = "apiHWNHMifi", + `X-Frame-Options` = "deny", `X-XSS-Protection` = "1; mode=block", + `Cache-Control` = "no-cache, no-store", Expires = "0", + Pragma = "no-cache", `Strict-Transport-Security` = "max-age=31536000 ; includeSubDomains", + `Content-Security-Policy` = "script-src 'self' 'unsafe-inline'", + `X-Content-Type-Options` = "nosniff", `Referrer-Policy` = "strict-origin-when-cross-origin", + `Feature-Policy` = "vibrate 'none'; geolocation 'none'", + Authorization = "REDACTED", `Set-Cookie` = "REDACTED"), class = "httr2_headers"), + body = raw(0), cache = new.env(parent = emptyenv())), class = "httr2_response") diff --git a/tests/testthat/test-documents.R b/tests/testthat/test-documents.R index 1ab63c7..4a9292e 100644 --- a/tests/testthat/test-documents.R +++ b/tests/testthat/test-documents.R @@ -26,3 +26,42 @@ with_mock_api({ }) }) + + +# rollback_to_version ---- + +without_internet({ + + test_that("Valid request", { + + expect_PUT( + rollback_to_version(document_uuid = "test_document_uuid", + version_uuid = "test_version_uuid"), + paste0("https://secure.objectiveconnect.co.uk/publicapi/1/", + "documents/test_document_uuid/rollback"), + "{\"targetVersionUuid\":\"test_version_uuid\"}" + ) + + }) + +}) + +with_mock_api({ + + test_that("Function returns invisible", { + + expect_invisible( + suppressMessages(rollback_to_version(document_uuid = "test_document", + version_uuid = "test_version")) + ) + + }) + + test_that("Function returns success message", { + + expect_message(rollback_to_version(document_uuid = "test_document", + version_uuid = "test_version")) + + }) + +}) From 251eabfa2b87b596d24831ea40bdbf64e6bad39a Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Wed, 9 Oct 2024 14:53:07 +0100 Subject: [PATCH 15/39] Add function to download document version --- NAMESPACE | 1 + R/download.R | 38 +++++++++++++++++ R/utils_download-upload.R | 82 ++++++++++++++++++++++++++++++++++++ _pkgdown.yml | 1 + man/download_file_version.Rd | 29 +++++++++++++ 5 files changed, 151 insertions(+) create mode 100644 man/download_file_version.Rd diff --git a/NAMESPACE b/NAMESPACE index c7a2e79..44c3042 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -7,6 +7,7 @@ export(comments) export(create_folder) export(delete_asset) export(download_file) +export(download_file_version) export(my_user_id) export(new_reply) export(new_thread) diff --git a/R/download.R b/R/download.R index f82e532..25f4b58 100644 --- a/R/download.R +++ b/R/download.R @@ -36,6 +36,44 @@ download_file <- function(document_uuid, } +#' Download a document version and save to disk +#' +#' @param document_uuid UUID of existing document version +#' @param folder Folder to save downloaded file to +#' @param overwrite Logical to indicate whether file should be overwritten if +#' already exists. +#' @inheritParams objr +#' +#' @return An httr2 [httr2::response()][response] (invisibly) +#' +#' @export + +download_file_version <- function(document_uuid, + folder, + overwrite = FALSE, + use_proxy = FALSE) { + + path <- create_file(folder) + + response <- objr( + endpoint = "documentversions", + url_path = list(document_uuid, "download"), + method = "GET", + path = path, + use_proxy = use_proxy + ) + + new_path <- rename_file(path, response, overwrite = overwrite) + + if (httr2::resp_status(response) == 200) { + cli::cli_alert_success("File downloaded: {new_path}.") + } + + invisible(response) + +} + + #' Read a data file into R #' #' @param document_uuid UUID of existing document diff --git a/R/utils_download-upload.R b/R/utils_download-upload.R index 37834af..d589948 100644 --- a/R/utils_download-upload.R +++ b/R/utils_download-upload.R @@ -101,3 +101,85 @@ check_file_exists <- function(path, invisible(path) } + + +#' Create file for download +#' +#' @param folder File path of folder to create file in +#' +#' @return File path of file created (invisibly). +#' +#' @noRd + +create_file <- function(folder, + error_call = rlang::caller_env()) { + + folder <- gsub("\\/*$", "", folder) + + if (!dir.exists(folder)) { + + cli::cli_abort( + "Can't find folder at {.code {folder}}.", + call = error_call + ) + + } + + path <- tempfile(tmpdir = folder) + + file.create(path) + + invisible(path) + +} + + +#' Rename downloaded file +#' +#' @param temp_path Temporary path of file downloaded. +#' @param response An httr2 [httr2::response()][response]. Must contain +#' 'Content-Disposition' header. +#' @param overwrite Logical to indicate whether to overwrite file if already +#' exists. +#' +#' @return File path of renamed file (invisibly). +#' +#' @noRd + +rename_file <- function(temp_path, + response, + overwrite, + error_call = rlang::caller_env(), + error_arg = rlang::caller_arg(overwrite)) { + + if (!httr2::resp_header_exists(response, "Content-Disposition")) { + cli::cli_abort( + "Response must contain `Content-Disposition` header.", + call = error_call + ) + } + + cont_disp <- httr2::resp_header(response, "Content-Disposition") + + file_name <- regmatches( + cont_disp, + m = regexpr("(?<=filename=\\\").*(?=\\\")", cont_disp, perl = T) + ) + + new_path <- file.path(dirname(temp_path), file_name) + + if (file.exists(new_path) && overwrite == FALSE) { + cli::cli_abort( + c( + "File already exists: {.path {new_path}}.", + "i" = "To overwrite, set {.code {error_arg} = TRUE}." + ), + call = error_call + ) + } + + file.rename(from = temp_path, to = new_path) + + invisible(new_path) + +} diff --git a/_pkgdown.yml b/_pkgdown.yml index 951c8c2..29755a0 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -72,6 +72,7 @@ reference: - subtitle: Download contents: - download_file + - download_file_version - read_data - subtitle: Upload diff --git a/man/download_file_version.Rd b/man/download_file_version.Rd new file mode 100644 index 0000000..5066557 --- /dev/null +++ b/man/download_file_version.Rd @@ -0,0 +1,29 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/download.R +\name{download_file_version} +\alias{download_file_version} +\title{Download a document version and save to disk} +\usage{ +download_file_version( + document_uuid, + folder, + overwrite = FALSE, + use_proxy = FALSE +) +} +\arguments{ +\item{document_uuid}{UUID of existing document version} + +\item{folder}{Folder to save downloaded file to} + +\item{overwrite}{Logical to indicate whether file should be overwritten if +already exists.} + +\item{use_proxy}{Logical to indicate whether to use proxy} +} +\value{ +An httr2 \link[=response]{httr2::response()} (invisibly) +} +\description{ +Download a document version and save to disk +} From 9c2f1e294226ff2cf4a3b0942d5475045bb5fd12 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Wed, 9 Oct 2024 15:12:33 +0100 Subject: [PATCH 16/39] Use new utils fns for download_file() --- R/download.R | 18 ++++----- man/download_file.Rd | 5 ++- man/download_file_version.Rd | 2 +- tests/testthat/test-download.R | 67 ++++++++++++++++++++-------------- 4 files changed, 53 insertions(+), 39 deletions(-) diff --git a/R/download.R b/R/download.R index 25f4b58..846c77f 100644 --- a/R/download.R +++ b/R/download.R @@ -2,6 +2,8 @@ #' #' @param document_uuid UUID of existing document #' @param folder Folder to save downloaded file to +#' @param overwrite Logical to indicate whether file should be overwritten if +#' already exists. Defaults to `FALSE`. #' @inheritParams objr #' #' @return An httr2 [httr2::response()][response] (invisibly) @@ -10,14 +12,10 @@ download_file <- function(document_uuid, folder, + overwrite = FALSE, use_proxy = FALSE) { - doc_info <- asset_info(document_uuid) - - path <- file.path(folder, paste0(doc_info$asset_name, ".", - doc_info$asset_ext)) - - file.create(path) + path <- create_file(folder) response <- objr( endpoint = "documents", @@ -27,8 +25,10 @@ download_file <- function(document_uuid, use_proxy = use_proxy ) + new_path <- rename_file(path, response, overwrite = overwrite) + if (httr2::resp_status(response) == 200) { - cli::cli_alert_success("File downloaded: {path}.") + cli::cli_alert_success("File downloaded: {.path {new_path}}.") } invisible(response) @@ -41,7 +41,7 @@ download_file <- function(document_uuid, #' @param document_uuid UUID of existing document version #' @param folder Folder to save downloaded file to #' @param overwrite Logical to indicate whether file should be overwritten if -#' already exists. +#' already exists. Defaults to `FALSE`. #' @inheritParams objr #' #' @return An httr2 [httr2::response()][response] (invisibly) @@ -66,7 +66,7 @@ download_file_version <- function(document_uuid, new_path <- rename_file(path, response, overwrite = overwrite) if (httr2::resp_status(response) == 200) { - cli::cli_alert_success("File downloaded: {new_path}.") + cli::cli_alert_success("File downloaded: {.path new_path}.") } invisible(response) diff --git a/man/download_file.Rd b/man/download_file.Rd index 17aee67..596e8df 100644 --- a/man/download_file.Rd +++ b/man/download_file.Rd @@ -4,13 +4,16 @@ \alias{download_file} \title{Download a file and save to disk} \usage{ -download_file(document_uuid, folder, use_proxy = FALSE) +download_file(document_uuid, folder, overwrite = FALSE, use_proxy = FALSE) } \arguments{ \item{document_uuid}{UUID of existing document} \item{folder}{Folder to save downloaded file to} +\item{overwrite}{Logical to indicate whether file should be overwritten if +already exists. Defaults to \code{FALSE}.} + \item{use_proxy}{Logical to indicate whether to use proxy} } \value{ diff --git a/man/download_file_version.Rd b/man/download_file_version.Rd index 5066557..242e244 100644 --- a/man/download_file_version.Rd +++ b/man/download_file_version.Rd @@ -17,7 +17,7 @@ download_file_version( \item{folder}{Folder to save downloaded file to} \item{overwrite}{Logical to indicate whether file should be overwritten if -already exists.} +already exists. Defaults to \code{FALSE}.} \item{use_proxy}{Logical to indicate whether to use proxy} } diff --git a/tests/testthat/test-download.R b/tests/testthat/test-download.R index 60db0d3..c1f4965 100644 --- a/tests/testthat/test-download.R +++ b/tests/testthat/test-download.R @@ -1,44 +1,54 @@ # download_file ---- -without_internet({ - - test_that("Valid request", { +with_file("test", { - expect_GET( - download_file(document_uuid = "test_document", - folder = "test"), - "https://secure.objectiveconnect.co.uk/publicapi/1/assets/test_document" - ) - - }) + dir.create("test") -}) + without_internet({ -with_mock_api({ + test_that("Valid request", { - test_that("Function returns invisible", { + expect_GET( + download_file(document_uuid = "test_document", + folder = "test"), + paste0("https://secure.objectiveconnect.co.uk/publicapi/1/", + "documents/test_document/download") + ) - expect_invisible( - suppressMessages(download_file(document_uuid = "test_document", - folder = tempdir())) - ) + }) }) - test_that("Function returns success message", { + with_mock_api({ - expect_message(download_file(document_uuid = "test_document", - folder = tempdir())) + test_that("Function returns invisible", { + expect_invisible( + suppressMessages(download_file(document_uuid = "test_document", + folder = "test")) + ) + }) - }) + test_that("Success message returned", { + expect_message(download_file(document_uuid = "test_document", + folder = "test", + overwrite = TRUE)) + }) - test_that("Function creates new file", { - suppressMessages(download_file(document_uuid = "test_document", - folder = tempdir(check = TRUE))) - expect_true(file.exists(paste0(tempdir(), "/test_document_name.txt"))) - }) + test_that("New file created", { + suppressMessages(download_file(document_uuid = "test_document", + folder = "test", + overwrite = TRUE)) + expect_true(file.exists(paste0("test", "/test_document_name.txt"))) + }) + + test_that("Error if file already exists and `overwrite = FALSE`", { + expect_error( + download_file(document_uuid = "test_document", + folder = "test") + ) + }) - file.remove(paste0(tempdir(), "/test_document_name.txt")) + }) }) @@ -51,7 +61,8 @@ without_internet({ expect_GET( read_data(document_uuid = "test_document"), - "https://secure.objectiveconnect.co.uk/publicapi/1/assets/test_document" + paste0("https://secure.objectiveconnect.co.uk/publicapi/1/documents/", + "test_document/download") ) }) From 00fc5c85e69c7e19b0e863e0ddb1ebbed383b54f Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Wed, 9 Oct 2024 15:16:33 +0100 Subject: [PATCH 17/39] Use magrittr pipe instead of base (to avoid R version dependency) --- DESCRIPTION | 1 + NAMESPACE | 2 ++ R/assets.R | 6 +++--- R/comments.R | 2 +- R/create_folder.R | 2 +- R/documents.R | 2 +- R/objr.R | 22 +++++++++++----------- R/participants.R | 2 +- R/utils-pipe.R | 14 ++++++++++++++ man/pipe.Rd | 20 ++++++++++++++++++++ 10 files changed, 55 insertions(+), 18 deletions(-) create mode 100644 R/utils-pipe.R create mode 100644 man/pipe.Rd diff --git a/DESCRIPTION b/DESCRIPTION index 64ea23e..096b399 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -16,6 +16,7 @@ Imports: curl, dplyr, httr2, + magrittr, rlang, rstudioapi Suggests: diff --git a/NAMESPACE b/NAMESPACE index 44c3042..41939ad 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,5 +1,6 @@ # Generated by roxygen2: do not edit by hand +export("%>%") export(allow_bypass_2fa) export(asset_info) export(assets) @@ -23,3 +24,4 @@ export(versions) export(workspaces) export(write_data) export(write_data_version) +importFrom(magrittr,"%>%") diff --git a/R/assets.R b/R/assets.R index d071cc9..335d830 100644 --- a/R/assets.R +++ b/R/assets.R @@ -31,7 +31,7 @@ assets <- function(workspace_uuid, ) content <- - httr2::resp_body_json(response)$content |> + httr2::resp_body_json(response)$content %>% lapply(\(x) data.frame(asset_info_list(x))) Reduce(dplyr::bind_rows, content) @@ -55,7 +55,7 @@ asset_info <- function(asset_uuid, endpoint = "assets", url_path = list(asset_uuid), use_proxy = use_proxy - ) |> + ) %>% httr2::resp_body_json() asset_info_list(response) @@ -81,7 +81,7 @@ delete_asset <- function(asset_uuid, method = "DELETE", url_path = list(asset_uuid), use_proxy = use_proxy - ) |> + ) %>% httr2::resp_body_json() if (tolower(response$status) == "complete") { diff --git a/R/comments.R b/R/comments.R index 3d341de..8ac2649 100644 --- a/R/comments.R +++ b/R/comments.R @@ -37,7 +37,7 @@ comments <- function(created_after = NULL, ) content <- - httr2::resp_body_json(response)$content |> + httr2::resp_body_json(response)$content %>% lapply( \(content) { data.frame( diff --git a/R/create_folder.R b/R/create_folder.R index 3496469..30b7b4d 100644 --- a/R/create_folder.R +++ b/R/create_folder.R @@ -26,7 +26,7 @@ create_folder <- function(folder_name, description = description ), use_proxy = use_proxy - ) |> + ) %>% httr2::resp_body_json() if (tolower(response$status) == "complete") { diff --git a/R/documents.R b/R/documents.R index 25926b7..02d0dd6 100644 --- a/R/documents.R +++ b/R/documents.R @@ -23,7 +23,7 @@ versions <- function(document_uuid, ) content <- - httr2::resp_body_json(response)$content |> + httr2::resp_body_json(response)$content %>% lapply(\(x) data.frame(versions_info_list(x))) Reduce(dplyr::bind_rows, content) diff --git a/R/objr.R b/R/objr.R index 8e9d7cf..f15331f 100644 --- a/R/objr.R +++ b/R/objr.R @@ -33,15 +33,15 @@ objr <- function(endpoint, # Build request request <- - httr2::request("https://secure.objectiveconnect.co.uk/publicapi/1") |> - httr2::req_url_path_append(endpoint) |> - httr2::req_method(method) |> + httr2::request("https://secure.objectiveconnect.co.uk/publicapi/1") %>% + httr2::req_url_path_append(endpoint) %>% + httr2::req_method(method) %>% httr2::req_headers(accept = accept, - `content-type` = content_type) |> - objr_auth() |> + `content-type` = content_type) %>% + objr_auth() %>% httr2::req_user_agent( "objr (https://scotgovanalysis.github.io/objr/)" - ) |> + ) %>% httr2::req_error(body = error) # Modify the URL path @@ -75,9 +75,9 @@ objr <- function(endpoint, response <- httr2::req_perform(request, path = path) # Return response - response |> - httr2::resp_check_status() |> - store_token() |> + response %>% + httr2::resp_check_status() %>% + store_token() %>% check_pages() } @@ -96,12 +96,12 @@ objr <- function(endpoint, #' #' @examples #' \dontrun{ -#' httr2::request("http://example.com") |> +#' httr2::request("http://example.com") %>% #' objr_auth() #' } #' #' token <- "test" -#' httr2::request("http://example.com") |> objr_auth() +#' httr2::request("http://example.com") %>% objr_auth() #' #' @noRd diff --git a/R/participants.R b/R/participants.R index 09d5b8f..afce204 100644 --- a/R/participants.R +++ b/R/participants.R @@ -16,7 +16,7 @@ participants <- function(workspace_uuid, use_proxy = FALSE) { ) content <- - httr2::resp_body_json(response)$content |> + httr2::resp_body_json(response)$content %>% lapply( \(content) { data.frame( diff --git a/R/utils-pipe.R b/R/utils-pipe.R new file mode 100644 index 0000000..fd0b1d1 --- /dev/null +++ b/R/utils-pipe.R @@ -0,0 +1,14 @@ +#' Pipe operator +#' +#' See \code{magrittr::\link[magrittr:pipe]{\%>\%}} for details. +#' +#' @name %>% +#' @rdname pipe +#' @keywords internal +#' @export +#' @importFrom magrittr %>% +#' @usage lhs \%>\% rhs +#' @param lhs A value or the magrittr placeholder. +#' @param rhs A function call using the magrittr semantics. +#' @return The result of calling `rhs(lhs)`. +NULL diff --git a/man/pipe.Rd b/man/pipe.Rd new file mode 100644 index 0000000..a648c29 --- /dev/null +++ b/man/pipe.Rd @@ -0,0 +1,20 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils-pipe.R +\name{\%>\%} +\alias{\%>\%} +\title{Pipe operator} +\usage{ +lhs \%>\% rhs +} +\arguments{ +\item{lhs}{A value or the magrittr placeholder.} + +\item{rhs}{A function call using the magrittr semantics.} +} +\value{ +The result of calling \code{rhs(lhs)}. +} +\description{ +See \code{magrittr::\link[magrittr:pipe]{\%>\%}} for details. +} +\keyword{internal} From 6a7096fb7af292eb050a125d094c6fedb3676111 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Wed, 9 Oct 2024 15:35:10 +0100 Subject: [PATCH 18/39] Set default value for overwrite --- R/utils_download-upload.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/utils_download-upload.R b/R/utils_download-upload.R index d589948..b7053a6 100644 --- a/R/utils_download-upload.R +++ b/R/utils_download-upload.R @@ -140,7 +140,7 @@ create_file <- function(folder, #' @param response An httr2 [httr2::response()][response]. Must contain #' 'Content-Disposition' header. #' @param overwrite Logical to indicate whether to overwrite file if already -#' exists. +#' exists. Defaults to `FALSE`. #' #' @return File path of renamed file (invisibly). #' @@ -148,7 +148,7 @@ create_file <- function(folder, rename_file <- function(temp_path, response, - overwrite, + overwrite = FALSE, error_call = rlang::caller_env(), error_arg = rlang::caller_arg(overwrite)) { From 86a761002600ea55089af67c8eb58fec67bb1f21 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 10 Oct 2024 11:04:41 +0100 Subject: [PATCH 19/39] Add extra error checks --- R/utils_download-upload.R | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/R/utils_download-upload.R b/R/utils_download-upload.R index b7053a6..334a431 100644 --- a/R/utils_download-upload.R +++ b/R/utils_download-upload.R @@ -150,7 +150,18 @@ rename_file <- function(temp_path, response, overwrite = FALSE, error_call = rlang::caller_env(), - error_arg = rlang::caller_arg(overwrite)) { + error_arg_overwrite = rlang::caller_arg(overwrite), + error_arg_path = rlang::caller_arg(temp_path)) { + + if (!file.exists(temp_path)) { + cli::cli_abort( + c( + "{.arg {error_arg_path}} must exist.", + "No file found: {.path {temp_path}}." + ), + call = error_call + ) + } if (!httr2::resp_header_exists(response, "Content-Disposition")) { cli::cli_abort( @@ -166,13 +177,23 @@ rename_file <- function(temp_path, m = regexpr("(?<=filename=\\\").*(?=\\\")", cont_disp, perl = T) ) + if (length(file_name) == 0) { + cli::cli_abort( + c( + "`Content-Disposition` header must contain file name.", + "i" = "Expected format: filename=\\\"my_file.csv\\\"" + ), + call = error_call + ) + } + new_path <- file.path(dirname(temp_path), file_name) if (file.exists(new_path) && overwrite == FALSE) { cli::cli_abort( c( "File already exists: {.path {new_path}}.", - "i" = "To overwrite, set {.code {error_arg} = TRUE}." + "i" = "To overwrite, set {.code {error_arg_overwrite} = TRUE}." ), call = error_call ) From 7cf5e428a4bc16b2057388921ff6692cc1b1de78 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 10 Oct 2024 11:43:21 +0100 Subject: [PATCH 20/39] Test create_file and rename_file --- tests/testthat/test-utils_download-upload.R | 121 ++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/tests/testthat/test-utils_download-upload.R b/tests/testthat/test-utils_download-upload.R index 42cf84b..ef0cda1 100644 --- a/tests/testthat/test-utils_download-upload.R +++ b/tests/testthat/test-utils_download-upload.R @@ -134,3 +134,124 @@ with_file("test", { }) }) + + +# create_file ---- + +test_that("Error if folder doesn't exist", { + + expect_error(create_file("folder")) + +}) + +with_tempdir({ + + test_that("Returns invisibly", { + expect_invisible(create_file(tempdir())) + }) + + test_that("Returns file path in folder", { + + expect_type((create_file(tempdir())), "character") + + # Use gsub as dirname returns a path using / (instead of \\ like tempdir()) + expect_equal(dirname(create_file(tempdir())), + gsub("\\\\", "/", tempdir())) + + }) + +}) + + +# rename_file ---- + +resp_expected <- httr2::response( + headers = list(`Content-Disposition` = "filename=\"new_name.csv\"") +) + +resp_bad_format <- httr2::response( + headers = list(`Content-Disposition` = "x") +) + +resp_no_header <- httr2::response( + headers = list(x = 1) +) + +test_that("Error if temp_file doesn't exist", { + expect_error(rename_file(temp_path = "test", resp_expected)) +}) + +with_tempfile("test", { + + file.create(test) + + test_that("Error if httr2 resp doesn't have expected header", { + expect_error(rename_file(test, resp_no_header)) + expect_error(rename_file(test, resp_bad_format)) + }) + +}) + +with_tempfile(c("test", "new"), { + + file.create(test) + file.create(new) + + resp_new <- httr2::response( + headers = list( + `Content-Disposition` = paste0("filename=\"", basename(new), "\"") + ) + ) + + test_that("Error if file already exists", { + expect_error(rename_file(test, resp_new)) + }) + + test_that("No error if overwrite = TRUE", { + expect_no_error(rename_file(test, resp_new, overwrite = TRUE)) + }) + + unlink(file.path(dirname(test), "new_name.csv")) + +}) + +with_tempfile("test", { + + file.create(test) + + test_that("File is renamed", { + rename_file(test, resp_expected) + expect_false(file.exists(test)) + expect_true(file.exists(file.path(dirname(test), "new_file.csv"))) + }) + + unlink(file.path(dirname(test), "new_name.csv")) + +}) + +with_tempfile("test", { + + file.create(test) + + test_that("Returns invisibly", { + expect_invisible(rename_file(test, resp_expected)) + }) + + unlink(file.path(dirname(test), "new_name.csv")) + +}) + +with_tempfile("test", { + + file.create(test) + + test_that("New file path returned", { + expect_equal( + (rename_file(test, resp_expected)), + file.path(dirname(test), "new_name.csv") + ) + }) + + unlink(file.path(dirname(test), "new_name.csv")) + +}) From 66a23aa52dbb9d622f9c5867a41d443bf631f83d Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 10 Oct 2024 13:10:10 +0100 Subject: [PATCH 21/39] Use with_tempfile() --- tests/testthat/test-utils_download-upload.R | 54 ++++++++++----------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/tests/testthat/test-utils_download-upload.R b/tests/testthat/test-utils_download-upload.R index ef0cda1..70b2f35 100644 --- a/tests/testthat/test-utils_download-upload.R +++ b/tests/testthat/test-utils_download-upload.R @@ -74,59 +74,55 @@ test_that("Error if `file_type` not supplied", { # read_temp ---- -path <- tempfile(fileext = c(".rds", ".rds", ".csv", ".txt")) +with_tempfile("test_rds", fileext = ".rds", { -readr::write_rds(test_data, path[1]) -readr::write_rds("x", path[2]) -readr::write_csv(test_data, path[3]) + test_that("Correct value returned", { -test_that("Correct value returned", { + readr::write_rds(test_data, test_rds) + expect_s3_class(read_temp(test_rds), "data.frame") + + readr::write_rds("x", test_rds) + expect_type(read_temp(test_rds), "character") - expect_s3_class(read_temp(path[1]), "data.frame") - expect_type(read_temp(path[2]), "character") - expect_s3_class(suppressMessages(read_temp(path[3])), "data.frame") + }) }) -test_that("Additional arguments passed to write_fn", { +with_tempfile("temp_csv", fileext = ".csv", { - x4 <- suppressMessages( - write_temp(test_data, - file_name = "test4", - file_type = "csv", - na = "MISSING") - ) + readr::write_csv(test_data, temp_csv) - file <- readr::read_csv(x4, show_col_types = FALSE) + test_that("Additional arguments passed to read_fn", { + x <- read_temp(temp_csv, id = "id", show_col_types = FALSE) + expect_true("id" %in% names(x)) + }) - expect_equal(unique(file$y), "MISSING") +}) - unlink(x4) +with_tempfile("test_txt", fileext = ".txt", { -}) + file.create(test_txt) -writeLines("test", path[4]) + test_that("Error if file not accepted type", { + expect_error(read_temp(test_txt)) + }) -test_that("Error if file not accepted type", { - expect_error(read_temp(test_data)) }) -unlink(path) - # check_file_exists ---- -with_file("test", { +with_tempfile("test", { - file.create("test") + file.create(test) test_that("Returns invisibly", { - expect_invisible(check_file_exists("test")) + expect_invisible(check_file_exists(test)) }) test_that("Returns path", { - x <- check_file_exists("test") - expect_equal(x, "test") + x <- check_file_exists(test) + expect_equal(x, test) }) test_that("Error if path doesn't exist", { From f2faa837001ed4a8176d2c6286c99a1d85722b7b Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 10 Oct 2024 17:05:02 +0100 Subject: [PATCH 22/39] Fix incorrect file name --- tests/testthat/test-utils_download-upload.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-utils_download-upload.R b/tests/testthat/test-utils_download-upload.R index 70b2f35..9e9e98d 100644 --- a/tests/testthat/test-utils_download-upload.R +++ b/tests/testthat/test-utils_download-upload.R @@ -218,7 +218,7 @@ with_tempfile("test", { test_that("File is renamed", { rename_file(test, resp_expected) expect_false(file.exists(test)) - expect_true(file.exists(file.path(dirname(test), "new_file.csv"))) + expect_true(file.exists(file.path(dirname(test), "new_name.csv"))) }) unlink(file.path(dirname(test), "new_name.csv")) From 693bd4b01fa334987afa237c6529d87db453240a Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 10 Oct 2024 17:05:15 +0100 Subject: [PATCH 23/39] Add more context to error message --- R/utils_download-upload.R | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/R/utils_download-upload.R b/R/utils_download-upload.R index 334a431..286e81b 100644 --- a/R/utils_download-upload.R +++ b/R/utils_download-upload.R @@ -87,12 +87,16 @@ read_temp <- function(x, ..., error_call = rlang::caller_env()) { #' @noRd check_file_exists <- function(path, - error_call = rlang::caller_env()) { + error_call = rlang::caller_env(), + error_arg = rlang::caller_arg(path)) { if (!file.exists(path)) { cli::cli_abort( - "Can't find file at {.code {path}}.", + c( + "{.arg {error_arg}} must exist.", + "Can't find file at {.path {path}}." + ), call = error_call ) From 7ffdb0498c3158a91dabf1287b8454c528fd16bb Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Fri, 11 Oct 2024 16:10:14 +0100 Subject: [PATCH 24/39] New fn to get file name from Content-Disposition header --- R/utils_download-upload.R | 43 ++++++++++------ tests/testthat/test-utils_download-upload.R | 56 ++++++++++++--------- 2 files changed, 59 insertions(+), 40 deletions(-) diff --git a/R/utils_download-upload.R b/R/utils_download-upload.R index 286e81b..0596fca 100644 --- a/R/utils_download-upload.R +++ b/R/utils_download-upload.R @@ -167,9 +167,34 @@ rename_file <- function(temp_path, ) } + file_name <- file_name_from_header(response) + + new_path <- file.path(dirname(temp_path), file_name) + + if (file.exists(new_path) && overwrite == FALSE) { + cli::cli_abort( + c( + "File already exists: {.path {new_path}}.", + "i" = "To overwrite, set {.code {error_arg_overwrite} = TRUE}." + ), + call = error_call + ) + } + + file.rename(from = temp_path, to = new_path) + + invisible(new_path) + +} + + +file_name_from_header <- function(response, + error_call = rlang::caller_env(), + error_arg = rlang::caller_arg(response)) { + if (!httr2::resp_header_exists(response, "Content-Disposition")) { cli::cli_abort( - "Response must contain `Content-Disposition` header.", + "{.arg {error_arg}} must contain `Content-Disposition` header.", call = error_call ) } @@ -191,20 +216,6 @@ rename_file <- function(temp_path, ) } - new_path <- file.path(dirname(temp_path), file_name) - - if (file.exists(new_path) && overwrite == FALSE) { - cli::cli_abort( - c( - "File already exists: {.path {new_path}}.", - "i" = "To overwrite, set {.code {error_arg_overwrite} = TRUE}." - ), - call = error_call - ) - } - - file.rename(from = temp_path, to = new_path) - - invisible(new_path) + file_name } diff --git a/tests/testthat/test-utils_download-upload.R b/tests/testthat/test-utils_download-upload.R index 9e9e98d..6c81197 100644 --- a/tests/testthat/test-utils_download-upload.R +++ b/tests/testthat/test-utils_download-upload.R @@ -161,31 +161,12 @@ with_tempdir({ # rename_file ---- -resp_expected <- httr2::response( +resp <- httr2::response( headers = list(`Content-Disposition` = "filename=\"new_name.csv\"") ) -resp_bad_format <- httr2::response( - headers = list(`Content-Disposition` = "x") -) - -resp_no_header <- httr2::response( - headers = list(x = 1) -) - test_that("Error if temp_file doesn't exist", { - expect_error(rename_file(temp_path = "test", resp_expected)) -}) - -with_tempfile("test", { - - file.create(test) - - test_that("Error if httr2 resp doesn't have expected header", { - expect_error(rename_file(test, resp_no_header)) - expect_error(rename_file(test, resp_bad_format)) - }) - + expect_error(rename_file(temp_path = "test", resp)) }) with_tempfile(c("test", "new"), { @@ -216,7 +197,7 @@ with_tempfile("test", { file.create(test) test_that("File is renamed", { - rename_file(test, resp_expected) + rename_file(test, resp) expect_false(file.exists(test)) expect_true(file.exists(file.path(dirname(test), "new_name.csv"))) }) @@ -230,7 +211,7 @@ with_tempfile("test", { file.create(test) test_that("Returns invisibly", { - expect_invisible(rename_file(test, resp_expected)) + expect_invisible(rename_file(test, resp)) }) unlink(file.path(dirname(test), "new_name.csv")) @@ -243,7 +224,7 @@ with_tempfile("test", { test_that("New file path returned", { expect_equal( - (rename_file(test, resp_expected)), + (rename_file(test, resp)), file.path(dirname(test), "new_name.csv") ) }) @@ -251,3 +232,30 @@ with_tempfile("test", { unlink(file.path(dirname(test), "new_name.csv")) }) + + +# file_name_from_header ---- + +test_that("Error if no `Content-Disposition` header", { + expect_error(file_name_from_header( + httr2::response(headers = list(x = 1)) + )) +}) + +test_that("Error if header in unexpected format", { + expect_error(file_name_from_header( + httr2::response(headers = list(`Content-Disposition` = "x")) + )) +}) + +test_that("Correct value returned", { + expect_equal( + file_name_from_header( + httr2::response( + headers = list(`Content-Disposition` = "filename=\"new_name.csv\"") + ) + ), + "new_name.csv" + ) +}) + From 2bc9b1f238091e5848d5f97256aca647560fe07b Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Fri, 11 Oct 2024 17:08:46 +0100 Subject: [PATCH 25/39] Add read_data_version function and combine Rd docs --- NAMESPACE | 1 + R/download.R | 53 +++++++++++++++++++++++++++--------- _pkgdown.yml | 1 - man/download_file.Rd | 17 ++++++++++-- man/download_file_version.Rd | 29 -------------------- man/read_data.Rd | 13 +++++++-- 6 files changed, 66 insertions(+), 48 deletions(-) delete mode 100644 man/download_file_version.Rd diff --git a/NAMESPACE b/NAMESPACE index 41939ad..202d5cb 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -16,6 +16,7 @@ export(objr) export(participant_bypass_2fa) export(participants) export(read_data) +export(read_data_version) export(rename_asset) export(rollback_to_version) export(upload_file) diff --git a/R/download.R b/R/download.R index 846c77f..e3f1aa9 100644 --- a/R/download.R +++ b/R/download.R @@ -1,6 +1,12 @@ #' Download a file and save to disk #' -#' @param document_uuid UUID of existing document +#' @description +#' * Use `download_file()` with an asset UUID for the latest version of a +#' document. +#' * Use `download_file_version()` with a document version UUID for a specific +#' version of a document. +#' +#' @param document_uuid UUID of asset or document version #' @param folder Folder to save downloaded file to #' @param overwrite Logical to indicate whether file should be overwritten if #' already exists. Defaults to `FALSE`. @@ -36,17 +42,8 @@ download_file <- function(document_uuid, } -#' Download a document version and save to disk -#' -#' @param document_uuid UUID of existing document version -#' @param folder Folder to save downloaded file to -#' @param overwrite Logical to indicate whether file should be overwritten if -#' already exists. Defaults to `FALSE`. -#' @inheritParams objr -#' -#' @return An httr2 [httr2::response()][response] (invisibly) -#' #' @export +#' @rdname download_file download_file_version <- function(document_uuid, folder, @@ -76,8 +73,13 @@ download_file_version <- function(document_uuid, #' Read a data file into R #' -#' @param document_uuid UUID of existing document -#' @param ... Additional arguments to pass to read function. See details. +#' @description +#' * Use `read_data()` with an asset UUID for the latest version of a document. +#' * Use `read_data_version()` with a document version UUID for a specific +#' version of a document. +#' +#' @param document_uuid UUID of asset or document version +#' @param ... Additional arguments passed to read function. See details. #' @inheritParams objr #' #' @details This function can be used to read the following data file types: @@ -124,3 +126,28 @@ read_data <- function(document_uuid, x } + + +#' @export +#' @rdname read_data + +read_data_version <- function(document_uuid, + ..., + use_proxy = FALSE) { + + resp <- suppressMessages( + download_file_version(document_uuid, + overwrite = TRUE, + folder = tempdir(check = TRUE), + use_proxy = use_proxy) + ) + + path <- file.path(tempdir(check = TRUE), file_name_from_header(resp)) + + x <- read_temp(path, ...) + + unlink(path) + + x + +} diff --git a/_pkgdown.yml b/_pkgdown.yml index 29755a0..951c8c2 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -72,7 +72,6 @@ reference: - subtitle: Download contents: - download_file - - download_file_version - read_data - subtitle: Upload diff --git a/man/download_file.Rd b/man/download_file.Rd index 596e8df..7d20931 100644 --- a/man/download_file.Rd +++ b/man/download_file.Rd @@ -2,12 +2,20 @@ % Please edit documentation in R/download.R \name{download_file} \alias{download_file} +\alias{download_file_version} \title{Download a file and save to disk} \usage{ download_file(document_uuid, folder, overwrite = FALSE, use_proxy = FALSE) + +download_file_version( + document_uuid, + folder, + overwrite = FALSE, + use_proxy = FALSE +) } \arguments{ -\item{document_uuid}{UUID of existing document} +\item{document_uuid}{UUID of asset or document version} \item{folder}{Folder to save downloaded file to} @@ -20,5 +28,10 @@ already exists. Defaults to \code{FALSE}.} An httr2 \link[=response]{httr2::response()} (invisibly) } \description{ -Download a file and save to disk +\itemize{ +\item Use \code{download_file()} with an asset UUID for the latest version of a +document. +\item Use \code{download_file_version()} with a document version UUID for a specific +version of a document. +} } diff --git a/man/download_file_version.Rd b/man/download_file_version.Rd deleted file mode 100644 index 242e244..0000000 --- a/man/download_file_version.Rd +++ /dev/null @@ -1,29 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/download.R -\name{download_file_version} -\alias{download_file_version} -\title{Download a document version and save to disk} -\usage{ -download_file_version( - document_uuid, - folder, - overwrite = FALSE, - use_proxy = FALSE -) -} -\arguments{ -\item{document_uuid}{UUID of existing document version} - -\item{folder}{Folder to save downloaded file to} - -\item{overwrite}{Logical to indicate whether file should be overwritten if -already exists. Defaults to \code{FALSE}.} - -\item{use_proxy}{Logical to indicate whether to use proxy} -} -\value{ -An httr2 \link[=response]{httr2::response()} (invisibly) -} -\description{ -Download a document version and save to disk -} diff --git a/man/read_data.Rd b/man/read_data.Rd index 402d9c2..043087e 100644 --- a/man/read_data.Rd +++ b/man/read_data.Rd @@ -2,14 +2,17 @@ % Please edit documentation in R/download.R \name{read_data} \alias{read_data} +\alias{read_data_version} \title{Read a data file into R} \usage{ read_data(document_uuid, ..., use_proxy = FALSE) + +read_data_version(document_uuid, ..., use_proxy = FALSE) } \arguments{ -\item{document_uuid}{UUID of existing document} +\item{document_uuid}{UUID of asset or document version} -\item{...}{Additional arguments to pass to read function. See details.} +\item{...}{Additional arguments passed to read function. See details.} \item{use_proxy}{Logical to indicate whether to use proxy} } @@ -17,7 +20,11 @@ read_data(document_uuid, ..., use_proxy = FALSE) For csv and xlsx files, a data frame. For rds files, an R object. } \description{ -Read a data file into R +\itemize{ +\item Use \code{read_data()} with an asset UUID for the latest version of a document. +\item Use \code{read_data_version()} with a document version UUID for a specific +version of a document. +} } \details{ This function can be used to read the following data file types: From 4153e6f7dfb6ccd7e9e7d06833c0209b3182d995 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Fri, 11 Oct 2024 17:36:14 +0100 Subject: [PATCH 26/39] Add download_helper() --- R/download.R | 132 ++++++++++++++++++++++++++++----------------------- 1 file changed, 73 insertions(+), 59 deletions(-) diff --git a/R/download.R b/R/download.R index e3f1aa9..f746dc9 100644 --- a/R/download.R +++ b/R/download.R @@ -1,3 +1,52 @@ +download_helper <- function(document_uuid, + folder, + asset_type = c("documents", "documentversions"), + download_type = c("download", "read"), + ..., + overwrite = FALSE, + use_proxy = FALSE) { + + asset_type <- rlang::arg_match(asset_type) + download_type <- rlang::arg_match(download_type) + + # Create temporary file in desired folder + path <- create_file(folder) + + response <- objr( + endpoint = asset_type, + url_path = list(document_uuid, "download"), + method = "GET", + path = path, + use_proxy = use_proxy + ) + + if (download_type == "download") { + + # Rename file to match asset name + new_path <- rename_file(path, response, overwrite = overwrite) + + # Show success message and return response invisibly + if (httr2::resp_status(response) == 200) { + cli::cli_alert_success("File downloaded: {.path {new_path}}.") + } + invisible(response) + + } + + if (download_type == "read") { + + # Read data from file path + x <- read_temp(response$body[1], ...) + + # Delete file created by download and return data + unlink(path) + x + + } + +} + + #' Download a file and save to disk #' #' @description @@ -21,23 +70,12 @@ download_file <- function(document_uuid, overwrite = FALSE, use_proxy = FALSE) { - path <- create_file(folder) - - response <- objr( - endpoint = "documents", - url_path = list(document_uuid, "download"), - method = "GET", - path = path, - use_proxy = use_proxy - ) - - new_path <- rename_file(path, response, overwrite = overwrite) - - if (httr2::resp_status(response) == 200) { - cli::cli_alert_success("File downloaded: {.path {new_path}}.") - } - - invisible(response) + download_helper(document_uuid, + folder, + asset_type = "documents", + download_type = "download", + overwrite = overwrite, + use_proxy = use_proxy) } @@ -50,23 +88,12 @@ download_file_version <- function(document_uuid, overwrite = FALSE, use_proxy = FALSE) { - path <- create_file(folder) - - response <- objr( - endpoint = "documentversions", - url_path = list(document_uuid, "download"), - method = "GET", - path = path, - use_proxy = use_proxy - ) - - new_path <- rename_file(path, response, overwrite = overwrite) - - if (httr2::resp_status(response) == 200) { - cli::cli_alert_success("File downloaded: {.path new_path}.") - } - - invisible(response) + download_helper(document_uuid, + folder, + asset_type = "documentversions", + download_type = "download", + overwrite = overwrite, + use_proxy = use_proxy) } @@ -111,19 +138,13 @@ read_data <- function(document_uuid, ..., use_proxy = FALSE) { - resp <- suppressMessages( - download_file(document_uuid, + download_helper(document_uuid, folder = tempdir(check = TRUE), + asset_type = "documents", + download_type = "read", + ..., + overwrite = FALSE, use_proxy = use_proxy) - ) - - path <- resp$body[1] - - x <- read_temp(path, ...) - - unlink(path) - - x } @@ -135,19 +156,12 @@ read_data_version <- function(document_uuid, ..., use_proxy = FALSE) { - resp <- suppressMessages( - download_file_version(document_uuid, - overwrite = TRUE, - folder = tempdir(check = TRUE), - use_proxy = use_proxy) - ) - - path <- file.path(tempdir(check = TRUE), file_name_from_header(resp)) - - x <- read_temp(path, ...) - - unlink(path) - - x + download_helper(document_uuid, + folder = tempdir(check = TRUE), + asset_type = "documentversions", + download_type = "read", + ..., + overwrite = FALSE, + use_proxy = use_proxy) } From 1d685fdcea7b6d925c0d24c4d8d661143241cd04 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 17 Oct 2024 13:39:05 +0100 Subject: [PATCH 27/39] Import rlang::.data and magrittr::`%>%` and remove %>% export --- NAMESPACE | 2 +- R/objr-package.R | 8 ++++++++ R/utils-pipe.R | 14 -------------- man/objr-package.Rd | 28 ++++++++++++++++++++++++++++ man/pipe.Rd | 20 -------------------- 5 files changed, 37 insertions(+), 35 deletions(-) create mode 100644 R/objr-package.R delete mode 100644 R/utils-pipe.R create mode 100644 man/objr-package.Rd delete mode 100644 man/pipe.Rd diff --git a/NAMESPACE b/NAMESPACE index 202d5cb..4deb958 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,6 +1,5 @@ # Generated by roxygen2: do not edit by hand -export("%>%") export(allow_bypass_2fa) export(asset_info) export(assets) @@ -26,3 +25,4 @@ export(workspaces) export(write_data) export(write_data_version) importFrom(magrittr,"%>%") +importFrom(rlang,.data) diff --git a/R/objr-package.R b/R/objr-package.R new file mode 100644 index 0000000..93d7c3b --- /dev/null +++ b/R/objr-package.R @@ -0,0 +1,8 @@ +#' @keywords internal +"_PACKAGE" + +## usethis namespace: start +#' @importFrom magrittr %>% +#' @importFrom rlang .data +## usethis namespace: end +NULL diff --git a/R/utils-pipe.R b/R/utils-pipe.R deleted file mode 100644 index fd0b1d1..0000000 --- a/R/utils-pipe.R +++ /dev/null @@ -1,14 +0,0 @@ -#' Pipe operator -#' -#' See \code{magrittr::\link[magrittr:pipe]{\%>\%}} for details. -#' -#' @name %>% -#' @rdname pipe -#' @keywords internal -#' @export -#' @importFrom magrittr %>% -#' @usage lhs \%>\% rhs -#' @param lhs A value or the magrittr placeholder. -#' @param rhs A function call using the magrittr semantics. -#' @return The result of calling `rhs(lhs)`. -NULL diff --git a/man/objr-package.Rd b/man/objr-package.Rd new file mode 100644 index 0000000..217d9d7 --- /dev/null +++ b/man/objr-package.Rd @@ -0,0 +1,28 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/objr-package.R +\docType{package} +\name{objr-package} +\alias{objr-package} +\title{objr: Wrapper for Objective APIs} +\description{ +An R package wrapper for Objective APIs including Objective Connect. +} +\seealso{ +Useful links: +\itemize{ + \item \url{https://github.com/ScotGovAnalysis/objr} + \item \url{https://ScotGovAnalysis.github.io/objr/} + \item Report bugs at \url{https://github.com/ScotGovAnalysis/objr/issues} +} + +} +\author{ +\strong{Maintainer}: Alice Hannah \email{alice.hannah@gov.scot} + +Other contributors: +\itemize{ + \item Scottish Government \email{statistics.enquiries@gov.scot} [copyright holder] +} + +} +\keyword{internal} diff --git a/man/pipe.Rd b/man/pipe.Rd deleted file mode 100644 index a648c29..0000000 --- a/man/pipe.Rd +++ /dev/null @@ -1,20 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/utils-pipe.R -\name{\%>\%} -\alias{\%>\%} -\title{Pipe operator} -\usage{ -lhs \%>\% rhs -} -\arguments{ -\item{lhs}{A value or the magrittr placeholder.} - -\item{rhs}{A function call using the magrittr semantics.} -} -\value{ -The result of calling \code{rhs(lhs)}. -} -\description{ -See \code{magrittr::\link[magrittr:pipe]{\%>\%}} for details. -} -\keyword{internal} From 7b83ec511e30bcaec2d5a1cc6457a4b1218356f8 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 17 Oct 2024 13:44:54 +0100 Subject: [PATCH 28/39] Use tidyr::hoist for converting response body (list) to tibble --- DESCRIPTION | 3 +- R/assets.R | 53 ++++++++++++++++++------------------ R/documents.R | 41 ++++++++++++---------------- man/asset_info.Rd | 2 +- man/assets.Rd | 2 +- tests/testthat/test-assets.R | 14 ++++------ 6 files changed, 53 insertions(+), 62 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 096b399..2e82595 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -18,7 +18,8 @@ Imports: httr2, magrittr, rlang, - rstudioapi + rstudioapi, + tidyr Suggests: httptest2, knitr, diff --git a/R/assets.R b/R/assets.R index 335d830..41bcee4 100644 --- a/R/assets.R +++ b/R/assets.R @@ -7,7 +7,7 @@ #' @param size Number of results to be returned per page. #' @inheritParams objr #' -#' @return Data frame +#' @return Tibble #' #' @export @@ -30,11 +30,8 @@ assets <- function(workspace_uuid, use_proxy = use_proxy ) - content <- - httr2::resp_body_json(response)$content %>% - lapply(\(x) data.frame(asset_info_list(x))) - - Reduce(dplyr::bind_rows, content) + tidyr::tibble(content = httr2::resp_body_json(response)$content) %>% + asset_info_list() } @@ -44,7 +41,7 @@ assets <- function(workspace_uuid, #' @param asset_uuid UUID of asset #' @inheritParams objr #' -#' @return Named list containing: uuid, name, type, extension, description. +#' @return Tibble #' #' @export @@ -58,7 +55,8 @@ asset_info <- function(asset_uuid, ) %>% httr2::resp_body_json() - asset_info_list(response) + dplyr::tibble(content = list(response)) %>% + asset_info_list() } @@ -122,26 +120,27 @@ rename_asset <- function(asset_uuid, } - -na_if_null <- function(x) { - if (is.null(x)) NA else x -} - asset_info_list <- function(x) { - list( - asset_name = x$name, - asset_ext = na_if_null(x$extension), - asset_type = x$type, - asset_uuid = x$uuid, - last_modified_by = paste(na_if_null(x[["modifiedBy"]]$givenName), - na_if_null(x[["modifiedBy"]]$familyName)), - last_modified = na_if_null(convert_from_epoch(x$modifiedTime)), - latest_version = na_if_null(x$contentVersion), - parent_name = na_if_null(x[["parent"]]$name), - parent_uuid = na_if_null(x[["parent"]]$uuid), - workspace_name = x[["workspace"]]$name, - workspace_uuid = x[["workspace"]]$uuid - ) + tidyr::hoist( + x, + .data$content, + asset_name = "name", + asset_ext = "extension", + asset_type = "type", + asset_uuid = "uuid", + name1 = c("modifiedBy", "givenName"), + name2 = c("modifiedBy", "familyName"), + last_modified = "modifiedTime", + latest_version = "contentVersion", + parent_name = c("parent", "name"), + parent_uuid = c("parent", "uuid"), + workspace_name = c("workspace", "name"), + workspace_uuid = c("workspace", "uuid"), + .transform = list(last_modified = convert_from_epoch) + ) %>% + dplyr::mutate(last_modified_by = paste(.data$name1, .data$name2), + .after = "last_modified") %>% + dplyr::select(-c("name1", "name2", "content")) } diff --git a/R/documents.R b/R/documents.R index 02d0dd6..58b43ac 100644 --- a/R/documents.R +++ b/R/documents.R @@ -22,11 +22,24 @@ versions <- function(document_uuid, use_proxy = use_proxy ) - content <- - httr2::resp_body_json(response)$content %>% - lapply(\(x) data.frame(versions_info_list(x))) - - Reduce(dplyr::bind_rows, content) + dplyr::tibble(content = httr2::resp_body_json(response)$content) %>% + tidyr::hoist( + .data$content, + asset_name = c("asset", "name"), + asset_ext = "extension", + asset_uuid = c("asset", "uuid"), + version = "version", + version_uuid = "uuid", + created_date = "createdTime", + name1 = c("createdBy", "givenName"), + name2 = c("createdBy", "familyName"), + workspace_name = c("asset", "workspace", "name"), + workspace_uuid = c("asset", "workspace", "uuid"), + .transform = list(created_date = convert_from_epoch) + ) %>% + dplyr::mutate(created_by = paste(.data$name1, .data$name2), + .after = "created_date") %>% + dplyr::select(-c("name1", "name2", "content")) } @@ -58,21 +71,3 @@ rollback_to_version <- function(document_uuid, invisible(response) } - - -versions_info_list <- function(x) { - - list( - asset_name = x[["asset"]]$name, - asset_ext = na_if_null(x$extension), - asset_uuid = x[["asset"]]$uuid, - version = x$version, - version_uuid = x$uuid, - created_date = na_if_null(convert_from_epoch(x$createdTime)), - created_by = paste(na_if_null(x[["createdBy"]]$givenName), - na_if_null(x[["createdBy"]]$familyName)), - workspace_name = x[["asset"]][["workspace"]]$name, - workspace_uuid = x[["asset"]][["workspace"]]$uuid - ) - -} diff --git a/man/asset_info.Rd b/man/asset_info.Rd index 855bed6..56bc8da 100644 --- a/man/asset_info.Rd +++ b/man/asset_info.Rd @@ -12,7 +12,7 @@ asset_info(asset_uuid, use_proxy = FALSE) \item{use_proxy}{Logical to indicate whether to use proxy} } \value{ -Named list containing: uuid, name, type, extension, description. +Tibble } \description{ Get asset information diff --git a/man/assets.Rd b/man/assets.Rd index f01db40..dddf354 100644 --- a/man/assets.Rd +++ b/man/assets.Rd @@ -25,7 +25,7 @@ document, folder and link.} \item{use_proxy}{Logical to indicate whether to use proxy} } \value{ -Data frame +Tibble } \description{ Get data frame of assets in workspace diff --git a/tests/testthat/test-assets.R b/tests/testthat/test-assets.R index f9713d2..4dac414 100644 --- a/tests/testthat/test-assets.R +++ b/tests/testthat/test-assets.R @@ -28,11 +28,11 @@ without_internet({ with_mock_api({ - test_that("Function returns dataframe", { + test_that("Function returns tibble", { expect_s3_class( assets(workspace_uuid = "test_workspace_uuid"), - "data.frame" + "tbl" ) }) @@ -69,15 +69,11 @@ without_internet({ with_mock_api({ - test_that("Function returns dataframe", { + test_that("Function returns tibble", { - expect_type( + expect_s3_class( asset_info(asset_uuid = "test_asset"), - "list" - ) - - expect_named( - asset_info(asset_uuid = "test_asset") + "tbl" ) }) From bcd23b9fa02f1ac39441f62a4fdc6d4fd743ec06 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Fri, 18 Oct 2024 15:27:59 +0100 Subject: [PATCH 29/39] Remove code to add time (as.POSIXct auto sets to 00:00:00) --- R/utils.R | 13 ++++--------- tests/testthat/test-comments.R | 4 ++-- tests/testthat/test-utils.R | 3 ++- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/R/utils.R b/R/utils.R index b3b015a..eaabcec 100644 --- a/R/utils.R +++ b/R/utils.R @@ -225,10 +225,10 @@ check_list <- function(x, #' Convert date or datetime object to number of milliseconds from epoch #' -#' @param date_time Date or datetime. +#' @param date_time Date or datetime with "POSIXct" class. #' #' @details -#' * If only a date is supplied, a time of 00:00:01 will be added. +#' * If only a date is supplied, a time of 00:00:00 will be added. #' * If NULL is supplied, NULL is returned. #' * If an invalid value is supplied (not date, datetime or NULL), an error will #' be produced. @@ -249,18 +249,13 @@ convert_to_epoch <- function(date_time, } # Check correct class if supplied - if (any(!class(date_time) %in% c("Date", "POSIXct", "POSIXt"))) { + if (!"POSIXct" %in% class(date_time)) { cli::cli_abort( - "{.arg {error_arg}} must be of Date or POSIXct class.", + "{.arg {error_arg}} must be of POSIXct class.", call = error_call ) } - # Add time if only date supplied - if (any(class(date_time) == "Date")) { - date_time <- as.POSIXct(paste(date_time, "00:00:01")) - } - as.integer(date_time) * 1000 } diff --git a/tests/testthat/test-comments.R b/tests/testthat/test-comments.R index b0368d2..302129b 100644 --- a/tests/testthat/test-comments.R +++ b/tests/testthat/test-comments.R @@ -10,7 +10,7 @@ without_internet({ ) expect_GET( - comments(created_after = as.Date("2024-01-01"), + comments(created_after = as.POSIXct("2024-01-01"), thread_uuid = "test_thread", mention_uuid = "test_mention", workgroup_uuid = "test_workgroup", @@ -18,7 +18,7 @@ without_internet({ size = "test_size"), paste0("https://secure.objectiveconnect.co.uk/publicapi/1/comments?", "createdAfter=", - as.integer(as.POSIXct("2024-01-01 00:00:01")) * 1000, "&", + as.integer(as.POSIXct("2024-01-01 00:00:00")) * 1000, "&", "threadUuid=test_thread&", "mentionUuid=test_mention&", "workgroupUuid=test_workgroup&", diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 097435f..8eb78b9 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -181,12 +181,13 @@ test_that("`x` returned invisibly", { # convert_to_epoch ---- -test_that("Error produced if not NULL or datetime", { +test_that("Error produced if not NULL or POSIXct", { expect_error(convert_to_epoch("invalid")) expect_error(convert_to_epoch(NA)) expect_error(convert_to_epoch("2024-01-01")) expect_error(convert_to_epoch(20240101)) + expect_error(convert_to_epoch(as.Date("2024-01-01"))) }) From 96c4ece2ed41856ab3430f3ab0b4bd917daf4b11 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 22 Oct 2024 14:19:45 +0100 Subject: [PATCH 30/39] Improve error handling fn --- R/objr.R | 56 ++++++++++++++++++++++++++++++-------- tests/testthat/test-objr.R | 13 ++++++++- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/R/objr.R b/R/objr.R index f15331f..45b3419 100644 --- a/R/objr.R +++ b/R/objr.R @@ -189,31 +189,63 @@ error <- function(response) { desc <- tryCatch(httr2::resp_body_json(response)$description, error = function(e) NULL) - extra <- NULL + message <- NULL + + if (status == 400) { + message <- c( + "!" = desc, + "!" = "Invalid argument(s) provided." + ) + } if (status == 401) { - extra <- c( - "Authorisation failed. Check username / password / token.", - paste( - "You might have an expired token in your R environment.", + message <- c( + "!" = "API authentication is invalid.", + "i" = "Is there an error in your username, password or token?", + "i" = paste( + "Do you have an expired token in your R environment?", "Remove it with `rm(token)`." + ), + "i" = paste( + "For more information, see", + "https://scotgovanalysis.github.io/objr/articles/authentication.html" ) ) } - if (status == 403 && grepl("REQUIRES_2FA", desc)) { - extra <- - "See https://scotgovanalysis.github.io/objr/articles/two-factor.html" + if (status == 403) { + message <- c( + "!" = "You are not permitted to perform this action." + ) + + if (!is.null(desc) && grepl("REQUIRES_2FA", desc)) { + message <- c( + message, + "i" = paste( + "You do not have permission to bypass two-factor authentication", + "in this workspace." + ), + "i" = paste( + "For more information, see", + "https://scotgovanalysis.github.io/objr/articles/two-factor.html" + ) + ) + } else { + message <- c( + message, + "i" = "This action may be disabled in your organisation." + ) + } } if (status == 404) { - extra <- c( - "Asset cannot be found.", - "Check asset UUID is correct." + message <- c( + desc, + "i" = "Have you checked that the UUID supplied is valid?" ) } - c(desc, extra) + if (is.null(message)) desc else message } diff --git a/tests/testthat/test-objr.R b/tests/testthat/test-objr.R index a083c5b..d2a54d3 100644 --- a/tests/testthat/test-objr.R +++ b/tests/testthat/test-objr.R @@ -108,13 +108,19 @@ test_that("Environment value created successfully", { test_that("Expect character value returned", { + expect_type( + error(httr2::response(status_code = 400)), + "character" + ) + expect_type( error(httr2::response(status_code = 401)), "character" ) expect_type( - error(httr2::response(status_code = 404)), + error(httr2::response(status_code = 403, + body = list(description = "x"))), "character" ) @@ -126,6 +132,11 @@ test_that("Expect character value returned", { "character" ) + expect_type( + error(httr2::response(status_code = 404)), + "character" + ) + }) test_that("NULL returned for invalid status code", { From ad6c70f876c0ccd0cff0ec2dc2d2ec530ec9c044 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 22 Oct 2024 15:19:17 +0100 Subject: [PATCH 31/39] Use tidyr::hoist --- R/comments.R | 35 ++++++++++++++++------------------- R/participants.R | 33 +++++++++++++++------------------ R/workspaces.R | 35 ++++++++++++++++------------------- 3 files changed, 47 insertions(+), 56 deletions(-) diff --git a/R/comments.R b/R/comments.R index 8ac2649..8c33516 100644 --- a/R/comments.R +++ b/R/comments.R @@ -36,25 +36,22 @@ comments <- function(created_after = NULL, use_proxy = use_proxy ) - content <- - httr2::resp_body_json(response)$content %>% - lapply( - \(content) { - data.frame( - type = content$commentType, - text = content$commentText, - author = paste(content$creator$givenName, - content$creator$familyName), - created_time = as.POSIXct(content$createdTime / 1000, - origin = "1970-01-01"), - thread_uuid = content$thread$uuid, - workspace_name = content$workspace$name, - workspace_uuid = content$workspace$uuid - ) - } - ) - - Reduce(dplyr::bind_rows, content) + dplyr::tibble(content = httr2::resp_body_json(response)$content) %>% + tidyr::hoist( + .data$content, + type = "commentType", + text = "commentText", + name1 = c("creator", "givenName"), + name2 = c("creator", "familyName"), + created_time = "createdTime", + thread_uuid = c("thread", "uuid"), + workspace_name = c("workspace", "name"), + workspace_uuid = c("workspace", "uuid"), + .transform = list(created_time = convert_from_epoch) + ) %>% + dplyr::mutate(author = paste(.data$name1, .data$name2), + .after = "text") %>% + dplyr::select(-c("name1", "name2", "content")) } diff --git a/R/participants.R b/R/participants.R index afce204..a4bb1e7 100644 --- a/R/participants.R +++ b/R/participants.R @@ -15,23 +15,20 @@ participants <- function(workspace_uuid, use_proxy = FALSE) { use_proxy = use_proxy ) - content <- - httr2::resp_body_json(response)$content %>% - lapply( - \(content) { - data.frame( - user_name = paste(content$user$givenName, - content$user$familyName), - user_email = content$user$email, - user_uuid = content$userUuid, - bypass_2fa = content$bypassTwoStep, - participant_uuid = content$uuid, - workspace_name = content$workspace$name, - workspace_uuid = content$workspace$uuid - ) - } - ) - - Reduce(rbind, content) + dplyr::tibble(content = httr2::resp_body_json(response)$content) %>% + tidyr::hoist( + .data$content, + name1 = c("user", "givenName"), + name2 = c("user", "familyName"), + user_email = c("user", "email"), + user_uuid = "userUuid", + bypass_2fa = "bypassTwoStep", + participant_uuid = "uuid", + workspace_name = c("workspace", "name"), + workspace_uuid = c("workspace", "uuid") + ) %>% + dplyr::mutate(user_name = paste(.data$name1, .data$name2), + .before = 0) %>% + dplyr::select(-c("name1", "name2", "content")) } diff --git a/R/workspaces.R b/R/workspaces.R index 8b01298..03d2b0f 100644 --- a/R/workspaces.R +++ b/R/workspaces.R @@ -22,24 +22,21 @@ workspaces <- function(workgroup_uuid = NULL, use_proxy = use_proxy ) - content <- - httr2::resp_body_json(response)$content |> - lapply( - \(content) { - data.frame( - workspace_name = content$name, - workspace_uuid = content$uuid, - participant_uuid = content$participant$uuid, - owner_name = paste(content$owner$givenName, - content$owner$familyName), - owner_email = content$owner$email, - owner_uuid = content$owner$uuid, - workgroup_name = content$workgroup$name, - workgroup_uuid = content$workgroup$uuid - ) - } - ) - - Reduce(dplyr::bind_rows, content) + dplyr::tibble(content = httr2::resp_body_json(response)$content) %>% + tidyr::hoist( + .data$content, + workspace_name = "name", + workspace_uuid = "uuid", + participant_uuid = c("participant", "uuid"), + name1 = c("owner", "givenName"), + name2 = c("owner", "familyName"), + owner_email = c("owner", "email"), + owner_uuid = c("owner", "uuid"), + workgroup_name = c("workgroup", "name"), + workgroup_uuid = c("workgroup", "uuid") + ) %>% + dplyr::mutate(owner_name = paste(.data$name1, .data$name2), + .before = "owner_email") %>% + dplyr::select(-c("name1", "name2", "content")) } From a19360cc558b5b4922a1cfb30276a76ef1bf8e85 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 22 Oct 2024 17:22:34 +0100 Subject: [PATCH 32/39] Add add_participants fn --- NAMESPACE | 1 + R/participants.R | 89 +++++++++++++++++++++++++++++++++++++++++ man/add_participants.Rd | 46 +++++++++++++++++++++ 3 files changed, 136 insertions(+) create mode 100644 man/add_participants.Rd diff --git a/NAMESPACE b/NAMESPACE index 4deb958..27fd5ff 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,5 +1,6 @@ # Generated by roxygen2: do not edit by hand +export(add_participants) export(allow_bypass_2fa) export(asset_info) export(assets) diff --git a/R/participants.R b/R/participants.R index a4bb1e7..917d369 100644 --- a/R/participants.R +++ b/R/participants.R @@ -32,3 +32,92 @@ participants <- function(workspace_uuid, use_proxy = FALSE) { dplyr::select(-c("name1", "name2", "content")) } + + +#' Add participant(s) to a workspace +#' +#' @param workspace_uuid UUID of workspace. +#' @param emails Character vector of email addresses to send invites to. +#' @param message Optionally, a message to include in email invite. +#' @param permissions Optionally, a character vector of permissions to give +#' invited participants. +#' +#' Valid permissions are: "Download", "CreateDocument", +#' "CreateFolder", "Edit", "Delete", "EditOnline", "Inviter", "Commenter" and +#' "ManageWorkspace". +#' +#' All members are given permission to preview documents. +#' @param member_visibility Either "standard" (default) to make new participants +#' visible to all other workspace members, or "bcc" to hide participants. +#' @param send_email_invite Default `TRUE` to send an email invite for +#' participants to join the workspace. If `FALSE`, no email will be sent. +#' @inheritParams objr +#' +#' @return API response (invisibly) +#' +#' @export + +add_participants <- function(workspace_uuid, + emails, + message = NULL, + permissions = NULL, + member_visibility = c("standard", "bcc"), + send_email_invite = TRUE, + use_proxy = FALSE) { + + member_visibility <- rlang::arg_match(member_visibility) + + expected_permissions <- c( + "Download", + "CreateDocument", + "CreateFolder", + "Edit", + "Delete", + "EditOnline", + "Inviter", + "Commenter", + "ManageWorkspace" + ) + + # Check requested permissions are valid + if (any(!permissions %in% expected_permissions)) { + diff <- setdiff(permissions, expected_permissions) + cli::cli_abort(c( + "{.arg permissions} must only contain valid permission types.", + "i" = "{.str {diff}} {?is/are} not {?an/} accepted permission{?s}.", + "i" = paste( + "Accepted permissions:", + "{.str {cli::cli_vec(expected_permissions,", + "style = list(`vec-last` = ' or '))}}." + ) + )) + } + + permission_values <- + rep("true", times = length(permissions)) %>% + magrittr::set_names(paste0("has", permissions)) + + response <- objr( + endpoint = "participants", + method = "POST", + body = append( + list( + workspaceUuid = workspace_uuid, + emails = list(emails), + isSilent = tolower(!send_email_invite), + message = message, + type = toupper(member_visibility) + ), + permission_values + ), + use_proxy = use_proxy + ) + + if (response$status == 200) { + cli::cli_alert_success("Participant{?s} invited: {.field {emails}}.") + } + + invisible(response) + +} + diff --git a/man/add_participants.Rd b/man/add_participants.Rd new file mode 100644 index 0000000..6f360ef --- /dev/null +++ b/man/add_participants.Rd @@ -0,0 +1,46 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/participants.R +\name{add_participants} +\alias{add_participants} +\title{Add participant(s) to a workspace} +\usage{ +add_participants( + workspace_uuid, + emails, + message = NULL, + permissions = NULL, + member_visibility = c("standard", "bcc"), + send_email_invite = TRUE, + use_proxy = FALSE +) +} +\arguments{ +\item{workspace_uuid}{UUID of workspace.} + +\item{emails}{Character vector of email addresses to send invites to.} + +\item{message}{Optionally, a message to include in email invite.} + +\item{permissions}{Optionally, a character vector of permissions to give +invited participants. + +Valid permissions are: "Download", "CreateDocument", +"CreateFolder", "Edit", "Delete", "EditOnline", "Inviter", "Commenter" and +"ManageWorkspace". + +All members are given permission to preview documents.} + +\item{member_visibility}{Either "standard" (default) to make new participants +visible to all other workspace members, or "bcc" to hide participants.} + +\item{send_email_invite}{Default \code{TRUE} to send an email invite for +participants to join the workspace. If \code{FALSE}, no email will be sent.} + +\item{use_proxy}{Logical to indicate whether to use proxy} +} +\value{ +API response (invisibly) +} +\description{ +Add participant(s) to a workspace +} From c3d7df177b138b554cceefe3b10a2e4d731a870e Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 22 Oct 2024 17:45:21 +0100 Subject: [PATCH 33/39] If permissions not supplied, pass NULL to API request --- R/participants.R | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/R/participants.R b/R/participants.R index 917d369..5a1e9f6 100644 --- a/R/participants.R +++ b/R/participants.R @@ -94,8 +94,12 @@ add_participants <- function(workspace_uuid, } permission_values <- - rep("true", times = length(permissions)) %>% - magrittr::set_names(paste0("has", permissions)) + if (!is.null(permissions)) { + rep("true", times = length(permissions)) %>% + magrittr::set_names(paste0("has", permissions)) + } else { + NULL + } response <- objr( endpoint = "participants", From 858e59ae33d8665bc5e9fef3d57d822aa213062a Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Tue, 22 Oct 2024 17:46:47 +0100 Subject: [PATCH 34/39] Test add_participants() --- tests/testthat/api/participants-6e5496-POST.R | 13 +++++ tests/testthat/test-participants.R | 54 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 tests/testthat/api/participants-6e5496-POST.R diff --git a/tests/testthat/api/participants-6e5496-POST.R b/tests/testthat/api/participants-6e5496-POST.R new file mode 100644 index 0000000..2ff367d --- /dev/null +++ b/tests/testthat/api/participants-6e5496-POST.R @@ -0,0 +1,13 @@ +structure(list(method = "POST", url = "https://api/participants", + status_code = 200L, headers = structure(list(Date = "Tue, 22 Oct 2024 16:37:40 GMT", + `Content-Type` = "application/json", `Transfer-Encoding` = "chunked", + Connection = "keep-alive", `X-CONNECT-MDC` = "apiiwaEofcZ", + `X-Frame-Options` = "deny", `X-XSS-Protection` = "1; mode=block", + `Cache-Control` = "no-cache, no-store", Expires = "0", + Pragma = "no-cache", `Strict-Transport-Security` = "max-age=31536000 ; includeSubDomains", + `Content-Security-Policy` = "script-src 'self' 'unsafe-inline'", + `X-Content-Type-Options` = "nosniff", `Referrer-Policy` = "strict-origin-when-cross-origin", + `Feature-Policy` = "vibrate 'none'; geolocation 'none'", + Authorization = "REDACTED", `Set-Cookie` = "REDACTED"), class = "httr2_headers"), + body = charToRaw("{\"content\":[],\"metadata\":{\"totalElements\":1,\"totalPages\":1,\"page\":0,\"offset\":0}}"), + cache = new.env(parent = emptyenv())), class = "httr2_response") diff --git a/tests/testthat/test-participants.R b/tests/testthat/test-participants.R index 6b4a056..aea51e0 100644 --- a/tests/testthat/test-participants.R +++ b/tests/testthat/test-participants.R @@ -1,3 +1,5 @@ +# participants ---- + without_internet({ test_that("Valid request created", { @@ -26,3 +28,55 @@ with_mock_api({ }) }) + + +# add_participants ---- + +without_internet({ + + test_that("Valid request created", { + expect_POST( + add_participants(workspace_uuid = "test_workspace", + emails = "test_email", + send_email_invite = FALSE, + permissions = "Delete"), + "https://secure.objectiveconnect.co.uk/publicapi/1/participants", + "{\"workspaceUuid\":\"test_workspace\",\"emails\":[\"test_email\"],", + "\"isSilent\":\"true\",\"message\":null,\"type\":\"STANDARD\",", + "\"hasDelete\":\"true\"}" + ) + }) + +}) + +test_that("Error if invalid permission supplied", { + expect_error( + add_participants(workspace_uuid = "test_workspace", + emails = "test_email", + permissions = "invalid") + ) +}) + +with_mock_api({ + + test_that("Function returns invisible", { + + expect_invisible( + suppressMessages(add_participants( + workspace_uuid = "test_workspace", + emails = "test_email" + )) + ) + + }) + + test_that("Success message", { + expect_message( + add_participants( + workspace_uuid = "test_workspace", + emails = "test_email" + ) + ) + }) + +}) From 694d297e3606c9d3248a96cf3aabed0e865ee16d Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Wed, 23 Oct 2024 10:28:44 +0100 Subject: [PATCH 35/39] Add new fn and vignette link --- _pkgdown.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/_pkgdown.yml b/_pkgdown.yml index 951c8c2..4c9cd9d 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -54,6 +54,7 @@ reference: contents: - workspaces - participants + - add_participants - title: Assets desc: Assets represent Documents and Folders that are added to Workspaces @@ -89,6 +90,7 @@ reference: - new_reply - title: Two-factor authentication + desc: See `vignette("two-factor")` for more information. contents: - allow_bypass_2fa - participant_bypass_2fa From 18e2ebc81c380e9560ee24383f1c2da3ca828c3e Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Wed, 23 Oct 2024 11:53:51 +0100 Subject: [PATCH 36/39] Address lintr feedback --- R/download.R | 2 +- R/participants.R | 7 ++++--- R/utils_download-upload.R | 2 +- tests/testthat/test-utils_download-upload.R | 1 - 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/R/download.R b/R/download.R index f746dc9..d8a53e1 100644 --- a/R/download.R +++ b/R/download.R @@ -23,7 +23,7 @@ download_helper <- function(document_uuid, if (download_type == "download") { # Rename file to match asset name - new_path <- rename_file(path, response, overwrite = overwrite) + new_path <- rename_file(path, response, overwrite = overwrite) # nolint: object_usage_linter # Show success message and return response invisibly if (httr2::resp_status(response) == 200) { diff --git a/R/participants.R b/R/participants.R index 5a1e9f6..1729789 100644 --- a/R/participants.R +++ b/R/participants.R @@ -81,10 +81,12 @@ add_participants <- function(workspace_uuid, # Check requested permissions are valid if (any(!permissions %in% expected_permissions)) { - diff <- setdiff(permissions, expected_permissions) cli::cli_abort(c( "{.arg permissions} must only contain valid permission types.", - "i" = "{.str {diff}} {?is/are} not {?an/} accepted permission{?s}.", + "i" = paste( + "{.str {setdiff(permissions, expected_permissions)}}", + "{?is/are} not {?an/} accepted permission{?s}." + ), "i" = paste( "Accepted permissions:", "{.str {cli::cli_vec(expected_permissions,", @@ -124,4 +126,3 @@ add_participants <- function(workspace_uuid, invisible(response) } - diff --git a/R/utils_download-upload.R b/R/utils_download-upload.R index 0596fca..edcf2c7 100644 --- a/R/utils_download-upload.R +++ b/R/utils_download-upload.R @@ -203,7 +203,7 @@ file_name_from_header <- function(response, file_name <- regmatches( cont_disp, - m = regexpr("(?<=filename=\\\").*(?=\\\")", cont_disp, perl = T) + m = regexpr("(?<=filename=\\\").*(?=\\\")", cont_disp, perl = TRUE) ) if (length(file_name) == 0) { diff --git a/tests/testthat/test-utils_download-upload.R b/tests/testthat/test-utils_download-upload.R index 6c81197..df156e9 100644 --- a/tests/testthat/test-utils_download-upload.R +++ b/tests/testthat/test-utils_download-upload.R @@ -258,4 +258,3 @@ test_that("Correct value returned", { "new_name.csv" ) }) - From 56a5cfc29e7203e7f86ce682287ee11f21aa6501 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 31 Oct 2024 14:27:13 +0000 Subject: [PATCH 37/39] Combine upload docs and rename arg to uuid --- R/upload.R | 84 ++++++++++++++---------------------- _pkgdown.yml | 2 - man/upload_file.Rd | 18 +++++--- man/upload_file_version.Rd | 18 -------- man/write_data.Rd | 25 +++++++---- man/write_data_version.Rd | 41 ------------------ tests/testthat/test-upload.R | 14 +++--- 7 files changed, 70 insertions(+), 132 deletions(-) delete mode 100644 man/upload_file_version.Rd delete mode 100644 man/write_data_version.Rd diff --git a/R/upload.R b/R/upload.R index 2f37e7a..e52535a 100644 --- a/R/upload.R +++ b/R/upload.R @@ -1,21 +1,27 @@ -#' Upload a file to create a new document +#' Upload a file +#' +#' @description +#' * Use `upload_file()` to create a new file in a workspace. +#' * Use `upload_file_version()` to create a new version of an existing +#' document. #' #' @param file File path of document to upload +#' @param uuid For `upload_file()`, a workspace UUID to create the new document +#' in. For `upload_file_version()`, an asset UUID to create a new version of. #' @param name Name to give document. If this isn't provided, the name of the #' file will be used. -#' @param workspace_uuid UUID of the workspace to create the new document in #' @param description Optional description of document. #' @param parent_uuid UUID of folder in the workspace to create the new #' document within. If not supplied, the document will be created in the #' top-level of the workspace. #' @inheritParams objr #' -#' @return An httr2 [httr2::response()][response] (invisibly) +#' @return API response (invisibly) #' #' @export upload_file <- function(file, - workspace_uuid, + uuid, name = NULL, description = NULL, parent_uuid = NULL, @@ -37,7 +43,7 @@ upload_file <- function(file, body = list( name = curl::form_data(name), description = form_data_null(description), - workspaceUuid = curl::form_data(workspace_uuid), + workspaceUuid = curl::form_data(uuid), parentUuid = form_data_null(parent_uuid), file = curl::form_file(file) ), @@ -55,23 +61,18 @@ upload_file <- function(file, } -#' Upload a file to create a new document version -#' -#' @param file File path of document to upload -#' @param document_uuid UUID of existing document -#' @inheritParams objr -#' #' @export +#' @rdname upload_file upload_file_version <- function(file, - document_uuid, + uuid, use_proxy = FALSE) { check_file_exists(file) response <- objr( endpoint = "documents", - url_path = list(document_uuid, "upload"), + url_path = list(uuid, "upload"), method = "POST", content_type = "multipart/form-data", body = list(file = curl::form_file(file)), @@ -79,7 +80,7 @@ upload_file_version <- function(file, ) # Get asset info - info <- asset_info(document_uuid) # nolint: object_usage_linter + info <- asset_info(uuid) # nolint: object_usage_linter if (httr2::resp_status(response) == 204) { cli::cli_alert_success(paste( @@ -93,9 +94,16 @@ upload_file_version <- function(file, } -#' Write a data file from R to create a new document +#' Write a data file from R +#' +#' @description +#' * Use `write_data()` to create a new file in a workspace. +#' * Use `write_data_version()` to write a data file as a new version of an +#' existing document. #' #' @param x R object to write to file. +#' @param uuid Either a workspace UUID to create a new document or an asset UUID +#' to create a new version of an existing document. #' @param file_name Name to give file. #' @param file_type Either "csv", "rds" or "xlsx". #' @param ... Additional arguments to pass to write function. See details. @@ -103,8 +111,9 @@ upload_file_version <- function(file, #' @inheritParams upload_file #' #' @details This function can be used to write the following data file types: -#' csv, rds, xlsx. Use the \code{file_type} argument to control which file type -#' to create. +#' csv, rds, xlsx. If writing to a new document, use the \code{file_type} +#' argument to control which file type to create. If writing a new version of an +#' existing document, the existing file type will be used. #' #' The function works by writing the R object to a temporary file and uploading #' the file to Objective Connect. The following functions are used to @@ -121,14 +130,14 @@ upload_file_version <- function(file, #' function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. # nolint end #' -#' @return An httr2 [httr2::response()][response] (invisibly) +#' @return API response (invisibly) #' #' @export write_data <- function(x, + uuid, file_name, file_type, - workspace_uuid, ..., description = NULL, parent_uuid = NULL, @@ -141,7 +150,7 @@ write_data <- function(x, on.exit(unlink(path), add = TRUE) resp <- upload_file(path, - workspace_uuid = workspace_uuid, + uuid = uuid, description = description, parent_uuid = parent_uuid, use_proxy = FALSE) @@ -151,43 +160,16 @@ write_data <- function(x, } -#' Write a data file from R to create a new document version -#' -#' @param x R object to write to file. -#' @param ... Additional arguments to pass to write function. See details. -#' @inheritParams objr -#' @inheritParams upload_file_version -#' -#' @details This function can be used to write the following data file types: -#' csv, rds, xlsx. The file type used is determined by the file type of the -#' existing document. -#' -#' The function works by writing the R object to a temporary file and uploading -#' the file to Objective Connect. The following functions are used to -#' write the data and any additional arguments (`...`) will be passed to these. -#' -#' | File Type | Function | -#' | --- | --- | -#' | csv | \code{readr::write_csv()} | -#' | rds | \code{readr::write_rds()} | -#' | xlsx | \code{writexl::write_xlsx()} | -#' -# nolint start: line_length_linter -#' If there are other data file types you would like to upload using this -#' function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. -# nolint end -#' -#' @return An httr2 [httr2::response()][response] (invisibly) -#' #' @export +#' @rdname write_data write_data_version <- function(x, - document_uuid, + uuid, ..., use_proxy = FALSE) { # Get asset info - info <- asset_info(document_uuid) + info <- asset_info(uuid) path <- write_temp(x, file_name = info$asset_name, @@ -198,7 +180,7 @@ write_data_version <- function(x, on.exit(unlink(path), add = TRUE) resp <- upload_file_version(path, - document_uuid, + uuid, use_proxy = use_proxy) invisible(resp) diff --git a/_pkgdown.yml b/_pkgdown.yml index 4c9cd9d..dbcc4ad 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -78,9 +78,7 @@ reference: - subtitle: Upload contents: - upload_file - - upload_file_version - write_data - - write_data_version - title: Comments desc: View comments and create new threads and replies in workspaces diff --git a/man/upload_file.Rd b/man/upload_file.Rd index 9044102..3c2914d 100644 --- a/man/upload_file.Rd +++ b/man/upload_file.Rd @@ -2,21 +2,25 @@ % Please edit documentation in R/upload.R \name{upload_file} \alias{upload_file} -\title{Upload a file to create a new document} +\alias{upload_file_version} +\title{Upload a file} \usage{ upload_file( file, - workspace_uuid, + uuid, name = NULL, description = NULL, parent_uuid = NULL, use_proxy = FALSE ) + +upload_file_version(file, uuid, use_proxy = FALSE) } \arguments{ \item{file}{File path of document to upload} -\item{workspace_uuid}{UUID of the workspace to create the new document in} +\item{uuid}{For \code{upload_file()}, a workspace UUID to create the new document +in. For \code{upload_file_version()}, an asset UUID to create a new version of.} \item{name}{Name to give document. If this isn't provided, the name of the file will be used.} @@ -30,8 +34,12 @@ top-level of the workspace.} \item{use_proxy}{Logical to indicate whether to use proxy} } \value{ -An httr2 \link[=response]{httr2::response()} (invisibly) +API response (invisibly) } \description{ -Upload a file to create a new document +\itemize{ +\item Use \code{upload_file()} to create a new file in a workspace. +\item Use \code{upload_file_version()} to create a new version of an existing +document. +} } diff --git a/man/upload_file_version.Rd b/man/upload_file_version.Rd deleted file mode 100644 index 57c4a4a..0000000 --- a/man/upload_file_version.Rd +++ /dev/null @@ -1,18 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/upload.R -\name{upload_file_version} -\alias{upload_file_version} -\title{Upload a file to create a new document version} -\usage{ -upload_file_version(file, document_uuid, use_proxy = FALSE) -} -\arguments{ -\item{file}{File path of document to upload} - -\item{document_uuid}{UUID of existing document} - -\item{use_proxy}{Logical to indicate whether to use proxy} -} -\description{ -Upload a file to create a new document version -} diff --git a/man/write_data.Rd b/man/write_data.Rd index 6d13c52..bbfa6ca 100644 --- a/man/write_data.Rd +++ b/man/write_data.Rd @@ -2,28 +2,32 @@ % Please edit documentation in R/upload.R \name{write_data} \alias{write_data} -\title{Write a data file from R to create a new document} +\alias{write_data_version} +\title{Write a data file from R} \usage{ write_data( x, + uuid, file_name, file_type, - workspace_uuid, ..., description = NULL, parent_uuid = NULL, use_proxy = FALSE ) + +write_data_version(x, uuid, ..., use_proxy = FALSE) } \arguments{ \item{x}{R object to write to file.} +\item{uuid}{Either a workspace UUID to create a new document or an asset UUID +to create a new version of an existing document.} + \item{file_name}{Name to give file.} \item{file_type}{Either "csv", "rds" or "xlsx".} -\item{workspace_uuid}{UUID of the workspace to create the new document in} - \item{...}{Additional arguments to pass to write function. See details.} \item{description}{Optional description of document.} @@ -35,15 +39,20 @@ top-level of the workspace.} \item{use_proxy}{Logical to indicate whether to use proxy} } \value{ -An httr2 \link[=response]{httr2::response()} (invisibly) +API response (invisibly) } \description{ -Write a data file from R to create a new document +\itemize{ +\item Use \code{write_data()} to create a new file in a workspace. +\item Use \code{write_data_version()} to write a data file as a new version of an +existing document. +} } \details{ This function can be used to write the following data file types: -csv, rds, xlsx. Use the \code{file_type} argument to control which file type -to create. +csv, rds, xlsx. If writing to a new document, use the \code{file_type} +argument to control which file type to create. If writing a new version of an +existing document, the existing file type will be used. The function works by writing the R object to a temporary file and uploading the file to Objective Connect. The following functions are used to diff --git a/man/write_data_version.Rd b/man/write_data_version.Rd deleted file mode 100644 index 6638d72..0000000 --- a/man/write_data_version.Rd +++ /dev/null @@ -1,41 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/upload.R -\name{write_data_version} -\alias{write_data_version} -\title{Write a data file from R to create a new document version} -\usage{ -write_data_version(x, document_uuid, ..., use_proxy = FALSE) -} -\arguments{ -\item{x}{R object to write to file.} - -\item{document_uuid}{UUID of existing document} - -\item{...}{Additional arguments to pass to write function. See details.} - -\item{use_proxy}{Logical to indicate whether to use proxy} -} -\value{ -An httr2 \link[=response]{httr2::response()} (invisibly) -} -\description{ -Write a data file from R to create a new document version -} -\details{ -This function can be used to write the following data file types: -csv, rds, xlsx. The file type used is determined by the file type of the -existing document. - -The function works by writing the R object to a temporary file and uploading -the file to Objective Connect. The following functions are used to -write the data and any additional arguments (\code{...}) will be passed to these.\tabular{ll}{ - File Type \tab Function \cr - csv \tab \code{readr::write_csv()} \cr - rds \tab \code{readr::write_rds()} \cr - xlsx \tab \code{writexl::write_xlsx()} \cr -} - - -If there are other data file types you would like to upload using this -function, please \href{https://github.com/ScotGovAnalysis/objr/issues/new}{open an issue on the GitHub repository}. -} diff --git a/tests/testthat/test-upload.R b/tests/testthat/test-upload.R index 6e91249..5d73aa2 100644 --- a/tests/testthat/test-upload.R +++ b/tests/testthat/test-upload.R @@ -10,7 +10,7 @@ with_file("test", { expect_POST( upload_file(file = "test", - workspace_uuid = "test_workspace"), + uuid = "test_workspace"), paste0("https://secure.objectiveconnect.co.uk/publicapi/1/documents ", "Multipart form:\n ", "name = test\n ", @@ -21,7 +21,7 @@ with_file("test", { expect_POST( upload_file(file = "test", name = "test_file_name", - workspace_uuid = "test_workspace"), + uuid = "test_workspace"), paste0("https://secure.objectiveconnect.co.uk/publicapi/1/documents ", "Multipart form:\n ", "name = test_file_name\n ", @@ -31,7 +31,7 @@ with_file("test", { expect_POST( upload_file_version(file = "test", - document_uuid = "test_asset"), + uuid = "test_asset"), paste0("https://secure.objectiveconnect.co.uk/publicapi/1/", "documents/test_asset/upload ", "Multipart form:", "\n ", @@ -49,12 +49,12 @@ with_file("test", { expect_invisible( suppressMessages(upload_file_version(file = "test", - document_uuid = "test_asset")) + uuid = "test_asset")) ) expect_invisible( suppressMessages(upload_file(file = "test", - workspace_uuid = "test_workspace")) + uuid = "test_workspace")) ) }) @@ -62,7 +62,7 @@ with_file("test", { test_that("Function returns success message", { expect_message(upload_file_version(file = "test", - document_uuid = "test_asset")) + uuid = "test_asset")) }) @@ -79,7 +79,7 @@ without_internet({ write_data(head(mtcars), file_name = "test1", file_type = "csv", - workspace_uuid = "test_workspace"), + uuid = "test_workspace"), paste0("https://secure.objectiveconnect.co.uk/publicapi/1/documents ", "Multipart form:\n ", "name = test1\n ", From 3e513dbbf8a40f01983938a0c971a6c70ebc795c Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Thu, 31 Oct 2024 16:01:31 +0000 Subject: [PATCH 38/39] Add default values for size and page to docs --- R/assets.R | 3 +-- R/documents.R | 3 +-- R/workspaces.R | 4 ++-- man/assets.Rd | 4 ++-- man/comments.Rd | 4 ++-- man/versions.Rd | 4 ++-- man/workspaces.Rd | 4 ++-- 7 files changed, 12 insertions(+), 14 deletions(-) diff --git a/R/assets.R b/R/assets.R index 41bcee4..3877a46 100644 --- a/R/assets.R +++ b/R/assets.R @@ -3,9 +3,8 @@ #' @param workspace_uuid UUID of workspace #' @param type List of asset types to return. Defaults to all types; #' document, folder and link. -#' @param page Page number of responses to return (0..N). -#' @param size Number of results to be returned per page. #' @inheritParams objr +#' @inheritParams workspaces #' #' @return Tibble #' diff --git a/R/documents.R b/R/documents.R index 58b43ac..dbd1bad 100644 --- a/R/documents.R +++ b/R/documents.R @@ -1,9 +1,8 @@ #' Get data frame of document versions #' #' @param document_uuid UUID of document (asset) -#' @param page Page number of responses to return (0..N). -#' @param size Number of results to be returned per page. #' @inheritParams objr +#' @inheritParams workspaces #' #' @return Data frame #' diff --git a/R/workspaces.R b/R/workspaces.R index 03d2b0f..09a0e2a 100644 --- a/R/workspaces.R +++ b/R/workspaces.R @@ -1,8 +1,8 @@ #' Get workspaces the current user is a member of #' #' @param workgroup_uuid UUID of workgroup to filter by -#' @param page Page number of responses to return (0..N). -#' @param size Number of results to be returned per page. +#' @param page Page number of responses to return (0..N). Default is 0. +#' @param size Number of results to be returned per page. Default is 20. #' @inheritParams objr #' #' @return Data frame diff --git a/man/assets.Rd b/man/assets.Rd index dddf354..98fb4ba 100644 --- a/man/assets.Rd +++ b/man/assets.Rd @@ -18,9 +18,9 @@ assets( \item{type}{List of asset types to return. Defaults to all types; document, folder and link.} -\item{page}{Page number of responses to return (0..N).} +\item{page}{Page number of responses to return (0..N). Default is 0.} -\item{size}{Number of results to be returned per page.} +\item{size}{Number of results to be returned per page. Default is 20.} \item{use_proxy}{Logical to indicate whether to use proxy} } diff --git a/man/comments.Rd b/man/comments.Rd index f64abc0..1cf9db5 100644 --- a/man/comments.Rd +++ b/man/comments.Rd @@ -25,9 +25,9 @@ day will be included.} \item{workgroup_uuid}{UUID of workgroup to filter by} -\item{page}{Page number of responses to return (0..N).} +\item{page}{Page number of responses to return (0..N). Default is 0.} -\item{size}{Number of results to be returned per page.} +\item{size}{Number of results to be returned per page. Default is 20.} \item{use_proxy}{Logical to indicate whether to use proxy} } diff --git a/man/versions.Rd b/man/versions.Rd index 2946bb7..9fb50cc 100644 --- a/man/versions.Rd +++ b/man/versions.Rd @@ -9,9 +9,9 @@ versions(document_uuid, page = NULL, size = NULL, use_proxy = FALSE) \arguments{ \item{document_uuid}{UUID of document (asset)} -\item{page}{Page number of responses to return (0..N).} +\item{page}{Page number of responses to return (0..N). Default is 0.} -\item{size}{Number of results to be returned per page.} +\item{size}{Number of results to be returned per page. Default is 20.} \item{use_proxy}{Logical to indicate whether to use proxy} } diff --git a/man/workspaces.Rd b/man/workspaces.Rd index 6e95cbe..2a4318d 100644 --- a/man/workspaces.Rd +++ b/man/workspaces.Rd @@ -9,9 +9,9 @@ workspaces(workgroup_uuid = NULL, page = NULL, size = NULL, use_proxy = FALSE) \arguments{ \item{workgroup_uuid}{UUID of workgroup to filter by} -\item{page}{Page number of responses to return (0..N).} +\item{page}{Page number of responses to return (0..N). Default is 0.} -\item{size}{Number of results to be returned per page.} +\item{size}{Number of results to be returned per page. Default is 20.} \item{use_proxy}{Logical to indicate whether to use proxy} } From a7bab1915382a9fb2c2ab66808efb3028e871ed9 Mon Sep 17 00:00:00 2001 From: alice-hannah Date: Fri, 1 Nov 2024 09:43:00 +0000 Subject: [PATCH 39/39] Fix bug in download_helper Issue in reading temp file for read functions. Use on.exit to delete temp files. --- R/download.R | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/R/download.R b/R/download.R index d8a53e1..4997793 100644 --- a/R/download.R +++ b/R/download.R @@ -20,15 +20,16 @@ download_helper <- function(document_uuid, use_proxy = use_proxy ) - if (download_type == "download") { + # Rename file to match asset name + new_path <- rename_file(path, response, overwrite = overwrite) # nolint: object_usage_linter - # Rename file to match asset name - new_path <- rename_file(path, response, overwrite = overwrite) # nolint: object_usage_linter + if (download_type == "download") { # Show success message and return response invisibly if (httr2::resp_status(response) == 200) { cli::cli_alert_success("File downloaded: {.path {new_path}}.") } + invisible(response) } @@ -36,10 +37,11 @@ download_helper <- function(document_uuid, if (download_type == "read") { # Read data from file path - x <- read_temp(response$body[1], ...) + x <- read_temp(new_path, ...) + + # Delete temp file when exiting function + on.exit(unlink(c(path, new_path)), add = TRUE) - # Delete file created by download and return data - unlink(path) x }