diff --git a/lib/private/group/metadata.php b/lib/private/group/metadata.php index 21a742a328..66eb032d4b 100644 --- a/lib/private/group/metadata.php +++ b/lib/private/group/metadata.php @@ -27,7 +27,8 @@ namespace OC\Group; class MetaData { const SORT_NONE = 0; - const SORT_USERCOUNT = 1; + const SORT_USERCOUNT = 1; // May have performance issues on LDAP backends + const SORT_GROUPNAME = 2; /** * @var string $user @@ -121,15 +122,19 @@ class MetaData { } /** - * sets the sort mode, currently 0 (none) and 1 (user entries, - * descending) are supported - * @param int $sortMode (SORT_NONE, SORT_USERCOUNT) + * sets the sort mode, see SORT_* constants for supported modes + * + * @param int $sortMode */ public function setSorting($sortMode) { - if($sortMode >= 0 && $sortMode <= 1) { - $this->sorting = $sortMode; - } else { - $this->sorting = 0; + switch ($sortMode) { + case self::SORT_USERCOUNT: + case self::SORT_GROUPNAME: + $this->sorting = $sortMode; + break; + + default: + $this->sorting = self::SORT_NONE; } } @@ -139,27 +144,29 @@ class MetaData { * @param array $sortKeys the sort key array, by reference * @param int $sortIndex the sort key index, by reference * @param array $data the group's meta data as returned by generateGroupMetaData() - * @return null */ private function addEntry(&$entries, &$sortKeys, &$sortIndex, $data) { $entries[] = $data; - if($this->sorting === 1) { + if ($this->sorting === self::SORT_USERCOUNT) { $sortKeys[$sortIndex] = $data['usercount']; $sortIndex++; + } else if ($this->sorting === self::SORT_GROUPNAME) { + $sortKeys[$sortIndex] = $data['name']; + $sortIndex++; } } /** * creates an array containing the group meta data - * @param \OC\Group\Group $group + * @param \OCP\IGroup $group * @param string $userSearch * @return array with the keys 'id', 'name' and 'usercount' */ - private function generateGroupMetaData(\OC\Group\Group $group, $userSearch) { + private function generateGroupMetaData(\OCP\IGroup $group, $userSearch) { return array( 'id' => $group->getGID(), 'name' => $group->getGID(), - 'usercount' => $group->count($userSearch) + 'usercount' => $this->sorting === self::SORT_USERCOUNT ? $group->count($userSearch) : 0, ); } @@ -170,15 +177,17 @@ class MetaData { * @param return null */ private function sort(&$entries, $sortKeys) { - if($this->sorting > 0) { + if ($this->sorting === self::SORT_USERCOUNT) { array_multisort($sortKeys, SORT_DESC, $entries); + } else if ($this->sorting === self::SORT_GROUPNAME) { + array_multisort($sortKeys, SORT_ASC, $entries); } } /** * returns the available groups * @param string $search a search string - * @return \OC\Group\Group[] + * @return \OCP\IGroup[] */ private function getGroups($search = '') { if($this->isAdmin) { diff --git a/settings/controller/groupscontroller.php b/settings/controller/groupscontroller.php index 1624b9e28a..c3c0ea5ff2 100644 --- a/settings/controller/groupscontroller.php +++ b/settings/controller/groupscontroller.php @@ -76,7 +76,7 @@ class GroupsController extends Controller { $groupsInfo = new \OC\Group\MetaData($this->userSession->getUser()->getUID(), $this->isAdmin, $this->groupManager); - $groupsInfo->setSorting($groupsInfo::SORT_USERCOUNT); + $groupsInfo->setSorting($groupsInfo::SORT_GROUPNAME); list($adminGroups, $groups) = $groupsInfo->get($groupPattern, $pattern); return new DataResponse( diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js index 32ddbf3ba0..0fcca39843 100644 --- a/settings/js/users/groups.js +++ b/settings/js/users/groups.js @@ -271,7 +271,8 @@ GroupList = { $(document).ready( function () { $userGroupList = $('#usergrouplist'); GroupList.initDeleteHandling(); - GroupList.getEveryoneCount(); + // TODO: disabled due to performance issues + // GroupList.getEveryoneCount(); // Display or hide of Create Group List Element $('#newgroup-form').hide(); diff --git a/settings/users.php b/settings/users.php index 0fc9fbeafc..44e2548be7 100644 --- a/settings/users.php +++ b/settings/users.php @@ -42,7 +42,7 @@ $config = \OC::$server->getConfig(); $isAdmin = OC_User::isAdminUser(OC_User::getUser()); $groupsInfo = new \OC\Group\MetaData(OC_User::getUser(), $isAdmin, $groupManager); -$groupsInfo->setSorting($groupsInfo::SORT_USERCOUNT); +$groupsInfo->setSorting($groupsInfo::SORT_GROUPNAME); list($adminGroup, $groups) = $groupsInfo->get(); $recoveryAdminEnabled = OC_App::isEnabled('encryption') && diff --git a/tests/lib/group/metadata.php b/tests/lib/group/metadata.php index 94944189ca..3f4019c2fa 100644 --- a/tests/lib/group/metadata.php +++ b/tests/lib/group/metadata.php @@ -16,7 +16,7 @@ class Test_MetaData extends \Test\TestCase { ->getMock(); } - private function getGroupMock() { + private function getGroupMock($countCallCount = 0) { $group = $this->getMockBuilder('\OC\Group\Group') ->disableOriginalConstructor() ->getMock(); @@ -28,7 +28,7 @@ class Test_MetaData extends \Test\TestCase { 'g2', 'g2', 'g2', 'g3', 'g3', 'g3')); - $group->expects($this->exactly(3)) + $group->expects($this->exactly($countCallCount)) ->method('count') ->with('') ->will($this->onConsecutiveCalls(2, 3, 5)); @@ -54,14 +54,15 @@ class Test_MetaData extends \Test\TestCase { $this->assertSame(2, count($ordinaryGroups)); $this->assertSame('g2', $ordinaryGroups[0]['name']); - $this->assertSame(3, $ordinaryGroups[0]['usercount']); + // user count is not loaded + $this->assertSame(0, $ordinaryGroups[0]['usercount']); } public function testGetWithSorting() { $groupManager = $this->getGroupManagerMock(); $groupMetaData = new \OC\Group\MetaData('foo', true, $groupManager); $groupMetaData->setSorting($groupMetaData::SORT_USERCOUNT); - $group = $this->getGroupMock(); + $group = $this->getGroupMock(3); $groups = array_fill(0, 3, $group); $groupManager->expects($this->once()) diff --git a/tests/settings/controller/groupscontrollertest.php b/tests/settings/controller/groupscontrollertest.php index 4b6c9d0256..3c15754846 100644 --- a/tests/settings/controller/groupscontrollertest.php +++ b/tests/settings/controller/groupscontrollertest.php @@ -111,26 +111,26 @@ class GroupsControllerTest extends \Test\TestCase { 0 => array( 'id' => 'admin', 'name' => 'admin', - 'usercount' => 18 + 'usercount' => 0,//User count disabled 18, ) ), 'groups' => array( 0 => array( - 'id' => 'secondGroup', - 'name' => 'secondGroup', - 'usercount' => 25 - ), - 1 => array( - 'id' => 'thirdGroup', - 'name' => 'thirdGroup', - 'usercount' => 14 - ), - 2 => array( 'id' => 'firstGroup', 'name' => 'firstGroup', - 'usercount' => 12 - ) + 'usercount' => 0,//User count disabled 12, + ), + 1 => array( + 'id' => 'secondGroup', + 'name' => 'secondGroup', + 'usercount' => 0,//User count disabled 25, + ), + 2 => array( + 'id' => 'thirdGroup', + 'name' => 'thirdGroup', + 'usercount' => 0,//User count disabled 14, + ), ) ) )