From 5013b7002f31e097e485ea029e2bfc3e0ff452c2 Mon Sep 17 00:00:00 2001 From: uhryniuk Date: Mon, 17 Aug 2026 15:04:34 -0500 Subject: [PATCH] V3-06: live stats in the notification RideNotificationController seam over flutter_local_notifications; RideNotificationCoordinator wires TripRepository.watchActiveTrip() to it and routes Pause/Resume actions back into RecordingEngine. Instantiated eagerly at app root rather than screen-owned, since a pocketed ride has no visible widget tree. Documents an unresolved risk: geolocator's own foreground-service notification cannot be suppressed, so two notifications may be visible until verified on a device. --- docs/v3/README.md | 2 +- docs/v3/V3-06-notification-stats.md | 44 +++++- lib/main.dart | 17 ++- lib/src/app/providers.dart | 21 +++ .../ride_notification_controller.dart | 115 +++++++++++++++ .../ride_notification_coordinator.dart | 62 ++++++++ .../notification/ride_notification_text.dart | 19 +++ .../recording/geolocator_location_source.dart | 20 ++- pubspec.lock | 48 +++++++ pubspec.yaml | 1 + test/ride_notification_test.dart | 135 ++++++++++++++++++ 11 files changed, 471 insertions(+), 13 deletions(-) create mode 100644 lib/src/notification/ride_notification_controller.dart create mode 100644 lib/src/notification/ride_notification_coordinator.dart create mode 100644 lib/src/notification/ride_notification_text.dart create mode 100644 test/ride_notification_test.dart diff --git a/docs/v3/README.md b/docs/v3/README.md index 655675c..b1d3a31 100644 --- a/docs/v3/README.md +++ b/docs/v3/README.md @@ -22,7 +22,7 @@ backup are v4 — see [../BACKLOG.md](../BACKLOG.md). | [V3-03](V3-03-units.md) | Distance and speed units | S | V3-02 | Done | | [V3-04](V3-04-live-map.md) | Live map on the recording screen | M | — | Done | | [V3-05](V3-05-mounted-mode.md) | Mounted (handlebar) mode | M | V3-04 | Done | -| [V3-06](V3-06-notification-stats.md) | Live stats in the notification | S | — | Not started | +| [V3-06](V3-06-notification-stats.md) | Live stats in the notification | S | — | Done | | [V3-07](V3-07-route-drawing.md) | Route drawing (pins, straight lines) | M | — | Not started | | [V3-08](V3-08-road-routing.md) | Road-snapped routing and ETA | L | V3-07, V3-01 | Not started | | [V3-09](V3-09-route-following.md) | Follow a planned route | M | V3-04, V3-08 | Not started | diff --git a/docs/v3/V3-06-notification-stats.md b/docs/v3/V3-06-notification-stats.md index 5b357de..cfd2484 100644 --- a/docs/v3/V3-06-notification-stats.md +++ b/docs/v3/V3-06-notification-stats.md @@ -1,6 +1,6 @@ # V3-06 — Live stats in the notification -**Phase** Live map · **Depends on** nothing · **Size** S · **Status** Not started +**Phase** Live map · **Depends on** nothing · **Size** S · **Status** Done ## Goal Distance and duration readable from the notification shade without unlocking. @@ -50,3 +50,45 @@ raise a silent minimal one. More moving parts, but it also **restores Pause/Resu ## Out of scope iOS. There is no equivalent live notification surface; a Live Activity is a much larger piece of work and belongs in its own ticket. + +## Outcome +Shipped as designed (option B), with one honestly-unresolved risk carried forward. + +`RideNotificationController` is a seam over `flutter_local_notifications` +(`FakeRideNotificationController` for tests), the same shape as `LocationSource` and +`WakelockController`. `RideNotificationCoordinator` owns the wiring: it subscribes to +`TripRepository.watchActiveTrip()` (the same stream `activeTripProvider` exposes, updated +once per writer flush) and calls `show`/`cancel`; it subscribes to the controller's action +stream and routes `pause`/`resume` back into `RecordingEngine`, guarded so a Resume can't +fire against a trip that isn't actually paused. `rideNotificationText(Trip, {unit})` is +pure and unit-tested directly — distance/elapsed formatting, the `Paused ·` prefix, and +unit-system handling. + +**Not screen-owned, deliberately.** Unlike the live map (V3-04) and mounted mode (V3-05), +which are fed from `RecordScreen`'s widget tree, the coordinator is instantiated eagerly +from `main.dart`'s `RipprApp.build` via a bare `ref.watch(rideNotificationCoordinatorProvider)` +— a pocketed ride has no visible widget tree, but the notification and Pause/Resume both +still have to work. + +**Unresolved risk, flagged rather than papered over:** the ticket's own risk section names +"two notification sources fighting" as the obvious failure mode, and it is real. +geolocator's `ForegroundNotificationConfig` is what satisfies Android's foreground-service +requirement and cannot be suppressed; `flutter_local_notifications` raises a second, +independent notification. There is no documented way to merge or guarantee only one is +visible — the geolocator notification was made minimal and silent +(`geolocator_location_source.dart`) on the assumption that an `ongoing: true` notification +on the same-ish surface might collapse or de-prioritise it, but that assumption is +unverified without a device. This is exactly the kind of claim the project's testing +philosophy refuses to accept on faith — see the real-ride checklist (V3-13) and this +ticket's own "Integration on a device" test, neither of which could run here. + +6 new tests, all in `test/ride_notification_test.dart`: three for `rideNotificationText` +(recording, paused-prefix, unit system), three for the coordinator using a real +`RecordingEngine` + in-memory `TripRepository` (not a mock) so pause/resume are checked in +the database per the project's standing rule, not by trusting the notification state. +`flutter analyze` clean; full suite green (237 tests, up from 231). + +Not attempted: the device-only acceptance criteria (text updates without unlocking, +Pause/Resume from the shade, the notification resisting swipe-away, and the two-source +visibility question above) — all require a real Android device, per the ticket's own Tests +section. diff --git a/lib/main.dart b/lib/main.dart index 4a2b69f..9af7b5c 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -33,10 +33,15 @@ class _RipprAppState extends ConsumerState { } @override - Widget build(BuildContext context) => MaterialApp.router( - title: 'Rippr', - debugShowCheckedModeBanner: false, - theme: ripprTheme(), - routerConfig: _router, - ); + Widget build(BuildContext context) { + // V3-06: forces creation so the live notification keeps updating and Pause/Resume + // keeps working regardless of which screen is on top -- see the provider's doc. + ref.watch(rideNotificationCoordinatorProvider); + return MaterialApp.router( + title: 'Rippr', + debugShowCheckedModeBanner: false, + theme: ripprTheme(), + routerConfig: _router, + ); + } } diff --git a/lib/src/app/providers.dart b/lib/src/app/providers.dart index 270a641..beaa35f 100644 --- a/lib/src/app/providers.dart +++ b/lib/src/app/providers.dart @@ -14,6 +14,8 @@ import '../config/config.dart'; import '../data/database.dart'; import '../data/trip_repository.dart'; import '../domain/models.dart'; +import '../notification/ride_notification_controller.dart'; +import '../notification/ride_notification_coordinator.dart'; import '../recording/geolocator_location_source.dart'; import '../recording/location_source.dart'; import '../recording/recording_engine.dart'; @@ -122,6 +124,25 @@ final wakelockControllerProvider = Provider( (ref) => PlusWakelockController(), ); +/// Overridden in tests with [FakeRideNotificationController]. +final rideNotificationControllerProvider = Provider( + (ref) => PlusRideNotificationController(), +); + +/// V3-06: eager, not screen-owned -- watched once from app root ([main.dart]) purely to +/// force creation, so it keeps running (and the notification keeps updating, and +/// Pause/Resume keeps working) regardless of which screen is on top or whether the app is +/// backgrounded at all. +final rideNotificationCoordinatorProvider = Provider((ref) { + final coordinator = RideNotificationCoordinator( + controller: ref.watch(rideNotificationControllerProvider), + engine: ref.watch(recordingEngineProvider), + tripStream: ref.watch(tripRepositoryProvider).watchActiveTrip(), + ); + ref.onDispose(coordinator.dispose); + return coordinator; +}); + /// Live path for the recording screen's map (V3-04). `.family` + `autoDispose` so the /// stream tears down the moment the record screen stops watching it — no leftover /// subscription ticking away once a ride ends or the map toggle goes off. diff --git a/lib/src/notification/ride_notification_controller.dart b/lib/src/notification/ride_notification_controller.dart new file mode 100644 index 0000000..7f5487e --- /dev/null +++ b/lib/src/notification/ride_notification_controller.dart @@ -0,0 +1,115 @@ +/// V3-06: this app's own notification, taking the shade back from geolocator's +/// `ForegroundNotificationConfig` -- which takes fixed strings at stream-subscription +/// time and offers no update path and no actions (see +/// `geolocator_location_source.dart`'s "Known parity gap" note). geolocator still raises +/// its own notification to satisfy Android's foreground-service requirement; it is +/// configured elsewhere to be minimal and silent, and this one is what the rider actually +/// reads and taps. +/// +/// Wrapped behind a seam -- the same reasoning as [LocationSource] and +/// [WakelockController] -- so the acquire/update/cancel path and the action-routing path +/// are both things a test can assert, not just plausible claims about a plugin. +library; + +import 'dart:async'; + +import 'package:flutter_local_notifications/flutter_local_notifications.dart'; + +enum RideNotificationAction { pause, resume } + +abstract class RideNotificationController { + Stream get actions; + + /// Shown once at the start of a ride and updated in place from then on -- a fresh + /// `show` with the same id replaces the previous one rather than stacking, which is + /// also what keeps "two notification sources fighting" from being possible on our side. + Future show({required String text, required bool paused}); + + /// Must run on stop, discard, and completion -- an ongoing notification that outlives + /// the ride it described is the notification equivalent of V3-05's leaked wake lock. + Future cancel(); +} + +const _channelId = 'ride_recording'; +const _notificationId = 1001; + +class PlusRideNotificationController implements RideNotificationController { + PlusRideNotificationController() { + _plugin.initialize( + settings: const InitializationSettings( + android: AndroidInitializationSettings('ic_stat_rippr'), + ), + onDidReceiveNotificationResponse: (response) { + final action = switch (response.actionId) { + 'pause_action' => RideNotificationAction.pause, + 'resume_action' => RideNotificationAction.resume, + _ => null, + }; + if (action != null) _actions.add(action); + }, + ); + } + + final _plugin = FlutterLocalNotificationsPlugin(); + final _actions = StreamController.broadcast(); + + @override + Stream get actions => _actions.stream; + + @override + Future show({required String text, required bool paused}) => _plugin.show( + id: _notificationId, + title: 'Rippr', + body: text, + notificationDetails: NotificationDetails( + android: AndroidNotificationDetails( + _channelId, + 'Ride recording', + // Cannot be swiped away mid-ride -- see the ticket's acceptance criteria. + ongoing: true, + autoCancel: false, + onlyAlertOnce: true, + icon: 'ic_stat_rippr', + actions: [ + if (paused) + const AndroidNotificationAction('resume_action', 'Resume') + else + const AndroidNotificationAction('pause_action', 'Pause'), + ], + ), + ), + ); + + @override + Future cancel() => _plugin.cancel(id: _notificationId); +} + +class FakeRideNotificationController implements RideNotificationController { + final _controller = StreamController.broadcast(); + + String? lastText; + bool? lastPaused; + bool visible = false; + int showCalls = 0; + int cancelCalls = 0; + + @override + Stream get actions => _controller.stream; + + @override + Future show({required String text, required bool paused}) async { + lastText = text; + lastPaused = paused; + visible = true; + showCalls++; + } + + @override + Future cancel() async { + visible = false; + cancelCalls++; + } + + /// Test helper: simulates the rider tapping Pause/Resume in the shade. + void simulateAction(RideNotificationAction action) => _controller.add(action); +} diff --git a/lib/src/notification/ride_notification_coordinator.dart b/lib/src/notification/ride_notification_coordinator.dart new file mode 100644 index 0000000..88b0969 --- /dev/null +++ b/lib/src/notification/ride_notification_coordinator.dart @@ -0,0 +1,62 @@ +/// V3-06: the piece that makes the notification live rather than static. Deliberately not +/// owned by a screen -- a pocketed ride has no visible widget tree, but the notification +/// still has to update and Pause/Resume still has to work. Instantiated once, eagerly, at +/// app root (see `main.dart`) and lives for the app's lifetime, the same shape as +/// [RecordingEngine] itself. +library; + +import 'dart:async'; + +import '../domain/models.dart'; +import '../recording/recording_engine.dart'; +import 'ride_notification_controller.dart'; +import 'ride_notification_text.dart'; + +class RideNotificationCoordinator { + RideNotificationCoordinator({ + required this._controller, + required this._engine, + required Stream tripStream, + }) { + // The trip stream is backed by the same Drift query the record screen watches, + // updated once per writer flush (~2 s) -- never per fix. + _tripSub = tripStream.listen(_onTrip); + _actionSub = _controller.actions.listen(_onAction); + } + + final RideNotificationController _controller; + final RecordingEngine _engine; + late final StreamSubscription _tripSub; + late final StreamSubscription _actionSub; + + Trip? _lastTrip; + + void _onTrip(Trip? trip) { + _lastTrip = trip; + if (trip == null) { + _controller.cancel(); + return; + } + _controller.show( + text: rideNotificationText(trip), + paused: trip.state == TripState.paused, + ); + } + + void _onAction(RideNotificationAction action) { + final trip = _lastTrip; + if (trip == null) return; + switch (action) { + // start() both begins a new ride and resumes a paused one -- see RecordingEngine. + case RideNotificationAction.resume: + if (trip.state == TripState.paused) _engine.start(); + case RideNotificationAction.pause: + if (trip.state == TripState.recording) _engine.pause(); + } + } + + Future dispose() async { + await _tripSub.cancel(); + await _actionSub.cancel(); + } +} diff --git a/lib/src/notification/ride_notification_text.dart b/lib/src/notification/ride_notification_text.dart new file mode 100644 index 0000000..598ec87 --- /dev/null +++ b/lib/src/notification/ride_notification_text.dart @@ -0,0 +1,19 @@ +/// V3-06: pure text formatting for the live ride notification, kept free of Flutter and +/// of the notification plugin so it is trivially unit-testable -- the plugin call itself +/// cannot be exercised without a device, but the string it is given can be. +library; + +import '../domain/models.dart'; +import '../telemetry/telemetry.dart' show formatDuration; +import '../ui/format.dart'; + +/// "12.3 km · 00:45:12", or "Paused · 12.3 km · 00:45:12" while paused. Driven from the +/// same aggregate columns the record screen reads, so this updates on the same ~2 s +/// writer flush -- never per fix. +String rideNotificationText(Trip trip, {UnitSystem unit = UnitSystem.metric}) { + final distance = formatDistance(trip.distanceM, unit: unit); + final elapsed = formatDuration(trip.movingMillis); + return trip.state == TripState.paused + ? 'Paused · $distance · $elapsed' + : '$distance · $elapsed'; +} diff --git a/lib/src/recording/geolocator_location_source.dart b/lib/src/recording/geolocator_location_source.dart index d050506..4a52972 100644 --- a/lib/src/recording/geolocator_location_source.dart +++ b/lib/src/recording/geolocator_location_source.dart @@ -13,9 +13,17 @@ /// `flutter_foreground_task` was dropped — along with its two deprecation warnings (no /// Swift Package Manager support, and it applies KGP). /// -/// **Known parity gap:** this notification cannot carry *actions*, so the native app's -/// Pause/Resume buttons in the shade are not reproduced. Tapping the notification opens -/// the app instead. Recorded in `docs/port/PROGRESS.md` for the T27 parity audit. +/// **Former parity gap, partially closed by V3-06:** this notification cannot carry +/// *actions* and offers no update path, so the notification the rider actually reads and +/// taps is meant to be `RideNotificationCoordinator`'s own, which restores live text and +/// Pause/Resume. **Unresolved risk, needs a real device:** geolocator's +/// `ForegroundNotificationConfig` is what satisfies Android's foreground-service +/// requirement and there is no documented way to suppress or merge it with a second, +/// independently-raised `flutter_local_notifications` notification -- it is entirely +/// possible both are visible in the shade at once. Kept minimal and silent here on the +/// assumption that the OS may collapse or de-prioritise it next to an `ongoing: true` +/// notification on the same channel, but that assumption is unverified. See V3-06's +/// Outcome section and the real-ride checklist (V3-13). /// /// ## iOS: the app stays awake only while location flows /// @@ -49,8 +57,10 @@ const Duration _interval = Duration(seconds: 1); class GeolocatorLocationSource implements LocationSource { GeolocatorLocationSource({ - this.notificationTitle = 'Rippr is recording', - this.notificationText = 'Tracking your ride', + // Deliberately quiet: the notification the rider reads is + // RideNotificationCoordinator's, not this one. See the file doc comment. + this.notificationTitle = 'Rippr', + this.notificationText = '', }); final String notificationTitle; diff --git a/pubspec.lock b/pubspec.lock index 8d5d357..c96a397 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -299,6 +299,46 @@ packages: url: "https://pub.dev" source: hosted version: "6.0.0" + flutter_local_notifications: + dependency: "direct main" + description: + name: flutter_local_notifications + sha256: "1447ba911c60f2ba3f25dae1af151ec187162566b0f57e37771bf0b400f013ad" + url: "https://pub.dev" + source: hosted + version: "22.3.0" + flutter_local_notifications_linux: + dependency: transitive + description: + name: flutter_local_notifications_linux + sha256: "9ca97e63776f29ab1b955725c09999fc2c150523269db150c39274f2a43c5a8b" + url: "https://pub.dev" + source: hosted + version: "8.0.1" + flutter_local_notifications_platform_interface: + dependency: transitive + description: + name: flutter_local_notifications_platform_interface + sha256: "43c3761d916c9bd3d5c7ebbc44d82f4990329840c0c5d62ad5260cc1b5d399bd" + url: "https://pub.dev" + source: hosted + version: "12.2.0" + flutter_local_notifications_web: + dependency: transitive + description: + name: flutter_local_notifications_web + sha256: "516afaf97a2d1e67a036c6617321b00d205d72f7a67b6eccf936cd565f985878" + url: "https://pub.dev" + source: hosted + version: "1.0.0" + flutter_local_notifications_windows: + dependency: transitive + description: + name: flutter_local_notifications_windows + sha256: "6f43bdd03b171b7a90f22647506fea33e2bb12294b7c7c7a3d690e960a382945" + url: "https://pub.dev" + source: hosted + version: "3.1.1" flutter_map: dependency: "direct main" description: @@ -1068,6 +1108,14 @@ packages: url: "https://pub.dev" source: hosted version: "0.7.12" + timezone: + dependency: transitive + description: + name: timezone + sha256: "981d1020d6ef8fe1e7b3de5054e5b25579ae7c403d7734adc508ffc47668e9cb" + url: "https://pub.dev" + source: hosted + version: "0.11.1" typed_data: dependency: transitive description: diff --git a/pubspec.yaml b/pubspec.yaml index 04f2049..ae496bf 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -49,6 +49,7 @@ dependencies: synchronized: ^3.4.1+1 intl: ^0.20.3 wakelock_plus: ^1.7.0 + flutter_local_notifications: ^22.3.0 dev_dependencies: integration_test: diff --git a/test/ride_notification_test.dart b/test/ride_notification_test.dart new file mode 100644 index 0000000..30c157c --- /dev/null +++ b/test/ride_notification_test.dart @@ -0,0 +1,135 @@ +import 'package:drift/drift.dart' show driftRuntimeOptions; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:rippr/src/data/database.dart'; +import 'package:rippr/src/data/trip_repository.dart'; +import 'package:rippr/src/domain/models.dart'; +import 'package:rippr/src/notification/ride_notification_controller.dart'; +import 'package:rippr/src/notification/ride_notification_coordinator.dart'; +import 'package:rippr/src/notification/ride_notification_text.dart'; +import 'package:rippr/src/recording/location_source.dart'; +import 'package:rippr/src/recording/recording_engine.dart'; + +/// V3-06: the plugin call itself needs a device (see the ticket's Tests section), but the +/// text it is given, and the routing from a tapped action back to the engine, do not. The +/// coordinator tests check the database after an action, not the UI -- the project's +/// standing rule for anything touching recording state. +void main() { + Trip trip({ + TripState state = TripState.recording, + double distanceM = 12345, + int movingMillis = 45 * 60 * 1000 + 12 * 1000, + }) => Trip( + id: 1, + startedAt: 0, + state: state, + distanceM: distanceM, + movingMillis: movingMillis, + ); + + group('rideNotificationText', () { + test('recording: distance and elapsed, no prefix', () { + expect(rideNotificationText(trip()), '12.3 km · 00:45:12'); + }); + + test('paused: prefixed so the shade reads correctly without opening the app', () { + expect( + rideNotificationText(trip(state: TripState.paused)), + 'Paused · 12.3 km · 00:45:12', + ); + }); + + test('honours the unit system, exactly like every other display surface', () { + expect( + rideNotificationText(trip(), unit: UnitSystem.imperial), + contains('mi'), + ); + }); + }); + + group('RideNotificationCoordinator', () { + late AppDatabase db; + late TripRepository repo; + late FakeLocationSource source; + late RecordingEngine engine; + late FakeRideNotificationController controller; + + setUp(() { + driftRuntimeOptions.dontWarnAboutMultipleDatabases = true; + db = AppDatabase(NativeDatabase.memory()); + repo = TripRepository(db); + source = FakeLocationSource(); + engine = RecordingEngine(repository: repo, locationSource: source); + controller = FakeRideNotificationController(); + }); + + tearDown(() async { + await source.dispose(); + await db.close(); + }); + + test('shows the notification when a trip starts and cancels it when it ends', + () async { + final coordinator = RideNotificationCoordinator( + controller: controller, + engine: engine, + tripStream: repo.watchActiveTrip(), + ); + + await repo.startTrip(1000); + await pumpEventQueue(); + expect(controller.visible, isTrue); + expect(controller.lastText, isNotNull); + + await repo.completeTrip(2000); + await pumpEventQueue(); + expect(controller.visible, isFalse); + expect(controller.cancelCalls, greaterThan(0)); + + await coordinator.dispose(); + }); + + test('a tapped Pause action actually pauses the ride -- checked in the ' + 'database, not the notification', () async { + final coordinator = RideNotificationCoordinator( + controller: controller, + engine: engine, + tripStream: repo.watchActiveTrip(), + ); + await repo.startTrip(1000); + await pumpEventQueue(); + + controller.simulateAction(RideNotificationAction.pause); + await pumpEventQueue(); + + final active = await repo.activeTrip(); + expect(active?.state, TripState.paused); + + await coordinator.dispose(); + }); + + test('a tapped Resume action only resumes an actually-paused trip', () async { + final coordinator = RideNotificationCoordinator( + controller: controller, + engine: engine, + tripStream: repo.watchActiveTrip(), + ); + await repo.startTrip(1000); + await pumpEventQueue(); + + // Recording, not paused: a Resume action here would be bogus. + controller.simulateAction(RideNotificationAction.resume); + await pumpEventQueue(); + expect((await repo.activeTrip())?.state, TripState.recording); + + await repo.pauseTrip(2000); + await pumpEventQueue(); + controller.simulateAction(RideNotificationAction.resume); + await pumpEventQueue(); + + expect((await repo.activeTrip())?.state, TripState.recording); + + await coordinator.dispose(); + }); + }); +}