Skip to content

Commit 00088a4

Browse files
committed
[#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.
1 parent f470080 commit 00088a4

File tree

6 files changed

+37
-21
lines changed

6 files changed

+37
-21
lines changed

model/dao/TaskDAO/PostgreSQLTaskDAO.php

+4-5
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,7 @@ private function checkOverlappingTasks($tasks) {
845845
*/
846846
public function create(TaskVO $taskVO) {
847847
$tasks = array($taskVO);
848-
return $this->batchCreate($tasks);
848+
return $this->batchCreate($tasks)[0];
849849
}
850850

851851
/** Task creator for PostgreSQL.
@@ -910,16 +910,15 @@ private function createInternal(TaskVO $taskVO) {
910910
* Equivalent to {@see create} for arrays of tasks.
911911
*
912912
* @param array $tasks array of {@link TaskVO} objects to be created.
913-
* @return OperationResult the result {@link OperationResult} with information about operation status
913+
* @return array OperationResult the array of {@link OperationResult} with information about operation status
914914
*/
915915
public function batchCreate($tasks) {
916916
$result = new OperationResult(false);
917917
if (!$this->checkOverlappingWithDBTasks($tasks)) {
918918
$result->setErrorNumber(10);
919919
$result->setMessage("Task creation failed:\nDetected overlapping times.");
920-
$result->setIsSuccessful(false);
921920
$result->setResponseCode(500);
922-
return $result;
921+
return array($result);
923922
}
924923

925924
$affectedRows = 0;
@@ -932,7 +931,7 @@ public function batchCreate($tasks) {
932931
$result->setIsSuccessful(true);
933932
$result->setResponseCode(201);
934933
}
935-
return $result;
934+
return array($result);
936935
}
937936

938937
/** Task deleter for PostgreSQL.

model/dao/TaskDAO/TaskDAO.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ public abstract function create(TaskVO $taskVO);
254254
* Equivalent to {@see create} for arrays of tasks.
255255
*
256256
* @param array $tasks array of {@link TaskVO} objects to be created.
257-
* @return OperationResult the result {@link OperationResult} with information about operation status
257+
* @return array OperationResult the array of {@link OperationResult} with information about operation status
258258
*/
259259
public abstract function batchCreate($tasks);
260260

model/facade/TasksFacade.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ abstract class TasksFacade {
7474
* @return OperationResult the result {@link OperationResult} with information about operation status
7575
*/
7676
static function CreateReport(TaskVO $task) {
77-
return TasksFacade::CreateReports(array($task));
77+
return TasksFacade::CreateReports(array($task))[0];
7878
}
7979

8080
/** Create Tasks Function
@@ -85,7 +85,7 @@ static function CreateReport(TaskVO $task) {
8585
* @param array $tasks the Task value objects we want to create. The objects
8686
* contained in the array will be updated with the autogenerated task ID
8787
* field in case of success.
88-
* @return OperationResult the result {@link OperationResult} with information about operation status
88+
* @return array OperationResult the array of {@link OperationResult} with information about operation status
8989
*/
9090
static function CreateReports($tasks) {
9191
$action = new CreateTasksAction($tasks);

model/facade/action/CreateTasksAction.php

+15-8
Original file line numberDiff line numberDiff line change
@@ -68,39 +68,46 @@ public function __construct($tasks) {
6868
*
6969
* Runs the action itself.
7070
*
71-
* @return OperationResult the result {@link OperationResult} with information about operation status
71+
* @return array OperationResult the array of {@link OperationResult} with information about operation status
7272
*/
7373
protected function doExecute() {
7474
$configDao = DAOFactory::getConfigDAO();
7575
$taskDao = DAOFactory::getTaskDAO();
7676
$projectDAO = DAOFactory::getProjectDAO();
7777
$discardedTasks = array();
78-
$discardedReason = "";
78+
$discardedResults = array();
7979

8080
//first check permission on task write
8181
foreach ($this->tasks as $i => $task) {
8282
if (!$configDao->isWriteAllowedForDate($task->getDate())) {
83+
$result = new OperationResult(false);
84+
$result->setErrorNumber(20);
85+
$result->setResponseCode(500);
86+
$result->setMessage("Error creating task:\nNot allowed to write to date.");
87+
$discardedResults[] = $result;
8388
$discardedTasks[] = $task;
84-
$discardedReason .= "Not allowed to write to date.\n";
8589
unset($this->tasks[$i]);
8690
continue;
8791
}
8892
$projectVO = $projectDAO->getById($task->getProjectId());
8993
if (!$projectVO || !$projectVO->getActivation()) {
94+
$result = new OperationResult(false);
95+
$result->setErrorNumber(30);
96+
$result->setResponseCode(500);
97+
$result->setMessage("Error creating task:\nNot allowed to write to project.");
9098
$discardedTasks[] = $task;
91-
$discardedReason .= "Not allowed to write to project.\n";
99+
$discardedResults[] = $result;
92100
unset($this->tasks[$i]);
93101
}
94102
}
95103

96-
$result = $taskDao->batchCreate($this->tasks);
104+
$results = $taskDao->batchCreate($this->tasks);
97105

98106
//TODO: do something meaningful with the list of discarded tasks
99107
if (!empty($discardedTasks)) {
100-
$result = new OperationResult(false);
101-
$result->setMessage("Some tasks were discarded:\n" . $discardedReason);
108+
return array_merge($discardedResults, $results);
102109
}
103-
return $result;
110+
return $results;
104111
}
105112

106113
}

web/js/tasks.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -936,7 +936,10 @@ var tasksStore = new Ext.ux.TasksStore({
936936
Ext.onReady(function () { // this may run in case of error in the initial load
937937
let parser = new DOMParser();
938938
let errorDoc = parser.parseFromString(res.responseText, "text/xml");
939-
let errorMessage = errorDoc.getElementsByTagName("error")[0].childNodes[0].nodeValue;
939+
let errorMessage = "";
940+
for (error of errorDoc.getElementsByTagName("error")) {
941+
errorMessage += error.childNodes[0].nodeValue + "\n";
942+
}
940943
App.setAlert(false, errorMessage);
941944
tasksStore.error = true;
942945
});

web/services/createTasksService.php

+11-4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
include_once(PHPREPORT_ROOT . '/web/services/WebServicesFunctions.php');
3131
include_once(PHPREPORT_ROOT . '/model/facade/TasksFacade.php');
3232
include_once(PHPREPORT_ROOT . '/model/vo/TaskVO.php');
33+
include_once(PHPREPORT_ROOT . '/model/OperationResult.php');
3334

3435
$parser = new XMLReader();
3536

@@ -226,12 +227,18 @@
226227

227228
} while ($parser->read());
228229

229-
$result = TasksFacade::CreateReports($createTasks);
230-
if (!$result->getIsSuccessful()) {
230+
$operationResults = TasksFacade::CreateReports($createTasks);
231+
$errors = array_filter($operationResults, function ($item) {
232+
return (!$item->getIsSuccessful());
233+
});
234+
if ($errors) {
231235
//if multiple failures, let's just return a 500
232236
http_response_code(500);
233-
$string = "<return service='deleteProjects'><errors>";
234-
$string .= "<error id='" . $result->getErrorNumber() . "'>" . $result->getMessage() . "</error>";
237+
$string = "<return service='createTasks'><errors>";
238+
foreach((array) $errors as $result){
239+
if (!$result->getIsSuccessful())
240+
$string .= "<error id='" . $result->getErrorNumber() . "'>" . $result->getMessage() . "</error>";
241+
}
235242
$string .= "</errors></return>";
236243
}
237244

0 commit comments

Comments
 (0)