Skip to content

Restrict email processing to INBOX only #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Restrict email processing to INBOX only #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
694aea9
Initial plan
Copilot Nov 11, 2025
17a91c0
Implement restriction: only process emails from INBOX
Copilot Nov 11, 2025
8b507e1
Merge branch 'main' into copilot/update-email-mover-logic
HNygard Dec 30, 2025
ad51979
Update organizer/src/tests/ThreadEmailMoverTest.php
HNygard Dec 30, 2025
5e6584e
Add debug logging when skipping non-INBOX mailboxes
Copilot Dec 30, 2025
ef455e2
Merge remote-tracking branch 'origin/main' into copilot/update-email-…
HNygard Jan 26, 2026
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
9 changes: 9 additions & 0 deletions organizer/src/class/ThreadEmailMover.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ 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') {
$this->connection->logDebug("Skipping mailbox '$mailbox' - only INBOX is processed");
return array(
'unmatched' => [],
'maxed_out' => false,
);
}
Comment on lines 46 to 53
Copy link

Copilot AI Dec 30, 2025

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
Owner

@HNygard HNygard Dec 30, 2025

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
Contributor Author

Copilot AI Dec 30, 2025

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
179 changes: 23 additions & 156 deletions organizer/src/tests/ThreadEmailMoverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,169 +310,36 @@ 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. 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',
'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']);
}

/**
* Integration test to verify that manually mapped emails in thread_email_mapping are handled correctly
*
Expand Down