uhryniuk
0e605ef174
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.
93 lines
5.0 KiB
Markdown
93 lines
5.0 KiB
Markdown
# V3-10 — Trip splitting
|
|
|
|
**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.
|
|
|
|
## Context
|
|
Merge exists and is well tested; split does not. The case is a rider who forgot to stop —
|
|
one "ride" that is really the trip out, lunch, and the trip home.
|
|
|
|
Merge already establishes the hard parts: re-parenting points and segments inside a
|
|
transaction, and recomputing aggregates rather than summing them.
|
|
|
|
## Design
|
|
Split at a **segment boundary** rather than an arbitrary point. Segments already mark where
|
|
the rider paused, which is exactly where a forgotten stop shows up — and it avoids
|
|
inventing a new boundary type or splitting a segment in half.
|
|
|
|
The original trip keeps the earlier segments; a new trip takes the later ones. Both get
|
|
aggregates recomputed from the points they actually own.
|
|
|
|
If a ride has only one segment there is nothing to split, and the UI should say so rather
|
|
than offering a dead control.
|
|
|
|
## Implementation
|
|
1. `TripRepository.splitTrip(tripId, atSegmentId)` inside a transaction:
|
|
create the new trip, re-parent segments and points from `atSegmentId` onward,
|
|
set `startedAt`/`endedAt` from the segments each trip now owns,
|
|
recompute aggregates for both
|
|
2. Trip detail: a split action listing segment boundaries with their times
|
|
3. Confirmation naming what the two resulting rides will be
|
|
|
|
## Acceptance criteria
|
|
- [ ] Splitting produces two trips whose point counts sum to the original
|
|
- [ ] Neither trip's distance includes the gap between them
|
|
- [ ] Both have plausible `startedAt`/`endedAt`
|
|
- [ ] Single-segment rides cannot be split, and the UI explains why
|
|
- [ ] Atomic — a failure part-way leaves the original intact
|
|
|
|
## Tests
|
|
- Point counts sum; no points orphaned
|
|
- Distance of the parts is less than the original by roughly the gap
|
|
- Split then merge returns to the original aggregates — a good round-trip property
|
|
- Rejects a single-segment trip
|
|
- Atomicity under a forced mid-transaction failure
|
|
|
|
## Risks
|
|
Getting `startedAt`/`endedAt` from the wrong source. Derive them from the segments each
|
|
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).
|