From 2744cae0ef356452c88278c6b550b52ef9334ef3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 08:03:34 +0000 Subject: [PATCH 1/6] Initial plan From fc5b0d978c3cb63fc5d029559644ce6c44d43107 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 08:11:57 +0000 Subject: [PATCH 2/6] Throw SentMailboxNotSetException with proper error handling and logging Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- lib/Controller/OutboxController.php | 8 +++++ lib/Send/Chain.php | 4 +++ lib/Send/SentMailboxHandler.php | 14 +++++++-- tests/Unit/Send/ChainTest.php | 36 ++++++++++++++++++++++ tests/Unit/Send/SentMailboxHandlerTest.php | 23 +++++++++----- 5 files changed, 75 insertions(+), 10 deletions(-) diff --git a/lib/Controller/OutboxController.php b/lib/Controller/OutboxController.php index 382333dabc..1637a779db 100644 --- a/lib/Controller/OutboxController.php +++ b/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( + 'Configuration error: Cannot send message without sent mailbox.', + Http::STATUS_FORBIDDEN, + [$message] + ); + } + if ($message->getStatus() !== LocalMessage::STATUS_PROCESSED) { return JsonResponse::error('Could not send message', Http::STATUS_INTERNAL_SERVER_ERROR, [$message]); } diff --git a/lib/Send/Chain.php b/lib/Send/Chain.php index a55aeebbee..4bdc487477 100644 --- a/lib/Send/Chain.php +++ b/lib/Send/Chain.php @@ -52,6 +52,10 @@ public function process(Account $account, LocalMessage $localMessage): LocalMess $client = $this->clientFactory->getClient($account); try { $result = $handlers->process($account, $localMessage, $client); + } catch (\OCA\Mail\Exception\SentMailboxNotSetException $e) { + // Set status to indicate the specific error + $localMessage->setStatus(LocalMessage::STATUS_NO_SENT_MAILBOX); + return $this->localMessageMapper->update($localMessage); } finally { $client->logout(); } diff --git a/lib/Send/SentMailboxHandler.php b/lib/Send/SentMailboxHandler.php index c86b4fcb61..6a48800432 100644 --- a/lib/Send/SentMailboxHandler.php +++ b/lib/Send/SentMailboxHandler.php @@ -10,8 +10,15 @@ 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, @@ -19,8 +26,11 @@ public function process( 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); } diff --git a/tests/Unit/Send/ChainTest.php b/tests/Unit/Send/ChainTest.php index 681f5e823b..85ef555624 100644 --- a/tests/Unit/Send/ChainTest.php +++ b/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; @@ -128,4 +129,39 @@ public function testProcessNotProcessed() { $this->chain->process($account, $localMessage); } + + public function testProcessNoSentMailbox() { + $mailAccount = new MailAccount(); + $mailAccount->setUserId('bob'); + $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->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()); + } } diff --git a/tests/Unit/Send/SentMailboxHandlerTest.php b/tests/Unit/Send/SentMailboxHandlerTest.php index 0f29375f5d..3ead555d6f 100644 --- a/tests/Unit/Send/SentMailboxHandlerTest.php +++ b/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); } } From 54fda6767dc7d5c7f0873a76689f12ca198786bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 08:13:37 +0000 Subject: [PATCH 3/6] Add STATUS_NO_SENT_MAILBOX constant and UI error message display Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- src/components/OutboxMessageListItem.vue | 11 +++++++++-- src/store/constants.js | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/components/OutboxMessageListItem.vue b/src/components/OutboxMessageListItem.vue index e8121222fa..4af6fddfe4 100644 --- a/src/components/OutboxMessageListItem.vue +++ b/src/components/OutboxMessageListItem.vue @@ -33,7 +33,7 @@ {{ 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 }, diff --git a/src/store/constants.js b/src/store/constants.js index ee76ba8f4e..ab916a971e 100644 --- a/src/store/constants.js +++ b/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 From dcdc55e8fe3940862a445348e20969d88fa75027 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 08:14:28 +0000 Subject: [PATCH 4/6] Add proper import for SentMailboxNotSetException in Chain Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- lib/Send/Chain.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Send/Chain.php b/lib/Send/Chain.php index 4bdc487477..2c9af93638 100644 --- a/lib/Send/Chain.php +++ b/lib/Send/Chain.php @@ -10,6 +10,7 @@ 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; @@ -52,7 +53,7 @@ public function process(Account $account, LocalMessage $localMessage): LocalMess $client = $this->clientFactory->getClient($account); try { $result = $handlers->process($account, $localMessage, $client); - } catch (\OCA\Mail\Exception\SentMailboxNotSetException $e) { + } catch (SentMailboxNotSetException $e) { // Set status to indicate the specific error $localMessage->setStatus(LocalMessage::STATUS_NO_SENT_MAILBOX); return $this->localMessageMapper->update($localMessage); From 2a33214c3c895e4a522e8190e0cecd8b00d146cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 08:16:18 +0000 Subject: [PATCH 5/6] Address code review feedback: Add logging in Chain and fix capitalization Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- lib/Send/Chain.php | 6 ++++++ src/components/OutboxMessageListItem.vue | 2 +- tests/Unit/Send/ChainTest.php | 11 +++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/Send/Chain.php b/lib/Send/Chain.php index 2c9af93638..15035a01fc 100644 --- a/lib/Send/Chain.php +++ b/lib/Send/Chain.php @@ -15,6 +15,7 @@ use OCA\Mail\IMAP\IMAPClientFactory; use OCA\Mail\Service\Attachment\AttachmentService; use OCP\DB\Exception; +use Psr\Log\LoggerInterface; class Chain { public function __construct( @@ -26,6 +27,7 @@ public function __construct( private AttachmentService $attachmentService, private LocalMessageMapper $localMessageMapper, private IMAPClientFactory $clientFactory, + private LoggerInterface $logger, ) { } @@ -54,6 +56,10 @@ public function process(Account $account, LocalMessage $localMessage): LocalMess 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); diff --git a/src/components/OutboxMessageListItem.vue b/src/components/OutboxMessageListItem.vue index 4af6fddfe4..9447482811 100644 --- a/src/components/OutboxMessageListItem.vue +++ b/src/components/OutboxMessageListItem.vue @@ -117,7 +117,7 @@ export default { details() { if (this.message.status === STATUS_NO_SENT_MAILBOX) { - return this.t('mail', 'No "sent" folder configured. Please pick one in the account settings.') + 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) { diff --git a/tests/Unit/Send/ChainTest.php b/tests/Unit/Send/ChainTest.php index 85ef555624..97c702d382 100644 --- a/tests/Unit/Send/ChainTest.php +++ b/tests/Unit/Send/ChainTest.php @@ -25,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; @@ -37,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); @@ -47,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, @@ -55,6 +58,7 @@ protected function setUp(): void { $this->attachmentService, $this->localMessageMapper, $this->clientFactory, + $this->logger, ); } @@ -133,6 +137,7 @@ public function testProcessNotProcessed() { public function testProcessNoSentMailbox() { $mailAccount = new MailAccount(); $mailAccount->setUserId('bob'); + $mailAccount->setId(456); $account = new Account($mailAccount); $localMessage = new LocalMessage(); $localMessage->setId(100); @@ -150,6 +155,12 @@ public function testProcessNoSentMailbox() { ->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()) From a3a1221c2a084c1f2550e6522ebf5d23277f6d78 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 08:18:21 +0000 Subject: [PATCH 6/6] Improve OutboxController error message for better user understanding Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- lib/Controller/OutboxController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Controller/OutboxController.php b/lib/Controller/OutboxController.php index 1637a779db..104d85fe3e 100644 --- a/lib/Controller/OutboxController.php +++ b/lib/Controller/OutboxController.php @@ -246,7 +246,7 @@ public function send(int $id): JsonResponse { if ($message->getStatus() === LocalMessage::STATUS_NO_SENT_MAILBOX) { return JsonResponse::error( - 'Configuration error: Cannot send message without sent mailbox.', + 'Cannot send message: No "Sent" folder configured for this account.', Http::STATUS_FORBIDDEN, [$message] );