Skip to content

Commit

Permalink
[#173] Allow to report multiple errors on the same creation batch.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jaragunde committed Feb 28, 2023
1 parent 8b762f4 commit c26db54
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 21 deletions.
9 changes: 4 additions & 5 deletions model/dao/TaskDAO/PostgreSQLTaskDAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -932,7 +931,7 @@ public function batchCreate($tasks) {
$result->setIsSuccessful(true);
$result->setResponseCode(201);
}
return $result;
return array($result);
}

/** Task deleter for PostgreSQL.
Expand Down
2 changes: 1 addition & 1 deletion model/dao/TaskDAO/TaskDAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions model/facade/TasksFacade.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
23 changes: 15 additions & 8 deletions model/facade/action/CreateTasksAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

}
5 changes: 4 additions & 1 deletion web/js/tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down
15 changes: 11 additions & 4 deletions web/services/createTasksService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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 = "<return service='deleteProjects'><errors>";
$string .= "<error id='" . $result->getErrorNumber() . "'>" . $result->getMessage() . "</error>";
$string = "<return service='createTasks'><errors>";
foreach((array) $errors as $result){
if (!$result->getIsSuccessful())
$string .= "<error id='" . $result->getErrorNumber() . "'>" . $result->getMessage() . "</error>";
}
$string .= "</errors></return>";
}

Expand Down

0 comments on commit c26db54

Please sign in to comment.