Improve unit test

5 files changed, 117 insertions, 23 deletions

diff --git a/assets/js/board.js b/assets/js/board.js
index 7742f3df..33e47053 100644
--- a/assets/js/board.js
+++ b/assets/js/board.js
@@ -212,9 +212,9 @@
     });
 
     [].forEach.call(document.querySelectorAll('[data-task-id]'), function (item) {
-            item.addEventListener('click', function() {
-                window.location.href = '?controller=task&action=show&task_id=' + item.getAttribute('data-task-id');
-            });
+        item.addEventListener('click', function() {
+            window.location.href = '?controller=task&action=show&task_id=' + item.getAttribute('data-task-id');
         });
+    });
 
 }());
diff --git a/locales/fr_FR/translations.php b/locales/fr_FR/translations.php
index 53dbd60e..b69c0445 100644
--- a/locales/fr_FR/translations.php
+++ b/locales/fr_FR/translations.php
@@ -211,5 +211,5 @@ return array(
     'Comment added successfully.' => 'Commentaire ajouté avec succès.',
     'Unable to create your comment.' => 'Impossible de sauvegarder votre commentaire.',
     'The description is required' => 'La description est obligatoire',
-    //'Edit this task' => '',
+    'Edit this task' => 'Modifier cette tâche',
 );
diff --git a/models/acl.php b/models/acl.php
index 767d62f8..86db3c32 100644
--- a/models/acl.php
+++ b/models/acl.php
@@ -21,6 +21,7 @@ class Acl extends Base
         'config' => array('index'),
     );
 
+    // Return true if the specified controller/action is allowed according to the given acl
     public function isAllowedAction(array $acl, $controller, $action)
     {
         if (isset($acl[$controller])) {
@@ -30,31 +31,37 @@ class Acl extends Base
         return false;
     }
 
+    // Return true if the given action is public
     public function isPublicAction($controller, $action)
     {
         return $this->isAllowedAction($this->public_actions, $controller, $action);
     }
 
+    // Return true if the given action is allowed for a regular user
     public function isUserAction($controller, $action)
     {
         return $this->isAllowedAction($this->user_actions, $controller, $action);
     }
 
+    // Return true if the logged user is admin
     public function isAdminUser()
     {
         return isset($_SESSION['user']['is_admin']) && $_SESSION['user']['is_admin'] === '1';
     }
 
+    // Return true if the logged user is not admin
     public function isRegularUser()
     {
         return isset($_SESSION['user']['is_admin']) && $_SESSION['user']['is_admin'] === '0';
     }
 
+    // Get the connected user id
     public function getUserId()
     {
         return isset($_SESSION['user']['id']) ? (int) $_SESSION['user']['id'] : 0;
     }
 
+    // Check if an action is allowed for the logged user
     public function isPageAccessAllowed($controller, $action)
     {
         return $this->isPublicAction($controller, $action) ||
diff --git a/models/project.php b/models/project.php
index 45cd1baa..238a60b4 100644
--- a/models/project.php
+++ b/models/project.php
@@ -12,6 +12,7 @@ class Project extends Base
     const ACTIVE = 1;
     const INACTIVE = 0;
 
+    // Get a list of people that can by assigned for tasks
     public function getUsersList($project_id)
     {
         $allowed_users = $this->getAllowedUsers($project_id);
@@ -24,6 +25,7 @@ class Project extends Base
         return array(t('Unassigned')) + $allowed_users;
     }
 
+    // Get a list of allowed people for a project
     public function getAllowedUsers($project_id)
     {
         return $this->db
@@ -34,6 +36,7 @@ class Project extends Base
             ->listing('user_id', 'username');
     }
 
+    // Get allowed and not allowed users for a project
     public function getAllUsers($project_id)
     {
         $users = array(
@@ -56,6 +59,7 @@ class Project extends Base
         return $users;
     }
 
+    // Allow a specific user for a given project
     public function allowUser($project_id, $user_id)
     {
         return $this->db
@@ -63,6 +67,7 @@ class Project extends Base
                     ->save(array('project_id' => $project_id, 'user_id' => $user_id));
     }
 
+    // Revoke a specific user for a given project
     public function revokeUser($project_id, $user_id)
     {
         return $this->db
@@ -72,6 +77,7 @@ class Project extends Base
                     ->remove();
     }
 
+    // Check if a specific user is allowed to access to a given project
     public function isUserAllowed($project_id, $user_id)
     {
         // If there is nobody specified, everybody have access to the project
@@ -82,13 +88,13 @@ class Project extends Base
 
         if ($nb_users < 1) return true;
 
-        // check if user has admin rights
+        // Check if user has admin rights
         $nb_users = $this->db
                     ->table(\Model\User::TABLE)
                     ->eq('id', $user_id)
                     ->eq('is_admin', 1)
                     ->count();
-         
+
         if ($nb_users > 0) return true;
 
         // Otherwise, allow only specific users
diff --git a/tests/ProjectTest.php b/tests/ProjectTest.php
index 33a35168..e6725b99 100644
--- a/tests/ProjectTest.php
+++ b/tests/ProjectTest.php
@@ -23,47 +23,128 @@ class ProjectTest extends PHPUnit_Framework_TestCase
         $this->assertNotEmpty($p->getById(1));
     }
 
-    public function testAllowUsers()
+    public function testAllowEverybody()
     {
+        // We create a regular user
+        $user = new User;
+        $user->create(array('username' => 'unittest', 'password' => 'unittest'));
+
         $p = new Project;
+        $this->assertEmpty($p->getAllowedUsers(1)); // Nobody is specified for the given project
+        $this->assertTrue($p->isUserAllowed(1, 1)); // Everybody should be allowed
+        $this->assertTrue($p->isUserAllowed(1, 2)); // Everybody should be allowed
+    }
 
-        // Everybody is allowed
-        $this->assertEmpty($p->getAllowedUsers(1));
-        $this->assertTrue($p->isUserAllowed(1, 1));
+    public function testAllowUser()
+    {
+        $p = new Project;
 
-        // Allow one user
+        // We allow the admin user
         $this->assertTrue($p->allowUser(1, 1));
+
+        // Non-existant project
         $this->assertFalse($p->allowUser(50, 1));
+
+        // Non-existant user
         $this->assertFalse($p->allowUser(1, 50));
+
+        // Our admin user should be allowed
         $this->assertEquals(array('1' => 'admin'), $p->getAllowedUsers(1));
         $this->assertTrue($p->isUserAllowed(1, 1));
 
-        // Disallow one user
+        // Our regular user should be forbidden
+        $this->assertFalse($p->isUserAllowed(1, 2));
+    }
+
+    public function testRevokeUser()
+    {
+        $p = new Project;
+
+        // We revoke our admin user
         $this->assertTrue($p->revokeUser(1, 1));
+
+        // We should have nobody in the users list
         $this->assertEmpty($p->getAllowedUsers(1));
-        $this->assertTrue($p->isUserAllowed(1, 1));
 
-        // Allow/disallow many users
-        $user = new User;
-        $user->create(array('username' => 'unittest', 'password' => 'unittest'));
+        // Our admin user and our regular user should be allowed
+        $this->assertTrue($p->isUserAllowed(1, 1));
+        $this->assertTrue($p->isUserAllowed(1, 2));
 
-        $this->assertTrue($p->allowUser(1, 1));
+        // We allow only the regular user
         $this->assertTrue($p->allowUser(1, 2));
 
+        // All users should be allowed (admin and regular)
+        $this->assertTrue($p->isUserAllowed(1, 1));
+        $this->assertTrue($p->isUserAllowed(1, 2));
+
+        // However, we should have only our regular user in the list
+        $this->assertEquals(array('2' => 'unittest'), $p->getAllowedUsers(1));
+
+        // We allow our admin, we should have both in the list
+        $this->assertTrue($p->allowUser(1, 1));
         $this->assertEquals(array('1' => 'admin', '2' => 'unittest'), $p->getAllowedUsers(1));
         $this->assertTrue($p->isUserAllowed(1, 1));
         $this->assertTrue($p->isUserAllowed(1, 2));
 
+        // We revoke the regular user
+        $this->assertTrue($p->revokeUser(1, 2));
+
+        // Only admin should be allowed
+        $this->assertTrue($p->isUserAllowed(1, 1));
+        $this->assertFalse($p->isUserAllowed(1, 2));
+
+        // We should have only admin in the list
+        $this->assertEquals(array('1' => 'admin'), $p->getAllowedUsers(1));
+
+        // We revoke the admin user
         $this->assertTrue($p->revokeUser(1, 1));
+        $this->assertEmpty($p->getAllowedUsers(1));
 
-        $this->assertEquals(array('2' => 'unittest'), $p->getAllowedUsers(1));
-        $this->assertTrue($p->isUserAllowed(1, 1)); // has admin priviledges
+        // Everybody should be allowed again
+        $this->assertTrue($p->isUserAllowed(1, 1));
         $this->assertTrue($p->isUserAllowed(1, 2));
-        
-        // Check if revoked regular user is not allowed
+    }
+
+    public function testUsersList()
+    {
+        $p = new Project;
+
+        // No restriction, we should have everybody
+        $this->assertEquals(
+            array('Unassigned', 'admin', 'unittest'),
+            $p->getUsersList(1)
+        );
+
+        // We allow only the regular user
+        $this->assertTrue($p->allowUser(1, 2));
+
+        $this->assertEquals(
+            array(0 => 'Unassigned', 2 => 'unittest'),
+            $p->getUsersList(1)
+        );
+
+        // We allow the admin user
         $this->assertTrue($p->allowUser(1, 1));
+
+        $this->assertEquals(
+            array(0 => 'Unassigned', 1 => 'admin', 2 => 'unittest'),
+            $p->getUsersList(1)
+        );
+
+        // We revoke only the regular user
         $this->assertTrue($p->revokeUser(1, 2));
-        $this->assertEquals(array('1' => 'admin'), $p->getAllowedUsers(1));
-        $this->assertFalse($p->isUserAllowed(1, 2)); // regulat user is not allowed
+
+        $this->assertEquals(
+            array(0 => 'Unassigned', 1 => 'admin'),
+            $p->getUsersList(1)
+        );
+
+        // We revoke only the admin user, we should have everybody
+        $this->assertTrue($p->revokeUser(1, 1));
+
+        $this->assertEquals(
+            array(0 => 'Unassigned', 1 => 'admin', 2 => 'unittest'),
+            $p->getUsersList(1)
+        );
     }
 }