From 694aea99da161bbfb1837e254ec7aff29c412b90 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Nov 2025 19:57:40 +0000 Subject: [PATCH 1/4] Initial plan From 17a91c064624c9ce8a1f59c7eac4b0b160436da3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Nov 2025 20:05:26 +0000 Subject: [PATCH 2/4] Implement restriction: only process emails from INBOX - Added check in processMailbox() to only process emails from INBOX folder - Updated tests to verify non-INBOX folders are not processed - Removed obsolete tests that expected emails to move between non-INBOX folders - All 8 unit tests pass (1 integration test requires database) Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com> --- organizer/src/class/ThreadEmailMover.php | 8 + organizer/src/tests/ThreadEmailMoverTest.php | 175 +++---------------- 2 files changed, 29 insertions(+), 154 deletions(-) diff --git a/organizer/src/class/ThreadEmailMover.php b/organizer/src/class/ThreadEmailMover.php index 22eeaec6..684f9040 100644 --- a/organizer/src/class/ThreadEmailMover.php +++ b/organizer/src/class/ThreadEmailMover.php @@ -36,6 +36,14 @@ public function __construct( * @return array Array of unmatched email addresses that need new threads */ public function processMailbox(string $mailbox, array $emailToFolder): array { + // Only process emails from INBOX - never move emails from other folders + if ($mailbox !== 'INBOX') { + return array( + 'unmatched' => [], + 'maxed_out' => false, + ); + } + $unmatchedAddresses = []; $emails = $this->emailProcessor->getEmails($mailbox); diff --git a/organizer/src/tests/ThreadEmailMoverTest.php b/organizer/src/tests/ThreadEmailMoverTest.php index 5a79c0c8..cb74c338 100644 --- a/organizer/src/tests/ThreadEmailMoverTest.php +++ b/organizer/src/tests/ThreadEmailMoverTest.php @@ -303,166 +303,33 @@ public function testProcessMailboxSavesErrorForUnmatchedInboxEmail() { // Tests for non-INBOX mailboxes /** - * Data provider for non-INBOX mailbox tests - * - * @return array Test cases with [description, sourceMailbox, emailAddresses, emailToFolderMapping, expectedTargetFolder, expectedUnmatchedCount, expectedUnmatchedEmail] + * Test that non-INBOX mailboxes are not processed + * This ensures emails are never moved out of folders other than INBOX */ - public static function nonInboxMailboxProvider(): array { - return [ - 'email matching current thread stays in folder' => [ - 'INBOX.Test - Thread 1', - ['test1@example.com'], - ['test1@example.com' => 'INBOX.Test - Thread 1'], - 'INBOX.Test - Thread 1', - 0, - null - ], - 'email matching different thread moves to that thread' => [ - 'INBOX.Test - Thread 1', - ['test2@example.com'], - [ - 'test1@example.com' => 'INBOX.Test - Thread 1', - 'test2@example.com' => 'INBOX.Test - Thread 2' - ], - 'INBOX.Test - Thread 2', - 0, - null - ], - 'unmatched email moves to INBOX' => [ - 'INBOX.Test - Thread 1', - ['unmatched@example.com'], - ['test1@example.com' => 'INBOX.Test - Thread 1'], - 'INBOX', - 1, - 'unmatched@example.com' - ], + public function testProcessNonInboxMailboxDoesNothing() { + // No emails should be fetched from non-INBOX mailboxes + $this->mockEmailProcessor->expects($this->never()) + ->method('getEmails'); + + // No emails should be moved + $this->mockFolderManager->expects($this->never()) + ->method('moveEmail'); + + // Test various non-INBOX mailboxes + $nonInboxMailboxes = [ + 'INBOX.Test - Thread 1', + 'INBOX.Sent', + 'INBOX.Projects.CustomerA - Thread', ]; - } - - /** - * @dataProvider nonInboxMailboxProvider - */ - public function testProcessNonInboxMailbox( - string $sourceMailbox, - array $emailAddresses, - array $emailToFolderMapping, - string $expectedTargetFolder, - int $expectedUnmatchedCount, - ?string $expectedUnmatchedEmail - ) { - $mockEmail = $this->createMock(\Imap\ImapEmail::class); - $mockEmail->uid = 1; - $mockEmail->subject = 'Test Subject'; - $mockEmail->timestamp = time(); - $mockEmail->expects($this->once()) - ->method('getEmailAddresses') - ->willReturn($emailAddresses); - - $this->mockEmailProcessor->expects($this->once()) - ->method('getEmails') - ->with($sourceMailbox) - ->willReturn([$mockEmail]); + foreach ($nonInboxMailboxes as $mailbox) { + $result = $this->threadEmailMover->processMailbox($mailbox, []); - $this->mockFolderManager->expects($this->once()) - ->method('moveEmail') - ->with(1, $expectedTargetFolder); - - $result = $this->threadEmailMover->processMailbox($sourceMailbox, $emailToFolderMapping); - - $this->assertCount($expectedUnmatchedCount, $result['unmatched']); - if ($expectedUnmatchedEmail !== null) { - $this->assertEquals($expectedUnmatchedEmail, $result['unmatched'][0]); + // Verify empty result + $this->assertEmpty($result['unmatched'], "Non-INBOX mailbox '$mailbox' should return empty unmatched list"); + $this->assertFalse($result['maxed_out'], "Non-INBOX mailbox '$mailbox' should return maxed_out = false"); } } - public function testProcessNonInboxMailboxWithMultipleEmails() { - // Test: Multiple emails in a thread folder with different destinations - $mockEmail1 = $this->createMock(\Imap\ImapEmail::class); - $mockEmail1->uid = 1; - $mockEmail1->subject = 'Test Subject 1'; - $mockEmail1->timestamp = time(); - - $mockEmail2 = $this->createMock(\Imap\ImapEmail::class); - $mockEmail2->uid = 2; - $mockEmail2->subject = 'Test Subject 2'; - $mockEmail2->timestamp = time(); - - $mockEmail3 = $this->createMock(\Imap\ImapEmail::class); - $mockEmail3->uid = 3; - $mockEmail3->subject = 'Test Subject 3'; - $mockEmail3->timestamp = time(); - - $mockEmail1->expects($this->once()) - ->method('getEmailAddresses') - ->willReturn(['stays@example.com']); - - $mockEmail2->expects($this->once()) - ->method('getEmailAddresses') - ->willReturn(['moves@example.com']); - - $mockEmail3->expects($this->once()) - ->method('getEmailAddresses') - ->willReturn(['unmatched@example.com']); - - $this->mockEmailProcessor->expects($this->once()) - ->method('getEmails') - ->with('INBOX.Entity - Thread A') - ->willReturn([$mockEmail1, $mockEmail2, $mockEmail3]); - - // Use a callback matcher to verify the moves - $expectedMoves = [ - [1, 'INBOX.Entity - Thread A'], // stays in same folder - [2, 'INBOX.Entity - Thread B'], // moves to different thread - [3, 'INBOX'] // moves to INBOX - ]; - $moveIndex = 0; - - $this->mockFolderManager->expects($this->exactly(3)) - ->method('moveEmail') - ->willReturnCallback(function($uid, $folder) use (&$moveIndex, $expectedMoves) { - $this->assertEquals($expectedMoves[$moveIndex][0], $uid, "Move #{$moveIndex}: UID mismatch"); - $this->assertEquals($expectedMoves[$moveIndex][1], $folder, "Move #{$moveIndex}: Folder mismatch"); - $moveIndex++; - }); - - $emailToFolder = [ - 'stays@example.com' => 'INBOX.Entity - Thread A', - 'moves@example.com' => 'INBOX.Entity - Thread B' - ]; - - $result = $this->threadEmailMover->processMailbox('INBOX.Entity - Thread A', $emailToFolder); - - // Only the unmatched address should be reported - $this->assertCount(1, $result['unmatched']); - $this->assertEquals('unmatched@example.com', $result['unmatched'][0]); - } - - public function testProcessSubfolderMailbox() { - // Test: Processing emails from a subfolder like "INBOX.Projects.CustomerA - Thread" - $mockEmail = $this->createMock(\Imap\ImapEmail::class); - $mockEmail->uid = 1; - - $mockEmail->expects($this->once()) - ->method('getEmailAddresses') - ->willReturn(['customer@example.com']); - - $this->mockEmailProcessor->expects($this->once()) - ->method('getEmails') - ->with('INBOX.Projects.CustomerA - Thread') - ->willReturn([$mockEmail]); - - $this->mockFolderManager->expects($this->once()) - ->method('moveEmail') - ->with(1, 'INBOX.Projects.CustomerB - Thread'); - $emailToFolder = [ - 'customer@example.com' => 'INBOX.Projects.CustomerB - Thread' - ]; - - $result = $this->threadEmailMover->processMailbox('INBOX.Projects.CustomerA - Thread', $emailToFolder); - - // No unmatched addresses - $this->assertEmpty($result['unmatched']); - } } From ad519795cff253d3f3d79b89ef1cae74ab881b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hallvard=20Nyg=C3=A5rd?= <168380+HNygard@users.noreply.github.com> Date: Tue, 30 Dec 2025 10:34:56 +0100 Subject: [PATCH 3/4] Update organizer/src/tests/ThreadEmailMoverTest.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- organizer/src/tests/ThreadEmailMoverTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/organizer/src/tests/ThreadEmailMoverTest.php b/organizer/src/tests/ThreadEmailMoverTest.php index f66fb5ac..1771e5f9 100644 --- a/organizer/src/tests/ThreadEmailMoverTest.php +++ b/organizer/src/tests/ThreadEmailMoverTest.php @@ -322,7 +322,9 @@ public function testProcessNonInboxMailboxDoesNothing() { $this->mockFolderManager->expects($this->never()) ->method('moveEmail'); - // Test various non-INBOX mailboxes + // Test various non-INBOX mailboxes. All of these calls to processMailbox() + // are expected to trigger the early return path, so getEmails() and + // moveEmail() must never be called in any iteration of this loop. $nonInboxMailboxes = [ 'INBOX.Test - Thread 1', 'INBOX.Sent', From 5e6584ee97fd6304db6076c80d309cbc4d189d4d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Dec 2025 09:39:11 +0000 Subject: [PATCH 4/4] Add debug logging when skipping non-INBOX mailboxes Addresses review feedback to make the skip behavior explicit in logs, helping operators understand when folders are being skipped. Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com> --- organizer/src/class/ThreadEmailMover.php | 1 + 1 file changed, 1 insertion(+) diff --git a/organizer/src/class/ThreadEmailMover.php b/organizer/src/class/ThreadEmailMover.php index ec85bd22..65270cc7 100644 --- a/organizer/src/class/ThreadEmailMover.php +++ b/organizer/src/class/ThreadEmailMover.php @@ -45,6 +45,7 @@ public function __construct( public function processMailbox(string $mailbox, array $emailToFolder): array { // Only process emails from INBOX - never move emails from other folders if ($mailbox !== 'INBOX') { + $this->connection->logDebug("Skipping mailbox '$mailbox' - only INBOX is processed"); return array( 'unmatched' => [], 'maxed_out' => false,