From cc7ebe22d994f47bf8d3a114ec6317118a3a546e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Wed, 8 Mar 2023 12:21:47 +0100 Subject: [PATCH] [#173,#496] Report detailed errors on user delete. Modify the user delete pipeline (service, facade, DAO) to use OperationResult. HybridUserDAO is changed to reuse the same operation from Postgres DAO, since it did not have any code related to LDAP. --- model/dao/UserDAO/HybridUserDAO.php | 23 +--------- model/dao/UserDAO/PostgreSQLUserDAO.php | 55 +++++++++++++++++------- model/dao/UserDAO/UserDAO.php | 3 +- model/facade/UsersFacade.php | 10 ++--- model/facade/action/DeleteUserAction.php | 9 +--- web/services/deleteUsersService.php | 8 ++-- 6 files changed, 52 insertions(+), 56 deletions(-) diff --git a/model/dao/UserDAO/HybridUserDAO.php b/model/dao/UserDAO/HybridUserDAO.php index 77b42d780..417c87009 100644 --- a/model/dao/UserDAO/HybridUserDAO.php +++ b/model/dao/UserDAO/HybridUserDAO.php @@ -468,29 +468,10 @@ public function create(UserVO $userVO) { * This function deletes the data of a User by its {@link UserVO}. * * @param UserVO $userVO the {@link UserVO} 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(UserVO $userVO) { - $affectedRows = 0; - - // Check for a user ID. - if($userVO->getId() >= 0) { - $currUserVO = $this->getById($userVO->getId()); - } - - // Otherwise delete a user. - if(sizeof($currUserVO) > 0) { - $sql = "DELETE FROM usr WHERE id=".$userVO->getId(); - - $res = pg_query($this->connect, $sql); - - if ($res == NULL) throw new SQLQueryErrorException(pg_last_error()); - - $affectedRows = pg_affected_rows($res); - } - - return $affectedRows; + return parent::delete($userVO); } } diff --git a/model/dao/UserDAO/PostgreSQLUserDAO.php b/model/dao/UserDAO/PostgreSQLUserDAO.php index 587cd0ab6..c740a1c12 100644 --- a/model/dao/UserDAO/PostgreSQLUserDAO.php +++ b/model/dao/UserDAO/PostgreSQLUserDAO.php @@ -503,29 +503,52 @@ public function create(UserVO $userVO) { * This function deletes the data of a User by its {@link UserVO}. * * @param UserVO $userVO the {@link UserVO} 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(UserVO $userVO) { - $affectedRows = 0; - - // Check for a user ID. - if($userVO->getId() >= 0) { - $currUserVO = $this->getById($userVO->getId()); - } - - // Otherwise delete a user. - if(isset($currUserVO)) { - $sql = "DELETE FROM usr WHERE id=".$userVO->getId(); + $result = new OperationResult(false); - $res = pg_query($this->connect, $sql); + $sql = "DELETE FROM usr WHERE id=:id"; - if ($res == NULL) throw new SQLQueryErrorException(pg_last_error()); + try { + $statement = $this->pdo->prepare($sql); + $statement->bindValue(":id", $userVO->getId(), PDO::PARAM_INT); + $statement->execute(); - $affectedRows = pg_affected_rows($res); + $result->setIsSuccessful(true); + $result->setMessage('User updated successfully.'); + $result->setResponseCode(201); + } + catch (PDOException $ex) { + $errorMessage = $ex->getMessage(); + $resultMessage = "Error updating user:\n"; + if(strpos($errorMessage, "Foreign key violation")) { + $result->setResponseCode(403); + if(strpos($errorMessage, "task")) { + $resultMessage .= "The user has assigned tasks."; + } + else if(strpos($errorMessage, "project_usr")) { + $resultMessage .= "The user is assigned to projects."; + } + else if(strpos($errorMessage, "history")) { + $resultMessage .= "There is user history data associated to the user."; + } + else { + // catch-all for any foreign key violations that we may add in the future + $resultMessage .= "There is data associated to the user."; + error_log($errorMessage); + } + } + else { + $resultMessage .= $errorMessage; + $result->setResponseCode(500); + } + $result->setErrorNumber($ex->getCode()); + $result->setMessage($resultMessage); + $result->setIsSuccessful(false); } - return $affectedRows; + return $result; } } diff --git a/model/dao/UserDAO/UserDAO.php b/model/dao/UserDAO/UserDAO.php index 1849da816..270fbd2af 100644 --- a/model/dao/UserDAO/UserDAO.php +++ b/model/dao/UserDAO/UserDAO.php @@ -286,8 +286,7 @@ public abstract function create(UserVO $userVO); * This function deletes the data of a User by its {@link UserVO}. * * @param UserVO $userVO the {@link UserVO} 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}, {@link InvalidOperationException} + * @return OperationResult the result {@link OperationResult} with information about operation status */ public abstract function delete(UserVO $userVO); diff --git a/model/facade/UsersFacade.php b/model/facade/UsersFacade.php index fdf276ec4..9b935df83 100644 --- a/model/facade/UsersFacade.php +++ b/model/facade/UsersFacade.php @@ -180,15 +180,11 @@ static function CreateUser(UserVO $user) { * This function is used for deleting a User. * * @param UserVO $user the User value object we want to delete. - * @return int it just indicates if there was any error (-1) or not (0). - * @throws {@link SQLQueryErrorException} + * @return OperationResult the result {@link OperationResult} with information about operation status */ static function DeleteUser(UserVO $user) { - - $action = new DeleteUserAction($user); - - return $action->execute(); - + $action = new DeleteUserAction($user); + return $action->execute(); } /** Update User Function diff --git a/model/facade/action/DeleteUserAction.php b/model/facade/action/DeleteUserAction.php index 290a3e4d6..c12fd179a 100644 --- a/model/facade/action/DeleteUserAction.php +++ b/model/facade/action/DeleteUserAction.php @@ -68,16 +68,11 @@ public function __construct(UserVO $user) { * * This is the function that contains the code that deletes the User from persistent storing. * - * @return int it just indicates if there was any error (-1) or not (0). + * @return OperationResult the result {@link OperationResult} with information about operation status */ protected function doExecute() { - $dao = DAOFactory::getUserDAO(); - if ($dao->delete($this->user)!=1) { - return -1; - } - - return 0; + return $dao->delete($this->user); } } diff --git a/web/services/deleteUsersService.php b/web/services/deleteUsersService.php index 372695ab2..f4ae4994b 100644 --- a/web/services/deleteUsersService.php +++ b/web/services/deleteUsersService.php @@ -128,9 +128,11 @@ if (count($deleteUsers) >= 1) foreach((array)$deleteUsers as $user) { - if (UsersFacade::DeleteUser($user) == -1) - { - $string = "There was some error while deleting the users"; + $result = UsersFacade::DeleteUser($user); + if (!$result->getIsSuccessful()) { + http_response_code($result->getResponseCode()); + $string = "" . + $result->getMessage() . ""; break; } }