diff --git a/.gitignore b/.gitignore index 8695b9b..1779abf 100644 --- a/.gitignore +++ b/.gitignore @@ -54,4 +54,6 @@ app.*.map.json # Local testing artifacts /flutter_*.png -/devtools_options.yaml \ No newline at end of file +/devtools_options.yaml + +FINDINGS.md \ No newline at end of file diff --git a/lib/main.dart b/lib/main.dart index abaf0e7..d931358 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -256,6 +256,9 @@ class _AuthGateState extends State { Future _ensureDefaultCollectionIfNeeded() async { final userId = _session?.user.id; + // Intentional policy: attempt bootstrap once per user per app session. + // This avoids repeated retries/toasts when backend issues are transient. + // Users can recover on next session or via a manual retry entry point. if (userId == null || userId == _lastEnsuredUserId) return; _lastEnsuredUserId = userId; diff --git a/lib/scanner_screen.dart b/lib/scanner_screen.dart index f8b55b6..1105cee 100644 --- a/lib/scanner_screen.dart +++ b/lib/scanner_screen.dart @@ -95,7 +95,7 @@ class _ScannerScreenState extends State final cameras = await availableCameras(); if (cameras.isEmpty) { - throw Exception('No camera available'); + throw const ValidationException('No camera available'); } final backCamera = cameras.firstWhere( diff --git a/lib/screens/collections_screen.dart b/lib/screens/collections_screen.dart index 3bf5828..1e1c8c5 100644 --- a/lib/screens/collections_screen.dart +++ b/lib/screens/collections_screen.dart @@ -86,7 +86,9 @@ class CollectionsScreenState extends State } final userId = supabase.auth.currentUser?.id; if (userId == null) { - throw Exception('You must be signed in to load collections.'); + throw const AuthRequiredException( + 'You must be signed in to load collections.', + ); } final prefs = await SharedPreferences.getInstance(); diff --git a/lib/screens/garage_screen.dart b/lib/screens/garage_screen.dart index 0059306..826d9c8 100644 --- a/lib/screens/garage_screen.dart +++ b/lib/screens/garage_screen.dart @@ -525,7 +525,9 @@ class GarageScreenState extends State { } else { final userId = supabase.auth.currentUser?.id; if (userId == null) { - throw Exception('You must be signed in to copy cars.'); + throw const AuthRequiredException( + 'You must be signed in to copy cars.', + ); } final sourceCars = _cars @@ -1153,7 +1155,9 @@ class GarageScreenState extends State { } else { final userId = supabase.auth.currentUser?.id; if (userId == null) { - throw Exception('You must be signed in to copy cars.'); + throw const AuthRequiredException( + 'You must be signed in to copy cars.', + ); } final existing = await supabase diff --git a/lib/screens/manage_collection_screen.dart b/lib/screens/manage_collection_screen.dart index d32df90..ef0ce9c 100644 --- a/lib/screens/manage_collection_screen.dart +++ b/lib/screens/manage_collection_screen.dart @@ -180,7 +180,7 @@ class _ManageCollectionScreenState extends State { const SizedBox(height: 12), DropdownButtonFormField( isExpanded: true, - value: inviteRole, + initialValue: inviteRole, decoration: const InputDecoration( labelText: 'Role', prefixIcon: Icon(Icons.security_outlined), diff --git a/lib/screens/scan_tab.dart b/lib/screens/scan_tab.dart index 06cff02..4da91cf 100644 --- a/lib/screens/scan_tab.dart +++ b/lib/screens/scan_tab.dart @@ -5,6 +5,7 @@ import '../scanner_screen.dart'; import '../services/collection_service.dart'; import '../services/main_collection_sync.dart'; import '../theme/app_colors.dart'; +import '../utils/error_utils.dart'; import '../utils/preferences_utils.dart'; class ScanTab extends StatefulWidget { @@ -52,7 +53,9 @@ class ScanTabState extends State { final list = await CollectionService.getMyCollections(); final userId = supabase.auth.currentUser?.id; if (userId == null) { - throw Exception('You must be signed in to load collections.'); + throw const AuthRequiredException( + 'You must be signed in to load collections.', + ); } final prefs = await SharedPreferences.getInstance(); @@ -436,7 +439,7 @@ class ScanTabState extends State { }) async { final userId = supabase.auth.currentUser?.id; if (userId == null) { - throw Exception('You must be signed in to add cars.'); + throw const AuthRequiredException('You must be signed in to add cars.'); } await supabase.from('hotwheels').insert({ @@ -455,7 +458,9 @@ class ScanTabState extends State { }) async { final userId = supabase.auth.currentUser?.id; if (userId == null) { - throw Exception('You must be signed in to create catalog entries.'); + throw const AuthRequiredException( + 'You must be signed in to create catalog entries.', + ); } final cleanedSeries = series?.trim(); @@ -477,7 +482,9 @@ class ScanTabState extends State { Future _ensureValidationVote(String hwId) async { final userId = supabase.auth.currentUser?.id; if (userId == null) { - throw Exception('You must be signed in to validate entries.'); + throw const AuthRequiredException( + 'You must be signed in to validate entries.', + ); } final existingVote = await supabase .from('car_votes') diff --git a/lib/services/collection_service.dart b/lib/services/collection_service.dart index 162be06..01143e4 100644 --- a/lib/services/collection_service.dart +++ b/lib/services/collection_service.dart @@ -1,6 +1,6 @@ -//import 'package:supabase_flutter/supabase_flutter.dart'; import 'package:flutter/foundation.dart'; import '../main.dart'; +import '../utils/error_utils.dart'; /// Data model for a collection. class Collection { @@ -50,14 +50,30 @@ class CollectionMember { bool get isViewer => role == 'viewer'; } +/// Typed error for collection service operations. +class CollectionServiceException extends AppException { + const CollectionServiceException(super.message); +} + /// Service for managing collections and membership. class CollectionService { CollectionService._(); + static Never _fail(String message) => throw CollectionServiceException(message); + + @visibleForTesting + static String normalizeRole(String role) => role.trim().toLowerCase(); + + @visibleForTesting + static bool isSupportedInviteRole(String role) { + final normalized = normalizeRole(role); + return normalized == 'member' || normalized == 'viewer'; + } + static String _requireUserId() { final userId = supabase.auth.currentUser?.id; if (userId == null) { - throw Exception('You must be signed in to perform this action.'); + _fail('You must be signed in to perform this action.'); } return userId; } @@ -115,30 +131,14 @@ class CollectionService { final itemCounts = await getCollectionItemCounts(collectionIdList); final memberCounts = {}; - await Future.wait(collectionIdList.map((collectionId) async { - try { - final rows = await supabase.rpc('get_collection_members', params: { - 'p_collection_id': collectionId, - }); - memberCounts[collectionId] = (rows as List).length; - } catch (e) { - debugPrint('CollectionService.getMyCollections member RPC error: $e'); - } - })); + final members = await supabase + .from('collection_members') + .select('collection_id') + .inFilter('collection_id', collectionIdList); - final unresolvedIds = collectionIdList - .where((id) => !memberCounts.containsKey(id)) - .toList(growable: false); - if (unresolvedIds.isNotEmpty) { - final members = await supabase - .from('collection_members') - .select('id, collection_id') - .inFilter('collection_id', unresolvedIds); - - for (final member in members) { - final collectionId = member['collection_id'] as String; - memberCounts[collectionId] = (memberCounts[collectionId] ?? 0) + 1; - } + for (final member in members) { + final collectionId = member['collection_id'] as String; + memberCounts[collectionId] = (memberCounts[collectionId] ?? 0) + 1; } final collections = []; @@ -362,16 +362,21 @@ class CollectionService { .maybeSingle(); if (collection == null) { - throw Exception('Collection not found.'); + _fail('Collection not found.'); } if (collection['owner_id'] != userId) { - throw Exception('Only the collection owner can perform this action.'); + _fail('Only the collection owner can perform this action.'); } + final trimmedDescription = description?.trim(); + final normalizedDescription = + (trimmedDescription != null && trimmedDescription.isNotEmpty) + ? trimmedDescription + : null; + await supabase.from('collections').update({ 'name': name, - if (description != null && description.isNotEmpty) - 'description': description, + 'description': normalizedDescription, }).eq('id', collectionId); } @@ -386,10 +391,10 @@ class CollectionService { .maybeSingle(); if (collection == null) { - throw Exception('Collection not found.'); + _fail('Collection not found.'); } if (collection['owner_id'] != userId) { - throw Exception('Only the collection owner can perform this action.'); + _fail('Only the collection owner can perform this action.'); } await supabase.from('collections').delete().eq('id', collectionId); @@ -423,9 +428,9 @@ class CollectionService { required String email, String role = 'member', }) async { - final normalizedRole = role.trim().toLowerCase(); - if (normalizedRole != 'member' && normalizedRole != 'viewer') { - throw Exception('Unsupported role "$role".'); + final normalizedRole = normalizeRole(role); + if (!isSupportedInviteRole(role)) { + _fail('Unsupported role "$role".'); } final currentUserId = _requireUserId(); @@ -437,10 +442,10 @@ class CollectionService { .maybeSingle(); if (collection == null) { - throw Exception('Collection not found.'); + _fail('Collection not found.'); } if (collection['owner_id'] != currentUserId) { - throw Exception('Only the collection owner can invite members.'); + _fail('Only the collection owner can invite members.'); } // Call an RPC to look up the user ID by email. @@ -449,14 +454,14 @@ class CollectionService { }); if (result == null || (result is List && result.isEmpty)) { - throw Exception( + _fail( 'No user found with that email. They must create an account first.'); } final userId = result is List ? result.first['id'] as String : result as String; if (userId == currentUserId) { - throw Exception('You are already in this collection.'); + _fail('You are already in this collection.'); } // Check if already a member. @@ -468,7 +473,7 @@ class CollectionService { .maybeSingle(); if (existing != null) { - throw Exception('This user is already a member of this collection.'); + _fail('This user is already a member of this collection.'); } try { @@ -481,7 +486,7 @@ class CollectionService { final message = e.toString(); if (normalizedRole == 'viewer' && message.contains('collection_members_role_check')) { - throw Exception( + _fail( 'Viewer role is currently unavailable. Please contact the app administrator.', ); } @@ -503,14 +508,14 @@ class CollectionService { .maybeSingle(); if (collection == null) { - throw Exception('Collection not found.'); + _fail('Collection not found.'); } if (collection['owner_id'] != currentUserId) { - throw Exception('Only the collection owner can remove members.'); + _fail('Only the collection owner can remove members.'); } if (memberUserId == collection['owner_id']) { - throw Exception('Collection owner cannot be removed.'); + _fail('Collection owner cannot be removed.'); } final membersBefore = await getMembers(collectionId); @@ -518,11 +523,11 @@ class CollectionService { membersBefore.where((member) => member.userId == memberUserId); if (targetMembership.isEmpty) { - throw Exception('Member not found in this collection.'); + _fail('Member not found in this collection.'); } final target = targetMembership.first; if (target.role == 'owner') { - throw Exception('Collection owner cannot be removed.'); + _fail('Collection owner cannot be removed.'); } var deleted = false; @@ -552,14 +557,14 @@ class CollectionService { } if (!deleted) { - throw Exception('Member removal failed.'); + _fail('Member removal failed.'); } final membersAfter = await getMembers(collectionId); final stillExists = membersAfter.any((member) => member.userId == memberUserId); if (stillExists) { - throw Exception( + _fail( 'Member could not be removed. Please try again in a moment.', ); } @@ -577,10 +582,10 @@ class CollectionService { .maybeSingle(); if (membership == null) { - throw Exception('You are not a member of this collection.'); + _fail('You are not a member of this collection.'); } if (membership['role'] == 'owner') { - throw Exception('Owner cannot leave. Delete the collection instead.'); + _fail('Owner cannot leave. Delete the collection instead.'); } await supabase diff --git a/lib/services/storage_service.dart b/lib/services/storage_service.dart index c58ace3..7329f32 100644 --- a/lib/services/storage_service.dart +++ b/lib/services/storage_service.dart @@ -3,6 +3,7 @@ import 'package:flutter/foundation.dart'; import 'package:image/image.dart' as img; import 'package:supabase_flutter/supabase_flutter.dart'; import '../main.dart'; +import '../utils/error_utils.dart'; /// Handles uploading / deleting car images in Supabase Storage. /// @@ -29,7 +30,9 @@ class StorageService { }) async { final user = supabase.auth.currentUser; if (user == null) { - throw Exception('You must be signed in to upload images.'); + throw const AuthRequiredException( + 'You must be signed in to upload images.', + ); } final userId = user.id; final path = '$userId/$entryId.jpg'; @@ -121,7 +124,7 @@ class StorageService { static Uint8List _compressImageBytes(Uint8List bytes) { final decoded = img.decodeImage(bytes); if (decoded == null) { - throw Exception('Invalid image file.'); + throw const ValidationException('Invalid image file.'); } img.Image working = decoded.width > _maxWidth @@ -153,7 +156,7 @@ class StorageService { } if (out.lengthInBytes > _maxImageBytes) { - throw Exception( + throw ValidationException( 'Image is too large after compression (${out.lengthInBytes} bytes).', ); } diff --git a/lib/utils/error_utils.dart b/lib/utils/error_utils.dart index 3beee1d..923af36 100644 --- a/lib/utils/error_utils.dart +++ b/lib/utils/error_utils.dart @@ -1,10 +1,40 @@ import 'package:flutter/foundation.dart'; import 'package:supabase_flutter/supabase_flutter.dart'; +/// Base app-level exception for expected, user-facing failures. +class AppException implements Exception { + final String message; + + const AppException(this.message); + + @override + String toString() => message; +} + +class AuthRequiredException extends AppException { + const AuthRequiredException(super.message); +} + +class PermissionDeniedException extends AppException { + const PermissionDeniedException(super.message); +} + +class NotFoundException extends AppException { + const NotFoundException(super.message); +} + +class ValidationException extends AppException { + const ValidationException(super.message); +} + String userMessageForError( Object error, { String fallback = 'Something went wrong. Please try again.', }) { + if (error is AppException) { + return error.message; + } + if (error is AuthException) { return error.message; } diff --git a/pubspec.lock b/pubspec.lock index 4d47a53..96886b1 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -588,10 +588,10 @@ packages: dependency: transitive description: name: matcher - sha256: "12956d0ad8390bbcc63ca2e1469c0619946ccb52809807067a7020d57e647aa6" + sha256: dc0b7dc7651697ea4ff3e69ef44b0407ea32c487a39fff6a4004fa585e901861 url: "https://pub.dev" source: hosted - version: "0.12.18" + version: "0.12.19" material_color_utilities: dependency: transitive description: @@ -1025,10 +1025,10 @@ packages: dependency: transitive description: name: test_api - sha256: "93167629bfc610f71560ab9312acdda4959de4df6fac7492c89ff0d3886f6636" + sha256: "8161c84903fd860b26bfdefb7963b3f0b68fee7adea0f59ef805ecca346f0c7a" url: "https://pub.dev" source: hosted - version: "0.7.9" + version: "0.7.10" typed_data: dependency: transitive description: diff --git a/test/services/collection_service_test.dart b/test/services/collection_service_test.dart new file mode 100644 index 0000000..526cc5d --- /dev/null +++ b/test/services/collection_service_test.dart @@ -0,0 +1,41 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:hwhub/services/collection_service.dart'; + +void main() { + group('Collection role permissions', () { + Collection collectionWithRole(String role) { + return Collection( + id: 'c1', + name: 'Garage', + ownerId: 'u1', + createdAt: DateTime(2026, 1, 1), + role: role, + ); + } + + test('owner and member can modify cars, viewer cannot', () { + expect(collectionWithRole('owner').canModifyCars, isTrue); + expect(collectionWithRole('member').canModifyCars, isTrue); + expect(collectionWithRole('viewer').canModifyCars, isFalse); + }); + + test('viewer role flag is detected correctly', () { + expect(collectionWithRole('viewer').isViewer, isTrue); + expect(collectionWithRole('member').isViewer, isFalse); + }); + }); + + group('CollectionService invite role validation', () { + test('normalizes role input', () { + expect(CollectionService.normalizeRole(' Viewer '), 'viewer'); + expect(CollectionService.normalizeRole('MEMBER'), 'member'); + }); + + test('allows only member and viewer roles', () { + expect(CollectionService.isSupportedInviteRole('member'), isTrue); + expect(CollectionService.isSupportedInviteRole('viewer'), isTrue); + expect(CollectionService.isSupportedInviteRole(' owner '), isFalse); + expect(CollectionService.isSupportedInviteRole('admin'), isFalse); + }); + }); +}