Merge pull request #4692 from nextcloud/unify-sharing-messages

don't mention the owner of a file in case of a re-share by mail.

2 files changed, 16 insertions, 44 deletions

diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php
index 7e0f7c5071e..3f618678ce1 100644
--- a/apps/sharebymail/lib/ShareByMailProvider.php
+++ b/apps/sharebymail/lib/ShareByMailProvider.php
@@ -345,7 +345,6 @@ class ShareByMailProvider implements IShareProvider {
 			$this->sendMailNotification(
 				$share->getNode()->getName(),
 				$link,
-				$share->getShareOwner(),
 				$share->getSharedBy(),
 				$share->getSharedWith()
 			);
@@ -367,37 +366,26 @@ class ShareByMailProvider implements IShareProvider {
 	/**
 	 * @param string $filename
 	 * @param string $link
-	 * @param string $owner
 	 * @param string $initiator
 	 * @param string $shareWith
 	 * @throws \Exception If mail couldn't be sent
 	 */
 	protected function sendMailNotification($filename,
 											$link,
-											$owner,
 											$initiator,
 											$shareWith) {
-		$ownerUser = $this->userManager->get($owner);
 		$initiatorUser = $this->userManager->get($initiator);
-		$ownerDisplayName = ($ownerUser instanceof IUser) ? $ownerUser->getDisplayName() : $owner;
 		$initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator;
-		if ($owner === $initiator) {
-			$subject = (string)$this->l->t('%s shared »%s« with you', array($ownerDisplayName, $filename));
-		} else {
-			$subject = (string)$this->l->t('%s shared »%s« with you on behalf of %s', array($ownerDisplayName, $filename, $initiatorDisplayName));
-		}
+		$subject = (string)$this->l->t('%s shared »%s« with you', array($initiatorDisplayName, $filename));
 
 		$message = $this->mailer->createMessage();
 
 		$emailTemplate = $this->mailer->createEMailTemplate();
 
 		$emailTemplate->addHeader();
-		$emailTemplate->addHeading($this->l->t('%s shared »%s« with you', [$ownerDisplayName, $filename]), false);
-		if ($owner === $initiator) {
-			$text = $this->l->t('%s shared »%s« with you.', [$ownerDisplayName, $filename]);
-		} else {
-			$text= $this->l->t('%s shared »%s« with you on behalf of %s.', [$ownerDisplayName, $filename, $initiator]);
-		}
+		$emailTemplate->addHeading($this->l->t('%s shared »%s« with you', [$initiatorDisplayName, $filename]), false);
+		$text = $this->l->t('%s shared »%s« with you.', [$initiatorDisplayName, $filename]);
+
 		$emailTemplate->addBodyText(
 			$text . ' ' . $this->l->t('Click the button below to open it.'),
 			$text
@@ -414,7 +402,7 @@ class ShareByMailProvider implements IShareProvider {
 		$senderName = $this->l->t(
 			'%s via %s',
 			[
-				$ownerDisplayName,
+				$initiatorDisplayName,
 				$instanceName
 			]
 		);
@@ -422,9 +410,9 @@ class ShareByMailProvider implements IShareProvider {
 
 		// The "Reply-To" is set to the sharer if an mail address is configured
 		// also the default footer contains a "Do not reply" which needs to be adjusted.
-		$ownerEmail = $ownerUser->getEMailAddress();
-		if($ownerEmail !== null) {
-			$message->setReplyTo([$ownerEmail => $ownerDisplayName]);
+		$initiatorEmail = $initiatorUser->getEMailAddress();
+		if($initiatorEmail !== null) {
+			$message->setReplyTo([$initiatorEmail => $initiatorDisplayName]);
 			$emailTemplate->addFooter($instanceName . ' - ' . $this->defaults->getSlogan());
 		} else {
 			$emailTemplate->addFooter();
diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php
index 269f8e8f414..e649a9dbd0b 100644
--- a/apps/sharebymail/tests/ShareByMailProviderTest.php
+++ b/apps/sharebymail/tests/ShareByMailProviderTest.php
@@ -766,12 +766,12 @@ class ShareByMailProviderTest extends TestCase {
 		$provider = $this->getInstance();
 		$user = $this->createMock(IUser::class);
 		$this->userManager
-			->expects($this->exactly(2))
+			->expects($this->once())
 			->method('get')
 			->with('OwnerUser')
 			->willReturn($user);
 		$user
-			->expects($this->exactly(2))
+			->expects($this->once())
 			->method('getDisplayName')
 			->willReturn('Mrs. Owner User');
 		$message = $this->createMock(Message::class);
@@ -867,29 +867,18 @@ class ShareByMailProviderTest extends TestCase {
 				'file.txt',
 				'https://example.com/file.txt',
 				'OwnerUser',
-				'OwnerUser',
 				'john@doe.com',
 			]);
 	}
 
 	public function testSendMailNotificationWithDifferentUserAndNoUserEmail() {
 		$provider = $this->getInstance();
-		$ownerUser = $this->createMock(IUser::class);
 		$initiatorUser = $this->createMock(IUser::class);
 		$this->userManager
-			->expects($this->at(0))
-			->method('get')
-			->with('OwnerUser')
-			->willReturn($ownerUser);
-		$this->userManager
-			->expects($this->at(1))
+			->expects($this->once())
 			->method('get')
 			->with('InitiatorUser')
 			->willReturn($initiatorUser);
-		$ownerUser
-			->expects($this->once())
-			->method('getDisplayName')
-			->willReturn('Mrs. Owner User');
 		$initiatorUser
 			->expects($this->once())
 			->method('getDisplayName')
@@ -910,13 +899,13 @@ class ShareByMailProviderTest extends TestCase {
 		$template
 			->expects($this->once())
 			->method('addHeading')
-			->with('Mrs. Owner User shared »file.txt« with you');
+			->with('Mr. Initiator User shared »file.txt« with you');
 		$template
 			->expects($this->once())
 			->method('addBodyText')
 			->with(
-				'Mrs. Owner User shared »file.txt« with you on behalf of InitiatorUser. Click the button below to open it.',
-				'Mrs. Owner User shared »file.txt« with you on behalf of InitiatorUser.'
+				'Mr. Initiator User shared »file.txt« with you. Click the button below to open it.',
+				'Mr. Initiator User shared »file.txt« with you.'
 			);
 		$template
 			->expects($this->once())
@@ -937,12 +926,8 @@ class ShareByMailProviderTest extends TestCase {
 			->expects($this->once())
 			->method('setFrom')
 			->with([
-				\OCP\Util::getDefaultEmailAddress('UnitTestCloud') => 'Mrs. Owner User via UnitTestCloud'
+				\OCP\Util::getDefaultEmailAddress('UnitTestCloud') => 'Mr. Initiator User via UnitTestCloud'
 			]);
-		$ownerUser
-			->expects($this->once())
-			->method('getEMailAddress')
-			->willReturn(null);
 		$message
 			->expects($this->never())
 			->method('setReplyTo');
@@ -953,7 +938,7 @@ class ShareByMailProviderTest extends TestCase {
 		$message
 			->expects($this->once())
 			->method('setSubject')
-			->willReturn('Mrs. Owner User shared »file.txt« with you');
+			->willReturn('Mr. Initiator User shared »file.txt« with you');
 		$template
 			->expects($this->once())
 			->method('renderText')
@@ -981,7 +966,6 @@ class ShareByMailProviderTest extends TestCase {
 			[
 				'file.txt',
 				'https://example.com/file.txt',
-				'OwnerUser',
 				'InitiatorUser',
 				'john@doe.com',
 			]);