From bd29366fe3723b723dcf81bd227a3b41bf7b2e96 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 21:48:22 +0000 Subject: [PATCH 1/3] Add dive-count sort and favorites to the Add Buddy picker Buddies in the "Add buddy" sheet can now be sorted by number of shared dives (descending by default) instead of just alphabetically, and can be marked as favorites with a star toggle that pins them to the top of the list regardless of sort. Adds buddies.is_favorite (schema v161) with the usual onUpgrade/beforeOpen migration pair. Addresses submersion-app/submersion#638. Co-authored-by: alpheios-one <275321969+alpheios-one@users.noreply.github.com> --- lib/core/database/database.dart | 33 +++- .../repositories/buddy_merge_repository.dart | 3 + .../data/repositories/buddy_repository.dart | 95 +++++++++++- .../buddies/domain/entities/buddy.dart | 5 + .../presentation/pages/buddy_edit_page.dart | 4 + .../providers/buddy_providers.dart | 72 +++++++-- .../presentation/widgets/buddy_picker.dart | 146 ++++++++++++++++-- .../migration_v161_buddy_favorite_test.dart | 113 ++++++++++++++ .../repositories/buddy_repository_test.dart | 88 +++++++++++ .../providers/buddy_providers_test.dart | 116 ++++++++++++++ .../buddy_picker_chip_interactions_test.dart | 10 +- .../widgets/buddy_picker_roles_test.dart | 22 ++- .../widgets/buddy_picker_test.dart | 138 +++++++++++------ 13 files changed, 756 insertions(+), 89 deletions(-) create mode 100644 test/core/database/migration_v161_buddy_favorite_test.dart diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index 964f389169..bf1339ceb4 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -1874,6 +1874,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()(); @@ -3162,7 +3163,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 = 160; + static const int currentSchemaVersion = 161; /// The oldest schema whose reader can apply this build's sync payloads /// without loss or misinterpretation (the compatibility floor). @@ -3444,6 +3445,9 @@ class AppDatabase extends _$AppDatabase { // service_records.service_type -> service_category rename. Renumbered // from 158 and then 159, which #1149 and #1177 claimed first on main. 160, + // v161 (issue #638): buddies.is_favorite, so frequently-dived buddies can + // be pinned to the top of the "Add buddy" picker regardless of sort. + 161, ]; /// Idempotent DDL for the v106 connector-suggestion columns (Lightroom @@ -4962,6 +4966,21 @@ class AppDatabase extends _$AppDatabase { } } + /// Idempotent DDL for the v161 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, @@ -8506,6 +8525,13 @@ class AppDatabase extends _$AppDatabase { await _assertServiceCategoryRename(); } if (from < 160) await reportProgress(); + // v161 (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 < 161) { + await _assertBuddyFavoriteColumn(); + } + if (from < 161) await reportProgress(); }, beforeOpen: (details) async { // Enable foreign keys @@ -8696,6 +8722,11 @@ class AppDatabase extends _$AppDatabase { // onUpgrade, and every read of a service record would throw. await _assertServiceCategoryRename(); + // v161 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 35c5132027..2f6a10b421 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), ); @@ -712,13 +718,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 @@ -728,7 +756,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(); @@ -747,6 +775,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, ), @@ -775,6 +804,63 @@ 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; + await (_db.update(_db.buddies)..where((t) => t.id.equals(buddyId))).write( + BuddiesCompanion(isFavorite: Value(isFavorite), updatedAt: Value(now)), + ); + 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 @@ -928,6 +1014,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 de7207407c..a60b55e414 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 9580f26d9a..b86bf4d126 100644 --- a/lib/features/buddies/presentation/providers/buddy_providers.dart +++ b/lib/features/buddies/presentation/providers/buddy_providers.dart @@ -60,6 +60,37 @@ 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, @@ -67,26 +98,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; @@ -312,6 +346,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..241c121b6b 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,37 @@ 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 + ? const SortState( + field: BuddySortField.name, + direction: SortDirection.ascending, + ) + : 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 +520,7 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { scrollController, buddies, _certsByBuddy, + sort, ); }, loading: () { @@ -497,6 +534,7 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { scrollController, _lastSearchResults!, _certsByBuddy, + sort, ), ), ], @@ -517,14 +555,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 +630,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 +768,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_v161_buddy_favorite_test.dart b/test/core/database/migration_v161_buddy_favorite_test.dart new file mode 100644 index 0000000000..5e46fec2ad --- /dev/null +++ b/test/core/database/migration_v161_buddy_favorite_test.dart @@ -0,0 +1,113 @@ +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; + +/// v161 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('v161 is in the migration ladder', () { + expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(161)); + expect(AppDatabase.migrationVersions, contains(161)); + }); + + 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 v161 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 820b733aa3..218f4c2cb1 100644 --- a/test/features/buddies/data/repositories/buddy_repository_test.dart +++ b/test/features/buddies/data/repositories/buddy_repository_test.dart @@ -257,6 +257,94 @@ 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, + ); + }); + }); + 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 37a3bbe5bd..ea6ddda6eb 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'; @@ -30,6 +32,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. @@ -219,5 +232,108 @@ 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); + }); }); } 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..7f36e76c14 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,12 @@ 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..98186b94a9 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,12 @@ 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 +116,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -115,13 +133,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 +171,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 +209,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 +252,9 @@ void main() { _buildPicker( selectedBuddies: [selectedBuddy], overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -244,7 +270,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -266,7 +294,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -296,7 +326,9 @@ void main() { _buildPicker( selectedBuddies: [selectedBuddy], overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -322,7 +354,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) async => []), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => [], + ), ], ), ); @@ -337,8 +371,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 +395,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 +403,9 @@ void main() { await tester.pumpWidget( _buildPicker( overrides: [ - allBuddiesProvider.overrideWith((ref) => completer.future), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) => completer.future, + ), ], ), ); @@ -384,7 +424,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 +434,20 @@ 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 +486,9 @@ void main() { _buildPicker( onChanged: (buddies) => result = buddies, overrides: [ - allBuddiesProvider.overrideWith((ref) async => _testBuddies), + allBuddiesWithDiveCountProvider.overrideWith( + (ref) async => _testBuddiesWithCount, + ), ], ), ); @@ -457,12 +501,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(); From 809bde58b244c5c818eb44a48760b4c564d0837e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:16:58 +0000 Subject: [PATCH 2/3] Format buddy favorites/sort files per dart format CI's format check failed because these files weren't run through dart format before the initial push. Co-authored-by: alpheios-one <275321969+alpheios-one@users.noreply.github.com> --- .../providers/buddy_providers.dart | 5 +- .../migration_v161_buddy_favorite_test.dart | 86 ++++++++----------- .../repositories/buddy_repository_test.dart | 10 +-- .../providers/buddy_providers_test.dart | 6 +- .../buddy_picker_chip_interactions_test.dart | 4 +- .../widgets/buddy_picker_test.dart | 7 +- 6 files changed, 47 insertions(+), 71 deletions(-) diff --git a/lib/features/buddies/presentation/providers/buddy_providers.dart b/lib/features/buddies/presentation/providers/buddy_providers.dart index b86bf4d126..e58b68c373 100644 --- a/lib/features/buddies/presentation/providers/buddy_providers.dart +++ b/lib/features/buddies/presentation/providers/buddy_providers.dart @@ -63,10 +63,7 @@ final allBuddiesWithDiveCountProvider = /// 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 { + FutureProvider.family, String>((ref, query) async { if (query.isEmpty) { return ref.watch(allBuddiesWithDiveCountProvider).value ?? []; } diff --git a/test/core/database/migration_v161_buddy_favorite_test.dart b/test/core/database/migration_v161_buddy_favorite_test.dart index 5e46fec2ad..dff00ac208 100644 --- a/test/core/database/migration_v161_buddy_favorite_test.dart +++ b/test/core/database/migration_v161_buddy_favorite_test.dart @@ -34,70 +34,60 @@ void main() { 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(''' + 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); + 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 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); - }, - ); + 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 v161 without running the onUpgrade block', - () async { - final nativeDb = NativeDatabase.memory( - setup: (rawDb) { - rawDb.execute( - 'PRAGMA user_version = ${AppDatabase.currentSchemaVersion}', - ); - rawDb.execute(''' + test('beforeOpen backstop adds the column when a parallel-branch collision ' + 'stranded a DB past v161 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 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 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( diff --git a/test/features/buddies/data/repositories/buddy_repository_test.dart b/test/features/buddies/data/repositories/buddy_repository_test.dart index 218f4c2cb1..46efb2a5a0 100644 --- a/test/features/buddies/data/repositories/buddy_repository_test.dart +++ b/test/features/buddies/data/repositories/buddy_repository_test.dart @@ -323,10 +323,7 @@ void main() { expect((await repository.getBuddyById(buddy.id))!.isFavorite, isTrue); await repository.toggleFavorite(buddy.id); - expect( - (await repository.getBuddyById(buddy.id))!.isFavorite, - isFalse, - ); + expect((await repository.getBuddyById(buddy.id))!.isFavorite, isFalse); }); test('setFavorite sets the flag explicitly', () async { @@ -338,10 +335,7 @@ void main() { expect((await repository.getBuddyById(buddy.id))!.isFavorite, isTrue); await repository.setFavorite(buddy.id, false); - expect( - (await repository.getBuddyById(buddy.id))!.isFavorite, - isFalse, - ); + expect((await repository.getBuddyById(buddy.id))!.isFavorite, isFalse); }); }); diff --git a/test/features/buddies/presentation/providers/buddy_providers_test.dart b/test/features/buddies/presentation/providers/buddy_providers_test.dart index ea6ddda6eb..30035abbb4 100644 --- a/test/features/buddies/presentation/providers/buddy_providers_test.dart +++ b/test/features/buddies/presentation/providers/buddy_providers_test.dart @@ -242,9 +242,9 @@ void main() { final container = makeContainer(); addTearDown(container.dispose); - await container.read(buddyListNotifierProvider.notifier).toggleFavorite( - buddy.id, - ); + await container + .read(buddyListNotifierProvider.notifier) + .toggleFavorite(buddy.id); final updated = await buddyRepo.getBuddyById(buddy.id); expect(updated!.isFavorite, isTrue); 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 7f36e76c14..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 @@ -50,9 +50,7 @@ Widget _buildPicker({ allBuddiesWithDiveCountProvider.overrideWith( (ref) async => [BuddyWithDiveCount(buddy: _alice, diveCount: 0)], ), - buddySearchWithDiveCountProvider.overrideWith( - (ref, q) async => const [], - ), + buddySearchWithDiveCountProvider.overrideWith((ref, q) async => const []), ], child: MaterialApp( localizationsDelegates: AppLocalizations.localizationsDelegates, diff --git a/test/features/buddies/presentation/widgets/buddy_picker_test.dart b/test/features/buddies/presentation/widgets/buddy_picker_test.dart index 98186b94a9..61b1967c1a 100644 --- a/test/features/buddies/presentation/widgets/buddy_picker_test.dart +++ b/test/features/buddies/presentation/widgets/buddy_picker_test.dart @@ -98,9 +98,7 @@ void main() { allBuddiesWithDiveCountProvider.overrideWith( (ref) async => _testBuddiesWithCount, ), - buddySearchWithDiveCountProvider.overrideWith( - (ref, q) async => [], - ), + buddySearchWithDiveCountProvider.overrideWith((ref, q) async => []), ], ), ); @@ -444,8 +442,7 @@ void main() { return Future.value( _withCount( _testBuddies.where( - (b) => - b.name.toLowerCase().contains(query.toLowerCase()), + (b) => b.name.toLowerCase().contains(query.toLowerCase()), ), ), ); From 4cb6e62ba10cb82f8708eaaee6ae1320b82cf2d3 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 17:00:45 -0400 Subject: [PATCH 3/3] fix(buddies): correct picker sort direction, guard setFavorite, drop claude.yml Addresses the three Copilot review findings on PR #1237. Sort toggle direction: text fields invert direction throughout this codebase, so SortDirection.descending is what renders A to Z (buddySortProvider on the standalone buddy list already defaults to name + descending for that reason). The picker's new toggle asked for ascending, which landed on the inverted branch and rendered Z to A. Three widget tests now pin the rendered order for the default sort, the toggled sort, and the toggle back. setFavorite phantom sync records: the update wrote unconditionally and then marked the record pending even when no row matched, leaving a sync record pointing at a buddy that does not exist. Drift's write() returns the affected row count, so the method now returns early on zero. toggleFavorite already guarded this with its read-before-write; both paths now have a regression test asserting sync_records stays empty for an unknown id. Removed .github/workflows/claude.yml: an issue_comment-triggered job with contents: write and pull-requests: write, gated only on the comment body containing "@claude", lets any commenter drive privileged automation with the base repo's secrets. --- .github/workflows/claude.yml | 42 ----------- .../data/repositories/buddy_repository.dart | 18 ++++- .../presentation/widgets/buddy_picker.dart | 5 +- .../repositories/buddy_repository_test.dart | 24 ++++++- .../widgets/buddy_picker_test.dart | 70 +++++++++++++++++++ 5 files changed, 112 insertions(+), 47 deletions(-) delete mode 100644 .github/workflows/claude.yml diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml deleted file mode 100644 index 83c427616c..0000000000 --- a/.github/workflows/claude.yml +++ /dev/null @@ -1,42 +0,0 @@ -name: Claude Code -on: - issue_comment: - types: [created] - pull_request_review_comment: - types: [created] -jobs: - claude: - if: contains(github.event.comment.body, '@claude') - runs-on: ubuntu-latest - permissions: - contents: write - pull-requests: write - issues: write - id-token: write - actions: read - steps: - - uses: actions/checkout@v6 - with: - fetch-depth: 0 - submodules: true - - - name: Read Flutter version - id: flutter-ver - run: echo "version=$(cat .github/flutter-version.txt)" >> "$GITHUB_OUTPUT" - - - uses: subosito/flutter-action@v2 - with: - flutter-version: ${{ steps.flutter-ver.outputs.version }} - channel: 'stable' - - - name: Install dependencies - run: flutter pub get - - - name: Run code generation - run: dart run build_runner build --delete-conflicting-outputs - - - uses: anthropics/claude-code-action@v1 - with: - claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - claude_args: | - --allowedTools "Bash(flutter pub get:*),Bash(flutter analyze:*),Bash(flutter test:*),Bash(dart format:*),Bash(git fetch:*),Bash(git merge:*),Bash(git rebase:*),Bash(git push:*),Bash(gh pr:*),Bash(gh issue comment:*)" diff --git a/lib/features/buddies/data/repositories/buddy_repository.dart b/lib/features/buddies/data/repositories/buddy_repository.dart index 5a4b4852f6..4d8ff761f5 100644 --- a/lib/features/buddies/data/repositories/buddy_repository.dart +++ b/lib/features/buddies/data/repositories/buddy_repository.dart @@ -879,9 +879,21 @@ class BuddyRepository { try { _log.info('Setting favorite=$isFavorite for buddy: $buddyId'); final now = DateTime.now().millisecondsSinceEpoch; - await (_db.update(_db.buddies)..where((t) => t.id.equals(buddyId))).write( - BuddiesCompanion(isFavorite: Value(isFavorite), updatedAt: Value(now)), - ); + 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, diff --git a/lib/features/buddies/presentation/widgets/buddy_picker.dart b/lib/features/buddies/presentation/widgets/buddy_picker.dart index 241c121b6b..3b6b6070e4 100644 --- a/lib/features/buddies/presentation/widgets/buddy_picker.dart +++ b/lib/features/buddies/presentation/widgets/buddy_picker.dart @@ -463,9 +463,12 @@ class _BuddySelectionSheetState extends ConsumerState<_BuddySelectionSheet> { 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.ascending, + direction: SortDirection.descending, ) : const SortState( field: BuddySortField.diveCount, diff --git a/test/features/buddies/data/repositories/buddy_repository_test.dart b/test/features/buddies/data/repositories/buddy_repository_test.dart index c470a76a3d..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(); }); @@ -337,6 +339,26 @@ void main() { 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', () { diff --git a/test/features/buddies/presentation/widgets/buddy_picker_test.dart b/test/features/buddies/presentation/widgets/buddy_picker_test.dart index 61b1967c1a..256b1a2ae5 100644 --- a/test/features/buddies/presentation/widgets/buddy_picker_test.dart +++ b/test/features/buddies/presentation/widgets/buddy_picker_test.dart @@ -514,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', + ]); + }); + }); }