fixup! fix: Junk/NotJunk flags

Signed-off-by: SebastianKrupinski <[email protected]>
nextcloud · Nov 12, 2024 · 18c755f · 18c755f
1 parent 1d5e5b4
commit 18c755f
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 27 deletions.
diff --git a/lib/Service/MailManager.php b/lib/Service/MailManager.php
@@ -48,19 +48,15 @@
 
 class MailManager implements IMailManager {
 	/**
-	 * https://tools.ietf.org/html/rfc3501#section-2.3.2
+	 * https://datatracker.ietf.org/doc/html/rfc9051#name-flags-message-attribute
 	 */
-	private const ALLOWED_FLAGS = [
+	private const SYSTEM_FLAGS = [
 		'seen' => [Horde_Imap_Client::FLAG_SEEN],
 		'answered' => [Horde_Imap_Client::FLAG_ANSWERED],
-		'forwarded' => [Horde_Imap_Client::FLAG_FORWARDED],
 		'flagged' => [Horde_Imap_Client::FLAG_FLAGGED],
 		'deleted' => [Horde_Imap_Client::FLAG_DELETED],
 		'draft' => [Horde_Imap_Client::FLAG_DRAFT],
 		'recent' => [Horde_Imap_Client::FLAG_RECENT],
-		'junk' => [Horde_Imap_Client::FLAG_JUNK],
-		'notjunk' => [Horde_Imap_Client::FLAG_NOTJUNK,],
-		'mdnsent' => [Horde_Imap_Client::FLAG_MDNSENT],
 	];
 
 	/** @var IMAPClientFactory */
@@ -768,16 +764,29 @@ public function getTagByImapLabel(string $imapLabel, string $userId): Tag {
 	 * @return array
 	 */
 	public function filterFlags(Horde_Imap_Client_Socket $client, Account $account, string $flag, string $mailbox): array {
-		// check for RFC server flags
-		if (array_key_exists($flag, self::ALLOWED_FLAGS) === true) {
-			return self::ALLOWED_FLAGS[$flag];
+		// check if flag is RFC defined system flag
+		if (array_key_exists($flag, self::SYSTEM_FLAGS) === true) {
+			return self::SYSTEM_FLAGS[$flag];
 		}
-
-		// Only allow flag setting if IMAP supports Permaflags
-		// @TODO check if there are length & char limits on permflags
-		if ($this->isPermflagsEnabled($client, $account, $mailbox) === true) {
+		// check if server supports custom keywords / this specific keyword
+		try {
+			$capabilities = $client->status($mailbox, Horde_Imap_Client::STATUS_PERMFLAGS);
+		} catch (Horde_Imap_Client_Exception $e) {
+			throw new ServiceException(
+				'Could not get message flag options from IMAP: ' . $e->getMessage(),
+				$e->getCode(),
+				$e
+			);
+		}
+		// check if server returned supported flags
+		if (!isset($capabilities['permflags'])) {
+			return [];
+		}
+		// check if server supports custom flags or specific flag
+		if (in_array("\*", $capabilities['permflags']) || in_array($flag, $capabilities['permflags'])) {
 			return [$flag];
 		}
+
 		return [];
 	}
 

diff --git a/src/store/actions.js b/src/store/actions.js
@@ -998,8 +998,8 @@ export default {
 
 			try {
 				await setEnvelopeFlags(envelope.databaseId, {
-					junk: !oldState,
-					notjunk: oldState,
+					$junk: !oldState,
+					$notjunk: oldState,
 				})
 			} catch (error) {
 				console.error('could not toggle message junk state', error)

diff --git a/tests/Unit/Service/MailManagerTest.php b/tests/Unit/Service/MailManagerTest.php
@@ -10,6 +10,7 @@
 namespace OCA\Mail\Tests\Unit\Service;
 
 use ChristophWurst\Nextcloud\Testing\TestCase;
+use Horde_Imap_Client;
 use Horde_Imap_Client_Socket;
 use OCA\Mail\Account;
 use OCA\Mail\Attachment;
@@ -326,44 +327,60 @@ public function testUnsetCustomFlagWithIMAPCapabilities(): void {
 		$this->manager->flagMessage($account, 'INBOX', 123, Tag::LABEL_IMPORTANT, false);
 	}
 
-	public function testFilterFlagStandard(): void {
+	public function testFilterFlagsWithSystemFlags(): void {
 		$account = $this->createMock(Account::class);
 		$client = $this->createMock(Horde_Imap_Client_Socket::class);
 		$flags = [
 			'seen' => [\Horde_Imap_Client::FLAG_SEEN],
 			'answered' => [\Horde_Imap_Client::FLAG_ANSWERED],
-			'forwarded' => [\Horde_Imap_Client::FLAG_FORWARDED],
 			'flagged' => [\Horde_Imap_Client::FLAG_FLAGGED],
 			'deleted' => [\Horde_Imap_Client::FLAG_DELETED],
 			'draft' => [\Horde_Imap_Client::FLAG_DRAFT],
 			'recent' => [\Horde_Imap_Client::FLAG_RECENT],
-			'junk' => [\Horde_Imap_Client::FLAG_JUNK],
-			'notjunk' => [\Horde_Imap_Client::FLAG_NOTJUNK],
-			'mdnsent' => [\Horde_Imap_Client::FLAG_MDNSENT],
 		];
 
-		//standard flags
+		// test all system flags
 		foreach ($flags as $k => $flag) {
 			$this->assertEquals($this->manager->filterFlags($client, $account, $k, 'INBOX'), $flags[$k]);
 		}
 	}
 
-	public function testSetFilterFlagsNoCapabilities() {
+	public function testFilterFlagsWithDefinedKeyword() {
 		$account = $this->createMock(Account::class);
 		$client = $this->createMock(Horde_Imap_Client_Socket::class);
 
-		$this->assertEquals([], $this->manager->filterFlags($client, $account, Tag::LABEL_IMPORTANT, 'INBOX'));
+		$client->expects($this->exactly(2))
+			->method('status')
+			->willReturn(['permflags' => ['\seen', '$junk', '$notjunk']]);
+
+		// test keyword supported
+		$this->assertEquals(['$junk'], $this->manager->filterFlags($client, $account, '$junk', 'INBOX'));
+		// test keyword unsupported
+		$this->assertEquals([], $this->manager->filterFlags($client, $account, '$autojunk', 'INBOX'));
 	}
 
-	public function testSetFilterFlagsImportant() {
+	public function testFilterFlagsWithCustomKeyword() {
 		$account = $this->createMock(Account::class);
 		$client = $this->createMock(Horde_Imap_Client_Socket::class);
 
-		$client->expects($this->once())
+		$client->expects($this->exactly(2))
 			->method('status')
-			->willReturn(['permflags' => [ '11' => "\*" ]]);
-
+			->willReturnOnConsecutiveCalls(
+				['permflags' => ['\seen', '$junk', '$notjunk', '\*']],
+				['permflags' => ['\seen', '$junk', '$notjunk']],
+			);
+
+		// test custom keyword supported
 		$this->assertEquals([Tag::LABEL_IMPORTANT], $this->manager->filterFlags($client, $account, Tag::LABEL_IMPORTANT, 'INBOX'));
+		// test custom keyword unsupported
+		$this->assertEquals([], $this->manager->filterFlags($client, $account, Tag::LABEL_IMPORTANT, 'INBOX'));
+	}
+
+	public function testFilterFlagsNoCapabilities() {
+		$account = $this->createMock(Account::class);
+		$client = $this->createMock(Horde_Imap_Client_Socket::class);
+
+		$this->assertEquals([], $this->manager->filterFlags($client, $account, Tag::LABEL_IMPORTANT, 'INBOX'));
 	}
 
 	public function testIsPermflagsEnabledTrue(): void {