Merge pull request #20157 from owncloud/users-fixeveryonecount

Fix everyone count for subadmins

5 files changed, 110 insertions, 60 deletions

diff --git a/settings/ajax/geteveryonecount.php b/settings/ajax/geteveryonecount.php
deleted file mode 100644
index 002c849fd39..00000000000
--- a/settings/ajax/geteveryonecount.php
+++ /dev/null
@@ -1,55 +0,0 @@
-<?php
-/**
- * @author Clark Tomlinson <fallen013@gmail.com>
- * @author Lukas Reschke <lukas@owncloud.com>
- * @author Morris Jobke <hey@morrisjobke.de>
- *
- * @copyright Copyright (c) 2015, ownCloud, Inc.
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License, version 3,
- * along with this program.  If not, see <http://www.gnu.org/licenses/>
- *
- */
-
-OC_JSON::callCheck();
-OC_JSON::checkSubAdminUser();
-
-$userCount = 0;
-
-$currentUser = \OC::$server->getUserSession()->getUser()->getUID();
-
-if (!OC_User::isAdminUser($currentUser)) {
-	$groups = \OC::$server->getGroupManager()->getSubAdmin()->getSubAdminsGroups(\OC::$server->getUserSession()->getUser());
-	// New class returns IGroup[] so convert back
-	foreach ($groups as $key => $group) {
-		$groups[$key] = $group->getGID();
-	}
-
-
-	foreach ($groups as $group) {
-		$userCount += count(OC_Group::usersInGroup($group));
-
-	}
-} else {
-
-	$userCountArray = \OC::$server->getUserManager()->countUsers();
-
-	if (!empty($userCountArray)) {
-		foreach ($userCountArray as $classname => $usercount) {
-			$userCount += $usercount;
-		}
-	}
-}
-
-
-OC_JSON::success(array('count' => $userCount));
diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php
index 8183bc4739b..82483a76245 100644
--- a/settings/controller/userscontroller.php
+++ b/settings/controller/userscontroller.php
@@ -548,4 +548,41 @@ class UsersController extends Controller {
 		);
 	}
 
+	/**
+	 * Count all unique users visible for the current admin/subadmin.
+	 *
+	 * @NoAdminRequired
+	 *
+	 * @return DataResponse
+	 */
+	public function stats() {
+		$userCount = 0;
+		if ($this->isAdmin) {
+			$countByBackend = $this->userManager->countUsers();
+
+			if (!empty($countByBackend)) {
+				foreach ($countByBackend as $count) {
+					$userCount += $count;
+				}
+			}
+		} else {
+			$groups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($this->userSession->getUser());
+
+			$uniqueUsers = [];
+			foreach ($groups as $group) {
+				foreach($group->getUsers() as $uid => $displayName) {
+					$uniqueUsers[$uid] = true;
+				}
+			}
+
+			$userCount = count($uniqueUsers);
+		}
+
+		return new DataResponse(
+			[
+				'totalUsers' => $userCount
+			]
+		);
+	}
+
 }
diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js
index c8d2ef7c5b1..2639191d918 100644
--- a/settings/js/users/groups.js
+++ b/settings/js/users/groups.js
@@ -304,10 +304,10 @@ GroupList = {
 		$.ajax({
 			type: "GET",
 			dataType: "json",
-			url: OC.generateUrl('/settings/ajax/geteveryonecount')
+			url: OC.generateUrl('/settings/users/stats')
 		}).success(function (data) {
-			$('#everyonegroup').data('usercount', data.count);
-			$('#everyonecount').text(data.count);
+			$('#everyonegroup').data('usercount', data.totalUsers);
+			$('#everyonecount').text(data.totalUsers);
 		});
 	}
 };
diff --git a/settings/routes.php b/settings/routes.php
index 10c3117c183..6ba38388d3a 100644
--- a/settings/routes.php
+++ b/settings/routes.php
@@ -47,6 +47,7 @@ $application->registerRoutes($this, [
 		['name' => 'AppSettings#changeExperimentalConfigState', 'url' => '/settings/apps/experimental', 'verb' => 'POST'],
 		['name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'],
 		['name' => 'Users#setMailAddress', 'url' => '/settings/users/{id}/mailAddress', 'verb' => 'PUT'],
+		['name' => 'Users#stats', 'url' => '/settings/users/stats', 'verb' => 'GET'],
 		['name' => 'LogSettings#setLogLevel', 'url' => '/settings/admin/log/level', 'verb' => 'POST'],
 		['name' => 'LogSettings#getEntries', 'url' => '/settings/admin/log/entries', 'verb' => 'GET'],
 		['name' => 'LogSettings#download', 'url' => '/settings/admin/log/download', 'verb' => 'GET'],
@@ -69,8 +70,6 @@ $this->create('settings_admin', '/settings/admin')
 	->actionInclude('settings/admin.php');
 // Settings ajax actions
 // users
-$this->create('settings_ajax_everyonecount', '/settings/ajax/geteveryonecount')
-	->actionInclude('settings/ajax/geteveryonecount.php');
 $this->create('settings_ajax_setquota', '/settings/ajax/setquota.php')
 	->actionInclude('settings/ajax/setquota.php');
 $this->create('settings_ajax_togglegroups', '/settings/ajax/togglegroups.php')
diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php
index e30a98447ae..f4b05671ce8 100644
--- a/tests/settings/controller/userscontrollertest.php
+++ b/tests/settings/controller/userscontrollertest.php
@@ -1677,4 +1677,73 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertSame($responseCode, $response->getStatus());
 	}
 
+	public function testStatsAdmin() {
+		$this->container['IsAdmin'] = true;
+
+		$this->container['UserManager']
+			->expects($this->at(0))
+			->method('countUsers')
+			->will($this->returnValue([128, 44]));
+
+		$expectedResponse = new DataResponse(
+			[
+				'totalUsers' => 172
+			]
+		);
+		$response = $this->container['UsersController']->stats();
+		$this->assertEquals($expectedResponse, $response);
+	}
+
+	/**
+	 * Tests that the subadmin stats return unique users, even
+	 * when a user appears in several groups.
+	 */
+	public function testStatsSubAdmin() {
+		$this->container['IsAdmin'] = false;
+
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+
+		$this->container['UserSession']
+			->expects($this->once())
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$group1 = $this->getMockBuilder('\OC\Group\Group')
+			->disableOriginalConstructor()->getMock();
+		$group1
+			->expects($this->once())
+			->method('getUsers')
+			->will($this->returnValue(['foo' => 'M. Foo', 'admin' => 'S. Admin']));
+
+		$group2 = $this->getMockBuilder('\OC\Group\Group')
+			->disableOriginalConstructor()->getMock();
+		$group2
+			->expects($this->once())
+			->method('getUsers')
+			->will($this->returnValue(['bar' => 'B. Ar']));
+
+		$subadmin = $this->getMockBuilder('\OC\SubAdmin')
+			->disableOriginalConstructor()
+			->getMock();
+		$subadmin
+			->expects($this->at(0))
+			->method('getSubAdminsGroups')
+			->will($this->returnValue([$group1, $group2]));
+
+		$this->container['GroupManager']
+			->expects($this->any())
+			->method('getSubAdmin')
+			->will($this->returnValue($subadmin));
+
+		$expectedResponse = new DataResponse(
+			[
+				'totalUsers' => 3
+			]
+		);
+
+		$response = $this->container['UsersController']->stats();
+		$this->assertEquals($expectedResponse, $response);
+	}
+
 }