From 132b6aeb9a220b5cf15399078676d098186a5434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bot=20of=20Thomas=20G=C3=BCttler?= Date: Thu, 14 May 2026 10:51:28 +0200 Subject: [PATCH] feat: recent searches history in SearchScreen (U3) (#47) --- .../search_history_repository.dart | 5 ++ lib/data/db/database.dart | 13 ++- .../search_history_repository_impl.dart | 57 ++++++++++++ lib/di.dart | 7 ++ lib/ui/screens/search_screen.dart | 86 +++++++++++++++++++ plan-claude.md | 12 +-- scripts/check_coverage.dart | 2 + .../unit/account_sync_manager_test.mocks.dart | 12 +-- test/unit/migration_test.dart | 15 +++- test/unit/undo_service_test.mocks.dart | 12 +-- test/widget/helpers.dart | 18 ++++ test/widget/search_screen_test.dart | 21 ++++- 12 files changed, 236 insertions(+), 24 deletions(-) create mode 100644 lib/core/repositories/search_history_repository.dart create mode 100644 lib/data/repositories/search_history_repository_impl.dart diff --git a/lib/core/repositories/search_history_repository.dart b/lib/core/repositories/search_history_repository.dart new file mode 100644 index 0000000..fbaa0ab --- /dev/null +++ b/lib/core/repositories/search_history_repository.dart @@ -0,0 +1,5 @@ +abstract interface class SearchHistoryRepository { + Future> getRecentSearches(); + Future saveSearch(String query); + Future clearHistory(); +} diff --git a/lib/data/db/database.dart b/lib/data/db/database.dart index a753091..dc18862 100644 --- a/lib/data/db/database.dart +++ b/lib/data/db/database.dart @@ -234,6 +234,13 @@ class Drafts extends Table { TextColumn get imapServerId => text().nullable()(); } +@DataClassName('SearchHistoryRow') +class SearchHistoryEntries extends Table { + IntColumn get id => integer().autoIncrement()(); + TextColumn get query => text()(); + DateTimeColumn get searchedAt => dateTime()(); +} + @DataClassName('UndoActionRow') class UndoActions extends Table { TextColumn get id => text()(); @@ -263,13 +270,14 @@ class UndoActions extends Table { SyncLogMailboxes, SyncHealth, UndoActions, + SearchHistoryEntries, ], ) class AppDatabase extends _$AppDatabase { AppDatabase([QueryExecutor? executor]) : super(executor ?? _openConnection()); @override - int get schemaVersion => 26; + int get schemaVersion => 27; Future _createEmailFts() async { await customStatement(''' @@ -492,6 +500,9 @@ class AppDatabase extends _$AppDatabase { SELECT rowid, subject, preview, from_json FROM emails '''); } + if (from < 27) { + await m.createTable(searchHistoryEntries); + } }, ); } diff --git a/lib/data/repositories/search_history_repository_impl.dart b/lib/data/repositories/search_history_repository_impl.dart new file mode 100644 index 0000000..ef81140 --- /dev/null +++ b/lib/data/repositories/search_history_repository_impl.dart @@ -0,0 +1,57 @@ +import 'package:drift/drift.dart'; +import 'package:sharedinbox/core/repositories/search_history_repository.dart'; +import 'package:sharedinbox/data/db/database.dart'; + +class SearchHistoryRepositoryImpl implements SearchHistoryRepository { + SearchHistoryRepositoryImpl(this._db); + final AppDatabase _db; + + static const _maxEntries = 10; + + @override + Future> getRecentSearches() async { + final rows = await (_db.select(_db.searchHistoryEntries) + ..orderBy([(t) => OrderingTerm.desc(t.searchedAt)]) + ..limit(_maxEntries)) + .get(); + return rows.map((r) => r.query).toList(); + } + + @override + Future saveSearch(String query) async { + final trimmed = query.trim(); + if (trimmed.isEmpty) return; + + await _db.transaction(() async { + // Remove existing entry for same query (deduplication). + await (_db.delete(_db.searchHistoryEntries) + ..where((t) => t.query.equals(trimmed))) + .go(); + + await _db.into(_db.searchHistoryEntries).insert( + SearchHistoryEntriesCompanion.insert( + query: trimmed, + searchedAt: DateTime.now(), + ), + ); + + // Prune to the most recent _maxEntries. + final keepIds = await (_db.select(_db.searchHistoryEntries) + ..orderBy([(t) => OrderingTerm.desc(t.searchedAt)]) + ..limit(_maxEntries)) + .map((r) => r.id) + .get(); + + if (keepIds.isNotEmpty) { + await (_db.delete(_db.searchHistoryEntries) + ..where((t) => t.id.isNotIn(keepIds))) + .go(); + } + }); + } + + @override + Future clearHistory() async { + await _db.delete(_db.searchHistoryEntries).go(); + } +} diff --git a/lib/di.dart b/lib/di.dart index c7f970e..e798f96 100644 --- a/lib/di.dart +++ b/lib/di.dart @@ -9,6 +9,7 @@ import 'package:sharedinbox/core/repositories/account_repository.dart'; import 'package:sharedinbox/core/repositories/draft_repository.dart'; import 'package:sharedinbox/core/repositories/email_repository.dart'; import 'package:sharedinbox/core/repositories/mailbox_repository.dart'; +import 'package:sharedinbox/core/repositories/search_history_repository.dart'; import 'package:sharedinbox/core/repositories/undo_repository.dart'; import 'package:sharedinbox/core/services/account_discovery_service.dart'; import 'package:sharedinbox/core/services/connection_test_service.dart'; @@ -25,6 +26,7 @@ import 'package:sharedinbox/data/repositories/account_repository_impl.dart'; import 'package:sharedinbox/data/repositories/draft_repository_impl.dart'; import 'package:sharedinbox/data/repositories/email_repository_impl.dart'; import 'package:sharedinbox/data/repositories/mailbox_repository_impl.dart'; +import 'package:sharedinbox/data/repositories/search_history_repository_impl.dart'; import 'package:sharedinbox/data/repositories/sync_log_repository_impl.dart'; import 'package:sharedinbox/data/repositories/undo_repository_impl.dart'; import 'package:sharedinbox/data/storage/flutter_secure_storage_impl.dart'; @@ -87,6 +89,11 @@ final undoRepositoryProvider = Provider((ref) { return UndoRepositoryImpl(ref.watch(dbProvider)); }); +final searchHistoryRepositoryProvider = + Provider((ref) { + return SearchHistoryRepositoryImpl(ref.watch(dbProvider)); +}); + final syncLogRepositoryProvider = Provider((ref) { return SyncLogRepositoryImpl(ref.watch(dbProvider)); }); diff --git a/lib/ui/screens/search_screen.dart b/lib/ui/screens/search_screen.dart index 00f9d7e..9563fde 100644 --- a/lib/ui/screens/search_screen.dart +++ b/lib/ui/screens/search_screen.dart @@ -10,6 +10,11 @@ import 'package:sharedinbox/core/utils/logger.dart'; import 'package:sharedinbox/di.dart'; import 'package:sharedinbox/ui/widgets/email_tile.dart'; +final _searchHistoryProvider = + FutureProvider.autoDispose>((ref) async { + return ref.watch(searchHistoryRepositoryProvider).getRecentSearches(); +}); + class SearchScreen extends ConsumerStatefulWidget { const SearchScreen({super.key, this.accountId}); final String? accountId; @@ -20,13 +25,24 @@ class SearchScreen extends ConsumerStatefulWidget { class _SearchScreenState extends ConsumerState { final _ctrl = TextEditingController(); + final _focusNode = FocusNode(); Timer? _debounce; _SearchResults? _results; bool _loading = false; + bool _fieldFocused = false; + + @override + void initState() { + super.initState(); + _focusNode.addListener(() { + if (mounted) setState(() => _fieldFocused = _focusNode.hasFocus); + }); + } @override void dispose() { _ctrl.dispose(); + _focusNode.dispose(); _debounce?.cancel(); super.dispose(); } @@ -45,6 +61,12 @@ class _SearchScreenState extends ConsumerState { Future _search(String query) async { setState(() => _loading = true); + unawaited( + ref + .read(searchHistoryRepositoryProvider) + .saveSearch(query) + .then((_) => ref.invalidate(_searchHistoryProvider)), + ); try { final emailRepo = ref.read(emailRepositoryProvider); final mailboxRepo = ref.read(mailboxRepositoryProvider); @@ -112,6 +134,7 @@ class _SearchScreenState extends ConsumerState { appBar: AppBar( title: TextField( controller: _ctrl, + focusNode: _focusNode, autofocus: true, decoration: const InputDecoration( hintText: 'Search folders, addresses, emails…', @@ -137,6 +160,9 @@ class _SearchScreenState extends ConsumerState { Widget _buildBody() { if (_loading) return const Center(child: CircularProgressIndicator()); if (_results == null) { + if (_fieldFocused && _ctrl.text.isEmpty) { + return _buildHistoryPanel(); + } return const Center(child: Text('Type 3+ characters to search')); } final r = _results!; @@ -169,6 +195,66 @@ class _SearchScreenState extends ConsumerState { ], ); } + + Widget _buildHistoryPanel() { + final history = ref.watch(_searchHistoryProvider); + return history.when( + loading: () => const Center(child: Text('Type 3+ characters to search')), + error: (_, __) => + const Center(child: Text('Type 3+ characters to search')), + data: (terms) { + if (terms.isEmpty) { + return const Center(child: Text('Type 3+ characters to search')); + } + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Padding( + padding: const EdgeInsets.fromLTRB(16, 12, 16, 4), + child: Row( + mainAxisAlignment: MainAxisAlignment.spaceBetween, + children: [ + Text( + 'Recent searches', + style: Theme.of(context).textTheme.labelLarge, + ), + TextButton( + onPressed: () async { + await ref + .read(searchHistoryRepositoryProvider) + .clearHistory(); + ref.invalidate(_searchHistoryProvider); + }, + child: const Text('Clear'), + ), + ], + ), + ), + Padding( + padding: const EdgeInsets.symmetric(horizontal: 12), + child: Wrap( + spacing: 8, + runSpacing: 4, + children: [ + for (final term in terms) + ActionChip( + label: Text(term), + onPressed: () { + _ctrl.text = term; + _ctrl.selection = TextSelection.fromPosition( + TextPosition(offset: term.length), + ); + unawaited(_search(term)); + }, + ), + ], + ), + ), + ], + ); + }, + ); + } } class _SearchResults { diff --git a/plan-claude.md b/plan-claude.md index 387cf99..b6674bd 100644 --- a/plan-claude.md +++ b/plan-claude.md @@ -43,10 +43,7 @@ Files: `lib/ui/screens/email_list_screen.dart`, `lib/core/utils/format_utils.dar ### R4 — Done: https://codeberg.org/guettli/sharedinbox/pulls/23 -### R5 🟡 Handle TLS certificate changes gracefully -`tls_error.dart` detects TLS errors but they bubble up as generic errors in the sync loop. -Detect `TlsError` specifically in `_AccountSync` and show a user-facing dialog offering to re-add the account or trust the new certificate. -Files: `lib/data/imap/tls_error.dart`, `lib/core/sync/account_sync_manager.dart`. +### R5 — Done: https://codeberg.org/guettli/sharedinbox/pulls/45 ### R6 — Done: https://codeberg.org/guettli/sharedinbox/pulls/24 @@ -107,6 +104,8 @@ Files: `lib/ui/screens/email_list_screen.dart`, `lib/core/repositories/email_rep ### T2 — Done: https://codeberg.org/guettli/sharedinbox/pulls/31 +### T3 — Done: https://codeberg.org/guettli/sharedinbox/pulls/43 + ### T3 🟡 Contract tests for all Repository interfaces The interfaces in `core/repositories/` have no shared contract test suite. Concrete impls can silently diverge. Add a shared `EmailRepositoryContract` abstract test class; run it against both `EmailRepositoryImpl` and any future mock/fake. Mirror this for `MailboxRepository` and `AccountRepository`. @@ -127,10 +126,7 @@ Files: `test/widget/email_list_screen_test.dart`. ### A2 — Done: https://codeberg.org/guettli/sharedinbox/pulls/33 -### A3 🟡 Make AccountSyncManager testable without real IMAP connections -`AccountSyncManager` accepts `ImapConnectFn` as a dependency but `_JmapAccountSync` constructs its HTTP client internally. -Pass an injectable `http.Client` to `_JmapAccountSync` (already done in `EmailRepositoryImpl`; mirror the pattern here). -Files: `lib/core/sync/account_sync_manager.dart`, `test/unit/account_sync_manager_test.dart`. +### A3 — Done: https://codeberg.org/guettli/sharedinbox/pulls/46 ### A4 🟡 Replace raw JSON strings in DB with structured encoding `fromJson`, `toAddresses`, `ccJson`, `references` are stored as raw JSON strings parsed on every model conversion. diff --git a/scripts/check_coverage.dart b/scripts/check_coverage.dart index 23a66dc..37f1e11 100644 --- a/scripts/check_coverage.dart +++ b/scripts/check_coverage.dart @@ -17,6 +17,7 @@ const _noCode = { 'lib/core/repositories/mailbox_repository.dart', 'lib/core/repositories/sync_log_repository.dart', 'lib/core/repositories/undo_repository.dart', + 'lib/core/repositories/search_history_repository.dart', 'lib/core/models/undo_action.dart', 'lib/core/storage/secure_storage.dart', }; @@ -61,6 +62,7 @@ const _excluded = { 'lib/data/repositories/mailbox_repository_impl.dart', 'lib/data/repositories/sync_log_repository_impl.dart', 'lib/data/repositories/undo_repository_impl.dart', + 'lib/data/repositories/search_history_repository_impl.dart', }; void main() { diff --git a/test/unit/account_sync_manager_test.mocks.dart b/test/unit/account_sync_manager_test.mocks.dart index 656a981..aef5e12 100644 --- a/test/unit/account_sync_manager_test.mocks.dart +++ b/test/unit/account_sync_manager_test.mocks.dart @@ -215,9 +215,9 @@ class MockEmailRepository extends _i1.Mock implements _i9.EmailRepository { @override _i4.Stream> observeEmails( - String accountId, - String mailboxPath, { - int limit = 50, + String? accountId, + String? mailboxPath, { + int? limit = 50, }) => (super.noSuchMethod( Invocation.method( @@ -233,9 +233,9 @@ class MockEmailRepository extends _i1.Mock implements _i9.EmailRepository { @override _i4.Stream> observeThreads( - String accountId, - String mailboxPath, { - int limit = 50, + String? accountId, + String? mailboxPath, { + int? limit = 50, }) => (super.noSuchMethod( Invocation.method( diff --git a/test/unit/migration_test.dart b/test/unit/migration_test.dart index 1b1c706..60af206 100644 --- a/test/unit/migration_test.dart +++ b/test/unit/migration_test.dart @@ -14,7 +14,7 @@ void main() { group('Migration', () { test('schemaVersion matches expected value', () async { final db = AppDatabase(NativeDatabase.memory()); - expect(db.schemaVersion, 26); + expect(db.schemaVersion, 27); await db.close(); }); @@ -171,6 +171,11 @@ void main() { // Verify FTS table was created and is queryable. await db.customSelect('SELECT count(*) FROM email_fts').get(); + // v27: search_history_entries table. + await db + .customSelect('SELECT count(*) FROM search_history_entries') + .get(); + await db.close(); if (dbFile.existsSync()) dbFile.deleteSync(); }); @@ -301,11 +306,16 @@ void main() { ); await db.customSelect('SELECT count(*) FROM email_fts').get(); + // v27: search_history_entries table. + await db + .customSelect('SELECT count(*) FROM search_history_entries') + .get(); + await db.close(); if (dbFile.existsSync()) dbFile.deleteSync(); }); - test('fresh install creates all tables at schemaVersion 26', () async { + test('fresh install creates all tables at schemaVersion 27', () async { final db = AppDatabase(NativeDatabase.memory()); await db.select(db.accounts).get(); @@ -328,6 +338,7 @@ void main() { 'threads', 'sync_health', 'undo_actions', + 'search_history_entries', ]), ); diff --git a/test/unit/undo_service_test.mocks.dart b/test/unit/undo_service_test.mocks.dart index a950bc3..d7fbd6a 100644 --- a/test/unit/undo_service_test.mocks.dart +++ b/test/unit/undo_service_test.mocks.dart @@ -75,9 +75,9 @@ class MockEmailRepository extends _i1.Mock implements _i3.EmailRepository { @override _i4.Stream> observeEmails( - String accountId, - String mailboxPath, { - int limit = 50, + String? accountId, + String? mailboxPath, { + int? limit = 50, }) => (super.noSuchMethod( Invocation.method( @@ -93,9 +93,9 @@ class MockEmailRepository extends _i1.Mock implements _i3.EmailRepository { @override _i4.Stream> observeThreads( - String accountId, - String mailboxPath, { - int limit = 50, + String? accountId, + String? mailboxPath, { + int? limit = 50, }) => (super.noSuchMethod( Invocation.method( diff --git a/test/widget/helpers.dart b/test/widget/helpers.dart index a1213f2..2b050b3 100644 --- a/test/widget/helpers.dart +++ b/test/widget/helpers.dart @@ -17,6 +17,7 @@ import 'package:sharedinbox/core/repositories/account_repository.dart'; import 'package:sharedinbox/core/repositories/draft_repository.dart'; import 'package:sharedinbox/core/repositories/email_repository.dart'; import 'package:sharedinbox/core/repositories/mailbox_repository.dart'; +import 'package:sharedinbox/core/repositories/search_history_repository.dart'; import 'package:sharedinbox/core/services/account_discovery_service.dart'; import 'package:sharedinbox/core/services/connection_test_service.dart'; import 'package:sharedinbox/core/services/managesieve_probe_service.dart'; @@ -508,3 +509,20 @@ Email testEmail({ isFlagged: isFlagged, hasAttachment: hasAttachment, ); + +class FakeSearchHistoryRepository implements SearchHistoryRepository { + final List _history = []; + + @override + Future> getRecentSearches() async => List.unmodifiable(_history); + + @override + Future saveSearch(String query) async { + _history.remove(query); + _history.insert(0, query); + if (_history.length > 10) _history.removeLast(); + } + + @override + Future clearHistory() async => _history.clear(); +} diff --git a/test/widget/search_screen_test.dart b/test/widget/search_screen_test.dart index 281a944..5bf93db 100644 --- a/test/widget/search_screen_test.dart +++ b/test/widget/search_screen_test.dart @@ -20,6 +20,9 @@ void main() { FakeMailboxRepository(), ), emailRepositoryProvider.overrideWithValue(FakeEmailRepository()), + searchHistoryRepositoryProvider.overrideWithValue( + FakeSearchHistoryRepository(), + ), ], ), ); @@ -42,6 +45,9 @@ void main() { FakeMailboxRepository(), ), emailRepositoryProvider.overrideWithValue(FakeEmailRepository()), + searchHistoryRepositoryProvider.overrideWithValue( + FakeSearchHistoryRepository(), + ), ], ), ); @@ -68,6 +74,9 @@ void main() { FakeMailboxRepository(), ), emailRepositoryProvider.overrideWithValue(FakeEmailRepository()), + searchHistoryRepositoryProvider.overrideWithValue( + FakeSearchHistoryRepository(), + ), ], ), ); @@ -97,6 +106,9 @@ void main() { emailRepositoryProvider.overrideWithValue( FakeEmailRepository(searchResults: [email]), ), + searchHistoryRepositoryProvider.overrideWithValue( + FakeSearchHistoryRepository(), + ), ], ), ); @@ -132,6 +144,9 @@ void main() { FakeMailboxRepository([archiveMailbox]), ), emailRepositoryProvider.overrideWithValue(FakeEmailRepository()), + searchHistoryRepositoryProvider.overrideWithValue( + FakeSearchHistoryRepository(), + ), ], ), ); @@ -162,6 +177,9 @@ void main() { emailRepositoryProvider.overrideWithValue( FakeEmailRepository(searchResults: [email]), ), + searchHistoryRepositoryProvider.overrideWithValue( + FakeSearchHistoryRepository(), + ), ], ), ); @@ -175,8 +193,9 @@ void main() { await tester.tap(find.byIcon(Icons.clear)); await tester.pumpAndSettle(); + // Results are gone; the recent-search chip for the prior query appears. expect(find.text('Found email'), findsNothing); - expect(find.text('Type 3+ characters to search'), findsOneWidget); + expect(find.text('found'), findsOneWidget); }); }); }