guettli/sharedinbox
1
0
Fork 0
Code Issues 21 Pull Requests 6 Actions Packages Projects Releases Wiki Activity

fix: restore Undo functionality for IMAP accounts by preserving data

Browse Source
This commit is contained in:
Thomas SharedInbox
2026-05-09 14:03:52 +02:00
parent 674b21298d
commit e2759ac062
15 changed files with 373 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
done.md
+15
View File
@@ -6,6 +6,21 @@ Tasks get moved from next.md to done.md
## Tasks
## Undo Feature Fix (IMAP)
Fixed a bug where undoing an email deletion or move would fail for IMAP accounts
because the local row was hard-deleted before the Undo action was performed.
- **Data Preservation**: The app now fetches and preserves the full email data in
the `UndoAction` before performing a move or delete.
- **Restoration Support**: Added `restoreEmails` to the repository to allow
re-inserting hard-deleted rows into the local database during an Undo.
- **Robust Cancellation**: Improved `UndoService` to attempt cancellation of both
`move` and `delete` pending changes, ensuring consistency even if a delete
was implemented as a move-to-trash.
- **IMAP Optimization**: Made `moveEmail` a no-op locally if the destination
mailbox matches the current one, preventing accidental re-deletion during Undo.
## Network Resilience: Exponential Backoff and Smart Retries
Improved the sync engine's reliability on intermittent connections.
lib/core/models/undo_action.dart
+6
View File
@@ -1,3 +1,5 @@
import 'package:sharedinbox/core/models/email.dart';
enum UndoType { move, delete }
class UndoAction {
@@ -8,6 +10,7 @@ class UndoAction {
required this.emailIds,
required this.sourceMailboxPath,
this.destinationMailboxPath,
this.originalEmails = const [],
});
final String id;
@@ -16,4 +19,7 @@ class UndoAction {
final List<String> emailIds;
final String sourceMailboxPath;
final String? destinationMailboxPath;
/// Full email data for restoring hard-deleted rows (e.g. IMAP move/delete).
final List<Email> originalEmails;
}
lib/core/repositories/email_repository.dart
+4
View File
@@ -70,6 +70,10 @@ abstract class EmailRepository {
/// queue. Returns true if a pending change was found and removed.
Future<bool> cancelPendingChange(String emailId, String changeType);
/// Restores previously deleted/moved emails to the local database.
/// Used for the "Undo" feature when the original rows were hard-deleted (IMAP).
Future<void> restoreEmails(List<Email> emails);
/// Emits the accountId whenever a new change is enqueued locally.
/// Used by AccountSyncManager to trigger an immediate flush.
Stream<String> get onChangesQueued;
lib/core/services/undo_service.dart
+15 -18
View File
@@ -31,33 +31,30 @@ class UndoService extends StateNotifier<UndoAction?> {
state = _history.isNotEmpty ? _history.last : null;
final repo = _ref.read(emailRepositoryProvider);
for (final id in action.emailIds) {
// Optimization: if the original change is still in the queue and hasn't
// been attempted yet, we can just remove it from the queue.
// Whether it was a move or a delete, the local state change needs
// to be reversed.
final cancelled = await repo.cancelPendingChange(
id,
action.type == UndoType.delete ? 'delete' : 'move',
);
// For IMAP, the rows were hard-deleted, so we must restore them first.
if (action.originalEmails.isNotEmpty) {
await repo.restoreEmails(action.originalEmails);
}
// Whether cancelled or not, we move the email back to its source
// to restore the local DB state and (if not cancelled) enqueue
// the reverse change on the server.
for (final id in action.emailIds) {
// Try to cancel the original change.
// Deletes might have been implemented as moves to Trash, so try both.
final cancelled = await repo.cancelPendingChange(id, 'delete') ||
await repo.cancelPendingChange(id, 'move');
// Move the email back to its source to reverse local DB state and
// (if not cancelled) enqueue the reverse change on the server.
try {
await repo.moveEmail(id, action.sourceMailboxPath);
if (cancelled) {
// If we cancelled the original change, and then moved it back,
// we've just enqueued a NEW 'move' change that is redundant
// (because the server never saw the first one).
// So we should cancel this one too!
// we've just enqueued a NEW 'move' change that is redundant.
await repo.cancelPendingChange(id, 'move');
}
} catch (e) {
// If the row is gone (hard delete), we can't undo it locally.
// TODO: Could consider re-fetching if it was a JMAP delete that
// hasn't synced yet, but that's complex.
// If it still fails, nothing more we can do locally.
}
}
}
lib/data/repositories/email_repository_impl.dart
+33
View File
@@ -1439,6 +1439,10 @@ class EmailRepositoryImpl implements EmailRepository {
.getSingle();
final account = (await _accounts.getAccount(row.accountId))!;
if (row.mailboxPath == destMailboxPath) {
return;
}
if (account.type == account_model.AccountType.jmap) {
await _enqueueChange(
account.id,
@@ -1578,6 +1582,35 @@ class EmailRepositoryImpl implements EmailRepository {
return false;
}
@override
Future<void> restoreEmails(List<model.Email> emails) async {
for (final e in emails) {
await _db.into(_db.emails).insertOnConflictUpdate(
EmailsCompanion.insert(
id: e.id,
accountId: e.accountId,
mailboxPath: e.mailboxPath,
uid: e.uid,
subject: Value(e.subject),
sentAt: Value(e.sentAt),
receivedAt: e.receivedAt,
fromJson: Value(jsonEncode(e.from)),
toAddresses: Value(jsonEncode(e.to)),
ccJson: Value(jsonEncode(e.cc)),
preview: Value(e.preview),
isSeen: Value(e.isSeen),
isFlagged: Value(e.isFlagged),
hasAttachment: Value(e.hasAttachment),
threadId: Value(e.threadId),
messageId: Value(e.messageId),
inReplyTo: Value(e.inReplyTo),
references: Value(e.references),
),
);
await _updateThread(e.accountId, e.mailboxPath, e.threadId ?? e.id);
}
}
/// Drains pending changes for [accountId] via the appropriate protocol.
/// Called at the start of each sync cycle. Returns count of applied changes.
@override
lib/ui/screens/email_detail_screen.dart
+1
View File
@@ -143,6 +143,7 @@ class _EmailDetailScreenState extends ConsumerState<EmailDetailScreen> {
type: UndoType.delete,
emailIds: [widget.emailId],
sourceMailboxPath: header.mailboxPath,
originalEmails: [header],
),
);
}
lib/ui/screens/email_list_screen.dart
+37
View File
@@ -330,6 +330,14 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
return;
}
final repo = ref.read(emailRepositoryProvider);
// Fetch full email data before moving so we can restore them if user clicks Undo.
final originalEmails = (await Future.wait(
ids.map((id) => repo.getEmail(id)),
))
.whereType<Email>()
.toList();
for (final id in ids) {
await repo.moveEmail(id, mailbox.path);
}
@@ -341,6 +349,7 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
emailIds: ids,
sourceMailboxPath: widget.mailboxPath,
destinationMailboxPath: mailbox.path,
originalEmails: originalEmails,
);
ref.read(undoServiceProvider.notifier).pushAction(action);
}
@@ -371,6 +380,15 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
if (confirmed != true) return;
_clearSelection();
final repo = ref.read(emailRepositoryProvider);
// Fetch full email data before deleting so we can restore them if user clicks Undo.
// This is especially important for IMAP where we hard-delete the row locally.
final originalEmails = (await Future.wait(
ids.map((id) => repo.getEmail(id)),
))
.whereType<Email>()
.toList();
for (final id in ids) {
await repo.deleteEmail(id);
}
@@ -381,6 +399,7 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
type: UndoType.delete,
emailIds: ids,
sourceMailboxPath: widget.mailboxPath,
originalEmails: originalEmails,
);
ref.read(undoServiceProvider.notifier).pushAction(action);
}
@@ -423,6 +442,14 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
if (chosen == null || !mounted) return;
_clearSelection();
final repo = ref.read(emailRepositoryProvider);
// Fetch full email data before moving so we can restore them if user clicks Undo.
final originalEmails = (await Future.wait(
ids.map((id) => repo.getEmail(id)),
))
.whereType<Email>()
.toList();
for (final id in ids) {
await repo.moveEmail(id, chosen);
}
@@ -434,6 +461,7 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
emailIds: ids,
sourceMailboxPath: widget.mailboxPath,
destinationMailboxPath: chosen,
originalEmails: originalEmails,
);
ref.read(undoServiceProvider.notifier).pushAction(action);
}
@@ -571,6 +599,13 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
? UndoType.move
: UndoType.delete;
// Fetch full email data before moving/deleting.
final originalEmails = (await Future.wait(
t.emailIds.map((id) => repo.getEmail(id)),
))
.whereType<Email>()
.toList();
if (direction == DismissDirection.startToEnd) {
final archive = await ref
.read(mailboxRepositoryProvider)
@@ -587,6 +622,7 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
emailIds: t.emailIds,
sourceMailboxPath: widget.mailboxPath,
destinationMailboxPath: archive.path,
originalEmails: originalEmails,
);
ref.read(undoServiceProvider.notifier).pushAction(action);
} else {
@@ -600,6 +636,7 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
type: type,
emailIds: t.emailIds,
sourceMailboxPath: widget.mailboxPath,
originalEmails: originalEmails,
);
ref.read(undoServiceProvider.notifier).pushAction(action);
}
plan.log
+9
View File
@@ -1,5 +1,14 @@
# Plan Log
## 2026-05-09
- Fixed Undo feature for IMAP accounts.
- Identified that IMAP moveEmail hard-deletes local rows, making Undo impossible without data.
- Added `originalEmails` to `UndoAction` and `restoreEmails` to `EmailRepository`.
- Updated UI to fetch email data before move/delete to support restoration.
- Fixed `UndoService` to restore rows and be more robust with pending change cancellation.
- Verified with `test/unit/undo_reproduction_test.dart` and updated unit tests.
- Successfully deployed to Android.
## 2026-05-09
- Implemented Network Resilience (Task 1/4 from next.md).
- Added exponential backoff logic (5s to 15m) to IMAP and JMAP sync loops.
test/integration/account_sync_manager_test.dart
+3
View File
@@ -134,6 +134,9 @@ class _FakeEmails implements EmailRepository {
@override
Future<bool> cancelPendingChange(String id, String type) async => false;
@override
Future<void> restoreEmails(List<Email> emails) async {}
@override
Future<void> deleteEmail(String id) async {}
test/unit/account_sync_manager_test.dart
+3
View File
@@ -57,6 +57,9 @@ class FakeEmailRepository implements EmailRepository {
@override
Future<bool> cancelPendingChange(String id, String type) async => false;
@override
Future<void> restoreEmails(List<Email> emails) async {}
@override
Future<void> deleteEmail(String id) async {}
@override
test/unit/account_sync_manager_test.mocks.dart
+11
View File
@@ -505,6 +505,17 @@ class MockEmailRepository extends _i1.Mock implements _i9.EmailRepository {
returnValue: _i4.Future<bool>.value(false),
) as _i4.Future<bool>);
@override
_i4.Future<void> restoreEmails(List<_i2.Email>? emails) =>
(super.noSuchMethod(
Invocation.method(
#restoreEmails,
[emails],
),
returnValue: _i4.Future<void>.value(),
returnValueForMissingStub: _i4.Future<void>.value(),
) as _i4.Future<void>);
@override
_i4.Stream<void> watchJmapPush(
String? accountId,
test/unit/undo_reproduction_test.dart
+184
View File
@@ -0,0 +1,184 @@
import 'package:drift/drift.dart' show Value;
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:sharedinbox/core/models/account.dart';
import 'package:sharedinbox/core/models/undo_action.dart';
import 'package:sharedinbox/data/db/database.dart' hide Account;
import 'package:sharedinbox/data/repositories/account_repository_impl.dart';
import 'package:sharedinbox/data/repositories/email_repository_impl.dart';
import 'package:sharedinbox/di.dart';
import 'account_repository_impl_test.dart' show MapSecureStorage;
import 'db_test_helper.dart';
void main() {
late AppDatabase db;
late EmailRepositoryImpl repo;
late AccountRepositoryImpl accounts;
late ProviderContainer container;
setUpAll(() {
configureSqliteForTests();
});
setUp(() async {
db = openTestDatabase();
accounts = AccountRepositoryImpl(db, MapSecureStorage());
repo = EmailRepositoryImpl(db, accounts);
container = ProviderContainer(
overrides: [
dbProvider.overrideWithValue(db),
accountRepositoryProvider.overrideWithValue(accounts),
emailRepositoryProvider.overrideWithValue(repo),
],
);
// Setup IMAP account
const account = Account(
id: 'acc1',
displayName: 'Alice',
email: 'alice@example.com',
imapHost: 'imap.example.com',
smtpHost: 'smtp.example.com',
);
await accounts.addAccount(account, 'password');
// Setup Inbox and Trash mailboxes
await db.into(db.mailboxes).insert(MailboxesCompanion.insert(
id: 'acc1:INBOX',
accountId: 'acc1',
path: 'INBOX',
name: 'Inbox',
),);
await db.into(db.mailboxes).insert(MailboxesCompanion.insert(
id: 'acc1:Trash',
accountId: 'acc1',
path: 'Trash',
name: 'Trash',
role: const Value('trash'),
),);
// Setup an email in Inbox
await db.into(db.emails).insert(EmailsCompanion.insert(
id: 'acc1:101',
accountId: 'acc1',
mailboxPath: 'INBOX',
uid: 101,
subject: const Value('Test Email'),
receivedAt: DateTime.now(),
),);
});
tearDown(() async {
await db.close();
container.dispose();
});
test('Undo deletion fails for IMAP (Reproduction)', () async {
const emailId = 'acc1:101';
// 0. Fetch BEFORE deleting
final original = await repo.getEmail(emailId);
// 1. Delete the email
await repo.deleteEmail(emailId);
// Verify it moved from INBOX (locally deleted for IMAP move)
final inInbox = await (db.select(db.emails)
..where((t) => t.id.equals(emailId))
..where((t) => t.mailboxPath.equals('INBOX')))
.get();
expect(inInbox, isEmpty, reason: 'Email should be gone from Inbox');
// 2. Push undo action and undo
final action = UndoAction(
id: 'undo1',
accountId: 'acc1',
type: UndoType.delete,
emailIds: [emailId],
sourceMailboxPath: 'INBOX',
originalEmails: [original!],
);
container.read(undoServiceProvider.notifier).pushAction(action);
await container.read(undoServiceProvider.notifier).undo();
// 3. Verify it is back in Inbox
final restored = await (db.select(db.emails)
..where((t) => t.id.equals(emailId))
..where((t) => t.mailboxPath.equals('INBOX')))
.get();
expect(restored, isNotEmpty, reason: 'Email should be restored to Inbox after undo');
});
test('Undo deletion works for JMAP', () async {
const emailId = 'jmap1:e101';
// Setup JMAP account
const jmapAccount = Account(
id: 'jmap1',
displayName: 'Alice JMAP',
email: 'alice@example.com',
type: AccountType.jmap,
jmapUrl: 'https://jmap.example.com/',
imapHost: 'imap.example.com',
smtpHost: 'smtp.example.com',
);
await accounts.addAccount(jmapAccount, 'password');
// Setup Inbox and Trash mailboxes for JMAP
await db.into(db.mailboxes).insert(MailboxesCompanion.insert(
id: 'jmap1:INBOX',
accountId: 'jmap1',
path: 'INBOX',
name: 'Inbox',
role: const Value('inbox'),
),);
await db.into(db.mailboxes).insert(MailboxesCompanion.insert(
id: 'jmap1:Trash',
accountId: 'jmap1',
path: 'Trash',
name: 'Trash',
role: const Value('trash'),
),);
// Setup an email in JMAP Inbox
await db.into(db.emails).insert(EmailsCompanion.insert(
id: emailId,
accountId: 'jmap1',
mailboxPath: 'INBOX',
uid: 0, // not used for JMAP ID
subject: const Value('JMAP Test Email'),
receivedAt: DateTime.now(),
),);
// 1. Delete the email
await repo.deleteEmail(emailId);
// Verify it moved to Trash locally (JMAP moveEmail updates mailboxPath)
final inTrash = await (db.select(db.emails)
..where((t) => t.id.equals(emailId))
..where((t) => t.mailboxPath.equals('Trash')))
.get();
expect(inTrash, isNotEmpty, reason: 'Email should be in Trash');
// 2. Push undo action and undo
const action = UndoAction(
id: 'undo2',
accountId: 'jmap1',
type: UndoType.delete,
emailIds: [emailId],
sourceMailboxPath: 'INBOX',
);
container.read(undoServiceProvider.notifier).pushAction(action);
await container.read(undoServiceProvider.notifier).undo();
// 3. Verify it is back in Inbox
final restored = await (db.select(db.emails)
..where((t) => t.id.equals(emailId))
..where((t) => t.mailboxPath.equals('INBOX')))
.get();
expect(restored, isNotEmpty, reason: 'JMAP email should be restored to Inbox after undo');
});
}
test/unit/undo_service_test.dart
+38
View File
@@ -2,6 +2,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/annotations.dart';
import 'package:mockito/mockito.dart';
import 'package:sharedinbox/core/models/email.dart';
import 'package:sharedinbox/core/models/undo_action.dart';
import 'package:sharedinbox/core/repositories/email_repository.dart';
import 'package:sharedinbox/di.dart';
@@ -99,6 +100,8 @@ void main() {
when(mockEmailRepo.moveEmail(any, any)).thenAnswer((_) async {});
when(mockEmailRepo.cancelPendingChange('e1', 'move'))
.thenAnswer((_) async => true);
when(mockEmailRepo.cancelPendingChange('e1', 'delete'))
.thenAnswer((_) async => false);
container.read(undoServiceProvider.notifier).pushAction(action);
await container.read(undoServiceProvider.notifier).undo();
@@ -109,6 +112,41 @@ void main() {
verify(mockEmailRepo.moveEmail('e1', 'INBOX')).called(1);
});
test('undo calls restoreEmails if originalEmails is provided', () async {
final email = Email(
id: 'e1',
accountId: 'acc1',
mailboxPath: 'INBOX',
uid: 101,
receivedAt: DateTime.now(),
from: [],
to: [],
cc: [],
isSeen: true,
isFlagged: false,
hasAttachment: false,
);
final action = UndoAction(
id: '1',
accountId: 'acc1',
type: UndoType.delete,
emailIds: ['e1'],
sourceMailboxPath: 'INBOX',
originalEmails: [email],
);
when(mockEmailRepo.restoreEmails(any)).thenAnswer((_) async {});
when(mockEmailRepo.moveEmail(any, any)).thenAnswer((_) async {});
when(mockEmailRepo.cancelPendingChange(any, any))
.thenAnswer((_) async => false);
container.read(undoServiceProvider.notifier).pushAction(action);
await container.read(undoServiceProvider.notifier).undo();
verify(mockEmailRepo.restoreEmails([email])).called(1);
verify(mockEmailRepo.moveEmail('e1', 'INBOX')).called(1);
});
test('undo does nothing if history is empty', () async {
await container.read(undoServiceProvider.notifier).undo();
verifyNever(mockEmailRepo.moveEmail(any, any));
test/unit/undo_service_test.mocks.dart
+11
View File
@@ -373,6 +373,17 @@ class MockEmailRepository extends _i1.Mock implements _i3.EmailRepository {
returnValue: _i4.Future<bool>.value(false),
) as _i4.Future<bool>);
@override
_i4.Future<void> restoreEmails(List<_i2.Email>? emails) =>
(super.noSuchMethod(
Invocation.method(
#restoreEmails,
[emails],
),
returnValue: _i4.Future<void>.value(),
returnValueForMissingStub: _i4.Future<void>.value(),
) as _i4.Future<void>);
@override
_i4.Stream<void> watchJmapPush(
String? accountId,
test/widget/helpers.dart
+3
View File
@@ -209,6 +209,9 @@ class FakeEmailRepository implements EmailRepository {
@override
Future<bool> cancelPendingChange(String id, String type) async => false;
@override
Future<void> restoreEmails(List<Email> emails) async {}
@override
Future<void> deleteEmail(String emailId) async {}