mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #8779 from nextcloud/backport/8255/show-group-display-names 

[stable13] Show group display names
This commit is contained in:
John Molakvoæ 2018-04-06 15:14:12 +02:00 committed by GitHub
parent b9ca121671 5e15c76d21
commit d45a889fe2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 178 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
apps/dav/lib/CalDAV/Activity/Provider/Base.php
View File

@ -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;
}
}

6
apps/dav/lib/CalDAV/Activity/Provider/Calendar.php
View File

@ -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;

6
apps/dav/lib/CalDAV/Activity/Provider/Event.php
View File

@ -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;

8
apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php
View File

@ -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();

64
apps/files_sharing/lib/Activity/Providers/Groups.php
View File

@ -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;
}
}

2
lib/private/Group/MetaData.php
View File

@ -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,
);
}

2
lib/private/Settings/Personal/PersonalInfo.php
View File

@ -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)
);

15
settings/Controller/GroupsController.php
View File

@ -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
);

56
tests/Settings/Controller/GroupsControllerTest.php
View File

@ -16,6 +16,7 @@ use OC\Settings\Controller\GroupsController;
use OC\User\User;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IRequest;
@ -66,6 +67,9 @@ class GroupsControllerTest extends \Test\TestCase {
$firstGroup
->method('getGID')
->will($this->returnValue('firstGroup'));
$firstGroup
->method('getDisplayName')
->will($this->returnValue('1st group'));
$firstGroup
->method('count')
->will($this->returnValue(12));
@ -74,6 +78,9 @@ class GroupsControllerTest extends \Test\TestCase {
$secondGroup
->method('getGID')
->will($this->returnValue('secondGroup'));
$secondGroup
->method('getDisplayName')
->will($this->returnValue('2nd group'));
$secondGroup
->method('count')
->will($this->returnValue(25));
@ -82,6 +89,9 @@ class GroupsControllerTest extends \Test\TestCase {
$thirdGroup
->method('getGID')
->will($this->returnValue('thirdGroup'));
$thirdGroup
->method('getDisplayName')
->will($this->returnValue('3rd group'));
$thirdGroup
->method('count')
->will($this->returnValue(14));
@ -90,6 +100,9 @@ class GroupsControllerTest extends \Test\TestCase {
$fourthGroup
->method('getGID')
->will($this->returnValue('admin'));
$fourthGroup
->method('getDisplayName')
->will($this->returnValue('Administrators'));
$fourthGroup
->method('count')
->will($this->returnValue(18));
@ -119,7 +132,7 @@ class GroupsControllerTest extends \Test\TestCase {
'adminGroups' => array(
0 => array(
'id' => 'admin',
'name' => 'admin',
'name' => 'Administrators',
'usercount' => 0,//User count disabled 18,
)
),
@ -127,17 +140,17 @@ class GroupsControllerTest extends \Test\TestCase {
array(
0 => array(
'id' => 'firstGroup',
'name' => 'firstGroup',
'name' => '1st group',
'usercount' => 0,//User count disabled 12,
),
1 => array(
'id' => 'secondGroup',
'name' => 'secondGroup',
'name' => '2nd group',
'usercount' => 0,//User count disabled 25,
),
2 => array(
'id' => 'thirdGroup',
'name' => 'thirdGroup',
'name' => '3rd group',
'usercount' => 0,//User count disabled 14,
),
)
@ -158,6 +171,9 @@ class GroupsControllerTest extends \Test\TestCase {
$firstGroup
->method('getGID')
->will($this->returnValue('firstGroup'));
$firstGroup
->method('getDisplayName')
->will($this->returnValue('1st group'));
$firstGroup
->method('count')
->will($this->returnValue(12));
@ -166,6 +182,9 @@ class GroupsControllerTest extends \Test\TestCase {
$secondGroup
->method('getGID')
->will($this->returnValue('secondGroup'));
$secondGroup
->method('getDisplayName')
->will($this->returnValue('2nd group'));
$secondGroup
->method('count')
->will($this->returnValue(25));
@ -174,6 +193,9 @@ class GroupsControllerTest extends \Test\TestCase {
$thirdGroup
->method('getGID')
->will($this->returnValue('thirdGroup'));
$thirdGroup
->method('getDisplayName')
->will($this->returnValue('3rd group'));
$thirdGroup
->method('count')
->will($this->returnValue(14));
@ -182,6 +204,9 @@ class GroupsControllerTest extends \Test\TestCase {
$fourthGroup
->method('getGID')
->will($this->returnValue('admin'));
$fourthGroup
->method('getDisplayName')
->will($this->returnValue('Administrators'));
$fourthGroup
->method('count')
->will($this->returnValue(18));
@ -212,7 +237,7 @@ class GroupsControllerTest extends \Test\TestCase {
'adminGroups' => array(
0 => array(
'id' => 'admin',
'name' => 'admin',
'name' => 'Administrators',
'usercount' => 18,
)
),
@ -220,17 +245,17 @@ class GroupsControllerTest extends \Test\TestCase {
array(
0 => array(
'id' => 'secondGroup',
'name' => 'secondGroup',
'name' => '2nd group',
'usercount' => 25,
),
1 => array(
'id' => 'thirdGroup',
'name' => 'thirdGroup',
'name' => '3rd group',
'usercount' => 14,
),
2 => array(
'id' => 'firstGroup',
'name' => 'firstGroup',
'name' => '1st group',
'usercount' => 12,
),
)
@ -264,15 +289,19 @@ class GroupsControllerTest extends \Test\TestCase {
->method('groupExists')
->with('NewGroup')
->will($this->returnValue(false));
$group = $this->createMock(IGroup::class);
$group->method('getDisplayName')
->willReturn('New group');
$this->groupManager
->expects($this->once())
->method('createGroup')
->with('NewGroup')
->will($this->returnValue(true));
->willReturn($group);
$expectedResponse = new DataResponse(
array(
'groupname' => 'NewGroup'
'groupname' => 'New group'
),
Http::STATUS_CREATED
);
@ -304,13 +333,14 @@ class GroupsControllerTest extends \Test\TestCase {
}
public function testDestroySuccessful() {
$group = $this->getMockBuilder(Group::class)
->disableOriginalConstructor()->getMock();
$group = $this->createMock(IGroup::class);
$this->groupManager
->expects($this->once())
->method('get')
->with('ExistingGroup')
->will($this->returnValue($group));
$group->method('getDisplayName')
->willReturn('Existing group');
$group
->expects($this->once())
->method('delete')
@ -319,7 +349,7 @@ class GroupsControllerTest extends \Test\TestCase {
$expectedResponse = new DataResponse(
array(
'status' => 'success',
'data' => array('groupname' => 'ExistingGroup')
'data' => array('groupname' => 'Existing group')
),
Http::STATUS_NO_CONTENT
);

19
tests/lib/Group/MetaDataTest.php
View File

@ -53,12 +53,19 @@ 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'));
'admin', 'admin',
'g2', 'g2',
'g3', 'g3'));
$group->expects($this->exactly(3))
->method('getDisplayName')
->will($this->onConsecutiveCalls(
'display admin',
'display g2',
'display g3'));
$group->expects($this->exactly($countCallCount))
->method('count')
@ -83,7 +90,7 @@ class MetaDataTest extends \Test\TestCase {
$this->assertSame(1, count($adminGroups));
$this->assertSame(2, count($ordinaryGroups));
$this->assertSame('g2', $ordinaryGroups[0]['name']);
$this->assertSame('display g2', $ordinaryGroups[0]['name']);
// user count is not loaded
$this->assertSame(0, $ordinaryGroups[0]['usercount']);
}
@ -103,7 +110,7 @@ class MetaDataTest extends \Test\TestCase {
$this->assertSame(1, count($adminGroups));
$this->assertSame(2, count($ordinaryGroups));
$this->assertSame('g3', $ordinaryGroups[0]['name']);
$this->assertSame('display g3', $ordinaryGroups[0]['name']);
$this->assertSame(5, $ordinaryGroups[0]['usercount']);
}