Merge pull request #11002 from nextcloud/bug/8849/hide-trash-widget

kesselb · web-flow · commit 2fd5134c2316 · 2025-04-29T15:48:54.000+02:00
fix: don't show important or unread emails in trash
diff --git a/lib/Contracts/IMailSearch.php b/lib/Contracts/IMailSearch.php
@@ -14,6 +14,7 @@
 use OCA\Mail\Db\Message;
 use OCA\Mail\Exception\ClientException;
 use OCA\Mail\Exception\ServiceException;
+use OCA\Mail\Service\Search\SearchQuery;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\IUser;
 
@@ -50,14 +51,12 @@ public function findMessages(Account $account,
 		?int $limit): array;
 
 	/**
-	 * @param IUser $user
-	 * @param string|null $filter
-	 * @param int|null $cursor
+	 * Run a search through all mailboxes of a user.
 	 *
 	 * @return Message[]
 	 *
 	 * @throws ClientException
 	 * @throws ServiceException
 	 */
-	public function findMessagesGlobally(IUser $user, ?string $filter, ?int $cursor, ?int $limit): array;
+	public function findMessagesGlobally(IUser $user, SearchQuery $query, ?int $limit): array;
 }
diff --git a/lib/Dashboard/ImportantMailWidget.php b/lib/Dashboard/ImportantMailWidget.php
@@ -9,6 +9,10 @@
 
 namespace OCA\Mail\Dashboard;
 
+use OCA\Mail\Service\Search\Flag;
+use OCA\Mail\Service\Search\GlobalSearchQuery;
+use OCA\Mail\Service\Search\SearchQuery;
+
 class ImportantMailWidget extends MailWidget {
 	/**
 	 * @inheritDoc
@@ -24,10 +28,10 @@ public function getTitle(): string {
 		return $this->l10n->t('Important mail');
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	public function getSearchFilter(): string {
-		return 'is:important';
+	public function getSearchQuery(string $userId): SearchQuery {
+		$query = new GlobalSearchQuery();
+		$query->addFlag(Flag::is(Flag::IMPORTANT));
+		$query->setExcludeMailboxIds($this->getMailboxIdsToExclude($userId));
+		return $query;
 	}
 }
diff --git a/lib/Dashboard/MailWidget.php b/lib/Dashboard/MailWidget.php
@@ -15,6 +15,7 @@
 use OCA\Mail\Exception\ClientException;
 use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Service\AccountService;
+use OCA\Mail\Service\Search\SearchQuery;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\Dashboard\IAPIWidget;
 use OCP\Dashboard\IAPIWidgetV2;
@@ -91,11 +92,7 @@ public function load(): void {
 		// No assets need to be loaded anymore as the widget is rendered from the API
 	}
 
-	/**
-	 * Get widget-specific search filter
-	 * @return string
-	 */
-	abstract public function getSearchFilter(): string;
+	abstract public function getSearchQuery(string $userId): SearchQuery;
 
 	/**
 	 * @param string $userId
@@ -110,16 +107,24 @@ protected function getEmails(string $userId, ?int $minTimestamp, int $limit = 7)
 		if ($user === null) {
 			return [];
 		}
-		$filter = $this->getSearchFilter();
-		$emails = $this->mailSearch->findMessagesGlobally($user, $filter, null, $limit);
 
+		$query = $this->getSearchQuery($userId);
 		if ($minTimestamp !== null) {
-			return array_filter($emails, static function (Message $email) use ($minTimestamp) {
-				return $email->getSentAt() > $minTimestamp;
-			});
+			$query->setStart((string)$minTimestamp);
+		}
+
+		return $this->mailSearch->findMessagesGlobally($user, $query, $limit);
+	}
+
+	protected function getMailboxIdsToExclude(string $userId): array {
+		$mailboxIdsToExclude = [];
+
+		foreach ($this->accountService->findByUserId($userId) as $account) {
+			$mailboxIdsToExclude[] = $account->getMailAccount()->getJunkMailboxId();
+			$mailboxIdsToExclude[] = $account->getMailAccount()->getTrashMailboxId();
 		}
 
-		return $emails;
+		return array_values(array_filter($mailboxIdsToExclude));
 	}
 
 	/**
diff --git a/lib/Dashboard/UnreadMailWidget.php b/lib/Dashboard/UnreadMailWidget.php
@@ -9,6 +9,10 @@
 
 namespace OCA\Mail\Dashboard;
 
+use OCA\Mail\Service\Search\Flag;
+use OCA\Mail\Service\Search\GlobalSearchQuery;
+use OCA\Mail\Service\Search\SearchQuery;
+
 class UnreadMailWidget extends MailWidget {
 	/**
 	 * @inheritDoc
@@ -24,10 +28,10 @@ public function getTitle(): string {
 		return $this->l10n->t('Unread mail');
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	public function getSearchFilter(): string {
-		return 'is:unread';
+	public function getSearchQuery(string $userId): SearchQuery {
+		$query = new GlobalSearchQuery();
+		$query->addFlag(Flag::not(Flag::SEEN));
+		$query->setExcludeMailboxIds($this->getMailboxIdsToExclude($userId));
+		return $query;
 	}
 }
diff --git a/lib/Db/MessageMapper.php b/lib/Db/MessageMapper.php
@@ -16,6 +16,7 @@
 use OCA\Mail\IMAP\Threading\DatabaseMessage;
 use OCA\Mail\Service\Search\Flag;
 use OCA\Mail\Service\Search\FlagExpression;
+use OCA\Mail\Service\Search\GlobalSearchQuery;
 use OCA\Mail\Service\Search\SearchQuery;
 use OCA\Mail\Support\PerformanceLogger;
 use OCA\Mail\Support\PerformanceLoggerTask;
@@ -1076,6 +1077,16 @@ public function findIdsGloballyByQuery(IUser $user, SearchQuery $query, ?int $li
 			->from('mail_mailboxes', 'mb')
 			->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())));
+
+		if ($query instanceof GlobalSearchQuery) {
+			$excludeMailboxIds = $query->getExcludeMailboxIds();
+			if (count($excludeMailboxIds) > 0) {
+				$selectMailboxIds->andWhere(
+					$qb->expr()->notIn('mb.id', $qb->createNamedParameter($excludeMailboxIds, IQueryBuilder::PARAM_INT_ARRAY))
+				);
+			}
+		}
+		
 		$select->where(
 			$qb->expr()->in('m.mailbox_id', $qb->createFunction($selectMailboxIds->getSQL()), IQueryBuilder::PARAM_INT_ARRAY)
 		);
diff --git a/lib/Search/Provider.php b/lib/Search/Provider.php
@@ -12,6 +12,7 @@
 use OCA\Mail\AppInfo\Application;
 use OCA\Mail\Contracts\IMailSearch;
 use OCA\Mail\Db\Message;
+use OCA\Mail\Service\Search\FilterStringParser;
 use OCP\IDateTimeFormatter;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -35,10 +36,13 @@ class Provider implements IProvider {
 	/** @var IURLGenerator */
 	private $urlGenerator;
 
-	public function __construct(IMailSearch $mailSearch,
+	public function __construct(
+		IMailSearch $mailSearch,
 		IL10N $l10n,
 		IDateTimeFormatter $dateTimeFormatter,
-		IURLGenerator $urlGenerator) {
+		IURLGenerator $urlGenerator,
+		private FilterStringParser $filterStringParser,
+	) {
 		$this->mailSearch = $mailSearch;
 		$this->l10n = $l10n;
 		$this->dateTimeFormatter = $dateTimeFormatter;
@@ -67,11 +71,16 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
 	}
 
 	protected function searchByFilter(IUser $user, ISearchQuery $query, string $filter): SearchResult {
+		$mailQuery = $this->filterStringParser->parse($filter);
+
 		$cursor = $query->getCursor();
+		if ($cursor !== null) {
+			$mailQuery->setCursor((int)$cursor);
+		}
+
 		$messages = $this->mailSearch->findMessagesGlobally(
 			$user,
-			$filter,
-			empty($cursor) ? null : ((int)$cursor),
+			$mailQuery,
 			$query->getLimit()
 		);
 
diff --git a/lib/Service/Search/GlobalSearchQuery.php b/lib/Service/Search/GlobalSearchQuery.php
@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\Service\Search;
+
+class GlobalSearchQuery extends SearchQuery {
+	/** @var int[] */
+	private array $excludeMailboxIds = [];
+
+	/**
+	 * @return int[]
+	 */
+	public function getExcludeMailboxIds(): array {
+		return $this->excludeMailboxIds;
+	}
+
+	/**
+	 * @param int[] $mailboxIds
+	 */
+	public function setExcludeMailboxIds(array $mailboxIds): void {
+		$this->excludeMailboxIds = $mailboxIds;
+	}
+}
diff --git a/lib/Service/Search/MailSearch.php b/lib/Service/Search/MailSearch.php
@@ -123,24 +123,16 @@ public function findMessages(Account $account,
 	}
 
 	/**
-	 * @param IUser $user
-	 * @param string|null $filter
-	 * @param int|null $cursor
+	 * Find messages across all mailboxes for a user
 	 *
 	 * @return Message[]
 	 *
-	 * @throws ClientException
 	 * @throws ServiceException
 	 */
-	public function findMessagesGlobally(IUser $user,
-		?string $filter,
-		?int $cursor,
+	public function findMessagesGlobally(
+		IUser $user,
+		SearchQuery $query,
 		?int $limit): array {
-		$query = $this->filterStringParser->parse($filter);
-		if ($cursor !== null) {
-			$query->setCursor($cursor);
-		}
-
 		return $this->messageMapper->findByIds($user->getUID(),
 			$this->getIdsGlobally($user, $query, $limit),
 			'DESC'
diff --git a/tests/Unit/Dashboard/ImportantMailWidgetTest.php b/tests/Unit/Dashboard/ImportantMailWidgetTest.php
@@ -0,0 +1,118 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Unit\Dashboard;
+
+use ChristophWurst\Nextcloud\Testing\TestCase;
+use OCA\Mail\Account;
+use OCA\Mail\Dashboard\ImportantMailWidget;
+use OCA\Mail\Db\MailAccount;
+use OCA\Mail\Db\Message;
+use OCA\Mail\Service\AccountService;
+use OCA\Mail\Service\Search\GlobalSearchQuery;
+use OCA\Mail\Service\Search\MailSearch;
+use OCA\Mail\Service\Search\SearchQuery as MailSearchQuery;
+use OCP\AppFramework\Services\IInitialState;
+use OCP\IL10N;
+use OCP\IURLGenerator;
+use OCP\IUser;
+use OCP\IUserManager;
+use PHPUnit\Framework\MockObject\MockObject;
+
+class ImportantMailWidgetTest extends TestCase {
+
+	private IL10N&MockObject $l10n;
+	private IURLGenerator&MockObject $urlGenerator;
+	private IUserManager&MockObject $userManager;
+	private AccountService&MockObject $accountService;
+	private MailSearch&MockObject $mailSearch;
+	private IInitialState&MockObject $initialState;
+	private ImportantMailWidget $widget;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->l10n = $this->createMock(IL10N::class);
+		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->userManager = $this->createMock(IUserManager::class);
+		$this->accountService = $this->createMock(AccountService::class);
+		$this->mailSearch = $this->createMock(MailSearch::class);
+		$this->initialState = $this->createMock(IInitialState::class);
+
+		$this->widget = new ImportantMailWidget(
+			$this->l10n,
+			$this->urlGenerator,
+			$this->userManager,
+			$this->accountService,
+			$this->mailSearch,
+			$this->initialState,
+			'bob'
+		);
+	}
+
+	public function testGetItems(): void {
+		$user = $this->createMock(IUser::class);
+		$this->userManager->expects($this->once())
+			->method('get')
+			->willReturn($user);
+		$message1 = new Message();
+		$message1->setSubject('Important');
+		$message1->setMailboxId(1);
+		$message2 = new Message();
+		$message2->setSubject('Also important');
+		$message2->setMailboxId(2);
+		$this->mailSearch->expects($this->once())
+			->method('findMessagesGlobally')
+			->with($user, $this->callback(function (GlobalSearchQuery $query) {
+				self::assertCount(1, $query->getFlags());
+				self::assertNull($query->getStart());
+				return true;
+			}))
+			->willReturn([$message1, $message2]);
+		$mailAccount = new MailAccount();
+		$account = new Account($mailAccount);
+		$this->accountService->expects($this->once())
+			->method('findByUserId')
+			->willReturn([$account]);
+
+		$items = $this->widget->getItems('bob', null, 7);
+
+		$this->assertCount(2, $items);
+	}
+
+	public function testGetItemsWithSince(): void {
+		$user = $this->createMock(IUser::class);
+		$this->userManager->expects($this->once())
+			->method('get')
+			->willReturn($user);
+		$message1 = new Message();
+		$message1->setSubject('Important');
+		$message1->setMailboxId(1);
+		$message2 = new Message();
+		$message2->setSubject('Also important');
+		$message2->setMailboxId(2);
+		$this->mailSearch->expects($this->once())
+			->method('findMessagesGlobally')
+			->with($user, $this->callback(function (MailSearchQuery $query) {
+				self::assertCount(1, $query->getFlags());
+				self::assertNotNull($query->getStart());
+				return true;
+			}))
+			->willReturn([$message1, $message2]);
+		$mailAccount = new MailAccount();
+		$account = new Account($mailAccount);
+		$this->accountService->expects($this->once())
+			->method('findByUserId')
+			->willReturn([$account]);
+
+		$items = $this->widget->getItems('bob', '1745340000', 7);
+
+		$this->assertCount(2, $items);
+	}
+}
diff --git a/tests/Unit/Search/FilteringProviderTest.php b/tests/Unit/Search/FilteringProviderTest.php
