From 41d9d8ed948f2755d1ce1a81673f1b72c2c4c170 Mon Sep 17 00:00:00 2001
From: anmol thapar <mr.anmolthapar@gmail.com>
Date: Tue, 20 Aug 2024 16:38:29 +0100
Subject: [PATCH] fix: Handle non-existent task ids in get_status function

---
 R/queue.R                                   | 23 ++++++++++-----------
 inst/schema/report_run_status_response.json |  3 ++-
 tests/testthat/test-zzz-e2e.R               |  4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/R/queue.R b/R/queue.R
index f9420cd..57859f8 100644
--- a/R/queue.R
+++ b/R/queue.R
@@ -79,24 +79,23 @@ Queue <- R6::R6Class("Queue", # nolint
     #' @param include_logs Whether to include logs in response or not
     #' @return status of redis queue job
     get_status = function(task_ids, include_logs = TRUE) {
-      valid_task_ids <- task_ids[rrq::rrq_task_exists(task_ids, controller = self$controller)]
-      invalid_task_ids <- setdiff(task_ids, valid_task_ids)
+      invalid_task_ids <- task_ids[!rrq::rrq_task_exists(task_ids, controller = self$controller)]
       if (length(invalid_task_ids) > 0) {
-        cli::cli_warn(paste("Job ids", paste(invalid_task_ids, collapse = ", "), "do not exist in the queue"))
+        porcelain::porcelain_stop(paste("Job ids [", paste(invalid_task_ids, collapse = ", "), "] do not exist in the queue"))
       }
-      statuses <- rrq::rrq_task_status(valid_task_ids, controller = self$controller)
-      tasks_times <- rrq::rrq_task_times(valid_task_ids, controller = self$controller)
-      queuePositions <- rrq::rrq_task_position(valid_task_ids, controller = self$controller)
+      statuses <- rrq::rrq_task_status(task_ids, controller = self$controller)
+      tasks_times <- rrq::rrq_task_times(task_ids, controller = self$controller)
+      queuePositions <- rrq::rrq_task_position(task_ids, controller = self$controller)
 
-      lapply(seq_along(valid_task_ids), function(index) {
+      lapply(seq_along(task_ids), function(index) {
         list(
           status = scalar(statuses[index]),
           queuePosition = if (statuses[index] == "PENDING") scalar(queuePositions[index]) else NULL,
-          timeQueued = scalar(tasks_times[valid_task_ids[index], 1]),
-          timeStarted = scalar(tasks_times[valid_task_ids[index], 2]),
-          timeComplete = scalar(tasks_times[valid_task_ids[index], 3]),
-          packetId = if (statuses[index] == "COMPLETE") scalar(rrq::rrq_task_result(valid_task_ids[index], controller = self$controller)) else NULL,
-          logs = if (include_logs) rrq::rrq_task_log(valid_task_ids[index], controller = self$controller) else NULL
+          timeQueued = scalar(tasks_times[task_ids[index], 1]),
+          timeStarted = scalar(tasks_times[task_ids[index], 2]),
+          timeComplete = scalar(tasks_times[task_ids[index], 3]),
+          packetId = if (statuses[index] == "COMPLETE") scalar(rrq::rrq_task_result(task_ids[index], controller = self$controller)) else NULL,
+          logs = if (include_logs) rrq::rrq_task_log(task_ids[index], controller = self$controller) else NULL
         )
       })
     },
diff --git a/inst/schema/report_run_status_response.json b/inst/schema/report_run_status_response.json
index 09ef644..8338063 100644
--- a/inst/schema/report_run_status_response.json
+++ b/inst/schema/report_run_status_response.json
@@ -26,7 +26,8 @@
                 }
             },
             "status": {
-                "type": "string"
+                "type": "string",
+                "enum": ["PENDING", "RUNNING","COMPLETE", "ERROR", "CANCELLED", "DIED", "TIMEOUT", "IMPOSSIBLE","DEFERRED", "MOVED"]
             },
             "packetId": {
                 "type": ["string", "null"]
diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R
index 207ca4d..395edae 100644
--- a/tests/testthat/test-zzz-e2e.R
+++ b/tests/testthat/test-zzz-e2e.R
@@ -213,7 +213,7 @@ test_that("can get status of multiple tasks without logs", {
   }
 })
 
-test_that("returns status of only job ids that exist", {
+test_that("returns error with tasks ids of non-extant task ids", {
   # run report
   data <- list(
     name = "data",
@@ -237,5 +237,5 @@ test_that("returns status of only job ids that exist", {
     httr::content_type("application/json")
   )
   
-  expect_equal(length(httr::content(res)$data) , 1)
+  expect_equal(httr::content(res)$errors[[1]]$detail , "Job ids [ non-existant-id ] do not exist in the queue")
 })