Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Loading
Loading status checks…
feat(sharebymail): improve share email format
Browse files Browse the repository at this point in the history 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
skjnldsv committed Aug 2, 2024

Verified

This commit was signed with the committer’s verified signature.
rouault Even Rouault
GPG key ID: 33EBBFC47B3DD87D
Verified
Learn about vigilant mode
1 parent 669e4e7 commit 96b2d57
Showing 2 changed files with 210 additions and 43 deletions.
53 changes: 37 additions & 16 deletions apps/sharebymail/lib/ShareByMailProvider.php
View file
Original file line number Diff line number Diff line change
@@ -328,21 +328,36 @@ protected function sendEmail(IShare $share, array $emails): void {
'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 @@ protected function sendPassword(IShare $share, string $password, array $emails):
$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 @@ protected function sendPassword(IShare $share, string $password, array $emails):
'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,22 +516,22 @@ protected function sendNote(IShare $share): void {
$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);

$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 @@ protected function sendPasswordToOwner(IShare $share, string $password): bool {
);
}

$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 @@ protected function sendPasswordToOwner(IShare $share, string $password): bool {
'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 @@ protected function sendPasswordToOwner(IShare $share, string $password): bool {
return true;
}

private function getAbsoluteImagePath(string $path):string {
return $this->urlGenerator->getAbsoluteURL(
$this->urlGenerator->imagePath('core', $path)
);
}

/**
* generate share token
*/
200 changes: 173 additions & 27 deletions apps/sharebymail/tests/ShareByMailProviderTest.php
View file
Original file line number Diff line number Diff line change
@@ -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 @@ public function testSendMailNotificationWithSameUserAndUserEmail() {
$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 @@ public function testSendMailNotificationWithSameUserAndUserEmail() {
$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 @@ public function testSendMailNotificationWithSameUserAndUserEmailAndNote() {
$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 @@ public function testSendMailNotificationWithSameUserAndUserEmailAndNote() {
$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 @@ public function testSendMailNotificationWithSameUserAndUserEmailAndNote() {
);
}

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 @@ public function testSendMailNotificationWithDifferentUserAndNoUserEmail() {
$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 @@ public function testSendMailNotificationWithDifferentUserAndNoUserEmail() {
$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 @@ public function testSendMailNotificationWithSameUserAndUserEmailAndReplyToDesact
$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 @@ public function testSendMailNotificationWithSameUserAndUserEmailAndReplyToDesact
$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 @@ public function testSendMailNotificationWithDifferentUserAndNoUserEmailAndReplyT
$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 @@ public function testSendMailNotificationWithDifferentUserAndNoUserEmailAndReplyT
$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')

0 comments on commit 96b2d57

Please sign in to comment.