Session expiry, arrival geofence guard, multi-destination stops
Three fixes found by running the app on a real handset against production. 1. An expired token left the app looking signed in and unable to work. MilerApi.onUnauthorized was declared and called on every 401 but never assigned, so the token was dropped and nothing else happened: the profile stayed on disk, logged_out stayed false, and the rider saw his own name over a dashboard whose every call returned 401. He reads that as "no work today". The teardown now lives in endSession() and both ways out of a session — the Log out button and the 401 path — use it. 2. Arrived was written locally even when the rider was not there. updateArrivedStatus answers false for three different things and the caller treated all of them as "the write did not land", which is only true of one. A geofence refusal and a server refusal now stop the rung and hand back the reason; a dead network still advances, as it should. 3. A multi-destination customer pickup collapsed onto one stop. GET /miler/bookings returns a row per destination once collected, all with the same bookingid and reference. Every local store keys on that id, so the accepted store deduped two of three drops away and their consignment ids were unrecoverable. orderid is now the stop key; bookingreference stays the booking's name. Cards show "Stop 2 of 3" and the receiver's own name and number rather than the sender's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EqVJPB9B4QuieZnBAAKgYQ
This commit is contained in:
@@ -193,6 +193,11 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
@override
|
||||
void initState() {
|
||||
super.initState();
|
||||
// Which leg this stop is on. Async because it reads the pivot store, so the
|
||||
// sheet opens in its delivery shape and corrects itself a frame later —
|
||||
// the base CTA is behind a slide, so there is no window in which the rider
|
||||
// can commit the wrong rung.
|
||||
unawaited(_resolveHubLeg());
|
||||
_camera = MilerMapCamera(controller: _mapController, vsync: this);
|
||||
_routeAnim =
|
||||
AnimationController(
|
||||
@@ -1360,6 +1365,73 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
collectedIds: widget.parentState?._collectedIds ?? const <String>{},
|
||||
).isDelivery;
|
||||
|
||||
/// True when this stop ends at a base rather than at a receiver's door.
|
||||
///
|
||||
/// Read from the row's own `next_action`, never from the rider's line — the
|
||||
/// same resolver [handOverAtHub] and the Deliveries queue use, so the card,
|
||||
/// the sheet and the write cannot disagree about where a parcel is going.
|
||||
///
|
||||
/// Held in state rather than computed in `build` because resolving it reads
|
||||
/// the pivot store, which is async.
|
||||
bool _isHubLeg = false;
|
||||
|
||||
/// The base this stop is handed in at, once [_resolveHubLeg] has looked.
|
||||
HandoverHub? _handoverBase;
|
||||
|
||||
/// Resolves [_isHubLeg] once, on open.
|
||||
///
|
||||
/// Silent on failure: a stop whose leg cannot be read falls back to the
|
||||
/// delivery shape it had before this existed, which is the behaviour every
|
||||
/// other screen already has.
|
||||
Future<void> _resolveHubLeg() async {
|
||||
try {
|
||||
final leg = NextLegResolver.resolve(
|
||||
widget.pickup,
|
||||
pivotAction:
|
||||
(await getPivotNextActions())[MilkRun.idOf(widget.pickup)] ?? '',
|
||||
);
|
||||
if (!mounted) return;
|
||||
final base = leg.isHub ? handoverBaseFor(widget.pickup) : null;
|
||||
setState(() {
|
||||
_isHubLeg = leg.isHub;
|
||||
_handoverBase = base;
|
||||
// ── The destination is the base, not the door he collected at ──
|
||||
//
|
||||
// `MilkRun.navigatesToCustomer` is false for a base leg — correctly, a
|
||||
// base-routed parcel must never point at its receiver 500 km away — so
|
||||
// `_pickupLocation` fell back to the **pickup** coordinates and the map
|
||||
// sent the rider back to the customer he had just collected from.
|
||||
//
|
||||
// A base with no coordinates is left alone: navigating to `0, 0` lands
|
||||
// in the Gulf of Guinea. The handover is still refused by the fence in
|
||||
// that case, with a sentence pointing at the office.
|
||||
if (base != null && base.isNavigable) {
|
||||
_pickupLocation = LatLng(base.latitude, base.longitude);
|
||||
}
|
||||
});
|
||||
} catch (e) {
|
||||
debugPrint('[HANDOVER] could not resolve the leg: $e');
|
||||
}
|
||||
}
|
||||
|
||||
/// Hands the parcel in at the base and closes the stop.
|
||||
///
|
||||
/// The base-routed twin of [_markDelivered]: same shape, different rung. It
|
||||
/// does not open [DeliveryProofPage] — there is no receiver to photograph and
|
||||
/// no OTP to take; the evidence the contract asks for is the position, which
|
||||
/// [handOverAtHub] stamps from the fix the fence judged.
|
||||
Future<void> _handOverAtBase() async {
|
||||
if (_isNavigating || !mounted) return;
|
||||
HapticFeedback.mediumImpact();
|
||||
// Straight through the one close path. [closeDelivery] routes
|
||||
// `handedOver` to [handOverAtHub] — which takes the fence, resolves the
|
||||
// consignment and posts `inward-at-hub` — and then files the same finished
|
||||
// record every other outcome files, so Activity, the carried set and the
|
||||
// queue all see one kind of closed stop. Calling [handOverAtHub] here as
|
||||
// well would post the handover twice.
|
||||
await _closeDelivery(DeliveryOutcome.handedOver);
|
||||
}
|
||||
|
||||
/// True once the rider has set off from **this screen** — slid Start
|
||||
/// delivery, or opened navigation on the collection leg.
|
||||
///
|
||||
@@ -1447,6 +1519,30 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
// [MilkRun.deliveryArrivalIsLocalOnly]), so a button for it would write
|
||||
// nothing. Once he sets off the stop is simply **active**, and the only
|
||||
// question left is whether it was handed over.
|
||||
// ── A base handover is its own rung ──
|
||||
//
|
||||
// It is not a delivery: there is no receiver, no proof photo, no OTP and no
|
||||
// "customer was out" outcome. It is also not nothing, which is what the app
|
||||
// offered before — [releaseForDelivery] refuses a base leg, correctly, and
|
||||
// the rider was left holding a parcel with a message telling him to hand it
|
||||
// over and no control that would record it.
|
||||
if (_isHubLeg) {
|
||||
final onTheRoad = _setOff;
|
||||
// Named where it is known. "Handed over at Coimbatore Base" is a rider
|
||||
// confirming a place; "Handed over at base" is him confirming a category,
|
||||
// and on a round with two bases that is the difference between a parcel
|
||||
// arriving and a parcel being looked for.
|
||||
final base = _handoverBase?.name ?? '';
|
||||
return MilerSlideAction(
|
||||
label: onTheRoad
|
||||
? (base.isEmpty ? 'Handed over at base' : 'Handed over at $base')
|
||||
: 'Slide to start ride',
|
||||
icon: onTheRoad ? LucideIcons.check : LucideIcons.chevronRight,
|
||||
color: ColorConstants.acceptGreen,
|
||||
onCommit: onTheRoad ? _handOverAtBase : _startRideToBase,
|
||||
);
|
||||
}
|
||||
|
||||
if (_isDeliveryLeg) {
|
||||
final onTheRoad = _setOff;
|
||||
final slide = MilerSlideAction(
|
||||
@@ -1572,6 +1668,16 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
return false;
|
||||
}
|
||||
|
||||
// ── Not refused, but not recorded either ──
|
||||
//
|
||||
// The rung advances locally below, which is right — a dead network must
|
||||
// not strand a rider at a door. What was wrong is that it advanced
|
||||
// *silently*: the rider saw ARRIVED and could not tell the difference
|
||||
// between a stop the hub knows about and one it does not.
|
||||
if (!ok && dc.lastArrivalNotice != null && mounted) {
|
||||
AppFeedback.warn(context, dc.lastArrivalNotice!);
|
||||
}
|
||||
|
||||
// The rung the rider sees, on the row he is looking at and in the store
|
||||
// that survives the refresh this flow triggers. `reached` does not
|
||||
// persist on every deployment yet — see `getArrivedOrderIds` — and the
|
||||
@@ -1607,8 +1713,100 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
/// here — no photo, a dead signature, a refused PUT — resolves to an empty
|
||||
/// string, which is what this call site was sending unconditionally until
|
||||
/// now. The local copy stays on the device either way.
|
||||
/// The parcel photograph's storage references, once uploaded. Null until it
|
||||
/// has been, and after a failed attempt — a failure is not a photograph.
|
||||
UploadRef? _parcelProof;
|
||||
|
||||
/// Uploads the door photograph once per stop.
|
||||
///
|
||||
/// Best-effort by design: a dead object store must not strand a rider at a
|
||||
/// customer's door holding parcels he cannot record. When it fails the parcel
|
||||
/// update goes without `photos` — which is honest, and better than an empty
|
||||
/// array claiming no photograph was taken.
|
||||
Future<void> _ensureParcelProofUploaded(int bookingId) async {
|
||||
if (_parcelProof != null || bookingId <= 0) return;
|
||||
final path = (_verificationProofPath ?? '').trim();
|
||||
if (path.isEmpty) return;
|
||||
|
||||
final file = File(path);
|
||||
if (!file.existsSync()) return;
|
||||
|
||||
// Kept on the device first, whatever the network does — see ProofStore.
|
||||
try {
|
||||
final orderId = (widget.pickup['orderid'] ?? '').toString();
|
||||
if (orderId.isNotEmpty) await ProofStore.save(orderId, path);
|
||||
} catch (e) {
|
||||
debugPrint('[PICKUP] could not keep a local copy of the proof: $e');
|
||||
}
|
||||
|
||||
try {
|
||||
_parcelProof = await MilerApi.uploadProofRef(
|
||||
file,
|
||||
purpose: MilerApi.proofPickup,
|
||||
// The BOOKING id, in the booking field. There is no consignment yet —
|
||||
// `pickup-complete` has not run — and this used to pass the booking id
|
||||
// in the `consignmentid` slot, filing every pickup proof under a
|
||||
// consignment number that did not exist.
|
||||
bookingId: bookingId,
|
||||
);
|
||||
} catch (e) {
|
||||
debugPrint('[PICKUP] parcel proof upload failed: $e');
|
||||
}
|
||||
|
||||
if (_parcelProof == null || !_parcelProof!.hasKey) {
|
||||
ApiConfig.logGap(
|
||||
'uploads/sign',
|
||||
'the parcel photo for booking $bookingId could not be uploaded (or the '
|
||||
'server returned no key); the parcels are being recorded without '
|
||||
'one and the copy stays on the device.',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The local path of the photograph the verification screen took, if any.
|
||||
String? _verificationProofPath;
|
||||
|
||||
/// Copies the uploaded key onto the verification map the provider reads.
|
||||
Map<String, dynamic> _withParcelPhotos(Map<String, dynamic> verification) {
|
||||
final key = _parcelProof?.key ?? '';
|
||||
if (key.isEmpty) return verification;
|
||||
final pickup = verification['pickup'];
|
||||
if (pickup is! Map) return verification;
|
||||
return {
|
||||
...verification,
|
||||
'pickup': {...pickup, 'photos': <String>[key]},
|
||||
};
|
||||
}
|
||||
|
||||
Future<String> _uploadParcelProof(String path, int bookingId) async {
|
||||
if (path.isEmpty || bookingId <= 0) return '';
|
||||
|
||||
// Already sent, on the way to `POST /parcel`. Pushing the same bytes twice
|
||||
// costs a rider at a doorstep several seconds on a mobile connection and
|
||||
// leaves two copies of one photograph in the bucket.
|
||||
await _ensureParcelProofUploaded(bookingId);
|
||||
if (_parcelProof != null) return _parcelProof!.url;
|
||||
|
||||
// ── Kept on the device first, whatever the network does ──
|
||||
//
|
||||
// The uploaded URL goes nowhere: `pickup-complete` takes latitude and
|
||||
// longitude and nothing else, so even a successful upload leaves the hub
|
||||
// with no reference to the photograph (see BE-2, and the gap logged in
|
||||
// `UpdatePickupProvider`). The rider was being made to photograph the
|
||||
// parcels — the flow will not let him confirm without it — for a record
|
||||
// that existed only in a bucket nobody could search.
|
||||
//
|
||||
// A local copy against the order id is the one form of this evidence that
|
||||
// is actually retrievable today: the office rings the rider and he has it.
|
||||
// Best-effort, and deliberately before the upload, because the case where
|
||||
// it matters most is the one where the network is the problem.
|
||||
try {
|
||||
final orderId = (widget.pickup['orderid'] ?? '').toString();
|
||||
if (orderId.isNotEmpty) await ProofStore.save(orderId, path);
|
||||
} catch (e) {
|
||||
debugPrint('[PICKUP] could not keep a local copy of the proof: $e');
|
||||
}
|
||||
|
||||
try {
|
||||
final url = await MilerApi.uploadProof(
|
||||
File(path),
|
||||
@@ -1686,6 +1884,22 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
await _openGoogleMapsNavigation();
|
||||
}
|
||||
|
||||
/// Sets off for the base.
|
||||
///
|
||||
/// Deliberately **not** [_startDelivery]. `start-delivery` moves the
|
||||
/// consignment to `Out_for_Delivery` against a receiver in another district;
|
||||
/// the hub then sees a rider actively delivering a parcel that is going onto
|
||||
/// a line-haul truck, and there is no way back from that state on the
|
||||
/// handset. A base leg has no release rung at all — the rider simply rides
|
||||
/// there, and the next thing the server hears is the handover.
|
||||
Future<void> _startRideToBase() async {
|
||||
if (_isNavigating || !mounted) return;
|
||||
setState(() => _setOff = true);
|
||||
HapticFeedback.mediumImpact();
|
||||
_hasOpenedNavigation = false;
|
||||
await _openGoogleMapsNavigation();
|
||||
}
|
||||
|
||||
/// Records a stop that could not be handed over.
|
||||
///
|
||||
/// The same chooser the proof page shows, reached without having to slide
|
||||
@@ -1845,7 +2059,15 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
|
||||
Map<String, dynamic> verifyResult = <String, dynamic>{'verified': true};
|
||||
|
||||
if (ServiceProfile.active.needsVerification) {
|
||||
// ── Asked of the stop, not of the rider ──
|
||||
//
|
||||
// `ServiceProfile.active.needsVerification` gives the same answer for
|
||||
// every row on the screen. On a meal tenant that walked a CX customer
|
||||
// pickup straight past the door flow — no parcel count, no destination,
|
||||
// no weight — and the booking pivoted on whatever the customer typed into
|
||||
// the app days earlier. [WorkPolicy] asks the row, and falls back to the
|
||||
// line only where the row says nothing.
|
||||
if (WorkPolicy.capturesShipmentDetails(widget.pickup)) {
|
||||
final verified = await openScreen<Map<String, dynamic>>(
|
||||
context,
|
||||
StopVerificationPage(pickup: widget.pickup),
|
||||
@@ -1870,7 +2092,7 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
// customer. Its answers ride along in the same map, and
|
||||
// `_startPickupNavigation` sends them — addresses first, because
|
||||
// `pickup-complete` routes the consignment from them.
|
||||
if (ServiceProfile.active.capturesShipmentAddresses) {
|
||||
if (WorkPolicy.capturesShipmentAddresses(widget.pickup)) {
|
||||
final seeded = Map<String, dynamic>.from(widget.pickup);
|
||||
// Don't ask for the weight twice: the verification page has just taken
|
||||
// it, so the desk opens with it filled in.
|
||||
@@ -2038,6 +2260,12 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
if (_isNavigating || !mounted) return;
|
||||
setState(() => _isNavigating = true);
|
||||
|
||||
// The photograph the verification screen took, remembered so the parcel
|
||||
// update and the pickup-complete proof both use the one upload.
|
||||
_verificationProofPath = (verificationData['parcelImage'] ?? '')
|
||||
.toString()
|
||||
.trim();
|
||||
|
||||
try {
|
||||
final dc = Get.put(PickupsController(), permanent: true);
|
||||
final d = widget.pickup;
|
||||
@@ -2184,9 +2412,23 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
}
|
||||
|
||||
if (pickupId > 0) {
|
||||
// ── The photograph is uploaded HERE, before the parcels are sent ──
|
||||
//
|
||||
// It used to be uploaded much later, inside `_payAndCreateOrder`, and
|
||||
// only its URL was kept — so `POST /parcel` went out with no `photos`
|
||||
// and the evidence ended up in a bucket nobody could search. `photos`
|
||||
// wants the storage **key**, so the upload has to happen before this
|
||||
// call, not after it.
|
||||
//
|
||||
// Uploaded once and remembered: `_payAndCreateOrder` reuses the same
|
||||
// ref for `proofImage` rather than pushing the same bytes twice.
|
||||
await _ensureParcelProofUploaded(pickupId);
|
||||
|
||||
final parcelRes = await UpdatePickupProvider().submitParcels(
|
||||
pickupId,
|
||||
verificationData,
|
||||
// The key rides in on the verification map the provider already
|
||||
// reads, so no third source of truth about this door.
|
||||
_withParcelPhotos(verificationData),
|
||||
);
|
||||
if (parcelRes?['status'] != true) {
|
||||
debugPrint('[PARCEL] booking $pickupId: not recorded — $parcelRes');
|
||||
@@ -2220,7 +2462,7 @@ class _PickupMapScreenState extends State<_PickupMapScreen>
|
||||
// the compliance stamp, the completed record, the next-stop list and the
|
||||
// hand-off screen — runs once and unchanged for either.
|
||||
final dynamic result =
|
||||
(ServiceProfile.active.initiatesShipment && shipmentCapture != null)
|
||||
(WorkPolicy.initiatesShipment(widget.pickup) && shipmentCapture != null)
|
||||
? await openScreen<Map<String, dynamic>>(
|
||||
context,
|
||||
ShipmentReviewPage(
|
||||
|
||||
Reference in New Issue
Block a user