From 9ac0be6ee5542e1224b6d3d6a3f4a14fce685881 Mon Sep 17 00:00:00 2001 From: uhryniuk Date: Mon, 24 Aug 2026 16:22:51 -0500 Subject: [PATCH] FB-05: closed-loop routes in Route Planner Add an isClosedLoop toggle to route plans (schema v3 -> v4 migration), threaded through the domain model, repository distance recomputation, the overflow menu, and the drawn polyline, so a route can loop back to its first waypoint. --- docs/feedback/FB-05-closed-loop-routes.md | 43 ++++++++++- lib/src/data/database.dart | 20 ++++- lib/src/data/database.g.dart | 83 ++++++++++++++++++++- lib/src/data/route_plan_repository.dart | 16 +++- lib/src/domain/models.dart | 6 ++ lib/src/ui/routes/route_planner_screen.dart | 19 +++++ test/migration_test.dart | 53 +++++++++++++ test/route_plan_repository_test.dart | 55 ++++++++++++++ test/route_planner_screen_test.dart | 72 ++++++++++++++++++ 9 files changed, 359 insertions(+), 8 deletions(-) diff --git a/docs/feedback/FB-05-closed-loop-routes.md b/docs/feedback/FB-05-closed-loop-routes.md index ed9504e..cb98a82 100644 --- a/docs/feedback/FB-05-closed-loop-routes.md +++ b/docs/feedback/FB-05-closed-loop-routes.md @@ -1,6 +1,6 @@ # FB-05 — Closed-loop routes in Route Planner -**Depends on** FB-04 (same file, heavily edited — sequence after it merges) · **Size** M · **Status** Not started +**Depends on** FB-04 (same file, heavily edited — sequence after it merges) · **Size** M · **Status** Done ## Goal Route planning currently only supports a one-way A→B→C path. Add the ability to close @@ -409,3 +409,44 @@ _OverflowMenu( Turn-by-turn following of a closed loop (V3-09, already deferred). Any UI indication of loop direction/rotation. Road-snapped routing for the closing segment (V3-08) — it stays a straight line like every other segment in this ticket's scope. + +## Outcome +Implemented exactly as designed, with one small addition to the migration step beyond +what the ticket spelled out: + +- Added `isClosedLoop` (`BoolColumn`, default `false`) to `RoutePlans` in + `lib/src/data/database.dart`, bumped `schemaVersion` from 3 to 4, and regenerated + `database.g.dart` via `dart run build_runner build`. +- **Deviation**: the migration guard needed to be `if (from >= 3 && from < 4)` rather + than the ticket's plain `if (from < 4)`. A v2 → v4 upgrade runs the existing + `if (from < 3)` branch first, which calls `m.createTable(routePlans)` — and since that + table definition already includes `isClosedLoop` (Drift generates `CREATE TABLE` from + the *current* Dart schema, not a historical snapshot), an unconditional `from < 4` + then tried to `ADD COLUMN isClosedLoop` a second time and hit a duplicate-column + SQLite error. Restricting the `addColumn` step to routes that already had the table + before v4 (`from >= 3`) fixes it; a v1/v2 → v4+ upgrade gets the column for free from + the fresh `createTable` instead. Added a v3-seed migration test + (`test/migration_test.dart`, `'a v3 database (FB-05) gains isClosedLoop defaulting to + false'`) covering exactly this path, plus verifying a v2 → v4+ jump still works via the + existing v2 test. +- Added `isClosedLoop` to the `RoutePlan` domain model, `copyWith`, the + `insertRoutePlan`/`_toRoutePlan` glue, and a new `setRoutePlanClosedLoop` method in + `database.dart`. +- Added `RoutePlanRepository.setClosedLoop` and updated `_recomputeDistance` to append + the first waypoint's coordinates as a closing point when the route is closed and has + 2+ waypoints, exactly per the design. +- Extended `_OverflowMenu` in `route_planner_screen.dart` with `canCloseLoop`/ + `isClosedLoop`/`onToggleLoop` and a "Close the loop" / "Open the loop" menu item, + wired at the call site. +- Extended the `PolylineLayer`'s point list with the conditional closing point. +- FB-04 had already changed the surrounding `route` access to a guarded `AsyncValue` + pattern (no more `.valueOrNull`) by the time this ticket landed; the new code reads + `route.isClosedLoop` off the same already-resolved `route` value the rest of the + build method uses, so no adjustment to that pattern was needed. + +Added migration, repository, and widget tests per the ticket's Tests section (in +`test/migration_test.dart`, `test/route_plan_repository_test.dart`, and +`test/route_planner_screen_test.dart`). + +`flutter analyze`: clean (only 4 pre-existing, unrelated `info`-level lints remain). +`flutter test`: all green, **411 tests passing** (up from the 404 baseline). diff --git a/lib/src/data/database.dart b/lib/src/data/database.dart index 912e535..1f0a3f2 100644 --- a/lib/src/data/database.dart +++ b/lib/src/data/database.dart @@ -121,6 +121,9 @@ class RoutePlans extends Table { /// Null in this ticket -- the polyline is derived from waypoints, not stored. V3-08 /// fills it with the road-snapped geometry, which is not cheaply re-derivable. TextColumn get geometry => text().nullable()(); + + /// Whether the route loops back to its first waypoint after the last one. See FB-05. + BoolColumn get isClosedLoop => boolean().withDefault(const Constant(false))(); } /// One pin on a [RoutePlans] row. @@ -146,7 +149,7 @@ class AppDatabase extends _$AppDatabase { AppDatabase(super.e); @override - int get schemaVersion => 3; + int get schemaVersion => 4; @override MigrationStrategy get migration => MigrationStrategy( @@ -162,6 +165,13 @@ class AppDatabase extends _$AppDatabase { await m.createTable(routePlans); await m.createTable(waypoints); } + // FB-05: closed-loop toggle, defaults to false for every pre-existing route. + // Guarded to `from >= 3` because a v2 -> v4+ upgrade just created `routePlans` + // fresh (above) with every current column, `isClosedLoop` included -- adding it + // again here would be a duplicate-column error. + if (from >= 3 && from < 4) { + await m.addColumn(routePlans, routePlans.isClosedLoop); + } }, beforeOpen: (details) async { // Non-negotiable: without this the CASCADE relationships above do nothing. @@ -553,6 +563,7 @@ class AppDatabase extends _$AppDatabase { distanceM: Value(route.distanceM), estimatedMillis: Value(route.estimatedMillis), geometry: Value(route.geometry), + isClosedLoop: Value(route.isClosedLoop), ), ); @@ -566,6 +577,12 @@ class AppDatabase extends _$AppDatabase { RoutePlansCompanion(distanceM: Value(distanceM)), ); + Future setRoutePlanClosedLoop(int id, bool value) => (update( + routePlans, + )..where((r) => r.id.equals(id))).write( + RoutePlansCompanion(isClosedLoop: Value(value)), + ); + /// Waypoints cascade with it. Future deleteRoutePlan(int id) => (delete(routePlans)..where((r) => r.id.equals(id))).go(); @@ -617,6 +634,7 @@ domain.RoutePlan _toRoutePlan(RoutePlanRow r) => domain.RoutePlan( distanceM: r.distanceM, estimatedMillis: r.estimatedMillis, geometry: r.geometry, + isClosedLoop: r.isClosedLoop, ); domain.Waypoint _toWaypoint(WaypointRow r) => domain.Waypoint( diff --git a/lib/src/data/database.g.dart b/lib/src/data/database.g.dart index c462fb1..98b68b7 100644 --- a/lib/src/data/database.g.dart +++ b/lib/src/data/database.g.dart @@ -1706,6 +1706,21 @@ class $RoutePlansTable extends RoutePlans type: DriftSqlType.string, requiredDuringInsert: false, ); + static const VerificationMeta _isClosedLoopMeta = const VerificationMeta( + 'isClosedLoop', + ); + @override + late final GeneratedColumn isClosedLoop = GeneratedColumn( + 'isClosedLoop', + aliasedName, + false, + type: DriftSqlType.bool, + requiredDuringInsert: false, + defaultConstraints: GeneratedColumn.constraintIsAlways( + 'CHECK ("isClosedLoop" IN (0, 1))', + ), + defaultValue: const Constant(false), + ); @override List get $columns => [ id, @@ -1715,6 +1730,7 @@ class $RoutePlansTable extends RoutePlans distanceM, estimatedMillis, geometry, + isClosedLoop, ]; @override String get aliasedName => _alias ?? actualTableName; @@ -1768,6 +1784,15 @@ class $RoutePlansTable extends RoutePlans geometry.isAcceptableOrUnknown(data['geometry']!, _geometryMeta), ); } + if (data.containsKey('isClosedLoop')) { + context.handle( + _isClosedLoopMeta, + isClosedLoop.isAcceptableOrUnknown( + data['isClosedLoop']!, + _isClosedLoopMeta, + ), + ); + } return context; } @@ -1807,6 +1832,10 @@ class $RoutePlansTable extends RoutePlans DriftSqlType.string, data['${effectivePrefix}geometry'], ), + isClosedLoop: attachedDatabase.typeMapping.read( + DriftSqlType.bool, + data['${effectivePrefix}isClosedLoop'], + )!, ); } @@ -1834,6 +1863,9 @@ class RoutePlanRow extends DataClass implements Insertable { /// Null in this ticket -- the polyline is derived from waypoints, not stored. V3-08 /// fills it with the road-snapped geometry, which is not cheaply re-derivable. final String? geometry; + + /// Whether the route loops back to its first waypoint after the last one. See FB-05. + final bool isClosedLoop; const RoutePlanRow({ required this.id, required this.name, @@ -1842,6 +1874,7 @@ class RoutePlanRow extends DataClass implements Insertable { required this.distanceM, this.estimatedMillis, this.geometry, + required this.isClosedLoop, }); @override Map toColumns(bool nullToAbsent) { @@ -1861,6 +1894,7 @@ class RoutePlanRow extends DataClass implements Insertable { if (!nullToAbsent || geometry != null) { map['geometry'] = Variable(geometry); } + map['isClosedLoop'] = Variable(isClosedLoop); return map; } @@ -1877,6 +1911,7 @@ class RoutePlanRow extends DataClass implements Insertable { geometry: geometry == null && nullToAbsent ? const Value.absent() : Value(geometry), + isClosedLoop: Value(isClosedLoop), ); } @@ -1895,6 +1930,7 @@ class RoutePlanRow extends DataClass implements Insertable { distanceM: serializer.fromJson(json['distanceM']), estimatedMillis: serializer.fromJson(json['estimatedMillis']), geometry: serializer.fromJson(json['geometry']), + isClosedLoop: serializer.fromJson(json['isClosedLoop']), ); } @override @@ -1910,6 +1946,7 @@ class RoutePlanRow extends DataClass implements Insertable { 'distanceM': serializer.toJson(distanceM), 'estimatedMillis': serializer.toJson(estimatedMillis), 'geometry': serializer.toJson(geometry), + 'isClosedLoop': serializer.toJson(isClosedLoop), }; } @@ -1921,6 +1958,7 @@ class RoutePlanRow extends DataClass implements Insertable { double? distanceM, Value estimatedMillis = const Value.absent(), Value geometry = const Value.absent(), + bool? isClosedLoop, }) => RoutePlanRow( id: id ?? this.id, name: name ?? this.name, @@ -1931,6 +1969,7 @@ class RoutePlanRow extends DataClass implements Insertable { ? estimatedMillis.value : this.estimatedMillis, geometry: geometry.present ? geometry.value : this.geometry, + isClosedLoop: isClosedLoop ?? this.isClosedLoop, ); RoutePlanRow copyWithCompanion(RoutePlansCompanion data) { return RoutePlanRow( @@ -1943,6 +1982,9 @@ class RoutePlanRow extends DataClass implements Insertable { ? data.estimatedMillis.value : this.estimatedMillis, geometry: data.geometry.present ? data.geometry.value : this.geometry, + isClosedLoop: data.isClosedLoop.present + ? data.isClosedLoop.value + : this.isClosedLoop, ); } @@ -1955,7 +1997,8 @@ class RoutePlanRow extends DataClass implements Insertable { ..write('activity: $activity, ') ..write('distanceM: $distanceM, ') ..write('estimatedMillis: $estimatedMillis, ') - ..write('geometry: $geometry') + ..write('geometry: $geometry, ') + ..write('isClosedLoop: $isClosedLoop') ..write(')')) .toString(); } @@ -1969,6 +2012,7 @@ class RoutePlanRow extends DataClass implements Insertable { distanceM, estimatedMillis, geometry, + isClosedLoop, ); @override bool operator ==(Object other) => @@ -1980,7 +2024,8 @@ class RoutePlanRow extends DataClass implements Insertable { other.activity == this.activity && other.distanceM == this.distanceM && other.estimatedMillis == this.estimatedMillis && - other.geometry == this.geometry); + other.geometry == this.geometry && + other.isClosedLoop == this.isClosedLoop); } class RoutePlansCompanion extends UpdateCompanion { @@ -1991,6 +2036,7 @@ class RoutePlansCompanion extends UpdateCompanion { final Value distanceM; final Value estimatedMillis; final Value geometry; + final Value isClosedLoop; const RoutePlansCompanion({ this.id = const Value.absent(), this.name = const Value.absent(), @@ -1999,6 +2045,7 @@ class RoutePlansCompanion extends UpdateCompanion { this.distanceM = const Value.absent(), this.estimatedMillis = const Value.absent(), this.geometry = const Value.absent(), + this.isClosedLoop = const Value.absent(), }); RoutePlansCompanion.insert({ this.id = const Value.absent(), @@ -2008,6 +2055,7 @@ class RoutePlansCompanion extends UpdateCompanion { this.distanceM = const Value.absent(), this.estimatedMillis = const Value.absent(), this.geometry = const Value.absent(), + this.isClosedLoop = const Value.absent(), }) : name = Value(name), createdAt = Value(createdAt); static Insertable custom({ @@ -2018,6 +2066,7 @@ class RoutePlansCompanion extends UpdateCompanion { Expression? distanceM, Expression? estimatedMillis, Expression? geometry, + Expression? isClosedLoop, }) { return RawValuesInsertable({ if (id != null) 'id': id, @@ -2027,6 +2076,7 @@ class RoutePlansCompanion extends UpdateCompanion { if (distanceM != null) 'distanceM': distanceM, if (estimatedMillis != null) 'estimatedMillis': estimatedMillis, if (geometry != null) 'geometry': geometry, + if (isClosedLoop != null) 'isClosedLoop': isClosedLoop, }); } @@ -2038,6 +2088,7 @@ class RoutePlansCompanion extends UpdateCompanion { Value? distanceM, Value? estimatedMillis, Value? geometry, + Value? isClosedLoop, }) { return RoutePlansCompanion( id: id ?? this.id, @@ -2047,6 +2098,7 @@ class RoutePlansCompanion extends UpdateCompanion { distanceM: distanceM ?? this.distanceM, estimatedMillis: estimatedMillis ?? this.estimatedMillis, geometry: geometry ?? this.geometry, + isClosedLoop: isClosedLoop ?? this.isClosedLoop, ); } @@ -2076,6 +2128,9 @@ class RoutePlansCompanion extends UpdateCompanion { if (geometry.present) { map['geometry'] = Variable(geometry.value); } + if (isClosedLoop.present) { + map['isClosedLoop'] = Variable(isClosedLoop.value); + } return map; } @@ -2088,7 +2143,8 @@ class RoutePlansCompanion extends UpdateCompanion { ..write('activity: $activity, ') ..write('distanceM: $distanceM, ') ..write('estimatedMillis: $estimatedMillis, ') - ..write('geometry: $geometry') + ..write('geometry: $geometry, ') + ..write('isClosedLoop: $isClosedLoop') ..write(')')) .toString(); } @@ -3980,6 +4036,7 @@ typedef $$RoutePlansTableCreateCompanionBuilder = RoutePlansCompanion Function({ Value distanceM, Value estimatedMillis, Value geometry, + Value isClosedLoop, }); typedef $$RoutePlansTableUpdateCompanionBuilder = RoutePlansCompanion Function({ Value id, @@ -3989,6 +4046,7 @@ typedef $$RoutePlansTableUpdateCompanionBuilder = RoutePlansCompanion Function({ Value distanceM, Value estimatedMillis, Value geometry, + Value isClosedLoop, }); final class $$RoutePlansTableReferences @@ -4059,6 +4117,11 @@ class $$RoutePlansTableFilterComposer builder: (column) => ColumnFilters(column), ); + ColumnFilters get isClosedLoop => $composableBuilder( + column: $table.isClosedLoop, + builder: (column) => ColumnFilters(column), + ); + Expression waypointsRefs( Expression Function($$WaypointsTableFilterComposer f) f, ) { @@ -4128,6 +4191,11 @@ class $$RoutePlansTableOrderingComposer column: $table.geometry, builder: (column) => ColumnOrderings(column), ); + + ColumnOrderings get isClosedLoop => $composableBuilder( + column: $table.isClosedLoop, + builder: (column) => ColumnOrderings(column), + ); } class $$RoutePlansTableAnnotationComposer @@ -4162,6 +4230,11 @@ class $$RoutePlansTableAnnotationComposer GeneratedColumn get geometry => $composableBuilder(column: $table.geometry, builder: (column) => column); + GeneratedColumn get isClosedLoop => $composableBuilder( + column: $table.isClosedLoop, + builder: (column) => column, + ); + Expression waypointsRefs( Expression Function($$WaypointsTableAnnotationComposer a) f, ) { @@ -4223,6 +4296,7 @@ class $$RoutePlansTableTableManager Value distanceM = const Value.absent(), Value estimatedMillis = const Value.absent(), Value geometry = const Value.absent(), + Value isClosedLoop = const Value.absent(), }) => RoutePlansCompanion( id: id, name: name, @@ -4231,6 +4305,7 @@ class $$RoutePlansTableTableManager distanceM: distanceM, estimatedMillis: estimatedMillis, geometry: geometry, + isClosedLoop: isClosedLoop, ), createCompanionCallback: ({ @@ -4241,6 +4316,7 @@ class $$RoutePlansTableTableManager Value distanceM = const Value.absent(), Value estimatedMillis = const Value.absent(), Value geometry = const Value.absent(), + Value isClosedLoop = const Value.absent(), }) => RoutePlansCompanion.insert( id: id, name: name, @@ -4249,6 +4325,7 @@ class $$RoutePlansTableTableManager distanceM: distanceM, estimatedMillis: estimatedMillis, geometry: geometry, + isClosedLoop: isClosedLoop, ), withReferenceMapper: (p0) => p0 .map( diff --git a/lib/src/data/route_plan_repository.dart b/lib/src/data/route_plan_repository.dart index 070dadd..a2d149d 100644 --- a/lib/src/data/route_plan_repository.dart +++ b/lib/src/data/route_plan_repository.dart @@ -32,6 +32,13 @@ class RoutePlanRepository { Future renameRoutePlan(int id, String name) => _db.renameRoutePlan(id, name); + /// Toggles whether the route loops back to its first waypoint. The closing segment + /// changes the total distance, so this recomputes it same as any waypoint edit. + Future setClosedLoop(int routeId, bool value) async { + await _db.setRoutePlanClosedLoop(routeId, value); + await _recomputeDistance(routeId); + } + /// Waypoints cascade with it. Future deleteRoutePlan(int id) => _db.deleteRoutePlan(id); @@ -93,10 +100,13 @@ class RoutePlanRepository { } Future _recomputeDistance(int routeId) async { + final route = await _db.getRoutePlan(routeId); final waypoints = await _db.waypointsForRoute(routeId); - final distance = geo.pathLengthMeters([ - for (final w in waypoints) geo.LatLon(w.latitude, w.longitude), - ]); + final points = [for (final w in waypoints) geo.LatLon(w.latitude, w.longitude)]; + if (route?.isClosedLoop == true && points.length >= 2) { + points.add(points.first); + } + final distance = geo.pathLengthMeters(points); await _db.setRoutePlanDistance(routeId, distance); } } diff --git a/lib/src/domain/models.dart b/lib/src/domain/models.dart index 5a04925..7c19470 100644 --- a/lib/src/domain/models.dart +++ b/lib/src/domain/models.dart @@ -241,6 +241,7 @@ class RoutePlan { this.distanceM = 0.0, this.estimatedMillis, this.geometry, + this.isClosedLoop = false, }); final int id; @@ -256,6 +257,9 @@ class RoutePlan { /// never stored -- it is fully derived from them, so there is nothing to persist. final String? geometry; + /// Whether the route loops back to its first waypoint after the last one. + final bool isClosedLoop; + RoutePlan copyWith({ int? id, String? name, @@ -264,6 +268,7 @@ class RoutePlan { double? distanceM, int? estimatedMillis, String? geometry, + bool? isClosedLoop, }) => RoutePlan( id: id ?? this.id, name: name ?? this.name, @@ -272,6 +277,7 @@ class RoutePlan { distanceM: distanceM ?? this.distanceM, estimatedMillis: estimatedMillis ?? this.estimatedMillis, geometry: geometry ?? this.geometry, + isClosedLoop: isClosedLoop ?? this.isClosedLoop, ); } diff --git a/lib/src/ui/routes/route_planner_screen.dart b/lib/src/ui/routes/route_planner_screen.dart index f73f940..00e9328 100644 --- a/lib/src/ui/routes/route_planner_screen.dart +++ b/lib/src/ui/routes/route_planner_screen.dart @@ -202,6 +202,8 @@ class _RoutePlannerScreenState extends ConsumerState Polyline( points: [ for (final w in waypoints) ll.LatLng(w.latitude, w.longitude), + if (route.isClosedLoop) + ll.LatLng(waypoints.first.latitude, waypoints.first.longitude), ], strokeWidth: 4, // Visibly distinct from a recorded path (see the @@ -315,10 +317,14 @@ class _RoutePlannerScreenState extends ConsumerState right: 8, child: _OverflowMenu( hasWaypoints: waypoints.isNotEmpty, + canCloseLoop: waypoints.length >= 2, + isClosedLoop: route.isClosedLoop, onRename: () => _rename(context, repo, route), onDownload: waypoints.isEmpty ? null : () => _downloadOfflineTiles(context, waypoints), + onToggleLoop: () => + repo.setClosedLoop(widget.routeId, !route.isClosedLoop), onDelete: () async { await repo.deleteRoutePlan(widget.routeId); widget.onBack?.call(); @@ -509,14 +515,20 @@ class _DropPinTooltipState extends State<_DropPinTooltip> class _OverflowMenu extends StatelessWidget { const _OverflowMenu({ required this.hasWaypoints, + required this.canCloseLoop, + required this.isClosedLoop, required this.onRename, required this.onDownload, + required this.onToggleLoop, required this.onDelete, }); final bool hasWaypoints; + final bool canCloseLoop; + final bool isClosedLoop; final VoidCallback onRename; final VoidCallback? onDownload; + final VoidCallback onToggleLoop; final VoidCallback onDelete; @override @@ -528,6 +540,7 @@ class _OverflowMenu extends StatelessWidget { onSelected: (value) => switch (value) { 'rename' => onRename(), 'download' => onDownload?.call(), + 'closeLoop' => onToggleLoop(), 'delete' => onDelete(), _ => null, }, @@ -543,6 +556,12 @@ class _OverflowMenu extends StatelessWidget { enabled: hasWaypoints, child: const Text('Download offline tiles'), ), + PopupMenuItem( + key: const Key('toggle-closed-loop'), + value: 'closeLoop', + enabled: canCloseLoop, + child: Text(isClosedLoop ? 'Open the loop' : 'Close the loop'), + ), const PopupMenuItem( key: Key('delete-route'), value: 'delete', diff --git a/test/migration_test.dart b/test/migration_test.dart index 3c6d037..76320b2 100644 --- a/test/migration_test.dart +++ b/test/migration_test.dart @@ -177,4 +177,57 @@ void main() { final waypoints = await db.waypointsForRoute(routeId); expect(waypoints, hasLength(1)); }); + + test('a v3 database (FB-05) gains isClosedLoop defaulting to false', () async { + // v3: identical to the v2 seed above, plus the route_plans/waypoints tables V3-07 + // added. + seedV1Database(); + final raw = sqlite3.sqlite3.open(dbFile.path); + raw.execute(''' + ALTER TABLE trips ADD COLUMN activity TEXT NOT NULL DEFAULT 'motorcycle'; + '''); + raw.execute(''' + CREATE TABLE route_plans ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL, + createdAt INTEGER NOT NULL, + activity TEXT NOT NULL DEFAULT 'motorcycle', + distanceM REAL NOT NULL DEFAULT 0, + estimatedMillis INTEGER NULL, + geometry TEXT NULL + ); + CREATE TABLE waypoints ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + routeId INTEGER NOT NULL REFERENCES route_plans (id) ON DELETE CASCADE, + ordinal INTEGER NOT NULL, + latitude REAL NOT NULL, + longitude REAL NOT NULL, + name TEXT NULL + ); + CREATE INDEX idx_waypoints_route ON waypoints (routeId); + '''); + raw.execute(''' + INSERT INTO route_plans (id, name, createdAt) VALUES (1, 'Existing route', 8000); + '''); + raw.execute('PRAGMA user_version = 3;'); + raw.close(); + + final db = AppDatabase(NativeDatabase(dbFile)); + addTearDown(db.close); + + // The pre-existing route survives with the new column defaulting to false. + final existing = await db.getRoutePlan(1); + expect(existing, isNotNull); + expect(existing!.isClosedLoop, isFalse); + + // A freshly inserted route also reads back false by default. + final newId = await db.insertRoutePlan( + const RoutePlan(name: 'New route', createdAt: 9000), + ); + expect((await db.getRoutePlan(newId))!.isClosedLoop, isFalse); + + // Toggling persists. + await db.setRoutePlanClosedLoop(1, true); + expect((await db.getRoutePlan(1))!.isClosedLoop, isTrue); + }); } diff --git a/test/route_plan_repository_test.dart b/test/route_plan_repository_test.dart index f85bb71..6e38ff4 100644 --- a/test/route_plan_repository_test.dart +++ b/test/route_plan_repository_test.dart @@ -113,6 +113,61 @@ void main() { expect(await db.waypointsForRoute(id), isEmpty); }); + test('setClosedLoop(true) adds the closing segment to distance', () async { + final id = await routes.createRoutePlan(1000); + await routes.addWaypoint(id, 51.0, -114.0); + await routes.addWaypoint(id, 51.01, -114.0); + await routes.addWaypoint(id, 51.02, -114.0); + + final open = (await routes.routePlanById(id))!.distanceM; + final waypoints = await routes.waypointsFor(id); + final closingSegment = geo.pathLengthMeters([ + geo.LatLon(waypoints.last.latitude, waypoints.last.longitude), + geo.LatLon(waypoints.first.latitude, waypoints.first.longitude), + ]); + + await routes.setClosedLoop(id, true); + final closed = (await routes.routePlanById(id))!.distanceM; + + expect(closed, closeTo(open + closingSegment, 1e-6)); + expect((await routes.routePlanById(id))!.isClosedLoop, isTrue); + }); + + test('setClosedLoop(false) after true returns distance to the open total', () async { + final id = await routes.createRoutePlan(1000); + await routes.addWaypoint(id, 51.0, -114.0); + await routes.addWaypoint(id, 51.01, -114.0); + await routes.addWaypoint(id, 51.02, -114.0); + + final open = (await routes.routePlanById(id))!.distanceM; + + await routes.setClosedLoop(id, true); + expect((await routes.routePlanById(id))!.distanceM, isNot(open)); + + await routes.setClosedLoop(id, false); + expect((await routes.routePlanById(id))!.distanceM, closeTo(open, 1e-6)); + expect((await routes.routePlanById(id))!.isClosedLoop, isFalse); + }); + + test('adding a waypoint to a closed-loop route keeps the closing segment correct', + () async { + final id = await routes.createRoutePlan(1000); + await routes.addWaypoint(id, 51.0, -114.0); + await routes.addWaypoint(id, 51.01, -114.0); + await routes.setClosedLoop(id, true); + + // Adding a third waypoint should recompute the closing segment against the new + // last waypoint, not the old one. + await routes.addWaypoint(id, 51.02, -114.0); + + final waypoints = await routes.waypointsFor(id); + final expected = geo.pathLengthMeters([ + for (final w in waypoints) geo.LatLon(w.latitude, w.longitude), + geo.LatLon(waypoints.first.latitude, waypoints.first.longitude), + ]); + expect((await routes.routePlanById(id))!.distanceM, closeTo(expected, 1e-6)); + }); + test('route plans never appear alongside trips and never affect ride totals', () async { await routes.createRoutePlan(1000, name: 'Plan A'); diff --git a/test/route_planner_screen_test.dart b/test/route_planner_screen_test.dart index f17cf21..e0fecc9 100644 --- a/test/route_planner_screen_test.dart +++ b/test/route_planner_screen_test.dart @@ -274,6 +274,78 @@ void main() { expect(find.textContaining('MB'), findsOneWidget); }); + screenTest('the "Close the loop" menu item is disabled with 0-1 waypoints, ' + 'enabled with 2+ (FB-05)', (tester) async { + final id = await repo.createRoutePlan(1000); + await repo.addWaypoint(id, 51.0, -114.0); + await pumpMap(tester, host(RoutePlannerScreen(routeId: id))); + + await tester.tap(find.byKey(const Key('route-overflow-menu'))); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pump(); + + var item = tester.widget>( + find.byKey(const Key('toggle-closed-loop')), + ); + expect(item.enabled, isFalse); + + // Close the (still-open) menu and add a second waypoint. + await tester.tapAt(const Offset(10, 10)); + await tester.pump(const Duration(milliseconds: 300)); + await repo.addWaypoint(id, 51.01, -114.0); + await tester.pump(const Duration(milliseconds: 50)); + + await tester.tap(find.byKey(const Key('route-overflow-menu'))); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pump(); + + item = tester.widget>( + find.byKey(const Key('toggle-closed-loop')), + ); + expect(item.enabled, isTrue); + }); + + screenTest('tapping "Close the loop" toggles isClosedLoop and flips the label ' + '(FB-05)', (tester) async { + final id = await repo.createRoutePlan(1000); + await repo.addWaypoint(id, 51.0, -114.0); + await repo.addWaypoint(id, 51.01, -114.0); + await pumpMap(tester, host(RoutePlannerScreen(routeId: id))); + + await tester.tap(find.byKey(const Key('route-overflow-menu'))); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pump(); + + expect(find.text('Close the loop'), findsOneWidget); + await tester.tap(find.byKey(const Key('toggle-closed-loop'))); + await tester.pump(const Duration(milliseconds: 300)); + + final route = await repo.routePlanById(id); + expect(route?.isClosedLoop, isTrue); + + await tester.tap(find.byKey(const Key('route-overflow-menu'))); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pump(); + expect(find.text('Open the loop'), findsOneWidget); + }); + + screenTest('a closed loop draws one more polyline point than there are waypoints ' + '(FB-05)', (tester) async { + final id = await repo.createRoutePlan(1000); + await repo.addWaypoint(id, 51.0, -114.0); + await repo.addWaypoint(id, 51.01, -114.0); + await repo.addWaypoint(id, 51.02, -114.0); + await repo.setClosedLoop(id, true); + await pumpMap(tester, host(RoutePlannerScreen(routeId: id))); + + final layer = tester.widget(find.byType(PolylineLayer)); + expect(layer.polylines.single.points, hasLength(4)); + }); + screenTest('the shell nav bar correctly shows Plan active on this screen ' '(UI-06)', (tester) async { final id = await repo.createRoutePlan(1000);