From 8fc267d5fd101920d654704be7495ea76edb6e9d Mon Sep 17 00:00:00 2001 From: Billie Ponder Date: Mon, 26 Oct 2026 07:04:37 -0400 Subject: [PATCH] fix(clubs,routes): owner no longer shown "Request to Join"; Route Planner starts at rider's area (#89) * fix(clubs): respect existing membership in join presentation (#72) * fix(routes): start planner from cached rider location (#76) --------- Co-authored-by: Developer --- lib/features/routes/route_planner_screen.dart | 19 +- .../clubs/club_membership_presentation.dart | 8 +- .../social/clubs/widgets/club_card.dart | 3 +- lib/services/routes/route_providers.dart | 14 ++ test/club_card_test.dart | 20 +- test/club_membership_presentation_test.dart | 47 ++++ test/route_planner_initial_location_test.dart | 211 ++++++++++++++++++ 7 files changed, 312 insertions(+), 10 deletions(-) create mode 100644 test/route_planner_initial_location_test.dart diff --git a/lib/features/routes/route_planner_screen.dart b/lib/features/routes/route_planner_screen.dart index e57958d..592772e 100644 --- a/lib/features/routes/route_planner_screen.dart +++ b/lib/features/routes/route_planner_screen.dart @@ -7,6 +7,7 @@ import '../../core/responsive/viewport_size.dart'; import '../../core/theme/theme_extensions.dart'; import '../../services/auth/auth_providers.dart'; import '../../services/map/map_provider_interface.dart'; +import '../../services/map/map_providers.dart'; import '../../services/map/unified_map_view.dart'; import '../../services/routes/navigation_handoff.dart'; import '../../services/routes/route_models.dart'; @@ -43,6 +44,8 @@ class _RoutePlannerScreenState extends ConsumerState { void initState() { super.initState(); WidgetsBinding.instance.addPostFrameCallback((_) { + if (!mounted) return; + ref.read(mapFollowStateProvider.notifier).recenter(); _loadInitialData(); }); } @@ -593,12 +596,16 @@ class _RoutePlannerScreenState extends ConsumerState { const maxAllowed = RouteService.maxTechnicalStops; final isAtLimit = state.stops.length >= maxAllowed; - // Center map on road geometry, first stop, or default + final lastKnownLocation = ref.watch(routePlannerLocationProvider).valueOrNull; + + // Route context takes priority over cached rider location and the US overview. final mapCenter = state.geometry.isNotEmpty ? state.geometry.first : (state.stops.isNotEmpty ? AppLatLng(state.stops.first.latitude, state.stops.first.longitude) - : const AppLatLng(37.7749, -122.4194)); + : lastKnownLocation ?? const AppLatLng(39.5, -98.35)); + final mapZoom = state.geometry.isNotEmpty || state.stops.isNotEmpty || lastKnownLocation != null + ? 12.0 : 3.0; // Build markers final markers = state.stops.map((stop) { @@ -723,6 +730,7 @@ class _RoutePlannerScreenState extends ConsumerState { maxAllowed, isAtLimit, mapCenter, + mapZoom, markers, polylines, isNarrow, @@ -737,6 +745,7 @@ class _RoutePlannerScreenState extends ConsumerState { maxAllowed, isAtLimit, mapCenter, + mapZoom, markers, polylines, isNarrow, @@ -754,6 +763,7 @@ class _RoutePlannerScreenState extends ConsumerState { int maxAllowed, bool isAtLimit, AppLatLng mapCenter, + double mapZoom, List markers, List polylines, bool isNarrow, @@ -779,7 +789,7 @@ class _RoutePlannerScreenState extends ConsumerState { UnifiedMapView( key: const ValueKey('route_planner_map_expanded'), center: mapCenter, - zoom: 12.0, + zoom: mapZoom, markers: markers, polylines: polylines, ), @@ -873,6 +883,7 @@ class _RoutePlannerScreenState extends ConsumerState { int maxAllowed, bool isAtLimit, AppLatLng mapCenter, + double mapZoom, List markers, List polylines, bool isNarrow, @@ -978,7 +989,7 @@ class _RoutePlannerScreenState extends ConsumerState { UnifiedMapView( key: const ValueKey('route_planner_map_compact'), center: mapCenter, - zoom: 12.0, + zoom: mapZoom, markers: markers, polylines: polylines, ), diff --git a/lib/features/social/clubs/club_membership_presentation.dart b/lib/features/social/clubs/club_membership_presentation.dart index be3c965..cfc8d1c 100644 --- a/lib/features/social/clubs/club_membership_presentation.dart +++ b/lib/features/social/clubs/club_membership_presentation.dart @@ -119,9 +119,9 @@ class ClubMembershipPresentation { ); } - // Authenticated user with an active membership - if (member != null && member.status == ClubMemberStatus.active) { - final isOwner = member.role == ClubRole.owner || club.ownerUid == currentUserId; + // Root ownership remains authoritative when the member record is missing or stale. + if (club.ownerUid == currentUserId || member?.isActive == true) { + final isOwner = member?.role == ClubRole.owner || club.ownerUid == currentUserId; if (isOwner) { return const ClubMembershipPresentation( state: ClubMembershipPresentationState.activeOwner, @@ -135,7 +135,7 @@ class ClubMembershipPresentation { ); } - if (member.role == ClubRole.admin) { + if (member?.role == ClubRole.admin) { return const ClubMembershipPresentation( state: ClubMembershipPresentationState.activeAdmin, statusBadge: 'ADMIN', diff --git a/lib/features/social/clubs/widgets/club_card.dart b/lib/features/social/clubs/widgets/club_card.dart index 934a3df..781c6d1 100644 --- a/lib/features/social/clubs/widgets/club_card.dart +++ b/lib/features/social/clubs/widgets/club_card.dart @@ -129,7 +129,8 @@ class ClubCard extends StatelessWidget { ), // Admission policy badge - _buildAdmissionBadge(context, club.admissionPolicy, colors), + if (role == null) + _buildAdmissionBadge(context, club.admissionPolicy, colors), // Region badge if available if (club.homeRegion != null && club.homeRegion!.trim().isNotEmpty) diff --git a/lib/services/routes/route_providers.dart b/lib/services/routes/route_providers.dart index f8d000f..7ef1509 100644 --- a/lib/services/routes/route_providers.dart +++ b/lib/services/routes/route_providers.dart @@ -1,4 +1,5 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:geolocator/geolocator.dart'; import '../auth/auth_providers.dart'; import '../map/map_provider_interface.dart'; import 'aws_location_route_provider.dart'; @@ -7,6 +8,19 @@ import 'route_models.dart'; import 'route_provider.dart'; import 'route_service.dart'; +/// One cached location read per planner visit; never starts GPS or asks permission. +final routePlannerLocationProvider = FutureProvider.autoDispose((ref) async { + try { + final position = await Geolocator.getLastKnownPosition(); + if (position == null || !RoutePlannerNotifier.isValidCoordinate(position.latitude, position.longitude)) { + return null; + } + return AppLatLng(position.latitude, position.longitude); + } catch (_) { + return null; + } +}); + /// Provider for RouteService. final routeServiceProvider = Provider((ref) { return RouteService(); diff --git a/test/club_card_test.dart b/test/club_card_test.dart index f2478ae..6a3fc99 100644 --- a/test/club_card_test.dart +++ b/test/club_card_test.dart @@ -43,7 +43,7 @@ void main() { expect(find.text('@pacific-coast-riders'), findsOneWidget); expect(find.text('Cruising the Pacific Coast Highway together.'), findsOneWidget); expect(find.text('42 members'), findsOneWidget); - expect(find.text('Request to Join'), findsOneWidget); + expect(find.text('Request to Join'), findsNothing); expect(find.text('NorCal'), findsOneWidget); expect(find.text('Sport Touring'), findsOneWidget); expect(find.text('OWNER'), findsOneWidget); @@ -72,6 +72,24 @@ void main() { expect(find.byIcon(Icons.lock_open_outlined), findsNothing); }); + for (final role in ClubRole.values) { + testWidgets('active ${role.name} has no admission prompt', (tester) async { + await tester.pumpWidget(MaterialApp( + home: Scaffold(body: ClubCard(club: sampleClub, role: role)), + )); + + expect(find.text('Request to Join'), findsNothing); + }); + } + + testWidgets('non-member retains request admission prompt', (tester) async { + await tester.pumpWidget(MaterialApp( + home: Scaffold(body: ClubCard(club: sampleClub)), + )); + + expect(find.text('Request to Join'), findsOneWidget); + }); + testWidgets('suppresses role pill for standard members and shows for admin/owner', (tester) async { // Test member role (should NOT be rendered) await tester.pumpWidget( diff --git a/test/club_membership_presentation_test.dart b/test/club_membership_presentation_test.dart index a7eede2..f07008b 100644 --- a/test/club_membership_presentation_test.dart +++ b/test/club_membership_presentation_test.dart @@ -31,6 +31,53 @@ void main() { expect(p.primaryAction, isNull); }); + test('root owner cannot join even with a missing or stale member record', () { + for (final status in [null, ...ClubMemberStatus.values]) { + final p = ClubMembershipPresentation.resolve( + club: baseClub.copyWith(admissionPolicy: ClubAdmissionPolicy.request), + member: status == null ? null : ClubMember( + uid: 'owner_uid', + clubId: baseClub.id, + role: ClubRole.member, + status: status, + updatedAt: DateTime(2026, 1, 1), + ), + currentUserId: 'owner_uid', + isAuthenticated: true, + ); + + expect(p.state, ClubMembershipPresentationState.activeOwner); + expect(p.primaryAction, isNull); + expect(p.secondaryAction, isNull); + expect(p.canLeave, isFalse); + } + }); + + test('request admission actions respect active, pending and outsider states', () { + for (final role in ClubRole.values) { + for (final status in [null, ClubMemberStatus.active, ClubMemberStatus.requested]) { + final p = ClubMembershipPresentation.resolve( + club: baseClub.copyWith(admissionPolicy: ClubAdmissionPolicy.request), + member: status == null ? null : ClubMember( + uid: 'rider_1', + clubId: baseClub.id, + role: role, + status: status, + updatedAt: DateTime(2026, 1, 1), + ), + currentUserId: 'rider_1', + isAuthenticated: true, + ); + + expect(p.primaryAction, status == null + ? ClubMembershipAction.requestToJoin + : status == ClubMemberStatus.requested + ? ClubMembershipAction.withdrawRequest + : role == ClubRole.owner ? null : ClubMembershipAction.leaveClub); + } + } + }); + test('resolves archived state when club.isArchived is true', () { final archivedClub = baseClub.copyWith(archivedAt: DateTime(2026, 2, 1)); final p = ClubMembershipPresentation.resolve( diff --git a/test/route_planner_initial_location_test.dart b/test/route_planner_initial_location_test.dart new file mode 100644 index 0000000..3d17cb3 --- /dev/null +++ b/test/route_planner_initial_location_test.dart @@ -0,0 +1,211 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_map/flutter_map.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:motorcycle_dash/features/routes/route_planner_screen.dart'; +import 'package:motorcycle_dash/services/auth/auth_providers.dart'; +import 'package:motorcycle_dash/services/auth/auth_user.dart'; +import 'package:motorcycle_dash/services/map/map_provider_interface.dart'; +import 'package:motorcycle_dash/services/map/map_providers.dart'; +import 'package:motorcycle_dash/services/map/open_street_map_provider.dart'; +import 'package:motorcycle_dash/services/map/unified_map_view.dart'; +import 'package:motorcycle_dash/services/routes/route_providers.dart'; + +class FakeMapProvider extends Fake implements OpenStreetMapProvider { + @override + Widget buildMapView({ + required BuildContext context, + required AppLatLng center, + required double zoom, + double bearing = 0.0, + required List markers, + required List polylines, + required bool isFollowMode, + required MapConfig config, + required ValueChanged? onPositionChanged, + required VoidCallback? onUserInteraction, + }) => const SizedBox.expand(); +} + +void main() { + const riderPosition = AppLatLng(33.749, -84.388); + const overview = AppLatLng(39.5, -98.35); + + Future openPlanner( + WidgetTester tester, + Future location, { + bool realMap = false, + bool initiallyFollowing = true, + }) async { + await tester.pumpWidget( + ProviderScope( + overrides: [ + currentUserProvider.overrideWith( + (ref) => const AuthUser(uid: 'rider'), + ), + routePlannerLocationProvider.overrideWith((ref) => location), + mapFollowStateProvider.overrideWith( + (ref) => MapFollowNotifier()..setFollow(initiallyFollowing), + ), + if (!realMap) + openStreetMapProviderInstance.overrideWithValue(FakeMapProvider()), + ], + child: const MaterialApp(home: RoutePlannerScreen()), + ), + ); + await tester.pumpAndSettle(); + return ProviderScope.containerOf( + tester.element(find.byType(RoutePlannerScreen)), + ); + } + + UnifiedMapView map(WidgetTester tester) => + tester.widget(find.byType(UnifiedMapView)); + + for (final size in [const Size(360, 640), const Size(800, 360)]) { + testWidgets( + 'known rider position centers the planner at $size without adding a stop', + (tester) async { + await tester.binding.setSurfaceSize(size); + addTearDown(() => tester.binding.setSurfaceSize(null)); + final container = await openPlanner( + tester, + Future.value(riderPosition), + ); + + expect(map(tester).center, riderPosition); + expect(map(tester).zoom, 12); + expect(container.read(routePlannerNotifierProvider).stops, isEmpty); + }, + ); + } + + testWidgets('pending or absent location shows an overview without blocking', ( + tester, + ) async { + final location = Completer(); + await openPlanner(tester, location.future); + + expect(find.text('ROUTE PLANNER'), findsOneWidget); + expect(map(tester).center, overview); + expect(map(tester).zoom, 3); + + location.complete(null); + await tester.pumpAndSettle(); + expect(map(tester).center, overview); + expect(map(tester).zoom, 3); + }); + + testWidgets('cached position arriving later updates the overview', ( + tester, + ) async { + final location = Completer(); + await openPlanner(tester, location.future); + expect(map(tester).zoom, 3); + + location.complete(riderPosition); + await tester.pumpAndSettle(); + expect(map(tester).center, riderPosition); + expect(map(tester).zoom, 12); + }); + + testWidgets( + 'previously panned map does not prevent initial cached centering', + (tester) async { + final location = Completer(); + await openPlanner( + tester, + location.future, + realMap: true, + initiallyFollowing: false, + ); + final controller = tester + .widget(find.byType(FlutterMap)) + .mapController!; + expect(controller.camera.center.latitude, overview.latitude); + + location.complete(riderPosition); + await tester.pumpAndSettle(); + expect( + controller.camera.center.latitude, + closeTo(riderPosition.latitude, 0.00001), + ); + expect( + controller.camera.center.longitude, + closeTo(riderPosition.longitude, 0.00001), + ); + expect(controller.camera.zoom, 12); + }, + ); + + testWidgets('route stops take priority over a late cached rider position', ( + tester, + ) async { + final location = Completer(); + final container = await openPlanner(tester, location.future); + container + .read(routePlannerNotifierProvider.notifier) + .addStop(displayName: 'Route start', latitude: 35.6, longitude: -83.5); + await tester.pumpAndSettle(); + + expect(map(tester).center, const AppLatLng(35.6, -83.5)); + expect(map(tester).zoom, 12); + location.complete(riderPosition); + await tester.pumpAndSettle(); + expect(map(tester).center, const AppLatLng(35.6, -83.5)); + expect(map(tester).zoom, 12); + }); + + test( + 'location source only reads cache and tolerates unavailable GPS', + () async { + const channel = MethodChannel('flutter.baseflow.com/geolocator'); + final calls = []; + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMethodCallHandler(channel, (call) async { + calls.add(call.method); + throw PlatformException(code: 'PERMISSION_DENIED'); + }); + addTearDown(() => messenger.setMockMethodCallHandler(channel, null)); + final container = ProviderContainer(); + addTearDown(container.dispose); + + expect(await container.read(routePlannerLocationProvider.future), isNull); + expect(calls, ['getLastKnownPosition']); + }, + ); + + for (final position in [riderPosition, null, const AppLatLng(0, 0)]) { + test( + 'cached location source validates $position without starting GPS', + () async { + const channel = MethodChannel('flutter.baseflow.com/geolocator'); + final calls = []; + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMethodCallHandler(channel, (call) async { + calls.add(call.method); + return position == null + ? null + : { + 'latitude': position.latitude, + 'longitude': position.longitude, + }; + }); + addTearDown(() => messenger.setMockMethodCallHandler(channel, null)); + final container = ProviderContainer(); + addTearDown(container.dispose); + + expect( + await container.read(routePlannerLocationProvider.future), + position == riderPosition ? riderPosition : null, + ); + expect(calls, ['getLastKnownPosition']); + }, + ); + } +}