Merge pull request #9562 from owncloud/users-groupfilter

Do not filter groups, but therefore update the user counts according to  the filter

lib/private/group/metadata.php

@@ -24,9 +24,9 @@ class MetaData {
 	protected $isAdmin;
 
 	/**
-	 * @var string[] $groups
+	 * @var array $metaData
 	 */
-	protected $groups = array();
+	protected $metaData = array();
 
 	/**
 	 * @var \OC\Group\Manager $groupManager
@@ -39,14 +39,9 @@ class MetaData {
 	protected $sorting = false;
 
 	/**
-	 * @var string $lastSearch
+	 * @param string $user the uid of the current user
-	 */
+	 * @param bool $isAdmin whether the current users is an admin
-	protected $lastSearch;
+	 * @param \OC\Group\Manager $groupManager
-
-	/**
-	 * @param string the uid of the current user
-	 * @param bool whether the current users is an admin
-	 * @param \OC\Group\Manager
 	 */
 	public function __construct(
 			$user,
@@ -63,14 +58,15 @@ class MetaData {
 	 * the array is structured as follows:
 	 * [0] array containing meta data about admin groups
 	 * [1] array containing meta data about unprivileged groups
-	 * @param string only effective when instance was created with isAdmin being
+	 * @param string $groupSearch only effective when instance was created with
-	 * true
+	 * isAdmin being true
+	 * @param string $userSearch the pattern users are search for
 	 * @return array
 	 */
-	public function get($search = '') {
+	public function get($groupSearch = '', $userSearch = '') {
-		if($this->lastSearch !== $search) {
+		$key = $groupSearch . '::' . $userSearch;
-			$this->lastSearch = $search;
+		if(isset($this->metaData[$key])) {
-			$this->groups = array();
+			return $this->metaData[$key];
 		}
 
 		$adminGroups = array();
@@ -80,8 +76,8 @@ class MetaData {
 		$sortAdminGroupsIndex = 0;
 		$sortAdminGroupsKeys = array();
 
-		foreach($this->getGroups($search) as $group) {
+		foreach($this->getGroups($groupSearch) as $group) {
-			$groupMetaData = $this->generateGroupMetaData($group);
+			$groupMetaData = $this->generateGroupMetaData($group, $userSearch);
 			if (strtolower($group->getGID()) !== 'admin') {
 				$this->addEntry(
 					$groups,
@@ -104,13 +100,14 @@ class MetaData {
 		$this->sort($groups, $sortGroupsKeys);
 		$this->sort($adminGroups, $sortAdminGroupsKeys);
 
-		return array($adminGroups, $groups);
+		$this->metaData[$key] = array($adminGroups, $groups);
+		return $this->metaData[$key];
 	}
 
 	/**
-	 * @brief sets the sort mode, currently 0 (none) and 1 (user entries,
+	 * sets the sort mode, currently 0 (none) and 1 (user entries,
 	 * descending) are supported
-	 * @param int the sortMode (SORT_NONE, SORT_USERCOUNT)
+	 * @param int $sortMode (SORT_NONE, SORT_USERCOUNT)
 	 */
 	public function setSorting($sortMode) {
 		if($sortMode >= 0 && $sortMode <= 1) {
@@ -121,11 +118,11 @@ class MetaData {
 	}
 
 	/**
-	 * @brief adds an group entry to the resulting array
+	 * adds an group entry to the resulting array
-	 * @param array the resulting array, by reference
+	 * @param array $entries the resulting array, by reference
-	 * @param array the sort key array, by reference
+	 * @param array $sortKeys the sort key array, by reference
-	 * @param array the sort key index, by reference
+	 * @param int $sortIndex the sort key index, by reference
-	 * @param array the group's meta data as returned by generateGroupMetaData()
+	 * @param array $data the group's meta data as returned by generateGroupMetaData()
 	 * @return null
 	 */
 	private function addEntry(&$entries, &$sortKeys, &$sortIndex, $data) {
@@ -137,22 +134,23 @@ class MetaData {
 	}
 
 	/**
-	 * @brief creates an array containing the group meta data
+	 * creates an array containing the group meta data
-	 * @param \OC\Group\Group
+	 * @param \OC\Group\Group $group
+	 * @param string $userSearch
 	 * @return array with the keys 'id', 'name' and 'usercount'
 	 */
-	private function generateGroupMetaData(\OC\Group\Group $group) {
+	private function generateGroupMetaData(\OC\Group\Group $group, $userSearch) {
 		return array(
-				'id' => str_replace(' ','', $group->getGID()),
+				'id' => $group->getGID(),
 				'name' => $group->getGID(),
-				'usercount' => $group->count()
+				'usercount' => $group->count($userSearch)
 			);
 	}
 
 	/**
-	 * @brief sorts the result array, if applicable
+	 * sorts the result array, if applicable
-	 * @param array the result array, by reference
+	 * @param array $entries the result array, by reference
-	 * @param array the array containing the sort keys
+	 * @param array $sortKeys the array containing the sort keys
 	 * @param return null
 	 */
 	private function sort(&$entries, $sortKeys) {
@@ -162,27 +160,15 @@ class MetaData {
 	}
 
 	/**
-	 * @brief returns the available groups
+	 * returns the available groups
-	 * @param string a search string
+	 * @param string $search a search string
 	 * @return \OC\Group\Group[]
 	 */
 	private function getGroups($search = '') {
-		if(count($this->groups) === 0) {
-			$this->fetchGroups($search);
-		}
-		return $this->groups;
-	}
-
-	/**
-	 * @brief fetches the group using the group manager or the subAdmin API
-	 * @param string a search string
-	 * @return null
-	 */
-	private function fetchGroups($search = '') {
 		if($this->isAdmin) {
-			$this->groups = $this->groupManager->search($search);
+			return $this->groupManager->search($search);
 		} else {
-			$this->groups = \OC_SubAdmin::getSubAdminsGroups($this->user);
+			return \OC_SubAdmin::getSubAdminsGroups($this->user);
 		}
 	}
 }

settings/ajax/grouplist.php

@@ -27,6 +27,12 @@ if (isset($_GET['pattern']) && !empty($_GET['pattern'])) {
 } else {
 	$pattern = '';
 }
+if (isset($_GET['filterGroups']) && !empty($_GET['filterGroups'])) {
+	$filterGroups = intval($_GET['filterGroups']) === 1;
+} else {
+	$filterGroups = false;
+}
+$groupPattern = $filterGroups ? $pattern : '';
 $groups = array();
 $adminGroups = array();
 $groupManager = \OC_Group::getManager();
@@ -36,13 +42,7 @@ $isAdmin = OC_User::isAdminUser(OC_User::getUser());
 //groups will be filtered out later
 $groupsInfo = new \OC\Group\MetaData(OC_User::getUser(), true, $groupManager);
 $groupsInfo->setSorting($groupsInfo::SORT_USERCOUNT);
-list($adminGroups, $groups) = $groupsInfo->get($pattern);
+list($adminGroups, $groups) = $groupsInfo->get($groupPattern, $pattern);
-
-$accessibleGroups = $groupManager->search($pattern);
-if(!$isAdmin) {
-	$subadminGroups = OC_SubAdmin::getSubAdminsGroups(OC_User::getUser());
-	$accessibleGroups = array_intersect($groups, $subadminGroups);
-}
 
 OC_JSON::success(
 	array('data' => array('adminGroups' => $adminGroups, 'groups' => $groups)));

settings/js/users/filter.js

@@ -14,6 +14,7 @@ function UserManagementFilter(filterInput, userList, groupList) {
 	this.filterInput = filterInput;
 	this.userList = userList;
 	this.groupList = groupList;
+	this.filterGroups = false;
 	this.thread = undefined;
 	this.oldval = this.filterInput.val();
 
@@ -55,7 +56,10 @@ UserManagementFilter.prototype.init = function() {
 UserManagementFilter.prototype.run = _.debounce(function() {
 		this.userList.empty();
 		this.userList.update(GroupList.getCurrentGID());
+		if(this.filterGroups) {
+			// user counts are being updated nevertheless
 			this.groupList.empty();
+		}
 		this.groupList.update();
 	},
 	300

settings/js/users/groups.js

@@ -114,7 +114,10 @@ GroupList = {
 		GroupList.updating = true;
 		$.get(
 			OC.generateUrl('/settings/ajax/grouplist'),
-			{pattern: filter.getPattern()},
+			{
+				pattern: filter.getPattern(),
+				filterGroups: filter.filterGroups ? 1 : 0
+			},
 			function (result) {
 
 				var lis = [];

tests/lib/group/metadata.php

@@ -0,0 +1,101 @@
+<?php
+
+/**
+ * Copyright (c) 2014 Arthur Schiwon <blizzz@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace Test\Group;
+
+class Test_MetaData extends \PHPUnit_Framework_TestCase {
+	private function getGroupManagerMock() {
+		return $this->getMockBuilder('\OC\Group\Manager')
+			->disableOriginalConstructor()
+			->getMock();
+	}
+
+	private function getGroupMock() {
+		$group = $this->getMockBuilder('\OC\Group\Group')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$group->expects($this->exactly(9))
+			->method('getGID')
+			->will($this->onConsecutiveCalls(
+				'admin', 'admin', 'admin',
+				'g2', 'g2', 'g2',
+				'g3', 'g3', 'g3'));
+
+		$group->expects($this->exactly(3))
+			->method('count')
+			->with('')
+			->will($this->onConsecutiveCalls(2, 3, 5));
+
+		return $group;
+	}
+
+
+	public function testGet() {
+		$groupManager = $this->getGroupManagerMock();
+		$groupMetaData = new \OC\Group\MetaData('foo', true, $groupManager);
+		$group = $this->getGroupMock();
+		$groups = array_fill(0, 3, $group);
+
+		$groupManager->expects($this->once())
+			->method('search')
+			->with('')
+			->will($this->returnValue($groups));
+
+		list($adminGroups, $ordinaryGroups) = $groupMetaData->get();
+
+		$this->assertSame(1, count($adminGroups));
+		$this->assertSame(2, count($ordinaryGroups));
+
+		$this->assertSame('g2', $ordinaryGroups[0]['name']);
+		$this->assertSame(3, $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();
+		$groups = array_fill(0, 3, $group);
+
+		$groupManager->expects($this->once())
+			->method('search')
+			->with('')
+			->will($this->returnValue($groups));
+
+		list($adminGroups, $ordinaryGroups) = $groupMetaData->get();
+
+		$this->assertSame(1, count($adminGroups));
+		$this->assertSame(2, count($ordinaryGroups));
+
+		$this->assertSame('g3', $ordinaryGroups[0]['name']);
+		$this->assertSame(5, $ordinaryGroups[0]['usercount']);
+	}
+
+	public function testGetWithCache() {
+		$groupManager = $this->getGroupManagerMock();
+		$groupMetaData = new \OC\Group\MetaData('foo', true, $groupManager);
+		$group = $this->getGroupMock();
+		$groups = array_fill(0, 3, $group);
+
+		$groupManager->expects($this->once())
+			->method('search')
+			->with('')
+			->will($this->returnValue($groups));
+
+		//two calls, if caching fails call counts for group and groupmanager
+		//are exceeded
+		$groupMetaData->get();
+		$groupMetaData->get();
+	}
+
+	//get() does not need to be tested with search parameters, because they are
+	//solely and only passed to GroupManager and Group.
+
+}