Improve API calls for subtasks

app/Controller/Subtask.php

@@ -58,7 +58,7 @@ class Subtask extends Base
         $task = $this->getTask();
         $values = $this->request->getValues();
 
-        list($valid, $errors) = $this->subTask->validate($values);
+        list($valid, $errors) = $this->subTask->validateCreation($values);
 
         if ($valid) {
 
@@ -119,7 +119,7 @@ class Subtask extends Base
         $subtask = $this->getSubtask();
 
         $values = $this->request->getValues();
-        list($valid, $errors) = $this->subTask->validate($values);
+        list($valid, $errors) = $this->subTask->validateModification($values);
 
         if ($valid) {
 

app/Model/SubTask.php

@@ -221,28 +221,65 @@ class SubTask extends Base
     }
 
     /**
-     * Validate creation/modification
+     * Validate creation
      *
      * @access public
      * @param  array   $values           Form values
      * @return array   $valid, $errors   [0] = Success or not, [1] = List of errors
      */
-    public function validate(array $values)
+    public function validateCreation(array $values)
     {
-        $v = new Validator($values, array(
+        $rules = array(
             new Validators\Required('task_id', t('The task id is required')),
-            new Validators\Integer('task_id', t('The task id must be an integer')),
             new Validators\Required('title', t('The title is required')),
-            new Validators\MaxLength('title', t('The maximum length is %d characters', 100), 100),
+        );
-            new Validators\Integer('user_id', t('The user id must be an integer')),
+
-            new Validators\Integer('status', t('The status must be an integer')),
+        $v = new Validator($values, $rules + $this->commonValidationRules());
-            new Validators\Numeric('time_estimated', t('The time must be a numeric value')),
-            new Validators\Numeric('time_spent', t('The time must be a numeric value')),
-        ));
 
         return array(
             $v->execute(),
             $v->getErrors()
         );
     }
+
+    /**
+     * Validate modification
+     *
+     * @access public
+     * @param  array   $values           Form values
+     * @return array   $valid, $errors   [0] = Success or not, [1] = List of errors
+     */
+    public function validateModification(array $values)
+    {
+        $rules = array(
+            new Validators\Required('id', t('The subtask id is required')),
+            new Validators\Required('task_id', t('The task id is required')),
+        );
+
+        $v = new Validator($values, $rules + $this->commonValidationRules());
+
+        return array(
+            $v->execute(),
+            $v->getErrors()
+        );
+    }
+
+    /**
+     * Common validation rules
+     *
+     * @access private
+     * @return array
+     */
+    private function commonValidationRules()
+    {
+        return array(
+            new Validators\Integer('id', t('The subtask id must be an integer')),
+            new Validators\Integer('task_id', t('The task id must be an integer')),
+            new Validators\MaxLength('title', t('The maximum length is %d characters', 100), 100),
+            new Validators\Integer('user_id', t('The user id must be an integer')),
+            new Validators\Integer('status', t('The status must be an integer')),
+            new Validators\Numeric('time_estimated', t('The time must be a numeric value')),
+            new Validators\Numeric('time_spent', t('The time must be a numeric value')),
+        );
+    }
 }

docs/api-json-rpc.markdown

@@ -1098,34 +1098,181 @@ Response example:
 ### createSubtask
 
 - Purpose: **Create a new subtask**
-- Parameters: Key/value pair composed of the **title** (integer), time_estimated (int, optional), task_id (int), user_id (int, optional)
+- Parameters:
+    - **task_id** (integer, required)
+    - **title** (integer, required)
+    - **assignee_id** (int, optional)
+    - **time_estimated** (int, optional)
+    - **time_spent** (int, optional)
+    - **status** (int, optional)
 - Result on success: **true**
 - Result on failure: **false**
 
+```json
+{
+    "jsonrpc": "2.0",
+    "method": "createSubtask",
+    "id": 2041554661,
+    "params": {
+        "task_id": 1,
+        "title": "Subtask #1"
+    }
+}
+```
+
+Response example:
+
+```json
+{
+    "jsonrpc": "2.0",
+    "id": 2041554661,
+    "result": true
+}
+```
+
 ### getSubtask
 
 - Purpose: **Get subtask information**
-- Parameters: **subtask_id** (integer)
+- Parameters:
+    - **subtask_id** (integer)
 - Result on success: **subtask properties**
 - Result on failure: **null**
 
+```json
+{
+    "jsonrpc": "2.0",
+    "method": "getSubtask",
+    "id": 133184525,
+    "params": {
+        "subtask_id": 1
+    }
+}
+```
+
+Response example:
+
+```json
+{
+    "jsonrpc": "2.0",
+    "id": 133184525,
+    "result": {
+        "id": "1",
+        "title": "Subtask #1",
+        "status": "0",
+        "time_estimated": "0",
+        "time_spent": "0",
+        "task_id": "1",
+        "user_id": "0"
+    }
+}
+```
+
 ### getAllSubtasks
 
 - Purpose: **Get all available subtasks**
-- Parameters: **none**
+- Parameters:
+    - **task_id** (integer, required)
 - Result on success: **List of subtasks**
 - Result on failure: **false**
 
+```json
+{
+    "jsonrpc": "2.0",
+    "method": "getAllSubtasks",
+    "id": 2087700490,
+    "params": {
+        "task_id": 1
+    }
+```
+
+Response example:
+
+```json
+{
+    "jsonrpc": "2.0",
+    "id": 2087700490,
+    "result": [
+        {
+            "id": "1",
+            "title": "Subtask #1",
+            "status": "0",
+            "time_estimated": "0",
+            "time_spent": "0",
+            "task_id": "1",
+            "user_id": "0",
+            "username": null,
+            "name": null,
+            "status_name": "Todo"
+        },
+        ...
+    ]
+}
+```
+
 ### updateSubtask
 
 - Purpose: **Update a subtask**
-- Parameters: Key/value pair composed of the **id** (integer), **title** (integer), status (integer, optional) time_estimated (int, optional), time_spent (int, optional), task_id (int), user_id (int, optional)
+- Parameters:
+    - **id** (integer, required)
+    - **task_id** (integer, required)
+    - **title** (integer, optional)
+    - **assignee_id** (integer, optional)
+    - **time_estimated (integer, optional)
+    - **time_spent** (integer, optional)
+    - **status** (integer, optional)
 - Result on success: **true**
 - Result on failure: **false**
 
+```json
+{
+    "jsonrpc": "2.0",
+    "method": "updateSubtask",
+    "id": 191749979,
+    "params": {
+        "id": 1,
+        "task_id": 1,
+        "status": 1,
+        "time_spent": 5,
+        "user_id": 1
+    }
+}
+```
+
+Response example:
+
+```json
+{
+    "jsonrpc": "2.0",
+    "id": 191749979,
+    "result": true
+}
+```
+
 ### removeSubtask
 
 - Purpose: **Remove a subtask**
-- Parameters: **subtask_id** (integer)
+- Parameters:
+    **subtask_id** (integer, required)
 - Result on success: **true**
 - Result on failure: **false**
+
+```json
+{
+    "jsonrpc": "2.0",
+    "method": "removeSubtask",
+    "id": 1382487306,
+    "params": {
+        "subtask_id": 1
+    }
+}
+```
+
+Response example:
+
+```json
+{
+    "jsonrpc": "2.0",
+    "id": 1382487306,
+    "result": true
+}
+```

jsonrpc.php

@@ -315,8 +315,24 @@ $server->register('removeComment', function($comment_id) use ($comment) {
 /**
  * Subtask procedures
  */
-$server->register('createSubtask', function(array $values) use ($subtask) {
+$server->register('createSubtask', function($task_id, $title, $user_id = 0, $time_estimated = 0, $time_spent = 0, $status = 0) use ($subtask) {
-    list($valid,) = $subtask->validate($values);
+
+    $values = array(
+        'title' => $title,
+        'task_id' => $task_id,
+        'user_id' => $user_id,
+        'time_estimated' => $time_estimated,
+        'time_spent' => $time_spent,
+        'status' => $status,
+    );
+
+    foreach ($values as $key => $value) {
+        if (is_null($value)) {
+            unset($values[$key]);
+        }
+    }
+
+    list($valid,) = $subtask->validateCreation($values);
     return $valid && $subtask->create($values);
 });
 
@@ -328,8 +344,25 @@ $server->register('getAllSubtasks', function($task_id) use ($subtask) {
     return $subtask->getAll($task_id);
 });
 
-$server->register('updateSubtask', function($values) use ($subtask) {
+$server->register('updateSubtask', function($id, $task_id, $title = null, $user_id = 0, $time_estimated = 0, $time_spent = 0, $status = 0) use ($subtask) {
-    list($valid,) = $subtask->validate($values);
+
+    $values = array(
+        'id' => $id,
+        'task_id' => $task_id,
+        'title' => $title,
+        'user_id' => $user_id,
+        'time_estimated' => $time_estimated,
+        'time_spent' => $time_spent,
+        'status' => $status,
+    );
+
+    foreach ($values as $key => $value) {
+        if (is_null($value)) {
+            unset($values[$key]);
+        }
+    }
+
+    list($valid,) = $subtask->validateModification($values);
     return $valid && $subtask->update($values);
 });
 

tests/functionals/ApiTest.php

@@ -345,7 +345,7 @@ class Api extends PHPUnit_Framework_TestCase
             'title' => 'subtask #1',
         );
 
-        $this->assertTrue($this->client->createSubtask($subtask));
+        $this->assertTrue($this->client->execute('createSubtask', $subtask));
     }
 
     public function testGetSubtask()
@@ -360,10 +360,12 @@ class Api extends PHPUnit_Framework_TestCase
 
     public function testUpdateSubtask()
     {
-        $subtask = $this->client->getSubtask(1);
+        $subtask = array();
+        $subtask['id'] = 1;
+        $subtask['task_id'] = 1;
         $subtask['title'] = 'test';
 
-        $this->assertTrue($this->client->updateSubtask($subtask));
+        $this->assertTrue($this->client->execute('updateSubtask', $subtask));
 
         $subtask = $this->client->getSubtask(1);
         $this->assertEquals('test', $subtask['title']);
@@ -377,7 +379,7 @@ class Api extends PHPUnit_Framework_TestCase
             'title' => 'Subtask #2',
         );
 
-        $this->assertTrue($this->client->createSubtask($subtask));
+        $this->assertTrue($this->client->execute('createSubtask', $subtask));
 
         $subtasks = $this->client->getAllSubtasks(1);
         $this->assertNotFalse($subtasks);