From f97fc51a776ddcce12a7b6782f23e5621895f17e Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Tue, 20 Apr 2021 17:12:01 +0200 Subject: [PATCH] Only show each thread once per message list Signed-off-by: Daniel Kesselberg --- appinfo/routes.php | 10 + lib/Contracts/IMailManager.php | 4 +- lib/Controller/MessagesController.php | 2 +- lib/Controller/ThreadController.php | 162 ++++++++++ lib/Db/MessageMapper.php | 161 +++++----- lib/Db/ThreadMapper.php | 74 +++++ lib/Service/MailManager.php | 4 +- src/components/Envelope.vue | 12 +- src/components/EnvelopeList.vue | 21 +- src/components/Mailbox.vue | 4 +- src/components/MenuEnvelope.vue | 7 +- src/components/MoveModal.vue | 26 +- src/components/Thread.vue | 20 +- src/service/ThreadService.js | 27 ++ src/store/actions.js | 27 +- .../Unit/Controller/ThreadControllerTest.php | 280 ++++++++++++++++++ tests/Unit/Service/MailManagerTest.php | 7 +- 17 files changed, 728 insertions(+), 120 deletions(-) create mode 100755 lib/Controller/ThreadController.php create mode 100644 lib/Db/ThreadMapper.php create mode 100644 src/service/ThreadService.js create mode 100644 tests/Unit/Controller/ThreadControllerTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index 4f80ed450d..d9cf23e30e 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -268,6 +268,16 @@ 'name' => 'sieve#updateActiveScript', 'url' => '/api/sieve/active/{id}', 'verb' => 'PUT' + ], + [ + 'name' => 'thread#delete', + 'url' => '/api/thread/{id}', + 'verb' => 'DELETE' + ], + [ + 'name' => 'thread#move', + 'url' => '/api/thread/{id}', + 'verb' => 'POST' ] ], 'resources' => [ diff --git a/lib/Contracts/IMailManager.php b/lib/Contracts/IMailManager.php index 34458c862c..d4ab75ee99 100644 --- a/lib/Contracts/IMailManager.php +++ b/lib/Contracts/IMailManager.php @@ -110,11 +110,11 @@ public function getImapMessage(Account $account, /** * @param Account $account - * @param int $messageId database message ID + * @param string $threadRootId thread root id * * @return Message[] */ - public function getThread(Account $account, int $messageId): array; + public function getThread(Account $account, string $threadRootId): array; /** * @param Account $sourceAccount diff --git a/lib/Controller/MessagesController.php b/lib/Controller/MessagesController.php index 62bcd1fb10..1a51b5dfaa 100755 --- a/lib/Controller/MessagesController.php +++ b/lib/Controller/MessagesController.php @@ -307,7 +307,7 @@ public function getThread(int $id): JSONResponse { return new JSONResponse([], Http::STATUS_FORBIDDEN); } - return new JSONResponse($this->mailManager->getThread($account, $id)); + return new JSONResponse($this->mailManager->getThread($account, $message->getThreadRootId())); } /** diff --git a/lib/Controller/ThreadController.php b/lib/Controller/ThreadController.php new file mode 100755 index 0000000000..c7bbfaa2be --- /dev/null +++ b/lib/Controller/ThreadController.php @@ -0,0 +1,162 @@ + + * + * Mail + * + * 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 + * + */ + +namespace OCA\Mail\Controller; + +use OCA\Mail\Contracts\IMailManager; +use OCA\Mail\Db\ThreadMapper; +use OCA\Mail\Exception\ClientException; +use OCA\Mail\Exception\ServiceException; +use OCA\Mail\Service\AccountService; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\JSONResponse; +use OCP\IRequest; +use Psr\Log\LoggerInterface; + +class ThreadController extends Controller { + + /** @var string */ + private $currentUserId; + + /** @var LoggerInterface */ + private $logger; + + /** @var AccountService */ + private $accountService; + + /** @var IMailManager */ + private $mailManager; + + /** @var ThreadMapper */ + private $threadMapper; + + public function __construct(string $appName, + IRequest $request, + string $UserId, + LoggerInterface $logger, + AccountService $accountService, + IMailManager $mailManager, + ThreadMapper $threadMapper) { + parent::__construct($appName, $request); + + $this->currentUserId = $UserId; + $this->logger = $logger; + $this->accountService = $accountService; + $this->mailManager = $mailManager; + $this->threadMapper = $threadMapper; + } + + /** + * @NoAdminRequired + * @TrapError + * + * @param int $id + * + * @return JSONResponse + * @throws ClientException + * @throws ServiceException + */ + public function delete(int $id): JSONResponse { + try { + $message = $this->mailManager->getMessage($this->currentUserId, $id); + $mailbox = $this->mailManager->getMailbox($this->currentUserId, $message->getMailboxId()); + $account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId()); + } catch (DoesNotExistException $e) { + return new JSONResponse([], Http::STATUS_FORBIDDEN); + } + + $mailAccount = $account->getMailAccount(); + $messageInTrash = $mailbox->getId() === $mailAccount->getTrashMailboxId(); + + $messages = $this->threadMapper->findMessageUidsAndMailboxNamesByAccountAndThreadRoot( + $mailAccount, + $message->getThreadRootId(), + $messageInTrash + ); + + foreach ($messages as $message) { + $this->logger->debug('deleting message', [ + 'messageId' => $message['messageUid'], + ]); + + $this->mailManager->deleteMessage( + $account, + $message['mailboxName'], + $message['messageUid'] + ); + } + + return new JSONResponse(); + } + + /** + * @NoAdminRequired + * @TrapError + * + * @param int $id + * @param int $destMailboxId + * + * @return JSONResponse + * @throws ClientException + * @throws ServiceException + */ + public function move(int $id, int $destMailboxId): JSONResponse { + try { + $message = $this->mailManager->getMessage($this->currentUserId, $id); + $srcMailbox = $this->mailManager->getMailbox($this->currentUserId, $message->getMailboxId()); + $srcAccount = $this->accountService->find($this->currentUserId, $srcMailbox->getAccountId()); + $dstMailbox = $this->mailManager->getMailbox($this->currentUserId, $destMailboxId); + $dstAccount = $this->accountService->find($this->currentUserId, $dstMailbox->getAccountId()); + } catch (DoesNotExistException $e) { + return new JSONResponse([], Http::STATUS_FORBIDDEN); + } + + $mailAccount = $srcAccount->getMailAccount(); + $messageInTrash = $srcMailbox->getId() === $mailAccount->getTrashMailboxId(); + + $messages = $this->threadMapper->findMessageUidsAndMailboxNamesByAccountAndThreadRoot( + $mailAccount, + $message->getThreadRootId(), + $messageInTrash + ); + + foreach ($messages as $message) { + $this->logger->debug('move message', [ + 'messageId' => $message['messageUid'], + 'destMailboxId' => $destMailboxId + ]); + + $this->mailManager->moveMessage( + $srcAccount, + $message['mailboxName'], + $message['messageUid'], + $dstAccount, + $dstMailbox->getName() + ); + } + + return new JSONResponse(); + } +} diff --git a/lib/Db/MessageMapper.php b/lib/Db/MessageMapper.php index ef63d3312b..a1119f2471 100644 --- a/lib/Db/MessageMapper.php +++ b/lib/Db/MessageMapper.php @@ -25,28 +25,28 @@ namespace OCA\Mail\Db; -use OCP\IUser; -use function ltrim; use OCA\Mail\Account; use OCA\Mail\Address; -use RuntimeException; -use OCP\IDBConnection; -use function array_map; -use function get_class; -use function mb_strcut; -use function array_keys; -use function array_combine; -use function array_udiff; use OCA\Mail\AddressList; +use OCA\Mail\IMAP\Threading\DatabaseMessage; use OCA\Mail\Service\Search\Flag; -use OCP\AppFramework\Db\QBMapper; -use OCP\DB\QueryBuilder\IQueryBuilder; -use OCA\Mail\Service\Search\SearchQuery; -use OCP\AppFramework\Utility\ITimeFactory; use OCA\Mail\Service\Search\FlagExpression; -use OCA\Mail\IMAP\Threading\DatabaseMessage; +use OCA\Mail\Service\Search\SearchQuery; use OCA\Mail\Support\PerformanceLogger; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\QBMapper; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use OCP\IUser; +use RuntimeException; +use function array_combine; +use function array_keys; +use function array_map; +use function array_udiff; +use function get_class; +use function ltrim; +use function mb_strcut; use function OCA\Mail\array_flat_map; /** @@ -197,6 +197,7 @@ public function findThreadingData(Account $account): array { $mailboxesQuery->select('id') ->from('mail_mailboxes') ->where($mailboxesQuery->expr()->eq('account_id', $messagesQuery->createNamedParameter($account->getId(), IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT)); + $messagesQuery->select('id', 'subject', 'message_id', 'in_reply_to', 'references', 'thread_root_id') ->from($this->getTableName()) ->where($messagesQuery->expr()->in('mailbox_id', $messagesQuery->createFunction($mailboxesQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY)) @@ -430,7 +431,7 @@ public function updateBulk(Account $account, Message ...$messages): array { return $messages; } - public function getTags(Message $message) : array { + public function getTags(Message $message): array { $mqb = $this->db->getQueryBuilder(); $mqb->select('tag_id') ->from('mail_message_tags') @@ -442,6 +443,7 @@ public function getTags(Message $message) : array { return $ids; } + /** * @param Message ...$messages * @@ -544,34 +546,22 @@ public function deleteByUid(Mailbox $mailbox, int ...$uids): void { /** * @param Account $account - * @param int $messageId + * @param string $threadRootId * * @return Message[] */ - public function findThread(Account $account, int $messageId): array { + public function findThread(Account $account, string $threadRootId): array { $qb = $this->db->getQueryBuilder(); - $subQb1 = $this->db->getQueryBuilder(); - - $mailboxIdsQuery = $subQb1 - ->select('id') - ->from('mail_mailboxes') - ->where($qb->expr()->eq('account_id', $qb->createNamedParameter($account->getId(), IQueryBuilder::PARAM_INT))); - - /** - * Select the message with the given ID or any that has the same thread ID - */ - $selectMessages = $qb - ->select('m2.*') - ->from($this->getTableName(), 'm1') - ->leftJoin('m1', $this->getTableName(), 'm2', $qb->expr()->eq('m1.thread_root_id', 'm2.thread_root_id')) + $qb->select('messages.*') + ->from($this->getTableName(), 'messages') + ->join('messages', 'mail_mailboxes', 'mailboxes', $qb->expr()->eq('messages.mailbox_id', 'mailboxes.id', IQueryBuilder::PARAM_INT)) ->where( - $qb->expr()->in('m1.mailbox_id', $qb->createFunction($mailboxIdsQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY), - $qb->expr()->in('m2.mailbox_id', $qb->createFunction($mailboxIdsQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY), - $qb->expr()->eq('m1.id', $qb->createNamedParameter($messageId, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT), - $qb->expr()->isNotNull('m1.thread_root_id') + $qb->expr()->eq('mailboxes.account_id', $qb->createNamedParameter($account->getId(), IQueryBuilder::PARAM_INT)), + $qb->expr()->eq('messages.thread_root_id', $qb->createNamedParameter($threadRootId, IQueryBuilder::PARAM_STR), IQueryBuilder::PARAM_STR) ) - ->orderBy('sent_at', 'desc'); - return $this->findRelatedData($this->findEntities($selectMessages), $account->getUserId()); + ->orderBy('messages.sent_at', 'desc'); + + return $this->findRelatedData($this->findEntities($qb), $account->getUserId()); } /** @@ -586,9 +576,9 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit $qb = $this->db->getQueryBuilder(); $select = $qb - ->selectDistinct('m.id') - ->addSelect('m.sent_at') - ->from($this->getTableName(), 'm'); + ->selectDistinct(['m.id', 'm.sent_at']) + ->from($this->getTableName(), 'm') + ->leftJoin('m', $this->getTableName(), 'm2', 'm.mailbox_id = m2.mailbox_id and m.thread_root_id = m2.thread_root_id and m.sent_at < m2.sent_at'); if (!empty($query->getFrom())) { $select->innerJoin('m', 'mail_recipients', 'r0', 'm.id = r0.message_id'); @@ -604,7 +594,7 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit } $select->where( - $qb->expr()->eq('mailbox_id', $qb->createNamedParameter($mailbox->getId()), IQueryBuilder::PARAM_INT) + $qb->expr()->eq('m.mailbox_id', $qb->createNamedParameter($mailbox->getId()), IQueryBuilder::PARAM_INT) ); if (!empty($query->getFrom())) { @@ -633,7 +623,7 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit $qb->expr()->orX( ...array_map(function (string $subject) use ($qb) { return $qb->expr()->iLike( - 'subject', + 'm.subject', $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($subject) . '%', IQueryBuilder::PARAM_STR), IQueryBuilder::PARAM_STR ); @@ -644,30 +634,31 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit if ($query->getCursor() !== null) { $select->andWhere( - $qb->expr()->lt('sent_at', $qb->createNamedParameter($query->getCursor(), IQueryBuilder::PARAM_INT)) + $qb->expr()->lt('m.sent_at', $qb->createNamedParameter($query->getCursor(), IQueryBuilder::PARAM_INT)) ); } if ($uids !== null) { $select->andWhere( - $qb->expr()->in('uid', $qb->createNamedParameter($uids, IQueryBuilder::PARAM_INT_ARRAY)) + $qb->expr()->in('m.uid', $qb->createNamedParameter($uids, IQueryBuilder::PARAM_INT_ARRAY)) ); } foreach ($query->getFlags() as $flag) { - $select->andWhere($qb->expr()->eq($this->flagToColumnName($flag), $qb->createNamedParameter($flag->isSet(), IQueryBuilder::PARAM_BOOL))); + $select->andWhere($qb->expr()->eq('m.' . $this->flagToColumnName($flag), $qb->createNamedParameter($flag->isSet(), IQueryBuilder::PARAM_BOOL))); } if (!empty($query->getFlagExpressions())) { $select->andWhere( ...array_map(function (FlagExpression $expr) use ($select) { - return $this->flagExpressionToQuery($expr, $select); + return $this->flagExpressionToQuery($expr, $select, 'm'); }, $query->getFlagExpressions()) ); } - $select = $select - ->orderBy('sent_at', 'desc'); + $select->andWhere($qb->expr()->isNull('m2.id')); + + $select->orderBy('m.sent_at', 'desc'); if ($limit !== null) { - $select = $select->setMaxResults($limit); + $select->setMaxResults($limit); } return array_map(function (Message $message) { @@ -680,9 +671,9 @@ public function findIdsGloballyByQuery(IUser $user, SearchQuery $query, ?int $li $qbMailboxes = $this->db->getQueryBuilder(); $select = $qb - ->selectDistinct('m.id') - ->addSelect('m.sent_at') - ->from($this->getTableName(), 'm'); + ->selectDistinct(['m.id', 'm.sent_at']) + ->from($this->getTableName(), 'm') + ->leftJoin('m', $this->getTableName(), 'm2', 'm.mailbox_id = m2.mailbox_id and m.thread_root_id = m2.thread_root_id and m.sent_at < m2.sent_at'); if (!empty($query->getFrom())) { $select->innerJoin('m', 'mail_recipients', 'r0', 'm.id = r0.message_id'); @@ -702,7 +693,7 @@ public function findIdsGloballyByQuery(IUser $user, SearchQuery $query, ?int $li ->join('mb', 'mail_accounts', 'a', $qb->expr()->eq('a.id', 'mb.account_id', IQueryBuilder::PARAM_INT)) ->where($qb->expr()->eq('a.user_id', $qb->createNamedParameter($user->getUID()))); $select->where( - $qb->expr()->in('mailbox_id', $qb->createFunction($selectMailboxIds->getSQL()), IQueryBuilder::PARAM_INT_ARRAY) + $qb->expr()->in('m.mailbox_id', $qb->createFunction($selectMailboxIds->getSQL()), IQueryBuilder::PARAM_INT_ARRAY) ); if (!empty($query->getFrom())) { @@ -731,7 +722,7 @@ public function findIdsGloballyByQuery(IUser $user, SearchQuery $query, ?int $li $qb->expr()->orX( ...array_map(function (string $subject) use ($qb) { return $qb->expr()->iLike( - 'subject', + 'm.subject', $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($subject) . '%', IQueryBuilder::PARAM_STR), IQueryBuilder::PARAM_STR ); @@ -742,30 +733,31 @@ public function findIdsGloballyByQuery(IUser $user, SearchQuery $query, ?int $li if ($query->getCursor() !== null) { $select->andWhere( - $qb->expr()->lt('sent_at', $qb->createNamedParameter($query->getCursor(), IQueryBuilder::PARAM_INT)) + $qb->expr()->lt('m.sent_at', $qb->createNamedParameter($query->getCursor(), IQueryBuilder::PARAM_INT)) ); } if ($uids !== null) { $select->andWhere( - $qb->expr()->in('uid', $qb->createNamedParameter($uids, IQueryBuilder::PARAM_INT_ARRAY)) + $qb->expr()->in('m.uid', $qb->createNamedParameter($uids, IQueryBuilder::PARAM_INT_ARRAY)) ); } foreach ($query->getFlags() as $flag) { - $select->andWhere($qb->expr()->eq($this->flagToColumnName($flag), $qb->createNamedParameter($flag->isSet(), IQueryBuilder::PARAM_BOOL))); + $select->andWhere($qb->expr()->eq('m.' . $this->flagToColumnName($flag), $qb->createNamedParameter($flag->isSet(), IQueryBuilder::PARAM_BOOL))); } if (!empty($query->getFlagExpressions())) { $select->andWhere( ...array_map(function (FlagExpression $expr) use ($select) { - return $this->flagExpressionToQuery($expr, $select); + return $this->flagExpressionToQuery($expr, $select, 'm'); }, $query->getFlagExpressions()) ); } - $select = $select - ->orderBy('sent_at', 'desc'); + $select->andWhere($qb->expr()->isNull('m2.id')); + + $select->orderBy('m.sent_at', 'desc'); if ($limit !== null) { - $select = $select->setMaxResults($limit); + $select->setMaxResults($limit); } return array_map(function (Message $message) { @@ -773,17 +765,17 @@ public function findIdsGloballyByQuery(IUser $user, SearchQuery $query, ?int $li }, $this->findEntities($select)); } - private function flagExpressionToQuery(FlagExpression $expr, IQueryBuilder $qb): string { - $operands = array_map(function (object $operand) use ($qb) { + private function flagExpressionToQuery(FlagExpression $expr, IQueryBuilder $qb, string $tableAlias): string { + $operands = array_map(function (object $operand) use ($qb, $tableAlias) { if ($operand instanceof Flag) { return $qb->expr()->eq( - $this->flagToColumnName($operand), + $tableAlias . '.' . $this->flagToColumnName($operand), $qb->createNamedParameter($operand->isSet(), IQueryBuilder::PARAM_BOOL), IQueryBuilder::PARAM_BOOL ); } if ($operand instanceof FlagExpression) { - return $this->flagExpressionToQuery($operand, $qb); + return $this->flagExpressionToQuery($operand, $qb, $tableAlias); } throw new RuntimeException('Invalid operand type ' . get_class($operand)); @@ -792,10 +784,10 @@ private function flagExpressionToQuery(FlagExpression $expr, IQueryBuilder $qb): switch ($expr->getOperator()) { case 'and': /** @psalm-suppress InvalidCast */ - return (string) $qb->expr()->andX(...$operands); + return (string)$qb->expr()->andX(...$operands); case 'or': /** @psalm-suppress InvalidCast */ - return (string) $qb->expr()->orX(...$operands); + return (string)$qb->expr()->orX(...$operands); default: throw new RuntimeException('Unknown operator ' . $expr->getOperator()); } @@ -924,23 +916,30 @@ public function findRelatedData(array $messages, string $userId): array { * @return int[] */ public function findNewIds(Mailbox $mailbox, array $ids): array { - $qb = $this->db->getQueryBuilder(); - $sub = $this->db->getQueryBuilder(); + $select = $this->db->getQueryBuilder(); + $subSelect = $this->db->getQueryBuilder(); - $subSelect = $sub - ->select($sub->func()->max('uid')) + $knowIdsParameter = $select->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY); + + $subSelect + ->select($subSelect->func()->min('sent_at')) ->from($this->getTableName()) ->where( - $sub->expr()->eq('mailbox_id', $qb->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)), - $sub->expr()->in('id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY), IQueryBuilder::PARAM_INT_ARRAY) + $subSelect->expr()->eq('mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)), + $subSelect->expr()->in('id', $knowIdsParameter) ); - $select = $qb - ->select('id') - ->from($this->getTableName()) + + $select + ->select('m.id') + ->from($this->getTableName(), 'm') + ->leftJoin('m', $this->getTableName(), 'm2', 'm.mailbox_id = m2.mailbox_id and m.thread_root_id = m2.thread_root_id and m.sent_at < m2.sent_at') ->where( - $qb->expr()->eq('mailbox_id', $qb->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)), - $qb->expr()->gt('uid', $qb->createFunction('(' . $subSelect->getSQL() . ')'), IQueryBuilder::PARAM_INT) - ); + $select->expr()->eq('m.mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)), + $select->expr()->notIn('m.id', $knowIdsParameter), + $select->expr()->gt('m.sent_at', $select->createFunction('(' . $subSelect->getSQL() . ')'), IQueryBuilder::PARAM_INT), + $select->expr()->isNull('m2.id') + ) + ->orderBy('m.sent_at', 'desc'); return $this->findIds($select); } @@ -1036,7 +1035,7 @@ public function getIdForUid(Mailbox $mailbox, $uid): ?int { if (empty($rows)) { return null; } - return (int) $rows[0]['id']; + return (int)$rows[0]['id']; } /** diff --git a/lib/Db/ThreadMapper.php b/lib/Db/ThreadMapper.php new file mode 100644 index 0000000000..c804cb2407 --- /dev/null +++ b/lib/Db/ThreadMapper.php @@ -0,0 +1,74 @@ + + * + * @author 2021 Daniel Kesselberg + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + */ + +namespace OCA\Mail\Db; + +use OCP\AppFramework\Db\QBMapper; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * @template-extends QBMapper + */ +class ThreadMapper extends QBMapper { + + public function __construct(IDBConnection $db) { + parent::__construct($db, 'mail_messages'); + } + + /** + * @return array + */ + public function findMessageUidsAndMailboxNamesByAccountAndThreadRoot(MailAccount $mailAccount, string $threadRootId, bool $trash): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('messages.uid', 'mailboxes.name') + ->from($this->tableName, 'messages') + ->join('messages', 'mail_mailboxes', 'mailboxes', 'messages.mailbox_id = mailboxes.id') + ->where( + $qb->expr()->eq('messages.thread_root_id', $qb->createNamedParameter($threadRootId, IQueryBuilder::PARAM_STR)), + $qb->expr()->eq('mailboxes.account_id', $qb->createNamedParameter($mailAccount->getId(), IQueryBuilder::PARAM_STR)) + ); + + $trashMailboxId = $mailAccount->getTrashMailboxId(); + if ($trashMailboxId !== null) { + if ($trash) { + $qb->andWhere($qb->expr()->eq('mailboxes.id', $qb->createNamedParameter($trashMailboxId, IQueryBuilder::PARAM_INT))); + } else { + $qb->andWhere($qb->expr()->neq('mailboxes.id', $qb->createNamedParameter($trashMailboxId, IQueryBuilder::PARAM_INT))); + } + } + + $result = $qb->execute(); + $rows = array_map(static function (array $row) { + return [ + 'messageUid' => (int)$row[0], + 'mailboxName' => (string)$row[1] + ]; + }, $result->fetchAll(\PDO::FETCH_NUM)); + $result->closeCursor(); + + return $rows; + } +} diff --git a/lib/Service/MailManager.php b/lib/Service/MailManager.php index bd87e3b131..5c733e0a5a 100644 --- a/lib/Service/MailManager.php +++ b/lib/Service/MailManager.php @@ -188,8 +188,8 @@ public function getImapMessage(Account $account, } } - public function getThread(Account $account, int $messageId): array { - return $this->dbMessageMapper->findThread($account, $messageId); + public function getThread(Account $account, string $threadRootId): array { + return $this->dbMessageMapper->findThread($account, $threadRootId); } public function getMessageIdForUid(Mailbox $mailbox, $uid): ?int { diff --git a/src/components/Envelope.vue b/src/components/Envelope.vue index 8a3deb6a0a..0e3e060a2c 100644 --- a/src/components/Envelope.vue +++ b/src/components/Envelope.vue @@ -95,7 +95,7 @@ - {{ t('mail', 'Move') }} + {{ t('mail', 'Move thread') }} - {{ t('mail', 'Delete') }} + {{ t('mail', 'Delete thread') }} @@ -325,10 +326,11 @@ export default { // Remove from selection first this.setSelected(false) // Delete - this.$emit('delete') + this.$emit('delete', this.data.databaseId) + try { - await this.$store.dispatch('deleteMessage', { - id: this.data.databaseId, + await this.$store.dispatch('deleteThread', { + envelope: this.data, }) } catch (error) { showError(await matchError(error, { diff --git a/src/components/EnvelopeList.vue b/src/components/EnvelopeList.vue index 330de250cd..c56350594f 100644 --- a/src/components/EnvelopeList.vue +++ b/src/components/EnvelopeList.vue @@ -71,8 +71,8 @@ @click.prevent="onOpenMoveModal"> {{ n( 'mail', - 'Move {number}', - 'Move {number}', + 'Move {number} thread', + 'Move {number} threads', selection.length, { number: selection.length, @@ -98,8 +98,8 @@ @click.prevent="deleteAllSelected"> {{ n( 'mail', - 'Delete {number}', - 'Delete {number}', + 'Delete {number} thread', + 'Delete {number} threads', selection.length, { number: selection.length, @@ -111,6 +111,7 @@ v-if="showMoveModal" :account="account" :envelopes="selectedEnvelopes" + :move-thread="true" @close="onCloseMoveModal" /> @@ -266,8 +267,8 @@ export default { }) this.unselectAll() }, - deleteAllSelected() { - this.selectedEnvelopes.forEach(async(envelope) => { + async deleteAllSelected() { + for (const envelope of this.selectedEnvelopes) { // Navigate if the message being deleted is the one currently viewed // Shouldn't we simply use $emit here? // Would be better to navigate after all messages have been deleted @@ -290,10 +291,10 @@ export default { }) } } - logger.info(`deleting message ${envelope.databaseId}`) + logger.info(`deleting thread ${envelope.threadRootId}`) try { - await this.$store.dispatch('deleteMessage', { - id: envelope.databaseId, + await this.$store.dispatch('deleteThread', { + envelope, }) } catch (error) { showError(await matchError(error, { @@ -306,7 +307,7 @@ export default { }, })) } - }) + } // Get new messages this.$store.dispatch('fetchNextEnvelopes', { diff --git a/src/components/Mailbox.vue b/src/components/Mailbox.vue index bb89799c31..a2c218ec9e 100644 --- a/src/components/Mailbox.vue +++ b/src/components/Mailbox.vue @@ -357,8 +357,8 @@ export default { logger.debug('deleting', { env }) this.onDelete(env.databaseId) try { - await this.$store.dispatch('deleteMessage', { - id: env.databaseId, + await this.$store.dispatch('deleteThread', { + envelope: env, }) } catch (error) { logger.error('could not delete envelope', { diff --git a/src/components/MenuEnvelope.vue b/src/components/MenuEnvelope.vue index 30428344fc..a85b404c6b 100644 --- a/src/components/MenuEnvelope.vue +++ b/src/components/MenuEnvelope.vue @@ -75,7 +75,7 @@ - {{ t('mail', 'Move') }} + {{ t('mail', 'Move message') }} - {{ t('mail', 'Delete') }} + {{ t('mail', 'Delete message') }} @@ -283,6 +283,9 @@ export default { // Delete this.$emit('delete', this.envelope.databaseId) + + logger.info(`deleting message ${this.envelope.databaseId}`) + try { await this.$store.dispatch('deleteMessage', { id: this.envelope.databaseId, diff --git a/src/components/MoveModal.vue b/src/components/MoveModal.vue index aa308f22b5..2081f09bad 100644 --- a/src/components/MoveModal.vue +++ b/src/components/MoveModal.vue @@ -2,8 +2,8 @@ @@ -26,6 +26,10 @@ export default { type: Array, required: true, }, + moveThread: { + type: Boolean, + default: false, + }, }, data() { return { @@ -41,18 +45,20 @@ export default { this.moving = true try { - const envelopeIds = this.envelopes - .filter((envelope) => envelope.mailboxId !== this.destMailboxId) - .map((envelope) => envelope.databaseId) + const envelopes = this.envelopes + .filter(envelope => envelope.mailboxId !== this.destMailboxId) - if (envelopeIds.length === 0) { + if (envelopes.length === 0) { return } - await Promise.all(envelopeIds.map((id) => this.$store.dispatch('moveMessage', { - id, - destMailboxId: this.destMailboxId, - }))) + for (const envelope of envelopes) { + if (this.moveThread) { + await this.$store.dispatch('moveThread', { envelope, destMailboxId: this.destMailboxId }) + } else { + await this.$store.dispatch('moveMessage', { id: envelope.databaseId, destMailboxId: this.destMailboxId }) + } + } await this.$store.dispatch('syncEnvelopes', { mailboxId: this.destMailboxId }) this.$emit('move') diff --git a/src/components/Thread.vue b/src/components/Thread.vue index 31f1d41b9d..690f66f60c 100644 --- a/src/components/Thread.vue +++ b/src/components/Thread.vue @@ -90,7 +90,25 @@ export default { return parseInt(this.$route.params.threadId, 10) }, thread() { - return this.$store.getters.getEnvelopeThread(this.threadId) + const envelopes = this.$store.getters.getEnvelopeThread(this.threadId) + const envelope = envelopes.find(envelope => envelope.databaseId === this.threadId) + + if (envelope === undefined) { + return [] + } + + const currentMailbox = this.$store.getters.getMailbox(envelope.mailboxId) + const trashMailbox = this.$store.getters.getMailboxes(currentMailbox.accountId).find(mailbox => mailbox.specialRole === 'trash') + + if (trashMailbox === undefined) { + return envelopes + } + + if (currentMailbox.databaseId === trashMailbox.databaseId) { + return envelopes.filter(envelope => envelope.mailboxId === trashMailbox.databaseId) + } else { + return envelopes.filter(envelope => envelope.mailboxId !== trashMailbox.databaseId) + } }, threadParticipants() { const recipients = this.thread.flatMap(envelope => { diff --git a/src/service/ThreadService.js b/src/service/ThreadService.js new file mode 100644 index 0000000000..21a9d9c43d --- /dev/null +++ b/src/service/ThreadService.js @@ -0,0 +1,27 @@ +import { generateUrl } from '@nextcloud/router' +import axios from '@nextcloud/axios' +import { convertAxiosError } from '../errors/convert' + +export async function deleteThread(id) { + const url = generateUrl('/apps/mail/api/thread/{id}', { + id, + }) + + try { + return await axios.delete(url) + } catch (e) { + throw convertAxiosError(e) + } +} + +export async function moveThread(id, destMailboxId) { + const url = generateUrl('/apps/mail/api/thread/{id}', { + id, + }) + + try { + return await axios.post(url, { destMailboxId }) + } catch (e) { + throw convertAxiosError(e) + } +} diff --git a/src/store/actions.js b/src/store/actions.js index 2c2062d9a3..26588957da 100644 --- a/src/store/actions.js +++ b/src/store/actions.js @@ -78,7 +78,8 @@ import SyncIncompleteError from '../errors/SyncIncompleteError' import MailboxLockedError from '../errors/MailboxLockedError' import { wait } from '../util/wait' import { updateAccount as updateSieveAccount } from '../service/SieveService' -import { UNIFIED_INBOX_ID, PAGE_SIZE } from './constants' +import { PAGE_SIZE, UNIFIED_INBOX_ID } from './constants' +import * as ThreadService from '../service/ThreadService' const sliceToPage = slice(0, PAGE_SIZE) @@ -770,4 +771,28 @@ export default { tagId: tag.id, }) }, + async deleteThread({ getters, commit }, { envelope }) { + commit('removeEnvelope', { id: envelope.databaseId }) + + try { + await ThreadService.deleteThread(envelope.databaseId) + console.debug('thread removed') + } catch (e) { + commit('addEnvelope', envelope) + console.error('could not delete thread', e) + throw e + } + }, + async moveThread({ getters, commit }, { envelope, destMailboxId }) { + commit('removeEnvelope', { id: envelope.databaseId }) + + try { + await ThreadService.moveThread(envelope.databaseId, destMailboxId) + console.debug('thread removed') + } catch (e) { + commit('addEnvelope', envelope) + console.error('could not move thread', e) + throw e + } + }, } diff --git a/tests/Unit/Controller/ThreadControllerTest.php b/tests/Unit/Controller/ThreadControllerTest.php new file mode 100644 index 0000000000..7629e2180b --- /dev/null +++ b/tests/Unit/Controller/ThreadControllerTest.php @@ -0,0 +1,280 @@ + + * + * Mail + * + * 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 + * + */ + +namespace OCA\Mail\Tests\Unit\Controller; + +use ChristophWurst\Nextcloud\Testing\TestCase; +use OCA\Mail\Account; +use OCA\Mail\Contracts\IMailManager; +use OCA\Mail\Controller\ThreadController; +use OCA\Mail\Db\MailAccount; +use OCA\Mail\Db\Mailbox; +use OCA\Mail\Db\Message; +use OCA\Mail\Db\ThreadMapper; +use OCA\Mail\Service\AccountService; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Http; +use OCP\IRequest; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; + +class ThreadControllerTest extends TestCase { + + /** @var string */ + private $appName; + + /** @var IRequest|MockObject */ + private $request; + + /** @var string */ + private $userId; + + /** @var MockObject|LoggerInterface */ + private $logger; + + /** @var AccountService|MockObject */ + private $accountService; + + /** @var IMailManager|MockObject */ + private $mailManager; + + /** @var ThreadMapper|MockObject */ + private $threadMapper; + + /** @var ThreadController */ + private $controller; + + protected function setUp(): void { + parent::setUp(); + + $this->appName = 'mail'; + $this->request = $this->getMockBuilder(IRequest::class)->getMock(); + $this->userId = 'john'; + $this->logger = $this->createMock(LoggerInterface::class); + $this->accountService = $this->createMock(AccountService::class); + $this->mailManager = $this->createMock(IMailManager::class); + $this->threadMapper = $this->createMock(ThreadMapper::class); + + $this->controller = new ThreadController( + $this->appName, + $this->request, + $this->userId, + $this->logger, + $this->accountService, + $this->mailManager, + $this->threadMapper + ); + } + + public function testDeleteForbidden(): void { + $this->mailManager + ->method('getMessage') + ->willThrowException(new DoesNotExistException('for some reason there is no such record')); + + $response = $this->controller->delete(100); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); + } + + public function testDeleteInbox(): void { + $mailAccount = new MailAccount(); + $mailAccount->setId(1); + $mailAccount->setTrashMailboxId(80); + + $mailbox = new Mailbox(); + $mailbox->setId(20); + $mailbox->setAccountId($mailAccount->getId()); + + $message = new Message(); + $message->setId(300); + $message->setMailboxId($mailbox->getId()); + $message->setThreadRootId('some-thread-root-id-1'); + + $this->mailManager + ->method('getMessage') + ->willReturn($message); + + $this->mailManager + ->method('getMailbox') + ->willReturn($mailbox); + + $this->accountService + ->method('find') + ->willReturn(new Account($mailAccount)); + + $this->threadMapper + ->expects(self::once()) + ->method('findMessageUidsAndMailboxNamesByAccountAndThreadRoot') + ->with($mailAccount, $message->getThreadRootId(), false) + ->willReturn([ + ['messageUid' => 200, 'mailboxName' => 'INBOX'], + ['messageUid' => 300, 'mailboxName' => 'INBOX'], + ]); + + $response = $this->controller->delete($message->getId()); + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + } + + public function testDeleteTrash(): void { + $mailAccount = new MailAccount(); + $mailAccount->setId(1); + $mailAccount->setTrashMailboxId(80); + + $mailbox = new Mailbox(); + $mailbox->setId(80); + $mailbox->setAccountId($mailAccount->getId()); + + $message = new Message(); + $message->setId(300); + $message->setMailboxId($mailbox->getId()); + $message->setThreadRootId('some-thread-root-id-1'); + + $this->mailManager + ->method('getMessage') + ->willReturn($message); + + $this->mailManager + ->method('getMailbox') + ->willReturn($mailbox); + + $this->accountService + ->method('find') + ->willReturn(new Account($mailAccount)); + + $this->threadMapper + ->expects(self::once()) + ->method('findMessageUidsAndMailboxNamesByAccountAndThreadRoot') + ->with($mailAccount, $message->getThreadRootId(), true) + ->willReturn([ + ['messageUid' => 200, 'mailboxName' => 'Trash'], + ['messageUid' => 300, 'mailboxName' => 'Trash'], + ]); + + $response = $this->controller->delete($message->getId()); + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + } + + public function testMoveForbidden() { + $this->mailManager + ->method('getMessage') + ->willThrowException(new DoesNotExistException('for some reason there is no such record')); + + $response = $this->controller->move(100, 20); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); + } + + public function testMoveInbox(): void { + $mailAccount = new MailAccount(); + $mailAccount->setId(1); + $mailAccount->setTrashMailboxId(80); + + $srcMailbox = new Mailbox(); + $srcMailbox->setId(20); + $srcMailbox->setName('INBOX'); + $srcMailbox->setAccountId($mailAccount->getId()); + + $dstMailbox = new Mailbox(); + $dstMailbox->setId(40); + $dstMailbox->setName('Archive'); + $dstMailbox->setAccountId($mailAccount->getId()); + + $message = new Message(); + $message->setId(300); + $message->setMailboxId($srcMailbox->getId()); + $message->setThreadRootId('some-thread-root-id-1'); + + $this->mailManager + ->method('getMessage') + ->willReturn($message); + + $this->mailManager + ->method('getMailbox') + ->willReturnMap([ + [$this->userId, $srcMailbox->getId(), $srcMailbox], + [$this->userId, $dstMailbox->getId(), $dstMailbox], + ]); + + $this->accountService + ->method('find') + ->willReturn(new Account($mailAccount)); + + $this->threadMapper + ->expects(self::once()) + ->method('findMessageUidsAndMailboxNamesByAccountAndThreadRoot') + ->with($mailAccount, $message->getThreadRootId(), false) + ->willReturn([ + ['messageUid' => 200, 'mailboxName' => 'INBOX'], + ['messageUid' => 300, 'mailboxName' => 'INBOX'], + ]); + + $response = $this->controller->move($message->getId(), $dstMailbox->getId()); + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + } + + public function testMoveTrash(): void { + $mailAccount = new MailAccount(); + $mailAccount->setId(1); + $mailAccount->setTrashMailboxId(80); + + $srcMailbox = new Mailbox(); + $srcMailbox->setId(80); + $srcMailbox->setName('Trash'); + $srcMailbox->setAccountId($mailAccount->getId()); + + $dstMailbox = new Mailbox(); + $dstMailbox->setId(20); + $dstMailbox->setName('Archive'); + $dstMailbox->setAccountId($mailAccount->getId()); + + $message = new Message(); + $message->setId(300); + $message->setMailboxId($srcMailbox->getId()); + $message->setThreadRootId('some-thread-root-id-1'); + + $this->mailManager + ->method('getMessage') + ->willReturn($message); + + $this->mailManager + ->method('getMailbox') + ->willReturnMap([ + [$this->userId, $srcMailbox->getId(), $srcMailbox], + [$this->userId, $dstMailbox->getId(), $dstMailbox], + ]); + + $this->accountService + ->method('find') + ->willReturn(new Account($mailAccount)); + + $this->threadMapper + ->expects(self::once()) + ->method('findMessageUidsAndMailboxNamesByAccountAndThreadRoot') + ->with($mailAccount, $message->getThreadRootId(), true) + ->willReturn([ + ['messageUid' => 200, 'mailboxName' => 'Trash'], + ['messageUid' => 300, 'mailboxName' => 'Trash'], + ]); + + $response = $this->controller->move($message->getId(), $dstMailbox->getId()); + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + } +} diff --git a/tests/Unit/Service/MailManagerTest.php b/tests/Unit/Service/MailManagerTest.php index f42b586d76..542a5fc59a 100644 --- a/tests/Unit/Service/MailManagerTest.php +++ b/tests/Unit/Service/MailManagerTest.php @@ -517,12 +517,13 @@ public function testTagNoIMAPCapabilities(): void { public function testGetThread(): void { $account = $this->createMock(Account::class); - $messageId = 123; + $threadRootId = ''; + $this->dbMessageMapper->expects($this->once()) ->method('findThread') - ->with($account, $messageId); + ->with($account, $threadRootId); - $this->manager->getThread($account, $messageId); + $this->manager->getThread($account, $threadRootId); } public function testGetMailAttachments(): void {