]> source.dussan.org Git - nextcloud-server.git/commitdiff
Add "Reply-To" support to sharing mails and refactor code
authorLukas Reschke <lukas@owncloud.com>
Fri, 10 Apr 2015 15:21:52 +0000 (17:21 +0200)
committerLukas Reschke <lukas@owncloud.com>
Fri, 10 Apr 2015 15:30:07 +0000 (17:30 +0200)
core/ajax/share.php
lib/private/share/mailnotifications.php
tests/lib/mail/message.php
tests/lib/share/MailNotificationsTest.php [new file with mode: 0644]

index bc83c41642cdf6397d6755873be3b0d230cfe673..d9bf97d6464a68b6d83b2f3fec5b983d59813172 100644 (file)
@@ -119,7 +119,14 @@ if (isset($_POST['action']) && isset($_POST['itemType']) && isset($_POST['itemSo
                        // don't send a mail to the user who shared the file
                        $recipientList = array_diff($recipientList, array(\OCP\User::getUser()));
 
-                       $mailNotification = new OC\Share\MailNotifications();
+                       $mailNotification = new \OC\Share\MailNotifications(
+                               \OC::$server->getUserSession()->getUser()->getUID(),
+                               \OC::$server->getConfig(),
+                               \OC::$server->getL10N('lib'),
+                               \OC::$server->getMailer(),
+                               \OC::$server->getLogger(),
+                               $defaults
+                       );
                        $result = $mailNotification->sendInternalShareMail($recipientList, $itemSource, $itemType);
 
                        \OCP\Share::setSendMailStatus($itemType, $itemSource, $shareType, $recipient, true);
@@ -151,7 +158,14 @@ if (isset($_POST['action']) && isset($_POST['itemType']) && isset($_POST['itemSo
                        $file = (string)$_POST['file'];
                        $to_address = (string)$_POST['toaddress'];
 
-                       $mailNotification = new \OC\Share\MailNotifications();
+                       $mailNotification = new \OC\Share\MailNotifications(
+                               \OC::$server->getUserSession()->getUser()->getUID(),
+                               \OC::$server->getConfig(),
+                               \OC::$server->getL10N('lib'),
+                               \OC::$server->getMailer(),
+                               \OC::$server->getLogger(),
+                               $defaults
+                       );
 
                        $expiration = null;
                        if (isset($_POST['expiration']) && $_POST['expiration'] !== '') {
index 68f2e66a09801c94291572778dfc86819630242b..1c15b6e3e1d31452a9092127b88d8a8de3f94a32 100644 (file)
@@ -6,6 +6,7 @@
  * @author Robin McCorkell <rmccorkell@karoshi.org.uk>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
  * @author Tom Needham <tom@owncloud.com>
+ * @author Lukas Reschke <lukas@owncloud.com>
  *
  * @copyright Copyright (c) 2015, ownCloud, Inc.
  * @license AGPL-3.0
 namespace OC\Share;
 
 use DateTime;
+use OCP\IConfig;
+use OCP\IL10N;
+use OCP\Mail\IMailer;
+use OCP\ILogger;
+use OCP\Defaults;
 
+/**
+ * Class MailNotifications
+ *
+ * @package OC\Share
+ */
 class MailNotifications {
 
-       /**
-        * sender userId
-        * @var null|string
-        */
-       private $senderId;
-
-       /**
-        * sender email address
-        * @var string
-        */
-       private $from;
-
-       /**
-        * @var string
-        */
+       /** @var string sender userId */
+       private $userId;
+       /** @var string sender email address */
+       private $replyTo;
+       /** @var string */
        private $senderDisplayName;
-
-       /**
-        * @var \OC_L10N
-        */
+       /** @var IL10N */
        private $l;
+       /** @var IConfig */
+       private $config;
+       /** @var IMailer */
+       private $mailer;
+       /** @var Defaults */
+       private $defaults;
+       /** @var ILogger */
+       private $logger;
 
        /**
-        *
-        * @param string $sender user id (if nothing is set we use the currently logged-in user)
+        * @param string $uid user id
+        * @param IConfig $config
+        * @param IL10N $l10n
+        * @param IMailer $mailer
+        * @param ILogger $logger
+        * @param Defaults $defaults
         */
-       public function __construct($sender = null) {
-               $this->l = \OC::$server->getL10N('lib');
-
-               $this->senderId = $sender;
-
-               $this->from = \OCP\Util::getDefaultEmailAddress('sharing-noreply');
-               if ($this->senderId) {
-                       $this->from = \OCP\Config::getUserValue($this->senderId, 'settings', 'email', $this->from);
-                       $this->senderDisplayName = \OCP\User::getDisplayName($this->senderId);
-               } else {
-                       $this->senderDisplayName = \OCP\User::getDisplayName();
-               }
+       public function __construct($uid,
+                                                               IConfig $config,
+                                                               IL10N $l10n,
+                                                               IMailer $mailer,
+                                                               ILogger $logger,
+                                                               Defaults $defaults) {
+               $this->l = $l10n;
+               $this->userId = $uid;
+               $this->config = $config;
+               $this->mailer = $mailer;
+               $this->logger = $logger;
+               $this->defaults = $defaults;
+
+               $this->replyTo = $this->config->getUserValue($this->userId, 'settings', 'email', null);
+               $this->senderDisplayName = \OCP\User::getDisplayName($this->userId);
        }
 
        /**
@@ -79,12 +92,11 @@ class MailNotifications {
         * @return array list of user to whom the mail send operation failed
         */
        public function sendInternalShareMail($recipientList, $itemSource, $itemType) {
-
-               $noMail = array();
+               $noMail = [];
 
                foreach ($recipientList as $recipient) {
                        $recipientDisplayName = \OCP\User::getDisplayName($recipient);
-                       $to = \OC::$server->getConfig()->getUserValue($recipient, 'settings', 'email', '');
+                       $to = $this->config->getUserValue($recipient, 'settings', 'email', '');
 
                        if ($to === '') {
                                $noMail[] = $recipientDisplayName;
@@ -100,7 +112,7 @@ class MailNotifications {
                                        $date = new DateTime($items[0]['expiration']);
                                        $expiration = $date->getTimestamp();
                                } catch (\Exception $e) {
-                                       \OCP\Util::writeLog('sharing', "Couldn't read date: " . $e->getMessage(), \OCP\Util::ERROR);
+                                       $this->logger->error("Couldn't read date: ".$e->getMessage(), ['app' => 'sharing']);
                                }
                        }
 
@@ -119,13 +131,29 @@ class MailNotifications {
 
                        $link = \OCP\Util::linkToAbsolute('files', 'index.php', $args);
 
-                       list($htmlMail, $alttextMail) = $this->createMailBody($filename, $link, $expiration);
+                       list($htmlBody, $textBody) = $this->createMailBody($filename, $link, $expiration);
 
                        // send it out now
                        try {
-                               \OCP\Util::sendMail($to, $recipientDisplayName, $subject, $htmlMail, $this->from, $this->senderDisplayName, 1, $alttextMail);
+                               $message = $this->mailer->createMessage();
+                               $message->setSubject($subject);
+                               $message->setTo([$to => $recipientDisplayName]);
+                               $message->setHtmlBody($htmlBody);
+                               $message->setPlainBody($textBody);
+                               $message->setFrom([
+                                       \OCP\Util::getDefaultEmailAddress('sharing-noreply') =>
+                                               (string)$this->l->t('%s via %s', [
+                                                       $this->senderDisplayName,
+                                                       $this->defaults->getName()
+                                               ]),
+                                       ]);
+                               if(!is_null($this->replyTo)) {
+                                       $message->setReplyTo([$this->replyTo]);
+                               }
+
+                               $this->mailer->send($message);
                        } catch (\Exception $e) {
-                               \OCP\Util::writeLog('sharing', "Can't send mail to inform the user about an internal share: " . $e->getMessage() , \OCP\Util::ERROR);
+                               $this->logger->error("Can't send mail to inform the user about an internal share: ".$e->getMessage(), ['app' => 'sharing']);
                                $noMail[] = $recipientDisplayName;
                        }
                }
@@ -144,19 +172,31 @@ class MailNotifications {
         * @return array $result of failed recipients
         */
        public function sendLinkShareMail($recipient, $filename, $link, $expiration) {
-               $subject = (string)$this->l->t('%s shared »%s« with you', array($this->senderDisplayName, $filename));
-               list($htmlMail, $alttextMail) = $this->createMailBody($filename, $link, $expiration);
-               $rs = explode(' ', $recipient);
-               $failed = array();
-               foreach ($rs as $r) {
-                       try {
-                               \OCP\Util::sendMail($r, $r, $subject, $htmlMail, $this->from, $this->senderDisplayName, 1, $alttextMail);
-                       } catch (\Exception $e) {
-                               \OCP\Util::writeLog('sharing', "Can't send mail with public link to $r: " . $e->getMessage(), \OCP\Util::ERROR);
-                               $failed[] = $r;
+               $subject = (string)$this->l->t('%s shared »%s« with you', [$this->senderDisplayName, $filename]);
+               list($htmlBody, $textBody) = $this->createMailBody($filename, $link, $expiration);
+
+               try {
+                       $message = $this->mailer->createMessage();
+                       $message->setSubject($subject);
+                       $message->setTo([$recipient]);
+                       $message->setHtmlBody($htmlBody);
+                       $message->setPlainBody($textBody);
+                       $message->setFrom([
+                               \OCP\Util::getDefaultEmailAddress('sharing-noreply') =>
+                                       (string)$this->l->t('%s via %s', [
+                                               $this->senderDisplayName,
+                                               $this->defaults->getName()
+                                       ]),
+                       ]);
+                       if(!is_null($this->replyTo)) {
+                               $message->setReplyTo([$this->replyTo]);
                        }
+
+                       return $this->mailer->send($message);
+               } catch (\Exception $e) {
+                       $this->logger->error("Can't send mail with public link to $recipient: ".$e->getMessage(), ['app' => 'sharing']);
+                       return [$recipient];
                }
-               return $failed;
        }
 
        /**
@@ -169,23 +209,23 @@ class MailNotifications {
         */
        private function createMailBody($filename, $link, $expiration) {
 
-               $formatedDate = $expiration ? $this->l->l('date', $expiration) : null;
+               $formattedDate = $expiration ? $this->l->l('date', $expiration) : null;
 
                $html = new \OC_Template("core", "mail", "");
                $html->assign ('link', $link);
                $html->assign ('user_displayname', $this->senderDisplayName);
                $html->assign ('filename', $filename);
-               $html->assign('expiration',  $formatedDate);
+               $html->assign('expiration',  $formattedDate);
                $htmlMail = $html->fetchPage();
 
-               $alttext = new \OC_Template("core", "altmail", "");
-               $alttext->assign ('link', $link);
-               $alttext->assign ('user_displayname', $this->senderDisplayName);
-               $alttext->assign ('filename', $filename);
-               $alttext->assign('expiration', $formatedDate);
-               $alttextMail = $alttext->fetchPage();
+               $plainText = new \OC_Template("core", "altmail", "");
+               $plainText->assign ('link', $link);
+               $plainText->assign ('user_displayname', $this->senderDisplayName);
+               $plainText->assign ('filename', $filename);
+               $plainText->assign('expiration', $formattedDate);
+               $plainTextMail = $plainText->fetchPage();
 
-               return array($htmlMail, $alttextMail);
+               return [$htmlMail, $plainTextMail];
        }
 
 }
index 0db2017d81e1aa9c976253d05cf576b45032fcce..c75cc18b650bb45aff22f428e2e7756422e84673 100644 (file)
@@ -62,6 +62,23 @@ class MessageTest extends TestCase {
                $this->assertSame(array('lukas@owncloud.com'), $this->message->getFrom());
        }
 
+       public function testSetReplyTo() {
+               $this->swiftMessage
+                       ->expects($this->once())
+                       ->method('setReplyTo')
+                       ->with(['lukas@owncloud.com']);
+               $this->message->setReplyTo(['lukas@owncloud.com']);
+       }
+
+       public function testGetReplyTo() {
+               $this->swiftMessage
+                       ->expects($this->once())
+                       ->method('getReplyTo')
+                       ->will($this->returnValue(['lukas@owncloud.com']));
+
+               $this->assertSame(['lukas@owncloud.com'], $this->message->getReplyTo());
+       }
+
        public function testSetTo() {
                $this->swiftMessage
                        ->expects($this->once())
diff --git a/tests/lib/share/MailNotificationsTest.php b/tests/lib/share/MailNotificationsTest.php
new file mode 100644 (file)
index 0000000..c74fe40
--- /dev/null
@@ -0,0 +1,237 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+use OC\Share\MailNotifications;
+use OCP\IConfig;
+use OCP\IL10N;
+use OCP\Mail\IMailer;
+use OCP\ILogger;
+use OCP\Defaults;
+
+/**
+ * Class MailNotificationsTest
+ */
+class MailNotificationsTest extends \Test\TestCase {
+       /** @var IConfig */
+       private $config;
+       /** @var IL10N */
+       private $l10n;
+       /** @var IMailer */
+       private $mailer;
+       /** @var ILogger */
+       private $logger;
+       /** @var Defaults */
+       private $defaults;
+
+
+       public function setUp() {
+               parent::setUp();
+
+               $this->config = $this->getMockBuilder('\OCP\IConfig')
+                       ->disableOriginalConstructor()->getMock();
+               $this->l10n = $this->getMockBuilder('\OCP\IL10N')
+                       ->disableOriginalConstructor()->getMock();
+               $this->mailer = $this->getMockBuilder('\OCP\Mail\IMailer')
+                       ->disableOriginalConstructor()->getMock();
+               $this->logger = $this->getMockBuilder('\OCP\ILogger')
+                       ->disableOriginalConstructor()->getMock();
+               $this->defaults = $this->getMockBuilder('\OCP\Defaults')
+                       ->disableOriginalConstructor()->getMock();
+
+               $this->l10n->expects($this->any())
+                       ->method('t')
+                       ->will($this->returnCallback(function($text, $parameters = array()) {
+                               return vsprintf($text, $parameters);
+                       }));
+       }
+
+       public function testSendLinkShareMailWithoutReplyTo() {
+               $message = $this->getMockBuilder('\OC\Mail\Message')
+                       ->disableOriginalConstructor()->getMock();
+
+               $message
+                       ->expects($this->once())
+                       ->method('setSubject')
+                       ->with('TestUser shared »MyFile« with you');
+               $message
+                       ->expects($this->once())
+                       ->method('setTo')
+                       ->with(['lukas@owncloud.com']);
+               $message
+                       ->expects($this->once())
+                       ->method('setHtmlBody');
+               $message
+                       ->expects($this->once())
+                       ->method('setPlainBody');
+               $message
+                       ->expects($this->once())
+                       ->method('setFrom')
+                       ->with([\OCP\Util::getDefaultEmailAddress('sharing-noreply') => 'TestUser via UnitTestCloud']);
+
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('createMessage')
+                       ->will($this->returnValue($message));
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('send')
+                       ->with($message)
+                       ->will($this->returnValue([]));
+
+               $this->defaults
+                       ->expects($this->once())
+                       ->method('getName')
+                       ->will($this->returnValue('UnitTestCloud'));
+
+               $this->config
+                       ->expects($this->at(0))
+                       ->method('getUserValue')
+                       ->with('TestUser', 'settings', 'email', null)
+                       ->will($this->returnValue('sharer@owncloud.com'));
+
+               $mailNotifications = new MailNotifications(
+                       'TestUser',
+                       $this->config,
+                       $this->l10n,
+                       $this->mailer,
+                       $this->logger,
+                       $this->defaults
+               );
+
+               $this->assertSame([], $mailNotifications->sendLinkShareMail('lukas@owncloud.com', 'MyFile', 'https://owncloud.com/file/?foo=bar', 3600));
+       }
+
+       public function testSendLinkShareMailWithReplyTo() {
+               $message = $this->getMockBuilder('\OC\Mail\Message')
+                       ->disableOriginalConstructor()->getMock();
+
+               $message
+                       ->expects($this->once())
+                       ->method('setSubject')
+                       ->with('TestUser shared »MyFile« with you');
+               $message
+                       ->expects($this->once())
+                       ->method('setTo')
+                       ->with(['lukas@owncloud.com']);
+               $message
+                       ->expects($this->once())
+                       ->method('setHtmlBody');
+               $message
+                       ->expects($this->once())
+                       ->method('setPlainBody');
+               $message
+                       ->expects($this->once())
+                       ->method('setFrom')
+                       ->with([\OCP\Util::getDefaultEmailAddress('sharing-noreply') => 'TestUser via UnitTestCloud']);
+               $message
+                       ->expects($this->once())
+                       ->method('setReplyTo')
+                       ->with(['sharer@owncloud.com']);
+
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('createMessage')
+                       ->will($this->returnValue($message));
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('send')
+                       ->with($message)
+                       ->will($this->returnValue([]));
+
+               $this->defaults
+                       ->expects($this->once())
+                       ->method('getName')
+                       ->will($this->returnValue('UnitTestCloud'));
+
+               $this->config
+                       ->expects($this->at(0))
+                       ->method('getUserValue')
+                       ->with('TestUser', 'settings', 'email', null)
+                       ->will($this->returnValue('sharer@owncloud.com'));
+
+               $mailNotifications = new MailNotifications(
+                       'TestUser',
+                       $this->config,
+                       $this->l10n,
+                       $this->mailer,
+                       $this->logger,
+                       $this->defaults
+               );
+               $this->assertSame([], $mailNotifications->sendLinkShareMail('lukas@owncloud.com', 'MyFile', 'https://owncloud.com/file/?foo=bar', 3600));
+       }
+
+       public function testSendLinkShareMailException() {
+               $message = $this->getMockBuilder('\OC\Mail\Message')
+                       ->disableOriginalConstructor()->getMock();
+
+               $message
+                       ->expects($this->once())
+                       ->method('setSubject')
+                       ->with('TestUser shared »MyFile« with you');
+               $message
+                       ->expects($this->once())
+                       ->method('setTo')
+                       ->with(['lukas@owncloud.com']);
+               $message
+                       ->expects($this->once())
+                       ->method('setHtmlBody');
+               $message
+                       ->expects($this->once())
+                       ->method('setPlainBody');
+               $message
+                       ->expects($this->once())
+                       ->method('setFrom')
+                       ->with([\OCP\Util::getDefaultEmailAddress('sharing-noreply') => 'TestUser via UnitTestCloud']);
+
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('createMessage')
+                       ->will($this->returnValue($message));
+               $this->mailer
+                       ->expects($this->once())
+                       ->method('send')
+                       ->with($message)
+                       ->will($this->throwException(new Exception('Some Exception Message')));
+
+               $this->defaults
+                       ->expects($this->once())
+                       ->method('getName')
+                       ->will($this->returnValue('UnitTestCloud'));
+
+               $this->config
+                       ->expects($this->at(0))
+                       ->method('getUserValue')
+                       ->with('TestUser', 'settings', 'email', null)
+                       ->will($this->returnValue('sharer@owncloud.com'));
+
+               $mailNotifications = new MailNotifications(
+                       'TestUser',
+                       $this->config,
+                       $this->l10n,
+                       $this->mailer,
+                       $this->logger,
+                       $this->defaults
+               );
+
+               $this->assertSame(['lukas@owncloud.com'], $mailNotifications->sendLinkShareMail('lukas@owncloud.com', 'MyFile', 'https://owncloud.com/file/?foo=bar', 3600));
+       }
+
+}