From aef86a5f5a385840ac264e61605519a4caa7c815 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 24 Jan 2018 13:07:47 +0100 Subject: [PATCH 01/17] Show the displayname in the users group list Signed-off-by: Joas Schilling --- lib/private/Settings/Personal/PersonalInfo.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Settings/Personal/PersonalInfo.php b/lib/private/Settings/Personal/PersonalInfo.php index 6411912513..813d06195b 100644 --- a/lib/private/Settings/Personal/PersonalInfo.php +++ b/lib/private/Settings/Personal/PersonalInfo.php @@ -174,7 +174,7 @@ class PersonalInfo implements ISettings { private function getGroups(IUser $user) { $groups = array_map( function(IGroup $group) { - return $group->getGID(); + return $group->getDisplayName(); }, $this->groupManager->getUserGroups($user) ); From 82137f443d050c5856071ea5cd5b3569738d6095 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 24 Jan 2018 13:39:17 +0100 Subject: [PATCH 02/17] Correctly return the group name Signed-off-by: Joas Schilling --- lib/private/Group/MetaData.php | 2 +- settings/Controller/GroupsController.php | 15 +++++---------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/lib/private/Group/MetaData.php b/lib/private/Group/MetaData.php index d5c8b581f8..9959430199 100644 --- a/lib/private/Group/MetaData.php +++ b/lib/private/Group/MetaData.php @@ -160,7 +160,7 @@ class MetaData { private function generateGroupMetaData(\OCP\IGroup $group, $userSearch) { return array( 'id' => $group->getGID(), - 'name' => $group->getGID(), + 'name' => $group->getDisplayName(), 'usercount' => $this->sorting === self::SORT_USERCOUNT ? $group->count($userSearch) : 0, ); } diff --git a/settings/Controller/GroupsController.php b/settings/Controller/GroupsController.php index 8985a76ec9..19b7c53f8b 100644 --- a/settings/Controller/GroupsController.php +++ b/settings/Controller/GroupsController.php @@ -28,6 +28,7 @@ use OC\AppFramework\Http; use OC\Group\MetaData; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; +use OCP\IGroup; use OCP\IGroupManager; use OCP\IL10N; use OCP\IRequest; @@ -108,13 +109,9 @@ class GroupsController extends Controller { Http::STATUS_CONFLICT ); } - if($this->groupManager->createGroup($id)) { - return new DataResponse( - array( - 'groupname' => $id - ), - Http::STATUS_CREATED - ); + $group = $this->groupManager->createGroup($id); + if($group instanceof IGroup) { + return new DataResponse(['groupname' => $group->getDisplayName()], Http::STATUS_CREATED); } return new DataResponse( @@ -140,9 +137,7 @@ class GroupsController extends Controller { return new DataResponse( array( 'status' => 'success', - 'data' => array( - 'groupname' => $id - ) + 'data' => ['groupname' => $group->getDisplayName()] ), Http::STATUS_NO_CONTENT ); From 0e2366233afd9ed201841e932757f96f4dd2ebb2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 24 Jan 2018 13:39:48 +0100 Subject: [PATCH 03/17] Start fixing Select2 options Signed-off-by: Joas Schilling --- .../js/usergroupmembershipplugin.js | 18 +++++++++++++----- settings/js/settings.js | 8 +++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/apps/workflowengine/js/usergroupmembershipplugin.js b/apps/workflowengine/js/usergroupmembershipplugin.js index 1c09e7d5cc..10f2382db2 100644 --- a/apps/workflowengine/js/usergroupmembershipplugin.js +++ b/apps/workflowengine/js/usergroupmembershipplugin.js @@ -64,11 +64,11 @@ // add admin groups $.each(response.data.adminGroups, function(id, group) { - results.push({ id: group.id }); + results.push({ id: group.id, displayname: group.name }); }); // add groups $.each(response.data.groups, function(id, group) { - results.push({ id: group.id }); + results.push({ id: group.id, displayname: group.name }); }); // TODO once limit and offset is implemented for groups we should paginate the search results @@ -79,13 +79,21 @@ } }, initSelection: function (element, callback) { - callback({id: element.val()}); + var groupId = element.val(); + if (groupId) { + callback({ + id: groupId, + displayname: groupId + 'FIXME' // FIXME + }); + } else { + callback(); + } }, formatResult: function (element) { - return '' + escapeHTML(element.id) + ''; + return '' + escapeHTML(element.displayname) + ''; }, formatSelection: function (element) { - return ''+escapeHTML(element.id)+''; + return ''+escapeHTML(element.displayname)+''; } }); } diff --git a/settings/js/settings.js b/settings/js/settings.js index 3a1e67f41c..ad0ab758e9 100644 --- a/settings/js/settings.js +++ b/settings/js/settings.js @@ -73,12 +73,10 @@ OC.Settings = _.extend(OC.Settings, { return element.id; }, initSelection: function(element, callback) { - var selection = - _.map(($(element).val() || []).split('|').sort(), - function(groupName) { + var selection = _.map(($(element).val() || []).split('|').sort(), function(groupId) { return { - id: groupName, - displayname: groupName + id: groupId, + displayname: groupId + 'FIXME' // FIXME }; }); callback(selection); From db9b2398dd50ccd46b20cacf2336669cf3eddde7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 24 Jan 2018 13:54:49 +0100 Subject: [PATCH 04/17] Fix group displaynames in activity Signed-off-by: Joas Schilling --- .../dav/lib/CalDAV/Activity/Provider/Base.php | 67 +++++++++++++------ .../lib/CalDAV/Activity/Provider/Calendar.php | 6 +- .../lib/CalDAV/Activity/Provider/Event.php | 6 +- .../lib/Activity/Providers/Groups.php | 64 +++++++++++++++--- 4 files changed, 108 insertions(+), 35 deletions(-) diff --git a/apps/dav/lib/CalDAV/Activity/Provider/Base.php b/apps/dav/lib/CalDAV/Activity/Provider/Base.php index b6d8a5be73..99ad903f24 100644 --- a/apps/dav/lib/CalDAV/Activity/Provider/Base.php +++ b/apps/dav/lib/CalDAV/Activity/Provider/Base.php @@ -26,6 +26,8 @@ namespace OCA\DAV\CalDAV\Activity\Provider; use OCA\DAV\CalDAV\CalDavBackend; use OCP\Activity\IEvent; use OCP\Activity\IProvider; +use OCP\IGroup; +use OCP\IGroupManager; use OCP\IL10N; use OCP\IUser; use OCP\IUserManager; @@ -35,14 +37,22 @@ abstract class Base implements IProvider { /** @var IUserManager */ protected $userManager; - /** @var string[] cached displayNames - key is the UID and value the displayname */ - protected $displayNames = []; + /** @var string[] */ + protected $userDisplayNames = []; + + /** @var IGroupManager */ + protected $groupManager; + + /** @var string[] */ + protected $groupDisplayNames = []; /** * @param IUserManager $userManager + * @param IGroupManager $groupManager */ - public function __construct(IUserManager $userManager) { + public function __construct(IUserManager $userManager, IGroupManager $groupManager) { $this->userManager = $userManager; + $this->groupManager = $groupManager; } /** @@ -112,31 +122,19 @@ abstract class Base implements IProvider { ]; } - /** - * @param string $id - * @return array - */ - protected function generateGroupParameter($id) { - return [ - 'type' => 'group', - 'id' => $id, - 'name' => $id, - ]; - } - /** * @param string $uid * @return array */ protected function generateUserParameter($uid) { - if (!isset($this->displayNames[$uid])) { - $this->displayNames[$uid] = $this->getDisplayName($uid); + if (!isset($this->userDisplayNames[$uid])) { + $this->userDisplayNames[$uid] = $this->getUserDisplayName($uid); } return [ 'type' => 'user', 'id' => $uid, - 'name' => $this->displayNames[$uid], + 'name' => $this->userDisplayNames[$uid], ]; } @@ -144,12 +142,39 @@ abstract class Base implements IProvider { * @param string $uid * @return string */ - protected function getDisplayName($uid) { + protected function getUserDisplayName($uid) { $user = $this->userManager->get($uid); if ($user instanceof IUser) { return $user->getDisplayName(); - } else { - return $uid; } + return $uid; + } + + /** + * @param string $gid + * @return array + */ + protected function generateGroupParameter($gid) { + if (!isset($this->groupDisplayNames[$gid])) { + $this->groupDisplayNames[$gid] = $this->getGroupDisplayName($gid); + } + + return [ + 'type' => 'group', + 'id' => $gid, + 'name' => $this->groupDisplayNames[$gid], + ]; + } + + /** + * @param string $gid + * @return string + */ + protected function getGroupDisplayName($gid) { + $group = $this->groupManager->get($gid); + if ($group instanceof IGroup) { + return $group->getDisplayName(); + } + return $gid; } } diff --git a/apps/dav/lib/CalDAV/Activity/Provider/Calendar.php b/apps/dav/lib/CalDAV/Activity/Provider/Calendar.php index ff12914428..db79b0f665 100644 --- a/apps/dav/lib/CalDAV/Activity/Provider/Calendar.php +++ b/apps/dav/lib/CalDAV/Activity/Provider/Calendar.php @@ -26,6 +26,7 @@ namespace OCA\DAV\CalDAV\Activity\Provider; use OCP\Activity\IEvent; use OCP\Activity\IEventMerger; use OCP\Activity\IManager; +use OCP\IGroupManager; use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUserManager; @@ -63,10 +64,11 @@ class Calendar extends Base { * @param IURLGenerator $url * @param IManager $activityManager * @param IUserManager $userManager + * @param IGroupManager $groupManager * @param IEventMerger $eventMerger */ - public function __construct(IFactory $languageFactory, IURLGenerator $url, IManager $activityManager, IUserManager $userManager, IEventMerger $eventMerger) { - parent::__construct($userManager); + public function __construct(IFactory $languageFactory, IURLGenerator $url, IManager $activityManager, IUserManager $userManager, IGroupManager $groupManager, IEventMerger $eventMerger) { + parent::__construct($userManager, $groupManager); $this->languageFactory = $languageFactory; $this->url = $url; $this->activityManager = $activityManager; diff --git a/apps/dav/lib/CalDAV/Activity/Provider/Event.php b/apps/dav/lib/CalDAV/Activity/Provider/Event.php index eabd2e517c..f13cb0c266 100644 --- a/apps/dav/lib/CalDAV/Activity/Provider/Event.php +++ b/apps/dav/lib/CalDAV/Activity/Provider/Event.php @@ -26,6 +26,7 @@ namespace OCA\DAV\CalDAV\Activity\Provider; use OCP\Activity\IEvent; use OCP\Activity\IEventMerger; use OCP\Activity\IManager; +use OCP\IGroupManager; use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUserManager; @@ -57,10 +58,11 @@ class Event extends Base { * @param IURLGenerator $url * @param IManager $activityManager * @param IUserManager $userManager + * @param IGroupManager $groupManager * @param IEventMerger $eventMerger */ - public function __construct(IFactory $languageFactory, IURLGenerator $url, IManager $activityManager, IUserManager $userManager, IEventMerger $eventMerger) { - parent::__construct($userManager); + public function __construct(IFactory $languageFactory, IURLGenerator $url, IManager $activityManager, IUserManager $userManager, IGroupManager $groupManager, IEventMerger $eventMerger) { + parent::__construct($userManager, $groupManager); $this->languageFactory = $languageFactory; $this->url = $url; $this->activityManager = $activityManager; diff --git a/apps/files_sharing/lib/Activity/Providers/Groups.php b/apps/files_sharing/lib/Activity/Providers/Groups.php index 53262e1931..9a8f7164c5 100644 --- a/apps/files_sharing/lib/Activity/Providers/Groups.php +++ b/apps/files_sharing/lib/Activity/Providers/Groups.php @@ -24,6 +24,12 @@ namespace OCA\Files_Sharing\Activity\Providers; use OCP\Activity\IEvent; +use OCP\Activity\IManager; +use OCP\IGroup; +use OCP\IGroupManager; +use OCP\IURLGenerator; +use OCP\IUserManager; +use OCP\L10N\IFactory; class Groups extends Base { @@ -32,6 +38,24 @@ class Groups extends Base { const SUBJECT_UNSHARED_GROUP_SELF = 'unshared_group_self'; const SUBJECT_UNSHARED_GROUP_BY = 'unshared_group_by'; + /** @var IGroupManager */ + protected $groupManager; + + /** @var string[] */ + protected $groupDisplayNames = []; + + /** + * @param IFactory $languageFactory + * @param IURLGenerator $url + * @param IManager $activityManager + * @param IUserManager $userManager + * @param IGroupManager $groupManager + */ + public function __construct(IFactory $languageFactory, IURLGenerator $url, IManager $activityManager, IUserManager $userManager, IGroupManager $groupManager) { + parent::__construct($languageFactory, $url, $activityManager, $userManager); + $this->groupManager = $groupManager; + } + /** * @param IEvent $event * @return IEvent @@ -103,24 +127,44 @@ class Groups extends Base { case self::SUBJECT_UNSHARED_GROUP_BY: return [ 'file' => $this->getFile($parameters[0], $event), - 'group' => [ - 'type' => 'group', - 'id' => $parameters[2], - 'name' => $parameters[2], - ], + 'group' => $this->generateGroupParameter($parameters[2]), 'actor' => $this->getUser($parameters[1]), ]; case self::SUBJECT_SHARED_GROUP_SELF: case self::SUBJECT_UNSHARED_GROUP_SELF: return [ 'file' => $this->getFile($parameters[0], $event), - 'group' => [ - 'type' => 'group', - 'id' => $parameters[1], - 'name' => $parameters[1], - ], + 'group' => $this->generateGroupParameter($parameters[1]), ]; } return []; } + + /** + * @param string $gid + * @return array + */ + protected function generateGroupParameter($gid) { + if (!isset($this->groupDisplayNames[$gid])) { + $this->groupDisplayNames[$gid] = $this->getGroupDisplayName($gid); + } + + return [ + 'type' => 'group', + 'id' => $gid, + 'name' => $this->groupDisplayNames[$gid], + ]; + } + + /** + * @param string $gid + * @return string + */ + protected function getGroupDisplayName($gid) { + $group = $this->groupManager->get($gid); + if ($group instanceof IGroup) { + return $group->getDisplayName(); + } + return $gid; + } } From cb2fbdd214ddda422fb7a57f5779ca450b6a0411 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 24 Jan 2018 14:22:20 +0100 Subject: [PATCH 05/17] Fix group display name in the sidebar of the user management Signed-off-by: Joas Schilling --- settings/js/users/groups.js | 14 +++++++------- settings/templates/users/part.grouplist.php | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js index 522291a00d..cfc0fa26e9 100644 --- a/settings/js/users/groups.js +++ b/settings/js/users/groups.js @@ -17,11 +17,11 @@ GroupList = { filter: '', filterGroups: false, - addGroup: function (gid, usercount) { + addGroup: function (gid, displayName, usercount) { var $li = $userGroupList.find('.isgroup:last-child').clone(); $li .data('gid', gid) - .find('.groupname').text(gid); + .find('.groupname').text(displayName); GroupList.setUserCount($li, usercount); $li.appendTo($userGroupList); @@ -128,22 +128,22 @@ GroupList = { } }, - createGroup: function (groupname) { + createGroup: function (groupid) { if (OC.PasswordConfirmation.requiresPasswordConfirmation()) { - OC.PasswordConfirmation.requirePasswordConfirmation(_.bind(this.createGroup, this, groupname)); + OC.PasswordConfirmation.requirePasswordConfirmation(_.bind(this.createGroup, this, groupid)); return; } $.post( OC.generateUrl('/settings/users/groups'), { - id: groupname + id: groupid }, function (result) { if (result.groupname) { var addedGroup = result.groupname; UserList.availableGroups = $.unique($.merge(UserList.availableGroups, [addedGroup])); - GroupList.addGroup(result.groupname); + GroupList.addGroup(groupid, result.groupname); } GroupList.toggleAddGroup(); }).fail(function(result) { @@ -173,7 +173,7 @@ GroupList = { GroupList.setUserCount(GroupList.getGroupLI(group.name).first(), group.usercount); } else { - var $li = GroupList.addGroup(group.name, group.usercount); + var $li = GroupList.addGroup(group.id, group.name, group.usercount); $li.addClass('appear transparent'); lis.push($li); diff --git a/settings/templates/users/part.grouplist.php b/settings/templates/users/part.grouplist.php index 469ed94adb..4128a6b76e 100644 --- a/settings/templates/users/part.grouplist.php +++ b/settings/templates/users/part.grouplist.php @@ -50,7 +50,7 @@ -
  • +
  • From 00122fa02704c2e7dc02fde172472167cf3ad42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 2 Mar 2018 15:21:35 +0100 Subject: [PATCH 06/17] Use group displayNames in users list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- lib/private/Group/Manager.php | 11 ++++ lib/private/SubAdmin.php | 19 +++++-- settings/Controller/UsersController.php | 11 ++-- settings/js/users/groups.js | 2 +- settings/js/users/users.js | 68 +++++++++++++++---------- settings/templates/users/main.php | 4 +- 6 files changed, 74 insertions(+), 41 deletions(-) diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php index 2d40b44799..1dd951a107 100644 --- a/lib/private/Group/Manager.php +++ b/lib/private/Group/Manager.php @@ -329,6 +329,17 @@ class Manager extends PublicEmitter implements IGroupManager { }, array_keys($this->getUserGroups($user))); } + /** + * 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 diff --git a/lib/private/SubAdmin.php b/lib/private/SubAdmin.php index cd16d07e43..44d79d8994 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(); @@ -131,6 +131,17 @@ class SubAdmin extends PublicEmitter { return $groups; } + /** + * 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 @@ -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 d311b9dcb8..6a067a6b59 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 cfc0fa26e9..fdad763f87 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 9bbdd48e99..2db43b24a0 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 = $('') } - 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($('')); + $groupsSelect.append($('')); } - $.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 3fc80fca0b..2d40f0fbb6 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); From ddcd37121fc88b43c4bd181625496663ae4fafe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 2 Mar 2018 16:35:17 +0100 Subject: [PATCH 07/17] Fix users loading on group click and group creation on select MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- settings/js/users/groups.js | 5 ++++- settings/js/users/users.js | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js index fdad763f87..08bd26b230 100644 --- a/settings/js/users/groups.js +++ b/settings/js/users/groups.js @@ -18,6 +18,9 @@ GroupList = { filterGroups: false, addGroup: function (gid, displayName, usercount) { + if (_.isUndefined(displayName)) { + displayName = gid; + } var $li = $userGroupList.find('.isgroup:last-child').clone(); $li .data('gid', gid) @@ -142,7 +145,7 @@ GroupList = { function (result) { if (result.groupname) { var addedGroup = result.groupname; - UserList.availableGroups[result.id] = {displayName: result.groupName}; + UserList.availableGroups[groupid] = {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 2db43b24a0..1a20edd19d 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -66,7 +66,7 @@ var UserList = { * } */ add: function (user) { - if (this.currentGid && this.currentGid !== '_everyone' && this.currentGid !== '_disabledUsers' && _.indexOf(user.groups, this.currentGid) < 0) { + if (this.currentGid && this.currentGid !== '_everyone' && this.currentGid !== '_disabledUsers' && Object.keys(user.groups).indexOf(this.currentGid) < 0) { return false; } @@ -454,7 +454,6 @@ var UserList = { if (!OC.isUserAdmin() && checked.length === 1 && checked[0] === group) { return false; } - if (add && OC.isUserAdmin() && _.isUndefined(UserList.availableGroups[group])) { GroupList.createGroup(group); if (_.isUndefined(UserList.availableGroups[group])) { From e6a7a9f251cd77a7b571716bbc679687a548cc47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 2 Mar 2018 18:02:18 +0100 Subject: [PATCH 08/17] Fixed select2 workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- apps/workflowengine/js/admin.js | 22 ++++++++- .../js/usergroupmembershipplugin.js | 48 ++++--------------- 2 files changed, 30 insertions(+), 40 deletions(-) diff --git a/apps/workflowengine/js/admin.js b/apps/workflowengine/js/admin.js index ab122a8cd6..6df7b14481 100644 --- a/apps/workflowengine/js/admin.js +++ b/apps/workflowengine/js/admin.js @@ -149,6 +149,7 @@ message: '', errorMessage: '', saving: false, + groups: [], initialize: function() { // this creates a new copy of the object to definitely have a new reference and being able to reset the model this.originalModel = JSON.parse(JSON.stringify(this.model)); @@ -161,6 +162,24 @@ if (this.model.get('id') === undefined) { this.hasChanged = true; } + var self = this; + $.ajax({ + url: OC.generateUrl('settings/users/groups'), + dataType: 'json', + quietMillis: 100, + }).done(function(response) { + // add admin groups + $.each(response.data.adminGroups, function(id, group) { + self.groups.push({ id: group.id, displayname: group.name+'FIXME' }); + }); + // add groups + $.each(response.data.groups, function(id, group) { + self.groups.push({ id: group.id, displayname: group.name+'FIXME' }); + }); + self.render(); + }).fail(function(response) { + console.error('Failure happened', response); + }); }, delete: function() { if (OC.PasswordConfirmation.requiresPasswordConfirmation()) { @@ -304,10 +323,11 @@ id = $element.data('id'), check = checks[id], valueElement = $element.find('.check-value').first(); + var self = this; _.each(OCA.WorkflowEngine.availablePlugins, function(plugin) { if (_.isFunction(plugin.render)) { - plugin.render(valueElement, check); + plugin.render(valueElement, check, self.groups); } }); }, this); diff --git a/apps/workflowengine/js/usergroupmembershipplugin.js b/apps/workflowengine/js/usergroupmembershipplugin.js index 10f2382db2..c664481562 100644 --- a/apps/workflowengine/js/usergroupmembershipplugin.js +++ b/apps/workflowengine/js/usergroupmembershipplugin.js @@ -34,7 +34,7 @@ ] }; }, - render: function(element, check) { + render: function(element, check, groups) { if (check['class'] !== 'OCA\\WorkflowEngine\\Check\\UserGroupMembership') { return; } @@ -42,48 +42,18 @@ $(element).css('width', '400px'); $(element).select2({ - ajax: { - url: OC.generateUrl('settings/users/groups'), - dataType: 'json', - quietMillis: 100, - data: function (term) { - return { - pattern: term, //search term - filterGroups: true, - sortGroups: 2 // by groupname - }; - }, - results: function (response) { - // TODO improve error case - if (response.data === undefined) { - console.error('Failure happened', response); - return; - } - - var results = []; - - // add admin groups - $.each(response.data.adminGroups, function(id, group) { - results.push({ id: group.id, displayname: group.name }); - }); - // add groups - $.each(response.data.groups, function(id, group) { - results.push({ id: group.id, displayname: group.name }); - }); - - // TODO once limit and offset is implemented for groups we should paginate the search results - return { - results: results, - more: false - }; - } - }, + data: groups, initSelection: function (element, callback) { var groupId = element.val(); - if (groupId) { + if (groupId && groups.length > 0) { callback({ id: groupId, - displayname: groupId + 'FIXME' // FIXME + displayname: groups.find(group =>group.id === groupId).displayname + }); + } else if (groupId) { + callback({ + id: groupId, + displayname: groupId }); } else { callback(); From 9440e9bb1e9079e9b6217788e89a398cc272320c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 2 Mar 2018 19:52:43 +0100 Subject: [PATCH 09/17] Fixed Controller Test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- settings/Controller/UsersController.php | 2 +- .../Controller/GroupsControllerTest.php | 51 ++++-- .../Controller/UsersControllerTest.php | 146 ++++++++++++------ 3 files changed, 145 insertions(+), 54 deletions(-) diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php index 6a067a6b59..2558c2e2c3 100644 --- a/settings/Controller/UsersController.php +++ b/settings/Controller/UsersController.php @@ -341,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->getUserGroupNames($user), + $this->groupManager->getUserGroupIds($user), $subAdminOfGroups )); if (($gid !== '_disabledUsers' && $user->isEnabled()) || diff --git a/tests/Settings/Controller/GroupsControllerTest.php b/tests/Settings/Controller/GroupsControllerTest.php index ecbfa9ea05..d43d4faf21 100644 --- a/tests/Settings/Controller/GroupsControllerTest.php +++ b/tests/Settings/Controller/GroupsControllerTest.php @@ -66,6 +66,9 @@ class GroupsControllerTest extends \Test\TestCase { $firstGroup ->method('getGID') ->will($this->returnValue('firstGroup')); + $firstGroup + ->method('getDisplayName') + ->will($this->returnValue('First group')); $firstGroup ->method('count') ->will($this->returnValue(12)); @@ -74,6 +77,9 @@ class GroupsControllerTest extends \Test\TestCase { $secondGroup ->method('getGID') ->will($this->returnValue('secondGroup')); + $secondGroup + ->method('getDisplayName') + ->will($this->returnValue('Second group')); $secondGroup ->method('count') ->will($this->returnValue(25)); @@ -82,6 +88,9 @@ class GroupsControllerTest extends \Test\TestCase { $thirdGroup ->method('getGID') ->will($this->returnValue('thirdGroup')); + $thirdGroup + ->method('getDisplayName') + ->will($this->returnValue('Third group')); $thirdGroup ->method('count') ->will($this->returnValue(14)); @@ -90,6 +99,9 @@ class GroupsControllerTest extends \Test\TestCase { $fourthGroup ->method('getGID') ->will($this->returnValue('admin')); + $fourthGroup + ->method('getDisplayName') + ->will($this->returnValue('Admin')); $fourthGroup ->method('count') ->will($this->returnValue(18)); @@ -119,7 +131,7 @@ class GroupsControllerTest extends \Test\TestCase { 'adminGroups' => array( 0 => array( 'id' => 'admin', - 'name' => 'admin', + 'name' => 'Admin', 'usercount' => 0,//User count disabled 18, ) ), @@ -127,17 +139,17 @@ class GroupsControllerTest extends \Test\TestCase { array( 0 => array( 'id' => 'firstGroup', - 'name' => 'firstGroup', + 'name' => 'First group', 'usercount' => 0,//User count disabled 12, ), 1 => array( 'id' => 'secondGroup', - 'name' => 'secondGroup', + 'name' => 'Second group', 'usercount' => 0,//User count disabled 25, ), 2 => array( 'id' => 'thirdGroup', - 'name' => 'thirdGroup', + 'name' => 'Third group', 'usercount' => 0,//User count disabled 14, ), ) @@ -158,6 +170,9 @@ class GroupsControllerTest extends \Test\TestCase { $firstGroup ->method('getGID') ->will($this->returnValue('firstGroup')); + $firstGroup + ->method('getDisplayName') + ->will($this->returnValue('First group')); $firstGroup ->method('count') ->will($this->returnValue(12)); @@ -166,6 +181,9 @@ class GroupsControllerTest extends \Test\TestCase { $secondGroup ->method('getGID') ->will($this->returnValue('secondGroup')); + $secondGroup + ->method('getDisplayName') + ->will($this->returnValue('Second group')); $secondGroup ->method('count') ->will($this->returnValue(25)); @@ -174,6 +192,9 @@ class GroupsControllerTest extends \Test\TestCase { $thirdGroup ->method('getGID') ->will($this->returnValue('thirdGroup')); + $thirdGroup + ->method('getDisplayName') + ->will($this->returnValue('Third group')); $thirdGroup ->method('count') ->will($this->returnValue(14)); @@ -182,6 +203,9 @@ class GroupsControllerTest extends \Test\TestCase { $fourthGroup ->method('getGID') ->will($this->returnValue('admin')); + $fourthGroup + ->method('getDisplayName') + ->will($this->returnValue('Admin')); $fourthGroup ->method('count') ->will($this->returnValue(18)); @@ -212,7 +236,7 @@ class GroupsControllerTest extends \Test\TestCase { 'adminGroups' => array( 0 => array( 'id' => 'admin', - 'name' => 'admin', + 'name' => 'Admin', 'usercount' => 18, ) ), @@ -220,17 +244,17 @@ class GroupsControllerTest extends \Test\TestCase { array( 0 => array( 'id' => 'secondGroup', - 'name' => 'secondGroup', + 'name' => 'Second group', 'usercount' => 25, ), 1 => array( 'id' => 'thirdGroup', - 'name' => 'thirdGroup', + 'name' => 'Third group', 'usercount' => 14, ), 2 => array( 'id' => 'firstGroup', - 'name' => 'firstGroup', + 'name' => 'First group', 'usercount' => 12, ), ) @@ -259,6 +283,8 @@ class GroupsControllerTest extends \Test\TestCase { } public function testCreateSuccessful() { + $group = $this->getMockBuilder(Group::class) + ->disableOriginalConstructor()->getMock(); $this->groupManager ->expects($this->once()) ->method('groupExists') @@ -268,7 +294,11 @@ class GroupsControllerTest extends \Test\TestCase { ->expects($this->once()) ->method('createGroup') ->with('NewGroup') - ->will($this->returnValue(true)); + ->will($this->returnValue($group)); + $group + ->expects($this->once()) + ->method('getDisplayName') + ->will($this->returnValue('NewGroup')); $expectedResponse = new DataResponse( array( @@ -315,6 +345,9 @@ class GroupsControllerTest extends \Test\TestCase { ->expects($this->once()) ->method('delete') ->will($this->returnValue(true)); + $group + ->method('getDisplayName') + ->will($this->returnValue('ExistingGroup')); $expectedResponse = new DataResponse( array( diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php index 1b59f15efb..165431516e 100644 --- a/tests/Settings/Controller/UsersControllerTest.php +++ b/tests/Settings/Controller/UsersControllerTest.php @@ -206,7 +206,7 @@ class UsersControllerTest extends \Test\TestCase { $foo ->expects($this->exactly(2)) ->method('getQuota') - ->will($this->returnValue('1024')); + ->will($this->returnValue(1024)); $foo ->method('getLastLogin') ->will($this->returnValue(500)); @@ -236,7 +236,7 @@ class UsersControllerTest extends \Test\TestCase { $admin ->expects($this->exactly(2)) ->method('getQuota') - ->will($this->returnValue('404')); + ->will($this->returnValue(404)); $admin ->expects($this->once()) ->method('getLastLogin') @@ -268,7 +268,7 @@ class UsersControllerTest extends \Test\TestCase { $bar ->expects($this->exactly(2)) ->method('getQuota') - ->will($this->returnValue('2323')); + ->will($this->returnValue(2323)); $bar ->method('getLastLogin') ->will($this->returnValue(3999)); @@ -296,8 +296,20 @@ class UsersControllerTest extends \Test\TestCase { ->will($this->returnValue(array('foo' => 'M. Foo', 'admin' => 'S. Admin', 'bar' => 'B. Ar'))); $this->groupManager ->expects($this->exactly(3)) - ->method('getUserGroupIds') - ->will($this->onConsecutiveCalls(array('Users', 'Support'), array('admins', 'Support'), array('External Users'))); + ->method('getUserGroupNames') + ->will($this->onConsecutiveCalls( + array( + 'Users' => array('displayName' => 'Users'), + 'Support' => array('displayName' => 'Support') + ), + array( + 'admins' => array('displayName' => 'admins'), + 'Support' => array('displayName' => 'Support') + ), + array( + 'External Users' => array('displayName' => 'External Users') + ) + )); $this->userManager ->expects($this->at(0)) ->method('get') @@ -319,17 +331,17 @@ class UsersControllerTest extends \Test\TestCase { ->getMock(); $subadmin ->expects($this->any()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($foo) ->will($this->returnValue([])); $subadmin ->expects($this->any()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($admin) ->will($this->returnValue([])); $subadmin ->expects($this->any()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($bar) ->will($this->returnValue([])); $this->groupManager @@ -347,10 +359,13 @@ class UsersControllerTest extends \Test\TestCase { 0 => array( 'name' => 'foo', 'displayname' => 'M. Foo', - 'groups' => array('Users', 'Support'), + 'groups' => array( + 'Users' => array('displayName' => 'Users'), + 'Support' => array('displayName' => 'Support') + ), 'subadmin' => array(), 'quota' => 1024, - 'quota_bytes' => 1024, + 'quota_bytes' => 1024.0, 'storageLocation' => '/home/foo', 'lastLogin' => 500000, 'backend' => 'OC_User_Database', @@ -363,10 +378,13 @@ class UsersControllerTest extends \Test\TestCase { 1 => array( 'name' => 'admin', 'displayname' => 'S. Admin', - 'groups' => array('admins', 'Support'), + 'groups' => array( + 'admins' => array('displayName' => 'admins'), + 'Support' => array('displayName' => 'Support') + ), 'subadmin' => array(), 'quota' => 404, - 'quota_bytes' => 404, + 'quota_bytes' => 404.0, 'storageLocation' => '/home/admin', 'lastLogin' => 12000, 'backend' => Dummy::class, @@ -379,10 +397,12 @@ class UsersControllerTest extends \Test\TestCase { 2 => array( 'name' => 'bar', 'displayname' => 'B. Ar', - 'groups' => array('External Users'), + 'groups' => array( + 'External Users' => array('displayName' => 'External Users') + ), 'subadmin' => array(), 'quota' => 2323, - 'quota_bytes' => 2323, + 'quota_bytes' => 2323.0, 'storageLocation' => '/home/bar', 'lastLogin' => 3999000, 'backend' => Dummy::class, @@ -553,6 +573,10 @@ class UsersControllerTest extends \Test\TestCase { ->expects($this->at(0)) ->method('getSubAdminsGroups') ->will($this->returnValue([$subgroup1, $subgroup2])); + $subadmin + ->expects($this->any()) + ->method('getSubAdminsGroupsName') + ->will($this->returnValue([])); $subadmin ->expects($this->any()) ->method('getSubAdminsGroups') @@ -574,8 +598,8 @@ class UsersControllerTest extends \Test\TestCase { 'displayname' => 'B. Ar', 'groups' => ['SubGroup1'], 'subadmin' => [], - 'quota' => 2323, - 'quota_bytes' => 2323, + 'quota' => '2323', + 'quota_bytes' => 2323.0, 'storageLocation' => '/home/bar', 'lastLogin' => 3999000, 'backend' => Dummy::class, @@ -590,8 +614,8 @@ class UsersControllerTest extends \Test\TestCase { 'displayname' => 'M. Foo', 'groups' => ['SubGroup2', 'SubGroup1'], 'subadmin' => [], - 'quota' => 1024, - 'quota_bytes' => 1024, + 'quota' => '1024', + 'quota_bytes' => 1024.0, 'storageLocation' => '/home/foo', 'lastLogin' => 500000, 'backend' => 'OC_User_Database', @@ -606,8 +630,8 @@ class UsersControllerTest extends \Test\TestCase { 'displayname' => 'S. Admin', 'groups' => ['SubGroup2'], 'subadmin' => [], - 'quota' => 404, - 'quota_bytes' => 404, + 'quota' => '404', + 'quota_bytes' => 404.0, 'storageLocation' => '/home/admin', 'lastLogin' => 12000, 'backend' => Dummy::class, @@ -731,14 +755,26 @@ class UsersControllerTest extends \Test\TestCase { ->will($this->returnValue([$foo, $admin, $bar])); $this->groupManager ->expects($this->exactly(3)) - ->method('getUserGroupIds') - ->will($this->onConsecutiveCalls(array('Users', 'Support'), array('admins', 'Support'), array('External Users'))); + ->method('getUserGroupNames') + ->will($this->onConsecutiveCalls( + array( + 'Users' => array('displayName' => 'Users'), + 'Support' => array('displayName' => 'Support') + ), + array( + 'admins' => array('displayName' => 'admins'), + 'Support' => array('displayName' => 'Support') + ), + array( + 'External Users' => array('displayName' => 'External Users') + ) + )); $subadmin = $this->getMockBuilder(SubAdmin::class) ->disableOriginalConstructor() ->getMock(); $subadmin->expects($this->any()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->will($this->returnValue([])); $this->groupManager ->expects($this->any()) @@ -755,7 +791,10 @@ class UsersControllerTest extends \Test\TestCase { 0 => array( 'name' => 'foo', 'displayname' => 'M. Foo', - 'groups' => array('Users', 'Support'), + 'groups' => array( + 'Users' => array('displayName' => 'Users'), + 'Support' => array('displayName' => 'Support') + ), 'subadmin' => array(), 'quota' => 1024, 'quota_bytes' => 1024, @@ -771,7 +810,10 @@ class UsersControllerTest extends \Test\TestCase { 1 => array( 'name' => 'admin', 'displayname' => 'S. Admin', - 'groups' => array('admins', 'Support'), + 'groups' => array( + 'admins' => array('displayName' => 'admins'), + 'Support' => array('displayName' => 'Support') + ), 'subadmin' => array(), 'quota' => 404, 'quota_bytes' => 404, @@ -787,7 +829,9 @@ class UsersControllerTest extends \Test\TestCase { 2 => array( 'name' => 'bar', 'displayname' => 'B. Ar', - 'groups' => array('External Users'), + 'groups' => array( + 'External Users' => array('displayName' => 'External Users') + ), 'subadmin' => array(), 'quota' => 2323, 'quota_bytes' => 2323, @@ -857,7 +901,7 @@ class UsersControllerTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $subadmin->expects($this->once()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->will($this->returnValue([])); $this->groupManager ->expects($this->any()) @@ -944,7 +988,7 @@ class UsersControllerTest extends \Test\TestCase { ->getMock(); $subadmin ->expects($this->any()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager @@ -1022,16 +1066,21 @@ class UsersControllerTest extends \Test\TestCase { ->will($this->onConsecutiveCalls($newGroup)); $this->groupManager ->expects($this->once()) - ->method('getUserGroupIds') + ->method('getUserGroupNames') ->with($user) - ->will($this->onConsecutiveCalls(array('NewGroup', 'ExistingGroup'))); + ->will($this->onConsecutiveCalls( + array( + 'NewGroup' => array('displayName' => 'NewGroup'), + 'ExistingGroup' => array('displayName' => 'ExistingGroup') + ) + )); $subadmin = $this->getMockBuilder(SubAdmin::class) ->disableOriginalConstructor() ->getMock(); $subadmin ->expects($this->once()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager @@ -1042,7 +1091,10 @@ class UsersControllerTest extends \Test\TestCase { $expectedResponse = new DataResponse( array( 'name' => 'foo', - 'groups' => array('NewGroup', 'ExistingGroup'), + 'groups' => array( + 'NewGroup' => array('displayName' => 'NewGroup'), + 'ExistingGroup' => array('displayName' => 'ExistingGroup') + ), 'storageLocation' => '/home/user', 'backend' => 'bar', 'lastLogin' => null, @@ -1100,18 +1152,20 @@ class UsersControllerTest extends \Test\TestCase { ->will($this->returnValue($newUser)); $this->groupManager ->expects($this->once()) - ->method('getUserGroupIds') + ->method('getUserGroupNames') ->with($user) - ->will($this->onConsecutiveCalls(['SubGroup1'])); + ->will($this->onConsecutiveCalls(array('SubGroup1' => + array('displayName' => 'SubGroup1') + ))); $this->groupManager ->expects($this->once()) - ->method('getUserGroupIds') + ->method('getUserGroupNames') ->with($newUser) ->will($this->onConsecutiveCalls(['SubGroup1'])); $subadmin = $this->createMock(\OC\SubAdmin::class); $subadmin->expects($this->atLeastOnce()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->willReturnMap([ [$user, [$subGroup1]], @@ -1135,7 +1189,7 @@ class UsersControllerTest extends \Test\TestCase { $expectedResponse = new DataResponse( array( 'name' => 'foo', - 'groups' => ['SubGroup1'], + 'groups' => array('SubGroup1' => array('displayName' => 'SubGroup1')), 'storageLocation' => '/home/user', 'backend' => 'bar', 'lastLogin' => 0, @@ -1563,7 +1617,7 @@ class UsersControllerTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $subadmin->expects($this->once()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager @@ -1629,7 +1683,7 @@ class UsersControllerTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $subadmin->expects($this->once()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager @@ -1676,7 +1730,7 @@ class UsersControllerTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $subadmin->expects($this->once()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager @@ -1714,7 +1768,7 @@ class UsersControllerTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $subadmin->expects($this->once()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager @@ -1771,7 +1825,7 @@ class UsersControllerTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $subadmin->expects($this->once()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager @@ -1793,7 +1847,7 @@ class UsersControllerTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $subadmin->expects($this->once()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager @@ -1858,6 +1912,10 @@ class UsersControllerTest extends \Test\TestCase { $subadmin = $this->getMockBuilder(SubAdmin::class) ->disableOriginalConstructor() ->getMock(); + $subadmin + ->expects($this->at(0)) + ->method('getSubAdminsGroupsName') + ->will($this->returnValue([$group1, $group2])); $subadmin ->expects($this->at(0)) ->method('getSubAdminsGroups') @@ -2430,7 +2488,7 @@ class UsersControllerTest extends \Test\TestCase { ->getMock(); $subadmin ->expects($this->any()) - ->method('getSubAdminsGroups') + ->method('getSubAdminsGroupsName') ->with($user) ->will($this->returnValue([])); $this->groupManager From d80f13ef99983b85895f65787c8bc61359b949a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Sat, 3 Mar 2018 12:22:27 +0100 Subject: [PATCH 10/17] Fix Metadata tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- tests/lib/Group/MetaDataTest.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/lib/Group/MetaDataTest.php b/tests/lib/Group/MetaDataTest.php index 04d2ff807b..53a3c79200 100644 --- a/tests/lib/Group/MetaDataTest.php +++ b/tests/lib/Group/MetaDataTest.php @@ -53,13 +53,20 @@ class MetaDataTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); - $group->expects($this->exactly(9)) + $group->expects($this->exactly(6)) ->method('getGID') ->will($this->onConsecutiveCalls( 'admin', 'admin', 'admin', 'g2', 'g2', 'g2', 'g3', 'g3', 'g3')); + $group->expects($this->exactly(3)) + ->method('getDisplayName') + ->will($this->onConsecutiveCalls( + 'admin', + 'g2', + 'g3')); + $group->expects($this->exactly($countCallCount)) ->method('count') ->with('') From 720e64ba55c5940a06df190d251d2a06ea66523e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Mon, 5 Mar 2018 10:10:05 +0100 Subject: [PATCH 11/17] Fixed caldav tests and metadata 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php | 8 +++++++- tests/lib/Group/MetaDataTest.php | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php b/apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php index affc1909e3..37a56f8804 100644 --- a/apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php +++ b/apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php @@ -29,6 +29,7 @@ use OCP\Activity\IProvider; use OCP\IL10N; use OCP\IUser; use OCP\IUserManager; +use OCP\IGroupManager; use Test\TestCase; class BaseTest extends TestCase { @@ -36,15 +37,20 @@ class BaseTest extends TestCase { /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ protected $userManager; + /** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $groupManager; + /** @var IProvider|Base|\PHPUnit_Framework_MockObject_MockObject */ protected $provider; protected function setUp() { parent::setUp(); $this->userManager = $this->createMock(IUserManager::class); + $this->groupManager = $this->createMock(IGroupManager::class); $this->provider = $this->getMockBuilder(Base::class) ->setConstructorArgs([ - $this->userManager + $this->userManager, + $this->groupManager ]) ->setMethods(['parse']) ->getMock(); diff --git a/tests/lib/Group/MetaDataTest.php b/tests/lib/Group/MetaDataTest.php index 53a3c79200..c24155aef3 100644 --- a/tests/lib/Group/MetaDataTest.php +++ b/tests/lib/Group/MetaDataTest.php @@ -56,9 +56,9 @@ class MetaDataTest extends \Test\TestCase { $group->expects($this->exactly(6)) ->method('getGID') ->will($this->onConsecutiveCalls( - 'admin', 'admin', 'admin', - 'g2', 'g2', 'g2', - 'g3', 'g3', 'g3')); + 'admin', 'admin', + 'g2', 'g2', + 'g3', 'g3')); $group->expects($this->exactly(3)) ->method('getDisplayName') From 1f9a5c0d7fb0db6c31ff7eefa28db0642be7b385 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 5 Mar 2018 12:58:31 +0100 Subject: [PATCH 12/17] Remove resolved FIXME Signed-off-by: Joas Schilling --- apps/workflowengine/js/admin.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/workflowengine/js/admin.js b/apps/workflowengine/js/admin.js index 6df7b14481..d31add77ef 100644 --- a/apps/workflowengine/js/admin.js +++ b/apps/workflowengine/js/admin.js @@ -170,11 +170,11 @@ }).done(function(response) { // add admin groups $.each(response.data.adminGroups, function(id, group) { - self.groups.push({ id: group.id, displayname: group.name+'FIXME' }); + self.groups.push({ id: group.id, displayname: group.name }); }); // add groups $.each(response.data.groups, function(id, group) { - self.groups.push({ id: group.id, displayname: group.name+'FIXME' }); + self.groups.push({ id: group.id, displayname: group.name }); }); self.render(); }).fail(function(response) { From 51ec928623fe5d2aa0b010546f59807513c32052 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 5 Mar 2018 13:28:39 +0100 Subject: [PATCH 13/17] Send the group ids to the server on user creation Signed-off-by: Joas Schilling --- settings/js/users/users.js | 1 + 1 file changed, 1 insertion(+) diff --git a/settings/js/users/users.js b/settings/js/users/users.js index 1a20edd19d..847cceb8cc 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -1071,6 +1071,7 @@ $(document).ready(function () { promise.then(function () { var groups = $('#newuser .groups').data('groups') || {}; + groups = Object.keys(groups); $.post( OC.generateUrl('/settings/users/users'), { From 6e0044b668b8eb99c7dfc5c47e6b8f9c9f23298c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Mon, 5 Mar 2018 16:16:43 +0100 Subject: [PATCH 14/17] Fixed sharing groups select and fixed search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- apps/workflowengine/js/admin.js | 7 +- .../js/usergroupmembershipplugin.js | 2 +- settings/js/settings.js | 121 +++++++++--------- 3 files changed, 63 insertions(+), 67 deletions(-) diff --git a/apps/workflowengine/js/admin.js b/apps/workflowengine/js/admin.js index d31add77ef..891e50c418 100644 --- a/apps/workflowengine/js/admin.js +++ b/apps/workflowengine/js/admin.js @@ -167,7 +167,7 @@ url: OC.generateUrl('settings/users/groups'), dataType: 'json', quietMillis: 100, - }).done(function(response) { + }).success(function(response) { // add admin groups $.each(response.data.adminGroups, function(id, group) { self.groups.push({ id: group.id, displayname: group.name }); @@ -177,8 +177,9 @@ self.groups.push({ id: group.id, displayname: group.name }); }); self.render(); - }).fail(function(response) { - console.error('Failure happened', response); + }).error(function(data) { + OC.Notification.error(t('workflowengine', 'Unable to retrieve the group list'), {type: 'error'}); + console.log(data); }); }, delete: function() { diff --git a/apps/workflowengine/js/usergroupmembershipplugin.js b/apps/workflowengine/js/usergroupmembershipplugin.js index c664481562..b89087bea0 100644 --- a/apps/workflowengine/js/usergroupmembershipplugin.js +++ b/apps/workflowengine/js/usergroupmembershipplugin.js @@ -42,7 +42,7 @@ $(element).css('width', '400px'); $(element).select2({ - data: groups, + data: { results: groups, text: 'displayname' }, initSelection: function (element, callback) { var groupId = element.val(); if (groupId && groups.length > 0) { diff --git a/settings/js/settings.js b/settings/js/settings.js index ad0ab758e9..dd4a3a4cc7 100644 --- a/settings/js/settings.js +++ b/settings/js/settings.js @@ -24,75 +24,70 @@ OC.Settings = _.extend(OC.Settings, { var self = this; options = options || {}; if ($elements.length > 0) { - // note: settings are saved through a "change" event registered - // on all input fields - $elements.select2(_.extend({ - placeholder: t('core', 'Groups'), - allowClear: true, - multiple: true, - toggleSelect: true, - separator: '|', - query: _.debounce(function(query) { - var queryData = {}; - if (self._cachedGroups && query.term === '') { - query.callback({results: self._cachedGroups}); - return; - } - if (query.term !== '') { - queryData = { - pattern: query.term, - filterGroups: 1 - }; - } - $.ajax({ - url: OC.generateUrl('/settings/users/groups'), - data: queryData, - dataType: 'json', - success: function(data) { - var results = []; + // Let's load the data and THEN init our select + $.ajax({ + url: OC.generateUrl('/settings/users/groups'), + dataType: 'json', + success: function(data) { + var results = []; - // add groups - if (!options.excludeAdmins) { - $.each(data.data.adminGroups, function(i, group) { - results.push({id:group.id, displayname:group.name}); + // add groups + if (!options.excludeAdmins) { + $.each(data.data.adminGroups, function(i, group) { + results.push({id:group.id, displayname:group.name}); + }); + } + $.each(data.data.groups, function(i, group) { + results.push({id:group.id, displayname:group.name}); + }); + }, + always: function() { + // note: settings are saved through a "change" event registered + // on all input fields + $elements.select2(_.extend({ + placeholder: t('core', 'Groups'), + allowClear: true, + multiple: true, + toggleSelect: true, + separator: '|', + data: { results: results, text: 'displayname' }, + initSelection: function(element, callback) { + var groups = $(element).val(); + var selection; + if (groups && results.length > 0) { + selection = _.map((groups || []).split('|').sort(), function(groupId) { + return { + id: groupId, + displayname: results.find(group =>group.id === groupId).displayname + }; + }); + } else if (groups) { + selection = _.map((groups || []).split('|').sort(), function(groupId) { + return { + id: groupId, + displayname: groupId + }; }); } - $.each(data.data.groups, function(i, group) { - results.push({id:group.id, displayname:group.name}); - }); - - if (query.term === '') { - // cache full list - self._cachedGroups = results; - } - query.callback({results: results}); + callback(selection); + }, + formatResult: function (element) { + return escapeHTML(element.displayname); + }, + formatSelection: function (element) { + return escapeHTML(element.displayname); + }, + escapeMarkup: function(m) { + // prevent double markup escape + return m; } - }); - }, 100, true), - id: function(element) { - return element.id; + }, extraOptions || {})); }, - initSelection: function(element, callback) { - var selection = _.map(($(element).val() || []).split('|').sort(), function(groupId) { - return { - id: groupId, - displayname: groupId + 'FIXME' // FIXME - }; - }); - callback(selection); - }, - formatResult: function (element) { - return escapeHTML(element.displayname); - }, - formatSelection: function (element) { - return escapeHTML(element.displayname); - }, - escapeMarkup: function(m) { - // prevent double markup escape - return m; + error : function(data) { + OC.Notification.show(t('settings', 'Unable to retrieve the group list'), {type: 'error'}); + console.log(data); } - }, extraOptions || {})); + }); } } }); - From d435c91fda91caae89f43c099037d6473cc55657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Mon, 5 Mar 2018 16:18:08 +0100 Subject: [PATCH 15/17] fixup! Fixed sharing groups select and fixed search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- settings/js/settings.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/settings/js/settings.js b/settings/js/settings.js index dd4a3a4cc7..16718bd5cb 100644 --- a/settings/js/settings.js +++ b/settings/js/settings.js @@ -40,8 +40,6 @@ OC.Settings = _.extend(OC.Settings, { $.each(data.data.groups, function(i, group) { results.push({id:group.id, displayname:group.name}); }); - }, - always: function() { // note: settings are saved through a "change" event registered // on all input fields $elements.select2(_.extend({ From f1478ea4401fa28d4dc358aca9d0513853190d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Wed, 7 Mar 2018 10:33:01 +0100 Subject: [PATCH 16/17] No ES6 :( MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- apps/workflowengine/js/usergroupmembershipplugin.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/workflowengine/js/usergroupmembershipplugin.js b/apps/workflowengine/js/usergroupmembershipplugin.js index b89087bea0..53f35fedf2 100644 --- a/apps/workflowengine/js/usergroupmembershipplugin.js +++ b/apps/workflowengine/js/usergroupmembershipplugin.js @@ -48,7 +48,9 @@ if (groupId && groups.length > 0) { callback({ id: groupId, - displayname: groups.find(group =>group.id === groupId).displayname + displayname: groups.find(function (group) { + return group.id === groupId; + }).displayname }); } else if (groupId) { callback({ From 23a1553539d67f7eb317abd3857640d056853d50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Thu, 8 Mar 2018 17:09:29 +0100 Subject: [PATCH 17/17] Fixed api tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- apps/provisioning_api/lib/Controller/UsersController.php | 7 ++++--- settings/js/apps.js | 7 +++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 60b8393e1e..087e65a1bc 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -790,9 +790,10 @@ class UsersController extends OCSController { } // Get the subadmin groups - $groups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($user); - foreach ($groups as $key => $group) { - $groups[$key] = $group->getGID(); + $subAdminGroups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($user); + $groups = []; + foreach ($subAdminGroups as $key => $group) { + $groups[] = $group->getGID(); } if(!$groups) { diff --git a/settings/js/apps.js b/settings/js/apps.js index b0cc1a11a2..52d8beccae 100644 --- a/settings/js/apps.js +++ b/settings/js/apps.js @@ -641,14 +641,13 @@ OC.Settings.Apps = OC.Settings.Apps || { $('#navigation li[data-id=' + previousEntry.id + ']').after(li); // draw attention to the newly added app entry - // by flashing it twice + // by flashing twice the more apps menu if(addedApps[entry.id]) { - $('#header .menutoggle') + $('#header #more-apps') .animate({opacity: 0.5}) .animate({opacity: 1}) .animate({opacity: 0.5}) - .animate({opacity: 1}) - .animate({opacity: 0.75}); + .animate({opacity: 1}); } }