[#492, #496] Manage and return SQL errors on task creation.

We capture SQL exceptions that could happen in task creation and wrap them into an OperationResult to be returned by the web service. Fixes #492 because it handles the situation that caused that error.
Igalia · Feb 28, 2023 · e6596c3 · e6596c3
1 parent c26db54
commit e6596c3
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 53 deletions.
diff --git a/model/dao/TaskDAO/PostgreSQLTaskDAO.php b/model/dao/TaskDAO/PostgreSQLTaskDAO.php
@@ -860,49 +860,51 @@ public function create(TaskVO $taskVO) {
      * @throws {@link SQLQueryErrorException}, {@link SQLUniqueViolationException}
      */
     private function createInternal(TaskVO $taskVO) {
-        $affectedRows = 0;
-
-        $sql = "INSERT INTO task (" .
-            "_date, " .
-            "init, " .
-            "_end, " .
-            "story, " .
-            "telework, " .
-            "onsite, " .
-            "text, " .
-            "ttype, " .
-            "phase, " .
-            "usrid, " .
-            "projectid, " .
-            "updated_at " .
-            ") VALUES(" .
-            DBPostgres::formatDate($taskVO->getDate()) . ", " .
-            DBPostgres::checkNull($taskVO->getInit()) . ", " .
-            DBPostgres::checkNull($taskVO->getEnd()) . ", " .
-            DBPostgres::checkStringNull($taskVO->getStory()) . ", " .
-            DBPostgres::boolToString($taskVO->getTelework()) . ", " .
-            DBPostgres::boolToString($taskVO->getOnsite()) . ", " .
-            DBPostgres::checkStringNull($taskVO->getText()) . ", " .
-            DBPostgres::checkStringNull($taskVO->getTtype()) . ", " .
-            DBPostgres::checkStringNull($taskVO->getPhase()) . ", " .
-            DBPostgres::checkNull($taskVO->getUserId()) . ", " .
-            DBPostgres::checkNull($taskVO->getProjectId()) . ", " .
-            "now()" .
-            ")" ;
-
-        $res = pg_query($this->connect, $sql);
-
-        if ($res == NULL)
-            if (strpos(pg_last_error(), "unique_task_usr_time"))
-                throw new SQLUniqueViolationException(pg_last_error());
-            else throw new SQLQueryErrorException(pg_last_error());
+        $result = new OperationResult(false);
 
-        $taskVO->setId(DBPostgres::getId($this->connect, "task_id_seq"));
+        $sql =
+            "INSERT INTO task (_date, init, _end, story, telework, onsite, " .
+                "text, ttype, phase, usrid, projectid, updated_at) " .
+            "VALUES (:date, :init, :end, :story, :telework, :onsite, " .
+                ":text, :ttype, :phase, :usrid, :projectid, now() )";
 
-        $affectedRows = pg_affected_rows($res);
+        try {
+            $statement = $this->pdo->prepare($sql);
+            $statement->bindValue(":date", DBPostgres::formatDate($taskVO->getDate()), PDO::PARAM_STR);
+            $statement->bindValue(":init", $taskVO->getInit(), PDO::PARAM_INT);
+            $statement->bindValue(":end", $taskVO->getEnd(), PDO::PARAM_INT);
+            $statement->bindValue(":story", $taskVO->getStory(), PDO::PARAM_STR);
+            $statement->bindValue(":telework", $taskVO->getTelework(), PDO::PARAM_BOOL);
+            $statement->bindValue(":onsite", $taskVO->getOnsite(), PDO::PARAM_BOOL);
+            $statement->bindValue(":text", $taskVO->getText(), PDO::PARAM_STR);
+            $statement->bindValue(":ttype", $taskVO->getTtype(), PDO::PARAM_STR);
+            $statement->bindValue(":phase", $taskVO->getPhase(), PDO::PARAM_STR);
+            $statement->bindValue(":usrid", $taskVO->getUserId(), PDO::PARAM_INT);
+            $statement->bindValue(":projectid", $taskVO->getProjectId(), PDO::PARAM_INT);
+            $statement->execute();
+
+            $taskVO->setId($this->pdo->lastInsertId('task_id_seq'));
 
-        return $affectedRows;
+            $result->setIsSuccessful(true);
+            $result->setMessage('Task created successfully.');
+            $result->setResponseCode(201);
+        }
+        catch (PDOException $ex) {
+            $errorMessage = $ex->getMessage();
+            $resultMessage = "Error creating task:\n";
+            if(strpos($errorMessage, "end_after_init_task")) {
+                $resultMessage .= "Start time later than end time.";
+            }
+            else {
+                $resultMessage .= $errorMessage;
+            }
+            $result->setErrorNumber($ex->getCode());
+            $result->setMessage($resultMessage);
+            $result->setIsSuccessful(false);
+            $result->setResponseCode(500);
+        }
 
+        return $result;
     }
 
     /** Task batch creator.
@@ -913,25 +915,20 @@ private function createInternal(TaskVO $taskVO) {
      * @return array OperationResult the array of {@link OperationResult} with information about operation status
      */
     public function batchCreate($tasks) {
-        $result = new OperationResult(false);
         if (!$this->checkOverlappingWithDBTasks($tasks)) {
+            $result = new OperationResult(false);
             $result->setErrorNumber(10);
             $result->setMessage("Task creation failed:\nDetected overlapping times.");
             $result->setResponseCode(500);
             return array($result);
         }
 
-        $affectedRows = 0;
-
+        $results = array();
         foreach ($tasks as $task) {
-            $affectedRows += $this->createInternal($task);
+            $results[] = $this->createInternal($task);
         }
 
-        if ($affectedRows == count($tasks)) {
-            $result->setIsSuccessful(true);
-            $result->setResponseCode(201);
-        }
-        return array($result);
+        return $results;
     }
 
     /** Task deleter for PostgreSQL.

diff --git a/model/vo/TaskVO.php b/model/vo/TaskVO.php
@@ -59,8 +59,8 @@ class TaskVO {
     protected $init = NULL;
     protected $_end = NULL;
     protected $story = NULL;
-    protected $telework = NULL;
-    protected $onsite = NULL;
+    protected $telework = false;
+    protected $onsite = false;
     protected $text = NULL;
     protected $ttype = NULL;
     protected $phase = NULL;

diff --git a/web/services/createTasksService.php b/web/services/createTasksService.php
@@ -78,8 +78,6 @@
 
                 $taskVO = new TaskVO();
 
-                $taskVO->setTelework(false);
-
                 $parser->read();
 
                 while ($parser->name != "task") {

diff --git a/web/services/updateTasksService.php b/web/services/updateTasksService.php
@@ -267,8 +267,10 @@
 
 
         if (count($updateTasks) >= 1)
-            if (TasksFacade::PartialUpdateReports($updateTasks) == -1)
+            if (TasksFacade::PartialUpdateReports($updateTasks) == -1) {
+                http_response_code(500);
                 $string = "<return service='updateTasks'><success>false</success><error id='1'>There was some error while updating the tasks</error></return>";
+            }
 
 
         if (!isset($string))