From ddb73063a7889c8dad79dd7148c01c9bc845d9f0 Mon Sep 17 00:00:00 2001
From: Frederic Guillot <fred@kanboard.net>
Date: Mon, 18 Jan 2016 21:20:35 -0500
Subject: Return the highest role for a project when a user is member of
 multiple groups

---
 ChangeLog                                   |  1 +
 app/Core/Security/AccessMap.php             | 20 ++++++++++++++++++++
 app/Model/ProjectGroupRole.php              |  6 ++++--
 tests/units/Core/Security/AccessMapTest.php | 14 ++++++++++++++
 tests/units/Model/ProjectGroupRoleTest.php  | 23 +++++++++++++++++++++++
 5 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5dc3c8d2..071b4329 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,7 @@ New features:
 
 Improvements:
 
+* Return the highest role for a project when a user is member of multiple groups
 * Show in user interface the saving state of the task
 * Add dropdown menu for subtasks, categories, swimlanes, columns, custom filters, task links and groups
 * Add new template hooks
diff --git a/app/Core/Security/AccessMap.php b/app/Core/Security/AccessMap.php
index 02a4ca45..f34c4b00 100644
--- a/app/Core/Security/AccessMap.php
+++ b/app/Core/Security/AccessMap.php
@@ -86,6 +86,26 @@ class AccessMap
         return $roles;
     }
 
+    /**
+     * Get the highest role from a list
+     *
+     * @access public
+     * @param  array  $roles
+     * @return string
+     */
+    public function getHighestRole(array $roles)
+    {
+        $rank = array();
+
+        foreach ($roles as $role) {
+            $rank[$role] = count($this->getRoleHierarchy($role));
+        }
+
+        asort($rank);
+
+        return key($rank);
+    }
+
     /**
      * Add new access rules
      *
diff --git a/app/Model/ProjectGroupRole.php b/app/Model/ProjectGroupRole.php
index 2fe22ca6..ee6ee7cb 100644
--- a/app/Model/ProjectGroupRole.php
+++ b/app/Model/ProjectGroupRole.php
@@ -48,11 +48,13 @@ class ProjectGroupRole extends Base
      */
     public function getUserRole($project_id, $user_id)
     {
-        return $this->db->table(self::TABLE)
+        $roles = $this->db->table(self::TABLE)
             ->join(GroupMember::TABLE, 'group_id', 'group_id', self::TABLE)
             ->eq(GroupMember::TABLE.'.user_id', $user_id)
             ->eq(self::TABLE.'.project_id', $project_id)
-            ->findOneColumn('role');
+            ->findAllByColumn('role');
+
+        return $this->projectAccessMap->getHighestRole($roles);
     }
 
     /**
diff --git a/tests/units/Core/Security/AccessMapTest.php b/tests/units/Core/Security/AccessMapTest.php
index 61693ce8..ae8044c9 100644
--- a/tests/units/Core/Security/AccessMapTest.php
+++ b/tests/units/Core/Security/AccessMapTest.php
@@ -17,6 +17,20 @@ class AccessMapTest extends Base
         $this->assertEquals(array('user', 'admin', 'manager'), $acl->getRoleHierarchy('user'));
     }
 
+    public function testGetHighestRole()
+    {
+        $acl = new AccessMap;
+        $acl->setRoleHierarchy('manager', array('member', 'viewer'));
+        $acl->setRoleHierarchy('member', array('viewer'));
+
+        $this->assertEquals('manager', $acl->getHighestRole(array('viewer', 'manager', 'member')));
+        $this->assertEquals('manager', $acl->getHighestRole(array('viewer', 'manager')));
+        $this->assertEquals('manager', $acl->getHighestRole(array('manager', 'member')));
+        $this->assertEquals('member', $acl->getHighestRole(array('viewer', 'member')));
+        $this->assertEquals('member', $acl->getHighestRole(array('member')));
+        $this->assertEquals('viewer', $acl->getHighestRole(array('viewer')));
+    }
+
     public function testAddRulesAndGetRoles()
     {
         $acl = new AccessMap;
diff --git a/tests/units/Model/ProjectGroupRoleTest.php b/tests/units/Model/ProjectGroupRoleTest.php
index 52b83082..29a9536b 100644
--- a/tests/units/Model/ProjectGroupRoleTest.php
+++ b/tests/units/Model/ProjectGroupRoleTest.php
@@ -337,4 +337,27 @@ class ProjectGroupRoleTest extends Base
         $projects = $groupRoleModel->getProjectsByUser(5, array(Project::INACTIVE));
         $this->assertCount(0, $projects);
     }
+
+    public function testUserInMultipleGroupsShouldReturnHighestRole()
+    {
+        $userModel = new User($this->container);
+        $projectModel = new Project($this->container);
+        $groupModel = new Group($this->container);
+        $groupRoleModel = new ProjectGroupRole($this->container);
+        $groupMemberModel = new GroupMember($this->container);
+
+        $this->assertEquals(1, $projectModel->create(array('name' => 'Test')));
+        $this->assertEquals(2, $userModel->create(array('username' => 'My user')));
+
+        $this->assertEquals(1, $groupModel->create('Group A'));
+        $this->assertEquals(2, $groupModel->create('Group B'));
+
+        $this->assertTrue($groupMemberModel->addUser(1, 1));
+        $this->assertTrue($groupMemberModel->addUser(2, 1));
+
+        $this->assertTrue($groupRoleModel->addGroup(1, 1, Role::PROJECT_MEMBER));
+        $this->assertTrue($groupRoleModel->addGroup(1, 2, Role::PROJECT_MANAGER));
+
+        $this->assertEquals(Role::PROJECT_MANAGER, $groupRoleModel->getUserRole(1, 1));
+    }
 }
-- 
cgit v1.2.3