[#496,#513] Return more detailed errors on task delete.

The DAO operation is also ported to PDO, addressing #513.
Igalia · Mar 8, 2023 · 13e9c2b · 13e9c2b
1 parent e49f20e
commit 13e9c2b
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 34 deletions.
diff --git a/model/dao/TaskDAO/PostgreSQLTaskDAO.php b/model/dao/TaskDAO/PostgreSQLTaskDAO.php
@@ -959,27 +959,33 @@ public function batchCreate($tasks) {
      * This function deletes the data of a Task by its {@link TaskVO}.
      *
      * @param TaskVO $taskVO the {@link TaskVO} with the data we want to delete from database.
-     * @return int the number of rows that have been affected (it should be 1).
-     * @throws {@link SQLQueryErrorException}
+     * @return OperationResult the result {@link OperationResult} with information about operation status
      */
     public function delete(TaskVO $taskVO) {
-        $affectedRows = 0;
-
-        // Check for a task ID.
-        if($taskVO->getId() >= 0) {
-            $currTaskVO = $this->getById($taskVO->getId());
-        }
+        $result = new OperationResult(false);
 
-        // Otherwise delete a task.
-        if(!is_null($currTaskVO)) {
-            $sql = "DELETE FROM task WHERE id=".$taskVO->getId();
+        $sql = "DELETE FROM task WHERE id=:id";
+        try {
+            $statement = $this->pdo->prepare($sql);
+            $statement->bindValue(":id", $taskVO->getId(), PDO::PARAM_INT);
+            $statement->execute();
 
-            $res = pg_query($this->connect, $sql);
-            if ($res == NULL) throw new SQLQueryErrorException(pg_last_error());
-                $affectedRows = pg_affected_rows($res);
+            $result->setIsSuccessful(true);
+            $result->setMessage('Task deleted successfully.');
+            $result->setResponseCode(200);
+        }
+        catch (PDOException $ex) {
+            $errorMessage = $ex->getMessage();
+            $resultMessage = "Task deletion failed: \n";
+            // unpredictable error, just return the native error code and message
+            $resultMessage .= $errorMessage;
+            $result->setErrorNumber($ex->getCode());
+            $result->setMessage($resultMessage);
+            $result->setIsSuccessful(false);
+            $result->setResponseCode(500);
         }
 
-        return $affectedRows;
+        return $result;
     }
 
     public function getLastTaskDate($userId, DateTime $date, $strictBeforeDate=True) {

diff --git a/model/dao/TaskDAO/TaskDAO.php b/model/dao/TaskDAO/TaskDAO.php
@@ -261,8 +261,7 @@ public abstract function batchCreate($tasks);
      * This function deletes the data of a Task by its {@link TaskVO}.
      *
      * @param TaskVO $taskVO the {@link TaskVO} with the data we want to delete from database.
-     * @return int the number of rows that have been affected (it should be 1).
-     * @throws {@link OperationErrorException}
+     * @return OperationResult the result {@link OperationResult} with information about operation status
      */
     public abstract function delete(TaskVO $taskVO);
 

diff --git a/model/facade/TasksFacade.php b/model/facade/TasksFacade.php
@@ -115,11 +115,11 @@ static function DeleteReport(TaskVO $task) {
      * @throws {@link SQLQueryErrorException}
      */
     static function DeleteReports($tasks) {
+        $results = array();
         foreach((array)$tasks as $task)
-            if ((TasksFacade::DeleteReport($task)) == -1)
-                return -1;
+            $results[] = TasksFacade::DeleteReport($task);
 
-        return 0;
+        return $results;
     }
 
     /** Partial Update Task Function

diff --git a/model/facade/action/DeleteReportAction.php b/model/facade/action/DeleteReportAction.php
@@ -32,6 +32,7 @@
 include_once(PHPREPORT_ROOT . '/model/facade/action/Action.php');
 include_once(PHPREPORT_ROOT . '/model/dao/DAOFactory.php');
 include_once(PHPREPORT_ROOT . '/model/vo/TaskVO.php');
+include_once(PHPREPORT_ROOT . '/model/OperationResult.php');
 
 /** Delete Task Action
  *
@@ -68,32 +69,42 @@ public function __construct(TaskVO $task) {
      *
      * This is the function that contains the code that deletes the Task from persistent storing.
      *
-     * @return int it just indicates if there was any error (<i>-1</i>) or not (<i>0</i>).
+     * @return OperationResult the result {@link OperationResult} with information about operation status
      */
     protected function doExecute() {
         //first check if current configuration allows deleting tasks in that date
         $configDao = DAOFactory::getConfigDAO();
         if(!$configDao->isWriteAllowedForDate($this->task->getDate())) {
-            return -1;
+            $result = new OperationResult(false);
+            $result->setErrorNumber(20);
+            $result->setResponseCode(500);
+            $result->setMessage("Error deleting task:\nNot allowed to write to date.");
+            return $result;
         }
 
         $dao = DAOFactory::getTaskDAO();
 
-        if (!$dao->checkTaskUserId($this->task->getId(), $this->task->getUserId()))
-            return -1;
+        if (!$dao->checkTaskUserId($this->task->getId(), $this->task->getUserId())) {
+            $result = new OperationResult(false);
+            $result->setErrorNumber(50);
+            $result->setResponseCode(500);
+            $result->setMessage("Error deleting task:\nBelongs to a different user.");
+            return $result;
+        }
 
         // Do not allow deleting tasks which belong to inactive projects
         $oldTask = $dao->getById($this->task->getId());
         $projectId = $oldTask->getProjectId();
         $projectVO = DAOFactory::getProjectDAO()->getById($projectId);
-        if (!$projectVO || !$projectVO->getActivation())
-            return -1;
-
-        if ($dao->delete($this->task)!=1) {
-            return -1;
+        if (!$projectVO || !$projectVO->getActivation()) {
+            $result = new OperationResult(false);
+            $result->setErrorNumber(30);
+            $result->setResponseCode(500);
+            $result->setMessage("Error updating task:\nNot allowed to write to project.");
+            return $result;
         }
 
-        return 0;
+        return $dao->delete($this->task);
     }
 
 }

diff --git a/web/services/deleteTasksService.php b/web/services/deleteTasksService.php
@@ -126,10 +126,20 @@
 
         } while ($parser->read());
 
-
-        if (count($deleteTasks) >= 1)
-            if (TasksFacade::DeleteReports($deleteTasks) == -1)
-                $string = "<return service='deleteTasks'><success>false</success><error id='1'>There was some error while deleting the tasks</error></return>";
+        $operationResults = TasksFacade::DeleteReports($deleteTasks);
+        $errors = array_filter($operationResults, function ($item) {
+            return (!$item->getIsSuccessful());
+        });
+        if ($errors) {
+            //if multiple failures, let's just return a 500
+            http_response_code(500);
+            $string = "<return service='deleteTasks'><errors>";
+            foreach((array) $errors as $result){
+                if (!$result->getIsSuccessful())
+                    $string .= "<error id='" . $result->getErrorNumber() . "'>" . $result->getMessage() . "</error>";
+            }
+            $string .= "</errors></return>";
+        }
 
         if (!isset($string))
             $string = "<return service='deleteTasks'><success>true</success><ok>Operation Success!</ok></return>";