From 7e7357c86476d3747f20b8c42c0b9a1d1fec85f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6?= <14975046+skjnldsv@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:44:59 +0200 Subject: [PATCH] feat(sharebymail): use the new email blocks for share notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The share notification now shows the sharer with their initials, the note in a quote box and a details card with the shared item, its expiration date and whether a password is required. The sharer's email address is only shown when replies are sent to them. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com> --- apps/sharebymail/lib/ShareByMailProvider.php | 42 ++++----- .../tests/ShareByMailProviderTest.php | 88 +++++++++++++------ 2 files changed, 77 insertions(+), 53 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 3a41fc790fb98..ee40ecf7e0cb7 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -26,6 +26,7 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; +use OCP\Mail\EMailDetails; use OCP\Mail\IEmailValidator; use OCP\Mail\IMailer; use OCP\Security\Events\GenerateSecurePasswordEvent; @@ -351,6 +352,11 @@ protected function sendEmail(IShare $share, array $emails): void { $initiatorUser = $this->userManager->get($initiator); $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; + // The sharer's address is only exposed when replies go to them + $initiatorEmail = null; + if ($initiatorUser instanceof IUser && $this->settingsManager->replyToInitiator()) { + $initiatorEmail = $initiatorUser->getEMailAddress(); + } $message = $this->mailer->createMessage(); $emailTemplate = $this->mailer->createEMailTemplate('sharebymail.RecipientNotification', [ @@ -364,25 +370,22 @@ protected function sendEmail(IShare $share, array $emails): void { $emailTemplate->setSubject($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename])); $emailTemplate->addHeader(); + $emailTemplate->addBodySender($initiatorDisplayName, $initiatorEmail ?? ''); $emailTemplate->addHeading($this->l->t('%1$s shared %2$s with you', [$initiatorDisplayName, $filename]), false); if ($note !== '') { - $emailTemplate->addBodyListItem( - htmlspecialchars($note), - $this->l->t('Note:'), - $this->getAbsoluteImagePath('caldav/description.png'), - $note - ); + $emailTemplate->addBodyNote($note, $this->l->t('Note from %s', [$initiatorDisplayName])); } + $details = new EMailDetails($filename); 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'), - ); + $details->addRow($this->l->t('Valid until'))->text($dateString); } + if ($share->getPassword() !== null) { + $details->addRow($this->l->t('Password'))->text($this->l->t('Required')); + } + $emailTemplate->addBodyDetails($details); $emailTemplate->addBodyButton( $this->l->t('Open shared item'), @@ -413,14 +416,9 @@ protected function sendEmail(IShare $share, array $emails): void { // 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. - if ($initiatorUser && $this->settingsManager->replyToInitiator()) { - $initiatorEmail = $initiatorUser->getEMailAddress(); - if ($initiatorEmail !== null) { - $message->setReplyTo([$initiatorEmail => $initiatorDisplayName]); - $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); - } else { - $emailTemplate->addFooter(); - } + if ($initiatorEmail !== null) { + $message->setReplyTo([$initiatorEmail => $initiatorDisplayName]); + $emailTemplate->addFooter($instanceName . ($this->defaults->getSlogan() !== '' ? ' - ' . $this->defaults->getSlogan() : '')); } else { $emailTemplate->addFooter(); } @@ -653,12 +651,6 @@ 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 */ diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index 59971ce925324..c0f7f45b7de87 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -26,6 +26,7 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; +use OCP\Mail\EMailDetails; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; use OCP\Mail\IMessage; @@ -396,7 +397,7 @@ public function testCreateSendPasswordByMailWithEnforcedPasswordProtectionWithPe $instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'autogeneratedPassword'])->willReturn($expectedShare); // Initially not set, but will be set by the autoGeneratePassword method. - $share->expects($this->exactly(3))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword'); + $share->expects($this->exactly(4))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword', 'autogeneratedPassword'); $share->expects($this->never())->method('setPassword'); $share->expects($this->once())->method('setPasswordHash')->with($this->callback(fn (string $hash): bool => Server::get(IHasher::class)->verify('autogeneratedPassword', $hash))); @@ -475,7 +476,7 @@ public function testCreateSendPasswordByMailWithPasswordAndWithEnforcedPasswordP $instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare', 'password' => 'password']); $instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'password'])->willReturn($expectedShare); - $share->expects($this->exactly(3))->method('getPassword')->willReturn('password'); + $share->expects($this->exactly(4))->method('getPassword')->willReturn('password'); $share->expects($this->never())->method('setPassword'); $share->expects($this->never())->method('setPasswordHash'); @@ -563,7 +564,7 @@ public function testCreateSendPasswordByTalkWithEnforcedPasswordProtectionWithPe $instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare', 'password' => 'autogeneratedPassword']); $instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'autogeneratedPassword'])->willReturn($expectedShare); - $share->expects($this->exactly(3))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword'); + $share->expects($this->exactly(4))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword', 'autogeneratedPassword'); $share->expects($this->never())->method('setPassword'); $share->expects($this->once())->method('setPasswordHash')->with($this->callback(fn (string $hash): bool => Server::get(IHasher::class)->verify('autogeneratedPassword', $hash))); @@ -1336,6 +1337,10 @@ public function testSendMailNotificationWithSameUserAndUserEmail(): void { $template ->expects($this->once()) ->method('addHeader'); + $template + ->expects($this->once()) + ->method('addBodySender') + ->with('Mrs. Owner User', 'owner@example.com'); $template ->expects($this->once()) ->method('addHeading') @@ -1413,6 +1418,44 @@ public function testSendMailNotificationWithSameUserAndUserEmail(): void { ); } + public function testSendEmailListsPasswordInDetails(): void { + $provider = $this->getInstance(); + $this->settingsManager->method('replyToInitiator')->willReturn(false); + $this->userManager->method('get')->with('OwnerUser')->willReturn(null); + $this->mailer->method('createMessage')->willReturn($this->createMock(Message::class)); + $this->mailer->method('send')->willReturn([]); + $this->defaults->method('getName')->willReturn('UnitTestCloud'); + $this->urlGenerator->method('linkToRouteAbsolute')->willReturn('https://example.com/file.txt'); + + $template = $this->createMock(IEMailTemplate::class); + $this->mailer->method('createEMailTemplate')->willReturn($template); + $template->expects($this->once()) + ->method('addBodySender') + ->with('OwnerUser', ''); + $template->expects($this->never()) + ->method('addBodyNote'); + $template->expects($this->once()) + ->method('addBodyDetails') + ->with($this->callback(function (EMailDetails $details): bool { + $rows = $details->getRows(); + return $details->getTitle() === 'file.txt' + && count($rows) === 1 + && $rows[0]->getLabel() === 'Password' + && $rows[0]->getParts()[0]['text'] === 'Required'; + })); + + $node = $this->createMock(File::class); + $node->method('getName')->willReturn('file.txt'); + $share = $this->createMock(IShare::class); + $share->method('getSharedBy')->willReturn('OwnerUser'); + $share->method('getNode')->willReturn($node); + $share->method('getNote')->willReturn(''); + $share->method('getToken')->willReturn('token'); + $share->method('getPassword')->willReturn('password'); + + self::invokePrivate($provider, 'sendEmail', [$share, ['john@doe.com']]); + } + public function testSendMailNotificationWithSameUserAndUserEmailAndNote(): void { $provider = $this->getInstance(); $user = $this->createMock(IUser::class); @@ -1444,21 +1487,10 @@ public function testSendMailNotificationWithSameUserAndUserEmailAndNote(): void ->method('addHeading') ->with('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' - ); + ->method('addBodyNote') + ->with('This is a note to the recipient', 'Note from Mrs. Owner User'); $template ->expects($this->once()) ->method('addBodyButton') @@ -1568,20 +1600,16 @@ public function testSendMailNotificationWithSameUserAndUserEmailAndExpiration(): ->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', - ); + ->method('addBodyDetails') + ->with($this->callback(function (EMailDetails $details): bool { + $rows = $details->getRows(); + return $details->getTitle() === 'file.txt' + && count($rows) === 1 + && $rows[0]->getLabel() === 'Valid until' + && $rows[0]->getParts()[0]['text'] === '2001-01-01'; + })); $template ->expects($this->once()) @@ -1777,6 +1805,10 @@ public function testSendMailNotificationWithSameUserAndUserEmailAndReplyToDesact $template ->expects($this->once()) ->method('addHeader'); + $template + ->expects($this->once()) + ->method('addBodySender') + ->with('Mrs. Owner User', ''); $template ->expects($this->once()) ->method('addHeading')