From c26db543135c0d125ada1f24e0c255b2681286ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Fri, 24 Feb 2023 12:51:28 +0100 Subject: [PATCH] [#173] Allow to report multiple errors on the same creation batch. This allows batchCreate to return an array of OperationResults, each potentially containing one error, and makes the UI go through all of them to report on screen. --- model/dao/TaskDAO/PostgreSQLTaskDAO.php | 9 ++++----- model/dao/TaskDAO/TaskDAO.php | 2 +- model/facade/TasksFacade.php | 4 ++-- model/facade/action/CreateTasksAction.php | 23 +++++++++++++++-------- web/js/tasks.js | 5 ++++- web/services/createTasksService.php | 15 +++++++++++---- 6 files changed, 37 insertions(+), 21 deletions(-) diff --git a/model/dao/TaskDAO/PostgreSQLTaskDAO.php b/model/dao/TaskDAO/PostgreSQLTaskDAO.php index 72a597002..7cf0ed6c5 100644 --- a/model/dao/TaskDAO/PostgreSQLTaskDAO.php +++ b/model/dao/TaskDAO/PostgreSQLTaskDAO.php @@ -845,7 +845,7 @@ private function checkOverlappingTasks($tasks) { */ public function create(TaskVO $taskVO) { $tasks = array($taskVO); - return $this->batchCreate($tasks); + return $this->batchCreate($tasks)[0]; } /** Task creator for PostgreSQL. @@ -910,16 +910,15 @@ private function createInternal(TaskVO $taskVO) { * Equivalent to {@see create} for arrays of tasks. * * @param array $tasks array of {@link TaskVO} objects to be created. - * @return OperationResult the result {@link OperationResult} with information about operation status + * @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->setErrorNumber(10); $result->setMessage("Task creation failed:\nDetected overlapping times."); - $result->setIsSuccessful(false); $result->setResponseCode(500); - return $result; + return array($result); } $affectedRows = 0; @@ -932,7 +931,7 @@ public function batchCreate($tasks) { $result->setIsSuccessful(true); $result->setResponseCode(201); } - return $result; + return array($result); } /** Task deleter for PostgreSQL. diff --git a/model/dao/TaskDAO/TaskDAO.php b/model/dao/TaskDAO/TaskDAO.php index f35dded56..d202a6974 100644 --- a/model/dao/TaskDAO/TaskDAO.php +++ b/model/dao/TaskDAO/TaskDAO.php @@ -254,7 +254,7 @@ public abstract function create(TaskVO $taskVO); * Equivalent to {@see create} for arrays of tasks. * * @param array $tasks array of {@link TaskVO} objects to be created. - * @return OperationResult the result {@link OperationResult} with information about operation status + * @return array OperationResult the array of {@link OperationResult} with information about operation status */ public abstract function batchCreate($tasks); diff --git a/model/facade/TasksFacade.php b/model/facade/TasksFacade.php index b2444f3ae..c90ec0d5b 100644 --- a/model/facade/TasksFacade.php +++ b/model/facade/TasksFacade.php @@ -74,7 +74,7 @@ abstract class TasksFacade { * @return OperationResult the result {@link OperationResult} with information about operation status */ static function CreateReport(TaskVO $task) { - return TasksFacade::CreateReports(array($task)); + return TasksFacade::CreateReports(array($task))[0]; } /** Create Tasks Function @@ -85,7 +85,7 @@ static function CreateReport(TaskVO $task) { * @param array $tasks the Task value objects we want to create. The objects * contained in the array will be updated with the autogenerated task ID * field in case of success. - * @return OperationResult the result {@link OperationResult} with information about operation status + * @return array OperationResult the array of {@link OperationResult} with information about operation status */ static function CreateReports($tasks) { $action = new CreateTasksAction($tasks); diff --git a/model/facade/action/CreateTasksAction.php b/model/facade/action/CreateTasksAction.php index 10520b9e9..5e8127945 100644 --- a/model/facade/action/CreateTasksAction.php +++ b/model/facade/action/CreateTasksAction.php @@ -68,39 +68,46 @@ public function __construct($tasks) { * * Runs the action itself. * - * @return OperationResult the result {@link OperationResult} with information about operation status + * @return array OperationResult the array of {@link OperationResult} with information about operation status */ protected function doExecute() { $configDao = DAOFactory::getConfigDAO(); $taskDao = DAOFactory::getTaskDAO(); $projectDAO = DAOFactory::getProjectDAO(); $discardedTasks = array(); - $discardedReason = ""; + $discardedResults = array(); //first check permission on task write foreach ($this->tasks as $i => $task) { if (!$configDao->isWriteAllowedForDate($task->getDate())) { + $result = new OperationResult(false); + $result->setErrorNumber(20); + $result->setResponseCode(500); + $result->setMessage("Error creating task:\nNot allowed to write to date."); + $discardedResults[] = $result; $discardedTasks[] = $task; - $discardedReason .= "Not allowed to write to date.\n"; unset($this->tasks[$i]); continue; } $projectVO = $projectDAO->getById($task->getProjectId()); if (!$projectVO || !$projectVO->getActivation()) { + $result = new OperationResult(false); + $result->setErrorNumber(30); + $result->setResponseCode(500); + $result->setMessage("Error creating task:\nNot allowed to write to project."); $discardedTasks[] = $task; - $discardedReason .= "Not allowed to write to project.\n"; + $discardedResults[] = $result; unset($this->tasks[$i]); } } - $result = $taskDao->batchCreate($this->tasks); + $results = $taskDao->batchCreate($this->tasks); //TODO: do something meaningful with the list of discarded tasks if (!empty($discardedTasks)) { - $result = new OperationResult(false); - $result->setMessage("Some tasks were discarded:\n" . $discardedReason); + return array_merge($discardedResults, $results); } - return $result; + return $results; } } diff --git a/web/js/tasks.js b/web/js/tasks.js index a12fbf4f4..e53d1bf00 100644 --- a/web/js/tasks.js +++ b/web/js/tasks.js @@ -936,7 +936,10 @@ var tasksStore = new Ext.ux.TasksStore({ Ext.onReady(function () { // this may run in case of error in the initial load let parser = new DOMParser(); let errorDoc = parser.parseFromString(res.responseText, "text/xml"); - let errorMessage = errorDoc.getElementsByTagName("error")[0].childNodes[0].nodeValue; + let errorMessage = ""; + for (error of errorDoc.getElementsByTagName("error")) { + errorMessage += error.childNodes[0].nodeValue + "\n"; + } App.setAlert(false, errorMessage); tasksStore.error = true; }); diff --git a/web/services/createTasksService.php b/web/services/createTasksService.php index 28f09baa3..757f381a4 100644 --- a/web/services/createTasksService.php +++ b/web/services/createTasksService.php @@ -30,6 +30,7 @@ include_once(PHPREPORT_ROOT . '/web/services/WebServicesFunctions.php'); include_once(PHPREPORT_ROOT . '/model/facade/TasksFacade.php'); include_once(PHPREPORT_ROOT . '/model/vo/TaskVO.php'); + include_once(PHPREPORT_ROOT . '/model/OperationResult.php'); $parser = new XMLReader(); @@ -226,12 +227,18 @@ } while ($parser->read()); - $result = TasksFacade::CreateReports($createTasks); - if (!$result->getIsSuccessful()) { + $operationResults = TasksFacade::CreateReports($createTasks); + $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 = ""; - $string .= "" . $result->getMessage() . ""; + $string = ""; + foreach((array) $errors as $result){ + if (!$result->getIsSuccessful()) + $string .= "" . $result->getMessage() . ""; + } $string .= ""; }