Skip to content

Commit f470080

Browse files
committedFeb 28, 2023
[#173] Report several kinds of errors on task creation.
We start reporting errors on the task create operation, in the following situations: * Overlapping tasks detected. * Trying to write to a date that has been locked. * Trying to assign a task to a project that's closed. These errors are detected prior to launching the SQL query. We haven't addressed SQL errors yet.
1 parent a2a96ce commit f470080

File tree

6 files changed

+39
-33
lines changed

6 files changed

+39
-33
lines changed
 

‎model/dao/TaskDAO/PostgreSQLTaskDAO.php

+13-7
Original file line numberDiff line numberDiff line change
@@ -841,8 +841,7 @@ private function checkOverlappingTasks($tasks) {
841841
* The internal id of <var>$taskVO</var> will be set after its creation.
842842
*
843843
* @param TaskVO $taskVO the {@link TaskVO} with the data we want to insert on database.
844-
* @return int the number of rows that have been affected (it should be 1).
845-
* @throws {@link SQLQueryErrorException}, {@link SQLUniqueViolationException}
844+
* @return OperationResult the result {@link OperationResult} with information about operation status
846845
*/
847846
public function create(TaskVO $taskVO) {
848847
$tasks = array($taskVO);
@@ -911,13 +910,16 @@ private function createInternal(TaskVO $taskVO) {
911910
* Equivalent to {@see create} for arrays of tasks.
912911
*
913912
* @param array $tasks array of {@link TaskVO} objects to be created.
914-
* @return int the number of rows that have been affected (it should be
915-
* equal to the size of $tasks).
916-
* @throws {@link SQLQueryErrorException}
913+
* @return OperationResult the result {@link OperationResult} with information about operation status
917914
*/
918915
public function batchCreate($tasks) {
916+
$result = new OperationResult(false);
919917
if (!$this->checkOverlappingWithDBTasks($tasks)) {
920-
return 0;
918+
$result->setErrorNumber(10);
919+
$result->setMessage("Task creation failed:\nDetected overlapping times.");
920+
$result->setIsSuccessful(false);
921+
$result->setResponseCode(500);
922+
return $result;
921923
}
922924

923925
$affectedRows = 0;
@@ -926,7 +928,11 @@ public function batchCreate($tasks) {
926928
$affectedRows += $this->createInternal($task);
927929
}
928930

929-
return $affectedRows;
931+
if ($affectedRows == count($tasks)) {
932+
$result->setIsSuccessful(true);
933+
$result->setResponseCode(201);
934+
}
935+
return $result;
930936
}
931937

932938
/** Task deleter for PostgreSQL.

‎model/dao/TaskDAO/TaskDAO.php

+2-5
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,7 @@ public abstract function batchPartialUpdate($tasks);
245245
* The internal id of <var>$taskVO</var> will be set after its creation.
246246
*
247247
* @param TaskVO $taskVO the {@link TaskVO} with the data we want to insert on database.
248-
* @return int the number of rows that have been affected (it should be 1).
249-
* @throws {@link OperationErrorException}, {@link SQLUniqueViolationException}
248+
* @return OperationResult the result {@link OperationResult} with information about operation status
250249
*/
251250
public abstract function create(TaskVO $taskVO);
252251

@@ -255,9 +254,7 @@ public abstract function create(TaskVO $taskVO);
255254
* Equivalent to {@see create} for arrays of tasks.
256255
*
257256
* @param array $tasks array of {@link TaskVO} objects to be created.
258-
* @return int the number of rows that have been affected (it should be
259-
* equal to the size of $tasks).
260-
* @throws {@link SQLQueryErrorException}
257+
* @return OperationResult the result {@link OperationResult} with information about operation status
261258
*/
262259
public abstract function batchCreate($tasks);
263260

‎model/facade/TasksFacade.php

+2-4
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,7 @@ abstract class TasksFacade {
7171
*
7272
* @param TaskVO $task the Task value object we want to create. It will be
7373
* updated with the autogenerated task ID field in case of success.
74-
* @return int it just indicates if there was any error (<i>-1</i>) or not (<i>0</i>).
75-
* @throws {@link SQLQueryErrorException}, {@link SQLUniqueViolationException}
74+
* @return OperationResult the result {@link OperationResult} with information about operation status
7675
*/
7776
static function CreateReport(TaskVO $task) {
7877
return TasksFacade::CreateReports(array($task));
@@ -86,8 +85,7 @@ static function CreateReport(TaskVO $task) {
8685
* @param array $tasks the Task value objects we want to create. The objects
8786
* contained in the array will be updated with the autogenerated task ID
8887
* field in case of success.
89-
* @return int it just indicates if there was any error (<i>-1</i>) or not (<i>0</i>).
90-
* @throws {@link SQLQueryErrorException}, {@link SQLUniqueViolationException}
88+
* @return OperationResult the result {@link OperationResult} with information about operation status
9189
*/
9290
static function CreateReports($tasks) {
9391
$action = new CreateTasksAction($tasks);

‎model/facade/action/CreateTasksAction.php

+9-10
Original file line numberDiff line numberDiff line change
@@ -68,40 +68,39 @@ public function __construct($tasks) {
6868
*
6969
* Runs the action itself.
7070
*
71-
* @return int it just indicates if there was any error (<i>-1</i>)
72-
* or not (<i>0</i>).
71+
* @return OperationResult the result {@link OperationResult} with information about operation status
7372
*/
7473
protected function doExecute() {
7574
$configDao = DAOFactory::getConfigDAO();
7675
$taskDao = DAOFactory::getTaskDAO();
7776
$projectDAO = DAOFactory::getProjectDAO();
7877
$discardedTasks = array();
78+
$discardedReason = "";
7979

8080
//first check permission on task write
8181
foreach ($this->tasks as $i => $task) {
8282
if (!$configDao->isWriteAllowedForDate($task->getDate())) {
8383
$discardedTasks[] = $task;
84+
$discardedReason .= "Not allowed to write to date.\n";
8485
unset($this->tasks[$i]);
8586
continue;
8687
}
8788
$projectVO = $projectDAO->getById($task->getProjectId());
8889
if (!$projectVO || !$projectVO->getActivation()) {
8990
$discardedTasks[] = $task;
91+
$discardedReason .= "Not allowed to write to project.\n";
9092
unset($this->tasks[$i]);
9193
}
9294
}
9395

94-
if ($taskDao->batchCreate($this->tasks) < count($this->tasks)) {
95-
return -1;
96-
}
96+
$result = $taskDao->batchCreate($this->tasks);
9797

9898
//TODO: do something meaningful with the list of discarded tasks
99-
if (empty($discardedTasks)) {
100-
return 0;
101-
}
102-
else {
103-
return -1;
99+
if (!empty($discardedTasks)) {
100+
$result = new OperationResult(false);
101+
$result->setMessage("Some tasks were discarded:\n" . $discardedReason);
104102
}
103+
return $result;
105104
}
106105

107106
}

‎web/js/tasks.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -932,9 +932,12 @@ var tasksStore = new Ext.ux.TasksStore({
932932
}
933933
tasksStore.error = false;
934934
},
935-
'exception': function () {
935+
'exception': function(proxy, type, action, eOpts, res) {
936936
Ext.onReady(function () { // this may run in case of error in the initial load
937-
App.setAlert(false, "Some Error Occurred While Saving The Changes (please check you haven't clipped working hours)");
937+
let parser = new DOMParser();
938+
let errorDoc = parser.parseFromString(res.responseText, "text/xml");
939+
let errorMessage = errorDoc.getElementsByTagName("error")[0].childNodes[0].nodeValue;
940+
App.setAlert(false, errorMessage);
938941
tasksStore.error = true;
939942
});
940943
},

‎web/services/createTasksService.php

+8-5
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,14 @@
226226

227227
} while ($parser->read());
228228

229-
230-
if (count($createTasks) >= 1)
231-
if (TasksFacade::CreateReports($createTasks) == -1)
232-
$string = "<return service='createTasks'><success>false</success><error id='1'>There was some error while creating the tasks</error></return>";
233-
229+
$result = TasksFacade::CreateReports($createTasks);
230+
if (!$result->getIsSuccessful()) {
231+
//if multiple failures, let's just return a 500
232+
http_response_code(500);
233+
$string = "<return service='deleteProjects'><errors>";
234+
$string .= "<error id='" . $result->getErrorNumber() . "'>" . $result->getMessage() . "</error>";
235+
$string .= "</errors></return>";
236+
}
234237

235238
if (!isset($string))
236239
{

0 commit comments

Comments
 (0)
Please sign in to comment.