Improve API calls for users

8 files changed, 291 insertions, 35 deletions

diff --git a/app/Model/User.php b/app/Model/User.php
index b8411533..c1a9dcc9 100644
--- a/app/Model/User.php
+++ b/app/Model/User.php
@@ -224,12 +224,12 @@ class User extends Base
         $this->db->startTransaction();
 
         // All tasks assigned to this user will be unassigned
-        $this->db->table(Task::TABLE)->eq('owner_id', $user_id)->update(array('owner_id' => ''));
-        $this->db->table(self::TABLE)->eq('id', $user_id)->remove();
+        $this->db->table(Task::TABLE)->eq('owner_id', $user_id)->update(array('owner_id' => 0));
+        $result = $this->db->table(self::TABLE)->eq('id', $user_id)->remove();
 
         $this->db->closeTransaction();
 
-        return true;
+        return $result;
     }
 
     /**
@@ -265,7 +265,6 @@ class User extends Base
     private function commonValidationRules()
     {
         return array(
-            new Validators\Required('username', t('The username is required')),
             new Validators\MaxLength('username', t('The maximum length is %d characters', 50), 50),
             new Validators\Unique('username', t('The username must be unique'), $this->db->getConnection(), self::TABLE, 'id'),
             new Validators\Email('email', t('Email address invalid')),
@@ -299,7 +298,11 @@ class User extends Base
      */
     public function validateCreation(array $values)
     {
-        $v = new Validator($values, array_merge($this->commonValidationRules(), $this->commonPasswordValidationRules()));
+        $rules = array(
+            new Validators\Required('username', t('The username is required')),
+        );
+
+        $v = new Validator($values, array_merge($rules, $this->commonValidationRules(), $this->commonPasswordValidationRules()));
 
         return array(
             $v->execute(),
@@ -318,6 +321,28 @@ class User extends Base
     {
         $rules = array(
             new Validators\Required('id', t('The user id is required')),
+            new Validators\Required('username', t('The username is required')),
+        );
+
+        $v = new Validator($values, array_merge($rules, $this->commonValidationRules()));
+
+        return array(
+            $v->execute(),
+            $v->getErrors()
+        );
+    }
+
+    /**
+     * Validate user API modification
+     *
+     * @access public
+     * @param  array   $values           Form values
+     * @return array   $valid, $errors   [0] = Success or not, [1] = List of errors
+     */
+    public function validateApiModification(array $values)
+    {
+        $rules = array(
+            new Validators\Required('id', t('The user id is required')),
         );
 
         $v = new Validator($values, array_merge($rules, $this->commonValidationRules()));
diff --git a/app/Schema/Mysql.php b/app/Schema/Mysql.php
index 86685067..196fb856 100644
--- a/app/Schema/Mysql.php
+++ b/app/Schema/Mysql.php
@@ -4,7 +4,12 @@ namespace Schema;
 
 use Core\Security;
 
-const VERSION = 26;
+const VERSION = 27;
+
+function version_27($pdo)
+{
+    $pdo->exec('CREATE UNIQUE INDEX users_username_idx ON users(username)');
+}
 
 function version_26($pdo)
 {
diff --git a/app/Schema/Postgres.php b/app/Schema/Postgres.php
index a7e57d66..3341d4a1 100644
--- a/app/Schema/Postgres.php
+++ b/app/Schema/Postgres.php
@@ -4,7 +4,12 @@ namespace Schema;
 
 use Core\Security;
 
-const VERSION = 7;
+const VERSION = 8;
+
+function version_8($pdo)
+{
+    $pdo->exec('CREATE UNIQUE INDEX users_username_idx ON users(username)');
+}
 
 function version_7($pdo)
 {
diff --git a/app/Schema/Sqlite.php b/app/Schema/Sqlite.php
index cab69fa0..108c07f2 100644
--- a/app/Schema/Sqlite.php
+++ b/app/Schema/Sqlite.php
@@ -4,7 +4,12 @@ namespace Schema;
 
 use Core\Security;
 
-const VERSION = 26;
+const VERSION = 27;
+
+function version_27($pdo)
+{
+    $pdo->exec('CREATE UNIQUE INDEX users_username_idx ON users(username)');
+}
 
 function version_26($pdo)
 {
diff --git a/docs/api-json-rpc.markdown b/docs/api-json-rpc.markdown
index 3fa61384..d32da9af 100644
--- a/docs/api-json-rpc.markdown
+++ b/docs/api-json-rpc.markdown
@@ -967,96 +967,191 @@ Response example:
 ### createUser
 
 - Purpose: **Create a new user**
-- Parameters: Key/value pair composed of the **username** (string), **password** (string), **confirmation** (string), **name** (string, optional), **email** (string, optional), is_admin (integer, optional), **default_project_id** (integer, optional)
+- Parameters:
+    - **username** Must be unique (string, required)
+    - **password** Must have at least 6 characters (string, required)
+    - **name** (string, optional)
+    - **email** (string, optional)
+    - **is_admin** Set the value 1 for admins or 0 for regular users (integer, optional)
+    - **default_project_id** (integer, optional)
 - Result on success: **true**
 - Result on failure: **false**
 
 Request example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "method": "createUser",
+    "id": 1518863034,
+    "params": {
+        "username": "biloute",
+        "password": "123456"
+    }
+}
 ```
 
 Response example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "id": 1518863034,
+    "result": true
+}
 ```
 
 ### getUser
 
 - Purpose: **Get user information**
-- Parameters: **user_id** (integer)
+- Parameters:
+    - **user_id** (integer, required)
 - Result on success: **user properties**
 - Result on failure: **null**
 
 Request example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "method": "getUser",
+    "id": 1769674781,
+    "params": {
+        "user_id": 1
+    }
+}
 ```
 
 Response example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "id": 1769674781,
+    "result": {
+        "id": "1",
+        "username": "biloute",
+        "password": "$2y$10$dRs6pPoBu935RpmsrhmbjevJH5MgZ7Kr9QrnVINwwyZ3.MOwqg.0m",
+        "is_admin": "0",
+        "default_project_id": "0",
+        "is_ldap_user": "0",
+        "name": "",
+        "email": "",
+        "google_id": null,
+        "github_id": null,
+        "notifications_enabled": "0"
+    }
+}
 ```
 
 ### getAllUsers
 
 - Purpose: **Get all available users**
-- Parameters: **none**
+- Parameters:
+    - **none**
 - Result on success: **List of users**
 - Result on failure: **false**
 
 Request example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "method": "getAllUsers",
+    "id": 1438712131
+}
 ```
 
 Response example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "id": 1438712131,
+    "result": [
+        {
+            "id": "1",
+            "username": "biloute",
+            "name": "",
+            "email": "",
+            "is_admin": "0",
+            "default_project_id": "0",
+            "is_ldap_user": "0",
+            "notifications_enabled": "0",
+            "google_id": null,
+            "github_id": null
+        },
+        ...
+    ]
+}
 ```
 
 ### updateUser
 
 - Purpose: **Update a user**
-- Parameters: Key/value pair composed of the **id** (integer), **username** (string), **password** (string), **confirmation** (string), **name** (string, optional), **email** (string, optional), is_admin (integer, optional), **default_project_id** (integer, optional)
+- Parameters:
+    - **id** (integer)
+    - **username** (string, optional)
+    - **name** (string, optional)
+    - **email** (string, optional)
+    - **is_admin** (integer, optional)
+    - **default_project_id** (integer, optional)
 - Result on success: **true**
 - Result on failure: **false**
 
 Request example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "method": "updateUser",
+    "id": 322123657,
+    "params": {
+        "id": 1,
+        "is_admin": 1
+    }
+}
 ```
 
 Response example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "id": 322123657,
+    "result": true
+}
 ```
 
 ### removeUser
 
 - Purpose: **Remove a user**
-- Parameters: **user_id** (integer)
+- Parameters:
+    - **user_id** (integer, required)
 - Result on success: **true**
 - Result on failure: **false**
 
 Request example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "method": "removeUser",
+    "id": 2094191872,
+    "params": {
+        "user_id": 1
+    }
+}
 ```
 
 Response example:
 
 ```json
-
+{
+    "jsonrpc": "2.0",
+    "id": 2094191872,
+    "result": true
+}
 ```
 
 
diff --git a/jsonrpc.php b/jsonrpc.php
index f7fab315..1a411718 100644
--- a/jsonrpc.php
+++ b/jsonrpc.php
@@ -135,6 +135,10 @@ $server->register('removeColumn', function($column_id) use ($board) {
     return $board->removeColumn($column_id);
 });
 
+
+/**
+ * Projects permissions procedures
+ */
 $server->register('getAllowedUsers', function($project_id) use ($project) {
     return $project->getUsersList($project_id, false, false);
 });
@@ -224,7 +228,18 @@ $server->register('moveTaskPosition', function($project_id, $task_id, $column_id
 /**
  * User procedures
  */
-$server->register('createUser', function(array $values) use ($user) {
+$server->register('createUser', function($username, $password, $name = '', $email = '', $is_admin = 0, $default_project_id = 0) use ($user) {
+
+    $values = array(
+        'username' => $username,
+        'password' => $password,
+        'confirmation' => $password,
+        'name' => $name,
+        'email' => $email,
+        'is_admin' => $is_admin,
+        'default_project_id' => $default_project_id,
+    );
+
     list($valid,) = $user->validateCreation($values);
     return $valid && $user->create($values);
 });
@@ -237,8 +252,24 @@ $server->register('getAllUsers', function() use ($user) {
     return $user->getAll();
 });
 
-$server->register('updateUser', function($values) use ($user) {
-    list($valid,) = $user->validateModification($values);
+$server->register('updateUser', function($id, $username = null, $name = null, $email = null, $is_admin = null, $default_project_id = null) use ($user) {
+
+    $values = array(
+        'id' => $id,
+        'username' => $username,
+        'name' => $name,
+        'email' => $email,
+        'is_admin' => $is_admin,
+        'default_project_id' => $default_project_id,
+    );
+
+    foreach ($values as $key => $value) {
+        if (is_null($value)) {
+            unset($values[$key]);
+        }
+    }
+
+    list($valid,) = $user->validateApiModification($values);
     return $valid && $user->update($values);
 });
 
@@ -356,7 +387,7 @@ $server->register('getAllSubtasks', function($task_id) use ($subtask) {
     return $subtask->getAll($task_id);
 });
 
-$server->register('updateSubtask', function($id, $task_id, $title = null, $user_id = 0, $time_estimated = 0, $time_spent = 0, $status = 0) use ($subtask) {
+$server->register('updateSubtask', function($id, $task_id, $title = null, $user_id = null, $time_estimated = null, $time_spent = null, $status = null) use ($subtask) {
 
     $values = array(
         'id' => $id,
diff --git a/tests/functionals/ApiTest.php b/tests/functionals/ApiTest.php
index e633c598..04b22856 100644
--- a/tests/functionals/ApiTest.php
+++ b/tests/functionals/ApiTest.php
@@ -233,28 +233,26 @@ class Api extends PHPUnit_Framework_TestCase
             'username' => 'toto',
             'name' => 'Toto',
             'password' => '123456',
-            'confirmation' => '123456',
         );
 
-        $this->assertTrue($this->client->createUser($user));
+        $this->assertTrue($this->client->execute('createUser', $user));
 
         $user = array(
-            'username' => 'titi',
             'name' => 'Titi',
             'password' => '123456',
-            'confirmation' => '789',
         );
 
-        $this->assertFalse($this->client->createUser($user));
+        $this->assertNull($this->client->execute('createUser', $user));
     }
 
     public function testGetUser()
     {
         $user = $this->client->getUser(2);
-
         $this->assertNotFalse($user);
         $this->assertTrue(is_array($user));
         $this->assertEquals('toto', $user['username']);
+
+        $this->assertNull($this->client->getUser(2222));
     }
 
     public function testUpdateUser()
@@ -264,14 +262,24 @@ class Api extends PHPUnit_Framework_TestCase
         $user['username'] = 'titi';
         $user['name'] = 'Titi';
 
-        $this->assertTrue($this->client->updateUser($user));
+        $this->assertTrue($this->client->execute('updateUser', $user));
 
         $user = $this->client->getUser(2);
-
         $this->assertNotFalse($user);
         $this->assertTrue(is_array($user));
         $this->assertEquals('titi', $user['username']);
         $this->assertEquals('Titi', $user['name']);
+
+        $user = array();
+        $user['id'] = 2;
+        $user['email'] = 'titi@localhost';
+
+        $this->assertTrue($this->client->execute('updateUser', $user));
+
+        $user = $this->client->getUser(2);
+        $this->assertNotFalse($user);
+        $this->assertTrue(is_array($user));
+        $this->assertEquals('titi@localhost', $user['email']);
     }
 
     public function testGetAllowedUsers()
diff --git a/tests/units/UserTest.php b/tests/units/UserTest.php
new file mode 100644
index 00000000..cc0b7c44
--- /dev/null
+++ b/tests/units/UserTest.php
@@ -0,0 +1,82 @@
+<?php
+
+require_once __DIR__.'/Base.php';
+
+use Model\User;
+use Model\Task;
+use Model\Project;
+
+class UserTest extends Base
+{
+    public function testCreate()
+    {
+        $u = new User($this->registry);
+        $this->assertTrue($u->create(array('username' => 'toto', 'password' => '123456', 'name' => 'Toto')));
+        $this->assertTrue($u->create(array('username' => 'titi', 'is_ldap_user' => 1)));
+        $this->assertFalse($u->create(array('username' => 'toto')));
+
+        $user = $u->getById(1);
+        $this->assertNotFalse($user);
+        $this->assertTrue(is_array($user));
+        $this->assertEquals('admin', $user['username']);
+        $this->assertEquals('', $user['name']);
+        $this->assertEquals(1, $user['is_admin']);
+        $this->assertEquals(0, $user['is_ldap_user']);
+
+        $user = $u->getById(2);
+        $this->assertNotFalse($user);
+        $this->assertTrue(is_array($user));
+        $this->assertEquals('toto', $user['username']);
+        $this->assertEquals('Toto', $user['name']);
+        $this->assertEquals(0, $user['is_admin']);
+        $this->assertEquals(0, $user['is_ldap_user']);
+
+        $user = $u->getById(3);
+        $this->assertNotFalse($user);
+        $this->assertTrue(is_array($user));
+        $this->assertEquals('titi', $user['username']);
+        $this->assertEquals('', $user['name']);
+        $this->assertEquals(0, $user['is_admin']);
+        $this->assertEquals(1, $user['is_ldap_user']);
+    }
+
+    public function testUpdate()
+    {
+        $u = new User($this->registry);
+        $this->assertTrue($u->create(array('username' => 'toto', 'password' => '123456', 'name' => 'Toto')));
+        $this->assertTrue($u->update(array('id' => 2, 'username' => 'biloute')));
+
+        $user = $u->getById(2);
+        $this->assertNotFalse($user);
+        $this->assertTrue(is_array($user));
+        $this->assertEquals('biloute', $user['username']);
+        $this->assertEquals('Toto', $user['name']);
+        $this->assertEquals(0, $user['is_admin']);
+        $this->assertEquals(0, $user['is_ldap_user']);
+    }
+
+    public function testRemove()
+    {
+        $u = new User($this->registry);
+        $t = new Task($this->registry);
+        $p = new Project($this->registry);
+
+        $this->assertTrue($u->create(array('username' => 'toto', 'password' => '123456', 'name' => 'Toto')));
+        $this->assertEquals(1, $p->create(array('name' => 'Project #1')));
+        $this->assertEquals(1, $t->create(array('title' => 'Task #1', 'project_id' => 1, 'owner_id' => 2)));
+
+        $task = $t->getById(1);
+        $this->assertEquals(1, $task['id']);
+        $this->assertEquals(2, $task['owner_id']);
+
+        $this->assertTrue($u->remove(1));
+        $this->assertTrue($u->remove(2));
+        $this->assertFalse($u->remove(2));
+        $this->assertFalse($u->remove(55));
+
+        // Make sure that assigned tasks are unassigned after removing the user
+        $task = $t->getById(1);
+        $this->assertEquals(1, $task['id']);
+        $this->assertEquals(0, $task['owner_id']);
+    }
+}