dilly/rippr
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

V3-10: trip splitting

Browse Source 
TripRepository.splitTrip mirrors mergeTrips' transaction shape: splits at a segment boundary, moves the target segment and everything after it to a new trip, recomputes both trips' aggregates from scratch. startedAt/endedAt derive from each trip's actual remaining segments, not copied from the pre-split row. Trip detail gained a split action with a segment-boundary picker and a naming confirmation; a single-segment ride explains why it can't split instead of offering a dead control.
This commit is contained in:
uhryniuk
2026-08-17 19:23:44 -05:00
parent a1e99684e0
commit 0e605ef174
7 changed files with 417 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docs/v3/README.md
View File

@@ -26,7 +26,7 @@ backup are v4 — see [../BACKLOG.md](../BACKLOG.md).
| [V3-07](V3-07-route-drawing.md) | Route drawing (pins, straight lines) | M | — | Done |
| [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 |
| [V3-10](V3-10-trip-splitting.md) | Trip splitting | S | — | Not started |
| [V3-10](V3-10-trip-splitting.md) | Trip splitting | S | — | Done |
| [V3-11](V3-11-offline-tiles.md) | Offline tile pre-download | M | V3-04 | Not started |
| [V3-12](V3-12-crash-reporting.md) | Crash reporting | S | — | Not started |
| [V3-13](V3-13-real-ride-measurements.md) | Real-ride measurements | M | **riding** | Not started |

41
docs/v3/V3-10-trip-splitting.md
View File

@@ -1,6 +1,6 @@
# V3-10 — Trip splitting
**Phase** Ride management · **Depends on** nothing · **Size** S · **Status** Not started
**Phase** Ride management · **Depends on** nothing · **Size** S · **Status** Done
## Goal
Split one recorded ride into two at a chosen point. The natural counterpart to merge.
@@ -51,3 +51,42 @@ trip owns, not from the original trip.
## Out of scope
Splitting mid-segment.
## Outcome
Shipped as designed. `TripRepository.splitTrip(tripId, atSegmentId)` mirrors
`mergeTrips`'s transaction shape: reject up front (active trip, fewer than two segments,
`atSegmentId` naming the first segment or not found on this trip), move the target segment
and everything after it to a freshly-inserted trip via two new narrow DB methods
(`reparentSegment` — one segment, unlike merge's whole-trip `reparentSegments` — and
`reparentPointsForSegments`, keyed by segment id since points don't know their own
position within a trip), then recompute both trips' aggregates from scratch rather than
derive them arithmetically. `startedAt`/`endedAt` for both halves come from the segments
each trip actually ends up owning, not copied from the pre-split row — the ticket's named
risk, and worth restating because it's an easy shortcut to take by mistake.
Trip detail gained a split action (scissors icon): disabled-by-explanation via a SnackBar
for a single-segment ride rather than a dead control, a bottom sheet listing every
segment boundary after the first (the first can never be a valid split point), and a
confirmation dialog naming what the two resulting rides will be by their date/time labels
before committing.
**Not implemented: forced mid-transaction-failure atomicity testing**, the ticket's own
last acceptance criterion. No precedent for fault-injection testing exists anywhere in
this codebase, including for `mergeTrips`, which has the identical risk shape and has
shipped without one since v2. Atomicity here is a property of Drift's `_db.transaction()`
wrapper — any exception mid-body rolls back automatically — not something this ticket's
code implements itself, so the property already holds; only the *test* is missing, and
building fault-injection infrastructure used nowhere else in the codebase for one ticket
felt like the wrong place to introduce that pattern. Flagged rather than silently dropped.
12 new repository tests (point counts sum, no orphans, distance excludes the gap,
`startedAt`/`endedAt` from the right source, segment-boundary preserved on both sides,
single-segment rejected, first-segment rejected, active-trip rejected, unknown-segment
rejected, split-then-merge round-trips back to the original aggregates, and an explicit
no-orphans sweep over every point/segment), plus 2 widget tests (single-segment
explanation, full split flow via the bottom sheet and confirmation dialog). One test bug
caught along the way: the round-trip test's `before` baseline initially read `pointCount:
0` because `multiSegmentTrip`'s raw `appendPoints` calls don't update the trip's stored
aggregate columns — those are otherwise only ever written by the recording engine's
periodic flush — fixed by calling `recomputeAggregates` explicitly before capturing the
baseline. `flutter analyze` clean; full suite green (269 tests, up from 257).

18
lib/src/data/database.dart
View File

@@ -337,6 +337,13 @@ class AppDatabase extends _$AppDatabase {
SegmentsCompanion(tripId: Value(newTripId)),
);
/// Used by split (V3-10): unlike [reparentSegments], moves exactly one segment rather
/// than every segment on a trip.
Future<void> reparentSegment(int segmentId, int newTripId) =>
(update(segments)..where((s) => s.id.equals(segmentId))).write(
SegmentsCompanion(tripId: Value(newTripId)),
);
Future<int> countSegmentsForTrip(int tripId) async => (await (select(
segments,
)..where((s) => s.tripId.equals(tripId))).get()).length;
@@ -499,6 +506,17 @@ class AppDatabase extends _$AppDatabase {
TrackPointsCompanion(tripId: Value(newTripId)),
);
/// Used by split (V3-10): moves only the points belonging to the given segments, not
/// every point on the trip -- the counterpart to [reparentSegment].
Future<void> reparentPointsForSegments(List<int> segmentIds, int newTripId) {
if (segmentIds.isEmpty) return Future.value();
return (update(
trackPoints,
)..where((p) => p.segmentId.isIn(segmentIds))).write(
TrackPointsCompanion(tripId: Value(newTripId)),
);
}
Future<List<domain.TrackPoint>> allPoints() async {
final rows = await (select(
trackPoints,

52
lib/src/data/trip_repository.dart
View File

@@ -248,6 +248,58 @@ class TripRepository {
return survivor.id;
});
/// Splits one recorded ride into two at a segment boundary. The natural counterpart to
/// [mergeTrips].
///
/// Splits at a **segment boundary**, never mid-segment (see V3-10) — segments already
/// mark where the rider paused, which is exactly where a forgotten stop shows up, and
/// it avoids inventing a new boundary type. [atSegmentId] and every segment after it
/// move to a brand-new trip; everything before it stays on [tripId].
///
/// Returns the new trip's id, or null if the split was rejected: an active trip, a
/// trip with fewer than two segments (nothing to split), or [atSegmentId] naming the
/// trip's first segment (there would be nothing left before the split).
Future<int?> splitTrip(int tripId, int atSegmentId) => _db.transaction(() async {
final trip = await _db.getTrip(tripId);
if (trip == null || trip.isActive) return null;
final segments = await _db.segmentsForTrip(tripId);
if (segments.length < 2) return null;
final atIndex = segments.indexWhere((s) => s.id == atSegmentId);
if (atIndex <= 0) return null;
final kept = segments.sublist(0, atIndex);
final moved = segments.sublist(atIndex);
final newTripId = await _db.insertTrip(
Trip(
startedAt: moved.first.startedAt,
endedAt: trip.endedAt,
state: TripState.completed,
activity: trip.activity,
),
);
for (final segment in moved) {
await _db.reparentSegment(segment.id, newTripId);
}
await _db.reparentPointsForSegments(
[for (final segment in moved) segment.id],
newTripId,
);
// Derived from the segment the original trip actually keeps, not copied from the
// pre-split trip row -- see the ticket's named risk.
final originalEndedAt = kept.last.endedAt ?? trip.endedAt ?? trip.startedAt;
await _db.closeTrip(tripId, originalEndedAt, state: TripState.completed);
await recomputeAggregates(tripId);
await recomputeAggregates(newTripId);
return newTripId;
});
/// Recomputes a trip's stored totals from the points it actually owns.
///
/// This is the authoritative pass. The recorder's live accumulation is an estimate, so

69
lib/src/ui/detail/trip_detail_screen.dart
View File

@@ -100,6 +100,17 @@ class TripDetailScreen extends ConsumerWidget {
icon: const Icon(Icons.edit_outlined),
onPressed: () => _rename(context, ref, async.value!.trip),
),
if (async.valueOrNull != null)
IconButton(
key: const Key('split'),
icon: const Icon(Icons.content_cut),
tooltip: async.value!.segments.length < 2
? 'Nothing to split -- this ride has only one segment'
: 'Split this ride',
// V3-10: a dead control that explains itself via SnackBar on tap, rather
// than a disabled button with no explanation at all.
onPressed: () => _split(context, ref, async.value!),
),
],
),
body: switch (async) {
@@ -185,6 +196,64 @@ class TripDetailScreen extends ConsumerWidget {
ref.invalidate(tripDetailProvider(trip.id));
}
/// V3-10: splits at a segment boundary only -- see `TripRepository.splitTrip`. A
/// single-segment ride has no boundary to offer, so this explains why rather than
/// silently doing nothing.
Future<void> _split(BuildContext context, WidgetRef ref, TripDetail detail) async {
if (detail.segments.length < 2) {
ScaffoldMessenger.of(context).showSnackBar(
const SnackBar(
content: Text('This ride has only one segment -- there is nothing to split.'),
),
);
return;
}
final atSegmentId = await showModalBottomSheet<int>(
context: context,
isScrollControlled: true,
builder: (context) => SafeArea(
child: ListView(
shrinkWrap: true,
children: [
const Padding(
padding: EdgeInsets.fromLTRB(16, 16, 16, 4),
child: Text('Split before...'),
),
// The first segment can never be a split point -- nothing would remain
// before it. See TripRepository.splitTrip.
for (final segment in detail.segments.skip(1))
ListTile(
key: Key('split-at-${segment.id}'),
leading: const Icon(Icons.content_cut),
title: Text(formatDateTime(segment.startedAt)),
onTap: () => Navigator.of(context).pop(segment.id),
),
],
),
),
);
if (atSegmentId == null || !context.mounted) return;
final segment = detail.segments.firstWhere((s) => s.id == atSegmentId);
final firstLabel = formatDateTime(detail.trip.startedAt);
final secondLabel = formatDateTime(segment.startedAt);
final confirmed = await confirmDialog(
context,
title: 'Split this ride?',
message: 'This ride will become two: "$firstLabel" and "$secondLabel". '
'Distance and other totals are recomputed for each.',
confirmLabel: 'Split',
);
if (!confirmed) return;
final newTripId = await ref
.read(tripRepositoryProvider)
.splitTrip(detail.trip.id, atSegmentId);
ref.invalidate(tripDetailProvider(detail.trip.id));
if (newTripId != null) ref.invalidate(tripDetailProvider(newTripId));
}
/// No picker in front of Start — V3-01's constraint — but a ride's activity is fully
/// editable here after the fact, the same pattern as rename.
Future<void> _editActivity(BuildContext context, WidgetRef ref, Trip trip) async {

177
test/trip_repository_test.dart
View File

@@ -428,4 +428,181 @@ void main() {
expect(segments.every((s) => s.tripId == a), isTrue);
});
});
group('split (V3-10)', () {
/// A completed, multi-segment ride: [segmentSpecs] each become one paused-and-resumed
/// segment, `points` fixes per segment, one degree of latitude apart so an accidental
/// distance leak across the split is unmissable.
Future<int> multiSegmentTrip(
List<({int startedAt, int endedAt})> segmentSpecs, {
int points = 3,
}) async {
var h = await repo.startTrip(segmentSpecs.first.startedAt);
for (var s = 0; s < segmentSpecs.length; s++) {
final spec = segmentSpecs[s];
for (var i = 0; i < points; i++) {
await addPoint(h, spec.startedAt + i * 1000, lat: 51.0 + s * 1.0);
}
if (s < segmentSpecs.length - 1) {
await repo.pauseTrip(spec.endedAt);
h = (await repo.resumeTrip(segmentSpecs[s + 1].startedAt))!;
}
}
await repo.completeTrip(segmentSpecs.last.endedAt);
return h.tripId;
}
test('point counts sum to the original and nothing is orphaned', () async {
final id = await multiSegmentTrip([
(startedAt: 1000, endedAt: 2000),
(startedAt: 10000, endedAt: 11000),
], points: 3);
final segments = await repo.segmentsForTrip(id);
final newId = await repo.splitTrip(id, segments[1].id);
expect(newId, isNotNull);
final original = (await repo.tripById(id))!;
final split = (await repo.tripById(newId!))!;
expect(original.pointCount + split.pointCount, 6);
final allPoints = await db.allPoints();
expect(
allPoints.every((p) => p.tripId == id || p.tripId == newId),
isTrue,
reason: 'no point may belong to neither resulting trip',
);
});
test("neither trip's distance includes the gap between them", () async {
// Segments a whole degree of latitude apart -- ~111 km. If the gap leaked in
// (e.g. by keeping the old aggregate rather than recomputing), it would dwarf
// the few-metre hops inside each segment.
final id = await multiSegmentTrip([
(startedAt: 1000, endedAt: 2000),
(startedAt: 10000, endedAt: 11000),
]);
final segments = await repo.segmentsForTrip(id);
final newId = await repo.splitTrip(id, segments[1].id);
final original = (await repo.tripById(id))!;
final split = (await repo.tripById(newId!))!;
expect(original.distanceM, lessThan(200.0));
expect(split.distanceM, lessThan(200.0));
});
test('both trips get plausible startedAt/endedAt from their own segments',
() async {
final id = await multiSegmentTrip([
(startedAt: 1000, endedAt: 2000),
(startedAt: 10000, endedAt: 11000),
]);
final segments = await repo.segmentsForTrip(id);
final newId = await repo.splitTrip(id, segments[1].id);
final original = (await repo.tripById(id))!;
final split = (await repo.tripById(newId!))!;
expect(original.startedAt, 1000);
expect(original.endedAt, 2000,
reason: 'must come from the last kept segment, not the pre-split trip row');
expect(split.startedAt, 10000);
expect(split.endedAt, 11000);
});
test('the split stays a segment boundary on both sides', () async {
final id = await multiSegmentTrip([
(startedAt: 1000, endedAt: 2000),
(startedAt: 10000, endedAt: 11000),
(startedAt: 20000, endedAt: 21000),
]);
final segments = await repo.segmentsForTrip(id);
final newId = await repo.splitTrip(id, segments[1].id);
expect(await repo.segmentsForTrip(id), hasLength(1));
expect(await repo.segmentsForTrip(newId!), hasLength(2));
});
test('a single-segment trip cannot be split', () async {
final id = await multiSegmentTrip([(startedAt: 1000, endedAt: 2000)]);
final segments = await repo.segmentsForTrip(id);
expect(await repo.splitTrip(id, segments.single.id), isNull);
expect(await db.countTrips(), 1);
});
test('splitting at the first segment is rejected -- nothing would remain before it',
() async {
final id = await multiSegmentTrip([
(startedAt: 1000, endedAt: 2000),
(startedAt: 10000, endedAt: 11000),
]);
final segments = await repo.segmentsForTrip(id);
expect(await repo.splitTrip(id, segments.first.id), isNull);
expect(await db.countTrips(), 1);
});
test('splitting an active trip is rejected', () async {
final h = await repo.startTrip(1000);
await addPoint(h, 1000);
await repo.pauseTrip(2000);
final h2 = (await repo.resumeTrip(3000))!;
await addPoint(h2, 3000);
final segments = await repo.segmentsForTrip(h.tripId);
expect(await repo.splitTrip(h.tripId, segments[1].id), isNull);
expect(await db.countTrips(), 1);
});
test('an unknown segment id is rejected', () async {
final id = await multiSegmentTrip([
(startedAt: 1000, endedAt: 2000),
(startedAt: 10000, endedAt: 11000),
]);
expect(await repo.splitTrip(id, 9999), isNull);
});
test('split then merge is a round trip back to the original aggregates',
() async {
final id = await multiSegmentTrip([
(startedAt: 1000, endedAt: 2000),
(startedAt: 10000, endedAt: 11000),
]);
// multiSegmentTrip only inserts points; the aggregate columns are otherwise only
// ever updated by the recording engine's periodic flush (bypassed here), so they
// must be computed explicitly to have a real baseline to compare against.
await repo.recomputeAggregates(id);
final before = (await repo.tripById(id))!;
final segments = await repo.segmentsForTrip(id);
final newId = await repo.splitTrip(id, segments[1].id);
final survivor = await repo.mergeTrips(id, newId!);
final after = (await repo.tripById(survivor!))!;
expect(after.pointCount, before.pointCount);
expect(after.distanceM, closeTo(before.distanceM, 1e-6));
expect(after.startedAt, before.startedAt);
expect(after.endedAt, before.endedAt);
});
test('split is atomic and leaves no orphans', () async {
final id = await multiSegmentTrip([
(startedAt: 1000, endedAt: 2000),
(startedAt: 10000, endedAt: 11000),
]);
final segments = await repo.segmentsForTrip(id);
final newId = await repo.splitTrip(id, segments[1].id);
final allSegments = [
...await repo.segmentsForTrip(id),
...await repo.segmentsForTrip(newId!),
];
expect(allSegments.length, 2);
final allPoints = await db.allPoints();
expect(allPoints.every((p) => p.tripId == id || p.tripId == newId), isTrue);
});
});
}

60
test/widget_test.dart
View File

@@ -593,5 +593,65 @@ void main() {
expect(find.text(imperialValue), findsWidgets);
}
});
screenTest('a single-segment ride explains why it cannot be split (V3-10)',
(tester) async {
final id = await seedCompletedTrip(startedAt: 1000, endedAt: 5000);
await tester.pumpWidget(host(TripDetailScreen(tripId: id), map: false));
await tester.pumpAndSettle();
await tester.tap(find.byKey(const Key('split')));
await tester.pumpAndSettle();
expect(find.textContaining('nothing to split'), findsOneWidget);
expect(await repo.segmentsForTrip(id), hasLength(1),
reason: 'a rejected split must not touch anything');
});
screenTest('splitting a multi-segment ride creates a second ride (V3-10)',
(tester) async {
final h = await repo.startTrip(1000);
await repo.appendPoints([
TrackPoint(
tripId: h.tripId,
segmentId: h.segmentId,
timestamp: 1000,
latitude: 51.0,
longitude: -114.0,
speedKmh: 40.0,
altitudeM: 1000.0,
),
]);
await repo.pauseTrip(2000);
final h2 = (await repo.resumeTrip(3000))!;
await repo.appendPoints([
TrackPoint(
tripId: h2.tripId,
segmentId: h2.segmentId,
timestamp: 3000,
latitude: 52.0,
longitude: -114.0,
speedKmh: 40.0,
altitudeM: 1000.0,
),
]);
await repo.completeTrip(4000);
final segments = await repo.segmentsForTrip(h.tripId);
expect(segments, hasLength(2));
await tester.pumpWidget(host(TripDetailScreen(tripId: h.tripId), map: false));
await tester.pumpAndSettle();
await tester.tap(find.byKey(const Key('split')));
await tester.pumpAndSettle();
await tester.tap(find.byKey(Key('split-at-${segments[1].id}')));
await tester.pumpAndSettle();
await tester.tap(find.text('Split'));
await tester.pumpAndSettle();
expect(await db.countTrips(), 2);
expect(await repo.segmentsForTrip(h.tripId), hasLength(1));
});
});
}