From adb60ae965c6e79a899d46f0770100d4d5ce6a64 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 4 May 2017 12:41:44 +0200 Subject: [PATCH] don't mention the owner of a file in case of a re-share by mail. The recipient doesn't care about the owner and probably don't even know it Signed-off-by: Bjoern Schiessle --- apps/sharebymail/lib/ShareByMailProvider.php | 28 +++++----------- .../tests/ShareByMailProviderTest.php | 32 +++++-------------- 2 files changed, 16 insertions(+), 44 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 7e0f7c5071..3f618678ce 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 269f8e8f41..e649a9dbd0 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', ]);