diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index 9134bd8db8..54a66723e7 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -1895,6 +1895,7 @@ class Buddies extends Table { TextColumn get phone => text().nullable()(); TextColumn get photoPath => text().nullable()(); TextColumn get notes => text().withDefault(const Constant(''))(); + BoolColumn get isFavorite => boolean().withDefault(const Constant(false))(); IntColumn get createdAt => integer()(); IntColumn get updatedAt => integer()(); @@ -3183,7 +3184,7 @@ class AppDatabase extends _$AppDatabase { /// The current schema version as a static constant so that pre-open checks /// (e.g. version-mismatch guard) can reference it without an instance. - static const int currentSchemaVersion = 166; + static const int currentSchemaVersion = 168; /// The oldest schema whose reader can apply this build's sync payloads /// without loss or misinterpretation (the compatibility floor). @@ -3487,6 +3488,13 @@ class AppDatabase extends _$AppDatabase { // reverse-geocoded country/region/town/body of water (issue #1187). // Renumbered from 162, which #731 landed past while this branch was open. 166, + // v167 is likewise absent: it is claimed by issue #1269 (PR #1276) on a + // branch that is still open. + // v168 (issue #638): buddies.is_favorite, so frequently-dived buddies can + // be pinned to the top of the "Add buddy" picker regardless of sort. + // Renumbered from 161, which #1235 landed on main while this branch was + // open. + 168, ]; /// Idempotent DDL for the v106 connector-suggestion columns (Lightroom @@ -5082,6 +5090,21 @@ class AppDatabase extends _$AppDatabase { } } + /// Idempotent DDL for the v168 buddies.is_favorite column (issue #638), + /// letting frequently-dived buddies be pinned to the top of the "Add + /// buddy" picker regardless of sort. Self-guards on the table existing, and + /// defaults every pre-existing row to not-favorited. + Future _assertBuddyFavoriteColumn() async { + final cols = await customSelect("PRAGMA table_info('buddies')").get(); + if (cols.isEmpty) return; + final names = cols.map((c) => c.read('name')).toSet(); + if (!names.contains('is_favorite')) { + await customStatement( + 'ALTER TABLE buddies ADD COLUMN is_favorite INTEGER NOT NULL DEFAULT 0', + ); + } + } + /// Idempotent DDL for the v159 dive_data_sources.time_offset_seconds /// column (issue #1177). Same dual-call contract (onUpgrade + beforeOpen /// backstop) as the other column-assert helpers. Nullable with no default, @@ -8647,6 +8670,13 @@ class AppDatabase extends _$AppDatabase { await _assertPlaceNameLanguageColumn(); } if (from < 166) await reportProgress(); + // v168 (issue #638): buddies.is_favorite, so frequently-dived buddies + // can be pinned to the top of the "Add buddy" picker regardless of + // sort. + if (from < 168) { + await _assertBuddyFavoriteColumn(); + } + if (from < 168) await reportProgress(); }, beforeOpen: (details) async { // Enable foreign keys @@ -8851,6 +8881,11 @@ class AppDatabase extends _$AppDatabase { // media row mapper reads it on every hydration. await _assertMediaManualElapsedColumn(); + // v168 backstop: re-assert buddies.is_favorite (issue #638). A + // database that arrives by restore or sync-adopt never runs + // onUpgrade, and every read of a buddy would throw without it. + await _assertBuddyFavoriteColumn(); + // v145 backstop: re-assert the gps_tracks provenance and trim columns. await _assertGpsTrackColumns(); diff --git a/lib/features/buddies/data/repositories/buddy_merge_repository.dart b/lib/features/buddies/data/repositories/buddy_merge_repository.dart index 79b5b4523e..4439ec81f9 100644 --- a/lib/features/buddies/data/repositories/buddy_merge_repository.dart +++ b/lib/features/buddies/data/repositories/buddy_merge_repository.dart @@ -114,6 +114,7 @@ class BuddyMergeRepository { certificationAgency: null, photoPath: row.photoPath, notes: row.notes, + isFavorite: row.isFavorite, createdAt: DateTime.fromMillisecondsSinceEpoch(row.createdAt), updatedAt: DateTime.fromMillisecondsSinceEpoch(row.updatedAt), ); @@ -444,6 +445,7 @@ class BuddyMergeRepository { phone: Value(buddy.phone), photoPath: Value(buddy.photoPath), notes: Value(buddy.notes), + isFavorite: Value(buddy.isFavorite), createdAt: Value(buddy.createdAt.millisecondsSinceEpoch), updatedAt: Value(buddy.updatedAt.millisecondsSinceEpoch), ), @@ -591,6 +593,7 @@ class BuddyMergeRepository { phone: Value(buddy.phone), photoPath: Value(buddy.photoPath), notes: Value(buddy.notes), + isFavorite: Value(buddy.isFavorite), updatedAt: Value(now), ), ); diff --git a/lib/features/buddies/data/repositories/buddy_repository.dart b/lib/features/buddies/data/repositories/buddy_repository.dart index 89db094cbe..4d8ff761f5 100644 --- a/lib/features/buddies/data/repositories/buddy_repository.dart +++ b/lib/features/buddies/data/repositories/buddy_repository.dart @@ -146,6 +146,7 @@ class BuddyRepository { ), photoPath: row.data['photo_path'] as String?, notes: (row.data['notes'] as String?) ?? '', + isFavorite: (row.data['is_favorite'] as int? ?? 0) == 1, createdAt: DateTime.fromMillisecondsSinceEpoch( row.data['created_at'] as int, ), @@ -175,6 +176,7 @@ class BuddyRepository { phone: Value(buddy.phone), photoPath: Value(buddy.photoPath), notes: Value(buddy.notes), + isFavorite: Value(buddy.isFavorite), createdAt: Value(now.millisecondsSinceEpoch), updatedAt: Value(now.millisecondsSinceEpoch), ), @@ -236,6 +238,7 @@ class BuddyRepository { ), photoPath: row.data['photo_path'] as String?, notes: (row.data['notes'] as String?) ?? '', + isFavorite: (row.data['is_favorite'] as int? ?? 0) == 1, createdAt: DateTime.fromMillisecondsSinceEpoch( row.data['created_at'] as int, ), @@ -283,6 +286,7 @@ class BuddyRepository { phone: Value(buddy.phone), photoPath: Value(buddy.photoPath), notes: Value(buddy.notes), + isFavorite: Value(buddy.isFavorite), updatedAt: Value(now), ), ); @@ -375,6 +379,7 @@ class BuddyRepository { ), photoPath: row.data['photo_path'] as String?, notes: (row.data['notes'] as String?) ?? '', + isFavorite: (row.data['is_favorite'] as int? ?? 0) == 1, createdAt: DateTime.fromMillisecondsSinceEpoch( row.data['created_at'] as int, ), @@ -436,6 +441,7 @@ class BuddyRepository { phone: b.phone, photoPath: b.photoPath, notes: b.notes, + isFavorite: b.isFavorite, createdAt: DateTime.fromMillisecondsSinceEpoch(b.createdAt), updatedAt: DateTime.fromMillisecondsSinceEpoch(b.updatedAt), ); @@ -750,13 +756,35 @@ class BuddyRepository { } } - /// Get all buddies with their dive counts in a single efficient query + /// Get all buddies with their dive counts in a single efficient query. + /// + /// [query] optionally filters by name/email/phone (case-insensitive), for + /// the "Add buddy" picker's search box, which needs dive counts too so it + /// can sort search results the same way as the unfiltered list. Future> getAllBuddiesWithDiveCount({ String? diverId, + String? query, }) async { try { - final diverFilter = diverId != null ? 'WHERE b.diver_id = ?' : ''; - final variables = [if (diverId != null) Variable.withString(diverId)]; + final conditions = [ + if (diverId != null) 'b.diver_id = ?', + if (query != null && query.isNotEmpty) + '(LOWER(b.name) LIKE ? OR LOWER(b.email) LIKE ? OR b.phone LIKE ?)', + ]; + final where = conditions.isEmpty + ? '' + : 'WHERE ${conditions.join(' AND ')}'; + final searchTerm = query != null && query.isNotEmpty + ? '%${query.toLowerCase()}%' + : null; + final variables = [ + if (diverId != null) Variable.withString(diverId), + if (searchTerm != null) ...[ + Variable.withString(searchTerm), + Variable.withString(searchTerm), + Variable.withString(searchTerm), + ], + ]; final results = await _db.customSelect(''' SELECT b.*, COALESCE(dc.dive_count, 0) as dive_count @@ -766,7 +794,7 @@ class BuddyRepository { FROM dive_buddies GROUP BY buddy_id ) dc ON b.id = dc.buddy_id - $diverFilter + $where ORDER BY b.name ASC ''', variables: variables).get(); @@ -785,6 +813,7 @@ class BuddyRepository { ), photoPath: row.data['photo_path'] as String?, notes: (row.data['notes'] as String?) ?? '', + isFavorite: (row.data['is_favorite'] as int? ?? 0) == 1, createdAt: DateTime.fromMillisecondsSinceEpoch( row.data['created_at'] as int, ), @@ -813,6 +842,75 @@ class BuddyRepository { } } + /// Toggle favorite status for a buddy + Future toggleFavorite(String buddyId) async { + try { + _log.info('Toggling favorite for buddy: $buddyId'); + final now = DateTime.now().millisecondsSinceEpoch; + final buddy = await (_db.select( + _db.buddies, + )..where((t) => t.id.equals(buddyId))).getSingleOrNull(); + if (buddy == null) return; + await (_db.update(_db.buddies)..where((t) => t.id.equals(buddyId))).write( + BuddiesCompanion( + isFavorite: Value(!buddy.isFavorite), + updatedAt: Value(now), + ), + ); + await _syncRepository.markRecordPending( + entityType: 'buddies', + recordId: buddyId, + localUpdatedAt: now, + ); + SyncEventBus.notifyLocalChange(); + _log.info('Toggled favorite for buddy: $buddyId'); + } catch (e, stackTrace) { + _log.error( + 'Failed to toggle favorite for buddy: $buddyId', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + /// Set favorite status for a buddy + Future setFavorite(String buddyId, bool isFavorite) async { + try { + _log.info('Setting favorite=$isFavorite for buddy: $buddyId'); + final now = DateTime.now().millisecondsSinceEpoch; + final updated = + await (_db.update( + _db.buddies, + )..where((t) => t.id.equals(buddyId))).write( + BuddiesCompanion( + isFavorite: Value(isFavorite), + updatedAt: Value(now), + ), + ); + // A stale or deleted buddyId updates nothing; marking it pending would + // leave a sync record pointing at a row that does not exist. + if (updated == 0) { + _log.info('No buddy matched id, skipping favorite update: $buddyId'); + return; + } + await _syncRepository.markRecordPending( + entityType: 'buddies', + recordId: buddyId, + localUpdatedAt: now, + ); + SyncEventBus.notifyLocalChange(); + _log.info('Set favorite=$isFavorite for buddy: $buddyId'); + } catch (e, stackTrace) { + _log.error( + 'Failed to set favorite for buddy: $buddyId', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + /// Get dive count for a buddy Future getDiveCountForBuddy(String buddyId) async { final result = await _db @@ -980,6 +1078,7 @@ class BuddyRepository { certificationAgency: null, photoPath: row.photoPath, notes: row.notes, + isFavorite: row.isFavorite, createdAt: DateTime.fromMillisecondsSinceEpoch(row.createdAt), updatedAt: DateTime.fromMillisecondsSinceEpoch(row.updatedAt), ); diff --git a/lib/features/buddies/domain/entities/buddy.dart b/lib/features/buddies/domain/entities/buddy.dart index df792bdf7f..9ae2701d5e 100644 --- a/lib/features/buddies/domain/entities/buddy.dart +++ b/lib/features/buddies/domain/entities/buddy.dart @@ -14,6 +14,7 @@ class Buddy extends Equatable { final CertificationAgency? certificationAgency; final String? photoPath; final String notes; + final bool isFavorite; final DateTime createdAt; final DateTime updatedAt; @@ -27,6 +28,7 @@ class Buddy extends Equatable { this.certificationAgency, this.photoPath, this.notes = '', + this.isFavorite = false, required this.createdAt, required this.updatedAt, }); @@ -65,6 +67,7 @@ class Buddy extends Equatable { CertificationAgency? certificationAgency, String? photoPath, String? notes, + bool? isFavorite, DateTime? createdAt, DateTime? updatedAt, }) { @@ -78,6 +81,7 @@ class Buddy extends Equatable { certificationAgency: certificationAgency ?? this.certificationAgency, photoPath: photoPath ?? this.photoPath, notes: notes ?? this.notes, + isFavorite: isFavorite ?? this.isFavorite, createdAt: createdAt ?? this.createdAt, updatedAt: updatedAt ?? this.updatedAt, ); @@ -94,6 +98,7 @@ class Buddy extends Equatable { certificationAgency, photoPath, notes, + isFavorite, createdAt, updatedAt, ]; diff --git a/lib/features/buddies/presentation/pages/buddy_edit_page.dart b/lib/features/buddies/presentation/pages/buddy_edit_page.dart index b667d4fce2..50698529f3 100644 --- a/lib/features/buddies/presentation/pages/buddy_edit_page.dart +++ b/lib/features/buddies/presentation/pages/buddy_edit_page.dart @@ -702,6 +702,10 @@ class _BuddyEditPageState extends ConsumerState { ? _mergeCtrl?.mergedPhotoPath : _originalBuddy?.photoPath, notes: _notesController.text.trim(), + // Preserve favorite status (issue #638): this form has no favorite + // control, so a full-constructor rebuild would otherwise silently + // reset it to false on every save. + isFavorite: _originalBuddy?.isFavorite ?? false, createdAt: _originalBuddy?.createdAt ?? now, updatedAt: now, ); diff --git a/lib/features/buddies/presentation/providers/buddy_providers.dart b/lib/features/buddies/presentation/providers/buddy_providers.dart index 3e558f3f97..8ca8c9d05a 100644 --- a/lib/features/buddies/presentation/providers/buddy_providers.dart +++ b/lib/features/buddies/presentation/providers/buddy_providers.dart @@ -61,6 +61,34 @@ final allBuddiesWithDiveCountProvider = return repository.getAllBuddiesWithDiveCount(diverId: validatedDiverId); }); +/// Search results with dive counts, for the "Add buddy" picker sheet, which +/// sorts by dive count and needs that even while a search query is active. +final buddySearchWithDiveCountProvider = + FutureProvider.family, String>((ref, query) async { + if (query.isEmpty) { + return ref.watch(allBuddiesWithDiveCountProvider).value ?? []; + } + final repository = ref.watch(buddyRepositoryProvider); + final validatedDiverId = await ref.watch( + validatedCurrentDiverIdProvider.future, + ); + ref.invalidateSelfWhen(repository.watchBuddiesChanges()); + return repository.getAllBuddiesWithDiveCount( + diverId: validatedDiverId, + query: query, + ); + }); + +/// Sort state for the "Add buddy" picker sheet. Defaults to dive count +/// descending (issue #638): divers with many buddies on file mostly care +/// about who they dive with often, not the full alphabet. +final buddyPickerSortProvider = StateProvider>( + (ref) => const SortState( + field: BuddySortField.diveCount, + direction: SortDirection.descending, + ), +); + /// Apply sorting to a list of buddies with dive counts List applyBuddyWithDiveCountSorting( List buddies, @@ -68,26 +96,29 @@ List applyBuddyWithDiveCountSorting( ) { final sorted = List.from(buddies); - sorted.sort((a, b) { - int comparison; - // For text fields, invert direction (user expects descending = A→Z) - final invertForText = sort.field == BuddySortField.name; + int byNameAscending(BuddyWithDiveCount a, BuddyWithDiveCount b) => + a.buddy.name.toLowerCase().compareTo(b.buddy.name.toLowerCase()); + sorted.sort((a, b) { switch (sort.field) { case BuddySortField.name: - comparison = a.buddy.name.toLowerCase().compareTo( - b.buddy.name.toLowerCase(), - ); + final comparison = byNameAscending(a, b); + // For text fields, invert direction (user expects descending = A→Z) + return sort.direction == SortDirection.ascending + ? -comparison + : comparison; case BuddySortField.diveCount: - comparison = a.diveCount.compareTo(b.diveCount); + final comparison = a.diveCount.compareTo(b.diveCount); + if (comparison == 0) { + // Ties (very common -- most buddies share 0 dives) break + // alphabetically, so the order is deterministic instead of left to + // an unstable sort. + return byNameAscending(a, b); + } + return sort.direction == SortDirection.ascending + ? comparison + : -comparison; } - - if (invertForText) { - return sort.direction == SortDirection.ascending - ? -comparison - : comparison; - } - return sort.direction == SortDirection.ascending ? comparison : -comparison; }); return sorted; @@ -347,6 +378,14 @@ class BuddyListNotifier extends StateNotifier>> { await refresh(); } + /// Toggle favorite status for a buddy (issue #638) + Future toggleFavorite(String buddyId) async { + await _repository.toggleFavorite(buddyId); + _ref.invalidate(buddyByIdProvider(buddyId)); + _ref.invalidate(allBuddiesWithDiveCountProvider); + await refresh(); + } + Future mergeBuddies( Buddy mergedBuddy, List buddyIds, diff --git a/lib/features/buddies/presentation/widgets/buddy_picker.dart b/lib/features/buddies/presentation/widgets/buddy_picker.dart index a997e18598..3b6b6070e4 100644 --- a/lib/features/buddies/presentation/widgets/buddy_picker.dart +++ b/lib/features/buddies/presentation/widgets/buddy_picker.dart @@ -2,6 +2,9 @@ import 'dart:async'; import 'package:flutter/material.dart'; import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/constants/sort_options.dart'; +import 'package:submersion/core/constants/sort_options_display.dart'; +import 'package:submersion/core/models/sort_state.dart'; import 'package:submersion/core/providers/provider.dart'; import 'package:go_router/go_router.dart'; @@ -9,6 +12,8 @@ import 'package:submersion/l10n/l10n_extension.dart'; import 'package:submersion/features/dive_roles/domain/entities/dive_role.dart'; import 'package:submersion/features/dive_roles/presentation/dive_role_display.dart'; import 'package:submersion/features/dive_roles/presentation/providers/dive_role_providers.dart'; +import 'package:submersion/features/buddies/data/repositories/buddy_repository.dart' + show BuddyWithDiveCount; import 'package:submersion/features/buddies/domain/entities/buddy.dart'; import 'package:submersion/features/buddies/presentation/providers/buddy_providers.dart'; import 'package:submersion/features/certifications/domain/entities/certification.dart'; @@ -311,7 +316,7 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { String _searchQuery = ''; String _debouncedQuery = ''; Timer? _debounceTimer; - List? _lastSearchResults; + List? _lastSearchResults; late List _localSelectedBuddies; Map> _certsByBuddy = const >{}; @@ -345,8 +350,9 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { @override Widget build(BuildContext context) { final buddiesAsync = _debouncedQuery.isEmpty - ? ref.watch(allBuddiesProvider) - : ref.watch(buddySearchProvider(_debouncedQuery)); + ? ref.watch(allBuddiesWithDiveCountProvider) + : ref.watch(buddySearchWithDiveCountProvider(_debouncedQuery)); + final sort = ref.watch(buddyPickerSortProvider); return DraggableScrollableSheet( initialChildSize: 0.7, @@ -435,6 +441,7 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { if (result != null) { // New buddy was created, refresh the list so they can select it ref.invalidate(allBuddiesProvider); + ref.invalidate(allBuddiesWithDiveCountProvider); } }, icon: const Icon(Icons.person_add), @@ -444,8 +451,40 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { ), ), ), - const SizedBox(height: 8), - const Divider(), + const SizedBox(height: 4), + + // Sort toggle (issue #638): alternates between dive-count-desc + // (the default -- who do I dive with most) and alphabetical. + // Favorites are pinned to the top regardless of this choice. + Padding( + padding: const EdgeInsets.symmetric(horizontal: 16), + child: Align( + alignment: Alignment.centerRight, + child: TextButton.icon( + onPressed: () { + final next = sort.field == BuddySortField.diveCount + // Text fields invert direction throughout this + // codebase: descending is what renders A->Z. Ascending + // here would flip the alphabetical toggle to Z->A. + ? const SortState( + field: BuddySortField.name, + direction: SortDirection.descending, + ) + : const SortState( + field: BuddySortField.diveCount, + direction: SortDirection.descending, + ); + ref.read(buddyPickerSortProvider.notifier).state = next; + }, + icon: Icon(sort.field.icon, size: 18), + label: Text( + '${context.l10n.buddies_action_sort}: ' + '${sort.field.localizedName(context.l10n)}', + ), + ), + ), + ), + const Divider(height: 1), // Buddy list Expanded( @@ -484,6 +523,7 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { scrollController, buddies, _certsByBuddy, + sort, ); }, loading: () { @@ -497,6 +537,7 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { scrollController, _lastSearchResults!, _certsByBuddy, + sort, ), ), ], @@ -517,14 +558,51 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { Widget _buildBuddyListView( ScrollController scrollController, - List buddies, + List buddies, Map> certsByBuddy, + SortState sort, ) { + // Favorites are pinned to the top regardless of the chosen sort field + // (issue #638); each partition is sorted independently so the toggle + // still reorders within both groups. + final favorites = applyBuddyWithDiveCountSorting( + buddies.where((b) => b.buddy.isFavorite).toList(), + sort, + ); + final others = applyBuddyWithDiveCountSorting( + buddies.where((b) => !b.buddy.isFavorite).toList(), + sort, + ); + + final rows = <_PickerRow>[ + if (favorites.isNotEmpty) + _PickerRow.header(context.l10n.diveLog_filterChip_favorites), + ...favorites.map(_PickerRow.entry), + if (favorites.isNotEmpty && others.isNotEmpty) const _PickerRow.divider(), + ...others.map(_PickerRow.entry), + ]; + return ListView.builder( controller: scrollController, - itemCount: buddies.length, + itemCount: rows.length, itemBuilder: (context, index) { - final buddy = buddies[index]; + final row = rows[index]; + if (row.isDivider) return const Divider(height: 1); + if (row.header != null) { + return Padding( + padding: const EdgeInsets.fromLTRB(16, 12, 16, 4), + child: Text( + row.header!, + style: Theme.of(context).textTheme.labelMedium?.copyWith( + color: Theme.of(context).colorScheme.primary, + fontWeight: FontWeight.bold, + ), + ), + ); + } + + final buddy = row.entry!.buddy; + final diveCount = row.entry!.diveCount; final isSelected = _localSelectedBuddies.any( (b) => b.buddy.id == buddy.id, ); @@ -555,15 +633,45 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { subtitle: buddy.certificationLevel == null ? null : Text(buddy.certificationLevel!.displayName), - trailing: isSelected - ? Chip( + trailing: Row( + mainAxisSize: MainAxisSize.min, + children: [ + if (diveCount > 0) + Padding( + padding: const EdgeInsets.only(right: 4), + child: Text( + context.l10n.buddies_label_diveCount(diveCount), + style: Theme.of(context).textTheme.bodySmall?.copyWith( + color: Theme.of(context).colorScheme.onSurfaceVariant, + ), + ), + ), + IconButton( + icon: Icon( + buddy.isFavorite ? Icons.star : Icons.star_border, + size: 20, + color: buddy.isFavorite + ? Theme.of(context).colorScheme.primary + : Theme.of(context).colorScheme.onSurfaceVariant, + ), + tooltip: buddy.isFavorite + ? context.l10n.diveLog_detail_tooltip_removeFromFavorites + : context.l10n.diveLog_detail_tooltip_addToFavorites, + visualDensity: VisualDensity.compact, + onPressed: () => ref + .read(buddyListNotifierProvider.notifier) + .toggleFavorite(buddy.id), + ), + if (isSelected) + Chip( label: Text( selectedRole?.localizedName(context.l10n) ?? context.l10n.diveRole_builtin_buddy, ), visualDensity: VisualDensity.compact, - ) - : null, + ), + ], + ), onTap: () { if (isSelected) { _removeBuddy(buddy.id); @@ -663,3 +771,20 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { } } } + +/// A single row in the Add-buddy list: a section header, a divider between +/// the favorites section and the rest, or a buddy entry. +class _PickerRow { + final String? header; + final bool isDivider; + final BuddyWithDiveCount? entry; + + const _PickerRow.header(this.header) : isDivider = false, entry = null; + + const _PickerRow.divider() : header = null, isDivider = true, entry = null; + + const _PickerRow.entry(BuddyWithDiveCount value) + : header = null, + isDivider = false, + entry = value; +} diff --git a/test/core/database/migration_v168_buddy_favorite_test.dart b/test/core/database/migration_v168_buddy_favorite_test.dart new file mode 100644 index 0000000000..9dfa58eff6 --- /dev/null +++ b/test/core/database/migration_v168_buddy_favorite_test.dart @@ -0,0 +1,103 @@ +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; + +/// v168 adds `buddies.is_favorite` (issue #638): a diver can pin frequently +/// dived buddies to the top of the "Add buddy" picker regardless of the +/// chosen sort. NOT NULL with a false default, so every pre-existing buddy +/// reads back as not-favorited. +void main() { + test('v168 is in the migration ladder', () { + expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(168)); + expect(AppDatabase.migrationVersions, contains(168)); + }); + + test('a fresh database has buddies.is_favorite', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final cols = await db.customSelect("PRAGMA table_info('buddies')").get(); + final names = cols.map((c) => c.read('name')).toSet(); + expect(names, contains('is_favorite')); + }); + + test('the column is NOT NULL with a false default', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final cols = await db.customSelect("PRAGMA table_info('buddies')").get(); + final column = cols.firstWhere( + (c) => c.read('name') == 'is_favorite', + ); + expect(column.read('notnull'), 1); + expect(column.read('dflt_value'), '0'); + }); + + test('a database stranded at v160 gains the column via onUpgrade and ' + 'existing rows default to not-favorited', () async { + final nativeDb = NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 160'); + rawDb.execute(''' + CREATE TABLE buddies ( + id TEXT NOT NULL PRIMARY KEY, diver_id TEXT, name TEXT NOT NULL, + email TEXT, phone TEXT, photo_path TEXT, + notes TEXT NOT NULL DEFAULT '', created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, hlc TEXT) + '''); + rawDb.execute( + "INSERT INTO buddies (id, name, created_at, updated_at) " + "VALUES ('b1', 'B1', 0, 0)", + ); + }, + ); + final db = AppDatabase(nativeDb); + addTearDown(db.close); + + final cols = await db.customSelect("PRAGMA table_info('buddies')").get(); + final names = cols.map((c) => c.read('name')).toSet(); + expect(names, contains('is_favorite')); + + final row = await db + .customSelect("SELECT is_favorite FROM buddies WHERE id = 'b1'") + .getSingle(); + expect(row.read('is_favorite'), 0); + }); + + test('beforeOpen backstop adds the column when a parallel-branch collision ' + 'stranded a DB past v168 without running the onUpgrade block', () async { + final nativeDb = NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute( + 'PRAGMA user_version = ${AppDatabase.currentSchemaVersion}', + ); + rawDb.execute(''' + CREATE TABLE buddies ( + id TEXT NOT NULL PRIMARY KEY, diver_id TEXT, name TEXT NOT NULL, + email TEXT, phone TEXT, photo_path TEXT, + notes TEXT NOT NULL DEFAULT '', created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, hlc TEXT) + '''); + }, + ); + final db = AppDatabase(nativeDb); + addTearDown(db.close); + + final cols = await db.customSelect("PRAGMA table_info('buddies')").get(); + final names = cols.map((c) => c.read('name')).toSet(); + expect(names, contains('is_favorite')); + }); + + test('the assert is a no-op when the buddies table is absent', () async { + final nativeDb = NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('CREATE TABLE unrelated (id TEXT)'); + }, + ); + final db = AppDatabase(nativeDb); + addTearDown(db.close); + + await db.customSelect('SELECT 1').get(); + }); +} diff --git a/test/features/buddies/data/repositories/buddy_repository_test.dart b/test/features/buddies/data/repositories/buddy_repository_test.dart index 850809479d..6c2c9255a0 100644 --- a/test/features/buddies/data/repositories/buddy_repository_test.dart +++ b/test/features/buddies/data/repositories/buddy_repository_test.dart @@ -1,5 +1,6 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/database/database.dart' show AppDatabase; import 'package:submersion/core/services/database_service.dart'; import 'package:submersion/features/buddies/data/repositories/buddy_repository.dart'; import 'package:submersion/features/buddies/domain/entities/buddy.dart'; @@ -10,9 +11,10 @@ import '../../../../helpers/test_database.dart'; void main() { late BuddyRepository repository; + late AppDatabase db; setUp(() async { - await setUpTestDatabase(); + db = await setUpTestDatabase(); repository = BuddyRepository(); }); @@ -257,6 +259,108 @@ void main() { }); }); + group('getAllBuddiesWithDiveCount (issue #638)', () { + Future insertDive(String id) async { + final db = DatabaseService.instance.database; + await db.customStatement( + "INSERT INTO dives (id, dive_date_time, created_at, updated_at) " + "VALUES ('$id', 1000, 1000, 1000)", + ); + } + + test('reports the correct dive count per buddy', () async { + await insertDive('d1'); + await insertDive('d2'); + final frequent = await repository.createBuddy( + createTestBuddy(id: 'frequent', name: 'Frequent Buddy'), + ); + final rare = await repository.createBuddy( + createTestBuddy(id: 'rare', name: 'Rare Buddy'), + ); + await repository.addBuddyToDive('d1', frequent.id, DiveRole.buddyId); + await repository.addBuddyToDive('d2', frequent.id, DiveRole.buddyId); + await repository.addBuddyToDive('d1', rare.id, DiveRole.buddyId); + + final results = await repository.getAllBuddiesWithDiveCount(); + final byId = {for (final r in results) r.buddy.id: r.diveCount}; + + expect(byId['frequent'], equals(2)); + expect(byId['rare'], equals(1)); + }); + + test('carries the isFavorite flag through', () async { + await repository.createBuddy( + createTestBuddy(id: 'fav', name: 'Favorite Buddy'), + ); + await repository.toggleFavorite('fav'); + + final results = await repository.getAllBuddiesWithDiveCount(); + final fav = results.firstWhere((r) => r.buddy.id == 'fav'); + + expect(fav.buddy.isFavorite, isTrue); + }); + + test('query filters by name, matching the picker search box', () async { + await repository.createBuddy( + createTestBuddy(id: 'alice', name: 'Alice'), + ); + await repository.createBuddy(createTestBuddy(id: 'bob', name: 'Bob')); + + final results = await repository.getAllBuddiesWithDiveCount( + query: 'ali', + ); + + expect(results.map((r) => r.buddy.id), equals(['alice'])); + }); + }); + + group('favorites (issue #638)', () { + test('toggleFavorite flips false to true and back', () async { + final buddy = await repository.createBuddy( + createTestBuddy(name: 'Toggle Buddy'), + ); + expect(buddy.isFavorite, isFalse); + + await repository.toggleFavorite(buddy.id); + expect((await repository.getBuddyById(buddy.id))!.isFavorite, isTrue); + + await repository.toggleFavorite(buddy.id); + expect((await repository.getBuddyById(buddy.id))!.isFavorite, isFalse); + }); + + test('setFavorite sets the flag explicitly', () async { + final buddy = await repository.createBuddy( + createTestBuddy(name: 'Set Favorite Buddy'), + ); + + await repository.setFavorite(buddy.id, true); + expect((await repository.getBuddyById(buddy.id))!.isFavorite, isTrue); + + await repository.setFavorite(buddy.id, false); + expect((await repository.getBuddyById(buddy.id))!.isFavorite, isFalse); + }); + + test( + 'setFavorite on an unknown id leaves no pending sync record', + () async { + await repository.setFavorite('does-not-exist', true); + + final pending = await db.select(db.syncRecords).get(); + expect(pending, isEmpty); + }, + ); + + test( + 'toggleFavorite on an unknown id leaves no pending sync record', + () async { + await repository.toggleFavorite('does-not-exist'); + + final pending = await db.select(db.syncRecords).get(); + expect(pending, isEmpty); + }, + ); + }); + group('getBuddyStats', () { test('should return stats with zero dives for new buddy', () async { final buddy = await repository.createBuddy( diff --git a/test/features/buddies/presentation/providers/buddy_providers_test.dart b/test/features/buddies/presentation/providers/buddy_providers_test.dart index 887ae74af0..2319429d7a 100644 --- a/test/features/buddies/presentation/providers/buddy_providers_test.dart +++ b/test/features/buddies/presentation/providers/buddy_providers_test.dart @@ -1,7 +1,9 @@ import 'package:drift/drift.dart' show Value; import 'package:flutter_test/flutter_test.dart'; import 'package:shared_preferences/shared_preferences.dart'; +import 'package:submersion/core/constants/sort_options.dart'; import 'package:submersion/core/database/database.dart' as db; +import 'package:submersion/core/models/sort_state.dart'; import 'package:submersion/core/providers/provider.dart'; import 'package:submersion/core/services/database_service.dart'; @@ -33,6 +35,17 @@ Buddy _makeBuddy({ ); } +BuddyWithDiveCount _withCount( + String name, { + int diveCount = 0, + bool isFavorite = false, +}) { + return BuddyWithDiveCount( + buddy: _makeBuddy(id: name, name: name).copyWith(isFavorite: isFavorite), + diveCount: diveCount, + ); +} + /// Inserts a dive row directly into the `dives` table, mirroring a sync apply /// that writes rows without going through any list notifier. This fires the /// `dives` table-change tick that count-aware providers subscribe to. @@ -241,6 +254,109 @@ void main() { 'without any manual refresh() call', ); }); + + test('toggleFavorite flips the flag and refreshes the list', () async { + final diver = await seedCurrentDiver(); + final buddy = await buddyRepo.createBuddy( + _makeBuddy(name: 'Fave Buddy', diverId: diver.id), + ); + + final container = makeContainer(); + addTearDown(container.dispose); + + await container + .read(buddyListNotifierProvider.notifier) + .toggleFavorite(buddy.id); + + final updated = await buddyRepo.getBuddyById(buddy.id); + expect(updated!.isFavorite, isTrue); + }); + }); + + group('applyBuddyWithDiveCountSorting (issue #638)', () { + test('sorts by dive count descending by default', () { + final buddies = [ + _withCount('Low', diveCount: 1), + _withCount('High', diveCount: 10), + _withCount('Mid', diveCount: 5), + ]; + + final sorted = applyBuddyWithDiveCountSorting( + buddies, + const SortState( + field: BuddySortField.diveCount, + direction: SortDirection.descending, + ), + ); + + expect(sorted.map((b) => b.buddy.name), ['High', 'Mid', 'Low']); + }); + + test('dive count ascending reverses the order', () { + final buddies = [ + _withCount('Low', diveCount: 1), + _withCount('High', diveCount: 10), + _withCount('Mid', diveCount: 5), + ]; + + final sorted = applyBuddyWithDiveCountSorting( + buddies, + const SortState( + field: BuddySortField.diveCount, + direction: SortDirection.ascending, + ), + ); + + expect(sorted.map((b) => b.buddy.name), ['Low', 'Mid', 'High']); + }); + + test('name sort is alphabetical regardless of dive count', () { + final buddies = [ + _withCount('Charlie', diveCount: 99), + _withCount('Alice', diveCount: 0), + _withCount('Bob', diveCount: 50), + ]; + + final sorted = applyBuddyWithDiveCountSorting( + buddies, + const SortState( + field: BuddySortField.name, + direction: SortDirection.descending, + ), + ); + + expect(sorted.map((b) => b.buddy.name), ['Alice', 'Bob', 'Charlie']); + }); + + test('does not mutate the input list', () { + final buddies = [ + _withCount('Low', diveCount: 1), + _withCount('High', diveCount: 10), + ]; + final original = List.of(buddies); + + applyBuddyWithDiveCountSorting( + buddies, + const SortState( + field: BuddySortField.diveCount, + direction: SortDirection.descending, + ), + ); + + expect(buddies, original); + }); + }); + + group('buddyPickerSortProvider (issue #638)', () { + test('defaults to dive count descending, not alphabetical', () { + final container = ProviderContainer(); + addTearDown(container.dispose); + + final sort = container.read(buddyPickerSortProvider); + + expect(sort.field, BuddySortField.diveCount); + expect(sort.direction, SortDirection.descending); + }); }); // Issue #982: the buddy detail page's shared-dives preview showed an diff --git a/test/features/buddies/presentation/widgets/buddy_picker_chip_interactions_test.dart b/test/features/buddies/presentation/widgets/buddy_picker_chip_interactions_test.dart index fbe5cda1b7..4c6cb5a9aa 100644 --- a/test/features/buddies/presentation/widgets/buddy_picker_chip_interactions_test.dart +++ b/test/features/buddies/presentation/widgets/buddy_picker_chip_interactions_test.dart @@ -2,6 +2,8 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/features/buddies/data/repositories/buddy_repository.dart' + show BuddyWithDiveCount; import 'package:submersion/features/buddies/domain/entities/buddy.dart'; import 'package:submersion/features/buddies/presentation/providers/buddy_providers.dart'; import 'package:submersion/features/buddies/presentation/widgets/buddy_picker.dart'; @@ -45,8 +47,10 @@ Widget _buildPicker({ validatedCurrentDiverIdProvider.overrideWith( (ref) async => validatedDiverId, ), - allBuddiesProvider.overrideWith((ref) async => [_alice]), - buddySearchProvider.overrideWith((ref, q) async => const []), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => [BuddyWithDiveCount(buddy: _alice, diveCount: 0)], + ), + buddySearchWithDiveCountProvider.overrideWith((ref, q) async => const []), ], child: MaterialApp( localizationsDelegates: AppLocalizations.localizationsDelegates, diff --git a/test/features/buddies/presentation/widgets/buddy_picker_roles_test.dart b/test/features/buddies/presentation/widgets/buddy_picker_roles_test.dart index 05aba8931f..96a930916e 100644 --- a/test/features/buddies/presentation/widgets/buddy_picker_roles_test.dart +++ b/test/features/buddies/presentation/widgets/buddy_picker_roles_test.dart @@ -1,6 +1,8 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/buddies/data/repositories/buddy_repository.dart' + show BuddyWithDiveCount; import 'package:submersion/features/buddies/domain/entities/buddy.dart'; import 'package:submersion/features/buddies/presentation/providers/buddy_providers.dart'; import 'package:submersion/features/buddies/presentation/widgets/buddy_picker.dart'; @@ -27,8 +29,8 @@ final _testRoles = [ /// Buddy with a pre-hydrated instructor cert level -- in production this /// comes from `_withPrimaryCerts`, but this widget test overrides -/// `allBuddiesProvider` directly, bypassing the repository, so the fixture -/// must carry the derived field itself. +/// `allBuddiesWithDiveCountProvider` directly, bypassing the repository, so +/// the fixture must carry the derived field itself. final _instructorBuddy = Buddy( id: 'buddy-1', name: 'Alice Instructor', @@ -64,6 +66,10 @@ final _instructorCert = Certification( updatedAt: _now, ); +List _withCount(Iterable buddies) => [ + for (final b in buddies) BuddyWithDiveCount(buddy: b, diveCount: 0), +]; + /// Sets a tall screen so that bottom sheets and role selectors fit without /// overflow. void _useTallScreen(WidgetTester tester) { @@ -87,8 +93,8 @@ void main() { testApp( overrides: [ allDiveRolesProvider.overrideWith((ref) async => _testRoles), - allBuddiesProvider.overrideWith( - (ref) async => [_instructorBuddy, _plainBuddy], + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _withCount([_instructorBuddy, _plainBuddy]), ), allBuddyCertificationsProvider.overrideWith( (ref) async => { @@ -117,8 +123,8 @@ void main() { testApp( overrides: [ allDiveRolesProvider.overrideWith((ref) async => _testRoles), - allBuddiesProvider.overrideWith( - (ref) async => [_credentialedBuddy, _plainBuddy], + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _withCount([_credentialedBuddy, _plainBuddy]), ), allBuddyCertificationsProvider.overrideWith( (ref) async => { @@ -164,8 +170,8 @@ void main() { testApp( overrides: [ allDiveRolesProvider.overrideWith((ref) async => _testRoles), - allBuddiesProvider.overrideWith( - (ref) async => [_credentialedBuddy, _plainBuddy], + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _withCount([_credentialedBuddy, _plainBuddy]), ), allBuddyCertificationsProvider.overrideWith( (ref) async => { diff --git a/test/features/buddies/presentation/widgets/buddy_picker_test.dart b/test/features/buddies/presentation/widgets/buddy_picker_test.dart index 1c2e9a220e..256b1a2ae5 100644 --- a/test/features/buddies/presentation/widgets/buddy_picker_test.dart +++ b/test/features/buddies/presentation/widgets/buddy_picker_test.dart @@ -4,6 +4,8 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/buddies/data/repositories/buddy_repository.dart' + show BuddyWithDiveCount; import 'package:submersion/features/buddies/domain/entities/buddy.dart'; import 'package:submersion/features/dive_roles/domain/entities/dive_role.dart'; import 'package:submersion/features/dive_roles/presentation/providers/dive_role_providers.dart'; @@ -37,6 +39,16 @@ final _testBuddies = [ Buddy(id: '3', name: 'Charlie Brown', createdAt: _now, updatedAt: _now), ]; +/// [_testBuddies] wrapped with a dive count of 0, matching what the picker +/// sheet's providers return (it sorts by dive count -- see issue #638). +final _testBuddiesWithCount = [ + for (final b in _testBuddies) BuddyWithDiveCount(buddy: b, diveCount: 0), +]; + +List _withCount(Iterable buddies) => [ + for (final b in buddies) BuddyWithDiveCount(buddy: b, diveCount: 0), +]; + Widget _buildPicker({ List selectedBuddies = const [], ValueChanged>? onChanged, @@ -83,8 +95,10 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), - buddySearchProvider.overrideWith((ref, q) async => []), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), + buddySearchWithDiveCountProvider.overrideWith((ref, q) async => []), ], ), ); @@ -100,7 +114,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -115,13 +131,15 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), - buddySearchProvider.overrideWith((ref, query) async { - return _testBuddies - .where( - (b) => b.name.toLowerCase().contains(query.toLowerCase()), - ) - .toList(); + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), + buddySearchWithDiveCountProvider.overrideWith((ref, query) async { + return _withCount( + _testBuddies.where( + (b) => b.name.toLowerCase().contains(query.toLowerCase()), + ), + ); }), ], ), @@ -151,13 +169,15 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), - buddySearchProvider.overrideWith((ref, query) async { - return _testBuddies - .where( - (b) => b.name.toLowerCase().contains(query.toLowerCase()), - ) - .toList(); + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), + buddySearchWithDiveCountProvider.overrideWith((ref, query) async { + return _withCount( + _testBuddies.where( + (b) => b.name.toLowerCase().contains(query.toLowerCase()), + ), + ); }), ], ), @@ -187,13 +207,15 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), - buddySearchProvider.overrideWith((ref, query) async { - return _testBuddies - .where( - (b) => b.name.toLowerCase().contains(query.toLowerCase()), - ) - .toList(); + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), + buddySearchWithDiveCountProvider.overrideWith((ref, query) async { + return _withCount( + _testBuddies.where( + (b) => b.name.toLowerCase().contains(query.toLowerCase()), + ), + ); }), ], ), @@ -228,7 +250,9 @@ void main() { _buildPicker( selectedBuddies: [selectedBuddy], overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -244,7 +268,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -266,7 +292,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -296,7 +324,9 @@ void main() { _buildPicker( selectedBuddies: [selectedBuddy], overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -322,7 +352,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => []), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => [], + ), ], ), ); @@ -337,8 +369,12 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), - buddySearchProvider.overrideWith((ref, query) async => []), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), + buddySearchWithDiveCountProvider.overrideWith( + (ref, query) async => [], + ), ], ), ); @@ -357,7 +393,7 @@ void main() { testWidgets('shows loading spinner when provider is loading', ( tester, ) async { - final completer = Completer>(); + final completer = Completer>(); addTearDown(() { if (!completer.isCompleted) completer.complete([]); }); @@ -365,7 +401,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) => completer.future), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) => completer.future, + ), ], ), ); @@ -384,7 +422,7 @@ void main() { testWidgets('caches search results and shows LinearProgressIndicator ' 'during subsequent loading', (tester) async { var callCount = 0; - final secondSearchCompleter = Completer>(); + final secondSearchCompleter = Completer>(); addTearDown(() { if (!secondSearchCompleter.isCompleted) { secondSearchCompleter.complete([]); @@ -394,18 +432,19 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), - buddySearchProvider.overrideWith((ref, query) { + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), + buddySearchWithDiveCountProvider.overrideWith((ref, query) { callCount++; if (callCount <= 1) { // First search completes immediately return Future.value( - _testBuddies - .where( - (b) => - b.name.toLowerCase().contains(query.toLowerCase()), - ) - .toList(), + _withCount( + _testBuddies.where( + (b) => b.name.toLowerCase().contains(query.toLowerCase()), + ), + ), ); } // Second search hangs in loading @@ -444,7 +483,9 @@ void main() { _buildPicker( onChanged: (buddies) => result = buddies, overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -457,12 +498,12 @@ void main() { await tester.tap(find.text('Instructor')); await tester.pumpAndSettle(); - // Tap "Done" -- it's a TextButton in the sheet header - // Find all TextButtons and tap the one inside the bottom sheet - // The "Done" button is rendered by the _BuddySelectionSheet header + // Tap "Done" -- it's the TextButton in the sheet header. The sheet + // also has a sort-toggle TextButton (issue #638), so disambiguate by + // label rather than by type alone. final doneButton = find.descendant( of: find.byType(DraggableScrollableSheet), - matching: find.byType(TextButton), + matching: find.widgetWithText(TextButton, 'Done'), ); await tester.tap(doneButton); await tester.pumpAndSettle(); @@ -473,4 +514,74 @@ void main() { expect(result![0].role.id, equals(DiveRole.instructorId)); }); }); + + group('BuddyPicker - sort toggle (issue #638)', () { + // Dive counts deliberately disagree with alphabetical order so the two + // sorts are distinguishable: by count it reads Charlie, Bob, Alice. + final rankedBuddies = [ + BuddyWithDiveCount(buddy: _testBuddies[0], diveCount: 1), // Alice + BuddyWithDiveCount(buddy: _testBuddies[1], diveCount: 5), // Bob + BuddyWithDiveCount(buddy: _testBuddies[2], diveCount: 9), // Charlie + ]; + + /// The buddy names as the sheet actually renders them, top to bottom. + List renderedNames(WidgetTester tester) => [ + for (final tile in tester.widgetList(find.byType(ListTile))) + (tile.title! as Text).data!, + ]; + + Future pumpSheet(WidgetTester tester) async { + _useTallScreen(tester); + await tester.pumpWidget( + _buildPicker( + overrides: [ + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => rankedBuddies, + ), + ], + ), + ); + await tester.pumpAndSettle(); + await _openSheet(tester); + } + + testWidgets('defaults to dive count, most dives first', (tester) async { + await pumpSheet(tester); + + expect(renderedNames(tester), [ + 'Charlie Brown', + 'Bob Jones', + 'Alice Smith', + ]); + }); + + testWidgets('toggling to name sorts A to Z, not Z to A', (tester) async { + await pumpSheet(tester); + + await tester.tap(find.widgetWithText(TextButton, 'Sort: Dive Count')); + await tester.pumpAndSettle(); + + expect(find.widgetWithText(TextButton, 'Sort: Name'), findsOneWidget); + expect(renderedNames(tester), [ + 'Alice Smith', + 'Bob Jones', + 'Charlie Brown', + ]); + }); + + testWidgets('toggling back restores the dive count order', (tester) async { + await pumpSheet(tester); + + await tester.tap(find.widgetWithText(TextButton, 'Sort: Dive Count')); + await tester.pumpAndSettle(); + await tester.tap(find.widgetWithText(TextButton, 'Sort: Name')); + await tester.pumpAndSettle(); + + expect(renderedNames(tester), [ + 'Charlie Brown', + 'Bob Jones', + 'Alice Smith', + ]); + }); + }); }