Add OperationResult class and begin handling sql errors more gracefully

model/OperationResult.php

@@ -0,0 +1,83 @@
+<?php
+/*
+ * Copyright (C) 2023 Igalia, S.L. <[email protected]>
+ *
+ * This file is part of PhpReport.
+ *
+ * PhpReport is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * PhpReport is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with PhpReport.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+/** File for OperationResult
+ *
+ *  This file just contains {@link OperationResult}.
+ *
+ * @filesource
+ * @package PhpReport
+ * @author Danielle Mayabb <[email protected]>
+ */
+
+/** Result returned after CRUD operations
+ *
+ *  This is the object returned for all CRUD operations
+ *
+ *  @property boolean $isSuccessful Whether the operation was successful or not
+ *  @property int $responseCode The http status code
+ *  @property int $errorNumber The php error number
+ *  @property string $message Error message that is passed up to front end. Can be php error or custom string.
+ */
+class OperationResult
+{
+    private $isSuccessful;
+    private $responseCode;
+    private $errorNumber;
+    private $message;
+
+    function __construct($isSuccessful, $responseCode = null) {
+        $this->isSuccessful = $isSuccessful;
+        $this->responseCode = $responseCode;
+    }
+
+    public function getIsSuccessful(){
+        return $this->isSuccessful;
+    }
+
+    public function setIsSuccessful($isSuccessful){
+        $this->isSuccessful = (boolean) $isSuccessful;
+    }
+
+    public function getResponseCode(){
+        return $this->responseCode;
+    }
+
+    public function setResponseCode($responseCode){
+        $this->responseCode = (int) $responseCode;
+    }
+
+    public function getErrorNumber(){
+        return $this->errorNumber;
+    }
+
+    public function setErrorNumber($errorNumber){
+        $this->errorNumber = (int) $errorNumber;
+    }
+
+    public function getMessage(){
+        return $this->message;
+    }
+
+    public function setMessage($message){
+        $this->message = (string) $message;
+    }
+}

model/dao/ProjectDAO/PostgreSQLProjectDAO.php

@@ -37,6 +37,7 @@
 include_once(PHPREPORT_ROOT . '/model/dao/ProjectDAO/ProjectDAO.php');
 include_once(PHPREPORT_ROOT . '/model/dao/ProjectUserDAO/PostgreSQLProjectUserDAO.php');
 include_once(PHPREPORT_ROOT . '/model/dao/TaskDAO/PostgreSQLTaskDAO.php');
+include_once(PHPREPORT_ROOT . '/model/OperationResult.php');
 
 /** DAO for Projects in PostgreSQL
  *
@@ -594,36 +595,73 @@ public function update(ProjectVO $projectVO) {
      * The internal id of <var>$projectVO</var> will be set after its creation.
      *
      * @param ProjectVO $projectVO the {@link ProjectVO} with the data we want to insert on database.
-     * @return int the number of rows that have been affected (it should be 1).
+     * @return OperationResult the result {@link OperationResult} with information about operation status
      * @throws {@link SQLQueryErrorException}
      */
     public function create(ProjectVO $projectVO) {
-        $affectedRows = 0;
+        $result = new OperationResult(false);
 
         $sql = "INSERT INTO project (activation, init, _end, invoice, est_hours,
-                areaid, customerid, type, description, moved_hours, sched_type) VALUES(" .
-                DBPostgres::boolToString($projectVO->getActivation()) . ", " .
-                DBPostgres::formatDate($projectVO->getInit()) . ", " .
-                DBPostgres::formatDate($projectVO->getEnd()) . ", " .
-                DBPostgres::checkNull($projectVO->getInvoice()) . ", " .
-                DBPostgres::checkNull($projectVO->getEstHours()) . ", " .
-                DBPostgres::checkNull($projectVO->getAreaId()) . ", " .
-                DBPostgres::checkNull($projectVO->getCustomerId()) . ", " .
-                DBPostgres::checkStringNull($projectVO->getType()) . ", " .
-                DBPostgres::checkStringNull($projectVO->getDescription()) . ", " .
-                DBPostgres::checkNull($projectVO->getMovedHours()) . ", " .
-                DBPostgres::checkStringNull($projectVO->getSchedType()) .")";
-
-        $res = pg_query($this->connect, $sql);
-
-        if ($res == NULL) throw new SQLQueryErrorException(pg_last_error());
-
-        $projectVO->setId(DBPostgres::getId($this->connect, "project_id_seq"));
-
-        $affectedRows = pg_affected_rows($res);
+        areaid, customerid, type, description, moved_hours, sched_type) VALUES(
+            :activation, :init, :end, :invoice, :est_hours,
+        :areaid, :customerid, :type, :description, :moved_hours, :sched_type)";
+
+        $initDateFormatted = (is_null($projectVO->getInit())) ? null : DBPostgres::formatDate($projectVO->getInit());
+        $endDateFormatted = (is_null($projectVO->getEnd())) ? null : DBPostgres::formatDate($projectVO->getEnd());
+
+        try {
+            $statement = $this->pdo->prepare($sql);
+            $statement->bindValue(":activation", $projectVO->getActivation(), PDO::PARAM_BOOL);
+            $statement->bindValue(":init", $initDateFormatted, PDO::PARAM_STR);
+            $statement->bindValue(":end", $endDateFormatted, PDO::PARAM_STR);
+            $statement->bindValue(":invoice", $projectVO->getInvoice(), PDO::PARAM_STR);
+            $statement->bindValue(":est_hours", $projectVO->getEstHours(), PDO::PARAM_STR);
+            $statement->bindValue(":areaid", $projectVO->getAreaId(), PDO::PARAM_INT);
+            $statement->bindValue(":customerid", $projectVO->getCustomerId(), PDO::PARAM_INT);
+            $statement->bindValue(":type", $projectVO->getType(), PDO::PARAM_STR);
+            $statement->bindValue(":description", $projectVO->getDescription(), PDO::PARAM_STR);
+            $statement->bindValue(":moved_hours", $projectVO->getMovedHours(), PDO::PARAM_STR);
+            $statement->bindValue(":sched_type", $projectVO->getSchedType(), PDO::PARAM_STR);
+            $statement->execute();
+
+            $projectVO->setId($this->pdo->lastInsertId('project_id_seq'));
+
+            $result->setIsSuccessful(true);
+            $result->setMessage('Project created successfully.');
+            $result->setResponseCode(201);
+        }
+        catch (PDOException $ex) {
+            //make sure to log the error as a failure, but return the OperationResult object
+            //successfully so that front end can see error and fail gracefully
+            $errorMessage = $ex->getMessage();
+            error_log('Project creation failed: ' . $errorMessage);
+            $result->setErrorNumber($ex->getCode());
+            $resultMessage = "Project creation failed: \n";
+
+            if(strpos($errorMessage, "Foreign key violation")){
+                if(strpos($errorMessage, "customerid")){
+                    $resultMessage .= "Customer not yet created. Please create customer first. \n";
+                }
+                if(strpos($errorMessage,"areaid")){
+                    $resultMessage .= "Area not yet created. Please create area first.";
+                }
+            }
+            else if (strpos($errorMessage, "Not null violation")){
+                if(strpos($errorMessage,"areaid")){
+                    $resultMessage .= "Area is null. Please choose area.";
+                }
+            }
+            else {
+                //if not a predictable error like FK/null violation, just return the native error code and message
+                $resultMessage .= $result . $errorMessage;
+            }
 
-        return $affectedRows;
+            $result->setMessage($resultMessage);
+            $result->setIsSuccessful(false);
+            $result->setResponseCode(500);
+        }
 
+        return $result;
     }
 
     /** Project deleter for PostgreSQL.

model/dao/ProjectDAO/ProjectDAO.php

@@ -210,7 +210,7 @@ public abstract function update(ProjectVO $projectVO);
      * The internal id of <var>$projectVO</var> will be set after its creation.
      *
      * @param ProjectVO $projectVO the {@link ProjectVO} with the data we want to insert on database.
-     * @return int the number of rows that have been affected (it should be 1).
+     * @return OperationResult the result {@link OperationResult} with information about operation status
      * @throws {@link OperationErrorException}
      */
     public abstract function create(ProjectVO $projectVO);
@@ -220,7 +220,7 @@ public abstract function create(ProjectVO $projectVO);
      * This function deletes the data of a Project by its {@link ProjectVO}.
      *
      * @param ProjectVO $projectVO the {@link ProjectVO} with the data we want to delete from database.
-     * @return int the number of rows that have been affected (it should be 1).
+     * @return OperationResult the result {@link OperationResult} with information about operation status
      * @throws {@link OperationErrorException}
      */
     public abstract function delete(ProjectVO $projectVO);

model/facade/ProjectsFacade.php

@@ -130,7 +130,7 @@ static function GetCustomProject($projectId) {
      *  This function is used for creating a new Project.
      *
      * @param ProjectVO $project the Project value object we want to create.
-     * @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
      * @throws {@link SQLQueryErrorException}
      */
     static function CreateProject(ProjectVO $project) {
@@ -147,17 +147,14 @@ static function CreateProject(ProjectVO $project) {
      *  If an error occurs, it stops creating.
      *
      * @param array $projects the Project value objects we want to create.
-     * @return int it just indicates if there was any error (<i>-1</i>) or not (<i>0</i>).
+     * @return array OperationResult the araray of results {@link OperationResult} with information about operation status
      * @throws {@link SQLQueryErrorException}
      */
     static function CreateProjects($projects) {
-
-    foreach((array)$projects as $project)
-        if ((ProjectsFacade::CreateProject($project)) == -1)
-            return -1;
-
-    return 0;
-
+        $operationResults = [];
+        foreach ((array) $projects as $project)
+            $operationResults[] = ProjectsFacade::CreateProject($project);
+        return $operationResults;
     }
 
     /** Delete Project Function

model/facade/action/CreateProjectAction.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/ProjectVO.php');
+include_once(PHPREPORT_ROOT . '/model/OperationResult.php');
 
 /** Create Project Action
  *
@@ -68,19 +69,13 @@ public function __construct(ProjectVO $project) {
      *
      * This is the function that contains the code that creates the new Project, storing it persistently.
      *
-     * @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
      * @throws {@link SQLQueryErrorException}
      */
     protected function doExecute() {
-
-    $dao = DAOFactory::getProjectDAO();
-        if ($dao->create($this->project)!=1) {
-            return -1;
-        }
-
-        return 0;
+        $dao = DAOFactory::getProjectDAO();
+        return $dao->create($this->project);
     }
-
 }
 
 

web/js/projectManagement.js

@@ -1106,9 +1106,14 @@ Ext.onReady(function(){
                 projectGrid.getSelectionModel().selectRow(rowIndex);
                 projectGrid.getView().focusRow(rowIndex);
             },
-            'exception': function(){
-                App.setAlert(false, "Some Error Occurred While Saving The Changes");
-            },
+            'exception': function(proxy, type, action, eOpts, res){
+                let parser = new DOMParser();
+                let errorDoc = parser.parseFromString(res.responseText, "text/xml");
+                let errorMessage = errorDoc.getElementsByTagName("error")[0].childNodes[0].nodeValue;
+                App.setAlert(false, errorMessage);
+                //if creation fails, reload data store so that "dirty" record from attempt doesn't persist
+                projectsStore.reload();
+            }
         }
     });
 

web/services/createProjectsService.php

@@ -30,6 +30,7 @@
     include_once(PHPREPORT_ROOT . '/web/services/WebServicesFunctions.php');
     include_once(PHPREPORT_ROOT . '/model/facade/ProjectsFacade.php');
     include_once(PHPREPORT_ROOT . '/model/vo/ProjectVO.php');
+    include_once(PHPREPORT_ROOT . '/model/OperationResult.php');
 
     $parser = new XMLReader();
 
@@ -208,11 +209,34 @@
 
 
     if (count($createProjects) >= 1)
-        if (ProjectsFacade::CreateProjects($createProjects) == -1)
-            $string = "<return service='createProjects'><error id='1'>There was some error while creating the projects</error></return>";
+        if(count($createProjects) == 1){
+            $operationResult = ProjectsFacade::CreateProject($createProjects[0]);
+            if (!$operationResult->getIsSuccessful()) {
+                http_response_code($operationResult->getResponseCode());
+                $string = "<return service='createProjects'><error id='" . $operationResult->getErrorNumber() . "'>" . $operationResult->getMessage() . "</error></return>";
+            }
+        }
+        else {
+            //leaving the possibility to create multiple projects in the future
+            $operationResults = ProjectsFacade::CreateProjects($createProjects);
+            $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='createProjects'><errors>";
+                    foreach((array) $errors as $result){
+                        if (!$result->getIsSuccessful())
+                            $string .= "<error id=''>" . $result->getMessage() . "</error>";
+                    }
+                $string .= "</errors></return>";
+            }
+        }
+
 
 
-    if (!$string)
+    if (!isset($string))
     {
         $string = "<return service='createProjects'><ok>Operation Success!</ok><projects>";