Use group displayNames in users list

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

6 files changed, 74 insertions, 41 deletions

diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php
index 2d40b447996..1dd951a1078 100644
--- a/lib/private/Group/Manager.php
+++ b/lib/private/Group/Manager.php
@@ -330,6 +330,17 @@ class Manager extends PublicEmitter implements IGroupManager {
 	}
 
 	/**
+	 * get an array of groupid and displayName for a user
+	 * @param IUser $user
+	 * @return array ['displayName' => displayname]
+	 */
+	public function getUserGroupNames(IUser $user) {
+		return array_map(function($group) {
+			return array('displayName' => $group->getDisplayName());
+		}, $this->getUserGroups($user));
+	}
+
+	/**
 	 * get a list of all display names in a group
 	 * @param string $gid
 	 * @param string $search
diff --git a/lib/private/SubAdmin.php b/lib/private/SubAdmin.php
index cd16d07e43d..44d79d8994e 100644
--- a/lib/private/SubAdmin.php
+++ b/lib/private/SubAdmin.php
@@ -62,7 +62,7 @@ class SubAdmin extends PublicEmitter {
 			$this->post_deleteUser($user);
 		});
 		$this->groupManager->listen('\OC\Group', 'postDelete', function($group) {
-			$this->post_deleteGroup($group);	
+			$this->post_deleteGroup($group);
 		});
 	}
 
@@ -123,7 +123,7 @@ class SubAdmin extends PublicEmitter {
 		while($row = $result->fetch()) {
 			$group = $this->groupManager->get($row['gid']);
 			if(!is_null($group)) {
-				$groups[] = $group;
+				$groups[$group->getGID()] = $group;
 			}
 		}
 		$result->closeCursor();
@@ -132,6 +132,17 @@ class SubAdmin extends PublicEmitter {
 	}
 
 	/**
+	 * get an array of groupid and displayName for a user
+	 * @param IUser $user
+	 * @return array ['displayName' => displayname]
+	 */
+	public function getSubAdminsGroupsName(IUser $user) {
+		return array_map(function($group) {
+			return array('displayName' => $group->getDisplayName());
+		}, $this->getSubAdminsGroups($user));
+	}
+
+	/**
 	 * get SubAdmins of a group
 	 * @param IGroup $group the group
 	 * @return IUser[]
@@ -185,7 +196,7 @@ class SubAdmin extends PublicEmitter {
 
 	/**
 	 * checks if a user is a SubAdmin of a group
-	 * @param IUser $user 
+	 * @param IUser $user
 	 * @param IGroup $group
 	 * @return bool
 	 */
@@ -210,7 +221,7 @@ class SubAdmin extends PublicEmitter {
 
 	/**
 	 * checks if a user is a SubAdmin
-	 * @param IUser $user 
+	 * @param IUser $user
 	 * @return bool
 	 */
 	public function isSubAdmin(IUser $user) {
diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php
index d311b9dcb80..6a067a6b59d 100644
--- a/settings/Controller/UsersController.php
+++ b/settings/Controller/UsersController.php
@@ -223,10 +223,7 @@ class UsersController extends Controller {
 			$restorePossible = true;
 		}
 
-		$subAdminGroups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($user);
-		foreach ($subAdminGroups as $key => $subAdminGroup) {
-			$subAdminGroups[$key] = $subAdminGroup->getGID();
-		}
+		$subAdminGroups = $this->groupManager->getSubAdmin()->getSubAdminsGroupsName($user);
 
 		$displayName = $user->getEMailAddress();
 		if (is_null($displayName)) {
@@ -243,7 +240,7 @@ class UsersController extends Controller {
 		return [
 			'name' => $user->getUID(),
 			'displayname' => $user->getDisplayName(),
-			'groups' => empty($userGroups) ? $this->groupManager->getUserGroupIds($user) : $userGroups,
+			'groups' => empty($userGroups) ? $this->groupManager->getUserGroupNames($user) : $userGroups,
 			'subadmin' => $subAdminGroups,
 			'quota' => $user->getQuota(),
 			'quota_bytes' => Util::computerFileSize($user->getQuota()),
@@ -344,7 +341,7 @@ class UsersController extends Controller {
 			foreach ($batch as $user) {
 				// Only add the groups, this user is a subadmin of
 				$userGroups = array_values(array_intersect(
-					$this->groupManager->getUserGroupIds($user),
+					$this->groupManager->getUserGroupNames($user),
 					$subAdminOfGroups
 				));
 				if (($gid !== '_disabledUsers' && $user->isEnabled()) ||
@@ -484,7 +481,7 @@ class UsersController extends Controller {
 				}
 			}
 			// fetch users groups
-			$userGroups = $this->groupManager->getUserGroupIds($user);
+			$userGroups = $this->groupManager->getUserGroupNames($user);
 
 			return new DataResponse(
 				$this->formatUserForIndex($user, $userGroups),
diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js
index cfc0fa26e95..fdad763f871 100644
--- a/settings/js/users/groups.js
+++ b/settings/js/users/groups.js
@@ -142,7 +142,7 @@ GroupList = {
 			function (result) {
 				if (result.groupname) {
 					var addedGroup = result.groupname;
-					UserList.availableGroups = $.unique($.merge(UserList.availableGroups, [addedGroup]));
+					UserList.availableGroups[result.id] = {displayName: result.groupName};
 					GroupList.addGroup(groupid, result.groupname);
 				}
 				GroupList.toggleAddGroup();
diff --git a/settings/js/users/users.js b/settings/js/users/users.js
index 9bbdd48e99e..2db43b24a0b 100644
--- a/settings/js/users/users.js
+++ b/settings/js/users/users.js
@@ -52,8 +52,8 @@ var UserList = {
 	 *            {
 	 * 				'name': 			'username',
 	 * 				'displayname': 		'Users display name',
-	 * 				'groups': 			['group1', 'group2'],
-	 * 				'subadmin': 		['group4', 'group5'],
+	 * 				'groups': 			{group1: {displayName: 'Group 1'}, group2: {displayName: 'Group 2'}}
+	 * 				'subadmin': 		{group5: {displayName: 'Group 5'}, group6: {displayName: 'Group 6'}}
 	 *				'quota': 			'10 GB',
 	 *				'quota_bytes':		'10737418240',
 	 *				'storageLocation':	'/srv/www/owncloud/data/username',
@@ -455,10 +455,10 @@ var UserList = {
 					return false;
 				}
 
-				if (add && OC.isUserAdmin() && UserList.availableGroups.indexOf(group) === -1) {
+				if (add && OC.isUserAdmin() && _.isUndefined(UserList.availableGroups[group])) {
 					GroupList.createGroup(group);
-					if (UserList.availableGroups.indexOf(group) === -1) {
-						UserList.availableGroups.push(group);
+					if (_.isUndefined(UserList.availableGroups[group])) {
+						UserList.availableGroups[group] = {displayName: group};
 					}
 				}
 
@@ -473,8 +473,8 @@ var UserList = {
 					},
 					success: function () {
 						GroupList.update();
-						if (add && UserList.availableGroups.indexOf(group) === -1) {
-							UserList.availableGroups.push(group);
+						if (add && _.isUndefined(UserList.availableGroups[group])) {
+							UserList.availableGroups[group] = {displayName: group};
 						}
 
 						if (add) {
@@ -647,11 +647,12 @@ var UserList = {
 	 * Creates a temporary jquery.multiselect selector on the given group field
 	 */
 	_triggerGroupEdit: function ($td, isSubadminSelect) {
+		var self = this;
 		var $groupsListContainer = $td.find('.groupsListContainer');
-		var placeholder = $groupsListContainer.attr('data-placeholder') || t('settings', 'no group');
+		var placeholder = $groupsListContainer.data('placeholder') || t('settings', 'no group');
 		var user = UserList.getUID($td);
-		var checked = $td.data('groups') || [];
-		var extraGroups = [].concat(checked);
+		var checked = $td.data('groups') || {};
+		var extraGroups = Object.assign({}, checked);
 
 		$td.find('.multiselectoptions').remove();
 
@@ -663,22 +664,21 @@ var UserList = {
 			$groupsSelect = $('<select multiple="multiple" class="subadminsselect multiselect button" title="' + placeholder + '"></select>')
 		}
 
-		function createItem (group) {
-			if (isSubadminSelect && group === 'admin') {
+		function createItem (gid, group) {
+			if (isSubadminSelect && group.displayName === 'admin') {
 				// can't become subadmin of "admin" group
 				return;
 			}
-			$groupsSelect.append($('<option value="' + escapeHTML(group) + '">' + escapeHTML(group) + '</option>'));
+			$groupsSelect.append($('<option value="' + escapeHTML(gid) + '">' + escapeHTML(group.displayName) + '</option>'));
 		}
 
-		$.each(this.availableGroups, function (i, group) {
+		$.each(this.availableGroups, function (gid, group) {
 			// some new groups might be selected but not in the available groups list yet
-			var extraIndex = extraGroups.indexOf(group);
-			if (extraIndex >= 0) {
+			if (extraGroups[gid] !== undefined) {
 				// remove extra group as it was found
-				extraGroups.splice(extraIndex, 1);
+				delete extraGroups[gid];
 			}
-			createItem(group);
+			createItem(gid, group);
 		});
 		$.each(extraGroups, function (i, group) {
 			createItem(group);
@@ -686,10 +686,13 @@ var UserList = {
 
 		$td.append($groupsSelect);
 
+		var checkedIds = Object.keys(checked).map(function(group, gid) {
+			return checked[group].displayName;
+		});
 		if (isSubadminSelect) {
-			UserList.applySubadminSelect($groupsSelect, user, checked);
+			UserList.applySubadminSelect($groupsSelect, user, checkedIds);
 		} else {
-			UserList.applyGroupSelect($groupsSelect, user, checked);
+			UserList.applyGroupSelect($groupsSelect, user, checkedIds);
 		}
 
 		$groupsListContainer.addClass('hidden');
@@ -699,7 +702,15 @@ var UserList = {
 			$td.find('.multiselect:not(.groupsListContainer)').parent().remove();
 			$td.find('.multiselectoptions').remove();
 			$groupsListContainer.removeClass('hidden');
-			UserList._updateGroupListLabel($td, e.checked);
+			// Pull all checked groups from this.availableGroups
+			var checked = Object.keys(self.availableGroups).reduce(function (previous, key) {
+				if(e.checked.indexOf(key) >= 0) {
+					return Object.assign(previous, {[key]:self.availableGroups[key]});
+				} else {
+					return previous;
+				}
+			}, {});
+			UserList._updateGroupListLabel($td, checked);
 		});
 	},
 
@@ -707,9 +718,12 @@ var UserList = {
 	 * Updates the groups list td with the given groups selection
 	 */
 	_updateGroupListLabel: function ($td, groups) {
-		var placeholder = $td.find('.groupsListContainer').attr('data-placeholder');
+		var placeholder = $td.find('.groupsListContainer').data('placeholder');
 		var $groupsEl = $td.find('.groupsList');
-		$groupsEl.text(groups.join(', ') || placeholder || t('settings', 'no group'));
+		var grouptext = Object.keys(groups).map(function(group, gid) {
+			return groups[group].displayName;
+		});
+		$groupsEl.text(grouptext.join(', ') || placeholder || t('settings', 'no group'));
 		$td.data('groups', groups);
 	}
 };
@@ -1029,7 +1043,7 @@ $(document).ready(function () {
 		OC.Search.clear();
 	});
 
-	UserList._updateGroupListLabel($('#newuser .groups'), []);
+	UserList._updateGroupListLabel($('#newuser .groups'), {});
 	var _submitNewUserForm = function (event) {
 		event.preventDefault();
 		if (OC.PasswordConfirmation.requiresPasswordConfirmation()) {
@@ -1057,7 +1071,7 @@ $(document).ready(function () {
 		}
 
 		promise.then(function () {
-			var groups = $('#newuser .groups').data('groups') || [];
+			var groups = $('#newuser .groups').data('groups') || {};
 			$.post(
 				OC.generateUrl('/settings/users/users'),
 				{
@@ -1070,8 +1084,8 @@ $(document).ready(function () {
 					if (result.groups) {
 						for (var i in result.groups) {
 							var gid = result.groups[i];
-							if (UserList.availableGroups.indexOf(gid) === -1) {
-								UserList.availableGroups.push(gid);
+							if (_.isUndefined(UserList.availableGroups[gid])) {
+								UserList.availableGroups[gid] = {displayName: gid};
 							}
 							var $li = GroupList.getGroupLI(gid);
 							var userCount = GroupList.getUserCount($li);
diff --git a/settings/templates/users/main.php b/settings/templates/users/main.php
index 3fc80fca0b9..2d40f0fbb64 100644
--- a/settings/templates/users/main.php
+++ b/settings/templates/users/main.php
@@ -21,10 +21,10 @@ style('settings', 'settings');
 $userlistParams = array();
 $allGroups=array();
 foreach($_["adminGroup"] as $group) {
-	$allGroups[] = $group['name'];
+	$allGroups[$group['id']] = array('displayName' => $group['name']);
 }
 foreach($_["groups"] as $group) {
-	$allGroups[] = $group['name'];
+	$allGroups[$group['id']] = array('displayName' => $group['name']);
 }
 $userlistParams['subadmingroups'] = $allGroups;
 $userlistParams['allGroups'] = json_encode($allGroups);