From 71c837723062cddac6c7b8413fcf5ee5210f73fe Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 28 Mar 2024 18:30:43 +0100 Subject: [PATCH] feat: Group activities send by mail Signed-off-by: Louis Chemineau --- lib/AppInfo/Application.php | 7 +- lib/GroupHelper.php | 100 +++++++++++----------- lib/MailQueueHandler.php | 148 +++++++++------------------------ tests/MailQueueHandlerTest.php | 21 ++++- 4 files changed, 113 insertions(+), 163 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index cbe53101b..00c65f945 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -31,11 +31,13 @@ use OCA\Activity\Consumer; use OCA\Activity\Data; use OCA\Activity\FilesHooksStatic; +use OCA\Activity\GroupHelper; use OCA\Activity\Listener\LoadSidebarScripts; use OCA\Activity\Listener\SetUserDefaults; use OCA\Activity\Listener\UserDeleted; use OCA\Activity\MailQueueHandler; use OCA\Activity\NotificationGenerator; +use OCA\Activity\UserSettings; use OCA\Activity\Dashboard\ActivityWidget; use OCA\Files\Event\LoadSidebar; use OCP\Activity\IManager; @@ -124,7 +126,10 @@ public function register(IRegistrationContext $context): void { $c->get(IManager::class), $c->get(IValidator::class), $c->get(IConfig::class), - $c->get(ILogger::class) + $c->get(ILogger::class), + $c->get(Data::class), + $c->get(GroupHelper::class), + $c->get(UserSettings::class), ); }); diff --git a/lib/GroupHelper.php b/lib/GroupHelper.php index 43ae73d6a..ed738a75e 100644 --- a/lib/GroupHelper.php +++ b/lib/GroupHelper.php @@ -31,12 +31,9 @@ class GroupHelper { /** @var IEvent[] */ - protected $event = []; - /** @var int */ - protected $lastEvent = 0; - - /** @var bool */ - protected $allowGrouping; + protected array $event = []; + protected int $lastEvent = 0; + protected bool $allowGrouping = true; /** @var IL10N */ protected $l; @@ -53,30 +50,36 @@ class GroupHelper { public function __construct(IL10N $l, IManager $activityManager, IValidator $richObjectValidator, - ILogger $logger) { - $this->allowGrouping = true; - + ILogger $logger + ) { $this->l = $l; $this->activityManager = $activityManager; $this->richObjectValidator = $richObjectValidator; $this->logger = $logger; } - /** - * @param IL10N $l - */ - public function setL10n(IL10N $l) { + public function resetEvents(): void { + $this->event = []; + $this->lastEvent = 0; + } + + public function setL10n(IL10N $l): void { $this->l = $l; } /** * Add an activity to the internal array - * - * @param array $activity */ - public function addActivity($activity) { + public function addActivity(array $activity): void { $id = (int) $activity['activity_id']; $event = $this->arrayToEvent($activity); + $this->addEvent($id, $event); + } + + /** + * Add an event to the internal array + */ + public function addEvent(int $id, IEvent $event): void { $language = $this->l->getLanguageCode(); foreach ($this->activityManager->getProviders() as $provider) { @@ -87,34 +90,37 @@ public function addActivity($activity) { } else { $event = $provider->parse($language, $event); } - try { - $this->richObjectValidator->validate($event->getRichSubject(), $event->getRichSubjectParameters()); - } catch (InvalidObjectExeption $e) { - $this->logger->logException($e); - $event->setRichSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed', []); - $event->setParsedSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed'); - } - - if ($event->getRichMessage()) { - try { - $this->richObjectValidator->validate($event->getRichMessage(), $event->getRichMessageParameters()); - } catch (InvalidObjectExeption $e) { - $this->logger->logException($e); - $event->setRichMessage('Rich message or a parameter is malformed', []); - $event->setParsedMessage('Rich message or a parameter is malformed'); - } - } + } catch (\InvalidArgumentException $e) { + } catch (\Throwable $e) { + $this->logger->error('Error while parsing activity event', ['exception' => $e]); + } + } - $this->activityManager->setFormattingObject('', 0); + try { + $this->richObjectValidator->validate($event->getRichSubject(), $event->getRichSubjectParameters()); + } catch (InvalidObjectExeption $e) { + $this->logger->logException($e); + $event->setRichSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed', []); + $event->setParsedSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed'); + } - $child = $event->getChildEvent(); - if ($child instanceof IEvent) { - unset($this->event[$this->lastEvent]); - } - } catch (\InvalidArgumentException $e) { + if ($event->getRichMessage()) { + try { + $this->richObjectValidator->validate($event->getRichMessage(), $event->getRichMessageParameters()); + } catch (InvalidObjectExeption $e) { + $this->logger->logException($e); + $event->setRichMessage('Rich message or a parameter is malformed', []); + $event->setParsedMessage('Rich message or a parameter is malformed'); } } + $this->activityManager->setFormattingObject('', 0); + + $child = $event->getChildEvent(); + if ($child instanceof IEvent) { + unset($this->event[$this->lastEvent]); + } + if (!$event->getParsedSubject()) { $this->logger->debug('Activity "' . $event->getRichSubject() . '" was not parsed by any provider'); return; @@ -129,7 +135,7 @@ public function addActivity($activity) { * * @return array translated activities ready for use */ - public function getActivities() { + public function getActivities(): array { $return = []; foreach ($this->event as $id => $event) { $return[] = $this->eventToArray($event, $id); @@ -148,11 +154,7 @@ public function getEvents(): array { return $return; } - /** - * @param array $row - * @return IEvent - */ - protected function arrayToEvent(array $row) { + protected function arrayToEvent(array $row): IEvent { $event = $this->activityManager->generateEvent(); $event->setApp((string) $row['app']) ->setType((string) $row['type']) @@ -168,10 +170,8 @@ protected function arrayToEvent(array $row) { } /** - * @param IEvent $event - * @return array */ - protected function eventToArray(IEvent $event, $id) { + protected function eventToArray(IEvent $event, $id): array { return [ 'activity_id' => $id, 'app' => $event->getApp(), @@ -198,10 +198,6 @@ protected function eventToArray(IEvent $event, $id) { ]; } - /** - * @param IEvent $event - * @return array - */ protected function getObjectsFromChildren(IEvent $event): array { $child = $event->getChildEvent(); $objects = []; diff --git a/lib/MailQueueHandler.php b/lib/MailQueueHandler.php index 2bca5fd34..63e45b0c9 100644 --- a/lib/MailQueueHandler.php +++ b/lib/MailQueueHandler.php @@ -31,13 +31,13 @@ use OCP\IConfig; use OCP\IDateTimeFormatter; use OCP\IDBConnection; +use OCP\IL10N; use OCP\ILogger; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\L10N\IFactory; use OCP\Mail\IMailer; -use OCP\RichObjectStrings\InvalidObjectExeption; use OCP\RichObjectStrings\IValidator; use OCP\Util; @@ -49,20 +49,13 @@ */ class MailQueueHandler { public const CLI_EMAIL_BATCH_SIZE = 500; - public const WEB_EMAIL_BATCH_SIZE = 25; - /** Number of entries we want to list in the email */ public const ENTRY_LIMIT = 200; - /** @var array */ - protected $languages; - - /** @var string */ - protected $senderAddress; - - /** @var string */ - protected $senderName; + protected array $languages; + protected string $senderAddress; + protected string $senderName; /** @var IDateTimeFormatter */ protected $dateFormatter; @@ -103,7 +96,11 @@ public function __construct(IDateTimeFormatter $dateFormatter, IManager $activityManager, IValidator $richObjectValidator, IConfig $config, - ILogger $logger) { + ILogger $logger, + protected Data $data, + protected GroupHelper $groupHelper, + protected UserSettings $userSettings, + ) { $this->dateFormatter = $dateFormatter; $this->connection = $connection; $this->mailer = $mailer; @@ -119,13 +116,13 @@ public function __construct(IDateTimeFormatter $dateFormatter, /** * Send an email to {$limit} users * - * @param int $limit Number of users we want to send an email to - * @param int $sendTime The latest send time - * @param bool $forceSending Ignores latest send and just sends all emails - * @param null|int $restrictEmails null or one of UserSettings::EMAIL_SEND_* + * @param $limit Number of users we want to send an email to + * @param $sendTime The latest send time + * @param $forceSending Ignores latest send and just sends all emails + * @param $restrictEmails null or one of UserSettings::EMAIL_SEND_* * @return int Number of users we sent an email to */ - public function sendEmails($limit, $sendTime, $forceSending = false, $restrictEmails = null) { + public function sendEmails(int $limit, int $sendTime, bool $forceSending = false, ?int $restrictEmails = null): int { // Get all users which should receive an email $affectedUsers = $this->getAffectedUsers($limit, $sendTime, $forceSending, $restrictEmails); if (empty($affectedUsers)) { @@ -186,14 +183,8 @@ public function sendEmails($limit, $sendTime, $forceSending = false, $restrictEm /** * Get the users we want to send an email to - * - * @param int|null $limit - * @param int $latestSend - * @param bool $forceSending - * @param int|null $restrictEmails - * @return array */ - protected function getAffectedUsers($limit, $latestSend, $forceSending, $restrictEmails) { + protected function getAffectedUsers(?int $limit, int $latestSend, bool $forceSending, ?int $restrictEmails): array { $query = $this->connection->getQueryBuilder(); $query->select('amq_affecteduser') ->selectAlias($query->createFunction('MIN(' . $query->getColumnName('amq_latest_send') . ')'), 'amq_trigger_time') @@ -237,12 +228,9 @@ protected function getAffectedUsers($limit, $latestSend, $forceSending, $restric /** * Get all items for the user we want to send an email to * - * @param string $affectedUser - * @param int $maxTime - * @param int $maxNumItems * @return array [data of the first max. 200 entries, total number of entries] */ - protected function getItemsForUser($affectedUser, $maxTime, $maxNumItems = self::ENTRY_LIMIT) { + protected function getItemsForUser(string $affectedUser, int $maxTime, int $maxNumItems = self::ENTRY_LIMIT): array { $query = $this->connection->getQueryBuilder(); $query->select('*') ->from('activity_mq') @@ -287,9 +275,9 @@ public function purgeItemsForUser(string $affectedUser): void { * Get a language object for a specific language * * @param string $lang Language identifier - * @return \OCP\IL10N Language object of $lang + * @return IL10N Language object of $lang */ - protected function getLanguage($lang) { + protected function getLanguage(string $lang): IL10N { if (!isset($this->languages[$lang])) { $this->languages[$lang] = $this->lFactory->get('activity', $lang); } @@ -300,9 +288,8 @@ protected function getLanguage($lang) { /** * Get the sender data * @param string $setting Either `email` or `name` - * @return string */ - protected function getSenderData($setting) { + protected function getSenderData(string $setting): string { if (empty($this->senderAddress)) { $this->senderAddress = Util::getDefaultEmailAddress('no-reply'); } @@ -320,15 +307,10 @@ protected function getSenderData($setting) { /** * Send a notification to one user * - * @param string $userName Username of the recipient - * @param string $email Email address of the recipient - * @param string $lang Selected language of the recipient - * @param string $timezone Selected timezone of the recipient - * @param int $maxTime * @return bool True if the entries should be removed, false otherwise * @throws \UnexpectedValueException */ - protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime) { + protected function sendEmailToUser(string $userName, string $email, string $lang, string $timezone, int $maxTime): bool { $user = $this->userManager->get($userName); if (!$user instanceof IUser) { return true; @@ -344,7 +326,9 @@ protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime $l = $this->getLanguage($lang); $this->activityManager->setCurrentUserId($userName); - $activityEvents = []; + $this->groupHelper->resetEvents(); + $this->groupHelper->setL10n($l); + foreach ($mailData as $activity) { $event = $this->activityManager->generateEvent(); try { @@ -358,24 +342,23 @@ protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime continue; } - $relativeDateTime = $this->dateFormatter->formatDateTimeRelativeDay( - (int) $activity['amq_timestamp'], - 'long', 'short', - new \DateTimeZone($timezone), $l - ); - - try { - $event = $this->parseEvent($lang, $event); - } catch (\InvalidArgumentException $e) { - continue; - } - - $activityEvents[] = [ - 'event' => $event, - 'relativeDateTime' => $relativeDateTime - ]; + $this->groupHelper->addEvent($activity['mail_id'], $event); } + $activityEvents = array_map( + function ($event) use ($timezone, $l) { + return [ + 'event' => $event, + 'relativeDateTime' => $this->dateFormatter->formatDateTimeRelativeDay( + $event->getTimestamp(), + 'long', 'short', + new \DateTimeZone($timezone), $l + ) + ]; + }, + $this->groupHelper->getEvents() + ); + $template = $this->mailer->createEMailTemplate('activity.Notification', [ 'displayname' => $user->getDisplayName(), 'url' => $this->urlGenerator->getAbsoluteURL('/'), @@ -393,7 +376,6 @@ protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime ); foreach ($activityEvents as $activity) { - /** @var IEvent $event */ $event = $activity['event']; $relativeDateTime = $activity['relativeDateTime']; @@ -426,10 +408,6 @@ protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime return true; } - /** - * @param IEvent $event - * @return string - */ protected function getHTMLSubject(IEvent $event): string { if ($event->getRichSubject() === '') { return htmlspecialchars($event->getParsedSubject()); @@ -455,58 +433,10 @@ protected function getHTMLSubject(IEvent $event): string { return str_replace($placeholders, $replacements, $event->getRichSubject()); } - /** - * @param string $lang - * @param IEvent $event - * @return IEvent - * @throws \InvalidArgumentException when the event could not be parsed - */ - protected function parseEvent($lang, IEvent $event) { - $this->activityManager->setFormattingObject($event->getObjectType(), $event->getObjectId()); - foreach ($this->activityManager->getProviders() as $provider) { - try { - $event = $provider->parse($lang, $event); - } catch (\InvalidArgumentException $e) { - /* Ignore */ - } catch (\Throwable $e) { - $this->logger->error('Error while parsing activity event', ['exception' => $e]); - } - } - $this->activityManager->setFormattingObject('', 0); - - try { - $this->richObjectValidator->validate($event->getRichSubject(), $event->getRichSubjectParameters()); - } catch (InvalidObjectExeption $e) { - $this->logger->logException($e); - $event->setRichSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed', []); - $event->setParsedSubject('Rich subject or a parameter for "' . $event->getRichSubject() . '" is malformed'); - } - - if ($event->getRichMessage()) { - try { - $this->richObjectValidator->validate($event->getRichMessage(), $event->getRichMessageParameters()); - } catch (InvalidObjectExeption $e) { - $this->logger->logException($e); - $event->setRichMessage('Rich message or a parameter is malformed', []); - $event->setParsedMessage('Rich message or a parameter is malformed'); - } - } - - if (!$event->getParsedSubject()) { - $this->logger->debug('Activity "' . $event->getRichSubject() . '" was not parsed by any provider'); - throw new \InvalidArgumentException('Activity "' . $event->getRichSubject() . '" was not parsed by any provider'); - } - - return $event; - } - /** * Delete all entries we dealt with - * - * @param array $affectedUsers - * @param int $maxTime */ - protected function deleteSentItems(array $affectedUsers, $maxTime) { + protected function deleteSentItems(array $affectedUsers, int $maxTime): void { if (empty($affectedUsers)) { return; } diff --git a/tests/MailQueueHandlerTest.php b/tests/MailQueueHandlerTest.php index 1d82ef8e4..c5095334e 100644 --- a/tests/MailQueueHandlerTest.php +++ b/tests/MailQueueHandlerTest.php @@ -26,7 +26,10 @@ namespace OCA\Activity\Tests; +use OCA\Activity\Data; +use OCA\Activity\GroupHelper; use OCA\Activity\MailQueueHandler; +use OCA\Activity\UserSettings; use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; @@ -83,6 +86,16 @@ class MailQueueHandlerTest extends TestCase { /** @var IDateTimeFormatter|MockObject */ protected $dateTimeFormatter; + /** @var MockObject|Data */ + protected Data $data; + + /** @var MockObject|GroupHelper */ + protected GroupHelper $groupHelper; + + /** @var MockObject|UserSettings */ + protected UserSettings $userSettings; + + protected function setUp(): void { parent::setUp(); @@ -92,6 +105,9 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->logger = $this->createMock(ILogger::class); $this->dateTimeFormatter = $this->createMock(IDateTimeFormatter::class); + $this->data = $this->createMock(Data::class); + $this->groupHelper = $this->createMock(GroupHelper::class); + $this->userSettings = $this->createMock(UserSettings::class); $connection = \OC::$server->getDatabaseConnection(); $query = $connection->prepare('INSERT INTO `*PREFIX*activity_mq` ' @@ -158,7 +174,10 @@ protected function setUp(): void { $this->activityManager, $this->richObjectValidator, $this->config, - $this->logger + $this->logger, + $this->data, + $this->groupHelper, + $this->userSettings, ); }