Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions organizer/src/class/ThreadEmailMover.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
Comment on lines +46 to +53

Copilot AI Dec 30, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strict equality check $mailbox !== 'INBOX' will prevent processing of any folder that isn't exactly 'INBOX'. This is the intended behavior based on the PR description.

However, this means functions like processSentFolder() in update-imap-functions.php that call processMailbox() with 'Sent' or 'INBOX.Sent' will now silently return empty results without any indication that they were skipped. Consider adding a debug log message here to make the skip explicit and easier to debug, for example:

$this->connection->logDebug("Skipping mailbox '$mailbox' - only INBOX is processed");

This would help operators understand the behavior when reviewing logs.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added debug logging in commit 5e6584e. The log message now reads: "Skipping mailbox '$mailbox' - only INBOX is processed"


$unmatchedAddresses = [];
$emails = $this->emailProcessor->getEmails($mailbox);

Expand Down
175 changes: 21 additions & 154 deletions organizer/src/tests/ThreadEmailMoverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
HNygard marked this conversation as resolved.
Outdated
$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']);
}
}