Improve unit test

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');
         });
+    });
 
 }());

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',
 );

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) ||

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

tests/ProjectTest.php

@@ -23,47 +23,128 @@ class ProjectTest extends PHPUnit_Framework_TestCase
         $this->assertNotEmpty($p->getById(1));
     }
 
-    public function testAllowUsers()
+    public function testAllowEverybody()
     {
-        $p = new Project;
-
-        // Everybody is allowed
-        $this->assertEmpty($p->getAllowedUsers(1));
-        $this->assertTrue($p->isUserAllowed(1, 1));
-
-        // Allow one user
-        $this->assertTrue($p->allowUser(1, 1));
-        $this->assertFalse($p->allowUser(50, 1));
-        $this->assertFalse($p->allowUser(1, 50));
-        $this->assertEquals(array('1' => 'admin'), $p->getAllowedUsers(1));
-        $this->assertTrue($p->isUserAllowed(1, 1));
-
-        // Disallow one user
-        $this->assertTrue($p->revokeUser(1, 1));
-        $this->assertEmpty($p->getAllowedUsers(1));
-        $this->assertTrue($p->isUserAllowed(1, 1));
-
-        // Allow/disallow many users
+        // 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
+    }
+
+    public function testAllowUser()
+    {
+        $p = new Project;
+
+        // 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));
+
+        // 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));
+
+        // Our admin user and our regular user should be allowed
+        $this->assertTrue($p->isUserAllowed(1, 1));
+        $this->assertTrue($p->isUserAllowed(1, 2));
+
+        // 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));
+
+        // Everybody should be allowed again
+        $this->assertTrue($p->isUserAllowed(1, 1));
+        $this->assertTrue($p->isUserAllowed(1, 2));
+    }
+
+    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(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('2' => 'unittest'), $p->getAllowedUsers(1));
-        $this->assertTrue($p->isUserAllowed(1, 1)); // has admin priviledges
-        $this->assertTrue($p->isUserAllowed(1, 2));
-        
-        // Check if revoked regular user is not allowed
-        $this->assertTrue($p->allowUser(1, 1));
-        $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', 2 => 'unittest'),
+            $p->getUsersList(1)
+        );
     }
 }