Throw SentMailboxNotSetException when sent mailbox not configured

lib/Controller/OutboxController.php

@@ -244,6 +244,14 @@ public function send(int $id): JsonResponse {
 
 		$message = $this->service->sendMessage($message, $account);
 
+		if ($message->getStatus() === LocalMessage::STATUS_NO_SENT_MAILBOX) {
+			return JsonResponse::error(
+				'Cannot send message: No "Sent" folder configured for this account.',
+				Http::STATUS_FORBIDDEN,
+				[$message]
+			);
+		}
+
 		if ($message->getStatus() !== LocalMessage::STATUS_PROCESSED) {
 			return JsonResponse::error('Could not send message', Http::STATUS_INTERNAL_SERVER_ERROR, [$message]);
 		}

lib/Send/Chain.php

@@ -10,10 +10,12 @@
 use OCA\Mail\Account;
 use OCA\Mail\Db\LocalMessage;
 use OCA\Mail\Db\LocalMessageMapper;
+use OCA\Mail\Exception\SentMailboxNotSetException;
 use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\IMAP\IMAPClientFactory;
 use OCA\Mail\Service\Attachment\AttachmentService;
 use OCP\DB\Exception;
+use Psr\Log\LoggerInterface;
 
 class Chain {
 	public function __construct(
@@ -25,6 +27,7 @@ public function __construct(
 		private AttachmentService $attachmentService,
 		private LocalMessageMapper $localMessageMapper,
 		private IMAPClientFactory $clientFactory,
+		private LoggerInterface $logger,
 	) {
 	}
 
@@ -52,6 +55,14 @@ public function process(Account $account, LocalMessage $localMessage): LocalMess
 		$client = $this->clientFactory->getClient($account);
 		try {
 			$result = $handlers->process($account, $localMessage, $client);
+		} catch (SentMailboxNotSetException $e) {
+			$this->logger->info('Message send aborted: No sent mailbox configured', [
+				'accountId' => $account->getId(),
+				'messageId' => $localMessage->getId(),
+			]);
+			// Set status to indicate the specific error
+			$localMessage->setStatus(LocalMessage::STATUS_NO_SENT_MAILBOX);
+			return $this->localMessageMapper->update($localMessage);
 		} finally {
 			$client->logout();
 		}

lib/Send/SentMailboxHandler.php

@@ -10,17 +10,27 @@
 use Horde_Imap_Client_Socket;
 use OCA\Mail\Account;
 use OCA\Mail\Db\LocalMessage;
+use OCA\Mail\Exception\SentMailboxNotSetException;
+use Psr\Log\LoggerInterface;
 
 class SentMailboxHandler extends AHandler {
+	public function __construct(
+		private LoggerInterface $logger,
+	) {
+	}
+
 	#[\Override]
 	public function process(
 		Account $account,
 		LocalMessage $localMessage,
 		Horde_Imap_Client_Socket $client,
 	): LocalMessage {
 		if ($account->getMailAccount()->getSentMailboxId() === null) {
-			$localMessage->setStatus(LocalMessage::STATUS_NO_SENT_MAILBOX);
-			return $localMessage;
+			$this->logger->warning('No sent mailbox configured for account', [
+				'accountId' => $account->getId(),
+				'userId' => $account->getUserId(),
+			]);
+			throw new SentMailboxNotSetException();
 		}
 		return $this->processNext($account, $localMessage, $client);
 	}

src/components/OutboxMessageListItem.vue

@@ -33,7 +33,7 @@
 				</template>
 			</ActionButton>
 			<ActionButton
-				v-if="message.status !== statusImapSentMailboxFail() && message.status !== statusSmtpError()"
+				v-if="message.status !== statusImapSentMailboxFail() && message.status !== statusSmtpError() && message.status !== statusNoSentMailbox()"
 				:close-after-click="true"
 				@click="sendMessageNow">
 				{{ t('mail', 'Send now') }}
@@ -70,6 +70,7 @@ import logger from '../logger.js'
 import OutboxAvatarMixin from '../mixins/OutboxAvatarMixin.js'
 import {
 	STATUS_IMAP_SENT_MAILBOX_FAIL,
+	STATUS_NO_SENT_MAILBOX,
 	STATUS_RAW,
 	STATUS_SMTP_ERROR,
 	UNDO_DELAY,
@@ -115,7 +116,9 @@ export default {
 		},
 
 		details() {
-			if (this.message.status === STATUS_IMAP_SENT_MAILBOX_FAIL) {
+			if (this.message.status === STATUS_NO_SENT_MAILBOX) {
+				return this.t('mail', 'No "Sent" folder configured. Please pick one in the account settings.')
+			} else if (this.message.status === STATUS_IMAP_SENT_MAILBOX_FAIL) {
 				return this.t('mail', 'Could not copy to "Sent" folder')
 			} else if (this.message.status === STATUS_SMTP_ERROR) {
 				return this.t('mail', 'Mail server error')
@@ -145,6 +148,10 @@ export default {
 			return STATUS_IMAP_SENT_MAILBOX_FAIL
 		},
 
+		statusNoSentMailbox() {
+			return STATUS_NO_SENT_MAILBOX
+		},
+
 		statusSmtpError() {
 			return STATUS_SMTP_ERROR
 		},

src/store/constants.js

@@ -14,6 +14,7 @@ export const UNDO_DELAY = TOAST_UNDO_TIMEOUT
 export const EDITOR_MODE_HTML = 'richtext'
 export const EDITOR_MODE_TEXT = 'plaintext'
 export const STATUS_RAW = 0
+export const STATUS_NO_SENT_MAILBOX = 1
 export const STATUS_IMAP_SENT_MAILBOX_FAIL = 11
 export const STATUS_SMTP_ERROR = 13
 

tests/Unit/Send/ChainTest.php

@@ -15,6 +15,7 @@
 use OCA\Mail\Db\LocalMessageMapper;
 use OCA\Mail\Db\MailAccount;
 use OCA\Mail\Db\MessageMapper;
+use OCA\Mail\Exception\SentMailboxNotSetException;
 use OCA\Mail\IMAP\IMAPClientFactory;
 use OCA\Mail\Send\AntiAbuseHandler;
 use OCA\Mail\Send\Chain;
@@ -24,6 +25,7 @@
 use OCA\Mail\Send\SentMailboxHandler;
 use OCA\Mail\Service\Attachment\AttachmentService;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 
 class ChainTest extends TestCase {
 	private Chain $chain;
@@ -36,6 +38,7 @@ class ChainTest extends TestCase {
 	private AttachmentService|MockObject $attachmentService;
 	private MockObject|LocalMessageMapper $localMessageMapper;
 	private MockObject&IMAPClientFactory $clientFactory;
+	private LoggerInterface|MockObject $logger;
 
 	protected function setUp(): void {
 		$this->sentMailboxHandler = $this->createMock(SentMailboxHandler::class);
@@ -46,6 +49,7 @@ protected function setUp(): void {
 		$this->attachmentService = $this->createMock(AttachmentService::class);
 		$this->localMessageMapper = $this->createMock(LocalMessageMapper::class);
 		$this->clientFactory = $this->createMock(IMAPClientFactory::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->chain = new Chain($this->sentMailboxHandler,
 			$this->antiAbuseHandler,
 			$this->sendHandler,
@@ -54,6 +58,7 @@ protected function setUp(): void {
 			$this->attachmentService,
 			$this->localMessageMapper,
 			$this->clientFactory,
+			$this->logger,
 		);
 	}
 
@@ -128,4 +133,46 @@ public function testProcessNotProcessed() {
 
 		$this->chain->process($account, $localMessage);
 	}
+
+	public function testProcessNoSentMailbox() {
+		$mailAccount = new MailAccount();
+		$mailAccount->setUserId('bob');
+		$mailAccount->setId(456);
+		$account = new Account($mailAccount);
+		$localMessage = new LocalMessage();
+		$localMessage->setId(100);
+		$localMessage->setStatus(LocalMessage::STATUS_RAW);
+		$client = $this->createMock(Horde_Imap_Client_Socket::class);
+		$client->expects(self::once())
+			->method('logout');
+
+		$this->sentMailboxHandler->expects(self::once())
+			->method('setNext');
+		$this->clientFactory->expects(self::once())
+			->method('getClient')
+			->willReturn($client);
+		$this->sentMailboxHandler->expects(self::once())
+			->method('process')
+			->with($account, $localMessage)
+			->willThrowException(new SentMailboxNotSetException());
+		$this->logger->expects(self::once())
+			->method('info')
+			->with('Message send aborted: No sent mailbox configured', [
+				'accountId' => 456,
+				'messageId' => 100,
+			]);
+		$this->attachmentService->expects(self::never())
+			->method('deleteLocalMessageAttachments');
+		$this->localMessageMapper->expects(self::never())
+			->method('deleteWithRecipients');
+		$this->localMessageMapper->expects(self::once())
+			->method('update')
+			->with($this->callback(function ($message) {
+				return $message->getStatus() === LocalMessage::STATUS_NO_SENT_MAILBOX;
+			}))
+			->willReturnArgument(0);
+
+		$result = $this->chain->process($account, $localMessage);
+		$this->assertEquals(LocalMessage::STATUS_NO_SENT_MAILBOX, $result->getStatus());
+	}
 }

tests/Unit/Send/SentMailboxHandlerTest.php

@@ -12,17 +12,21 @@
 use OCA\Mail\Account;
 use OCA\Mail\Db\LocalMessage;
 use OCA\Mail\Db\MailAccount;
+use OCA\Mail\Exception\SentMailboxNotSetException;
 use OCA\Mail\Send\AntiAbuseHandler;
 use OCA\Mail\Send\SentMailboxHandler;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 
 class SentMailboxHandlerTest extends TestCase {
 	private AntiAbuseHandler|MockObject $antiAbuseHandler;
+	private LoggerInterface|MockObject $logger;
 	private SentMailboxHandler $handler;
 
 	protected function setUp(): void {
 		$this->antiAbuseHandler = $this->createMock(AntiAbuseHandler::class);
-		$this->handler = new SentMailboxHandler();
+		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->handler = new SentMailboxHandler($this->logger);
 		$this->handler->setNext($this->antiAbuseHandler);
 	}
 
@@ -46,17 +50,20 @@ public function testNoSentMailbox(): void {
 		$mailAccount->setUserId('bob');
 		$mailAccount->setId(123);
 		$account = new Account($mailAccount);
-		$localMessage = $this->getMockBuilder(LocalMessage::class);
-		$localMessage->addMethods(['setStatus']);
-		$mock = $localMessage->getMock();
+		$localMessage = new LocalMessage();
 		$client = $this->createMock(Horde_Imap_Client_Socket::class);
 
-		$mock->expects(self::once())
-			->method('setStatus')
-			->with(LocalMessage::STATUS_NO_SENT_MAILBOX);
+		$this->logger->expects(self::once())
+			->method('warning')
+			->with('No sent mailbox configured for account', [
+				'accountId' => 123,
+				'userId' => 'bob',
+			]);
+
 		$this->antiAbuseHandler->expects(self::never())
 			->method('process');
 
-		$this->handler->process($account, $mock, $client);
+		$this->expectException(SentMailboxNotSetException::class);
+		$this->handler->process($account, $localMessage, $client);
 	}
 }