Merge pull request #12389 from nextcloud/fix-dav-groupid

Fix displayName return of dav groups request

2 files changed, 17 insertions, 48 deletions

diff --git a/apps/dav/lib/DAV/GroupPrincipalBackend.php b/apps/dav/lib/DAV/GroupPrincipalBackend.php
index 166af7c6b27..02f8edf3928 100644
--- a/apps/dav/lib/DAV/GroupPrincipalBackend.php
+++ b/apps/dav/lib/DAV/GroupPrincipalBackend.php
@@ -28,7 +28,6 @@ use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IUserSession;
 use OCP\Share\IManager as IShareManager;
-use OCP\IL10N;
 use OCP\IUser;
 use Sabre\DAV\Exception;
 use \Sabre\DAV\PropPatch;
@@ -47,23 +46,17 @@ class GroupPrincipalBackend implements BackendInterface {
 	/** @var IShareManager */
 	private $shareManager;
 
-	/** @var IL10N */
-	private $l10n;
-
 	/**
 	 * @param IGroupManager $IGroupManager
 	 * @param IUserSession $userSession
 	 * @param IShareManager $shareManager
-	 * @param IL10N $l10n
 	 */
 	public function __construct(IGroupManager $IGroupManager,
 								IUserSession $userSession,
-								IShareManager $shareManager,
-								IL10N $l10n) {
+								IShareManager $shareManager) {
 		$this->groupManager = $IGroupManager;
 		$this->userSession = $userSession;
 		$this->shareManager = $shareManager;
-		$this->l10n = $l10n;
 	}
 
 	/**
@@ -293,10 +286,12 @@ class GroupPrincipalBackend implements BackendInterface {
 	 */
 	protected function groupToPrincipal($group) {
 		$groupId = $group->getGID();
+		// getDisplayName returns UID if none
+		$displayName = $group->getDisplayName();
 
 		return [
 			'uri' => 'principals/groups/' . urlencode($groupId),
-			'{DAV:}displayname' => $this->l10n->t('%s (group)', [$groupId]),
+			'{DAV:}displayname' => $displayName,
 			'{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'GROUP',
 		];
 	}
@@ -307,11 +302,12 @@ class GroupPrincipalBackend implements BackendInterface {
 	 */
 	protected function userToPrincipal($user) {
 		$userId = $user->getUID();
+		// getDisplayName returns UID if none
 		$displayName = $user->getDisplayName();
 
 		$principal = [
 			'uri' => 'principals/users/' . $userId,
-			'{DAV:}displayname' => is_null($displayName) ? $userId : $displayName,
+			'{DAV:}displayname' => $displayName,
 			'{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'INDIVIDUAL',
 		];
 
diff --git a/apps/dav/tests/unit/DAV/GroupPrincipalTest.php b/apps/dav/tests/unit/DAV/GroupPrincipalTest.php
index af94df05254..c9a72e0b948 100644
--- a/apps/dav/tests/unit/DAV/GroupPrincipalTest.php
+++ b/apps/dav/tests/unit/DAV/GroupPrincipalTest.php
@@ -30,7 +30,6 @@ use OC\Group\Group;
 use OCA\DAV\DAV\GroupPrincipalBackend;
 use OCP\IGroup;
 use OCP\IGroupManager;
-use OCP\IL10N;
 use OCP\IUser;
 use OCP\IUserSession;
 use OCP\Share\IManager;
@@ -47,9 +46,6 @@ class GroupPrincipalTest extends \Test\TestCase {
 	/** @var IManager | \PHPUnit_Framework_MockObject_MockObject */
 	private $shareManager;
 
-	/** @var IL10N | \PHPUnit_Framework_MockObject_MockObject */
-	private $l10n;
-
 	/** @var GroupPrincipalBackend */
 	private $connector;
 
@@ -57,13 +53,11 @@ class GroupPrincipalTest extends \Test\TestCase {
 		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->shareManager = $this->createMock(IManager::class);
-		$this->l10n = $this->createMock(IL10N::class);
 
 		$this->connector = new GroupPrincipalBackend(
 			$this->groupManager,
 			$this->userSession,
-			$this->shareManager,
-			$this->l10n);
+			$this->shareManager);
 		parent::setUp();
 	}
 
@@ -81,25 +75,15 @@ class GroupPrincipalTest extends \Test\TestCase {
 			->with('')
 			->will($this->returnValue([$group1, $group2]));
 
-		$this->l10n->expects($this->at(0))
-			->method('t')
-			->with('%s (group)', ['foo'])
-			->will($this->returnValue('foo (Gruppe)'));
-
-		$this->l10n->expects($this->at(1))
-			->method('t')
-			->with('%s (group)', ['bar'])
-			->will($this->returnValue('bar (Gruppe)'));
-
 		$expectedResponse = [
 			0 => [
 				'uri' => 'principals/groups/foo',
-				'{DAV:}displayname' => 'foo (Gruppe)',
+				'{DAV:}displayname' => 'Group foo',
 				'{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'GROUP',
 			],
 			1 => [
 				'uri' => 'principals/groups/bar',
-				'{DAV:}displayname' => 'bar (Gruppe)',
+				'{DAV:}displayname' => 'Group bar',
 				'{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'GROUP',
 			]
 		];
@@ -126,14 +110,9 @@ class GroupPrincipalTest extends \Test\TestCase {
 			->with('foo')
 			->will($this->returnValue($group1));
 
-		$this->l10n->expects($this->at(0))
-			->method('t')
-			->with('%s (group)', ['foo'])
-			->will($this->returnValue('foo (Gruppe)'));
-
 		$expectedResponse = [
 			'uri' => 'principals/groups/foo',
-			'{DAV:}displayname' => 'foo (Gruppe)',
+			'{DAV:}displayname' => 'Group foo',
 			'{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'GROUP',
 		];
 		$response = $this->connector->getPrincipalByPath('principals/groups/foo');
@@ -148,14 +127,9 @@ class GroupPrincipalTest extends \Test\TestCase {
 			->with('foo')
 			->will($this->returnValue($fooUser));
 
-		$this->l10n->expects($this->at(0))
-			->method('t')
-			->with('%s (group)', ['foo'])
-			->will($this->returnValue('foo (Gruppe)'));
-
 		$expectedResponse = [
 			'uri' => 'principals/groups/foo',
-			'{DAV:}displayname' => 'foo (Gruppe)',
+			'{DAV:}displayname' => 'Group foo',
 			'{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'GROUP',
 		];
 		$response = $this->connector->getPrincipalByPath('principals/groups/foo');
@@ -181,14 +155,9 @@ class GroupPrincipalTest extends \Test\TestCase {
 			->with('foo/bar')
 			->will($this->returnValue($group1));
 
-		$this->l10n->expects($this->at(0))
-			->method('t')
-			->with('%s (group)', ['foo/bar'])
-			->will($this->returnValue('foo/bar (Gruppe)'));
-
 		$expectedResponse = [
 			'uri' => 'principals/groups/foo%2Fbar',
-			'{DAV:}displayname' => 'foo/bar (Gruppe)',
+			'{DAV:}displayname' => 'Group foo/bar',
 			'{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'GROUP',
 		];
 		$response = $this->connector->getPrincipalByPath('principals/groups/foo/bar');
@@ -348,7 +317,11 @@ class GroupPrincipalTest extends \Test\TestCase {
 		$fooGroup
 			->expects($this->exactly(1))
 			->method('getGID')
-			->will($this->returnValue($gid));
+			->willReturn($gid);
+		$fooGroup
+			->expects($this->exactly(1))
+			->method('getDisplayName')
+			->willReturn('Group '.$gid);
 		return $fooGroup;
 	}
 }