feat(sharebymail): improve share email format

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php
index ab9433f51c83062e29b18b36a8b19a1d62bd4d38..b21f6440c648fe7e443c238751395917d43c2231 100644 (file)
--- a/apps/sharebymail/lib/ShareByMailProvider.php
+++ b/apps/sharebymail/lib/ShareByMailProvider.php
@@ -328,21 +328,36 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
                        'note' => $note
                ]);
 
-               $emailTemplate->setSubject($this->l->t('%1$s shared »%2$s« with you', [$initiatorDisplayName, $filename]));
+               $emailTemplate->setSubject($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename]));
                $emailTemplate->addHeader();
-               $emailTemplate->addHeading($this->l->t('%1$s shared »%2$s« with you', [$initiatorDisplayName, $filename]), false);
-               $text = $this->l->t('%1$s shared »%2$s« with you.', [$initiatorDisplayName, $filename]);
+               $emailTemplate->addHeading($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename]), false);
+               $text = $this->l->t('%1$s shared %2$s with you.', [$initiatorDisplayName, $filename]);
 
                if ($note !== '') {
-                       $emailTemplate->addBodyText(htmlspecialchars($note), $note);
+                       $emailTemplate->addBodyListItem(
+                               htmlspecialchars($note),
+                               $this->l->t('Note:'),
+                               $this->getAbsoluteImagePath('caldav/description.png'),
+                               $note
+                       );
+               }
+
+               if ($expiration !== null) {
+                       $dateString = (string)$this->l->l('date', $expiration, ['width' => 'medium']);
+                       $emailTemplate->addBodyListItem(
+                               $this->l->t('This share is valid until %s at midnight', [$dateString]),
+                               $this->l->t('Expiration:'),
+                               $this->getAbsoluteImagePath('caldav/time.png'),
+                       );
                }
 
                $emailTemplate->addBodyText(
                        htmlspecialchars($text . ' ' . $this->l->t('Click the button below to open it.')),
                        $text
                );
+
                $emailTemplate->addBodyButton(
-                       $this->l->t('Open »%s«', [$filename]),
+                       $this->l->t('Open %s', [$filename]),
                        $link
                );
 
@@ -415,8 +430,8 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
                $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator;
                $initiatorEmailAddress = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null;
 
-               $plainBodyPart = $this->l->t("%1\$s shared »%2\$s« with you.\nYou should have already received a separate mail with a link to access it.\n", [$initiatorDisplayName, $filename]);
-               $htmlBodyPart = $this->l->t('%1$s shared »%2$s« with you. You should have already received a separate mail with a link to access it.', [$initiatorDisplayName, $filename]);
+               $plainBodyPart = $this->l->t("%1\$s shared %2\$s with you.\nYou should have already received a separate mail with a link to access it.\n", [$initiatorDisplayName, $filename]);
+               $htmlBodyPart = $this->l->t('%1$s shared %2$s with you. You should have already received a separate mail with a link to access it.', [$initiatorDisplayName, $filename]);
 
                $message = $this->mailer->createMessage();
 
@@ -428,9 +443,9 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
                        'shareWith' => $shareWith,
                ]);
 
-               $emailTemplate->setSubject($this->l->t('Password to access »%1$s« shared to you by %2$s', [$filename, $initiatorDisplayName]));
+               $emailTemplate->setSubject($this->l->t('Password to access %1$s shared to you by %2$s', [$filename, $initiatorDisplayName]));
                $emailTemplate->addHeader();
-               $emailTemplate->addHeading($this->l->t('Password to access »%s«', [$filename]), false);
+               $emailTemplate->addHeading($this->l->t('Password to access %s', [$filename]), false);
                $emailTemplate->addBodyText(htmlspecialchars($htmlBodyPart), $plainBodyPart);
                $emailTemplate->addBodyText($this->l->t('It is protected with the following password:'));
                $emailTemplate->addBodyText($password);
@@ -501,14 +516,14 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
                $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator;
                $initiatorEmailAddress = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null;
 
-               $plainHeading = $this->l->t('%1$s shared »%2$s« with you and wants to add:', [$initiatorDisplayName, $filename]);
-               $htmlHeading = $this->l->t('%1$s shared »%2$s« with you and wants to add', [$initiatorDisplayName, $filename]);
+               $plainHeading = $this->l->t('%1$s shared %2$s with you and wants to add:', [$initiatorDisplayName, $filename]);
+               $htmlHeading = $this->l->t('%1$s shared %2$s with you and wants to add', [$initiatorDisplayName, $filename]);
 
                $message = $this->mailer->createMessage();
 
                $emailTemplate = $this->mailer->createEMailTemplate('shareByMail.sendNote');
 
-               $emailTemplate->setSubject($this->l->t('»%s« added a note to a file shared with you', [$initiatorDisplayName]));
+               $emailTemplate->setSubject($this->l->t('%s added a note to a file shared with you', [$initiatorDisplayName]));
                $emailTemplate->addHeader();
                $emailTemplate->addHeading(htmlspecialchars($htmlHeading), $plainHeading);
                $emailTemplate->addBodyText(htmlspecialchars($note), $note);
@@ -516,7 +531,7 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
                $link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare',
                        ['token' => $share->getToken()]);
                $emailTemplate->addBodyButton(
-                       $this->l->t('Open »%s«', [$filename]),
+                       $this->l->t('Open %s', [$filename]),
                        $link
                );
 
@@ -564,7 +579,7 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
                        );
                }
 
-               $bodyPart = $this->l->t('You just shared »%1$s« with %2$s. The share was already sent to the recipient. Due to the security policies defined by the administrator of %3$s each share needs to be protected by password and it is not allowed to send the password directly to the recipient. Therefore you need to forward the password manually to the recipient.', [$filename, $shareWith, $this->defaults->getName()]);
+               $bodyPart = $this->l->t('You just shared %1$s with %2$s. The share was already sent to the recipient. Due to the security policies defined by the administrator of %3$s each share needs to be protected by password and it is not allowed to send the password directly to the recipient. Therefore you need to forward the password manually to the recipient.', [$filename, $shareWith, $this->defaults->getName()]);
 
                $message = $this->mailer->createMessage();
                $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.OwnerPasswordNotification', [
@@ -575,9 +590,9 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
                        'shareWith' => $shareWith,
                ]);
 
-               $emailTemplate->setSubject($this->l->t('Password to access »%1$s« shared by you with %2$s', [$filename, $shareWith]));
+               $emailTemplate->setSubject($this->l->t('Password to access %1$s shared by you with %2$s', [$filename, $shareWith]));
                $emailTemplate->addHeader();
-               $emailTemplate->addHeading($this->l->t('Password to access »%s«', [$filename]), false);
+               $emailTemplate->addHeading($this->l->t('Password to access %s', [$filename]), false);
                $emailTemplate->addBodyText($bodyPart);
                $emailTemplate->addBodyText($this->l->t('This is the password:'));
                $emailTemplate->addBodyText($password);
@@ -611,6 +626,12 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
                return true;
        }
 
+       private function getAbsoluteImagePath(string $path):string {
+               return $this->urlGenerator->getAbsoluteURL(
+                       $this->urlGenerator->imagePath('core', $path)
+               );
+       }
+
        /**
         * generate share token
         */

diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php
index c7f41176c44138985ed72b1bdc9635e59aaafdcc..e03fba723128eb03f2e48232457c7a1751165b1b 100644 (file)
--- a/apps/sharebymail/tests/ShareByMailProviderTest.php
+++ b/apps/sharebymail/tests/ShareByMailProviderTest.php
@@ -5,6 +5,7 @@
  */
 namespace OCA\ShareByMail\Tests;
 
+use DateTime;
 use OC\Mail\Message;
 use OCA\ShareByMail\Settings\SettingsManager;
 use OCA\ShareByMail\ShareByMailProvider;
@@ -1298,19 +1299,19 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('addHeading')
-                       ->with('Mrs. Owner User shared »file.txt« with you');
+                       ->with('Mrs. Owner User shared file.txt with you');
                $template
                        ->expects($this->once())
                        ->method('addBodyText')
                        ->with(
-                               'Mrs. Owner User shared »file.txt« with you. Click the button below to open it.',
-                               'Mrs. Owner User shared »file.txt« with you.'
+                               'Mrs. Owner User shared file.txt with you. Click the button below to open it.',
+                               'Mrs. Owner User shared file.txt with you.'
                        );
                $template
                        ->expects($this->once())
                        ->method('addBodyButton')
                        ->with(
-                               'Open »file.txt«',
+                               'Open file.txt',
                                'https://example.com/file.txt'
                        );
                $message
@@ -1346,7 +1347,7 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('setSubject')
-                       ->with('Mrs. Owner User shared »file.txt« with you');
+                       ->with('Mrs. Owner User shared file.txt with you');
                $message
                        ->expects($this->once())
                        ->method('useTemplate')
@@ -1411,19 +1412,32 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('addHeading')
-                       ->with('Mrs. Owner User shared »file.txt« with you');
+                       ->with('Mrs. Owner User shared file.txt with you');
                $template
-                       ->expects($this->exactly(2))
+                       ->expects($this->once())
                        ->method('addBodyText')
-                       ->withConsecutive(
-                               ['This is a note to the recipient', 'This is a note to the recipient'],
-                               ['Mrs. Owner User shared »file.txt« with you. Click the button below to open it.', 'Mrs. Owner User shared »file.txt« with you.'],
+                       ->with('Mrs. Owner User shared file.txt with you. Click the button below to open it.', 'Mrs. Owner User shared file.txt with you.');
+
+               $this->urlGenerator->expects($this->once())->method('imagePath')
+                       ->with('core', 'caldav/description.png')
+                       ->willReturn('core/img/caldav/description.png');
+               $this->urlGenerator->expects($this->once())->method('getAbsoluteURL')
+                       ->with('core/img/caldav/description.png')
+                       ->willReturn('https://example.com/core/img/caldav/description.png');
+               $template
+                       ->expects($this->once())
+                       ->method('addBodyListItem')
+                       ->with(
+                               'This is a note to the recipient',
+                               'Note:',
+                               'https://example.com/core/img/caldav/description.png',
+                               'This is a note to the recipient'
                        );
                $template
                        ->expects($this->once())
                        ->method('addBodyButton')
                        ->with(
-                               'Open »file.txt«',
+                               'Open file.txt',
                                'https://example.com/file.txt'
                        );
                $message
@@ -1459,7 +1473,7 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('setSubject')
-                       ->with('Mrs. Owner User shared »file.txt« with you');
+                       ->with('Mrs. Owner User shared file.txt with you');
                $message
                        ->expects($this->once())
                        ->method('useTemplate')
@@ -1495,6 +1509,138 @@ class ShareByMailProviderTest extends TestCase {
                );
        }
 
+       public function testSendMailNotificationWithSameUserAndUserEmailAndExpiration() {
+               $provider = $this->getInstance();
+               $user = $this->createMock(IUser::class);
+               $this->settingsManager->expects($this->any())->method('replyToInitiator')->willReturn(true);
+               $this->userManager
+                       ->expects($this->once())
+                       ->method('get')
+                       ->with('OwnerUser')
+                       ->willReturn($user);
+               $user
+                       ->expects($this->once())
+                       ->method('getDisplayName')
+                       ->willReturn('Mrs. Owner User');
+               $message = $this->createMock(Message::class);
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('createMessage')
+                       ->willReturn($message);
+               $template = $this->createMock(IEMailTemplate::class);
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('createEMailTemplate')
+                       ->willReturn($template);
+               $template
+                       ->expects($this->once())
+                       ->method('addHeader');
+               $template
+                       ->expects($this->once())
+                       ->method('addHeading')
+                       ->with('Mrs. Owner User shared file.txt with you');
+               $template
+                       ->expects($this->once())
+                       ->method('addBodyText')
+                       ->with('Mrs. Owner User shared file.txt with you. Click the button below to open it.', 'Mrs. Owner User shared file.txt with you.');
+
+               $expiration = new DateTime('2001-01-01');
+               $this->l->expects($this->once())
+                       ->method('l')
+                       ->with('date', $expiration, ['width' => 'medium'])
+                       ->willReturn('2001-01-01');
+               $this->urlGenerator->expects($this->once())->method('imagePath')
+                       ->with('core', 'caldav/time.png')
+                       ->willReturn('core/img/caldav/time.png');
+               $this->urlGenerator->expects($this->once())->method('getAbsoluteURL')
+                       ->with('core/img/caldav/time.png')
+                       ->willReturn('https://example.com/core/img/caldav/time.png');
+               $template
+                       ->expects($this->once())
+                       ->method('addBodyListItem')
+                       ->with(
+                               'This share is valid until 2001-01-01 at midnight',
+                               'Expiration:',
+                               'https://example.com/core/img/caldav/time.png',
+                       );
+
+               $template
+                       ->expects($this->once())
+                       ->method('addBodyButton')
+                       ->with(
+                               'Open file.txt',
+                               'https://example.com/file.txt'
+                       );
+               $message
+                       ->expects($this->once())
+                       ->method('setTo')
+                       ->with(['john@doe.com']);
+               $this->defaults
+                       ->expects($this->once())
+                       ->method('getName')
+                       ->willReturn('UnitTestCloud');
+               $message
+                       ->expects($this->once())
+                       ->method('setFrom')
+                       ->with([
+                               \OCP\Util::getDefaultEmailAddress('UnitTestCloud') => 'Mrs. Owner User via UnitTestCloud'
+                       ]);
+               $user
+                       ->expects($this->once())
+                       ->method('getEMailAddress')
+                       ->willReturn('owner@example.com');
+               $message
+                       ->expects($this->once())
+                       ->method('setReplyTo')
+                       ->with(['owner@example.com' => 'Mrs. Owner User']);
+               $this->defaults
+                       ->expects($this->exactly(2))
+                       ->method('getSlogan')
+                       ->willReturn('Testing like 1990');
+               $template
+                       ->expects($this->once())
+                       ->method('addFooter')
+                       ->with('UnitTestCloud - Testing like 1990');
+               $template
+                       ->expects($this->once())
+                       ->method('setSubject')
+                       ->with('Mrs. Owner User shared file.txt with you');
+               $message
+                       ->expects($this->once())
+                       ->method('useTemplate')
+                       ->with($template);
+
+               $this->mailer->expects($this->once())
+                       ->method('validateMailAddress')
+                       ->willReturn(true);
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('send')
+                       ->with($message);
+
+               $this->urlGenerator->expects($this->once())->method('linkToRouteAbsolute')
+                       ->with('files_sharing.sharecontroller.showShare', ['token' => 'token'])
+                       ->willReturn('https://example.com/file.txt');
+
+               $node = $this->getMockBuilder(File::class)->getMock();
+               $node->expects($this->any())->method('getName')->willReturn('file.txt');
+
+               $share = $this->getMockBuilder(IShare::class)->getMock();
+               $share->expects($this->any())->method('getSharedBy')->willReturn('OwnerUser');
+               $share->expects($this->any())->method('getSharedWith')->willReturn('john@doe.com');
+               $share->expects($this->any())->method('getNode')->willReturn($node);
+               $share->expects($this->any())->method('getId')->willReturn(42);
+               $share->expects($this->any())->method('getNote')->willReturn('');
+               $share->expects($this->any())->method('getExpirationDate')->willReturn($expiration);
+               $share->expects($this->any())->method('getToken')->willReturn('token');
+               
+               self::invokePrivate(
+                       $provider,
+                       'sendMailNotification',
+                       [$share]
+               );
+       }
+
        public function testSendMailNotificationWithDifferentUserAndNoUserEmail() {
                $provider = $this->getInstance();
                $initiatorUser = $this->createMock(IUser::class);
@@ -1524,19 +1670,19 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('addHeading')
-                       ->with('Mr. Initiator User shared »file.txt« with you');
+                       ->with('Mr. Initiator User shared file.txt with you');
                $template
                        ->expects($this->once())
                        ->method('addBodyText')
                        ->with(
-                               'Mr. Initiator User shared »file.txt« with you. Click the button below to open it.',
-                               'Mr. Initiator User shared »file.txt« with you.'
+                               '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())
                        ->method('addBodyButton')
                        ->with(
-                               'Open »file.txt«',
+                               'Open file.txt',
                                'https://example.com/file.txt'
                        );
                $message
@@ -1563,7 +1709,7 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('setSubject')
-                       ->with('Mr. Initiator User shared »file.txt« with you');
+                       ->with('Mr. Initiator User shared file.txt with you');
                $message
                        ->expects($this->once())
                        ->method('useTemplate')
@@ -1628,19 +1774,19 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('addHeading')
-                       ->with('Mrs. Owner User shared »file.txt« with you');
+                       ->with('Mrs. Owner User shared file.txt with you');
                $template
                        ->expects($this->once())
                        ->method('addBodyText')
                        ->with(
-                               'Mrs. Owner User shared »file.txt« with you. Click the button below to open it.',
-                               'Mrs. Owner User shared »file.txt« with you.'
+                               'Mrs. Owner User shared file.txt with you. Click the button below to open it.',
+                               'Mrs. Owner User shared file.txt with you.'
                        );
                $template
                        ->expects($this->once())
                        ->method('addBodyButton')
                        ->with(
-                               'Open »file.txt«',
+                               'Open file.txt',
                                'https://example.com/file.txt'
                        );
                $message
@@ -1671,7 +1817,7 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('setSubject')
-                       ->with('Mrs. Owner User shared »file.txt« with you');
+                       ->with('Mrs. Owner User shared file.txt with you');
                $message
                        ->expects($this->once())
                        ->method('useTemplate')
@@ -1736,19 +1882,19 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('addHeading')
-                       ->with('Mr. Initiator User shared »file.txt« with you');
+                       ->with('Mr. Initiator User shared file.txt with you');
                $template
                        ->expects($this->once())
                        ->method('addBodyText')
                        ->with(
-                               'Mr. Initiator User shared »file.txt« with you. Click the button below to open it.',
-                               'Mr. Initiator User shared »file.txt« with you.'
+                               '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())
                        ->method('addBodyButton')
                        ->with(
-                               'Open »file.txt«',
+                               'Open file.txt',
                                'https://example.com/file.txt'
                        );
                $message
@@ -1775,7 +1921,7 @@ class ShareByMailProviderTest extends TestCase {
                $template
                        ->expects($this->once())
                        ->method('setSubject')
-                       ->with('Mr. Initiator User shared »file.txt« with you');
+                       ->with('Mr. Initiator User shared file.txt with you');
                $message
                        ->expects($this->once())
                        ->method('useTemplate')