rippr/docs/feedback/FB-05-closed-loop-routes.md

FB-05 — Closed-loop routes in Route Planner

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 the route into a loop that returns to its starting pin, updating the drawn polyline and the distance stat to include the closing segment.

Context

Direct user feedback (docs/FEEDBACK.md, "Route" section):

...there is no way to create a loop with the pins, it only creates a unidirectional path.

Data model — no loop concept exists today

lib/src/domain/models.dart, RoutePlan (~lines 229-276):

class RoutePlan {
  const RoutePlan({
    this.id = 0,
    required this.name,
    required this.createdAt,
    this.activity = Activity.motorcycle,
    this.distanceM = 0.0,
    this.estimatedMillis,
    this.geometry,
  });

  final int id;
  final String name;
  final int createdAt;
  final Activity activity;
  final double distanceM;
  final int? estimatedMillis;
  final String? geometry;

  RoutePlan copyWith({
    int? id, String? name, int? createdAt, Activity? activity,
    double? distanceM, int? estimatedMillis, String? geometry,
  }) => RoutePlan(
    id: id ?? this.id, name: name ?? this.name, createdAt: createdAt ?? this.createdAt,
    activity: activity ?? this.activity, distanceM: distanceM ?? this.distanceM,
    estimatedMillis: estimatedMillis ?? this.estimatedMillis, geometry: geometry ?? this.geometry,
  );
}

Waypoint (~lines 278-309) has an ordinal for ordering, and waypoints are only ever appended at the end (RoutePlanRepository.addWaypoint, below) — there is no "return to start" field or concept anywhere in the model.

Polyline and distance — strictly open, no wraparound

lib/src/ui/routes/route_planner_screen.dart, the PolylineLayer (~lines 182-201):

if (waypoints.length >= 2)
  PolylineLayer(
    polylines: [
      Polyline(
        points: [
          for (final w in waypoints) ll.LatLng(w.latitude, w.longitude),
        ],
        strokeWidth: 4,
        pattern: StrokePattern.dashed(segments: const [8, 6]),
        color: colors.primary,
      ),
    ],
  ),

lib/src/data/route_plan_repository.dart, distance recomputation (full relevant section):

Future<void> addWaypoint(int routeId, double latitude, double longitude) async {
  final existing = await _db.waypointsForRoute(routeId);
  await _db.insertWaypoint(
    Waypoint(routeId: routeId, ordinal: existing.length, latitude: latitude, longitude: longitude),
  );
  await _recomputeDistance(routeId);
}

Future<void> _recomputeDistance(int routeId) async {
  final waypoints = await _db.waypointsForRoute(routeId);
  final distance = geo.pathLengthMeters([
    for (final w in waypoints) geo.LatLon(w.latitude, w.longitude),
  ]);
  await _db.setRoutePlanDistance(routeId, distance);
}

geo.pathLengthMeters (lib/src/geo/geo.dart ~line 185) sums consecutive-pair distances generically over whatever point list it's given — it needs no changes itself, just a point list that includes the closing segment when appropriate.

FloatingPill's Distance/Est. Time/Pins stats read straight off route.distanceM/ route.estimatedMillis/waypoints.length (route_planner_screen.dart ~lines 280-292) — Distance updates for free once _recomputeDistance accounts for the closing segment; Pins and Est. Time are unaffected by this ticket.

Schema — current version and migration pattern to follow

lib/src/data/database.dart:

@DataClassName('RoutePlanRow')
class RoutePlans extends Table {
  @override
  String get tableName => 'route_plans';

  IntColumn get id => integer().autoIncrement()();
  TextColumn get name => text()();
  IntColumn get createdAt => integer()();
  TextColumn get activity => textEnum<domain.Activity>().withDefault(const Constant('motorcycle'))();
  RealColumn get distanceM => real().withDefault(const Constant(0))();
  IntColumn get estimatedMillis => integer().nullable()();
  TextColumn get geometry => text().nullable()();
}

@DriftDatabase(tables: [Trips, Segments, TrackPoints, RoutePlans, Waypoints])
class AppDatabase extends _$AppDatabase {
  @override
  int get schemaVersion => 3;

  @override
  MigrationStrategy get migration => MigrationStrategy(
    onCreate: (m) => m.createAll(),
    onUpgrade: (m, from, to) async {
      if (from < 2) {
        await m.addColumn(trips, trips.activity);
      }
      if (from < 3) {
        await m.createTable(routePlans);
        await m.createTable(waypoints);
      }
    },
    ...

The existing if (from < 2) { await m.addColumn(trips, trips.activity); } is the exact pattern to follow for a new nullable/defaulted column.

RoutePlanRepository insert/conversion glue, in database.dart:

Future<int> insertRoutePlan(domain.RoutePlan route) => into(routePlans).insert(
  RoutePlansCompanion.insert(
    name: route.name, createdAt: route.createdAt, activity: Value(route.activity),
    distanceM: Value(route.distanceM), estimatedMillis: Value(route.estimatedMillis),
    geometry: Value(route.geometry),
  ),
);

Future<void> setRoutePlanDistance(int id, double distanceM) => (update(routePlans)
  ..where((r) => r.id.equals(id))).write(RoutePlansCompanion(distanceM: Value(distanceM)));

domain.RoutePlan _toRoutePlan(RoutePlanRow r) => domain.RoutePlan(
  id: r.id, name: r.name, createdAt: r.createdAt, activity: r.activity,
  distanceM: r.distanceM, estimatedMillis: r.estimatedMillis, geometry: r.geometry,
);

Overflow menu — where the toggle belongs

route_planner_screen.dart, _OverflowMenu (~lines 493-530-ish) already hosts Rename/Download/Delete as a PopupMenuButton inside a GlassPanel:

class _OverflowMenu extends StatelessWidget {
  const _OverflowMenu({
    required this.hasWaypoints, required this.onRename, required this.onDownload, required this.onDelete,
  });

  final bool hasWaypoints;
  final VoidCallback onRename;
  final VoidCallback? onDownload;
  final VoidCallback onDelete;

  @override
  Widget build(BuildContext context) => GlassPanel(
    borderRadius: const BorderRadius.all(Radius.circular(999)),
    child: PopupMenuButton<String>(
      key: const Key('route-overflow-menu'),
      icon: const Icon(Icons.more_vert),
      onSelected: (value) => switch (value) {
        'rename' => onRename(),
        'download' => onDownload?.call(),
        'delete' => onDelete(),
        _ => null,
      },
      itemBuilder: (context) => [
        const PopupMenuItem(key: Key('rename-route'), value: 'rename', child: Text('Rename')),
        ...

And it's constructed at the call site (~lines 298-310):

_OverflowMenu(
  hasWaypoints: waypoints.isNotEmpty,
  onRename: () => _rename(context, repo, route),
  onDownload: waypoints.isEmpty ? null : () => _downloadOfflineTiles(context, waypoints),
  onDelete: () async {
    await repo.deleteRoutePlan(widget.routeId);
    widget.onBack?.call();
  },
),

Design

• Schema: add BoolColumn get isClosedLoop => boolean().withDefault(const Constant(false))(); to RoutePlans in database.dart. Bump schemaVersion from 3 to 4; add if (from < 4) { await m.addColumn(routePlans, routePlans.isClosedLoop); } to onUpgrade.
• Domain model: add final bool isClosedLoop; to RoutePlan (default false in the constructor), thread it through copyWith.
• DB glue: add isClosedLoop: Value(route.isClosedLoop) to insertRoutePlan's RoutePlansCompanion.insert(...) call, isClosedLoop: r.isClosedLoop to _toRoutePlan, and a new method mirroring setRoutePlanDistance:

  Future<void> setRoutePlanClosedLoop(int id, bool value) => (update(routePlans)
    ..where((r) => r.id.equals(id))).write(RoutePlansCompanion(isClosedLoop: Value(value)));
  

• Repository: add to RoutePlanRepository:

  Future<void> setClosedLoop(int routeId, bool value) async {
    await _db.setRoutePlanClosedLoop(routeId, value);
    await _recomputeDistance(routeId); // the closing segment changes the total
  }
  

  Update _recomputeDistance to fetch the route's own flag and append the first waypoint's coordinates when closed, before summing:

  Future<void> _recomputeDistance(int routeId) async {
    final route = await _db.getRoutePlan(routeId);
    final waypoints = await _db.waypointsForRoute(routeId);
    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);
  }
  

  (_recomputeDistance is already called from every waypoint-mutating method — addWaypoint, moveWaypoint, deleteWaypoint, reorderWaypoint — so the closing segment is kept correct automatically as pins are edited, with no other call site changes needed.)
• UI toggle: add a 4th item to _OverflowMenu, enabled only when there are enough waypoints for a loop to mean anything (hasWaypoints alone isn't enough — need at least 2 to form any segment at all; reuse the existing hasWaypoints bool but also thread through whether there are >= 2 waypoints, or simplify by passing a new canCloseLoop: waypoints.length >= 2 param):

  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 canCloseLoop;
    final bool isClosedLoop;
    final VoidCallback onToggleLoop;
  
    @override
    Widget build(BuildContext context) => GlassPanel(
      ...
      child: PopupMenuButton<String>(
        ...
        onSelected: (value) => switch (value) {
          'rename' => onRename(),
          'download' => onDownload?.call(),
          'closeLoop' => onToggleLoop(),
          'delete' => onDelete(),
          _ => null,
        },
        itemBuilder: (context) => [
          const PopupMenuItem(key: Key('rename-route'), value: 'rename', child: Text('Rename')),
          ...
          PopupMenuItem(
            key: const Key('toggle-closed-loop'),
            value: 'closeLoop',
            enabled: canCloseLoop,
            child: Text(isClosedLoop ? 'Open the loop' : 'Close the loop'),
          ),
          ...
        ],
      ),
    );
  }
  

  Wire it at the call site:

  _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 { ... },
  ),
  

• Polyline: append the closing point when the route is closed and there are at least 2 waypoints:

  if (waypoints.length >= 2)
    PolylineLayer(
      polylines: [
        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,
          pattern: StrokePattern.dashed(segments: const [8, 6]),
          color: colors.primary,
        ),
      ],
    ),
  

  Same Polyline object, same styling — just one more point in the list, so the closing segment renders with identical dashed styling to the rest of the route.
• FloatingPill/_initialFit: no changes needed — Distance updates automatically via _recomputeDistance; Pins/Est. Time are correctly unaffected; _initialFit's bounds-fitting is based on waypoint positions regardless of whether the last segment loops back (the closing point is always one of the existing waypoints' own coordinates, already inside the fitted bounds).

Implementation

1. Add isClosedLoop column to RoutePlans in database.dart, bump schemaVersion to 4, add the onUpgrade migration step.
2. Add isClosedLoop field to RoutePlan domain model + copyWith.
3. Add isClosedLoop to insertRoutePlan's companion and _toRoutePlan in database.dart; add setRoutePlanClosedLoop.
4. Add RoutePlanRepository.setClosedLoop; update _recomputeDistance to append the closing point when isClosedLoop.
5. Extend _OverflowMenu with the new toggle item and params; wire it at the call site in route_planner_screen.dart.
6. Extend the PolylineLayer's point list with the conditional closing point.

Acceptance criteria

• "Close the loop" appears in the overflow menu, disabled when there are fewer than 2 waypoints, enabled otherwise.
• Toggling it on redraws the polyline with a visible segment from the last waypoint back to the first, same dashed styling as the rest of the route.
• Toggling it on increases the Distance stat by the length of the new closing segment; toggling it back off returns Distance to the open-path total.
• The menu label reflects current state ("Close the loop" when open, "Open the loop" when already closed).
• Adding/moving/deleting a waypoint on a closed-loop route keeps the closing segment correct automatically (it's recomputed via the existing _recomputeDistance call already present in every waypoint-mutating method).
• Deleting waypoints down to fewer than 2 while closed doesn't crash — the waypoints.length >= 2 guards in both the polyline and distance code make the closing point simply disappear along with the rest of the route drawing, same as today's behavior for an open path with 0-1 waypoints.
• A pre-existing (schema v3) database opens cleanly post-migration with isClosedLoop defaulting to false on every existing route.
• flutter analyze clean, flutter test green, test count only goes up.

Tests

• Migration test (test/migration_test.dart, following the exact pattern of the existing 'a v2 database (V3-07) gains route_plans/waypoints and keeps its trips' test): seed a v3 database (v1 seed + activity column + route_plans/waypoints tables created via raw SQL, PRAGMA user_version = 3;), open it with AppDatabase, insert a route via db.insertRoutePlan, and assert isClosedLoop reads back false by default, and that toggling it via setRoutePlanClosedLoop persists.
• Repository tests (test/route_plan_repository_test.dart, alongside the existing 'adding waypoints appends in order and updates distance live' etc.):
  • setClosedLoop(true) on a route with 2+ waypoints increases distanceM by exactly the closing segment's length (compute the expected delta directly with geo.pathLengthMeters/a manual haversine call between the last and first waypoint, matching how other distance tests in this file already assert exact values).
  • setClosedLoop(false) after true returns distanceM to the pre-toggle value.
  • Adding a waypoint to an already-closed route keeps the distance correct (recomputed including the new closing segment against the new last-added point, not the old one).
• Widget tests (test/route_planner_screen_test.dart):
  • The "Close the loop" menu item is disabled with 0-1 waypoints, enabled with 2+ (mirrors the existing "offline-tiles download menu item is disabled with no pins" test's structure — open the overflow menu, inspect the PopupMenuItem.enabled property via its key).
  • Tapping it toggles route.isClosedLoop (assert via repo.routePlanById(id)?.isClosedLoop, the same pattern the existing rename test uses to verify writes through to the repository) and the label switches to "Open the loop".
  • With the loop closed, find.byType(Polyline) (or inspecting the PolylineLayer's polylines list directly, matching whatever existing pattern this test file uses to inspect polyline data) has one more point than the waypoint count.

Risks

• None significant — this is additive (new column, new optional toggle) and every touched call site (_recomputeDistance) already runs on every mutation, so there's no new code path that could silently skip recomputation.

Out of scope

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).