From c649ee3414a7914c2226588f7cfe18ca3b94e962 Mon Sep 17 00:00:00 2001 From: Thomas SharedInbox Date: Fri, 15 May 2026 17:35:36 +0200 Subject: [PATCH] fix(snooze): create Snoozed folder automatically on first use (#75) Two bugs prevented snoozing in a brand-new IMAP/JMAP account: - IMAP flush read `payload['mailboxPath']` which doesn't exist in snooze payloads (they use 'src'); selecting the wrong (null) mailbox caused the operation to fail. Now uses `payload['mailboxPath'] ?? payload['src']`. - JMAP flush had no path to create the Snoozed mailbox when the folder didn't already exist on the server. Flush now calls `Mailbox/set` to create it whenever `dest == 'Snoozed'` (the sentinel used when the folder was absent at enqueue time), then substitutes the real JMAP mailbox ID. Tests added for both code paths using a spy IMAP client and a mock JMAP HTTP client respectively. Co-Authored-By: Claude Sonnet 4.6 --- .../repositories/email_repository_impl.dart | 26 ++- test/unit/email_repository_impl_test.dart | 158 +++++++++++++++++- test/unit/fake_imap.dart | 59 +++++++ 3 files changed, 240 insertions(+), 3 deletions(-) diff --git a/lib/data/repositories/email_repository_impl.dart b/lib/data/repositories/email_repository_impl.dart index 503b7ec..19c4690 100644 --- a/lib/data/repositories/email_repository_impl.dart +++ b/lib/data/repositories/email_repository_impl.dart @@ -2047,7 +2047,8 @@ class EmailRepositoryImpl implements EmailRepository { ) async { final payload = jsonDecode(row.payload) as Map; final uid = payload['uid'] as int; - final mailboxPath = payload['mailboxPath'] as String; + // snooze/unsnooze payloads use 'src' for the source folder; all others use 'mailboxPath'. + final mailboxPath = (payload['mailboxPath'] ?? payload['src']) as String; final seq = imap.MessageSequence.fromId(uid, isUid: true); await client.selectMailboxByPath(mailboxPath); @@ -2186,8 +2187,29 @@ class EmailRepositoryImpl implements EmailRepository { final until = payload['until'] as String; final timestamp = until.replaceAll(':', '').replaceAll('-', ''); final keyword = 'snz:$timestamp'; - final destMailboxId = payload['dest'] as String; + var destMailboxId = payload['dest'] as String; final srcMailboxId = payload['src'] as String; + // When the Snoozed folder didn't exist at enqueue time, 'dest' holds + // the literal name 'Snoozed' rather than a JMAP mailbox ID. Create it. + if (destMailboxId == 'Snoozed') { + final createResps = await jmap.call([ + [ + 'Mailbox/set', + { + 'accountId': jmap.accountId, + 'create': { + 'new-snoozed': {'name': 'Snoozed', 'role': 'snoozed'}, + }, + }, + '0', + ], + ]); + final createResult = _responseArgs(createResps, 0, 'Mailbox/set'); + final created = createResult['created'] as Map?; + final newId = (created?['new-snoozed'] + as Map?)?['id'] as String?; + if (newId != null) destMailboxId = newId; + } responses = await jmap.call([ [ 'Email/set', diff --git a/test/unit/email_repository_impl_test.dart b/test/unit/email_repository_impl_test.dart index 892a8a9..c7e7e72 100644 --- a/test/unit/email_repository_impl_test.dart +++ b/test/unit/email_repository_impl_test.dart @@ -16,7 +16,7 @@ import 'package:sharedinbox/data/repositories/email_repository_impl.dart'; import 'account_repository_impl_test.dart' show MapSecureStorage; import 'db_test_helper.dart'; -import 'fake_imap.dart' show FakeImapClient; +import 'fake_imap.dart' show FakeImapClient, SnoozeSpyImapClient; // ── Helpers ─────────────────────────────────────────────────────────────────── const _account = Account( @@ -664,6 +664,48 @@ void main() { // 4+1 = 5 = _maxChangeAttempts → evicted expect(await r.db.select(r.db.pendingChanges).get(), isEmpty); }); + + test('snooze flush selects src mailbox and moves email to Snoozed', + () async { + final spy = SnoozeSpyImapClient(); + final r = _makeRepos( + imapConnect: (_, __, ___) async => spy, + ); + await r.accounts.addAccount(_account, 'pw'); + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: 'acc-1:5', + accountId: 'acc-1', + mailboxPath: 'Snoozed', + uid: 5, + receivedAt: DateTime(2024), + ), + ); + await r.db.into(r.db.pendingChanges).insert( + PendingChangesCompanion.insert( + accountId: 'acc-1', + resourceType: 'Email', + resourceId: 'acc-1:5', + changeType: 'snooze', + payload: jsonEncode({ + 'uid': 5, + 'src': 'INBOX', + 'dest': 'Snoozed', + 'until': '2026-05-10T15:00:00.000', + }), + createdAt: DateTime.now(), + ), + ); + + await r.emails.flushPendingChanges('acc-1', 'pw'); + + // Change successfully applied — removed from queue. + expect(await r.db.select(r.db.pendingChanges).get(), isEmpty); + // Source mailbox extracted from 'src', not 'mailboxPath'. + expect(spy.selectedMailbox, 'INBOX'); + expect(spy.createdMailbox, 'Snoozed'); + expect(spy.movedToMailbox, 'Snoozed'); + }); }); group('Snooze', () { @@ -1565,6 +1607,120 @@ void main() { // 4+1 = 5 = _maxChangeAttempts → evicted expect(await r.db.select(r.db.pendingChanges).get(), isEmpty); }); + + test('snooze creates Snoozed folder via Mailbox/set when dest is Snoozed', + () async { + final List> capturedBodies = []; + final client = MockClient((req) async { + if (req.url.path.contains('well-known')) { + return http.Response( + jsonEncode({ + 'apiUrl': 'https://jmap.example.com/api/', + 'accounts': { + 'acct1': {'name': 'alice@example.com', 'isPersonal': true}, + }, + 'primaryAccounts': { + 'urn:ietf:params:jmap:core': 'acct1', + 'urn:ietf:params:jmap:mail': 'acct1', + }, + 'capabilities': {}, + 'username': 'alice@example.com', + 'state': 'sess1', + }), + 200, + ); + } + final body = jsonDecode(req.body) as Map; + capturedBodies.add(body); + final calls = body['methodCalls'] as List; + final methodName = (calls.first as List)[0] as String; + if (methodName == 'Mailbox/set') { + return http.Response( + jsonEncode({ + 'sessionState': 's1', + 'methodResponses': [ + [ + 'Mailbox/set', + { + 'accountId': 'acct1', + 'created': { + 'new-snoozed': {'id': 'mbx-snoozed'}, + }, + }, + '0', + ], + ], + }), + 200, + ); + } + return http.Response( + jsonEncode({ + 'sessionState': 's1', + 'methodResponses': [ + [ + 'Email/set', + {'accountId': 'acct1', 'updated': {}}, + '0', + ], + ], + }), + 200, + ); + }); + + final r = _makeRepos(httpClient: client); + await seedChange( + r.db, + r.accounts, + changeType: 'snooze', + payload: jsonEncode({ + 'uid': 0, + 'src': 'mbx-inbox', + 'dest': 'Snoozed', + 'until': '2026-05-10T15:00:00.000', + }), + ); + + await r.emails.flushPendingChanges('jmap-1', 'pw'); + + // Change successfully applied — removed from queue. + expect(await r.db.select(r.db.pendingChanges).get(), isEmpty); + + // First API call should be Mailbox/set to create the Snoozed folder. + expect(capturedBodies, hasLength(2)); + final firstCall = + ((capturedBodies.first['methodCalls'] as List).first as List)[0]; + expect(firstCall, 'Mailbox/set'); + + // Second call should be Email/set using the newly created mailbox ID. + final secondCallArgs = ((capturedBodies[1]['methodCalls'] as List).first + as List)[1] as Map; + final update = (secondCallArgs['update'] as Map)['e1'] + as Map; + expect(update['mailboxIds/mbx-snoozed'], true); + }); + + test('snooze uses existing mailbox ID when dest is already a JMAP ID', + () async { + final r = _makeRepos(httpClient: mockFlush(200)); + await seedChange( + r.db, + r.accounts, + changeType: 'snooze', + payload: jsonEncode({ + 'uid': 0, + 'src': 'mbx-inbox', + 'dest': 'mbx-snoozed', + 'until': '2026-05-10T15:00:00.000', + }), + ); + + await r.emails.flushPendingChanges('jmap-1', 'pw'); + + // Change applied without needing Mailbox/set (dest was already a valid ID). + expect(await r.db.select(r.db.pendingChanges).get(), isEmpty); + }); }); group('JMAP syncEmails body caching', () { diff --git a/test/unit/fake_imap.dart b/test/unit/fake_imap.dart index e924fdb..0df8b84 100644 --- a/test/unit/fake_imap.dart +++ b/test/unit/fake_imap.dart @@ -17,6 +17,65 @@ class FakeImapClient extends imap.ImapClient { Future logout() async {} } +/// Spy IMAP client that records snooze-related operations and succeeds silently. +class SnoozeSpyImapClient extends FakeImapClient { + String? selectedMailbox; + String? createdMailbox; + String? movedToMailbox; + + imap.Mailbox _fakeMailbox(String path) => imap.Mailbox( + encodedName: path, + encodedPath: path, + pathSeparator: '/', + flags: [], + ); + + @override + Future selectMailboxByPath( + String path, { + bool enableCondStore = false, + imap.QResyncParameters? qresync, + }) async { + selectedMailbox = path; + return _fakeMailbox(path); + } + + @override + Future createMailbox(String path) async { + createdMailbox = path; + return _fakeMailbox(path); + } + + @override + Future uidStore( + imap.MessageSequence sequence, + List flags, { + imap.StoreAction? action, + bool? silent, + int? unchangedSinceModSequence, + }) async => + imap.StoreImapResult(); + + @override + Future uidMove( + imap.MessageSequence sequence, { + imap.Mailbox? targetMailbox, + String? targetMailboxPath, + }) async { + movedToMailbox = targetMailboxPath; + return imap.GenericImapResult(); + } + + @override + Future uidFetchMessages( + imap.MessageSequence sequence, + String? fetchContentDefinition, { + int? changedSinceModSequence, + Duration? responseTimeout, + }) async => + const imap.FetchImapResult([], null); +} + /// Minimal fake SMTP client; only `quit` is exercised by ConnectionTestService. class FakeSmtpClient extends imap.SmtpClient { FakeSmtpClient() : super('fake.host');