Skip to content
Snippets Groups Projects
Commit fe13f214 authored by Morris Jobke's avatar Morris Jobke Committed by GitHub
Browse files

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.
parents 1a83f119 adb60ae9
No related branches found
No related tags found
No related merge requests found
...@@ -345,7 +345,6 @@ class ShareByMailProvider implements IShareProvider { ...@@ -345,7 +345,6 @@ class ShareByMailProvider implements IShareProvider {
$this->sendMailNotification( $this->sendMailNotification(
$share->getNode()->getName(), $share->getNode()->getName(),
$link, $link,
$share->getShareOwner(),
$share->getSharedBy(), $share->getSharedBy(),
$share->getSharedWith() $share->getSharedWith()
); );
...@@ -367,37 +366,26 @@ class ShareByMailProvider implements IShareProvider { ...@@ -367,37 +366,26 @@ class ShareByMailProvider implements IShareProvider {
/** /**
* @param string $filename * @param string $filename
* @param string $link * @param string $link
* @param string $owner
* @param string $initiator * @param string $initiator
* @param string $shareWith * @param string $shareWith
* @throws \Exception If mail couldn't be sent * @throws \Exception If mail couldn't be sent
*/ */
protected function sendMailNotification($filename, protected function sendMailNotification($filename,
$link, $link,
$owner,
$initiator, $initiator,
$shareWith) { $shareWith) {
$ownerUser = $this->userManager->get($owner);
$initiatorUser = $this->userManager->get($initiator); $initiatorUser = $this->userManager->get($initiator);
$ownerDisplayName = ($ownerUser instanceof IUser) ? $ownerUser->getDisplayName() : $owner;
$initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator;
if ($owner === $initiator) { $subject = (string)$this->l->t('%s shared »%s« with you', array($initiatorDisplayName, $filename));
$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));
}
$message = $this->mailer->createMessage(); $message = $this->mailer->createMessage();
$emailTemplate = $this->mailer->createEMailTemplate(); $emailTemplate = $this->mailer->createEMailTemplate();
$emailTemplate->addHeader(); $emailTemplate->addHeader();
$emailTemplate->addHeading($this->l->t('%s shared »%s« with you', [$ownerDisplayName, $filename]), false); $emailTemplate->addHeading($this->l->t('%s shared »%s« with you', [$initiatorDisplayName, $filename]), false);
if ($owner === $initiator) { $text = $this->l->t('%s shared »%s« with you.', [$initiatorDisplayName, $filename]);
$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->addBodyText( $emailTemplate->addBodyText(
$text . ' ' . $this->l->t('Click the button below to open it.'), $text . ' ' . $this->l->t('Click the button below to open it.'),
$text $text
...@@ -414,7 +402,7 @@ class ShareByMailProvider implements IShareProvider { ...@@ -414,7 +402,7 @@ class ShareByMailProvider implements IShareProvider {
$senderName = $this->l->t( $senderName = $this->l->t(
'%s via %s', '%s via %s',
[ [
$ownerDisplayName, $initiatorDisplayName,
$instanceName $instanceName
] ]
); );
...@@ -422,9 +410,9 @@ class ShareByMailProvider implements IShareProvider { ...@@ -422,9 +410,9 @@ class ShareByMailProvider implements IShareProvider {
// The "Reply-To" is set to the sharer if an mail address is configured // 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. // also the default footer contains a "Do not reply" which needs to be adjusted.
$ownerEmail = $ownerUser->getEMailAddress(); $initiatorEmail = $initiatorUser->getEMailAddress();
if($ownerEmail !== null) { if($initiatorEmail !== null) {
$message->setReplyTo([$ownerEmail => $ownerDisplayName]); $message->setReplyTo([$initiatorEmail => $initiatorDisplayName]);
$emailTemplate->addFooter($instanceName . ' - ' . $this->defaults->getSlogan()); $emailTemplate->addFooter($instanceName . ' - ' . $this->defaults->getSlogan());
} else { } else {
$emailTemplate->addFooter(); $emailTemplate->addFooter();
......
...@@ -766,12 +766,12 @@ class ShareByMailProviderTest extends TestCase { ...@@ -766,12 +766,12 @@ class ShareByMailProviderTest extends TestCase {
$provider = $this->getInstance(); $provider = $this->getInstance();
$user = $this->createMock(IUser::class); $user = $this->createMock(IUser::class);
$this->userManager $this->userManager
->expects($this->exactly(2)) ->expects($this->once())
->method('get') ->method('get')
->with('OwnerUser') ->with('OwnerUser')
->willReturn($user); ->willReturn($user);
$user $user
->expects($this->exactly(2)) ->expects($this->once())
->method('getDisplayName') ->method('getDisplayName')
->willReturn('Mrs. Owner User'); ->willReturn('Mrs. Owner User');
$message = $this->createMock(Message::class); $message = $this->createMock(Message::class);
...@@ -867,29 +867,18 @@ class ShareByMailProviderTest extends TestCase { ...@@ -867,29 +867,18 @@ class ShareByMailProviderTest extends TestCase {
'file.txt', 'file.txt',
'https://example.com/file.txt', 'https://example.com/file.txt',
'OwnerUser', 'OwnerUser',
'OwnerUser',
'john@doe.com', 'john@doe.com',
]); ]);
} }
public function testSendMailNotificationWithDifferentUserAndNoUserEmail() { public function testSendMailNotificationWithDifferentUserAndNoUserEmail() {
$provider = $this->getInstance(); $provider = $this->getInstance();
$ownerUser = $this->createMock(IUser::class);
$initiatorUser = $this->createMock(IUser::class); $initiatorUser = $this->createMock(IUser::class);
$this->userManager $this->userManager
->expects($this->at(0)) ->expects($this->once())
->method('get')
->with('OwnerUser')
->willReturn($ownerUser);
$this->userManager
->expects($this->at(1))
->method('get') ->method('get')
->with('InitiatorUser') ->with('InitiatorUser')
->willReturn($initiatorUser); ->willReturn($initiatorUser);
$ownerUser
->expects($this->once())
->method('getDisplayName')
->willReturn('Mrs. Owner User');
$initiatorUser $initiatorUser
->expects($this->once()) ->expects($this->once())
->method('getDisplayName') ->method('getDisplayName')
...@@ -910,13 +899,13 @@ class ShareByMailProviderTest extends TestCase { ...@@ -910,13 +899,13 @@ class ShareByMailProviderTest extends TestCase {
$template $template
->expects($this->once()) ->expects($this->once())
->method('addHeading') ->method('addHeading')
->with('Mrs. Owner User shared »file.txt« with you'); ->with('Mr. Initiator User shared »file.txt« with you');
$template $template
->expects($this->once()) ->expects($this->once())
->method('addBodyText') ->method('addBodyText')
->with( ->with(
'Mrs. Owner User shared »file.txt« with you on behalf of InitiatorUser. Click the button below to open it.', 'Mr. Initiator User shared »file.txt« with you. 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.'
); );
$template $template
->expects($this->once()) ->expects($this->once())
...@@ -937,12 +926,8 @@ class ShareByMailProviderTest extends TestCase { ...@@ -937,12 +926,8 @@ class ShareByMailProviderTest extends TestCase {
->expects($this->once()) ->expects($this->once())
->method('setFrom') ->method('setFrom')
->with([ ->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 $message
->expects($this->never()) ->expects($this->never())
->method('setReplyTo'); ->method('setReplyTo');
...@@ -953,7 +938,7 @@ class ShareByMailProviderTest extends TestCase { ...@@ -953,7 +938,7 @@ class ShareByMailProviderTest extends TestCase {
$message $message
->expects($this->once()) ->expects($this->once())
->method('setSubject') ->method('setSubject')
->willReturn('Mrs. Owner User shared »file.txt« with you'); ->willReturn('Mr. Initiator User shared »file.txt« with you');
$template $template
->expects($this->once()) ->expects($this->once())
->method('renderText') ->method('renderText')
...@@ -981,7 +966,6 @@ class ShareByMailProviderTest extends TestCase { ...@@ -981,7 +966,6 @@ class ShareByMailProviderTest extends TestCase {
[ [
'file.txt', 'file.txt',
'https://example.com/file.txt', 'https://example.com/file.txt',
'OwnerUser',
'InitiatorUser', 'InitiatorUser',
'john@doe.com', 'john@doe.com',
]); ]);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment