diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 5d8bfec..86aa4f8 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -4,9 +4,17 @@ - - - + + + @@ -19,15 +27,43 @@ - - - - + + + - @@ -135,10 +171,6 @@ - - - - diff --git a/android/key.properties b/android/key.properties deleted file mode 100644 index 8a16a8a..0000000 --- a/android/key.properties +++ /dev/null @@ -1,4 +0,0 @@ -storePassword=W8UzA5NVaDTvix79rekmn8834m81tcKX -keyPassword=W8UzA5NVaDTvix79rekmn8834m81tcKX -keyAlias=doormile -storeFile=doormilerider-keystore.jks diff --git a/lib/controllers/auth.dart b/lib/controllers/auth.dart index 88f5f54..e84d858 100644 --- a/lib/controllers/auth.dart +++ b/lib/controllers/auth.dart @@ -10,9 +10,37 @@ import 'package:miler/utils/device.dart'; import 'package:miler/controllers/profile_controller.dart'; import 'package:miler/Models/login/login.dart'; import 'package:miler/data/api_config.dart'; +import 'package:miler/data/miler_api.dart'; import 'package:miler/views/helpers/widgets/miler_sheet_kit.dart'; -enum AuthNext { verifyPin, otp, notRegistered, error } +/// Which screen the phone number on the sign-in form has earned. +/// +/// ── `otp` is gone, and it was never real ── +/// +/// There is no OTP route on the miler side — `MilerApi` carries the whole auth +/// surface and it is login / set-pin / verify-pin / device-token. The old `otp` +/// branch fired when the directory said "no such account", sent the rider to a +/// code screen that verified nothing (`verifyOtp` returned `true` without +/// checking), and dead-ended at a Create-MPIN screen that could not write a +/// PIN. A rider who took it could not come back. +/// +/// The server answers this question directly now — see [MilerApi.pinSetOf]. +enum AuthNext { + /// `pin_set: true` — he has a PIN. Enter-PIN, exactly as before. + verifyPin, + + /// `pin_set: false` — a rider who has never signed in. Set-PIN. + setPin, + + /// 404. No miler account on this number. + notRegistered, + + /// 403. The row exists but is not an active miler. + inactive, + + /// The directory could not be reached. Not evidence about the rider. + error, +} class AuthController extends GetxController { final RxBool sendingOtp = false.obs; @@ -41,11 +69,18 @@ class AuthController extends GetxController { static const String _prefsUserEmailKey = 'user_email'; static const String _prefsContactNoKey = 'contactno'; static const String _prefsAddressKey = 'user_address'; - static const String _prefsForceMasterPinKey = 'force_master_pin'; - static const String _masterPinValue = '1234'; - static const String forceMasterPinPrefKey = _prefsForceMasterPinKey; - static const String masterPinValue = _masterPinValue; - bool _forceMasterPinFlow = false; + // ── The master-PIN constants are gone ── + // + // `_masterPinValue = '1234'`, `masterPinValue`, `forceMasterPinPrefKey` and + // `_forceMasterPinFlow` were declared here and read by nothing — the feature + // they belonged to was removed and its constants were not. A public constant + // named `masterPinValue` holding a four-digit PIN is an invitation to the + // next person looking for a shortcut, and it read as though the app still had + // a back door. Removed with the set-PIN work rather than left to be + // rediscovered. + // + // Riders set their own PIN now; `Creat_mpin.dart` refuses `1234` and `1111` + // along with every other trivial sequence. Future _notifyProfileController() async { try { if (Get.isRegistered()) { @@ -103,6 +138,26 @@ class AuthController extends GetxController { ); } + /// Which screen this phone number has earned, asked of the server. + /// + /// ── One call, one boolean, no guessing ── + /// + /// This used to ask `milerAccountExists`, which read *only the status code* + /// of `POST /miler/login` and threw the body away. From "an account exists" + /// it inferred Enter-PIN, and from "it does not" it inferred an OTP branch + /// that verified nothing and dead-ended at a screen which could not write a + /// PIN. A `null` — the directory unreachable — was read as "he has an + /// account", because the OTP direction was the worse place to be wrong. + /// + /// The server answers directly now. `pin_set` is the whole decision, and it + /// is read as a boolean rather than off the message beside it, which is prose + /// and will be reworded. + /// + /// The fallback when the field is absent — an older server, or a body that + /// did not parse — is **Enter-PIN**, for the same reason the old `null` case + /// chose it: a rider who does have a PIN can sign in, and one who does not + /// gets a refusal he can report. Sending him to Set-PIN on a guess earns a + /// 409 and a screen he cannot leave. Future precheckPhone(String phone) async { try { final normalized = _normalizePhone(phone); @@ -111,31 +166,34 @@ class AuthController extends GetxController { // The mocked "Demo Rider" (userid 9999) that used to be written here is // gone. It bypassed the server entirely and left a fake identity in prefs - // that outlived the session it was created for — every screen reading - // 'userid' got 9999 until the app was reinstalled. The real user is - // established by verify-pin and nowhere else. + // that outlived the session it was created for. The real user is + // established by verify-pin / set-pin and nowhere else. await prefs.setString(_prefsContactNoKey, normalized); - // On the live backend, ask whether this phone already belongs to an - // active miler account with a PIN on file. If it does, go straight to the - // MPIN screen: OTP delivery isn't live yet, and the OTP path ends at - // Create-MPIN, which would overwrite the PIN the account was issued. - // Seeded development accounts take exactly this branch — enter the phone, - // enter the seeded MPIN, done. - // Null means the directory could not be reached — see - // [AuthProvider.milerAccountExists]. Treat it as "he has an account", - // because that is true of every rider who gets this far and because the - // MPIN screen is the only one that can tell him what went wrong. The OTP - // branch is the dead end: it ends at Create-MPIN, which cannot write a - // PIN, so guessing wrong in that direction locks a rider out. - final exists = await _api.milerAccountExists(normalized); - if (exists ?? true) { - lastDecision = AuthNext.verifyPin; + final res = await MilerApi.login(normalized); + debugPrint( + '[AUTH][PRECHECK] $normalized -> ${res.status} ' + 'pin_set=${MilerApi.pinSetOf(res)} raw=${res.raw}', + ); + + if (res.status == 404) { + lastDecision = AuthNext.notRegistered; + return lastDecision!; + } + if (res.status == 403 || res.status == 401) { + lastDecision = AuthNext.inactive; + return lastDecision!; + } + if (!res.ok) { + // 5xx, a timeout, a body that did not parse. Not a fact about the + // rider, and not a reason to send him anywhere final. + lastDecision = AuthNext.error; return lastDecision!; } - // The directory answered, and said there is no such account. - lastDecision = AuthNext.otp; + lastDecision = MilerApi.pinSetOf(res) == false + ? AuthNext.setPin + : AuthNext.verifyPin; return lastDecision!; } catch (e) { debugPrint('Precheck phone error: $e'); @@ -144,6 +202,13 @@ class AuthController extends GetxController { } } + /// Why the last [setPin] failed, in the rider's words. Null on success. + String? lastSetPinFailure; + + /// True when [setPin] was refused because the account already has a PIN — + /// the caller sends the rider to Enter-PIN rather than showing an error. + bool lastSetPinWasAlreadySet = false; + Future sendOtp([String? phoneArg]) async { if (sendingOtp.value) return false; if (phoneArg != null && phoneArg.isNotEmpty) { @@ -179,52 +244,129 @@ class AuthController extends GetxController { return true; } + /// Creates this rider's PIN and signs him in. `POST /miler/set-pin`. + /// + /// ── What this replaces ── + /// + /// It called `AuthProvider.updatePin`, which had no route to call and + /// returned a manufactured `403 "Your MPIN is issued by your office and + /// cannot be changed from the app."` — correct while `reset-pin` was the only + /// PIN write and it needed an admin token, and a dead end for the rider + /// standing on the Create-MPIN screen. + /// + /// Riders set their own PIN on first sign-in now. The call returns a **full + /// session**, so this lands the rider logged in — there is no verify-pin + /// afterwards and no second screen. + /// + /// Returns true when the session is real. On a `409` — the account already + /// has a PIN — [lastSetPinWasAlreadySet] is set and the caller sends him to + /// Enter-PIN rather than showing him an error he cannot act on. Future setPin(String newPin) async { + lastSetPinFailure = null; + lastSetPinWasAlreadySet = false; + + final phone = currentPhone; + if (phone == null || phone.isEmpty) { + lastSetPinFailure = + 'We lost your number. Go back and enter it again.'; + return false; + } + if (newPin.length != 4 || int.tryParse(newPin) == null) { + lastSetPinFailure = 'Enter a 4-digit PIN.'; + return false; + } + try { final prefs = await SharedPreferences.getInstance(); - int? userId = - prefs.getInt(_prefsPendingPinUserIdKey) ?? - prefs.getInt(_prefsUserIdKey); - if (newPin.length != 4 || int.tryParse(newPin) == null) { - _showBottomSheet( - title: 'Invalid PIN', - message: 'Please enter a valid 4-digit PIN.', - ); + String deviceId = ''; + String fcmToken = ''; + try { + deviceId = await DeviceUtils.ensureDeviceId(prefs); + } catch (e) { + debugPrint('[AUTH] device id unavailable, continuing: $e'); + } + try { + fcmToken = await DeviceUtils.ensureFcmToken(prefs); + } catch (e) { + debugPrint('[AUTH] fcm token unavailable, continuing: $e'); + } + + // Same rule as verify-pin: only THIS attempt may grant a session, so a + // stale token cannot make a refused set-pin look accepted. + await ApiConfig.clearToken(); + + final Login res = await _api.loginParsed( + contactNo: phone, + deviceType: Platform.operatingSystem, + configId: 6, + deviceId: deviceId, + fcmToken: fcmToken, + pinRaw: newPin, + firstTime: true, + ); + + // `_loginNew` normalises the envelope as `code: ok ? 200 : httpStatus`, + // so on a refusal this IS the server's status line. `httpstatus` is on + // the envelope too but `Login` does not parse it. + final int http = res.code ?? 0; + + // ── 409 is not a failure the rider can fix by trying again ── + // + // It means the account already has a PIN — he is not a first-time rider + // after all, or he set one on another handset. The caller sends him to + // Enter-PIN; telling him "could not save your PIN" would leave him + // retyping a PIN the server will never accept. + if (http == 409) { + lastSetPinWasAlreadySet = true; + lastSetPinFailure = + 'You already have a PIN on this number. Enter it to sign in.'; return false; } - if (userId == null) { - _showBottomSheet( - title: 'Error', - message: 'User ID not found. Please try again.', - ); + if (http == 404) { + lastSetPinFailure = + 'That number is not registered as a Miler. Contact your manager.'; + return false; + } + if (http == 403 || http == 401) { + lastSetPinFailure = + 'This account is not active. Contact your manager.'; return false; } - final int pinNum = int.parse(newPin); - final res = await _api.updatePin(userId: userId, pin: pinNum); - if (res.statusCode >= 200 && res.statusCode < 300) { + final bool serverAccepted = res.status == true; + final String? token = await ApiConfig.getToken(); + final bool haveSession = token != null && token.isNotEmpty; + + if (serverAccepted && !haveSession) { + // The PIN was created and there is nothing to sign in with. An + // integration fault, and it must never be reported as the rider's + // mistake — see the same branch in [verifyPinWithServer]. + debugPrint('[AUTH] set-pin succeeded but returned no usable token'); + lastSetPinFailure = + 'Your PIN was saved, but the server did not return a session. ' + 'Sign in with your new PIN.'; + lastSetPinWasAlreadySet = true; + return false; + } + + if (serverAccepted && haveSession) { await prefs.setString('dbPin', newPin); + await prefs.setBool('logged_out', false); + await prefs.setString(_prefsContactNoKey, phone); await prefs.remove(_prefsPendingPinUserIdKey); + await _notifyProfileController(); return true; } - // The server's own sentence, not a slice of its JSON. A rider reading - // `{"status":false,"code":403,...}` learns nothing he can act on. - String reason = ''; - try { - final decoded = json.decode(res.body); - if (decoded is Map) reason = (decoded['message'] ?? '').toString(); - } catch (_) {} - if (reason.trim().isEmpty) { - reason = 'Could not set your MPIN. Please contact your manager.'; - } - _showBottomSheet(title: 'MPIN not changed', message: reason); + + final String serverMsg = (res.message ?? '').trim(); + lastSetPinFailure = serverMsg.isNotEmpty && serverMsg.length < 140 + ? serverMsg + : 'Could not set your PIN. Check your connection and try again.'; return false; } catch (e) { debugPrint('setPin error: $e'); - _showBottomSheet( - title: 'Error', - message: 'Something went wrong while setting the PIN.', - ); + lastSetPinFailure = + 'Something went wrong while setting your PIN. Try again.'; return false; } } diff --git a/lib/controllers/pickups_controller.dart b/lib/controllers/pickups_controller.dart index 06d8922..e9776e5 100644 --- a/lib/controllers/pickups_controller.dart +++ b/lib/controllers/pickups_controller.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'package:flutter/foundation.dart'; +import 'package:miler/data/geofence.dart'; import 'package:miler/data/work_repository.dart'; import 'package:flutter/material.dart'; import 'package:latlong2/latlong.dart' show LatLng; @@ -19,133 +20,38 @@ import 'package:minio/io.dart'; import 'package:miler/utils/kalman_filter.dart'; import 'package:miler/utils/mqtt_service.dart'; import 'package:miler/controllers/connectivity_mixin.dart'; -import 'package:miler/views/helpers/constants/Colorconstants.dart'; import 'package:miler/data/meal_run_mock.dart'; import 'package:miler/views/helpers/widgets/app_widgets.dart'; -/// ── PROXIMITY ENFORCEMENT IS OFF ── +/// ── PROXIMITY ENFORCEMENT IS ON ── /// -/// Off again on 2026-08-25, the same day it was turned on, at the founder's -/// call. Nothing was found wrong with it — this is a decision about when to -/// switch it on, not a retraction of the work. +/// The fence itself moved to `lib/data/geofence.dart` on 2026-09-16. What used +/// to live here — four constants, a `_freshFix` ladder and a 130-line +/// `_checkGeofence` — could only be reached from a `GetxController`, so Home's +/// bulk gate grew a second copy of the arithmetic with a different radius, and +/// none of it could be tested without a real handset. /// -/// It is off for **every** status — arrived, picked, picked up, delivery -/// arrived, delivered, cancelled — and on both gates, this one and the bulk -/// check on Home. Half a fence is worse than none: it would block one route -/// into a rung and wave through another, with two different messages and no -/// explanation of why one worked. +/// [Geofence] is that logic, extracted and corrected. Four things changed with +/// it, and each one was a way a rider could mark a stop he was not standing at: /// -/// ── What it costs while it is off ── +/// * **It is on.** `kGeofenceEnforced` defaulted to `false`, so +/// `_checkGeofence` returned `true` on its third line in every shipped +/// build. +/// * **The radius is 100 m, not 10.** Ten metres is inside consumer-GPS error, +/// which is what got the fence switched off twice before. +/// * **It fails closed.** The old `catch` returned `true` with the comment +/// "allow on error (fail-safe)". Any throw from the location stack opened +/// every rung in the app. +/// * **The phone's error is no longer credited to the rider.** The comparison +/// was `distance - accuracy > radius`, so slack grew with inaccuracy and a +/// ±250 m fix bought 250 m of it. A fix too vague to resolve the fence is +/// refused instead — see `kGeofenceMaxAccuracyMetres`. /// -/// This is the one control that decides whether the app is telling the truth -/// about where a rider was standing when he said a stop was done. With it off, -/// "Picked up" means he pressed a button, not that he was there — the -/// timestamps and the GPS still ride along on every status write, so the office -/// can audit after the fact, but nothing is refused at the moment of the press. -/// -/// ── What is ready for the day it goes back on ── -/// -/// Recorded here because the work is done and the flag is the only thing -/// holding it, so nobody has to rediscover any of it. The fence was off from -/// 2026-08-19 because it refused real work — riders pressing **Picked up** at a -/// counter were told "You're 4.2 km from this stop" — and that was a -/// measurement problem rather than a strictness one. Three causes, all fixed: -/// -/// • **The fix was not worth measuring with.** Home handed the fence a -/// `LocationAccuracy.low` position — a ~1 km hint on Android — or a cached -/// one of any age. See [_freshFix] and [kGeofenceFixMaxAge]. -/// • **The phone's own error was charged to the rider.** The comparison is -/// `distance - accuracy > radius`, so a rider 12 m out on a ±20 m fix is not -/// refused for a precision the hardware never provided. -/// • **There were three radii.** A configured `pickupradius` defaulting to -/// 100 here, a hardcoded 500 in Home's bulk gate, and no agreement between -/// them. There is one now: [kGeofenceRadiusMeters], set to 10. -/// -/// All three are live in the code below and simply do not run while this is -/// false. The fourth cause is real and none of this fixes it: **the booking's -/// own coordinates are often wrong**, because they come from wherever the -/// customer dropped a pin. A stop carrying *no* coordinates is allowed through -/// — that is the hub's data, not something a rider can resolve from a doorstep -/// — but a stop carrying wrong ones will still refuse him. -/// -/// ── Turning it on ── -/// -/// flutter run --dart-define=ENFORCE_GEOFENCE=true -/// flutter build apk --dart-define=ENFORCE_GEOFENCE=true -/// -/// Or flip the default. Ten metres is tight — at or inside consumer GPS -/// accuracy — so if the first reports are riders blocked at doors, raise -/// [kGeofenceRadiusMeters] before reaching for this switch again. -/// -/// Every bypass is logged in every build mode (see `_checkGeofence`), so a -/// build's own log says which way it was compiled. -/// -/// ── The history, so none of the old mistakes come back ── -/// -/// This was once `kDebugMode`, which meant the fence was off in exactly the -/// builds anyone tested with — so it was never really tested. It was then -/// pinned to a hard `false`, which left no way to walk the flow at a desk. The -/// shape below survives both: one named constant, one define, one default, and -/// the default is a product decision rather than a side effect of how the app -/// was built. -const bool kGeofenceEnforced = bool.fromEnvironment( - 'ENFORCE_GEOFENCE', - defaultValue: false, -); - -/// The name every call site reads. Derived, so there is one switch and not two. -const bool kBypassGeofenceForTesting = !kGeofenceEnforced; - -/// ───────────────────────────────────────────────────────────────────────── -/// HOW CLOSE IS "AT THE STOP" — 10 metres -/// -/// A product decision, taken deliberately and tight: the rider must be *at the -/// door*, not on the street outside it, before he can mark a stop arrived, -/// picked or delivered. -/// -/// ── Why this is a constant and no longer the server's `pickupradius` ── -/// -/// The fence used to read `pickupradius` out of prefs, which login writes from -/// the profile and defaults to 100. That made the strictness of the app's one -/// honesty control a per-tenant configuration value nobody on this side could -/// see, and it silently disagreed with the second gate on Home, which was -/// hardcoded to 500. Three numbers for one rule. This is the rule. -/// -/// ── What 10 metres actually demands, stated plainly ── -/// -/// This is at or inside the accuracy of consumer GPS. A phone reports a fix -/// with an error radius, and 5–15 m in the open is normal while 30–50 m -/// between buildings is ordinary rather than exceptional. A fence smaller than -/// the error it is measured with will refuse a rider who is genuinely standing -/// at the door — which is the exact failure that got this whole control turned -/// off once before, and the reason two things below are not optional: -/// -/// • **The fix must be worth 10 m.** [LocationAccuracy.best], not the `low` -/// the callers were passing — `low` is a ~1 km hint on Android and against -/// a 10 m fence it is not a measurement, it is a coin toss. See -/// [_freshFix]. -/// -/// • **The phone's own error is credited to the rider.** The check is -/// `distance - accuracy > radius`, not `distance > radius`: a rider 12 m -/// away on a fix that says ±20 m has not been shown to be outside the -/// fence, and refusing him is asserting a precision the hardware did not -/// provide. He is refused when the *phone* says he is outside, not when the -/// arithmetic does. -/// -/// Together those keep 10 m meaning "at the door" without it meaning "when the -/// satellites are kind". If riders still report being blocked at a door, this -/// number is the knob — raise it here, in one place, rather than turning the -/// fence off again. -const double kGeofenceRadiusMeters = 10; - -/// How stale a cached fix may be before the fence refuses to measure with it. -/// -/// A last-known position is instant and free and can be an hour old. Against a -/// 100 m fence that was survivable; against 10 m it is how a rider marks a -/// delivery arrived from the previous street because that is where the phone -/// last looked. -const Duration kGeofenceFixMaxAge = Duration(seconds: 30); - +/// What is left in this file is the wiring: [_checkGeofence] adapts [Geofence] +/// to the eight call sites below, and every one of them returns **before** +/// building a payload when it answers false. That is asserted end-to-end in +/// `geofence_test.dart` with an HTTP client that fails the test if any request +/// is sent. class PickupsController extends GetxController with ConnectivityControllerMixin { MilerKalmanFilter? _kf; @@ -332,12 +238,12 @@ class PickupsController extends GetxController // ---------------- Location helpers ---------------- /// The error radius, in metres, of the fix [_ensureLatLng] last obtained. /// - /// Read by [_checkGeofence], which credits it to the rider — see - /// [kGeofenceRadiusMeters]. Starts at zero so a fence measured before any fix - /// has been taken is strict rather than accidentally generous. + /// Telemetry only. The fence no longer reads this — [Geofence] takes its own + /// fix and rejects one it cannot trust rather than crediting its error to the + /// rider. Kept because the payload builders log it. double _lastFixAccuracy = 0; - /// A position good enough to measure a [kGeofenceRadiusMeters] fence with. + /// A position good enough to put on a status payload. /// /// ── What this replaced, and why it had to go ── /// @@ -380,7 +286,7 @@ class PickupsController extends GetxController } else if (cached != null) { debugPrint( '[GEOFENCE] cached fix is ${age?.inSeconds}s old — too stale to ' - 'measure a ${kGeofenceRadiusMeters.toStringAsFixed(0)}m fence', + 'measure a ${kGeofenceRadiusMetres.toStringAsFixed(0)}m fence', ); } } catch (_) {} @@ -398,34 +304,28 @@ class PickupsController extends GetxController final needsFetch = (lat == '0' || lat.isEmpty || lng == '0' || lng.isEmpty); - // ── The two shortcuts below are disabled while the fence is on ── + // ── These shortcuts are safe again, because nothing is decided on them ── // - // Every caller of this method feeds its answer to [_checkGeofence] and - // then puts the same pair on the payload. Both shortcuts hand back a - // position of unknown provenance: the first trusts whatever the screen - // passed in — Home passes a `LocationAccuracy.low` fix, which is a ~1 km - // hint — and the second reuses a cached value with no age on it at all. - // Neither carries an accuracy, so [_lastFixAccuracy] would be stale too - // and the fence would measure a 10 m rule with a number it cannot - // characterise. + // They used to be gated on the fence being off, and rightly: the fence + // measured with whatever this returned, and both shortcuts hand back a + // position of unknown provenance — the first trusts whatever the screen + // passed in (Home passes a `LocationAccuracy.low` fix, a ~1 km hint on + // Android), the second reuses a cached value with no age on it at all. // - // With the fence enforced this takes one real fix per status write. That - // is a few seconds, once, at a door the rider is standing still at — and - // it is the whole basis on which the app is about to refuse or allow his - // press. With the fence off the shortcuts stand: nothing is being decided - // on the answer, it is telemetry. - if (kBypassGeofenceForTesting) { - // Fast path: if we have valid coordinates, use them immediately - if (!needsFetch) return {'lat': outLat, 'lng': outLng}; + // [Geofence] takes its own `best` fix now and characterises it before + // deciding, so this method is back to what its name says: the coordinates + // that go **on the payload**. Telemetry, not evidence. Taking a second + // 8-second fix for it would cost the rider time at every door to improve + // a number nobody adjudicates. + // Fast path: if we have valid coordinates, use them immediately + if (!needsFetch) return {'lat': outLat, 'lng': outLng}; - // Reuse recently cached coordinates first if fresh (e.g. within 30s) - // For now just check if they exist to save time - if (currentLat.value.isNotEmpty && - currentLat.value != '0' && - currentLng.value.isNotEmpty && - currentLng.value != '0') { - return {'lat': currentLat.value, 'lng': currentLng.value}; - } + // Reuse recently cached coordinates first if fresh (e.g. within 30s) + if (currentLat.value.isNotEmpty && + currentLat.value != '0' && + currentLng.value.isNotEmpty && + currentLng.value != '0') { + return {'lat': currentLat.value, 'lng': currentLng.value}; } final serviceEnabled = await Geolocator.isLocationServiceEnabled(); @@ -1212,12 +1112,15 @@ class PickupsController extends GetxController // ---------------- Geofencing helpers ---------------- // - // The radius is [kGeofenceRadiusMeters] and nothing else. It read the - // server's `pickupradius` out of prefs, which meant the app's one honesty - // control was a per-tenant number nobody here could see — and it disagreed - // with Home's own hardcoded 500. `pickupradius` is still stored at login; - // it simply no longer decides this. - double get _geofenceRadius => kGeofenceRadiusMeters; + // The radius is [kGeofenceRadiusMetres], in `lib/data/geofence.dart`, and + // nothing else. It read the server's `pickupradius` out of prefs, which made + // the app's one honesty control a per-tenant number nobody here could see — + // and it disagreed with Home's own hardcoded 500. `pickupradius` is still + // stored at login; it simply no longer decides this. + + /// The fence's answer to the last rung that asked, for screens that want to + /// show the live distance rather than only the refusal. + GeofenceDecision? lastGeofence; // Helper method to show snackbar reliably in both debug and release builds // @@ -1304,6 +1207,18 @@ class PickupsController extends GetxController /// a stop the hub keeps rejecting. See [updatePickedStatus]. String? lastPickupRefusal; + /// The proximity gate for one rung. **False means do not call the API.** + /// + /// Adapts [Geofence] to the call sites below. The rider's own coordinates are + /// no longer taken as arguments: the fence takes its *own* fix, at + /// [LocationAccuracy.best], because the pair the screens pass in comes from a + /// `low`-accuracy Home poll or an unaged cache and carries no accuracy at all + /// — so the fence could not tell how much to trust the number it was + /// measuring a 100 m rule with. The parameters stay for the eight call sites + /// that build a payload from the same values; they are ignored here. + /// + /// Sets [lastBlockedReason] to the rider-facing sentence on every refusal, so + /// a caller can say what happened instead of reporting a network failure. Future _checkGeofence( double targetLat, double targetLng, @@ -1313,132 +1228,20 @@ class PickupsController extends GetxController ) async { lastBlockedReason = null; - // Opt-in via `--dart-define=BYPASS_GEOFENCE=true`; enforced otherwise. See - // the declaration for why it is a define rather than a constant. - // - // Logged with `debugPrint` in *every* build mode, deliberately — not behind - // `kDebugMode` like the diagnostics below. The whole risk of this flag is a - // build going out with proximity silently off, so the one thing it must - // never be is quiet. - if (kBypassGeofenceForTesting) { - debugPrint( - '[GEOFENCE] OFF for $action — enforcement is disabled in this build. ' - 'Stop completion is NOT proximity-verified. ' - 'See kGeofenceEnforced.', - ); - return true; - } - - // Validate coordinates - ensure they are valid GPS coordinates - final bool hasValidTarget = - targetLat != 0 && - targetLng != 0 && - targetLat.abs() <= 90 && - targetLng.abs() <= 180; - final bool hasValidCurrent = - currentLat != 0 && - currentLng != 0 && - currentLat.abs() <= 90 && - currentLng.abs() <= 180; - - // ── Two ways to have no coordinates, and only one of them is the rider's ── - // - // This used to treat both the same and wave both through: "Missing - // coordinates. Proceeding with update." That is the bypass that makes a - // fence decorative — turn location off and every rung opens — and it was - // survivable only because the fence itself was off. - // - // **No target.** The booking carries no pin. That is the hub's data, the - // rider cannot fix it from a doorstep, and blocking him leaves the stop - // unworkable by anyone. Allowed, and logged, exactly as before. - if (!hasValidTarget) { - debugPrint( - '[GEOFENCE] $action allowed: the stop carries no coordinates ' - '($targetLat, $targetLng), so proximity cannot be checked. This is a ' - 'data gap on the booking, not a rider who is somewhere else.', - ); - return true; - } - - // **No fix.** Location is off, permission is denied, or the GPS did not - // settle in time. This one the rider *can* fix, and it is the difference - // between a fence and a suggestion — so it is refused, and the message - // says which of the three to go and change. - if (!hasValidCurrent) { - debugPrint( - '[GEOFENCE] $action refused: no usable fix ' - '($currentLat, $currentLng)', - ); - lastBlockedReason = - 'Your phone could not find your location, so this stop cannot be ' - 'marked ${action.toLowerCase()}. Turn location on, allow it for ' - 'Miler, and step outside if you can.'; - _showErrorSnackbar('Location Error', lastBlockedReason!, seconds: 5); - return false; - } - - try { - final radius = _geofenceRadius; - final distance = Geolocator.distanceBetween( - targetLat, - targetLng, - currentLat, - currentLng, - ); - final distanceKm = distance / 1000.0; - final distanceMeters = distance; - - // ── The phone's own error is credited to the rider ── - // - // A fix carries an accuracy in metres, and at a 10 m fence that number - // is the same size as the thing being measured. Comparing a raw distance - // against 10 m asserts a precision the hardware did not provide, and the - // rider standing at the door on a ±25 m fix is the one it refuses. - // - // So the fence is measured against the *nearest point the phone allows*: - // 12 m away on a ±20 m fix has not been shown to be outside it. He is - // blocked when the phone says he is outside, not when the arithmetic - // does. See [kGeofenceRadiusMeters]. - final slack = _lastFixAccuracy; - final effective = (distance - slack).clamp(0.0, double.infinity); - - debugPrint( - '[GEOFENCE] $action | target ($targetLat, $targetLng) ' - '| rider ($currentLat, $currentLng) ' - '| ${distanceMeters.toStringAsFixed(1)}m ±${slack.toStringAsFixed(0)}m ' - '→ ${effective.toStringAsFixed(1)}m vs ${radius.toStringAsFixed(0)}m', - ); - - if (effective > radius) { - // ── One sentence, in metres he can act on ── - // - // Was three lines of "Distance: 4213 m (4.21 km) / Required: Within - // 100 m" — a readout, in a snackbar, on a phone in a jacket pocket. - // What the rider needs is how far he still has to go. - final String away = distanceMeters >= 1000 - ? '${distanceKm.toStringAsFixed(1)} km' - : '${distanceMeters.toStringAsFixed(0)} m'; - lastBlockedReason = - "You're $away from this stop — get within " - '${radius.toStringAsFixed(0)} m to mark it ' - '${action.toLowerCase()}'; - _showErrorSnackbar('Location Error', lastBlockedReason!, seconds: 5); - return false; - } - return true; - } catch (e) { - if (kDebugMode) { - debugPrint('[GEOFENCE] Error calculating distance: $e'); - } - // On error, show warning but allow (fail-safe) - _showErrorSnackbar( - 'Location Warning', - 'Unable to verify distance. Proceeding with caution.', - bgColor: ColorConstants.warning, - seconds: 3, - ); - return true; // Allow on error (fail-safe) + final decision = await Geofence.check( + targetLat: targetLat, + targetLng: targetLng, + action: action, + ); + + lastGeofence = decision; + if (decision.allowed) return true; + + lastBlockedReason = decision.reason; + if (decision.reason != null) { + _showErrorSnackbar('Location Error', decision.reason!, seconds: 5); } + return false; } // Get last pickup location (for calculating riderkms between pickup) @@ -1824,6 +1627,14 @@ class PickupsController extends GetxController final mock = _mockStatus(pickupId, 'arrived'); if (mock != null) return mock; + // Cleared on entry, not only on the paths that set them. The caller now + // reads these to decide whether a failed arrival may still advance the rung + // locally, and a value left over from an earlier stop would answer for this + // one — refusing an arrival because a different door refused twenty minutes + // ago. `lastBlockedReason` gets the same treatment inside `_checkGeofence`. + lastArrivalRefusal = null; + lastArrivalNotice = null; + // START LOADING IMMEDIATELY for better UX arrivedShimmer.value = true; try { @@ -1878,6 +1689,20 @@ class PickupsController extends GetxController ? resp!['message'].toString() : 'Your office would not accept this arrival.') : null; + // ── A write that never landed must not look like one that did ── + // + // `lastArrivalRefusal` stays null on a transport failure, deliberately: + // the caller lets a rider with no signal carry on rather than stranding + // him at a door. But it then advanced the rung *silently*, so the rider + // saw ARRIVED and had no way to know the hub had not been told — and + // the first person to find out was an operator wondering why he had + // been at a kitchen for forty minutes. + // + // He still carries on. He is simply told. + lastArrivalNotice = lastArrivalRefusal == null + ? 'Marked arrived on your phone. Your office has not been told — ' + 'you had no connection. Tell them if it matters.' + : null; debugPrint('[UPDATE][ARRIVED][FAILED] resp=${jsonEncode(resp)}'); return false; } @@ -1957,18 +1782,25 @@ class PickupsController extends GetxController final dLat = double.tryParse(dropLat) ?? 0.0; final dLng = double.tryParse(dropLng) ?? 0.0; + // ── Unconditional ── + // + // This was wrapped in `if (dLat != 0 && dLng != 0)`, so a consignment + // whose drop pin was missing skipped the fence entirely — silently, with + // no log and no record. That is the one case where the fence matters + // most: nobody can say afterwards where the rider was standing. A missing + // pin is now [GeofenceOutcome.noTarget] and is refused with a sentence + // that sends the rider to his office. + // // Fenced against the DROP, not the pickup. Using the pickup coordinates - // here would fence the rider to the kitchen he left an hour ago. - if (dLat != 0 && dLng != 0) { - final inFence = await _checkGeofence( - dLat, - dLng, - rLat, - rLng, - 'Delivery arrived', - ); - if (!inFence) return false; - } + // here would fence the rider to the counter he left an hour ago. + final inFence = await _checkGeofence( + dLat, + dLng, + rLat, + rLng, + 'Delivery arrived', + ); + if (!inFence) return false; // The clock the completion record reads, same key the pickup leg uses. final prefs = await SharedPreferences.getInstance(); @@ -2027,16 +1859,25 @@ class PickupsController extends GetxController final dLat = double.tryParse(dropLat) ?? 0.0; final dLng = double.tryParse(dropLng) ?? 0.0; - if (dLat != 0 && dLng != 0) { - final inFence = await _checkGeofence( - dLat, - dLng, - rLat, - rLng, - 'Delivered', - ); - if (!inFence) return false; - } + // ── Unconditional ── + // + // This was wrapped in `if (dLat != 0 && dLng != 0)`, so a consignment + // whose drop pin was missing skipped the fence entirely — silently, with + // no log and no record. That is the one case where the fence matters + // most: nobody can say afterwards where the rider was standing. A missing + // pin is now [GeofenceOutcome.noTarget] and is refused with a sentence + // that sends the rider to his office. + // + // Fenced against the DROP, not the pickup. Using the pickup coordinates + // here would fence the rider to the counter he left an hour ago. + final inFence = await _checkGeofence( + dLat, + dLng, + rLat, + rLng, + 'Delivered', + ); + if (!inFence) return false; _pickedTime = _formatDateTimeFull(DateTime.now()); diff --git a/lib/data/accepted_store.dart b/lib/data/accepted_store.dart index 9371ca7..bff9b7e 100644 --- a/lib/data/accepted_store.dart +++ b/lib/data/accepted_store.dart @@ -57,6 +57,84 @@ Future _drainLegacy(String base, WorkScope scope) async { debugPrint('[SCOPE] drained legacy "$base" into ${scope.key}'); } +/// Moves records out of the old line-suffixed keys into this scope's key. +/// +/// ── Why this one MERGES where [_drainLegacy] drops ── +/// +/// A line-suffixed key is not anonymous the way a global one is. It already +/// names the rider and the tenant — `completed_bookings::u38.t13.milkMan` — +/// so everything in it provably belongs to this scope. There is nothing to +/// attribute and nothing to guess, and dropping it would throw away work the +/// rider actually did. +/// +/// Both old drawers are merged, because the whole point of removing the line +/// is that one rider's day is one day: a meal round and a logistics collection +/// finished in the same shift belong in the same history. Ids already present +/// win, so a re-run cannot duplicate a row. +Future _drainLineScoped(String base, WorkScope scope) async { + final prefs = await SharedPreferences.getInstance(); + final target = scope.scoped(base); + + // Read the target through `get`, not the typed accessors: these base keys + // hold a JSON blob for record stores and a String list for id stores, and + // `getStringList` throws outright when it meets the blob. + final Object? existing = prefs.get(target); + final merged = >[]; + final mergedIds = {}; + final ids = {}; + + if (existing is String) { + for (final row in _decode(existing)) { + merged.add(row); + final id = _rowKey(row); + if (id.isNotEmpty) mergedIds.add(id); + } + } else if (existing is List) { + ids.addAll(existing.map((e) => e.toString())); + } + + var moved = false; + for (final legacy in scope.legacyScopedKeys(base)) { + if (legacy == target || !prefs.containsKey(legacy)) continue; + final Object? value = prefs.get(legacy); + + if (value is String) { + for (final row in _decode(value)) { + final id = _rowKey(row); + if (id.isNotEmpty && !mergedIds.add(id)) continue; + merged.add(scope.stamp(row)); + } + } else if (value is List) { + ids.addAll(value.map((e) => e.toString())); + } + await prefs.remove(legacy); + moved = true; + } + if (!moved) return; + + if (merged.isNotEmpty) { + await prefs.setString(target, jsonEncode(merged)); + } else if (ids.isNotEmpty) { + await prefs.setStringList(target, ids.toList()); + } + debugPrint('[SCOPE] merged line-scoped "$base" into ${scope.key}'); +} + +/// The identity a stored row is de-duplicated on while merging. +String _rowKey(Map row) { + for (final k in const [ + 'orderid', + 'OrderId', + 'bookingid', + 'consignmentid', + 'id', + ]) { + final v = row[k]; + if (v != null && v.toString().trim().isNotEmpty) return v.toString(); + } + return ''; +} + /// Runs the one-time drain for every legacy key. Cheap after the first call — /// `containsKey` on a loaded prefs map. Future migrateLegacyStores() async { @@ -75,6 +153,9 @@ Future migrateLegacyStores() async { _kOrderLabelsKeyBase, ]) { await _drainLegacy(base, scope); + // Then fold in anything the old line-suffixed keys still hold. Order + // matters: the global drain writes the scoped key, and this merges into it. + await _drainLineScoped(base, scope); } } diff --git a/lib/data/api_config.dart b/lib/data/api_config.dart index ccce2b4..ed83baa 100644 --- a/lib/data/api_config.dart +++ b/lib/data/api_config.dart @@ -295,6 +295,44 @@ class ApiConfig { 'referenceno', 'orderid', ]); + + // ── One customer pickup can be several drops ── + // + // A customer-app booking carries N destinations and `GET /miler/bookings` + // returns ONE ROW PER DESTINATION once the pickup has been collected — + // same `bookingid`, same `bookingreference`, different door. The backend + // labels them for us: `destinationseq` is which one this is and + // `destinationcount` how many there are (both absent, or 1, on every + // console and milk-run booking, which is the whole existing world). + // + // Everything the rider's device remembers about a stop is keyed on + // `orderid` — the accepted store dedupes on it, the consignment-id map + // files under it, the collected and out-for-delivery sets hold it, the ETA + // and km keys are built from it. So three drops sharing one `orderid` is + // not a display bug: `addAcceptedBookings` deduped two of the three away + // before any screen saw them, and the two consignment ids that lost the + // race were unrecoverable, which is a bag the rider is holding with no + // door to take it to. + // + // So `orderid` becomes the **stop** key and carries the destination on it. + // The booking's own reference is untouched below in `bookingreference`, + // which is what every screen shows the rider and what he reads out on the + // phone. Single-destination bookings are byte-identical to before — the + // suffix only exists where there is something to tell apart. + final destinationCount = + int.tryParse( + (pick(['destinationcount', 'destinationCount']) ?? '').toString(), + ) ?? + 1; + final destinationSeq = + int.tryParse( + (pick(['destinationseq', 'destinationSeq']) ?? '').toString(), + ) ?? + 0; + final bookingKey = ref ?? id; + final stopKey = destinationCount > 1 + ? '$bookingKey#$destinationSeq' + : bookingKey; final status = pick([ 'status', 'bookingstatus', @@ -320,11 +358,37 @@ class ApiConfig { return { // identity 'pickupid': id, - 'orderid': ref ?? id, + // The STOP key — see the note above. `bookingreference` is the booking's + // own name and is what gets shown; this is what gets remembered. + 'orderid': stopKey, 'orderheaderid': id, 'bookingid': id, // keep original too 'bookingreference': s(ref), + // ── Which drop of the visit this is ── + // + // Carried so a card can say "Stop 2 of 3" rather than showing three rows + // that are identical down to the reference number. `destinationcount` is + // 1 for every single-destination and console booking, and the UI treats + // 1 as "say nothing". + 'destinationseq': destinationSeq, + 'destinationcount': destinationCount, + + // ── The parcel's own number, and who is waiting for it ── + // + // All three exist per DESTINATION, not per booking: on a three-drop + // pickup each door has its own tracking number and its own receiver, and + // the booking-level customer is the SENDER, who is not at any of them. + // Dropped by this adapter until now, so the rider arrived at a stranger's + // door with the sender's name on his screen and no number to read out. + 'trackingno': s(pick(['trackingno', 'trackingNo', 'tracking_no'])), + 'recipientname': s( + pick(['recipientname', 'recipientName', 'recipient_name']), + ), + 'recipientphone': s( + pick(['recipientphone', 'recipientPhone', 'recipient_phone']), + ), + // status 'orderstatus': fromConsignment.isNotEmpty ? fromConsignment diff --git a/lib/data/geofence.dart b/lib/data/geofence.dart new file mode 100644 index 0000000..15837de --- /dev/null +++ b/lib/data/geofence.dart @@ -0,0 +1,578 @@ +/// ───────────────────────────────────────────────────────────────────────── +/// THE PROXIMITY FENCE — one radius, one policy, one answer +/// +/// A rider may not tell the hub he is at a door he is not at. Every +/// location-sensitive rung — arrived, picked, picked up, delivery arrived, +/// delivered, handed over — is measured here and nowhere else. +/// +/// ── Why this is its own file ── +/// +/// The measurement used to live inside `PickupsController`, which cost three +/// things: +/// +/// * it could only be reached from a `GetxController`, so Home's bulk gate +/// grew a second copy with a different radius (500 m against the +/// controller's 100, later 10); +/// * it could not be tested without a real `Geolocator`, so the arithmetic was +/// never exercised at the values that decide a rider's day — 99 m and 101 m; +/// * it returned a bare `bool`, so a screen could not tell "he is 240 m away" +/// from "his GPS is off", and showed one sentence for both. +/// +/// This returns a [GeofenceDecision] carrying `allowed`, `distanceMetres`, +/// `reason`, `accuracyMetres` and `timestamp`, and takes its position through +/// an injectable seam ([positionProvider]) so the whole table is a unit test +/// rather than a walk around a car park. +/// +/// ── THE RULE ── +/// +/// distance > kGeofenceRadiusMetres → BLOCKED, and no API call is made. +/// +/// Blocked means blocked. A caller that receives `allowed == false` must return +/// before it builds a payload — not warn and continue. `geofence_test.dart` +/// asserts that with a `MockClient` that fails the test if any request is sent. +/// +/// ── Fail CLOSED ── +/// +/// Every failure to *measure* is a refusal: no permission, no fix, a stale one, +/// a vague one, a throw from the platform channel, and a booking carrying no +/// coordinates. The previous implementation allowed on four of those six, which +/// made the fence decorative — switching location off opened every rung in the +/// app. +/// ───────────────────────────────────────────────────────────────────────── +library; + +import 'package:flutter/foundation.dart'; +import 'package:geolocator/geolocator.dart'; + +/// The one switch, and it is **ON**. +/// +/// ── The default changed on 2026-09-16, and it is not to change back ── +/// +/// This was `defaultValue: false`, so every release build shipped with the +/// fence off and `_checkGeofence` returned `true` on its third line. "Picked +/// up" meant the rider pressed a button, not that he was there. +/// +/// The define survives as an **ops kill switch**, not a development +/// convenience: if the fence starts refusing real work at real doors, a build +/// with `--dart-define=ENFORCE_GEOFENCE=false` gets the fleet moving again +/// within one release rather than one sprint. Every bypassed check logs, in +/// every build mode, so a build's own log says which way it was compiled. +/// +/// Raise [kGeofenceRadiusMetres] before reaching for this. +const bool kGeofenceEnforced = bool.fromEnvironment( + 'ENFORCE_GEOFENCE', + defaultValue: true, +); + +/// How close "at the stop" is, in metres. +/// +/// ── Why 100 and not 10 ── +/// +/// This was 10 m for one day. Ten metres is at or inside the error radius of +/// consumer GPS: a phone reporting ±15 m in the open and ±40 m between +/// buildings cannot resolve a 10 m fence at all, so the control stopped +/// measuring proximity and started measuring whether the satellites were kind. +/// That is the exact failure that got the fence switched off twice before. +/// +/// 100 m is the operational radius: comfortably outside normal GPS error, and +/// tight enough that a rider two streets away is refused. It is one number, +/// here, and it is deliberately **not** read from the server's `pickupradius` — +/// that made the app's one honesty control a per-tenant value nobody on this +/// side could see, and it silently disagreed with a hardcoded 500 in Home's own +/// gate. +const double kGeofenceRadiusMetres = 100; + +/// How stale a fix may be before it is not a measurement. +/// +/// On a round, a stale position is reliably the *previous* stop. Anything much +/// longer than this and the fence measures from the last door. +const Duration kGeofenceFixMaxAge = Duration(seconds: 30); + +/// The worst fix the fence will decide on, in metres. +/// +/// ── Rejected, not credited ── +/// +/// The previous implementation credited the phone's error to the rider — +/// `distance - accuracy > radius` — so a ±250 m fix bought 250 m of slack and a +/// rider a quarter-kilometre away was waved through. Against a 100 m fence that +/// inverts the control: the worse the fix, the easier it is to pass. +/// +/// A fix too vague to decide with is a fix the app must not decide with. At or +/// under this it is used raw; over it the rung is refused with "step into the +/// open", which is a thing the rider can act on. 50 m is generous against a +/// 100 m fence — typical urban Android fixes are 5–30 m — and it leaves the +/// fence measuring distance rather than luck. +const double kGeofenceMaxAccuracyMetres = 50; + +/// How long to wait for the GPS to settle before giving up on a live fix. +const Duration kGeofenceFixTimeout = Duration(seconds: 8); + +/// How long one live fix may serve several checks. +/// +/// ── Why this exists: a bulk action is ONE press at ONE place ── +/// +/// A rider ticking twenty bags at a counter and pressing "Picked up" produces +/// twenty calls to [Geofence.check], and without this each one took its own +/// `LocationAccuracy.best` fix with an 8-second ceiling. Twenty GPS settles for +/// a man who has not moved: 40–160 seconds of a rider standing at a counter +/// watching a spinner, which is most of the "status updates take two minutes" +/// report. +/// +/// Worse, the bulk arrival gate on Home worked around it with a 3-second +/// `.timeout(..., onTimeout: () => everything is inside the fence)`. Twenty +/// sequential fixes can never finish in three seconds, so that timeout fired +/// essentially every time and waved every order through — a hole straight +/// through the 100 m rule. One fix closes the hole by making the checks +/// instant, which is the better fix in both senses. +/// +/// ── Why fifteen seconds ── +/// +/// It is the window in which "where the rider is" has not meaningfully changed +/// against a 100 m fence. Walking pace is ~1.4 m/s, so fifteen seconds is about +/// 20 m of possible drift — a fifth of the radius, and well inside +/// [kGeofenceFixMaxAge], which already governs how old a position may be before +/// it stops being a measurement. Riding away from a stop at 30 km/h covers +/// 125 m in the same window, so a rider who genuinely leaves is outside the +/// fence on the next fix rather than the cached one: the cache is short enough +/// that it cannot carry him to the next door. +const Duration kGeofenceFixReuse = Duration(seconds: 15); + +/// Why the fence answered the way it did. +enum GeofenceOutcome { + /// Inside the radius. The only outcome that allows. + inside, + + /// Measured, and too far. [GeofenceDecision.distanceMetres] says how far. + outside, + + /// The booking carries no usable pin, so proximity cannot be checked. + /// + /// **Refused.** This used to be allowed-and-logged on the argument that the + /// rider cannot fix the hub's data from a doorstep. True, and it still made + /// the fence optional: a booking with a blank latitude opened every rung on + /// that stop. The office can fix a missing pin in a minute and the rider is + /// told to ring them. + noTarget, + + /// Location services are switched off on the handset. + serviceDisabled, + + /// The rider has not granted location permission. + permissionDenied, + + /// Permission is denied permanently; only Settings can restore it. + permissionDeniedForever, + + /// The GPS did not produce a fix in [kGeofenceFixTimeout] and there is no + /// recent cached one. + timeout, + + /// No fix at all, and not specifically a timeout. + noFix, + + /// The only fix available is older than [kGeofenceFixMaxAge]. + staleFix, + + /// The fix is worse than [kGeofenceMaxAccuracyMetres]. + poorAccuracy, + + /// The platform threw. Refused — see the class note on failing closed. + error, +} + +/// The fence's answer about one rung at one stop. +@immutable +class GeofenceDecision { + const GeofenceDecision({ + required this.allowed, + required this.outcome, + required this.timestamp, + this.distanceMetres, + this.accuracyMetres, + this.reason, + this.riderLat, + this.riderLng, + }); + + /// May the caller proceed to the API call? + /// + /// True for exactly one outcome — [GeofenceOutcome.inside]. Everything else + /// is a refusal, including every way of failing to measure. + final bool allowed; + + final GeofenceOutcome outcome; + + /// Metres from the rider to the stop, or null when it could not be measured. + final double? distanceMetres; + + /// The error radius the phone reported on the fix used, in metres. + final double? accuracyMetres; + + /// When the decision was taken. + final DateTime timestamp; + + /// The fix the decision was taken on, when there was one. + /// + /// Carried so a caller that has just been allowed through can stamp the same + /// position onto its payload instead of asking the GPS a second time. Two + /// fixes seconds apart are two different answers, and the one the fence + /// judged is the one the hub should be told about. + final double? riderLat; + final double? riderLng; + + /// One sentence, in the rider's words, naming what he must do. Null only when + /// [allowed] and there is nothing to say. + /// + /// Never a developer error, a status code, or a coordinate pair. + final String? reason; + + bool get isBlocked => !allowed; + + @override + String toString() => + 'GeofenceDecision(${outcome.name}, allowed=$allowed, ' + 'distance=${distanceMetres?.toStringAsFixed(1)}m, ' + 'accuracy=${accuracyMetres?.toStringAsFixed(0)}m)'; +} + +/// A position reading, or the reason there is not one. +@immutable +class GeofenceFix { + const GeofenceFix({this.position, this.failure}); + + /// A reading good enough to hand to the fence. + final Position? position; + + /// Why there is none. Ignored when [position] is set. + final GeofenceOutcome? failure; + + bool get hasPosition => position != null; +} + +/// Measures one rung against one stop. +/// +/// Stateless and static on purpose: Home's bulk gate and the per-stop rung in +/// `PickupsController` are different code paths that must never answer +/// differently, and the surest way to guarantee that is for there to be nothing +/// to construct differently. +abstract final class Geofence { + /// How the fence gets the rider's position. Swapped in tests. + /// + /// Kept as a settable function rather than a constructor argument because the + /// call sites are spread across controllers and widgets with no shared owner + /// to thread an instance through — and the alternative on offer was another + /// copy of the arithmetic. + static Future Function() positionProvider = _devicePosition; + + /// Restores the real device provider and drops any cached fix. + /// Call in `tearDown`. + static void useDevice() { + positionProvider = _devicePosition; + resetCache(); + } + + /// The last live fix, and when it was taken. See [kGeofenceFixReuse]. + static Position? _cachedFix; + static DateTime? _cachedAt; + + /// Forgets the cached fix. + /// + /// Called on sign-out and from tests. Also worth calling if a screen knows + /// the rider has travelled — though it should rarely be needed, because the + /// window is shorter than any journey between two stops. + static void resetCache() { + _cachedFix = null; + _cachedAt = null; + } + + /// True when a coordinate pair is a usable point on Earth. + /// + /// `0,0` is in the Gulf of Guinea and is what every unset latitude in this + /// codebase decays to, so it is treated as absent rather than as a place. + static bool isUsable(double? lat, double? lng) => + lat != null && + lng != null && + lat != 0 && + lng != 0 && + lat.abs() <= 90 && + lng.abs() <= 180; + + /// Metres between two points on the WGS-84 ellipsoid. + /// + /// Delegates to [Geolocator.distanceBetween] — never a latitude/longitude + /// comparison and never a flat-plane subtraction, both of which have been + /// tried in this app's history and are wrong by a factor that varies with + /// where the rider is standing. + static double distanceBetween( + double targetLat, + double targetLng, + double riderLat, + double riderLng, + ) => Geolocator.distanceBetween(targetLat, targetLng, riderLat, riderLng); + + /// The fence, for one [action] at one target. + /// + /// [action] is the rung's name in the rider's vocabulary — "Arrived", + /// "Delivered", "Handed over" — and is quoted back in + /// [GeofenceDecision.reason] so one sentence serves every rung. + static Future check({ + required double? targetLat, + required double? targetLng, + required String action, + double radiusMetres = kGeofenceRadiusMetres, + }) async { + final now = DateTime.now(); + final verb = action.toLowerCase(); + + // Logged with `debugPrint` in *every* build mode, deliberately. The whole + // risk of this switch is a build going out with proximity silently off, so + // the one thing it must never be is quiet. + if (!kGeofenceEnforced) { + debugPrint( + '[GEOFENCE] OFF for $action — this build was compiled with ' + 'ENFORCE_GEOFENCE=false. Stop completion is NOT proximity-verified.', + ); + return GeofenceDecision( + allowed: true, + outcome: GeofenceOutcome.inside, + timestamp: now, + ); + } + + // ── No pin on the booking: REFUSED ── + // + // Checked before the phone is asked for anything, because no fix can answer + // a question with no target and the rider should not wait 8s to be told so. + if (!isUsable(targetLat, targetLng)) { + final decision = GeofenceDecision( + allowed: false, + outcome: GeofenceOutcome.noTarget, + timestamp: now, + reason: + 'This stop has no map location on it, so Miler cannot confirm you ' + 'are there. Ring your office and ask them to add the address ' + 'location.', + ); + debugPrint( + '[GEOFENCE] $action refused: stop carries no coordinates ' + '($targetLat, $targetLng) — data gap on the booking', + ); + return decision; + } + + final GeofenceFix fix; + try { + fix = await positionProvider(); + } catch (e) { + // ── A throw is a refusal ── + // + // The old implementation caught here and returned `true` with the comment + // "allow on error (fail-safe)". That is fail-*open*: any throw from the + // location stack — and the permission plugin throws rather than returning + // on several OEM builds — opened every rung in the app. + debugPrint('[GEOFENCE] $action refused: position provider threw: $e'); + return GeofenceDecision( + allowed: false, + outcome: GeofenceOutcome.error, + timestamp: now, + reason: _reasonForFixFailure(GeofenceOutcome.error, verb), + ); + } + + if (!fix.hasPosition) { + final outcome = fix.failure ?? GeofenceOutcome.noFix; + final decision = GeofenceDecision( + allowed: false, + outcome: outcome, + timestamp: now, + reason: _reasonForFixFailure(outcome, verb), + ); + debugPrint('[GEOFENCE] $action refused: $decision'); + return decision; + } + + final pos = fix.position!; + final accuracy = pos.accuracy; + + // ── The rider's own position must be a place ── + // + // A plugin that answers `0,0` rather than throwing is what this catches; + // without it the fence measures from the Gulf of Guinea and refuses every + // stop in India with a distance in the thousands of kilometres. + if (!isUsable(pos.latitude, pos.longitude)) { + final decision = GeofenceDecision( + allowed: false, + outcome: GeofenceOutcome.noFix, + timestamp: now, + accuracyMetres: accuracy, + reason: _reasonForFixFailure(GeofenceOutcome.noFix, verb), + ); + debugPrint('[GEOFENCE] $action refused: $decision'); + return decision; + } + + final distance = distanceBetween( + targetLat!, + targetLng!, + pos.latitude, + pos.longitude, + ); + + // ── A fix too vague to decide with ── + // + // Refused rather than credited. Checked *after* the distance is computed so + // the decision still carries the measurement for the log, but before the + // radius comparison so a vague fix can never pass one. See + // [kGeofenceMaxAccuracyMetres]. + if (accuracy > kGeofenceMaxAccuracyMetres) { + final decision = GeofenceDecision( + allowed: false, + outcome: GeofenceOutcome.poorAccuracy, + timestamp: now, + distanceMetres: distance, + accuracyMetres: accuracy, + riderLat: pos.latitude, + riderLng: pos.longitude, + reason: + 'Your phone is not sure where you are yet. Step into the open for a ' + 'moment, then mark this stop $verb.', + ); + debugPrint('[GEOFENCE] $action refused: $decision'); + return decision; + } + + // ── Raw distance against the radius ── + // + // Not `distance - accuracy`. The slack that arithmetic granted grew with + // the phone's own error, so the worse the fix the further out a rider could + // stand — the control running backwards. The error is handled once, above, + // by refusing to decide on a fix that cannot resolve 100 m. + final inside = distance <= radiusMetres; + + final decision = GeofenceDecision( + allowed: inside, + outcome: inside ? GeofenceOutcome.inside : GeofenceOutcome.outside, + timestamp: now, + distanceMetres: distance, + accuracyMetres: accuracy, + riderLat: pos.latitude, + riderLng: pos.longitude, + reason: inside ? null : _tooFarSentence(distance, radiusMetres, verb), + ); + debugPrint( + '[GEOFENCE] $action | target ($targetLat, $targetLng) ' + '| rider (${pos.latitude}, ${pos.longitude}) | $decision', + ); + return decision; + } + + /// "You're 240 m away. Move within 100 m to mark this stop arrived." + /// + /// Distance first, because that is the number the rider acts on, and in the + /// unit he thinks in — metres under a kilometre, kilometres over it. + static String _tooFarSentence(double distance, double radius, String verb) { + final away = distance >= 1000 + ? '${(distance / 1000).toStringAsFixed(1)} km' + : '${distance.round()} m'; + return "You're $away away. Move within ${radius.round()} m to mark this " + 'stop $verb.'; + } + + static String _reasonForFixFailure(GeofenceOutcome outcome, String verb) => + switch (outcome) { + GeofenceOutcome.serviceDisabled => + 'Location is switched off on your phone. Turn it on, then mark this ' + 'stop $verb.', + GeofenceOutcome.permissionDenied => + 'Miler needs your location to confirm you are at this stop. Allow ' + 'location access, then mark it $verb.', + GeofenceOutcome.permissionDeniedForever => + 'Location access is blocked for Miler. Open Settings → Permissions → ' + 'Location and allow it, then mark this stop $verb.', + GeofenceOutcome.staleFix => + 'Your phone last found you a while ago. Wait a moment for it to catch ' + 'up, then mark this stop $verb.', + GeofenceOutcome.timeout => + 'Your phone is still looking for your location. Step into the open, ' + 'wait a moment, then mark this stop $verb.', + _ => + 'Your phone could not find your location, so this stop cannot be ' + 'marked $verb. Turn location on, allow it for Miler, and step ' + 'outside if you can.', + }; + + /// The real device fix: service, permission, then the best reading the + /// hardware will give inside [kGeofenceFixTimeout]. + /// + /// Falls back to a cached position **only** when it is fresher than + /// [kGeofenceFixMaxAge]; a last-known fix has no age limit of its own and on + /// a round it is reliably the previous stop. + static Future _devicePosition() async { + // ── One press at one place is one fix ── + // + // Checked before the service and permission calls, because those are + // platform-channel round trips too and a batch of twenty pays them twenty + // times for an answer that cannot have changed. A cached fix is only ever + // stored after a *successful* live read, so this can never serve a stale or + // vague one — the age check below is the only way in. + final cached = _cachedFix; + final at = _cachedAt; + if (cached != null && + at != null && + DateTime.now().difference(at) <= kGeofenceFixReuse) { + return GeofenceFix(position: cached); + } + + try { + if (!await Geolocator.isLocationServiceEnabled()) { + return const GeofenceFix(failure: GeofenceOutcome.serviceDisabled); + } + + var permission = await Geolocator.checkPermission(); + if (permission == LocationPermission.denied) { + permission = await Geolocator.requestPermission(); + } + if (permission == LocationPermission.deniedForever) { + return const GeofenceFix( + failure: GeofenceOutcome.permissionDeniedForever, + ); + } + if (permission == LocationPermission.denied) { + return const GeofenceFix(failure: GeofenceOutcome.permissionDenied); + } + + try { + final pos = await Geolocator.getCurrentPosition( + locationSettings: const LocationSettings( + // `best`, not `low`. A `low` fix is a ~1 km hint on Android; against + // a 100 m fence that is not a degraded measurement, it is noise + // being treated as evidence. + accuracy: LocationAccuracy.best, + timeLimit: kGeofenceFixTimeout, + ), + ); + _cachedFix = pos; + _cachedAt = DateTime.now(); + return GeofenceFix(position: pos); + } catch (_) { + // No live fix. A recent cached one is a measurement; a stale one is not. + final cached = await Geolocator.getLastKnownPosition(); + if (cached == null) { + return const GeofenceFix(failure: GeofenceOutcome.timeout); + } + final age = DateTime.now().difference(cached.timestamp); + if (age > kGeofenceFixMaxAge) { + debugPrint( + '[GEOFENCE] cached fix is ${age.inSeconds}s old — too stale to ' + 'measure a ${kGeofenceRadiusMetres.round()}m fence', + ); + return const GeofenceFix(failure: GeofenceOutcome.staleFix); + } + return GeofenceFix(position: cached); + } + } catch (e) { + debugPrint('[GEOFENCE] position lookup threw: $e'); + return const GeofenceFix(failure: GeofenceOutcome.error); + } + } +} diff --git a/lib/data/miler_api.dart b/lib/data/miler_api.dart index cb5d245..2b7c36e 100644 --- a/lib/data/miler_api.dart +++ b/lib/data/miler_api.dart @@ -336,11 +336,14 @@ class MilerApi { } // ═══════════════════════════════════════════════════════════════════════ - // AUTH (3 routes — reset-pin is intentionally not one of them) + // AUTH (4 routes — reset-pin is intentionally not one of them) // ═══════════════════════════════════════════════════════════════════════ /// Step 1. 404 when there is no account, 403 when the row is not role 5 or /// not Active — both surface as `ok: false` with the server's message. + /// + /// Answers `{success, message, phone, pin_set}` — **top level, not under + /// `data`**. Read the branch with [pinSetOf]; never off the message string. static Future login(String phone) => _send( 'POST', '/miler/login', @@ -352,6 +355,79 @@ class MilerApi { }, ); + /// Does this account already have a PIN on file? + /// + /// ── The one thing the login response is asked for ── + /// + /// Riders set their own PIN on first sign-in now; the console no longer + /// issues one. `pin_set` is how the server says which of the two screens the + /// rider is owed, and it is a **boolean** — the message beside it is prose + /// and prose gets reworded. Branching on "PIN verification required" would + /// break the day somebody improves that sentence. + /// + /// Null when the response did not carry the field at all: an older server, or + /// a failure. A caller that cannot tell should send the rider to Enter-PIN, + /// which is the safe direction — a rider who does have a PIN can use it, and + /// one who does not gets a refusal he can report, rather than being walked + /// into a Set-PIN screen that will 409. + static bool? pinSetOf(ApiResult res) { + final raw = res.raw; + final v = raw is Map ? (raw['pin_set'] ?? raw['pinSet']) : null; + if (v is bool) return v; + if (v is String) { + final t = v.toLowerCase(); + if (t == 'true') return true; + if (t == 'false') return false; + } + return null; + } + + /// First-time PIN creation, self-service. `POST /miler/set-pin`. + /// + /// ── This route did not exist, and its absence shaped the whole screen ── + /// + /// The only PIN-write the backend had was `POST /miler/reset-pin`, which + /// needs an ADMIN token — it was once open, and reset-pin followed by + /// verify-pin took over any rider account given nothing but a phone number. + /// So the app could not let a rider set a PIN at all, and `AuthProvider` + /// answered its own Create-MPIN screen with a manufactured 403 telling him + /// his office issues it. + /// + /// That is no longer true. This route is rider-authenticated by phone, + /// **cannot overwrite an existing PIN** (409 if one is set), and returns a + /// full session — the same `{token, user}` shape as [verifyPin] — so the + /// rider lands signed in without a second round trip. + /// + /// Refusals: `404` no such phone, `403` inactive or not a miler, `409` a PIN + /// already exists — send that rider to Enter-PIN instead. + static Future setPin({ + required String phone, + required String pin, + String? deviceToken, + }) async { + final res = await _send( + 'POST', + '/miler/set-pin', + auth: false, + body: { + 'phone': phone, + 'new_pin': pin, + 'configid': configId, + if (hasTenantId) 'tenantid': tenantId, + if (deviceToken != null && deviceToken.isNotEmpty) + 'device_token': deviceToken, + }, + ); + // Same token handling as verify-pin, deliberately: this IS a sign-in, and + // a second implementation of "where does the session come from" is how the + // two paths drift. + if (res.ok && res.raw is Map) { + final token = _str((res.raw as Map)['token']); + if (token.isNotEmpty) await ApiConfig.setToken(token); + } + return res; + } + /// Step 2. On success the token is stored and the caller gets `user`. /// /// The login is under `user` / `user.profile` — there is no `data` key on @@ -1001,10 +1077,24 @@ class MilerApi { /// /// The signature expires in ten minutes, so a failed upload is re-*signed* /// rather than retried against the old URL. + /// ── A pickup proof has no consignment, and must not pretend to ── + /// + /// The server builds the storage key as `{folder}/{tag}-{id}-…`, choosing the + /// id from `consignmentid`, then `bookingid`, then the rider. A pickup photo + /// is taken *before* `pickup-complete` mints anything, so there is no + /// consignment to key it on — and this method only offered `consignmentid`. + /// The one caller that needed it passed a **booking** id in that slot, so + /// every pickup proof this app has ever uploaded was filed under a + /// consignment number that does not exist, colliding with whatever real + /// consignment later took it. + /// + /// [bookingId] is the field the server already has for this case. Send + /// whichever one the stop actually has. static Future signUpload({ required String purpose, String contentType = 'image/jpeg', Object? consignmentId, + Object? bookingId, }) => _send( 'POST', '/miler/uploads/sign', @@ -1012,6 +1102,7 @@ class MilerApi { 'purpose': purpose, 'contentType': contentType, if (consignmentId != null) 'consignmentid': consignmentId, + if (bookingId != null) 'bookingid': bookingId, }, ); @@ -1034,6 +1125,35 @@ class MilerApi { File file, { required String purpose, Object? consignmentId, + Object? bookingId, + }) async => + (await uploadProofRef( + file, + purpose: purpose, + consignmentId: consignmentId, + bookingId: bookingId, + ))?.url; + + /// The same upload, returning **both** references the platform uses. + /// + /// ── Why the key matters, and why it was being thrown away ── + /// + /// `POST /miler/uploads/sign` answers with `uploadurl`, `url` **and `key`**. + /// This method used to keep only `url` and discard the key, which was fine + /// while the only consumer was `deliver` — that route takes a `photourl`. + /// + /// It is not fine for `POST /miler/bookings/:id/parcel`, whose `photos` field + /// wants the **storage key**, not a URL. The server's own note says why: the + /// customer is served a short-lived signed link *derived from* the key, never + /// a permanent one. Sending a URL there would store a link that either + /// expires or, worse, never does. + /// + /// So both come back and each caller takes the one its route is specified in. + static Future uploadProofRef( + File file, { + required String purpose, + Object? consignmentId, + Object? bookingId, }) async { if (!file.existsSync()) return null; @@ -1045,6 +1165,7 @@ class MilerApi { purpose: purpose, contentType: contentType, consignmentId: consignmentId, + bookingId: bookingId, ); if (!signed.ok) { debugPrint('[UPLOAD] sign failed: ${signed.status} ${signed.message}'); @@ -1057,6 +1178,7 @@ class MilerApi { .toString() .trim(); final publicUrl = (map?['url'] ?? '').toString().trim(); + final objectKey = (map?['key'] ?? '').toString().trim(); if (uploadUrl.isEmpty || publicUrl.isEmpty) { debugPrint('[UPLOAD] sign returned no url pair'); return null; @@ -1078,7 +1200,9 @@ class MilerApi { body: await file.readAsBytes(), ) .timeout(const Duration(seconds: 30)); - if (res.statusCode >= 200 && res.statusCode < 300) return publicUrl; + if (res.statusCode >= 200 && res.statusCode < 300) { + return UploadRef(url: publicUrl, key: objectKey); + } debugPrint('[UPLOAD] PUT ${res.statusCode} — ${res.body}'); } catch (e) { debugPrint('[UPLOAD] PUT failed: $e'); @@ -1169,7 +1293,10 @@ class MilerApi { if (heading != null) 'heading': _str(heading), if (accuracy != null) 'accuracy': _str(accuracy), if (status != null && status.isNotEmpty) 'status': status, - if (orderId != null) 'orderid': _str(orderId), + // The device's own stop key can carry a `#seq` destination suffix (see + // ApiConfig's adapter); the server has never seen one and this is a + // breadcrumb tag, not a join key. Send the booking's half. + if (orderId != null) 'orderid': _str(orderId).split('#').first, if (battery != null) 'battery': _str(battery), 'is_charging': isCharging, if (connection != null) 'connection': connection, @@ -1298,6 +1425,26 @@ class MilerApi { /// Dimensions are centimetres and weight is kilograms. They are what /// `pickup-complete` recomputes chargeable weight from, so a parcel submitted /// without them bills on the booked figure rather than the real one. +/// Where an uploaded image lives, in the two forms the platform uses. +/// +/// `deliver` takes [url] as `photourl`; `parcel` takes [key] as one entry of +/// `photos`. Neither is a substitute for the other — see [MilerApi.uploadProofRef]. +@immutable +class UploadRef { + const UploadRef({required this.url, required this.key}); + + /// The object's public URL. + final String url; + + /// The object's storage key — `{folder}/{tag}-{id}-{date}-{time}-{rand}.{ext}`. + /// + /// May be empty if an older server build does not return one; a caller that + /// needs it must treat empty as "no photo" rather than sending a blank entry. + final String key; + + bool get hasKey => key.isNotEmpty; +} + class ParcelEntry { final Object? parcelId; final double weight; @@ -1305,12 +1452,33 @@ class ParcelEntry { final double width; final double height; + /// Storage keys of the photographs taken of this parcel at the door. + /// + /// ── The field the app never sent ── + /// + /// `POST /miler/bookings/:id/parcel` has accepted `photos` for as long as the + /// route has existed, and the server's own note says why it matters: *"Weight + /// without a photograph is a number the customer has no way to check, and + /// this is the only point in the flow where anyone is standing next to the + /// parcel."* + /// + /// Meanwhile the rider app **compels** the photograph — `StopVerificationPage` + /// will not let him confirm without one — uploaded it to object storage, and + /// then dropped the reference on the floor. The evidence existed in a bucket + /// nobody could search, for every CX pickup this app has ever done. + /// + /// **Keys, not URLs.** The customer is served a short-lived signed link + /// derived from the key; a URL stored here either expires or never does. + /// See [MilerApi.uploadProofRef]. + final List photos; + const ParcelEntry({ this.parcelId, required this.weight, this.length = 0, this.width = 0, this.height = 0, + this.photos = const [], }); Map toJson() => { @@ -1319,6 +1487,10 @@ class ParcelEntry { 'length': length, 'width': width, 'height': height, + // Omitted entirely when there is none, rather than sent as `[]`: an empty + // array is a claim that no photograph was taken, and a failed upload is + // not that claim. + if (photos.isNotEmpty) 'photos': photos, }; } diff --git a/lib/data/milk_run.dart b/lib/data/milk_run.dart index 4b9c9ba..2de4fe9 100644 --- a/lib/data/milk_run.dart +++ b/lib/data/milk_run.dart @@ -78,9 +78,68 @@ class MilkRun { static const bool deliveryArrivalIsLocalOnly = true; /// Order id of a stop, however the payload spells it. + /// + /// On a customer-app pickup with several destinations this is the **stop** + /// key, not the booking's — `DM-482913#1` — because everything the device + /// remembers is filed under it and three doors sharing one key means three + /// doors sharing one memory. The suffix is added by `ApiConfig` where the row + /// is adapted; see the note there. [bookingKeyOf] is the plain booking key, + /// and `bookingreference` on the row is what the rider is shown. static String idOf(Map stop) => (stop['orderid'] ?? stop['orderId'] ?? '').toString(); + /// The booking this stop belongs to, with any destination suffix removed. + /// + /// Used where the fact being recorded is about the **visit** rather than the + /// drop. Collection is the case that matters: the rider collects a whole + /// pickup once, so the collected set is written with the booking key while + /// the delivery stops that come out of it each carry their own. See + /// [wasCollected]. + static String bookingKeyOf(Map stop) { + final raw = idOf(stop); + final cut = raw.indexOf('#'); + if (cut > 0) return raw.substring(0, cut); + final ref = (stop['bookingreference'] ?? '').toString().trim(); + return ref.isNotEmpty ? ref : raw; + } + + /// How many doors this one pickup visit is for. 1 for everything but a + /// multi-destination customer-app booking. + static int destinationCountOf(Map stop) => + int.tryParse((stop['destinationcount'] ?? '').toString()) ?? 1; + + /// Which door of the visit this stop is, counting from 0. + static int destinationSeqOf(Map stop) => + int.tryParse((stop['destinationseq'] ?? '').toString()) ?? 0; + + /// `Stop 2 of 3`, or `''` when the pickup has only one door. + /// + /// Empty rather than `Stop 1 of 1` deliberately: a label that appears on + /// every card in the app stops being read, and the fact it carries — this + /// bag is one of several from the same counter — is only ever news when + /// there are several. + static String stopLabel(Map stop) { + final count = destinationCountOf(stop); + if (count <= 1) return ''; + return 'Stop ${destinationSeqOf(stop) + 1} of $count'; + } + + /// True when the visit this stop came from has been collected. + /// + /// Asks for the stop's own key first and the booking's second, and the second + /// question is the one that matters. A multi-destination pickup is collected + /// **once**, under the booking key, while it is still a single pre-pickup + /// row; the poll after `pickup-complete` then replaces that row with one per + /// door, each carrying a key the collected set has never seen. Asking only + /// the stop key would drop every one of those bags back to "not collected" + /// at the moment the rider actually had them in his hands. + static bool wasCollected( + Map stop, + Set collectedIds, + ) => + collectedIds.contains(idOf(stop)) || + collectedIds.contains(bookingKeyOf(stop)); + /// The consignment this stop became once it was collected, or '' before that. /// /// The delivery route keys on this and not on the booking id — they come from @@ -326,7 +385,7 @@ class MilkRun { // this function did before and is still right for a milk run, whose rows // often carry no consignment state at all. if (!ServiceProfile.active.deliversToCustomer) return false; - if (collectedIds.contains(idOf(stop))) return true; + if (wasCollected(stop, collectedIds)) return true; final status = stopStatusOf(stop); return status.isPicked || status.isDeliveryLeg; } diff --git a/lib/data/mock_backend.dart b/lib/data/mock_backend.dart index 2ca040d..ec120f1 100644 --- a/lib/data/mock_backend.dart +++ b/lib/data/mock_backend.dart @@ -68,6 +68,19 @@ import 'package:flutter/foundation.dart'; /// flutter run --dart-define=MOCK_BACKEND=true # the canned day const bool kMockBackend = bool.fromEnvironment('MOCK_BACKEND'); +/// One customer pickup with three doors, for looking at. +/// +/// **Off unless asked for**, and separate from [kMockBackend] on purpose. The +/// bookings route above is deliberately empty — "inventing one here would put +/// fictional consignments in front of a parcel rider" — and that stays exactly +/// true by default. This is the one shape that cannot be reached any other way: +/// `GET /miler/bookings` only returns a row per destination once a +/// multi-destination customer-app booking has been collected, so seeing the +/// three-door card on a bench needs either a seeded backend or this. +/// +/// flutter run --dart-define=MOCK_BACKEND=true --dart-define=MOCK_MULTIDROP=true +const bool kMockMultiDrop = bool.fromEnvironment('MOCK_MULTIDROP'); + /// Canned answers for the routes the rider's day passes through. /// /// Everything else gets a generic success, on purpose: the alternative is a @@ -108,6 +121,103 @@ class MockBackend { /// of it and no log line makes it look like a real session. static const String token = 'MOCK-TOKEN-not-a-real-session'; + /// One collected customer pickup, as three rows — the shape + /// `milerStopsForBooking` returns once `pickup-complete` has fanned a + /// multi-destination booking out into a consignment per door. + /// + /// Written in the **raw wire shape**, not the app's stop shape, because that + /// is what the mock intercepts: these go through `ApiConfig.pickupFromBooking` + /// exactly as a real response would, so the adapter is being exercised rather + /// than bypassed. Same `bookingid` and `bookingreference` on all three, which + /// is the whole point — they are told apart only by `destinationseq`. + /// + /// Ids are `MOCK-` per rule 3 so `purgeDemoRecords()` can find anything that + /// leaks into a store. + static const List> multiDropVisit = [ + { + 'bookingid': 900482913, + 'bookingreference': 'MOCK-482913', + 'status': 'Converted_To_Consignment', + 'consignmentid': 900500, + 'consignmentstatus': 'Out_for_Delivery', + 'trackingno': 'MOCK-TRK-8841020', + 'destinationseq': 0, + 'destinationcount': 3, + 'next_action': 'deliver', + 'step': 1, + 'pickup_source_type': 'customer', + 'pickup_source_name': 'Ravi Kumar', + 'pickupaddress': '14, Cross Cut Road, Gandhipuram, Coimbatore 641012', + 'pickuplatitude': 11.0183, + 'pickuplongitude': 76.9725, + 'deliveryaddress': '22, Anna Nagar West, Chennai, Tamil Nadu', + 'deliverylatitude': 11.0210, + 'deliverylongitude': 76.9810, + 'recipientname': 'Anitha Raghavan', + 'recipientphone': '9840012201', + 'customername': 'Ravi Kumar', + 'customerphone': '9843311220', + 'parcels': [ + {'bookingparcelid': 900001, 'itemcategory': 'General'}, + {'bookingparcelid': 900002, 'itemcategory': 'General'}, + ], + }, + { + 'bookingid': 900482913, + 'bookingreference': 'MOCK-482913', + 'status': 'Converted_To_Consignment', + 'consignmentid': 900501, + 'consignmentstatus': 'Out_for_Delivery', + 'trackingno': 'MOCK-TRK-8841021', + 'destinationseq': 1, + 'destinationcount': 3, + 'next_action': 'deliver', + 'step': 2, + 'pickup_source_type': 'customer', + 'pickup_source_name': 'Ravi Kumar', + 'pickupaddress': '14, Cross Cut Road, Gandhipuram, Coimbatore 641012', + 'pickuplatitude': 11.0183, + 'pickuplongitude': 76.9725, + 'deliveryaddress': '7, Panampilly Nagar, Ernakulam, Kerala', + 'deliverylatitude': 11.0301, + 'deliverylongitude': 76.9902, + 'recipientname': 'Suresh Menon', + 'recipientphone': '9847712345', + 'customername': 'Ravi Kumar', + 'customerphone': '9843311220', + 'parcels': [ + {'bookingparcelid': 900003, 'itemcategory': 'General'}, + ], + }, + { + 'bookingid': 900482913, + 'bookingreference': 'MOCK-482913', + 'status': 'Converted_To_Consignment', + 'consignmentid': 900502, + 'consignmentstatus': 'Out_for_Delivery', + 'trackingno': 'MOCK-TRK-8841022', + 'destinationseq': 2, + 'destinationcount': 3, + 'next_action': 'deliver', + 'step': 3, + 'pickup_source_type': 'customer', + 'pickup_source_name': 'Ravi Kumar', + 'pickupaddress': '14, Cross Cut Road, Gandhipuram, Coimbatore 641012', + 'pickuplatitude': 11.0183, + 'pickuplongitude': 76.9725, + 'deliveryaddress': '90, Indiranagar 2nd Stage, Bengaluru, Karnataka', + 'deliverylatitude': 11.0405, + 'deliverylongitude': 77.0011, + 'recipientname': 'Meena Prakash', + 'recipientphone': '9880045512', + 'customername': 'Ravi Kumar', + 'customerphone': '9843311220', + 'parcels': [ + {'bookingparcelid': 900004, 'itemcategory': 'General'}, + ], + }, + ]; + /// Answers [method] [path], or null when the caller should fall through to /// the network. Null is never returned while [enabled] — see the class note. static Map? respond( @@ -160,7 +270,7 @@ class MockBackend { // at all. A parcel booking list has no fixture and inventing one here would // put fictional consignments in front of a parcel rider. if (route.endsWith('/bookings') || route.endsWith('/assignments')) { - return _ok(const []); + return _ok(kMockMultiDrop ? multiDropVisit : const []); } // ── Everything the rider writes ── diff --git a/lib/data/session.dart b/lib/data/session.dart new file mode 100644 index 0000000..40643c7 --- /dev/null +++ b/lib/data/session.dart @@ -0,0 +1,77 @@ +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:miler/data/accepted_store.dart'; +import 'package:miler/data/api_config.dart'; +import 'package:miler/data/geofence.dart'; +import 'package:miler/data/mutation_guard.dart'; +import 'package:miler/data/proof_store.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// ENDING A SESSION, IN ONE PLACE +/// +/// A rider's session ends two ways, and until now only one of them was +/// implemented: +/// +/// * **He presses Log out.** The Account screen tore everything down — +/// scoped stores, doorstep photos, bearer token, in-flight guard, cached +/// fix — raised `logged_out` and sent him to sign-in. +/// * **The server stops accepting his token.** `MilerApi` drops the token on +/// any 401 and calls `onUnauthorized`, which **nothing ever assigned**. So +/// the credential vanished and nothing else did. +/// +/// The second case left a state that looks signed in and cannot work: the +/// profile is still in SharedPreferences, `logged_out` is still `false`, so +/// the app draws the rider's own name over a dashboard whose every call comes +/// back `401 authorization header is required`. Observed on a real handset — +/// Karthikeyan CBE Rider, rider 23, name and tenant on screen, `authtoken` +/// absent from disk, Home / Deliveries / Activity and the background heartbeat +/// all failing in a loop. +/// +/// What that costs is not a blank screen. A rider opens the app, sees himself +/// signed in and sees no jobs, and concludes there is no work today — there is +/// no prompt anywhere telling him to sign in again. He waits; the stops go +/// undelivered; support cannot tell over the phone. +/// +/// So the teardown lives here, both callers use it, and neither can drift from +/// the other. The rule it encodes: **the credential and the appearance of +/// being signed in end together, always.** +/// ───────────────────────────────────────────────────────────────────────── + +/// Tears down everything this device holds about the signed-in rider. +/// +/// Safe to call twice — every step is idempotent — which matters because a 401 +/// rarely arrives alone: the home poll, the deliveries queue and the heartbeat +/// can all get one within the same second. +/// +/// Deliberately knows nothing about navigation. Where the rider goes next is a +/// UI decision and belongs to the caller; this is the part that must happen +/// identically whether he chose to leave or was shown the door. +Future endSession() async { + // Finished work, skipped stops, carried bags and bag labels are scoped per + // rider/tenant/line — this drops *this* scope so nothing survives into the + // next rider's session on a shared handset. + await clearScopedStores(); + + // Doorstep photos are exactly the kind of record that must not outlive the + // session that took them. + await ProofStore.clearScope(); + + // The credential goes with the session, not at the next sign-in. Anything + // that reads the token without checking a flag — a background isolate, the + // notification handler, a heartbeat that outlives a route change — must not + // be able to keep calling as the rider who has left. + await ApiConfig.clearToken(); + + // A stale in-flight mutation must not find a key still held from the session + // being closed. + MutationGuard.reset(); + + // A cached position is a fact about a rider and must not outlive him: the + // next person to sign in would have his first proximity check measured from + // wherever the last rider was standing. + Geofence.resetCache(); + + // The flag the launch path reads to decide between Home and sign-in. + final prefs = await SharedPreferences.getInstance(); + await prefs.setBool('logged_out', true); +} diff --git a/lib/data/task_profile.dart b/lib/data/task_profile.dart new file mode 100644 index 0000000..653264c --- /dev/null +++ b/lib/data/task_profile.dart @@ -0,0 +1,446 @@ +import 'package:flutter/foundation.dart'; + +import 'package:miler/data/next_leg.dart'; +import 'package:miler/data/service_profile.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// WHAT **THIS ONE JOB** REQUIRES — asked of the job, never of the rider +/// +/// There is one kind of Miler. The same rider can be carrying ten meal parcels +/// out of a kitchen, five customer pickups booked in the Doormile app, and a +/// hub handover, all inside one shift. So nothing about a rider can decide how +/// a stop behaves: a rider is not "a milk man" or "a logistics rider", he is a +/// person with a queue, and every item in that queue brings its own workflow. +/// +/// This is the replacement for reading [ServiceProfile.active]. That static +/// answers "who is this rider", which is the wrong question and gives the same +/// answer for every row on the screen — so a customer pickup sitting next to a +/// kitchen collection got the kitchen's rules, or vanished. +/// +/// ── Read, never inferred ── +/// +/// Every field below is derived from what the **server said about this row**: +/// `pickup_source_type` and `next_action`, both already on `GET /miler/bookings` +/// and both already parsed ([PickupSource], [NextLeg]). Nothing here looks at a +/// tenant, a business name, an address, a pincode or a missing id. Where the +/// server has not said, the answer is [WorkKind.unknown] and the screens draw +/// the plain, safe shape — the same doctrine [PickupSource] already states: +/// the server's word, or an honest "not known", never a guess. +/// ───────────────────────────────────────────────────────────────────────── +enum WorkKind { + /// Collect in bulk from a place of business — a kitchen, a base, a shop. + /// The meal round's morning load and an ordinary hub collection are the same + /// shape of work: one address, a manifest, many bags. + sourcePickup, + + /// Collect from a person's front door, against a booking that customer made + /// in the Doormile app. Needs the verify-at-the-door flow: count what is + /// actually there, weigh it, capture the destination(s), take payment if the + /// booking asks for it. + customerPickup, + + /// Already in the rider's hands, going to a receiver. + customerDelivery, + + /// Already in the rider's hands, going to a base. The network carries it from + /// there — this rider does not ride it to another city. + hubHandover, + + /// Nothing left for this rider on this item. + done, + + /// In custody, and the server has not said where it goes. Deliberately not + /// folded into [customerDelivery] or [done]: see [NextLeg.unknown], which + /// makes the same call for the same reason. + unknown, +} + +/// The capabilities one work item needs, resolved from that item alone. +@immutable +class TaskProfile { + const TaskProfile._({ + required this.kind, + required this.groupsBySource, + required this.leavesHomeOnAccept, + required this.deliversToCustomer, + required this.endsAtHub, + required this.capturesShipmentDetails, + required this.mayCollectCash, + }); + + /// What kind of work this row is. + final WorkKind kind; + + /// This stop is collected in bulk from a named place of business, so Home can + /// head it with that place — a kitchen load is one visit, not fourteen rows. + /// + /// ── NOT a drop-in for `ServiceProfile.active.sourceIsKitchen` ── + /// + /// Read this before migrating a grouping call site. `sourceIsKitchen` looks + /// like "does this group?" and is not: it **selects which key to group by**. + /// + /// • On the milk round, stops group under the **source they were collected + /// from** — the kitchen. + /// • On logistics, stops group under the **base that assigned the work**, + /// drops and collections alike, *including a collection made at a + /// customer's own door*. `hub_pickup_group_test.dart` states the rule: + /// "one building assigned them, so one node holds them". + /// + /// Substituting this flag for that one splits a single hub run into two + /// groups — a customer collection stops keying to the assigning base and + /// wanders off under its own name. That was tried, and those tests caught it. + /// + /// ── And DO NOT migrate a grouping site to this flag ── + /// + /// This was attempted twice and reverted twice. The second attempt is the + /// instructive one: with the safe default below, `hub_pickup_group_test.dart` + /// went green and the **milk round lost every kitchen heading** — + /// `service_flow_test.dart` reports `Found 0 widgets with text "Kitchen 2"`. + /// + /// The reason is the thing to understand before trying again: **the rows do + /// not carry the discriminator**. Neither the meal fixtures nor the logistics + /// fixtures set `pickup_source_type`, so per-stop classification cannot tell + /// a kitchen load from a customer door — both land on [WorkKind.unknown]. + /// `ServiceProfile.active.sourceIsKitchen` was not merely *deciding* the + /// grouping; it was *supplying information the payload lacks*. Removing it + /// removes the fact, and no default can be right for both lines at once: + /// grouping by default breaks logistics, not grouping breaks the milk round. + /// + /// What a grouping site actually needs is the stop's own **source identity**, + /// which the rows DO carry — `sourcename`, `pickuplocationid`, `sourceid`. + /// `MilkRun.sourceKeyOf` already reads exactly those and returns `id:…` or + /// `name:…` for a stop with a real named counter, falling back to `at:…` or + /// `none` for one without. That distinction is the grouping key, and it works + /// on today's payloads. Build on it, not on this flag. + /// + /// This flag stays honest about its own narrow claim: "the server said this + /// is collected from a business". It is correct for rows that carry + /// `pickup_source_type`, and it is silent — not wrong — for rows that do not. + final bool groupsBySource; + + /// Whether accepting moves it off Home immediately. + /// + /// A customer pickup does: it becomes an appointment the rider owes someone, + /// and it belongs on Bookings until he has collected it. A source pickup does + /// not: accepting a kitchen load does not put anything in his hands, so it + /// stays on Home until he is actually carrying it. + /// + /// Replaces `ServiceProfile.active.handoffAt`. + final bool leavesHomeOnAccept; + + /// Finishes at a receiver's door. + final bool deliversToCustomer; + + /// Finishes by handing over at a base. + final bool endsAtHub; + + /// The door flow has to capture what is being shipped and where it is going — + /// package count, weights, destinations, recipients. Only a customer pickup + /// does this; a kitchen manifest is already known before the rider arrives. + final bool capturesShipmentDetails; + + /// Money may change hands on this item. Whether it actually does is the + /// row's own COD figure — this only says the flow must be able to. + final bool mayCollectCash; + + /// The safe shape: a plain stop, no grouping, no door capture, no promises + /// about where it ends. + static const TaskProfile unknownTask = TaskProfile._( + kind: WorkKind.unknown, + groupsBySource: false, + leavesHomeOnAccept: false, + deliversToCustomer: false, + endsAtHub: false, + capturesShipmentDetails: false, + mayCollectCash: false, + ); + + static const TaskProfile sourcePickup = TaskProfile._( + kind: WorkKind.sourcePickup, + groupsBySource: true, + leavesHomeOnAccept: false, + deliversToCustomer: false, + endsAtHub: false, + capturesShipmentDetails: false, + mayCollectCash: false, + ); + + static const TaskProfile customerPickup = TaskProfile._( + kind: WorkKind.customerPickup, + groupsBySource: false, + leavesHomeOnAccept: true, + deliversToCustomer: false, + endsAtHub: false, + capturesShipmentDetails: true, + mayCollectCash: true, + ); + + static const TaskProfile customerDelivery = TaskProfile._( + kind: WorkKind.customerDelivery, + groupsBySource: false, + leavesHomeOnAccept: true, + deliversToCustomer: true, + endsAtHub: false, + capturesShipmentDetails: false, + mayCollectCash: true, + ); + + static const TaskProfile hubHandover = TaskProfile._( + kind: WorkKind.hubHandover, + groupsBySource: false, + leavesHomeOnAccept: true, + deliversToCustomer: false, + endsAtHub: true, + capturesShipmentDetails: false, + mayCollectCash: false, + ); + + static const TaskProfile done = TaskProfile._( + kind: WorkKind.done, + groupsBySource: false, + leavesHomeOnAccept: true, + deliversToCustomer: false, + endsAtHub: false, + capturesShipmentDetails: false, + mayCollectCash: false, + ); + + /// Which surface this item belongs on. The whole point of the type. + /// + /// • **Home** — assigned, not yet in the rider's hands, and not yet an + /// appointment he has taken on. + /// • **Bookings** — a customer visit he has accepted and still owes. + /// • **Deliveries** — anything already in his custody. + /// • **Activity** — finished. + WorkSurface surfaceWhen({required bool accepted}) => switch (kind) { + WorkKind.done => WorkSurface.activity, + WorkKind.customerPickup => + accepted ? WorkSurface.bookings : WorkSurface.home, + WorkKind.sourcePickup => WorkSurface.home, + WorkKind.customerDelivery || + WorkKind.hubHandover || + WorkKind.unknown => WorkSurface.deliveries, + }; + + /// Resolve a queue row. + /// + /// Order matters: `next_action` is asked first because it is the server's + /// statement about **where this parcel is in its life**, and that outranks + /// where it was collected from. A parcel picked up at a customer's door is a + /// delivery once it is in the bag; it stops being a pickup the moment it is + /// collected, and only the leg knows that. + static TaskProfile of(Map row) { + final leg = NextLegResolver.rowNextAction(row); + + switch (leg) { + case NextActionWord.deliver: + case NextActionWord.startDelivery: + return customerDelivery; + case NextActionWord.inwardAtHub: + return hubHandover; + case NextActionWord.handedToHub: + case NextActionWord.none: + return done; + } + + // Still to be collected — `next_action: pickup`, or a row too old to carry + // one. `stoptype` is the second witness: the server derives it from the + // booking status, so it answers even when next_action is absent. + final stopType = (row['stoptype'] ?? '').toString().trim().toLowerCase(); + if (leg.isEmpty && stopType == 'delivery') { + // Collected, and nothing said where it goes. Custody without a + // destination — NextLeg.unknown's case exactly. + return unknownTask; + } + + return switch (PickupSource.of(row)) { + PickupSource.customer => customerPickup, + PickupSource.hub || + PickupSource.merchant || + PickupSource.store => sourcePickup, + // ── No word from the server: take the safe shape, not a kind ── + // + // This said [sourcePickup] once, and that was the bug. `sourcePickup` + // carries `groupsBySource: true`, so every row too old to carry a + // `pickup_source_type` — which is every fixture in the existing suite, + // and every booking written before these fields shipped — started + // grouping under whatever name it could scrape together. A logistics day + // that is meant to bucket under one assigning base split into a group per + // customer. `hub_pickup_group_test.dart` caught it. + // + // An absent field is not evidence of a kitchen. It is evidence of + // nothing, and [unknownTask] is what nothing looks like: no grouping, no + // door capture, no promise about where it ends. Same doctrine + // [PickupSource] already states — the server's word, or an honest "not + // known", never a guess. + PickupSource.unknown => unknownTask, + }; + } +} + +/// The four places work can live in the rider app. +enum WorkSurface { home, bookings, deliveries, activity } + +/// What a whole trip looks like, derived from the stops actually in it. +/// +/// ── Why a trip needs its own answer ── +/// +/// A few things on Home describe the **day**, not one stop: whether it starts +/// at a kitchen, whether it ends by returning to a base. Those used to read the +/// rider's line, which made them constant for the whole app — true enough when +/// a rider only ever did one kind of work. +/// +/// Under mixed work they are no longer constant, and they are no longer even +/// exclusive: one trip can hold a kitchen load *and* five customer pickups, and +/// can finish by handing some parcels to a base while the last meal goes to +/// somebody's door. So the question changes from "which line is this rider on" +/// to "what is actually in this trip", and the honest answer is a fold over the +/// stops. +/// +/// [endsAtHub] is `any`, not `every`, deliberately: if even one parcel has to +/// be handed in at a base, the rider's day ends at that base. Telling him +/// "END · HOME" while he is still carrying something the network needs is the +/// failure worth avoiding. +abstract final class TripShape { + /// The trip collects in bulk somewhere — so Home can head that group with the + /// place it came from. + static bool groupsBySource(Iterable> stops) => + stops.any((s) => TaskProfile.of(s).groupsBySource); + + /// Something in the trip has to be handed over at a base before the rider is + /// finished. + static bool endsAtHub(Iterable> stops) => + stops.any((s) => TaskProfile.of(s).endsAtHub); + + /// The trip contains at least one customer visit the rider owes — the work + /// that belongs on Bookings rather than Home once accepted. + static bool hasCustomerPickup(Iterable> stops) => + stops.any((s) => TaskProfile.of(s).kind == WorkKind.customerPickup); +} + +/// ───────────────────────────────────────────────────────────────────────── +/// THE BRIDGE — per item where the row knows, per line where it does not +/// +/// [TaskProfile] is the right answer and it could not simply replace +/// [ServiceProfile.active] in one pass, for a reason worth stating plainly +/// rather than discovering twice: **most rows do not carry the discriminator +/// yet.** `pickup_source_type` and `next_action` shipped recently; a booking +/// written before them, and every fixture in the existing suite, carries +/// neither. [TaskProfile.of] correctly answers [WorkKind.unknown] for those, +/// and a screen that took `unknown` at face value would lose the milk round its +/// kitchen headings and the logistics day its door capture — which is what +/// happened both times a grouping site was migrated wholesale. +/// +/// So this is the migration path, not a second source of truth: +/// +/// the row said something → the row decides, per item +/// the row said nothing → the rider's line decides, exactly as before +/// +/// Every method below is that shape. The consequence is the one that matters +/// for mixed work: a CX customer pickup sitting next to a kitchen collection +/// now gets **its own** rules on the strength of its own `pickup_source_type`, +/// instead of inheriting whatever the rider's tenant happens to say. The +/// fallback keeps every existing row behaving precisely as it does today, which +/// is what makes this safe to ship in the same release as the fence. +/// +/// As the backend fills these fields in on every row (see handoff BE-6), the +/// fallbacks stop being reachable and can be deleted a call site at a time. +/// ───────────────────────────────────────────────────────────────────────── +abstract final class WorkPolicy { + /// Does the rider have to establish what is being shipped at this stop — + /// count the parcels, weigh them, capture destination and recipient, take + /// payment? + /// + /// This is the question that most needed taking off the rider. It had exactly + /// one call site, `ServiceProfile.active.needsVerification`, and that static + /// gives the *same answer for every row on the screen*: on a meal tenant a CX + /// customer pickup was walked straight past the door flow — no parcel count, + /// no destination, no weight — and the booking pivoted on whatever the + /// customer had typed into the app days earlier. On a parcel tenant the + /// reverse: a kitchen collection of fourteen known bags asked the rider to + /// itemise a manifest that was already on his screen. + /// + /// A customer pickup needs it. A collection from a counter does not — the + /// manifest is known before he arrives. + static bool capturesShipmentDetails(Map row) { + final profile = TaskProfile.of(row); + if (profile.kind == WorkKind.unknown) { + return ServiceProfile.active.needsVerification; + } + return profile.capturesShipmentDetails; + } + + /// Does this stop establish the shipment's addresses and price — the + /// logistics desk, as against a proof-of-collection photo? + /// + /// Same shape and the same reason. Falls back to + /// `ServiceProfile.active.capturesShipmentAddresses`. + static bool capturesShipmentAddresses(Map row) { + final profile = TaskProfile.of(row); + if (profile.kind == WorkKind.unknown) { + return ServiceProfile.active.capturesShipmentAddresses; + } + return profile.capturesShipmentDetails; + } + + /// Does collecting here bring a shipment into existence? + static bool initiatesShipment(Map row) { + final profile = TaskProfile.of(row); + if (profile.kind == WorkKind.unknown) { + return ServiceProfile.active.initiatesShipment; + } + return profile.kind == WorkKind.customerPickup; + } + + /// Does an accepted stop stay on Home until the rider is actually carrying + /// it, rather than moving to Bookings the moment he accepts? + /// + /// The per-item replacement for `ServiceProfile.active.handsOffAtCollection` + /// (`handoffAt == HandoffPoint.collected`). A **source pickup** stays: taking + /// on a kitchen load puts nothing in his hands and he still has to go and + /// collect it. A **customer pickup** does not: accepting it is an appointment + /// he now owes somebody, and it belongs on Bookings until he has been. + /// + /// Asking the rider's line for this gave one answer for every row on the + /// screen — so on a meal tenant an accepted CX pickup sat on Home beside ten + /// kitchen bags with nothing to distinguish it, and on a parcel tenant a + /// kitchen load vanished off Home the instant it was accepted. + static bool staysOnHomeUntilCollected(Map row) { + final profile = TaskProfile.of(row); + if (profile.kind == WorkKind.unknown) { + return ServiceProfile.active.handsOffAtCollection; + } + return !profile.leavesHomeOnAccept; + } + + /// Which surface this item belongs on. + /// + /// The whole point of [TaskProfile], and the answer the four-tab shell cannot + /// yet act on in full — see the release report. Correct per item today, so + /// the queues can be split without re-deriving the rule. + static WorkSurface surfaceOf( + Map row, { + required bool accepted, + }) => TaskProfile.of(row).surfaceWhen(accepted: accepted); + + /// True when this one item ends by handing over at a base. + /// + /// Read from the row's `next_action`. Unlike the flags above there is no + /// fallback and none is wanted: [TaskProfile.hubHandover] is reached only + /// when the server has said `inward_at_hub`, and guessing a base leg from the + /// rider's line is how a hyperlocal parcel gets routed to a counter. + static bool endsAtHub(Map row) => + TaskProfile.of(row).kind == WorkKind.hubHandover; + + /// True when this one item ends at a receiver's door. + /// + /// Falls back to the line, because a row with no `next_action` genuinely does + /// not say — and on a meal run every stop does end at a door. + static bool deliversToCustomer(Map row) { + final profile = TaskProfile.of(row); + if (profile.kind == WorkKind.unknown) { + return ServiceProfile.active.deliversToCustomer; + } + return profile.deliversToCustomer; + } +} diff --git a/lib/data/work_domain.dart b/lib/data/work_domain.dart index 2f22b36..b6ef141 100644 --- a/lib/data/work_domain.dart +++ b/lib/data/work_domain.dart @@ -87,7 +87,9 @@ abstract final class WorkBoundary { }) { final status = stopStatusOf(stop); if (status.isPicked || status.isDeliveryLeg) return true; - return collectedIds.contains(MilkRun.idOf(stop)); + // Booking key as well as stop key: a multi-destination pickup is collected + // once, before it splits into one row per door. See [MilkRun.wasCollected]. + return MilkRun.wasCollected(stop, collectedIds); } /// True when this order is finished or withdrawn, whatever screen it was on. diff --git a/lib/data/work_scope.dart b/lib/data/work_scope.dart index 4fef863..165fdf7 100644 --- a/lib/data/work_scope.dart +++ b/lib/data/work_scope.dart @@ -42,13 +42,8 @@ import 'package:miler/data/service_profile.dart'; class WorkScope { final int userId; final int tenantId; - final ServiceLine line; - const WorkScope({ - required this.userId, - required this.tenantId, - required this.line, - }); + const WorkScope({required this.userId, required this.tenantId}); /// The scope of the session running right now. /// @@ -65,26 +60,17 @@ class WorkScope { ? raw : int.tryParse(raw?.toString() ?? '') ?? 0; final tenantId = await ApiConfig.storedTenantId(); - return WorkScope( - userId: userId, - tenantId: tenantId, - line: ServiceProfile.active.line, - ); + return WorkScope(userId: userId, tenantId: tenantId); } catch (e) { debugPrint('[SCOPE] could not resolve the session scope: $e'); - return WorkScope( - userId: 0, - tenantId: 0, - line: ServiceProfile.active.line, - ); + return const WorkScope(userId: 0, tenantId: 0); } } /// A signed-out or unreadable session. Deliberately still a real scope /// rather than null: code paths that run before login write to their own /// drawer instead of into the last rider's. - static WorkScope get anonymous => - WorkScope(userId: 0, tenantId: 0, line: ServiceProfile.active.line); + static WorkScope get anonymous => const WorkScope(userId: 0, tenantId: 0); /// The suffix that turns a store key into this scope's key. /// @@ -93,11 +79,36 @@ class WorkScope { /// All three parts are in it because all three can change independently: a /// rider can move tenant, a tenant can run either line, and one device can /// see several riders. - String get key => 'u$userId.t$tenantId.${line.name}'; + String get key => 'u$userId.t$tenantId'; /// Scopes a legacy global key. String scoped(String baseKey) => '$baseKey::$key'; + /// The keys this scope's records used to live under, newest convention first. + /// + /// ── Why the line came out of the key ── + /// + /// The key used to carry a third part, the rider's service line: + /// `completed_bookings::u38.t13.milkMan`. That was right while a rider was + /// one thing for the life of his account. He is not: one Miler carries meal + /// parcels, logistics collections and customer pickups in the same shift, and + /// keying his records by a line splits one day's work across two drawers — + /// with whichever drawer the app resolved into today being the only one he + /// can see. Work he finished an hour ago disappears because a label changed. + /// + /// Rider and tenant still scope it. Those are facts about *who* stored the + /// record, which is what ownership means. The line was a fact about *what he + /// was doing*, which belongs on the work item, not on the drawer. + /// + /// Every historical spelling is listed so a rider upgrading mid-shift keeps + /// what he has already done. + Iterable legacyScopedKeys(String baseKey) sync* { + for (final line in ServiceLine.values) { + yield '$baseKey::u$userId.t$tenantId.${line.name}'; + } + } + + /// Does [record] provably belong to this scope? /// /// Used on rows that were written before scoping existed, and as a @@ -127,12 +138,9 @@ class WorkScope { 'scopeuserid', ]); final rowTenant = intOf(const ['tenantid', 'TenantId', 'scopetenantid']); - final rowLine = (record['scopeline'] ?? '').toString(); - - if (rowUser == 0 && rowTenant == 0 && rowLine.isEmpty) return false; + if (rowUser == 0 && rowTenant == 0) return false; if (rowUser != 0 && rowUser != userId) return false; if (rowTenant != 0 && rowTenant != tenantId) return false; - if (rowLine.isNotEmpty && rowLine != line.name) return false; return true; } @@ -168,11 +176,8 @@ class WorkScope { 'scopeuserid', ]); final rowTenant = intOf(const ['tenantid', 'TenantId', 'scopetenantid']); - final rowLine = (record['scopeline'] ?? '').toString(); - if (rowUser != 0 && userId != 0 && rowUser != userId) return true; if (rowTenant != 0 && tenantId != 0 && rowTenant != tenantId) return true; - if (rowLine.isNotEmpty && rowLine != line.name) return true; return false; } @@ -184,18 +189,16 @@ class WorkScope { ...record, 'scopeuserid': userId, 'scopetenantid': tenantId, - 'scopeline': line.name, }; @override bool operator ==(Object other) => other is WorkScope && other.userId == userId && - other.tenantId == tenantId && - other.line == line; + other.tenantId == tenantId; @override - int get hashCode => Object.hash(userId, tenantId, line); + int get hashCode => Object.hash(userId, tenantId); @override String toString() => 'WorkScope($key)'; diff --git a/lib/main.dart b/lib/main.dart index 2b595a7..ed2e08b 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -1,4 +1,3 @@ -import 'dart:async' show unawaited; import 'dart:io'; import 'package:miler/helpers/http_overrides.dart'; import 'package:flutter/material.dart'; @@ -22,7 +21,10 @@ import 'package:flutter_screenutil/flutter_screenutil.dart'; import 'package:miler/background/backgroundservice.dart'; import 'package:miler/helpers/shift_end_alarm.dart'; import 'package:miler/data/accepted_store.dart'; +import 'package:miler/data/miler_api.dart'; import 'package:miler/data/service_profile.dart'; +import 'package:miler/data/session.dart'; +import 'package:miler/views/onboardscreens/Sign_in.dart'; import 'package:miler/xpress/delivery_bootstrap.dart'; import 'package:miler/views/helpers/constants/app_theme.dart'; import 'package:miler/views/helpers/widgets/page_transitions.dart'; @@ -77,9 +79,29 @@ Future recheckVersion() async { Future main() async { WidgetsFlutterBinding.ensureInitialized(); - // One-time drain of the pre-scoping global stores. See [migrateLegacyStores] - // for why unattributable rows are dropped rather than adopted. - unawaited(migrateLegacyStores()); + // ── Awaited, because the screens read what it writes ── + // + // This was `unawaited(...)`, which meant the one-time drain of the + // pre-scoping stores raced the first frame. On the launch after an upgrade — + // the only launch where it does anything — Home, Deliveries and Activity + // could all read the scoped key *before* the migration had written it, and a + // rider mid-shift opened the app to an empty day. The rows were not lost; he + // simply could not see them until he killed and reopened the app, which is + // not a thing he knows to do. + // + // It is cheap on every other launch: `containsKey` against an already-loaded + // preferences map, for eight base keys, and it returns before the first + // `await` when there is nothing to move. + // + // Guarded, and deliberately not fatal. A migration that throws — a corrupt + // blob, a prefs backend that will not open — must not stop the rider signing + // in and working. He loses local history he can no longer see anyway; the + // server's record is untouched by any of this. + try { + await migrateLegacyStores(); + } catch (e, st) { + debugPrint('[SCOPE] legacy store migration failed, continuing: $e\n$st'); + } HttpOverrides.global = MyHttpOverrides(); try { @@ -102,6 +124,25 @@ Future main() async { // way to receive it at all. Refreshed from `GET /miler/profile` first so the // resolution below reads the current answer. Cheap and non-blocking for a // rider we already have a name for; see [refreshTenantFromProfile]. + // ── What happens when the server stops accepting this rider ── + // + // `MilerApi` drops the bearer token on any 401 and calls `onUnauthorized`. + // Nothing ever assigned it, so the second half never happened: the credential + // was deleted and the app went on believing it was signed in, because the + // profile and `logged_out = false` were both still on disk. Every call after + // that came back `401 authorization header is required`, forever, behind a + // dashboard showing the rider's own name. + // + // Observed on a handset: rider 23, name and tenant drawn on Account, no + // `authtoken` in SharedPreferences, Home / Deliveries / Activity and the + // heartbeat all failing in a loop. What it costs is not a blank screen — it + // is a rider who sees no jobs, concludes there is no work today, and is never + // told to sign in again. + // + // Registered here, before the line branch below, because both lines make + // authenticated calls and either can be the one to receive the 401. + MilerApi.onUnauthorized = _handleSessionExpired; + await refreshTenantFromProfile(); final profile = await TenantController.to.load(); @@ -140,6 +181,53 @@ Future main() async { runApp(const _RootApp()); } +/// True while a session-expiry teardown is already under way. +/// +/// A 401 rarely arrives alone — the home poll, the deliveries queue and the +/// background heartbeat can each get one within the same second, and every one +/// of them calls this. Without the latch the rider would be torn down three +/// times and pushed at the sign-in screen three times, which on GetX stacks +/// three routes he then has to dismiss. +bool _signingOut = false; + +/// Ends the session the server has stopped accepting, and says so. +/// +/// Deliberately not silent. The rider did not choose this, so he is told what +/// happened in the words of the thing that happened — his session ended — and +/// landed somewhere he can act on it. Dropping him on sign-in with no message +/// reads as the app having crashed and lost his work. +Future _handleSessionExpired() async { + if (_signingOut) return; + _signingOut = true; + try { + await endSession(); + + // A 401 can reach a background isolate, where there is no navigator and + // `Get.offAll` would throw. The teardown above has still happened, so the + // next launch reads `logged_out` and opens sign-in — the rider is not left + // holding a dead session either way. + if (Get.context == null) { + debugPrint('[AUTH] session expired with no UI attached — ' + 'cleared, sign-in will open on next launch'); + return; + } + + Get.offAll(() => const SignIn()); + Get.snackbar( + 'Signed out', + 'Your session ended. Please sign in again to see your jobs.', + snackPosition: SnackPosition.BOTTOM, + ); + } catch (e, st) { + // Never let the teardown itself throw into an API response handler: the + // call that received the 401 has its own error path and this must not + // replace it with a crash. + debugPrint('[AUTH] session teardown failed: $e\n$st'); + } finally { + _signingOut = false; + } +} + /// Paints the status and navigation bars to match the page behind them. void _applySystemChrome() { SystemChrome.setSystemUIOverlayStyle( diff --git a/lib/providers/auth/auth_provider.dart b/lib/providers/auth/auth_provider.dart index b5ef2be..770a24a 100644 --- a/lib/providers/auth/auth_provider.dart +++ b/lib/providers/auth/auth_provider.dart @@ -93,6 +93,11 @@ class AuthProvider { required String fcmToken, int? pin, String? pinRaw, + + /// True for a rider setting his PIN for the first time — routes to + /// `POST /miler/set-pin` and sends the digits as `new_pin`. The response, + /// the token handling and the persisted identity are identical either way. + bool firstTime = false, }) async { // The legacy `rider/login` path on the retired backend that used to sit // behind a flag here is gone with the rest of the old backend. @@ -101,6 +106,8 @@ class AuthProvider { pin: pin, pinRaw: pinRaw, fcmToken: fcmToken, + path: firstTime ? '/miler/set-pin' : '/miler/verify-pin', + pinField: firstTime ? 'new_pin' : 'pin', ); } @@ -117,13 +124,27 @@ class AuthProvider { /// AssignMilerToBooking pushes "New Pickup Assigned" to exactly that token. /// Omitting it leaves the profile's token empty and silently makes the app /// poll-only. + /// ── One normaliser, two routes ── + /// + /// `POST /miler/set-pin` answers with the **same envelope** as verify-pin — + /// `{success, token, tenantid, tenantname, user:{authname, contactno, + /// profile:{…}}}` — because it is also a sign-in: a rider who sets his PIN is + /// logged in by that one call and does not verify afterwards. + /// + /// So the path and the PIN field name are arguments rather than a second copy + /// of this function. Everything below — finding the token in any of the nine + /// places this contract has worn it, splitting the display name, building the + /// legacy `{status, details:{…}}` envelope that [loginParsed] persists from — + /// is the part that must not drift between the two paths, and now cannot. Future _loginNew({ required String contactNo, int? pin, String? pinRaw, String? fcmToken, + String path = '/miler/verify-pin', + String pinField = 'pin', }) async { - final uri = Uri.parse(ApiConfig.url('/miler/verify-pin')); + final uri = Uri.parse(ApiConfig.url(path)); // The backend bcrypt-compares the PIN as a STRING, so a leading zero is // significant. Prefer the raw text the rider typed — round-tripping through // int drops it ("0512" -> 512 -> "512") and fails a valid PIN. @@ -132,7 +153,7 @@ class AuthProvider { : pin?.toString(); final body = { 'phone': contactNo, - if (pinValue != null) 'pin': pinValue, + if (pinValue != null) pinField: pinValue, // Riders live in partition 1001. It defaults server-side, so omitting it // appeared to work — but a miler row created without it can never log in, // and sending it explicitly is the only way the app and the console agree @@ -168,7 +189,7 @@ class AuthProvider { // Signing in with nothing to sign in to. This is the one call that cannot // be intercepted in [MilerApi] — auth posts its own request, because it // runs before there is a token for the shared transport to attach. - final mocked = MockBackend.respond('POST', '/miler/verify-pin', body: body); + final mocked = MockBackend.respond('POST', path, body: body); final http.Response res = mocked != null ? http.Response( @@ -451,6 +472,7 @@ class AuthProvider { required String fcmToken, int? pin, String? pinRaw, + bool firstTime = false, }) async { final res = await login( contactNo: contactNo, @@ -460,6 +482,7 @@ class AuthProvider { fcmToken: fcmToken, pin: pin, pinRaw: pinRaw, + firstTime: firstTime, ); final Map jsonMap = res.body.isNotEmpty ? json.decode(res.body) as Map @@ -608,51 +631,39 @@ class AuthProvider { return Login.fromJson(jsonMap); } + /// Deprecated. `POST /miler/reset-pin` is admin-only and this app must never + /// call it — it was once open, and reset-pin followed by verify-pin took over + /// any rider account given nothing but a phone number. + /// + /// ── The 403 this used to manufacture is gone ── + /// + /// There was no rider-facing PIN write, so this returned a synthetic + /// `403 "Your MPIN is issued by your office and cannot be changed from the + /// app."` — true when written, and a dead end for the Create-MPIN screen. + /// + /// `POST /miler/set-pin` is that route, shipped 2026-09-16. It is + /// self-service, cannot overwrite an existing PIN (409), and returns a full + /// session. Riders set their own PIN on first sign-in and the console no + /// longer issues one, so nothing needs this method any more. + @Deprecated('Use AuthController.setPin, which calls POST /miler/set-pin.') Future updatePin({ required int userId, required int pin, }) async { - // ── There is no rider-facing set-PIN endpoint, and that is deliberate ── - // - // The only PIN-write route on the backend is `POST /miler/reset-pin`, and - // it requires an ADMIN token. It was once open, and reset-pin followed by - // verify-pin took over any rider account given nothing but a phone number. - // The app must not call it; rider PIN resets go through ops. - // - // So this stays a no-op success: the Create-MPIN screen's flow completes - // and the PIN the account was issued with remains the one that works. - // Making it fail instead would strand a rider on a screen with no way - // forward, which is worse and no more honest. - // ── It used to answer 200 ── - // - // "Making it fail instead would strand a rider on a screen with no way - // forward, which is worse and no more honest." Half of that was right and - // the conclusion was wrong. What the manufactured 200 actually did: - // - // 1. Create-MPIN told the rider his new MPIN was saved. - // 2. The app wrote it to `dbPin` locally. - // 3. The server never heard about it. - // 4. Every login from then on returned `incorrect PIN`, forever, with no - // way for the rider to tell that the PIN he was typing had never - // existed anywhere but his own handset. - // - // Being stranded on a screen that tells you who can help is not worse than - // that. It is the only version of this that a rider can act on. ApiConfig.logGap( 'updatePin', - 'No rider-facing set-PIN route; reset-pin is admin-only by design. ' - 'Refusing rather than reporting a write that did not happen.', + 'reset-pin is admin-only; first-time PIN creation goes through ' + 'POST /miler/set-pin. This method should have no callers.', ); return http.Response( json.encode({ 'status': false, - 'code': 403, + 'code': 410, 'message': - 'Your MPIN is issued by your office and cannot be changed from the ' - 'app. Ask your supervisor to reset it, then sign in with the MPIN ' - 'they give you.', + 'This app no longer changes PINs through reset-pin. Set your PIN ' + 'on the sign-in screen.', }), - 403, + 410, headers: {'content-type': 'application/json'}, ); } diff --git a/lib/providers/pickuplog/pickuplog_provider.dart b/lib/providers/pickuplog/pickuplog_provider.dart index 98988da..8986ed7 100644 --- a/lib/providers/pickuplog/pickuplog_provider.dart +++ b/lib/providers/pickuplog/pickuplog_provider.dart @@ -240,6 +240,48 @@ class UpdatePickupProvider { // consignment id, and pressing **Delivered** at the door was refused by // the app itself. Recorded here, at the only moment it is guaranteed to // be in hand. See [rememberConsignmentId]. + // ── What the rider captured, and what the contract can carry ── + // + // `POST /miler/bookings/:id/pickup-complete` takes **latitude and + // longitude and nothing else**. The door flow, meanwhile, *demands* a + // photograph of the parcels and — on a short count or a broken seal — + // a written explanation before it will let the rider confirm + // (`StopVerificationPage`, `_needsPickupNote`). + // + // So the app was compelling a rider to produce exactly the evidence + // that matters in a dispute, uploading the photo to object storage, and + // then dropping the URL and the note on the floor: this method reads + // `dropimage` for the delivery leg and has never read `pickupimage`, + // `proofimage` or `notes` on this one. Nobody at the hub could see any + // of it, and nobody knew it was missing. + // + // Two things change here, and neither of them invents a field: + // + // * the photo and the note are kept **on the device**, against the + // order, so the record exists and the office can ask for it — see + // `ProofStore` and the completed-stop record; + // * the gap is *logged*, every time, with what was dropped. It is the + // same channel every other missing route reports through, so it + // shows up where the rest of the contract gaps do instead of being + // a thing one person remembers. + // + // BACKEND DEPENDENCY: see handoff BE-2. When `pickup-complete` accepts + // `pickupimageurl` / `notes` / `condition`, send them here and delete + // this block. + final droppedProof = (data['pickupimage'] ?? data['proofimage'] ?? '') + .toString() + .trim(); + if (droppedProof.isNotEmpty || notes.trim().isNotEmpty) { + ApiConfig.logGap( + 'pickup-complete', + 'The rider captured evidence this route cannot carry — ' + 'photo=${droppedProof.isNotEmpty ? 'yes' : 'no'}, ' + 'note=${notes.trim().isNotEmpty ? '"${notes.trim()}"' : 'none'}. ' + 'pickup-complete accepts latitude/longitude only. Held on the ' + 'device against booking $id. See BE-2.', + ); + } + final picked = await MilerApi.pickupComplete(id, lat: lat, lon: lon); // ── Which lifecycle the server is running, read from the server ── // @@ -566,7 +608,37 @@ class UpdatePickupProvider { // and it keeps the chargeable total correct even though the per-parcel // figures are an even split rather than a measurement. final double each = totalWeight / count; - final parcels = [for (var i = 0; i < count; i++) ParcelEntry(weight: each)]; + + // ── The photographs, at last ── + // + // `photos` has been on this contract since the route shipped and the app + // has never sent it. The keys arrive on the verification map (see + // `_withParcelPhotos`); an empty list means the upload failed, and the + // field is then omitted rather than sent empty — an empty array is a claim + // that nobody photographed anything. + // + // Attached to the FIRST parcel only. The rider takes one photograph of the + // load, not one per box — repeating the same key on every parcel would + // report N photographs where one was taken. + final photoKeys = [ + for (final k in (pickup['photos'] as List?) ?? const []) + if (k.toString().trim().isNotEmpty) k.toString().trim(), + ]; + + final parcels = [ + for (var i = 0; i < count; i++) + ParcelEntry( + weight: each, + photos: i == 0 ? photoKeys : const [], + ), + ]; + if (photoKeys.isEmpty) { + ApiConfig.logGap( + 'submitParcels', + 'booking $bookingId: no photo key available — the parcels are being ' + 'recorded without the door photograph.', + ); + } final res = await MilerApi.submitParcels(bookingId, parcels); debugPrint( diff --git a/lib/views/Dashboard/home/homepage.dart b/lib/views/Dashboard/home/homepage.dart index d7677e3..f8f528d 100644 --- a/lib/views/Dashboard/home/homepage.dart +++ b/lib/views/Dashboard/home/homepage.dart @@ -67,6 +67,7 @@ import 'package:miler/data/order_manifest.dart'; import 'package:miler/data/milk_run.dart'; import 'package:miler/data/order_events.dart'; import 'package:miler/data/work_domain.dart'; +import 'package:miler/data/geofence.dart'; import 'package:miler/data/work_repository.dart'; import 'package:miler/data/pickup_locations.dart'; import 'package:miler/data/service_day.dart'; @@ -600,55 +601,67 @@ class _HomepageState extends State } // ==================== PROXIMITY METHODS ==================== - Future _isNearPickupLocation(Map Booking) async { - // ── The second proximity gate ── - // - // `PickupsController._checkGeofence` guards a single stop. This one guards - // the *bulk* "mark selected as arrived" action on Home, and it runs before - // the controller is ever called. - // - // It honours the same switch and, since this change, the same **radius**. - // It was hardcoded to 500 m while the controller read a configured 100 — - // so the app had two fences of different sizes on two routes into the same - // rung, and which one a rider met depended on whether he had ticked boxes - // or slid a sheet. One number now: [kGeofenceRadiusMeters]. - if (kBypassGeofenceForTesting) { - debugPrint( - '[GEOFENCE] OFF for bulk arrival — enforcement is disabled in this ' - 'build. Stop completion is NOT proximity-verified. ' - 'See kGeofenceEnforced.', - ); - return true; - } - try { - final pickupLat = _parseDouble( - Booking['pickuplat'] ?? Booking['PickupLat'], - ); - final pickupLng = _parseDouble( - Booking['pickuplon'] ?? Booking['PickupLon'], - ); - if (pickupLat == 0 || pickupLng == 0) return true; - final riderLoc = await _getValidCoordinates(); - if (riderLoc == null) return true; - final double riderLat = double.tryParse(riderLoc.$1) ?? 0; - final double riderLng = double.tryParse(riderLoc.$2) ?? 0; - if (riderLat == 0 || riderLng == 0) return true; - final double distanceInMeters = Geolocator.distanceBetween( - riderLat, - riderLng, - pickupLat, - pickupLng, - ); - // The phone's own error is credited to the rider here too — see - // [kGeofenceRadiusMeters] for why a 10 m fence measured with a ±20 m fix - // has to, and `_getValidCoordinates` for the fix it is measured with. - final double slack = _lastFixAccuracy; - return (distanceInMeters - slack) <= kGeofenceRadiusMeters; - } catch (e) { - debugPrint('[PROXIMITY] Error checking proximity: $e'); - return true; - } + /// The fence's answer for the last stop [_isNearPickupLocation] measured. + /// + /// Kept so the bulk-arrival sheet can name a distance instead of repeating a + /// generic "you are too far" for every ticked box. + GeofenceDecision? _lastBulkGeofence; + + /// The bulk gate on Home: the *selected stops* route into the arrived rung. + /// + /// ── One fence, reached two ways ── + /// + /// `PickupsController._checkGeofence` guards a single stop; this guards the + /// "mark selected as arrived" action, and it runs before the controller is + /// ever called. The two used to be separate implementations with separate + /// radii — 500 m hardcoded here against the controller's configured 100 — so + /// which fence a rider met depended on whether he ticked boxes or slid a + /// sheet. Both now call [Geofence] and there is no arithmetic left in this + /// file to drift. + /// + /// ── It used to fail open in four places ── + /// + /// A booking with no pin, a rider fix that could not be read, a zero + /// coordinate pair and any thrown exception each ended in `return true`. All + /// four are refusals now; [Geofence] decides which sentence the rider sees. + Future _isNearPickupLocation(Map booking) async { + final decision = await Geofence.check( + targetLat: _parseDouble(booking['pickuplat'] ?? booking['PickupLat']), + targetLng: _parseDouble(booking['pickuplon'] ?? booking['PickupLon']), + action: 'Arrived', + ); + _lastBulkGeofence = decision; + return decision.allowed; + } + + /// How many bulk status writes may be in flight at once. + /// + /// Not unbounded. Twenty simultaneous requests from one handset queue at the + /// radio on a rider's 3G connection and finish *slower* than a handful, and a + /// thundering herd from one rider is a poor shape to hand the backend. Six is + /// enough to hide the per-request latency — the thing that made the old + /// sequential loop take a minute — without any of that. + static const int _bulkLanes = 6; + + /// Straight-line kilometres from the rider to a stop, measured on the device. + /// + /// Handed to `updatePickedStatus` as `actualKms` purely so its priority-3 + /// branch — an OSRM road-distance request per order, six-second timeout — + /// never runs during a bulk action. It is the same straight line that branch + /// already falls back to when the routing service fails. + double _bulkLegKm( + String riderLat, + String riderLng, + String stopLat, + String stopLng, + ) { + final rLat = double.tryParse(riderLat) ?? 0; + final rLng = double.tryParse(riderLng) ?? 0; + final sLat = double.tryParse(stopLat) ?? 0; + final sLng = double.tryParse(stopLng) ?? 0; + if (rLat == 0 || rLng == 0 || sLat == 0 || sLng == 0) return 0; + return Geolocator.distanceBetween(rLat, rLng, sLat, sLng) / 1000.0; } Future> _checkProximityForOrders( @@ -695,7 +708,7 @@ class _HomepageState extends State content: Text( specificMessage ?? 'You must be within ' - '${kGeofenceRadiusMeters.toStringAsFixed(0)} metres of the ' + '${kGeofenceRadiusMetres.toStringAsFixed(0)} metres of the ' 'pickup location to mark this booking as arrived.', style: TextStyle( fontSize: FontConstants.regular(context), @@ -2054,6 +2067,34 @@ class _HomepageState extends State // is slide again at a stop that will keep refusing. See // [PickupsController.lastPickupRefusal]. if (nextStatus == 'PICKED') return dc.lastPickupRefusal ?? 'refused'; + + // ── Optimistic covers a write that did not LAND, never a rider who is + // not THERE ── + // + // The paragraph above is right about the case it describes: a rider at + // a counter with no signal must not have his own report of where he is + // snapped back by a late poll. + // + // But `updateArrivedStatus` answers false for three different things, + // and only one of them is that case. It is also false when the geofence + // refused — the rider is 1.4km from the door and no request was ever + // made — and when the server itself rejected the arrival on a business + // rule. Advancing on those wrote ARRIVED onto a stop the rider had not + // reached, and the admin console, which only ever hears what the server + // was told, went on showing the stop as pending. That is the divergence + // reported from the field: arrived on the phone, not arrived in the + // office, and no way for either side to tell who was wrong. + // + // So the two "he is not there / the office said no" cases stop here and + // are handed back to be shown. A dead network still falls through and + // still advances — with [PickupsController.lastArrivalNotice] telling + // him the office has not been told. + // + // `lastBlockedReason` is read first and is always fresh: `_checkGeofence` + // clears it on entry, so a non-null value can only have come from the + // check that just ran. + final notThere = dc.lastBlockedReason ?? dc.lastArrivalRefusal; + if (notThere != null) return notThere; } // ── The rung the SERVER produced, not the one the button is named after ── @@ -2930,13 +2971,22 @@ class _HomepageState extends State } if (targetStatus == 'ARRIVED') { - final proximityResults = await _checkProximityForOrders(selectedOrderIds) - .timeout( - const Duration(seconds: 3), - onTimeout: () => Map.fromEntries( - selectedOrderIds.map((id) => MapEntry(id, true)), - ), - ); + // ── The 3-second fail-OPEN is gone ── + // + // This was wrapped in `.timeout(3s, onTimeout: () => everything inside + // the fence)`. Each check took its own 8-second GPS fix, so twenty of + // them could never finish in three seconds — the timeout fired on + // essentially every bulk arrival and waved every order through whatever + // the distance was. A hole straight through the 100 m rule, and one that + // looked like a safety net. + // + // It is not needed now: [Geofence] reuses one live fix for + // [kGeofenceFixReuse], so twenty checks cost one GPS settle and complete + // in milliseconds. If the fix itself cannot be had, each check refuses on + // its own terms and says which of location-off / permission / no-signal + // it was — which is the answer the rider can act on, and the one a + // blanket "allow everything" was hiding. + final proximityResults = await _checkProximityForOrders(selectedOrderIds); final farOrders = proximityResults.entries .where((e) => !e.value) @@ -2948,7 +2998,8 @@ class _HomepageState extends State await _showProximityWarning( context, specificMessage: farOrders.length == selectedOrderIds.length - ? 'You must be within 500 meters of the pickup location(s) to mark bookings as arrived.' + ? 'You must be within ${kGeofenceRadiusMetres.round()} m of the pickup ' + 'location to mark it arrived.' : 'Some selected bookings are too far from their pickup locations.', ); } @@ -3041,11 +3092,41 @@ class _HomepageState extends State /// than at the next poll. final Set bulkCollected = {}; - for (final orderId in selectedOrderIds) { + // ── Phase 1: the network, overlapped ── + // + // This was a plain sequential `for`, so twenty orders meant twenty full + // round trips end to end — status call, GPS fix, routing call and four + // preferences writes each, nothing overlapping anything. Measured at + // 3–5 s per order, which is the 1–2 minutes riders were reporting for a + // twenty-bag counter pick. + // + // The writes are independent — different bookings, different + // [MutationGuard] keys (`picked:1042`), and the backend carries an + // `Idempotency-Key` per action — so they can be in flight together. The + // *local* bookkeeping is NOT done here: it is collected and applied once, + // below, so the order of local state changes stays deterministic and a + // growing JSON blob is not rewritten twenty times. + // + // Bounded to [_bulkLanes] rather than `Future.wait` over everything: + // twenty simultaneous requests on a rider's 3G connection queue at the + // radio and finish slower than six, and it is a kinder shape for the + // backend than a thundering herd from one handset. + final List bulkIds = []; + final Map> bulkStops = {}; + for (final id in selectedOrderIds) { // Not `_ordersMap` — a stop past ACCEPT is no longer in it, and every // rung after the first was silently skipped here. See [_bulkStopFor]. - final Booking = _bulkStopFor(orderId); - if (Booking == null) continue; + final stop = _bulkStopFor(id); + if (stop == null) continue; + bulkIds.add(id); + bulkStops[id] = stop; + } + + /// Which orders the backend confirmed. + final Set bulkOk = {}; + + Future runOne(String orderId) async { + final Booking = bulkStops[orderId]!; final pickupId = int.tryParse('${Booking['pickupid'] ?? 0}') ?? 0; final orderHeaderId = @@ -3085,10 +3166,7 @@ class _HomepageState extends State // Home, still selectable, and pressing again costs a tap. So it // records what the backend confirmed and says what it did not — // the same rule `_advanceStop` already applies to PICKED. - if (success) { - _hiddenOrderIds.add(orderId); - acceptedBookings.add(Booking); - } else { + if (!success) { debugPrint( '[BULK] accept REFUSED for $orderId — leaving it on Home', ); @@ -3110,10 +3188,7 @@ class _HomepageState extends State // successful bulk arrival was a network call, a success count and // a row that came straight back on ACCEPTED — which is what "mark // as arrived does nothing" looked like from the outside. - if (success) { - Booking['orderstatus'] = 'arrived'; - await addArrivedOrderIds([orderId]); - } + if (success) Booking['orderstatus'] = 'arrived'; } else if (targetStatus == 'PICKED') { debugPrint('[BULK] Picking Booking $orderId'); success = await dc.updatePickedStatus( @@ -3125,11 +3200,31 @@ class _HomepageState extends State ridersLng: bulkRidersLng, pickupLat: pickupLat, pickupLng: pickupLng, + // ── Pre-computed, so the routing call never fires ── + // + // Left at 0 this falls through to priority 3, which is an OSRM + // road-distance request per order with a 6-second timeout. On a + // bulk pick the rider never "started" each stop individually, so + // the cumulative-tracking key is absent for most of them and that + // request ran for nearly all twenty — to compute a number that + // `UpdatePickupProvider` never reads and `pickup-complete` never + // sends. The backend derives earnings from the coordinates by its + // own haversine; this figure only feeds Activity's local + // "km ridden". + // + // So it is measured here, on the device, for free — the same + // straight line the priority-3 branch already falls back to when + // OSRM fails. + actualKms: _bulkLegKm( + bulkRidersLat, + bulkRidersLng, + pickupLat, + pickupLng, + ), proofImage: bulkProofImageUrl, ); if (success) { _startPickupPosting(Booking); - _hiddenOrderIds.add(orderId); // ── The write that says the load is in his hands ── // @@ -3146,11 +3241,8 @@ class _HomepageState extends State // time are wrong" half of it. // // Recorded only after the backend confirmed, same rule as accept. - await addCollectedOrderIds([orderId]); + // Applied in phase 2 — see [bulkCollected]. bulkCollected.add(orderId); - // One copy of the day, and it is now stale — Deliveries must not - // render this order from a response that predates the hand-over. - unawaited(WorkRepository.instance.invalidate()); // ── Picked is a hand-over on one line and a finish on the other ── // @@ -3163,14 +3255,8 @@ class _HomepageState extends State // drops he makes himself — so filing it as completed sent it to // Activity instead of to Deliveries, and the delivery leg had // nothing to work. See [ServiceProfile.handsOffAtCollection]. - if (ServiceProfile.active.handsOffAtCollection) { - await addAcceptedBookings([Booking]); - } else { - await removeAcceptedBookings([orderId]); - await addCompletedBookings([Booking]); - } - // Off the arrived rung — it has been collected. - await removeArrivedOrderIds([orderId]); + // The handsOffAtCollection split is applied in phase 2, once, + // against the whole batch. } } } catch (e) { @@ -3179,12 +3265,78 @@ class _HomepageState extends State } if (success) { - successCount++; + bulkOk.add(orderId); } else { debugPrint('[BULK] Failed to update Booking $orderId'); } } + // Bounded-concurrency drain. `next++` is safe: there is no `await` + // between reading and incrementing it, and Dart does not preempt inside a + // synchronous run of statements. + var next = 0; + final int lanes = _bulkLanes < bulkIds.length + ? _bulkLanes + : (bulkIds.isEmpty ? 1 : bulkIds.length); + await Future.wait( + List.generate(lanes, (_) async { + while (true) { + final i = next++; + if (i >= bulkIds.length) break; + await runOne(bulkIds[i]); + } + }), + ); + successCount += bulkOk.length; + + // ── Phase 2: every local store written once ── + // + // Each of these helpers decodes a JSON blob, mutates it and writes it + // back. Called per order inside the loop that was O(n²) writes against a + // growing list, and it left a window where a rider backgrounding the app + // mid-batch had half a batch filed. + final List okIds = bulkIds.where(bulkOk.contains).toList(); + final List> okStops = [ + for (final id in okIds) bulkStops[id]!, + ]; + + if (okIds.isNotEmpty) { + // ── ARRIVED is deliberately NOT hidden ── + // + // Accept and Picked take the row off Home; arriving does not. An + // arrived stop is still work in front of the rider and stays on the + // list — see `arrived_stays_on_home_test.dart`. Hiding all three here + // would have made a bulk arrival look like the stops had vanished. + if (targetStatus == 'ACCEPTED') { + _hiddenOrderIds.addAll(okIds); + acceptedBookings.addAll(okStops); + } else if (targetStatus == 'ARRIVED') { + await addArrivedOrderIds(okIds); + } else if (targetStatus == 'PICKED') { + _hiddenOrderIds.addAll(okIds); + await addCollectedOrderIds(okIds); + // ── Picked is a hand-over on one line and a finish on the other ── + // + // Right for **logistics**, where what he collects goes to the base + // and the booking's own story ends at the counter. On a **milk run** + // collection is the *middle* of the day — every bag is followed by a + // round of drops he makes himself — so filing it as completed would + // send it to Activity instead of Deliveries and the delivery leg + // would have nothing to work. See [ServiceProfile.handsOffAtCollection]. + if (ServiceProfile.active.handsOffAtCollection) { + await addAcceptedBookings(okStops); + } else { + await removeAcceptedBookings(okIds); + await addCompletedBookings(okStops); + } + // Off the arrived rung — it has been collected. + await removeArrivedOrderIds(okIds); + // One copy of the day, and it is now stale — Deliveries must not + // render these orders from a response that predates the hand-over. + unawaited(WorkRepository.instance.invalidate()); + } + } + // The leg every card reads is derived from this set, so it has to land // before the next build — not at the next poll three seconds later. if (bulkCollected.isNotEmpty && mounted) { diff --git a/lib/views/Dashboard/home/stop_card.dart b/lib/views/Dashboard/home/stop_card.dart index 8e48cce..28d58b2 100644 --- a/lib/views/Dashboard/home/stop_card.dart +++ b/lib/views/Dashboard/home/stop_card.dart @@ -3,6 +3,7 @@ import 'package:lucide_icons_flutter/lucide_icons.dart'; import 'package:flutter_screenutil/flutter_screenutil.dart'; import 'package:miler/data/service_profile.dart'; +import 'package:miler/data/task_profile.dart'; import 'package:miler/views/Dashboard/home/trip.dart'; import 'package:miler/views/Dashboard/pickups/route_metrics.dart'; import 'package:miler/views/Dashboard/pickups/stop_type.dart'; @@ -164,7 +165,8 @@ class StopCard extends StatelessWidget { // he is riding to. After it, the drop leads — and that card lives on the // work tab, where [PickupCard] does exactly the same thing in the other // direction. - final twoLeg = ServiceProfile.active.deliversToCustomer && drop.isNotEmpty; + // Per row: a CX pickup on a meal tenant is not a two-leg meal stop. + final twoLeg = WorkPolicy.deliversToCustomer(stop) && drop.isNotEmpty; final grouped = groupedUnderSource && source.isNotEmpty; final headline = (twoLeg && !grouped && source.isNotEmpty) ? source @@ -278,7 +280,7 @@ class StopCard extends StatelessWidget { final chip = _live ? LiveMark(label: state == StopState.arrived ? 'Arrived' : 'Active') : (state == StopState.pending && - !ServiceProfile.active.handsOffAtCollection) + !WorkPolicy.staysOnHomeUntilCollected(stop)) ? null : StopStateChip(state: state, filled: true); @@ -410,7 +412,7 @@ class StopCard extends StatelessWidget { // On a meal run the parcel counts are the one-label rule restated // as an item count, which is exactly the second number this app // does not allow. Only the money survives. - countsHidden: ServiceProfile.active.deliversToCustomer, + countsHidden: WorkPolicy.deliversToCustomer(stop), ); final showLabel = !labelShownAbove && printed.isNotEmpty; if (!showLabel && meta.isEmpty) return const SizedBox.shrink(); diff --git a/lib/views/Dashboard/home/trip_card.dart b/lib/views/Dashboard/home/trip_card.dart index d230c05..5b5a138 100644 --- a/lib/views/Dashboard/home/trip_card.dart +++ b/lib/views/Dashboard/home/trip_card.dart @@ -21,6 +21,7 @@ import 'package:miler/data/stop_area.dart'; import 'package:miler/data/milk_run.dart'; import 'package:miler/data/pickup_locations.dart'; import 'package:miler/data/service_profile.dart'; +import 'package:miler/data/task_profile.dart'; /// ───────────────────────────────────────────────────────────────────────── /// TRIP CARD — one slot's whole route, start to finish. @@ -694,7 +695,28 @@ class TripCard extends StatelessWidget { // give. `trip_brief_layout_test.dart` sweeps for exactly this. Flexible( child: Text( - ServiceProfile.active.endsAtHub ? 'RETURN · BASE' : 'END · HOME', + // ── Line OR row, and the OR is the point ── + // + // This was migrated to `TripShape.endsAtHub(trip.stops)` alone and + // reverted within the hour, for the reason `task_profile.dart` + // already documents about `sourceIsKitchen`: **the rows cannot + // answer this one.** A base leg is only knowable from + // `next_action: inward_at_hub`, which a parcel does not carry until + // it has been collected — so a logistics trip of five *pre-pickup* + // stops folds to `false` and a rider who has always ended his day + // at a base was told "END · HOME". `two_lines_test` and + // `card_density_test` both caught it. + // + // The line knows where the day ends; it is a fact about the rider's + // round, not about any one parcel. The fold is kept beside it + // because it can only ever *add* — a meal rider carrying one + // hub-routed parcel now correctly reads RETURN · BASE, which + // neither source could say on its own. `any`, not `every`: one + // parcel the network needs is enough to send him to a base. + (ServiceProfile.active.endsAtHub || + TripShape.endsAtHub(trip.stops)) + ? 'RETURN · BASE' + : 'END · HOME', maxLines: 1, overflow: TextOverflow.ellipsis, style: MilerType.eyebrow, @@ -1073,7 +1095,8 @@ class TripCard extends StatelessWidget { // logistics rider genuinely does return collected // shipments to the depot, so the label follows the // capability rather than the screen position. - : (ServiceProfile.active.endsAtHub + : ((ServiceProfile.active.endsAtHub || + TripShape.endsAtHub(trip.stops)) ? 'RETURN · BASE' : 'END · HOME'), maxLines: 1, diff --git a/lib/views/Dashboard/pickups/card.dart b/lib/views/Dashboard/pickups/card.dart index 898f1fe..f20852f 100644 --- a/lib/views/Dashboard/pickups/card.dart +++ b/lib/views/Dashboard/pickups/card.dart @@ -136,7 +136,6 @@ class PickupCard extends StatelessWidget { ], 'Address not available'); final String orderId = _val(['orderid']); final String notes = _val(['notes', 'Notes']); - final String phone = _val(['pickupcontactno']); final int deliverQty = deliveryParcelCount(item); final int collectQty = pickupParcelCount(item); @@ -157,15 +156,54 @@ class PickupCard extends StatelessWidget { // The receiver names the far end whenever the rider is carrying it there — // whether that is a milk round (line-level) or a hyperlocal parcel the // backend routed direct (per-order). + // Asked of the ROW first. On a meal tenant this static answered true for + // every card on the screen, so a CX customer pickup sitting next to a + // kitchen load was drawn as though the rider were already carrying it to a + // receiver. [WorkPolicy] reads the row's own `next_action` / + // `pickup_source_type` and only falls back to the line where the row is + // silent — which keeps every existing payload rendering exactly as it does + // today. final bool carryingToCustomer = - ServiceProfile.active.deliversToCustomer || leg.isCustomer; + WorkPolicy.deliversToCustomer(item) || leg.isCustomer; final bool pickupIsSource = carryingToCustomer && source.isNotEmpty; final String pickupName = pickupIsSource ? source : customer; final String dropAddress = _val(['dropaddress', 'DropAddress']); - final String dropName = pickupIsSource + // ── Who is actually at the far end ── + // + // `pickupcustomer` is the booking's customer, which on a parcel booking is + // the **sender** — the person the rider collected FROM. Using it to name the + // drop put the sender's name over the receiver's door, and on a + // multi-destination pickup it put the same name over all three of them. + // The receiver is carried per destination and is the honest answer whenever + // the payload has one. + final String recipient = _val(['recipientname']); + final String dropName = recipient.isNotEmpty + ? recipient + : pickupIsSource ? customer : (dropAddress.isEmpty ? '' : 'Collection centre'); + // ── The Call button has to dial the door he is standing at ── + // + // `pickupcontactno` is the booking's customer — the SENDER — and it was + // what this button dialled on every card, delivery legs included. So a + // rider at the receiver's gate pressed Call and rang the man he had + // collected from that morning, in another city; on a three-drop pickup he + // rang him at all three gates. The receiver's own number is carried per + // destination and is the right one to ring whenever the rider is on his way + // to that receiver — the same test the name above uses, so the card cannot + // show one person and phone another. + final String recipientPhone = _val(['recipientphone']); + final String phone = (recipient.isNotEmpty && recipientPhone.isNotEmpty) + ? recipientPhone + : _val(['pickupcontactno']); + + /// `Stop 2 of 3` on a customer pickup with several doors, `''` otherwise. + /// Sits with the leg eyebrow because it answers the same question — what + /// kind of stop is this — and because three cards that differ only in their + /// address are three chances to deliver the wrong bag. + final String stopLabel = MilkRun.stopLabel(item); + /// True once the rider is carrying it: the drop becomes the destination and /// takes the top of the card. // ── The immediate destination leads; never a hub leg's receiver ── @@ -179,6 +217,46 @@ class PickupCard extends StatelessWidget { (pickupIsSource || leg.isCustomer) && (dropName.isNotEmpty || dropAddress.isNotEmpty); + // ── On a base leg the BASE leads ── + // + // The card was right to refuse the receiver — he is in another district and + // leading with him is the navigation mistake the rule above exists to + // prevent. But refusing the receiver only left `pickupName`, so a handover + // card read: + // + // BASE HANDOVER + // Anitha R + // Gandhipuram + // + // …the customer he collected from an hour ago, under a heading telling him + // to go to a base. The place he is actually going appeared nowhere on the + // card and only on the sheet's slider, after he had already set off. On a + // round with two bases that is a parcel left in the wrong building. + // + // `next_hub` rides on the row (the backend sends all six fields), so the + // destination is known here. A base with no coordinates still leads with + // its name — it is a caption rather than a destination, and the fence + // refuses the handover anyway with a sentence pointing at the office. + final HandoverHub? handoverBase = leg.isHub + ? (HandoverHub.from(item['next_hub'] ?? item['nexthub']) ?? + PickupLocations.baseFor( + item['next_hub_id'] ?? item['nexthubid'] ?? item['hubid'], + )) + : null; + final bool baseLeads = handoverBase != null; + + /// What the card is headed with, in one place so the name and the address + /// below it can never describe two different places. + final String headlineName = baseLeads + ? handoverBase.name + : (dropLeads ? dropName : pickupName); + final String headlineAddress = baseLeads + ? [ + handoverBase.address, + handoverBase.pincode, + ].where((v) => v.trim().isNotEmpty).join(', ') + : ''; + // ── No accent stripe any more ── // // The card carried a 4px coloured bar down its left edge, on the argument @@ -261,9 +339,15 @@ class PickupCard extends StatelessWidget { // has to say which before the rider reads the place. // Drawn only once the parcel is actually his; an // uncollected stop has no leg yet. - if (leg.leg.eyebrow.isNotEmpty) ...[ + if (leg.leg.eyebrow.isNotEmpty || + stopLabel.isNotEmpty) ...[ Text( - leg.leg.eyebrow, + [ + if (leg.leg.eyebrow.isNotEmpty) + leg.leg.eyebrow, + if (stopLabel.isNotEmpty) + stopLabel.toUpperCase(), + ].join(' · '), style: TextStyle( fontSize: 10.sp, fontWeight: FontWeight.w800, @@ -279,9 +363,7 @@ class PickupCard extends StatelessWidget { SizedBox(height: 3.h), ], Text( - (dropLeads ? dropName : pickupName).isEmpty - ? 'Not named' - : (dropLeads ? dropName : pickupName), + headlineName.isEmpty ? 'Not named' : headlineName, maxLines: 2, overflow: TextOverflow.ellipsis, style: TextStyle( @@ -312,6 +394,17 @@ class PickupCard extends StatelessWidget { // lost — see [areaOf]. Text( () { + // A base is read at a gate, so it gets its street + // and pincode rather than the locality summary the + // other legs use — "Peelamedu" is not enough to + // find a loading bay by. + if (baseLeads) { + // Empty rather than repeating the name: a base + // with no address on the row has nothing more + // to say here, and printing "Salem Base" twice + // reads as a rendering fault. + return headlineAddress; + } final full = dropLeads ? (dropAddress.isEmpty ? address @@ -1560,7 +1653,7 @@ String _destinationName(Map item) { String _destinationAddress(Map item) { final area = areaOf( item, - preferDrop: ServiceProfile.active.deliversToCustomer, + preferDrop: WorkPolicy.deliversToCustomer(item), ); if (area.isNotEmpty) return area; @@ -1569,6 +1662,6 @@ String _destinationAddress(Map item) { final drop = (item['dropaddress'] ?? item['DropAddress'] ?? '') .toString() .trim(); - if (ServiceProfile.active.deliversToCustomer && drop.isNotEmpty) return drop; + if (WorkPolicy.deliversToCustomer(item) && drop.isNotEmpty) return drop; return (item['pickupaddress'] ?? '').toString().trim(); } diff --git a/lib/views/Dashboard/pickups/delivery_actions.dart b/lib/views/Dashboard/pickups/delivery_actions.dart index b59aed1..1cbbc2f 100644 --- a/lib/views/Dashboard/pickups/delivery_actions.dart +++ b/lib/views/Dashboard/pickups/delivery_actions.dart @@ -36,6 +36,15 @@ enum DeliveryOutcome { /// It is not going to happen. `POST /miler/bookings/:id/cancel`. cancelled, + + /// Handed in at a base, ending this rider's custody. + /// `POST /miler/consignments/:id/inward-at-hub` — see [handOverAtHub]. + /// + /// Not a kind of [delivered]. A base handover has no receiver, no proof + /// photo, no OTP and no COD, and it must never reach `deliver` — that route + /// moves the consignment to `Out_for_Delivery` against a receiver in another + /// district, which is a state the handset cannot undo. + handedOver, } extension DeliveryOutcomeX on DeliveryOutcome { @@ -43,6 +52,7 @@ extension DeliveryOutcomeX on DeliveryOutcome { DeliveryOutcome.delivered => 'Delivered', DeliveryOutcome.skipped => 'Skip', DeliveryOutcome.cancelled => 'Cancelled', + DeliveryOutcome.handedOver => 'Handed over', }; IconData get icon => switch (this) { @@ -54,12 +64,15 @@ extension DeliveryOutcomeX on DeliveryOutcome { // skip-reason sheet's "Delivery paused" row already wears. DeliveryOutcome.skipped => LucideIcons.clock, DeliveryOutcome.cancelled => LucideIcons.circleX, + // A building, not a tick: the parcel is not finished, it has changed hands. + DeliveryOutcome.handedOver => LucideIcons.building2, }; Color get colour => switch (this) { DeliveryOutcome.delivered => ColorConstants.acceptGreen, DeliveryOutcome.skipped => ColorConstants.warning, DeliveryOutcome.cancelled => ColorConstants.errorRed, + DeliveryOutcome.handedOver => ColorConstants.acceptGreen, }; /// What Activity will show once the round is over. @@ -67,6 +80,7 @@ extension DeliveryOutcomeX on DeliveryOutcome { DeliveryOutcome.delivered => 'Delivered', DeliveryOutcome.skipped => 'Skipped', DeliveryOutcome.cancelled => 'Cancelled', + DeliveryOutcome.handedOver => 'Handed over', }; } @@ -583,6 +597,14 @@ Future?> _closeDelivery( // reportable from `Collected_By_Miler` as well as `Out_for_Delivery` — a // customer who is not home is not home whether or not the rider remembered // to press Start round. See [ConsignmentStateX.canSkip]. + // ── A handover is deliberately NOT gated here ── + // + // [DeliverGate] answers the question "may this be *delivered*", and a + // base-routed parcel is `Created` — which it reads as `awaitingInward` and + // refuses with "this parcel hasn't been released for delivery yet". Correct + // for a delivery, exactly wrong for the rung that performs the inward. + // `inward-at-hub` is the server's own judge of its preconditions and answers + // `INVALID_STATE` when they are not met. final gated = outcome == DeliveryOutcome.delivered || outcome == DeliveryOutcome.skipped; @@ -820,6 +842,15 @@ Future?> _closeDelivery( notes: notes, ); + case DeliveryOutcome.handedOver: + // The fence, the consignment lookup and the `inward-at-hub` write all + // live in [handOverAtHub] — this is the one close path, so the record + // that gets filed is the same shape whichever rung produced it. + ok = await handOverAtHub(stop); + if (!ok && lastHandoverFailure != null && context.mounted) { + AppFeedback.error(context, lastHandoverFailure!); + } + case DeliveryOutcome.skipped: // ── Straight to the consignment route ── // @@ -1050,3 +1081,142 @@ Future?> _closeDelivery( 'notes': notes, }; } + +// ═══════════════════════════════════════════════════════════════════════════ +// THE BASE HANDOVER — the leg the rider could not finish +// ═══════════════════════════════════════════════════════════════════════════ + +/// Why the last [handOverAtHub] refused, in the rider's words. Null on success. +String? lastHandoverFailure; + +/// The base this stop must be handed in at, or null when it is not a base leg. +/// +/// Two sources, in order. `next_hub` rides on the row itself (request 25) and +/// is the authoritative one. [PickupLocations.baseFor] is the fallback for a +/// row that names a hub id without expanding it — a shape older payloads use. +HandoverHub? handoverBaseFor(Map stop) { + final fromRow = HandoverHub.from(stop['next_hub'] ?? stop['nexthub']); + if (fromRow != null) return fromRow; + return PickupLocations.baseFor( + stop['next_hub_id'] ?? stop['nexthubid'] ?? stop['hubid'], + ); +} + +/// Hands a base-routed consignment in, ending this rider's custody of it. +/// +/// ── The leg that existed everywhere except the app ── +/// +/// `MilerApi.inwardAtHub` and `MilerLifecycle.inwardAtHub` were both written, +/// documented and covered by `hub_handover_contract_test.dart`. Neither had a +/// single call site. So with `MILER_HUB_HANDOVER_ENABLED` on server-side, a +/// Gandhipuram → Chennai parcel arrived on Deliveries as [NextLeg.hub], +/// [releaseForDelivery] correctly refused to start a customer round for it — +/// telling the rider "hand it over at base" — and there was no control in the +/// app that could record him doing so. The parcel sat in his queue until hub +/// staff inwarded it from the console, and the assignment closed against +/// nobody, so the job reported zero distance and zero value on his earnings. +/// +/// ── Correct in both positions of the server flag ── +/// +/// This never asks which way the flag is set, because the app must not mirror +/// it. With the flag **off**, `pickup-complete` inwards the parcel itself and +/// the row comes back `Inwarded_at_Hub` / `handed_to_hub` — [NextLeg.closed] — +/// so this function is never reached and no control is drawn. With it **on**, +/// the row is `Created` / `inward_at_hub` — [NextLeg.hub] — and this is the +/// rung that finishes it. The leg is read from the row, every time. +/// +/// ── Fenced at the base, like every other presence claim ── +/// +/// 100 m, against the base's own coordinates. A base with no coordinates is +/// refused rather than waved through, for the same reason a customer stop with +/// no pin is: nobody can say afterwards where the rider was standing. +/// +/// Idempotent twice over — the shared `Idempotency-Key` covers a retry after a +/// dropped response, and a parcel already inwarded answers 200 with +/// `already_inwarded: true`, which [MilerLifecycle.inwardAtHub] reads as the +/// success it is. A rider pressing again on bad signal at a loading bay is +/// confirmed, not refused. +Future handOverAtHub(Map stop) async { + lastHandoverFailure = null; + + final orderId = MilkRun.idOf(stop); + final leg = NextLegResolver.resolve( + stop, + pivotAction: (await getPivotNextActions())[orderId] ?? '', + ); + if (!leg.isHub) { + // Not a refusal the rider caused — the row says this parcel is not going to + // a base. Saying so beats posting a handover the server will reject. + lastHandoverFailure = + 'This parcel is not going to a base. Check the stop and try again.'; + debugPrint('[HANDOVER] $orderId is not a base leg — $leg'); + return false; + } + + final base = handoverBaseFor(stop); + + // ── The fence, before anything else is spent ── + // + // A base with no coordinates lands on [GeofenceOutcome.noTarget] and is + // refused with a sentence pointing at the office, which is the only party who + // can add the missing pin. + final decision = await Geofence.check( + targetLat: base?.latitude, + targetLng: base?.longitude, + action: 'Handed over', + ); + if (!decision.allowed) { + lastHandoverFailure = decision.reason; + debugPrint('[HANDOVER] $orderId refused by the fence — $decision'); + return false; + } + + final consignmentId = await resolveConsignmentId(stop); + if (consignmentId.isEmpty) { + lastHandoverFailure = + 'This stop has no shipment reference yet, so it cannot be handed over. ' + 'Ask your office to check it — pressing again will not help.'; + debugPrint('[HANDOVER] no consignment id for $orderId'); + return false; + } + + debugPrint( + '[TRACE][HANDOVER] consignment=$consignmentId base=${base?.id} ' + 'POST /miler/consignments/$consignmentId/inward-at-hub — calling', + ); + final res = await MilerApi.inwardAtHub( + consignmentId, + hubId: (base?.id.isNotEmpty ?? false) ? base!.id : null, + // The position the fence just judged, not a fresh one: two fixes seconds + // apart are two different answers and the hub's history row should carry + // the one the app actually allowed the handover on. + lat: decision.riderLat, + lon: decision.riderLng, + ); + final t = MilerLifecycle.inwardAtHub(res); + MilerLifecycle.report('inward-at-hub', t); + debugPrint( + '[TRACE][HANDOVER] consignment=$consignmentId -> ${res.status} ' + '${res.code} confirmed=${t.isConfirmed} raw=${res.raw}', + ); + + if (t.isConfirmed) return true; + + // ── A 200 that names no state is not proof ── + // + // The rule request 15 exists for, and the one this app has been bitten by on + // `reached`: a bare success is not a transition. The rider is not shown a + // handover the hub may not have recorded. + if (t.isUnconfirmed) { + lastHandoverFailure = + 'Your office did not confirm the handover. Check with the base before ' + 'you leave the parcel.'; + return false; + } + + final serverMsg = (res.message).trim(); + lastHandoverFailure = serverMsg.isNotEmpty && serverMsg.length < 140 + ? 'The base would not accept this parcel: $serverMsg' + : 'The base would not accept this parcel. Ask your office to check it.'; + return false; +} diff --git a/lib/views/Dashboard/pickups/map.dart b/lib/views/Dashboard/pickups/map.dart index af58917..cee2fd3 100644 --- a/lib/views/Dashboard/pickups/map.dart +++ b/lib/views/Dashboard/pickups/map.dart @@ -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 {}, ).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 _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 _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 _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 _withParcelPhotos(Map 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': [key]}, + }; + } + Future _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 _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 verifyResult = {'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>( 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.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>( context, ShipmentReviewPage( diff --git a/lib/views/Dashboard/pickups/pickups.dart b/lib/views/Dashboard/pickups/pickups.dart index ddb8190..8ea28b8 100644 --- a/lib/views/Dashboard/pickups/pickups.dart +++ b/lib/views/Dashboard/pickups/pickups.dart @@ -15,7 +15,7 @@ library; import 'dart:async'; import 'dart:convert'; import 'dart:io'; -import 'package:flutter/foundation.dart' show kDebugMode, mapEquals, setEquals; +import 'package:flutter/foundation.dart' show mapEquals, setEquals; import 'package:flutter/material.dart'; import 'package:lucide_icons_flutter/lucide_icons.dart'; import 'package:miler/views/helpers/constants/design_constants.dart'; @@ -82,6 +82,10 @@ import 'package:miler/data/service_day.dart'; import 'package:miler/data/work_domain.dart'; import 'package:miler/data/work_repository.dart'; import 'package:miler/data/miler_api.dart'; +import 'package:miler/data/lifecycle.dart'; +import 'package:miler/data/geofence.dart'; +import 'package:miler/data/task_profile.dart'; +import 'package:miler/data/pickup_locations.dart'; import 'package:miler/data/assignment_lookup.dart'; import 'package:miler/utils/external_navigation.dart'; @@ -943,33 +947,10 @@ class _MyPickupsState extends State fontFamily: FontConstants.fontFamily, ), ), - _fetchDiagLine(), ], ); } - /// The last fetch's stage counts, shown under an empty state in debug only. - /// Never compiled into release — [kDebugMode] is a const, so the whole widget - /// folds away. - Widget _fetchDiagLine() { - if (!kDebugMode || _fetchDiag.isEmpty) return const SizedBox.shrink(); - return Padding( - padding: EdgeInsets.only(top: 18.h), - child: Text( - 'debug · $_fetchDiag\n' - '${_bookingTrips.length} trip(s) · showing ${_visibleStops.length}', - textAlign: TextAlign.center, - style: TextStyle( - fontSize: 10.sp, - height: 1.5, - fontWeight: FontWeight.w600, - color: ColorConstants.secondaryText, - fontFamily: FontConstants.fontFamily, - ), - ), - ); - } - Widget _buildTripRail() { // The whole run, finished stops included — see [_railStops]. Drawn as long // as *anything* happened today, so a rider who has just closed his last @@ -2182,8 +2163,10 @@ class _MyPickupsState extends State // separate causes have made this tab look empty — a gate on the raw API // count, a missing empty state on an unfilled trip tab, and stops // filtered out as still-pending — and every one of them looked identical - // on screen. The counts are now shown on the empty state itself, so the - // failing stage is visible instead of guessed at. + // on screen. The counts go to the log so the failing stage can be read + // off a `flutter logs` rather than guessed at. They are deliberately NOT + // drawn on the empty state any more: that put developer diagnostics in + // front of the rider on every debug and demo build. _fetchDiag = // Which line the app resolved the rider onto. Three of the reports // that landed here as "the tab is wrong" were really "the app thinks @@ -3118,9 +3101,16 @@ class _MyPickupsState extends State // is the picture now, at a size worth looking at, and // it keeps its own copy. // - // [_fetchDiagLine] stays: it is `kDebugMode`-gated and - // folds away entirely in release, so nothing ships - // underneath the art. + // Nothing is drawn under the art. A stage-count + // readout used to sit here — `debug · line=milkMan + // api=0 accepted=0 …` — on the argument that it was + // `kDebugMode`-gated and folded away in release. + // True, and it still meant every internal build, + // every demo and every screenshot showed a rider a + // line of diagnostics under a picture telling him he + // was all caught up. The counts are still gathered + // and still logged — see `[MYPICKUPS][DIAG]` — where + // the person who needs them is looking. child: Center( child: Column( mainAxisSize: MainAxisSize.min, @@ -3140,7 +3130,6 @@ class _MyPickupsState extends State ), ), ), - _fetchDiagLine(), ], ), ), diff --git a/lib/views/Dashboard/pickups/sheet.dart b/lib/views/Dashboard/pickups/sheet.dart index aae32b3..b171e58 100644 --- a/lib/views/Dashboard/pickups/sheet.dart +++ b/lib/views/Dashboard/pickups/sheet.dart @@ -193,7 +193,7 @@ class _PickupBottomSheetState extends State<_PickupBottomSheet> // stays "Confirm pickup". final String confirmLabel = isDelivery ? 'Confirm delivery' - : (ServiceProfile.active.initiatesShipment + : (WorkPolicy.initiatesShipment(widget.pickup) ? 'Initiate order' : 'Confirm pickup'); diff --git a/lib/views/Dashboard/profile/Profilepage.dart b/lib/views/Dashboard/profile/Profilepage.dart index 001f126..06194c1 100644 --- a/lib/views/Dashboard/profile/Profilepage.dart +++ b/lib/views/Dashboard/profile/Profilepage.dart @@ -35,6 +35,7 @@ import 'package:miler/controllers/profile_controller.dart'; import 'package:miler/data/service_profile.dart'; import 'package:shared_preferences/shared_preferences.dart'; import 'package:miler/data/accepted_store.dart'; +import 'package:miler/data/session.dart'; import 'package:miler/data/proof_store.dart'; import 'package:miler/controllers/rewards_controller.dart'; import 'package:miler/controllers/summary_controller.dart'; @@ -866,8 +867,32 @@ void _showLogoutDialog(BuildContext context) { // Doorstep photos are exactly the kind of record that must // not outlive the session that took them. await ProofStore.clearScope(); - final prefs = await SharedPreferences.getInstance(); - await prefs.setBool('logged_out', true); + + // ── The session ends here, not at the next sign-in ── + // + // This did not clear the bearer token. The rider was sent + // to the sign-in screen and `logged_out` kept him there, so + // it *looked* finished — but the token stayed in + // SharedPreferences under `authtoken`, still valid, until + // the next `verifyPinWithServer` happened to overwrite it. + // + // Two things followed. Anything that reads the token + // without checking the flag — a background isolate, the + // notification handler, a heartbeat that outlives the + // route change — could go on making authenticated calls as + // the rider who just left. And a handset handed to the next + // rider carried the previous one's credential on disk. + // + // Logging out is the one moment the app is certain the + // session is over. The credential goes then. + // Everything above plus the token, the in-flight guard, the + // cached fix and the `logged_out` flag now live in one + // place — see [endSession]. The other caller is the 401 + // handler in `main.dart`: a session the server has stopped + // accepting has to end exactly as thoroughly as one the + // rider chose to end, and when the two were written out + // separately only this one existed. + await endSession(); Get.offAll(() => const SignIn()); }, height: ButtonSizes.secondary, diff --git a/lib/views/helpers/widgets/app_widgets.dart b/lib/views/helpers/widgets/app_widgets.dart index 8d20ede..57fb9b5 100644 --- a/lib/views/helpers/widgets/app_widgets.dart +++ b/lib/views/helpers/widgets/app_widgets.dart @@ -1856,6 +1856,23 @@ class AppFeedback { duration: const Duration(seconds: 2), ); + /// It went through on the phone but not to the office. + /// + /// The band between [error] and [success], and the app needed one: a status + /// the rider set with no signal is neither. Refusing it would strand him at a + /// door; a green tick would tell him the hub knows, which it does not. Amber + /// says "done here, not there", which is the only true thing available. + /// + /// Longer than [info] on purpose — it carries a consequence the rider may + /// need to act on later. + static void warn(BuildContext context, String message) => _show( + context, + message, + icon: LucideIcons.triangleAlert, + fill: ColorConstants.warning, + duration: const Duration(seconds: 5), + ); + /// Neutral news: something changed that the rider did not ask for, or a /// background action finished. Never used for failures. static void info(BuildContext context, String message) => _show( diff --git a/lib/views/offline/offline_banner.dart b/lib/views/offline/offline_banner.dart index 5d2d4ad..72bfe4f 100644 --- a/lib/views/offline/offline_banner.dart +++ b/lib/views/offline/offline_banner.dart @@ -25,10 +25,23 @@ import 'package:miler/views/helpers/constants/miler_type.dart'; /// ── Why a strip is the right shape ── /// /// Everything the rider needs at the door was already fetched. Connectivity is -/// needed to *report* the stop, not to work it. So the honest message is not -/// "you are offline" but "keep going, this will sync" — and that is a line of +/// needed to *report* the stop, not to work it. So the right shape is a line of /// text, not a screen. /// +/// ── What it must not say ── +/// +/// It said "Changes will sync when you're back online". **There is no mutation +/// queue in this app** — no outbox, no replay, no retry loop. A status the +/// rider sets with no signal is attempted once and fails, and nothing ever +/// sends it again. The banner was promising a mechanism that does not exist, +/// to the one person who would be blamed when the hub had no record of the +/// stop. +/// +/// The copy now says what is actually true: he can keep working the stop, and +/// the *status* needs signal. Building the queue is a real piece of work and a +/// separate one (see the release report); until it exists this line must not +/// imply it. +/// /// The full-screen page survives for the one case where it is truthful: a cold /// start with no cached session, where there genuinely is nothing to show. /// @@ -46,7 +59,7 @@ class OfflineBanner extends StatelessWidget { Widget build(BuildContext context) { return Semantics( liveRegion: true, - label: 'Offline. Your changes will sync when you are back online.', + label: 'Offline. Status updates need a connection. Stop details are still available.', excludeSemantics: true, child: Material( color: ColorConstants.warning, @@ -64,9 +77,10 @@ class OfflineBanner extends StatelessWidget { SizedBox(width: 9.w), Expanded( child: Text( - // States what is true and what happens next. "You're - // offline" alone tells a rider mid-stop nothing he can use. - 'Offline · Changes will sync when you’re back online', + // States what is true and what he can still do. It must + // not promise a sync — nothing in this app replays a failed + // mutation. See the class note. + 'Offline · Status updates need a connection', maxLines: 2, overflow: TextOverflow.ellipsis, style: MilerType.micro.on(ColorConstants.onAccent).semibold, diff --git a/lib/views/onboardscreens/Creat_mpin.dart b/lib/views/onboardscreens/Creat_mpin.dart index a8aa15a..aa96c99 100644 --- a/lib/views/onboardscreens/Creat_mpin.dart +++ b/lib/views/onboardscreens/Creat_mpin.dart @@ -6,6 +6,8 @@ import 'package:miler/views/helpers/widgets/page_transitions.dart'; import 'package:miler/controllers/auth.dart'; import 'package:miler/views/helpers/constants/Font_constant.dart'; import 'package:miler/views/helpers/constants/Colorconstants.dart'; +import 'package:miler/widget/Bottom_page.dart'; +import 'package:miler/views/helpers/widgets/app_widgets.dart'; import 'package:miler/views/onboardscreens/Mpin.dart'; import 'package:miler/views/onboardscreens/auth_scaffold.dart'; @@ -178,11 +180,35 @@ class _CreateMpinBodyState extends State<_CreateMpinBody> { final ok = await _auth.setPin(_pin(_newMpinControllers)); if (!mounted) return; setState(() => isLoading = false); + + // ── Setting the PIN IS the sign-in ── + // + // `POST /miler/set-pin` returns a full session — token and user — so there + // is no verify-pin afterwards and no second PIN to type. This used to push + // the rider to the unlock screen to enter the PIN he had just chosen, which + // was the only thing it could do while the write had no route behind it. if (ok) { - openScreen(context, Mpin()); - } else { - setState(() => _error = 'Could not save your PIN. Please try again.'); + Get.offAll(() => const BottomPage()); + return; } + + // A 409 means he is not a first-time rider after all — he already has a + // PIN, here or on another handset. Sending him to Enter-PIN is the useful + // answer; "could not save your PIN" would leave him retyping one the + // server will never accept. + if (_auth.lastSetPinWasAlreadySet) { + if (_auth.lastSetPinFailure != null) { + AppFeedback.infoGlobal(_auth.lastSetPinFailure!); + } + openScreen(context, Mpin()); + return; + } + + setState( + () => _error = + _auth.lastSetPinFailure ?? + 'Could not save your PIN. Please try again.', + ); } @override diff --git a/lib/views/onboardscreens/Sign_in.dart b/lib/views/onboardscreens/Sign_in.dart index ca347d1..0f85f99 100644 --- a/lib/views/onboardscreens/Sign_in.dart +++ b/lib/views/onboardscreens/Sign_in.dart @@ -4,7 +4,7 @@ import 'package:flutter/services.dart'; import 'package:flutter_screenutil/flutter_screenutil.dart'; import 'package:get/get.dart'; import 'package:miler/views/helpers/widgets/page_transitions.dart'; -import 'package:miler/views/onboardscreens/otp_page.dart'; +import 'package:miler/views/onboardscreens/Creat_mpin.dart'; import 'package:miler/views/onboardscreens/Mpin.dart'; import 'package:miler/views/onboardscreens/auth_scaffold.dart'; import 'package:miler/controllers/auth.dart'; @@ -138,21 +138,43 @@ class _SignInState extends State { if (!mounted) return; setState(() => isLoading = false); - if (decision == AuthNext.otp) { - await controller.auth.sendOtp(phoneController.text); - if (!mounted) return; - openScreen(context, OtpPage()); - } else if (decision == AuthNext.verifyPin) { - openScreen(context, Mpin()); - } else { - // notRegistered / error. Reported on the screen rather than in a snackbar - // under the keyboard, where the old version put it. - setState(() { - _showError = true; - _errorText = - 'We could not sign you in with that number. Check it and try ' - 'again, or contact your manager.'; - }); + // ── The server decides which screen, on one boolean ── + // + // `POST /miler/login` answers `pin_set`. False means a rider who has never + // signed in — the console no longer issues PINs, so he sets his own. True + // is the path every existing rider has always taken. + // + // This used to branch to an OTP screen when the directory said "no such + // account": the code was never verified (`verifyOtp` returned true without + // checking) and it dead-ended at a Create-MPIN screen with no route to + // call. A rider who took it could not get back. + switch (decision) { + case AuthNext.setPin: + openScreen(context, CreateMpin()); + case AuthNext.verifyPin: + openScreen(context, Mpin()); + case AuthNext.notRegistered: + setState(() { + _showError = true; + _errorText = + 'That number is not registered as a Miler. Check it, or ask ' + 'your manager to add you.'; + }); + case AuthNext.inactive: + setState(() { + _showError = true; + _errorText = + 'This account is not active. Contact your manager.'; + }); + case AuthNext.error: + // The directory could not be reached. Not a fact about the rider, and + // deliberately worded as the temporary thing it is. + setState(() { + _showError = true; + _errorText = + 'We could not reach the server. Check your connection and try ' + 'again.'; + }); } } diff --git a/test/activity_trips_test.dart b/test/activity_trips_test.dart index 8388e71..284503d 100644 --- a/test/activity_trips_test.dart +++ b/test/activity_trips_test.dart @@ -207,10 +207,22 @@ void main() { ); }); - test('the other line’s records are not this one’s', () async { + test('but the other line’s records ARE this one’s', () async { + // Reversed 2026-09-16 with the removal of the line from [WorkScope]. + // Under one-Miler this rule lost a rider his own work: a meal drop and a + // parcel collection finished in the same shift are both his, and filing + // them in two drawers meant Activity showed him half his day and no + // explanation for the other half. + // + // Ownership is still enforced — see the test above, which is the one that + // was actually protecting anything. await addCompletedBookings([done('a')], terminalStatus: 'delivered'); ServiceProfile.setActive(ServiceProfile.parcel); - expect(await getCompletedOrderIds(), isEmpty); + expect( + await getCompletedOrderIds(), + contains('a'), + reason: 'one rider, one day, one history', + ); }); }); diff --git a/test/arrived_needs_the_rider_there_test.dart b/test/arrived_needs_the_rider_there_test.dart new file mode 100644 index 0000000..4730a4d --- /dev/null +++ b/test/arrived_needs_the_rider_there_test.dart @@ -0,0 +1,103 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// ARRIVED IS A REPORT OF WHERE THE RIDER IS, SO IT HAS TO BE TRUE +/// +/// `_advanceStop` writes the ARRIVED rung optimistically, and the reasoning for +/// that is sound and stays: a rider standing at a counter with no signal must +/// not have his own report snapped back because a poll was late. +/// +/// What was wrong is that `updateArrivedStatus` answers `false` for THREE +/// different things and the caller treated all three as that one: +/// +/// 1. the request never landed — no signal. Optimistic is right. +/// 2. **the geofence refused** — the rider is 1.4km from the door and no +/// request was ever made. +/// 3. **the server refused** — it was asked and said no, on a business rule. +/// +/// On 2 and 3 the app wrote ARRIVED onto a stop the rider had not reached. The +/// admin console only ever hears what the server was told, so it went on showing +/// the stop as pending — arrived on the phone, not arrived in the office, with +/// no way for either side to know which was right. That is the divergence +/// reported from the field. +/// +/// Read from source: `_advanceStop` is a private method on a State class in a +/// 4,000-line page, and standing up GetX, ScreenUtil, Geolocator and a live +/// controller to assert one branch would test the harness more than the rule. +/// The regression class here is *a missing guard*, and the cheapest honest +/// check for a missing guard is to look for it. +/// ───────────────────────────────────────────────────────────────────────── +void main() { + final homepage = File( + 'lib/views/Dashboard/home/homepage.dart', + ).readAsStringSync(); + + final controller = File( + 'lib/controllers/pickups_controller.dart', + ).readAsStringSync(); + + group('a blocked arrival does not advance the rung', () { + test('the caller consults the geofence verdict before advancing', () { + expect( + homepage, + contains('dc.lastBlockedReason'), + reason: + 'Without this the optimistic branch advances ARRIVED on a stop the ' + 'geofence just refused, and the console never hears about it.', + ); + }); + + test('it also stops on a refusal the server actually sent', () { + expect( + homepage, + contains('dc.lastArrivalRefusal'), + reason: + 'A 4xx is the office saying no. Advancing past it shows the rider ' + 'as arrived somewhere the office has on record he is not.', + ); + }); + + test('the reason is handed back, not swallowed', () { + // A rider who is simply told "refused" slides again at a stop that will + // keep refusing. The geofence reason carries the distance and the target. + expect(homepage, contains('if (notThere != null) return notThere;')); + }); + }); + + group('a dead network still advances', () { + test('the optimistic path is intact', () { + // The fix narrowed the optimistic branch; it did not delete it. A rider + // at a counter with no signal still gets his rung, and + // `lastArrivalNotice` is what tells him the office has not been told. + expect(controller, contains('lastArrivalNotice')); + expect( + controller, + contains('Marked arrived on your phone'), + reason: 'the no-signal notice is the half that must NOT become a block', + ); + }); + + test('a transport failure sets no refusal, so it falls through', () { + // `lastArrivalRefusal` is assigned null when the code is below 400 — + // that null is load-bearing now, because it is what lets the optimistic + // branch still run for a rider with no signal. + expect(controller, contains('lastArrivalRefusal = code >= 400')); + }); + }); + + group('the verdicts are fresh, not left over', () { + test('an arrival clears both signals on entry', () { + // Read only when the write fails, and the caller now acts on them — so a + // value left from an earlier stop would refuse an arrival because a + // different door refused twenty minutes ago. + expect(controller, contains('lastArrivalRefusal = null')); + expect(controller, contains('lastArrivalNotice = null')); + }); + + test('the geofence check clears its own reason on entry', () { + expect(controller, contains('lastBlockedReason = null')); + }); + }); +} diff --git a/test/base_handover_card_test.dart b/test/base_handover_card_test.dart new file mode 100644 index 0000000..8d5f4f5 --- /dev/null +++ b/test/base_handover_card_test.dart @@ -0,0 +1,233 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_screenutil/flutter_screenutil.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:miler/data/api_config.dart'; +import 'package:miler/data/service_profile.dart'; +import 'package:miler/views/Dashboard/pickups/pickups.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// A HANDOVER CARD NAMES THE BASE IT IS SENDING HIM TO +/// +/// The card already refused to lead with the receiver, and was right to — on a +/// base leg he is in another district, and heading the card with him is the +/// navigation mistake that rule exists to prevent. +/// +/// But refusing the receiver only left the *pickup*, so a handover card read: +/// +/// BASE HANDOVER +/// Anitha R +/// Gandhipuram +/// +/// …the customer he had collected from an hour earlier, under a heading telling +/// him to go to a base. The place he was actually going appeared nowhere on the +/// card — only on the sheet's slider, after he had already set off. On a round +/// with two bases that is a parcel left in the wrong building. +/// +/// `next_hub` rides on the row (the backend sends all six fields: id, name, +/// address, pincode, latitude, longitude), so the destination is knowable at +/// card-build time and there is no excuse for the card not to say it. +/// ───────────────────────────────────────────────────────────────────────── +void main() { + setUp(() => ServiceProfile.setActive(ServiceProfile.parcel)); + tearDown(() => ServiceProfile.setActive(ServiceProfile.parcel)); + + Future pump(WidgetTester tester, Widget child) async { + tester.view.physicalSize = const Size(1170, 2532); + tester.view.devicePixelRatio = 3.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + + await tester.pumpWidget( + ScreenUtilInit( + designSize: const Size(390, 844), + builder: (_, _) => MaterialApp( + home: Scaffold(body: SingleChildScrollView(child: child)), + ), + ), + ); + await tester.pumpAndSettle(); + } + + /// A collected parcel routed to a base — the row as the queue returns it with + /// `MILER_HUB_HANDOVER_ENABLED` on. + Map hubStop({Map? hub}) => + ApiConfig.pickupFromBooking({ + 'bookingid': 4821, + 'orderid': 'ORDER-HUB-1', + 'status': 'Converted_To_Consignment', + 'consignmentstatus': 'Created', + 'consignmentid': 77, + 'next_action': 'inward_at_hub', + 'sourcename': 'Anitha R', + 'pickupcustomer': 'Anitha R', + 'pickupaddress': '12 Gandhipuram, Coimbatore', + 'recipientname': 'Ravi Kumar', + 'dropaddress': '88 Anna Nagar, Chennai 600040', + 'next_hub': + hub ?? + const { + 'id': 1, + 'name': 'Coimbatore Base', + 'address': '14 Avinashi Road, Peelamedu', + 'pincode': '641004', + 'latitude': 11.0272, + 'longitude': 76.9905, + }, + }); + + /// The same parcel, going to a customer instead. + Map customerStop() => ApiConfig.pickupFromBooking(const { + 'bookingid': 4822, + 'orderid': 'ORDER-CX-1', + 'consignmentstatus': 'Out_for_Delivery', + 'consignmentid': 78, + 'next_action': 'deliver', + 'pickupcustomer': 'Anitha R', + 'pickupaddress': '12 Gandhipuram, Coimbatore', + 'recipientname': 'Ravi Kumar', + 'dropaddress': '4 Race Course, Coimbatore 641018', + }); + + testWidgets('the BASE leads, not the sender he collected from', ( + tester, + ) async { + await pump( + tester, + PickupCard( + item: hubStop(), + displayStep: 1, + distanceMeters: 4200, + collectedIds: const {'ORDER-HUB-1'}, + ), + ); + + expect(find.text('BASE HANDOVER'), findsOneWidget); + expect( + find.text('Coimbatore Base'), + findsOneWidget, + reason: 'the rider must know WHICH base without opening a sheet', + ); + expect( + find.text('Anitha R'), + findsNothing, + reason: + 'the sender he collected from an hour ago is not the destination', + ); + }); + + testWidgets('the base address is readable at a gate', (tester) async { + await pump( + tester, + PickupCard( + item: hubStop(), + displayStep: 1, + distanceMeters: 4200, + collectedIds: const {'ORDER-HUB-1'}, + ), + ); + + // Street and pincode, not the locality summary the other legs use — + // "Peelamedu" alone does not find a loading bay. + expect( + find.textContaining('14 Avinashi Road'), + findsOneWidget, + ); + expect(find.textContaining('641004'), findsOneWidget); + }); + + testWidgets('the distant receiver never takes the headline', (tester) async { + await pump( + tester, + PickupCard( + item: hubStop(), + displayStep: 1, + distanceMeters: 4200, + collectedIds: const {'ORDER-HUB-1'}, + ), + ); + + // The rule that predates this change and must survive it: a Chennai + // receiver on a parcel going to a Coimbatore base must not be drawn as the + // place the rider is heading. + expect(find.text('Ravi Kumar'), findsNothing); + }); + + testWidgets('a base with no coordinates still names itself', (tester) async { + // It is a caption rather than a destination — navigating to 0,0 lands in + // the Gulf of Guinea — but the rider still needs to read which base it is. + // The geofence refuses the handover separately, with a sentence pointing at + // the office. + await pump( + tester, + PickupCard( + item: hubStop(hub: const {'id': 2, 'name': 'Salem Base'}), + displayStep: 1, + distanceMeters: 4200, + collectedIds: const {'ORDER-HUB-1'}, + ), + ); + + expect(find.text('BASE HANDOVER'), findsOneWidget); + // Once — the name is the headline, and the address line stays empty rather + // than repeating it. + expect(find.text('Salem Base'), findsOneWidget); + }); + + testWidgets('a customer delivery still leads with the RECIPIENT', ( + tester, + ) async { + // The other half of the contract. This change must not push the base + // treatment onto a leg that has a real receiver — that fix (recipient, not + // sender) was itself a production bug once. + await pump( + tester, + PickupCard( + item: customerStop(), + displayStep: 1, + distanceMeters: 1400, + collectedIds: const {'ORDER-CX-1'}, + ), + ); + + expect(find.text('CUSTOMER DELIVERY'), findsOneWidget); + expect(find.text('Ravi Kumar'), findsOneWidget); + + // The sender IS still on the card, and should be: once the parcel is in the + // box the pickup is history, but it is what establishes custody and on a + // run with two counters it is how he knows which shelf the label came off. + // It sits under a hairline at half the weight — so "leads" is a claim about + // TYPE SIZE, and that is what gets asserted rather than mere presence. + expect(find.text('Anitha R'), findsOneWidget); + + double sizeOf(String text) => + tester.widget(find.text(text)).style?.fontSize ?? 0; + + expect( + sizeOf('Ravi Kumar'), + greaterThan(sizeOf('Anitha R')), + reason: 'the receiver leads; the sender is context beneath him', + ); + }); + + testWidgets('on a base leg the sender is not drawn as context either', ( + tester, + ) async { + // The context block is gated on `dropLeads`, which is false for a base leg + // — so a handover card carries the base and nothing that could be mistaken + // for a destination. + await pump( + tester, + PickupCard( + item: hubStop(), + displayStep: 1, + distanceMeters: 4200, + collectedIds: const {'ORDER-HUB-1'}, + ), + ); + + expect(find.text('Anitha R'), findsNothing); + expect(find.text('Ravi Kumar'), findsNothing); + expect(find.text('Coimbatore Base'), findsOneWidget); + }); +} diff --git a/test/deliver_identity_test.dart b/test/deliver_identity_test.dart index 759c54f..3c3a8db 100644 --- a/test/deliver_identity_test.dart +++ b/test/deliver_identity_test.dart @@ -56,22 +56,35 @@ void main() { ); }); - test('the mapping is per rider and per line', () async { + test('the mapping is per rider — and follows him across lines', () async { + // ── This asserted the opposite until 2026-09-16 ── + // + // The scope key carried a third part, the rider's service line: + // `consignment_ids::u38.t13.milkMan`. That was right while a rider was + // one thing for the life of his account. He is not — one Miler carries + // meal parcels, logistics collections and customer pickups in the same + // shift — and keying his records by a line split one day's work across + // two drawers, with whichever drawer the app resolved into being the only + // one he could see. + // + // For a *consignment id* that was not a cosmetic loss: the id is what + // `deliver` and `skip` key on, so a stop collected while the app read one + // line and delivered after it read the other could not be closed at all. + // The rider was told "this order was never picked up on the system". await rememberConsignmentId('ORDER-1', '46'); ServiceProfile.setActive(ServiceProfile.parcel); expect( (await getConsignmentIds())['ORDER-1'], - isNull, - reason: "one line's consignment mapping is not another's", + '46', + reason: 'one rider, one day, one drawer — whatever he is carrying', ); SharedPreferences.setMockInitialValues({'userid': 99}); - ServiceProfile.setActive(ServiceProfile.milkMan); expect( (await getConsignmentIds())['ORDER-1'], isNull, - reason: "another rider's handset drawer is not this one", + reason: "another rider's handset drawer is still not this one", ); }); diff --git a/test/delivery_leg_test.dart b/test/delivery_leg_test.dart index 3afc1c7..e7fc60f 100644 --- a/test/delivery_leg_test.dart +++ b/test/delivery_leg_test.dart @@ -27,22 +27,39 @@ void main() { tearDown(() => ServiceProfile.setActive(ServiceProfile.parcel)); - group('the three outcomes', () { - test('are exactly delivered, skip and cancelled', () { + group('the four ways a stop in custody can end', () { + test('are delivered, skip, cancelled and handed over', () { + // ── `handedOver` was added 2026-09-16 ── + // + // There used to be three, and the missing fourth is why a base-routed + // parcel could not be closed at all: `MilerApi.inwardAtHub` existed, was + // documented and was covered by `hub_handover_contract_test.dart`, and + // had no call site. A rider carrying a Gandhipuram → Chennai consignment + // was told "hand it over at base" by `releaseForDelivery` and given no + // control that would record him doing so. + // + // It is deliberately NOT a kind of `delivered`: a base handover has no + // receiver, no proof photo, no OTP and no COD, and it must never reach + // `deliver` — that route moves the consignment to `Out_for_Delivery` + // against a receiver in another district, which the handset cannot undo. expect(DeliveryOutcome.values.map((o) => o.label), [ 'Delivered', 'Skip', 'Cancelled', + 'Handed over', ]); }); test('delivered is the only one that completes the order', () { // Skip is a return visit — it increments `attemptcount` rather than - // failing the parcel — and cancelled is a withdrawal. Neither is a - // delivery, and Activity must not file them as one. + // failing the parcel — and cancelled is a withdrawal. Handed over ends + // this RIDER's custody while the parcel's own journey carries on inside + // the network. None of the three is a delivery, and Activity must not + // file them as one. expect(DeliveryOutcome.delivered.pastTense, 'Delivered'); expect(DeliveryOutcome.skipped.pastTense, 'Skipped'); expect(DeliveryOutcome.cancelled.pastTense, 'Cancelled'); + expect(DeliveryOutcome.handedOver.pastTense, 'Handed over'); }); test('each one is coloured by what it means, not by where it sits', () { diff --git a/test/delivery_proof_test.dart b/test/delivery_proof_test.dart index 065ae42..e7b11a3 100644 --- a/test/delivery_proof_test.dart +++ b/test/delivery_proof_test.dart @@ -141,10 +141,16 @@ void main() { ); }); - test('the other line cannot either', () async { + test('but the same rider can, whatever line he is reading', () async { + // Reversed 2026-09-16 with the removal of the line from [WorkScope]. + // A doorstep photo is evidence the rider produced; hiding it from him + // because a tenant label resolved differently after a refresh is how a + // proof goes missing at exactly the moment somebody disputes a delivery. + // + // The ownership rule above is untouched and is the one that matters. await ProofStore.save('ORDER-1', (await cameraShot('a')).path); ServiceProfile.setActive(ServiceProfile.parcel); - expect(await ProofStore.pathFor('ORDER-1'), isNull); + expect(await ProofStore.pathFor('ORDER-1'), isNotNull); }); test('logout deletes the photos, not just the index', () async { diff --git a/test/geofence_flag_test.dart b/test/geofence_flag_test.dart index f07c8e1..aabfa58 100644 --- a/test/geofence_flag_test.dart +++ b/test/geofence_flag_test.dart @@ -1,67 +1,92 @@ import 'package:flutter_test/flutter_test.dart'; -import 'package:miler/controllers/pickups_controller.dart'; +import 'package:miler/data/geofence.dart'; -/// Whichever way the proximity fence is set, it must be set **once** and on -/// purpose. +/// ───────────────────────────────────────────────────────────────────────── +/// THE SWITCH, AND THE FOUR SHAPES THAT MUST NOT COME BACK /// -/// [kGeofenceEnforced] decides whether "Picked up" means the rider was standing -/// at the door or merely pressed a button, and it has already been wrong in -/// three directions: it was `kDebugMode` — so every debug build had no fence -/// and the fence was therefore never tested — then a hard `false`, so the flow -/// could not be walked at a desk at all, then a define defaulting to off. +/// This file used to assert the opposite of what it asserts now, and that is +/// the point of keeping it rather than deleting it. /// -/// **It is OFF, decided 2026-08-25.** It was turned on that morning with a -/// radius of [kGeofenceRadiusMeters] and turned off again the same day, at the -/// founder's call. Nothing was found wrong with it — this is a decision about -/// *when* to switch it on. +/// It read: /// -/// So the direction this file guards has flipped twice in a day, which is -/// exactly the pattern that made the old defaults dangerous, and it is why the -/// three tests below are not all about the direction. The measurement work is -/// unchanged and still asserted: a `best` fix with a shelf life, the phone's -/// error credited to the rider, and one radius rather than three. All of it is -/// dormant while [kGeofenceEnforced] is false and none of it needs revisiting -/// on the day it goes back on. +/// expect(kGeofenceEnforced, isFalse); // "recorded decision", 2026-08-25 +/// expect(kGeofenceRadiusMeters, 10); +/// +/// So the test suite was **enforcing the release blocker**: a developer who +/// turned the fence on to fix it would be met with a red test telling him the +/// off state was deliberate. Between them those two lines meant every shipped +/// build let a rider mark a stop picked up from anywhere on earth, and the +/// suite agreed that was correct. +/// +/// Reversed 2026-09-16 as part of the Play Store release audit. The fence now +/// lives in `lib/data/geofence.dart` and the behaviour is exercised properly in +/// `geofence_test.dart` — 41 cases, the full distance table, every failure +/// mode, and an HTTP client that fails the test if a blocked rung posts +/// anything. +/// +/// What is left here is the short list of *specific past mistakes*, each of +/// which shipped, so that none of them can return quietly. +/// ───────────────────────────────────────────────────────────────────────── void main() { - test('there is one switch, and the legacy name derives from it', () { - // Two constants that could disagree is exactly how a build ends up with - // the fence on in one gate and off in the other — the bulk check on Home - // and the per-stop check in the controller are separate code paths that - // must never answer differently. - expect(kBypassGeofenceForTesting, !kGeofenceEnforced); - }); - - test('enforcement is off, and that is the recorded decision', () { - // If this fails, one of two things happened, and both should be read rather - // than silenced: - // - // • the default was turned back on — record the date and the reason here, - // as the previous flips did; or - // • this run passed `--dart-define=ENFORCE_GEOFENCE=true`, in which case - // the failure is correct and is telling you this build has the fence on. + test('MISTAKE 1 — the fence must not default to off', () { + // `bool.fromEnvironment('ENFORCE_GEOFENCE', defaultValue: false)`. Every + // `flutter build appbundle --release` passes no dart-define, so the default + // *is* the shipped behaviour. It had been `kDebugMode` before that — off in + // exactly the builds anyone tested with — and a hard `false` before that. + // Three spellings, one effect. expect( kGeofenceEnforced, - isFalse, + isTrue, reason: - 'Proximity enforcement is expected to be OFF (see kGeofenceEnforced). ' - 'Either the default was restored, or this run passed ' - '--dart-define=ENFORCE_GEOFENCE=true.', + 'The release build must enforce proximity. If this run passed ' + '--dart-define=ENFORCE_GEOFENCE=false, this failure is correct.', ); }); - test('the radius is one number, and it is the tight one', () { - // Asserted while the fence is off, deliberately. The app used to hold - // three — a configured `pickupradius` defaulting to 100 in the controller, - // a hardcoded 500 in Home's bulk gate, and no agreement between them — so - // which fence a rider met depended on whether he ticked boxes or slid a - // sheet. Whichever way the switch above is set, that must not come back. - expect(kGeofenceRadiusMeters, 10); + test('MISTAKE 2 — the radius must not be smaller than the GPS', () { + // 10 m is at or inside the error radius of consumer GPS, so the fence + // stopped measuring proximity and started measuring whether the satellites + // were kind. It refused riders standing at doors, which is what got the + // whole control switched off twice. + expect(kGeofenceRadiusMetres, 100); + expect( + kGeofenceRadiusMetres, + greaterThan(kGeofenceMaxAccuracyMetres), + reason: 'a fence smaller than the fix that measures it cannot be met', + ); }); - test('a cached fix has a shelf life', () { - // On a round, a stale position is reliably the *previous* stop. Anything - // much longer than this and the fence starts measuring from the last door. + test('MISTAKE 3 — there must be exactly one radius', () { + // The app once held three at the same time: a per-tenant `pickupradius` + // from prefs defaulting to 100, a hardcoded 500 in Home's bulk gate, and 10 + // in the controller. Which fence a rider met depended on whether he ticked + // boxes on Home or slid the sheet on a stop. + // + // Both gates call `Geofence.check` now and neither holds arithmetic, so + // this asserts the constant is the only knob there is to turn. + expect(kGeofenceRadiusMetres, isA()); + expect(kGeofenceRadiusMetres, greaterThan(0)); + }); + + test('MISTAKE 4 — a cached fix must have a shelf life', () { + // A last-known position is instant, free, and can be an hour old. On a + // round it is reliably the *previous* stop, which is how a rider marks a + // delivery arrived from the last street he was on. expect(kGeofenceFixMaxAge, const Duration(seconds: 30)); }); + + test('the fence still has a way to be switched off in an emergency', () { + // Deliberately kept, and deliberately not the default. If the fence starts + // refusing real work at real doors, `--dart-define=ENFORCE_GEOFENCE=false` + // gets the fleet moving inside one release rather than one sprint — and + // every bypassed check logs in every build mode, so a build's own log says + // which way it was compiled. + // + // Raise `kGeofenceRadiusMetres` before reaching for it. + expect( + const bool.fromEnvironment('ENFORCE_GEOFENCE', defaultValue: true), + kGeofenceEnforced, + ); + }); } diff --git a/test/geofence_test.dart b/test/geofence_test.dart new file mode 100644 index 0000000..bc7ff00 --- /dev/null +++ b/test/geofence_test.dart @@ -0,0 +1,588 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:geolocator/geolocator.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:miler/controllers/pickups_controller.dart'; +import 'package:miler/data/geofence.dart'; +import 'package:miler/data/miler_api.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// THE PROXIMITY FENCE +/// +/// This is the control that decides whether "Picked up" means the rider was +/// standing at the door or merely pressed a button, and it has been wrong in +/// four directions inside one year: compiled off by default, set to a 10 m +/// radius no consumer GPS can resolve, failing **open** on any thrown +/// exception, and crediting the phone's own error to the rider so that a vague +/// fix bought slack rather than a refusal. +/// +/// So the tests below are not a spot check of the arithmetic. They pin, in +/// order: +/// +/// 1. the **switch** — on by default, because that default has flipped twice; +/// 2. the **radius** — 100 m, inclusive at the boundary; +/// 3. the **distance table** the spec asks for, 0 m to 250 m; +/// 4. every way of **failing to measure**, each of which must refuse; +/// 5. the one that matters most — **a blocked rung sends no HTTP request.** +/// +/// (5) is the whole point. A fence that shows a message and posts the status +/// anyway is worse than no fence: the hub is told the rider was there *and* +/// the app claims to have checked. +/// ───────────────────────────────────────────────────────────────────────── + +/// A stop in Coimbatore. Any point would do; a real one keeps the numbers +/// recognisable when a failure prints them. +const double kTargetLat = 11.0272; +const double kTargetLng = 76.9905; + +/// Metres per degree of latitude. Constant enough for a fixture — the tests +/// assert against what [Geolocator.distanceBetween] actually measures, not +/// against this, so the approximation cannot make a test pass wrongly. +const double _metresPerDegreeLat = 111320.0; + +/// A rider fix [metres] due north of the stop. +Position riderAt( + double metres, { + double accuracy = 8, + Duration age = Duration.zero, +}) => Position( + latitude: kTargetLat + (metres / _metresPerDegreeLat), + longitude: kTargetLng, + timestamp: DateTime.now().subtract(age), + accuracy: accuracy, + altitude: 0, + altitudeAccuracy: 0, + heading: 0, + headingAccuracy: 0, + speed: 0, + speedAccuracy: 0, +); + +void useFix(Position p) => + Geofence.positionProvider = () async => GeofenceFix(position: p); + +void useFailure(GeofenceOutcome outcome) => + Geofence.positionProvider = () async => GeofenceFix(failure: outcome); + +void main() { + tearDown(Geofence.useDevice); + + // ─────────────────────────────────────────────────────────────────────── + group('the switch and the radius', () { + test('enforcement is ON by default', () { + // It was `defaultValue: false`, so every release build shipped with the + // fence returning true on its third line. If this fails, either the + // default was reverted — which is the release blocker this file exists to + // prevent — or the run passed `--dart-define=ENFORCE_GEOFENCE=false`, in + // which case the failure is correct and is telling you so. + expect( + kGeofenceEnforced, + isTrue, + reason: + 'Proximity enforcement must be ON by default. Either the default ' + 'was reverted, or this run passed ENFORCE_GEOFENCE=false.', + ); + }); + + test('the radius is 100 metres, and there is only one of them', () { + // The app once held three: a configured `pickupradius` defaulting to 100, + // a hardcoded 500 in Home's bulk gate, and 10 in the controller. Which + // fence a rider met depended on whether he ticked boxes or slid a sheet. + expect(kGeofenceRadiusMetres, 100); + }); + + test('one fix may serve a batch, but only briefly', () { + // [kGeofenceFixReuse] is what makes a twenty-order bulk action cost one + // GPS settle instead of twenty. It is a performance change to a safety + // control, so the two bounds that keep it honest are asserted here. + expect( + kGeofenceFixReuse, + lessThanOrEqualTo(kGeofenceFixMaxAge), + reason: + 'the reuse window must sit inside the staleness rule — a fix too ' + 'old to measure with cannot become acceptable by being cached', + ); + // At walking pace (~1.4 m/s) the window is ~20 m of possible drift + // against a 100 m fence. Riding away covers the radius inside it, so a + // rider who genuinely leaves is refused on the next real fix rather than + // carried to the next door by the cached one. + expect( + kGeofenceFixReuse.inSeconds * 1.4, + lessThan(kGeofenceRadiusMetres / 2), + reason: 'drift during the window must stay well under the radius', + ); + }); + + test('resetting the cache is available to sign-out and tests', () { + // A cached position is a fact about a rider. It must not outlive him on a + // shared handset. + Geofence.resetCache(); + Geofence.useDevice(); + }); + + test('a cached fix has a shelf life, and a vague one is rejected', () { + expect(kGeofenceFixMaxAge, const Duration(seconds: 30)); + expect( + kGeofenceMaxAccuracyMetres, + lessThan(kGeofenceRadiusMetres), + reason: + 'a fix whose error is as large as the fence cannot resolve it, and ' + 'must be refused rather than credited', + ); + }); + }); + + // ─────────────────────────────────────────────────────────────────────── + group('the distance table', () { + // Every case the spec names. Each asserts the *rule* against what + // Geolocator actually measured, so a fixture that drifts by a metre cannot + // quietly invert a boundary case. + for (final metres in const [0.0, 25.0, 50.0, 99.0, 100.0, 101.0, 125.0, + 250.0]) { + test('${metres.round()} m', () async { + final pos = riderAt(metres); + useFix(pos); + + final measured = Geofence.distanceBetween( + kTargetLat, + kTargetLng, + pos.latitude, + pos.longitude, + ); + expect( + measured, + closeTo(metres, 1.0), + reason: 'fixture should sit where it claims', + ); + + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + + final shouldAllow = measured <= kGeofenceRadiusMetres; + expect( + d.allowed, + shouldAllow, + reason: + 'at ${measured.toStringAsFixed(1)} m against a ' + '${kGeofenceRadiusMetres.round()} m fence', + ); + expect( + d.outcome, + shouldAllow ? GeofenceOutcome.inside : GeofenceOutcome.outside, + ); + expect(d.distanceMetres, closeTo(measured, 0.001)); + expect(d.accuracyMetres, 8); + expect(d.timestamp, isNotNull); + }); + } + + test('the boundary is inclusive — 100 m is in, a hair over is out', () { + // Stated as arithmetic rather than as a fixture, because this is the one + // place a `<` for a `<=` changes a rider's day and no GPS fixture can be + // placed accurately enough to catch it. + expect(99.999 <= kGeofenceRadiusMetres, isTrue); + expect(100.0 <= kGeofenceRadiusMetres, isTrue); + expect(100.001 <= kGeofenceRadiusMetres, isFalse); + }); + + test('a refusal tells the rider how far, and in a unit he uses', () async { + useFix(riderAt(240)); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.reason, contains('240 m')); + expect(d.reason, contains('100 m')); + expect(d.reason, contains('arrived')); + // Never a developer error, a status code or a coordinate pair. + expect(d.reason, isNot(contains('Exception'))); + expect(d.reason, isNot(contains('null'))); + }); + + test('over a kilometre reads in kilometres', () async { + useFix(riderAt(4200)); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Delivered', + ); + expect(d.reason, contains('4.2 km')); + expect(d.reason, contains('delivered')); + }); + + test('the verb follows the rung', () async { + useFix(riderAt(500)); + for (final action in const [ + 'Arrived', + 'Picked', + 'Picked up', + 'Delivery arrived', + 'Delivered', + 'Handed over', + ]) { + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: action, + ); + expect(d.allowed, isFalse, reason: action); + expect(d.reason, contains(action.toLowerCase()), reason: action); + } + }); + }); + + // ─────────────────────────────────────────────────────────────────────── + group('every way of failing to measure is a refusal', () { + // The old implementation allowed on four of these six. That is what made + // the fence decorative: switching location off opened every rung. + + test('GPS switched off', () async { + useFailure(GeofenceOutcome.serviceDisabled); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.serviceDisabled); + expect(d.reason, contains('Location is switched off')); + }); + + test('permission denied', () async { + useFailure(GeofenceOutcome.permissionDenied); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Delivered', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.permissionDenied); + expect(d.reason, contains('Allow location access')); + }); + + test('permission denied forever points at Settings', () async { + useFailure(GeofenceOutcome.permissionDeniedForever); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.permissionDeniedForever); + expect(d.reason, contains('Settings')); + }); + + test('a timeout waiting for the fix', () async { + useFailure(GeofenceOutcome.timeout); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.timeout); + }); + + test('a stale fix is not a measurement', () async { + // On a round, a stale position is reliably the *previous* stop — which is + // how a rider marks a delivery arrived from the last street. + useFailure(GeofenceOutcome.staleFix); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.staleFix); + }); + + test('a fix too vague to resolve the fence is refused, not credited', + () async { + // The inversion this replaces: the old check was + // `distance - accuracy > radius`, so a ±250 m fix bought 250 m of slack. + // The worse the fix, the easier it was to pass. Here the rider is 10 m + // away — comfortably inside — and is still refused, because the phone + // cannot show that he is. + useFix(riderAt(10, accuracy: kGeofenceMaxAccuracyMetres + 1)); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.poorAccuracy); + expect(d.reason, contains('Step into the open')); + expect(d.accuracyMetres, kGeofenceMaxAccuracyMetres + 1); + }); + + test('a vague fix cannot pass even when it is standing on the stop', + () async { + useFix(riderAt(0, accuracy: 500)); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Delivered', + ); + expect(d.allowed, isFalse, reason: 'distance 0 must not beat accuracy'); + }); + + test('a fix at the accuracy ceiling is still usable', () async { + useFix(riderAt(20, accuracy: kGeofenceMaxAccuracyMetres)); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isTrue, reason: 'the ceiling is inclusive'); + }); + + test('the provider throwing is a refusal, not a pass', () async { + // This is the `catch { return true; // fail-safe }` that used to open + // every rung in the app on any throw from the location stack. + Geofence.positionProvider = () async => throw Exception('platform'); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.error); + expect(d.reason, isNotNull); + expect(d.reason, isNot(contains('platform')), + reason: 'the rider must not be shown the exception'); + }); + + test('a rider fix of 0,0 is absent, not the Gulf of Guinea', () async { + useFix( + Position( + latitude: 0, + longitude: 0, + timestamp: DateTime.now(), + accuracy: 5, + altitude: 0, + altitudeAccuracy: 0, + heading: 0, + headingAccuracy: 0, + speed: 0, + speedAccuracy: 0, + ), + ); + final d = await Geofence.check( + targetLat: kTargetLat, + targetLng: kTargetLng, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.noFix); + }); + }); + + // ─────────────────────────────────────────────────────────────────────── + group('a stop with no coordinates', () { + // This used to return true — "Missing coordinates. Proceeding with update." + // A booking with a blank latitude therefore opened every rung on that stop, + // which is the bypass that makes a fence decorative. + for (final pair in const [ + (null, null, 'both null'), + (0.0, 0.0, 'both zero'), + (kTargetLat, 0.0, 'longitude zero'), + (0.0, kTargetLng, 'latitude zero'), + (95.0, kTargetLng, 'latitude out of range'), + (kTargetLat, 200.0, 'longitude out of range'), + ]) { + test('is refused — ${pair.$3}', () async { + useFix(riderAt(0)); + final d = await Geofence.check( + targetLat: pair.$1, + targetLng: pair.$2, + action: 'Arrived', + ); + expect(d.allowed, isFalse); + expect(d.outcome, GeofenceOutcome.noTarget); + // The rider cannot fix the hub's data from a doorstep, so the sentence + // sends him to the people who can. + expect(d.reason, contains('office')); + }); + } + + test('it does not even ask the phone for a fix', () async { + // An 8-second GPS settle to answer a question with no target is time + // taken off a rider's round for nothing. + var asked = false; + Geofence.positionProvider = () async { + asked = true; + return GeofenceFix(position: riderAt(0)); + }; + await Geofence.check( + targetLat: null, + targetLng: null, + action: 'Arrived', + ); + expect(asked, isFalse); + }); + }); + + // ─────────────────────────────────────────────────────────────────────── + group('BLOCKED MEANS NO REQUEST IS SENT', () { + // The rule the whole control rests on. Every one of these drives the real + // `PickupsController` rung with an HTTP client that fails the test if it is + // ever called — so this asserts the *call sites*, not the policy object. + + late PickupsController controller; + late List sent; + + setUp(() { + SharedPreferences.setMockInitialValues({'userid': 38}); + sent = []; + MilerApi.client = MockClient((req) async { + sent.add('${req.method} ${req.url.path}'); + // A realistic `reached` reply, in the shape production actually + // returns — `success` is the envelope key (`ApiConfig.toLegacyEnvelope` + // reads it; `status` is ignored), and the arrival stamp sits inside + // `data`. A bare 200 is deliberately NOT read as a transition (see + // `MilerLifecycle.reached`), so a thin fixture would fail the positive + // case below for a reason that has nothing to do with the fence. + return http.Response( + '{"success":true,"data":{"bookingid":4821,' + '"status":"Miler_Assigned","reachedat":"2026-09-16T10:14:02Z"}}', + 200, + ); + }); + controller = PickupsController(); + }); + + tearDown(() { + MilerApi.client = http.Client(); + Geofence.useDevice(); + }); + + test('Arrived, 250 m away — no request', () async { + useFix(riderAt(250)); + final ok = await controller.updateArrivedStatus( + pickupId: 4821, + orderHeaderId: 4821, + pickupLat: '$kTargetLat', + pickupLng: '$kTargetLng', + ); + expect(ok, isFalse); + expect(sent, isEmpty, reason: 'the fence refused; nothing may be posted'); + expect(controller.lastBlockedReason, contains('Move within 100 m')); + }); + + test('Arrived with GPS off — no request', () async { + useFailure(GeofenceOutcome.serviceDisabled); + final ok = await controller.updateArrivedStatus( + pickupId: 4821, + orderHeaderId: 4821, + pickupLat: '$kTargetLat', + pickupLng: '$kTargetLng', + ); + expect(ok, isFalse); + expect(sent, isEmpty); + }); + + test('Arrived with permission denied forever — no request', () async { + useFailure(GeofenceOutcome.permissionDeniedForever); + final ok = await controller.updateArrivedStatus( + pickupId: 4821, + orderHeaderId: 4821, + pickupLat: '$kTargetLat', + pickupLng: '$kTargetLng', + ); + expect(ok, isFalse); + expect(sent, isEmpty); + }); + + test('Arrived on a stop with no pin — no request', () async { + useFix(riderAt(0)); + final ok = await controller.updateArrivedStatus( + pickupId: 4821, + orderHeaderId: 4821, + pickupLat: '0', + pickupLng: '0', + ); + expect(ok, isFalse); + expect( + sent, + isEmpty, + reason: + 'a booking with no coordinates used to be waved through — that is ' + 'the bypass that made the fence decorative', + ); + }); + + test('Delivery arrived, 250 m away — no request', () async { + useFix(riderAt(250)); + final ok = await controller.updateDeliveryArrived( + pickupId: 4821, + dropLat: '$kTargetLat', + dropLng: '$kTargetLng', + ); + expect(ok, isFalse); + expect(sent, isEmpty); + }); + + test('Delivery arrived with NO drop pin — no request', () async { + // The conditional that used to wrap this call site — + // `if (dLat != 0 && dLng != 0)` — skipped the fence silently for exactly + // the stops where nobody could say afterwards where the rider was. + useFix(riderAt(0)); + final ok = await controller.updateDeliveryArrived( + pickupId: 4821, + dropLat: '0', + dropLng: '0', + ); + expect(ok, isFalse); + expect(sent, isEmpty); + }); + + test('Delivered, 250 m away — no request', () async { + useFix(riderAt(250)); + final ok = await controller.updateDeliveredStatus( + pickupId: 4821, + consignmentId: '77', + deliveredToName: 'Anitha R', + dropLat: '$kTargetLat', + dropLng: '$kTargetLng', + ); + expect(ok, isFalse); + expect(sent, isEmpty); + }); + + test('Delivered with NO drop pin — no request', () async { + useFix(riderAt(0)); + final ok = await controller.updateDeliveredStatus( + pickupId: 4821, + consignmentId: '77', + deliveredToName: 'Anitha R', + dropLat: '0', + dropLng: '0', + ); + expect(ok, isFalse); + expect(sent, isEmpty); + }); + + test('inside the fence, the request DOES go out', () async { + // The other half of the contract, and the reason this group is not just + // an assertion that everything is broken. + useFix(riderAt(40)); + final ok = await controller.updateArrivedStatus( + pickupId: 4821, + orderHeaderId: 4821, + pickupLat: '$kTargetLat', + pickupLng: '$kTargetLng', + ); + expect(ok, isTrue); + expect(sent, isNotEmpty); + expect(sent.single, contains('/miler/bookings/4821/reached')); + expect(controller.lastBlockedReason, isNull); + }); + }); +} diff --git a/test/hub_handover_action_test.dart b/test/hub_handover_action_test.dart new file mode 100644 index 0000000..f26fedc --- /dev/null +++ b/test/hub_handover_action_test.dart @@ -0,0 +1,249 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:miler/data/api_config.dart'; +import 'package:miler/data/geofence.dart'; +import 'package:miler/data/miler_api.dart'; +import 'package:miler/data/next_leg.dart'; +import 'package:miler/data/service_profile.dart'; +import 'package:miler/views/Dashboard/pickups/pickups.dart'; + +import 'geofence_test.dart' show kTargetLat, kTargetLng, riderAt; + +/// ───────────────────────────────────────────────────────────────────────── +/// THE BASE HANDOVER, AS A RIDER CAN NOW ACTUALLY PERFORM IT +/// +/// `MilerApi.inwardAtHub` and `MilerLifecycle.inwardAtHub` were written, +/// documented and covered by `hub_handover_contract_test.dart` — and had **zero +/// call sites**. So with `MILER_HUB_HANDOVER_ENABLED` on server-side, a +/// Gandhipuram → Chennai parcel arrived on Deliveries as [NextLeg.hub], +/// `releaseForDelivery` correctly refused to start a customer round for it, and +/// nothing in the app could record the rider handing it in. It sat in his queue +/// until hub staff inwarded it from a console, and the assignment closed +/// against nobody — so the job reported zero distance and zero value on his +/// earnings. +/// +/// `hub_handover_contract_test.dart` still owns the *contract* — the adapter +/// fields, the `next_action` vocabulary, both positions of the server flag, and +/// how the response is read. This file owns the *action*: that a rider standing +/// at the base can close the leg, that one standing anywhere else cannot, and +/// that nothing is posted when the fence refuses. +/// ───────────────────────────────────────────────────────────────────────── +void main() { + late List sent; + late List bodies; + + /// The row as `GET /miler/bookings` returns it with the server flag ON: the + /// pivot left the consignment `Created` and pointed the rider at a base. + Map hubRow() => ApiConfig.pickupFromBooking({ + 'bookingid': 4821, + 'orderid': 'ORDER-HUB-1', + 'status': 'Converted_To_Consignment', + 'consignmentstatus': 'Created', + 'consignmentid': 77, + 'next_action': 'inward_at_hub', + 'next_hub': { + 'id': 1, + 'name': 'Coimbatore Base', + 'address': '14 Avinashi Road, Peelamedu', + 'pincode': '641004', + 'latitude': kTargetLat, + 'longitude': kTargetLng, + }, + }); + + setUp(() { + ServiceProfile.setActive(ServiceProfile.parcel); + SharedPreferences.setMockInitialValues({'userid': 38, 'authtoken': 't'}); + sent = []; + bodies = []; + MilerApi.client = MockClient((req) async { + sent.add('${req.method} ${req.url.path}'); + bodies.add(req.body); + return http.Response( + '{"success":true,"data":{"consignmentstatus":"Inwarded_at_Hub",' + '"inwardedat":"2026-09-16T14:22:10Z","next_action":"handed_to_hub",' + '"already_inwarded":false}}', + 200, + ); + }); + }); + + tearDown(() { + MilerApi.client = http.Client(); + Geofence.useDevice(); + ServiceProfile.setActive(ServiceProfile.parcel); + }); + + group('the leg is read from the row', () { + test('a base-routed row resolves to a base leg and names its base', () { + final row = hubRow(); + expect(NextLegResolver.resolve(row).leg, NextLeg.hub); + + final base = handoverBaseFor(row); + expect(base, isNotNull); + expect(base!.name, 'Coimbatore Base'); + expect(base.isNavigable, isTrue); + }); + + test('a customer-routed row is not a base leg and has no base', () { + final row = ApiConfig.pickupFromBooking({ + 'bookingid': 4822, + 'orderid': 'ORDER-CX-1', + 'consignmentstatus': 'Out_for_Delivery', + 'next_action': 'deliver', + }); + expect(NextLegResolver.resolve(row).leg, NextLeg.customer); + expect(handoverBaseFor(row), isNull); + }); + }); + + group('the fence guards the handover like every other presence claim', () { + test('250 m from the base — refused, and NOTHING is posted', () async { + Geofence.positionProvider = + () async => GeofenceFix(position: riderAt(250)); + + final ok = await handOverAtHub(hubRow()); + + expect(ok, isFalse); + expect(sent, isEmpty, reason: 'a refused handover must not reach inward-at-hub'); + expect(lastHandoverFailure, contains('Move within 100 m')); + expect(lastHandoverFailure, contains('handed over')); + }); + + test('GPS off — refused, and nothing is posted', () async { + Geofence.positionProvider = + () async => const GeofenceFix(failure: GeofenceOutcome.serviceDisabled); + + expect(await handOverAtHub(hubRow()), isFalse); + expect(sent, isEmpty); + expect(lastHandoverFailure, contains('Location is switched off')); + }); + + test('a base with no coordinates is refused, not waved through', () async { + // The old doctrine allowed a target-less stop through on the argument + // that the rider cannot fix the hub's data from a doorstep. True — and it + // made the fence optional for exactly the stops nobody could audit. + Geofence.positionProvider = () async => GeofenceFix(position: riderAt(0)); + + final row = hubRow(); + row['next_hub'] = const {'id': 2, 'name': 'Salem Base'}; + + expect(await handOverAtHub(row), isFalse); + expect(sent, isEmpty); + expect(lastHandoverFailure, contains('office')); + }); + }); + + group('inside the fence, the leg actually closes', () { + setUp(() { + Geofence.positionProvider = () async => GeofenceFix(position: riderAt(30)); + }); + + test('it posts inward-at-hub, with the base and the judged position', + () async { + final ok = await handOverAtHub(hubRow()); + + expect(ok, isTrue); + expect(sent, hasLength(1)); + expect(sent.single, contains('/miler/consignments/77/inward-at-hub')); + + // The body the contract asks for: the base it is reconciled against, and + // the coordinates stamped onto the history row as evidence of where the + // hand-over happened. + expect(bodies.single, contains('"hub_id":"1"')); + expect(bodies.single, contains('"latitude"')); + expect(bodies.single, contains('"longitude"')); + expect(lastHandoverFailure, isNull); + }); + + test('the position posted is the one the fence judged', () async { + final fix = riderAt(30); + Geofence.positionProvider = () async => GeofenceFix(position: fix); + + await handOverAtHub(hubRow()); + + // Not a second GPS read. Two fixes seconds apart are two different + // answers, and the hub should be told the one the app allowed on. + expect(bodies.single, contains(fix.latitude.toString())); + }); + + test('a parcel already inwarded is a success, not a failure', () async { + // He handed it over, the reply was lost, he pressed again at a loading + // bay with one bar of signal. The parcel is on the base's counter and he + // cannot prove that from where he stands. + MilerApi.client = MockClient((req) async { + sent.add('${req.method} ${req.url.path}'); + return http.Response( + '{"success":true,"data":{"consignmentstatus":"Inwarded_at_Hub",' + '"already_inwarded":true}}', + 200, + ); + }); + + expect(await handOverAtHub(hubRow()), isTrue); + }); + + test('a 200 that names no state is NOT treated as a handover', () async { + // The rule `reached` taught this app the hard way: a bare success is not + // a transition. A rider must not walk away from a parcel on the strength + // of one. + MilerApi.client = MockClient((req) async { + sent.add('${req.method} ${req.url.path}'); + return http.Response('{"success":true}', 200); + }); + + expect(await handOverAtHub(hubRow()), isFalse); + expect(lastHandoverFailure, contains('did not confirm')); + expect(lastHandoverFailure, contains('before you leave the parcel')); + }); + + test('a server refusal is passed through in the server\'s own words', + () async { + MilerApi.client = MockClient((req) async { + sent.add('${req.method} ${req.url.path}'); + return http.Response( + '{"success":false,"code":"INVALID_STATE",' + '"message":"already out for delivery"}', + 400, + ); + }); + + expect(await handOverAtHub(hubRow()), isFalse); + expect(lastHandoverFailure, contains('already out for delivery')); + }); + }); + + test('a customer leg is never posted to inward-at-hub', () async { + Geofence.positionProvider = () async => GeofenceFix(position: riderAt(0)); + + final row = ApiConfig.pickupFromBooking({ + 'bookingid': 4822, + 'orderid': 'ORDER-CX-1', + 'consignmentstatus': 'Out_for_Delivery', + 'consignmentid': 78, + 'next_action': 'deliver', + }); + + expect(await handOverAtHub(row), isFalse); + expect(sent, isEmpty); + expect(lastHandoverFailure, contains('not going to a base')); + }); + + test('with the server flag OFF the rider never reaches this rung', () { + // `pickup-complete` inwards the parcel itself and the row comes back + // `Inwarded_at_Hub` / `handed_to_hub`, which is [NextLeg.closed]. The sheet + // draws no base CTA and this function is never called. The app reads the + // row and never mirrors the flag — that is what makes one build correct in + // both positions of it. + final row = ApiConfig.pickupFromBooking({ + 'bookingid': 4821, + 'status': 'Converted_To_Consignment', + 'consignmentstatus': 'Inwarded_at_Hub', + 'next_action': 'handed_to_hub', + }); + expect(NextLegResolver.resolve(row).leg, NextLeg.closed); + }); +} diff --git a/test/logout_clears_session_test.dart b/test/logout_clears_session_test.dart new file mode 100644 index 0000000..80d7c5e --- /dev/null +++ b/test/logout_clears_session_test.dart @@ -0,0 +1,185 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:miler/data/api_config.dart'; +import 'package:miler/data/mutation_guard.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// LOGGING OUT ENDS THE SESSION, NOT JUST THE ROUTE +/// +/// The logout handler cleared the scoped stores and the doorstep photos, wrote +/// `logged_out = true`, and sent the rider to the sign-in screen. It did **not** +/// clear the bearer token. So the app *looked* signed out — the flag kept him +/// on the sign-in screen — while `authtoken` sat in SharedPreferences, still +/// valid, until the next `verifyPinWithServer` happened to overwrite it. +/// +/// Two consequences, and the second is the one that matters on a shared +/// handset: +/// +/// * anything that reads the token without consulting the flag — a background +/// isolate, the notification handler, a heartbeat that outlives the route +/// change — could keep making authenticated calls as the rider who left; +/// * a phone handed to the next rider carried the previous one's credential on +/// disk. +/// +/// Two tests, because the bug had two halves: the credential lifecycle (does +/// clearing actually clear?) and the call site (does logout actually call it?). +/// The first would have passed throughout — `clearToken` was never broken, +/// nobody called it — which is exactly why the second one reads the source. +/// ───────────────────────────────────────────────────────────────────────── +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('the credential lifecycle', () { + test('a cleared token does not come back', () async { + SharedPreferences.setMockInitialValues({'authtoken': 'rider-38-session'}); + expect(await ApiConfig.getToken(), 'rider-38-session'); + + await ApiConfig.clearToken(); + + expect( + await ApiConfig.getToken(), + isNull, + reason: 'the next rider on this handset must start with no session', + ); + }); + + test('an empty token reads as no token, not as a session', () async { + // `getToken` returns null for '' deliberately: a blank string is truthy + // enough to pass a `!= null` check, and that is how a signed-out app ends + // up sending `Authorization: Bearer `. + SharedPreferences.setMockInitialValues({'authtoken': ''}); + expect(await ApiConfig.getToken(), isNull); + }); + + test('clearing an already-clear session is not an error', () async { + // Logout runs it unconditionally, including on a handset that never + // signed in — a crash there would strand the rider on the account screen. + SharedPreferences.setMockInitialValues({}); + await ApiConfig.clearToken(); + expect(await ApiConfig.getToken(), isNull); + }); + + test('the in-flight guard does not survive the session', () async { + // A mutation still held when the rider signs out would have its key + // dropped for the *next* session, silently refusing his first press. + await MutationGuard.run('accept:1042', () async { + expect(MutationGuard.isBusy('accept:1042'), isTrue); + return true; + }); + MutationGuard.reset(); + expect(MutationGuard.isBusy('accept:1042'), isFalse); + }); + }); + + group('the teardown itself', () { + // Read from source. The steps are a sequence of calls with no return value + // to assert on, and the regression class is a *missing call* — the cheapest + // honest guard against a missing call is to look for it. + // + // These used to read Profilepage.dart. The teardown moved into + // `endSession` when the 401 path needed to perform exactly the same steps; + // the assertions followed it rather than being deleted. + final source = File('lib/data/session.dart').readAsStringSync(); + + test('it clears the bearer token', () { + expect( + source, + contains('ApiConfig.clearToken()'), + reason: + 'Ending a session must end the credential, not only the route. ' + 'Without this the token outlives the rider on a shared handset.', + ); + }); + + test('it clears the scoped stores and the doorstep photos', () { + expect(source, contains('clearScopedStores()')); + expect(source, contains('ProofStore.clearScope()')); + }); + + test('it raises the flag the launch path reads', () { + // `app_bootstrap` and `main` both branch on this to decide the start + // screen. Clearing the token without it would send a rider to the + // dashboard with no credential. + expect(source, contains("setBool('logged_out', true)")); + }); + + test('it frees any mutation still holding a key', () { + expect(source, contains('MutationGuard.reset()')); + }); + + test('it drops the cached GPS fix', () { + // [Geofence] reuses one live fix across a batch. A position is a fact + // about a rider; the next person on this handset must not have his first + // proximity check measured from where the last one was standing. + expect(source, contains('Geofence.resetCache()')); + }); + }); + + group('both ways out of a session use it', () { + // ── The half that was missing entirely ── + // + // A session ends two ways: the rider presses Log out, or the server stops + // accepting his token. Only the first was implemented. + // + // `MilerApi` drops the token on any 401 and calls `onUnauthorized` — and + // NOTHING EVER ASSIGNED IT. Three references in the whole codebase, all + // inside miler_api.dart, zero assignments. So the credential was deleted + // and nothing else was: the profile stayed on disk, `logged_out` stayed + // false, and the app kept drawing a signed-in dashboard whose every call + // came back `401 authorization header is required`. + // + // Found on a real handset — rider 23, name and tenant on screen, no + // `authtoken` in SharedPreferences, Home / Deliveries / Activity and the + // heartbeat all failing in a loop. The rider sees no jobs and concludes + // there is no work today; nothing anywhere tells him to sign in again. + + test('the manual Log out delegates to the shared teardown', () { + final source = File( + 'lib/views/Dashboard/profile/Profilepage.dart', + ).readAsStringSync(); + expect( + source, + contains('endSession()'), + reason: 'Log out must use the same teardown the 401 path uses, or the ' + 'two drift and one of them stops clearing something.', + ); + }); + + test('the 401 handler is actually assigned', () { + // The assertion that would have caught the shipped bug. `onUnauthorized` + // being declared and called proves nothing — it was both, and null. + final source = File('lib/main.dart').readAsStringSync(); + expect( + source, + contains('MilerApi.onUnauthorized ='), + reason: + 'MilerApi calls onUnauthorized on every 401. Left unassigned, a ' + 'rider whose token expires is dropped into a session that looks ' + 'signed in and can never make a successful call again.', + ); + }); + + test('the 401 handler ends the session and opens sign-in', () { + final source = File('lib/main.dart').readAsStringSync(); + expect(source, contains('endSession()')); + expect( + source, + contains('SignIn()'), + reason: 'clearing the credential without moving the rider leaves him ' + 'on a dashboard that cannot load anything', + ); + }); + + test('a burst of 401s tears down once, not once per call', () { + // A 401 rarely arrives alone: the home poll, the deliveries queue and the + // heartbeat can each get one within the same second. Without a latch the + // rider is pushed at the sign-in screen three times, and GetX stacks + // three routes he then has to dismiss. + final source = File('lib/main.dart').readAsStringSync(); + expect(source, contains('_signingOut')); + }); + }); +} diff --git a/test/miler_contract_regression_test.dart b/test/miler_contract_regression_test.dart index 0fedcd1..60f23e1 100644 --- a/test/miler_contract_regression_test.dart +++ b/test/miler_contract_regression_test.dart @@ -527,16 +527,22 @@ void main() { }, ); - test('the geofence bypass does not stop the write leaving', () async { - // The suspicion this rules out. `_checkGeofence` returns true early when - // enforcement is off and its success branch contains nothing else — so - // the bypass skips the distance *rejection* and nothing more. + test('the provider itself holds no proximity gate', () async { + // ── What this test used to say ── // - // Asserted through the provider rather than by reading the flag, because - // "the request went out with the fence off" is the claim, and the flag is - // only evidence for it. - expect(kBypassGeofenceForTesting, isTrue); - + // `expect(kBypassGeofenceForTesting, isTrue)` — it asserted the fence was + // OFF, and then that the write still went out. The flag is gone: the + // fence is enforced by default now and lives in `lib/data/geofence.dart`. + // + // The claim underneath it survives and is worth keeping: **the fence is + // in the controller, not in the transport.** `UpdatePickupProvider` maps + // a legacy payload onto a v1 route and posts it, full stop. If a + // proximity check ever appears down here there would be two fences again, + // reachable by different paths, which is the shape that gave the app a + // 500 m radius on Home and a 10 m one on a stop. + // + // The controller-level rule — blocked means no request — is asserted + // end-to-end in `geofence_test.dart`. await UpdatePickupProvider().updatePickup({ 'pickupid': 4211, 'orderstatus': 'arrived', @@ -544,7 +550,7 @@ void main() { 'riderslon': '76.955800', }); - expect(sent, isNotEmpty, reason: 'the bypass swallowed the request'); + expect(sent, isNotEmpty, reason: 'the provider swallowed the request'); expect(only('reached').url.path, contains('/4211/reached')); }); }); diff --git a/test/mixed_work_policy_test.dart b/test/mixed_work_policy_test.dart new file mode 100644 index 0000000..723c3cd --- /dev/null +++ b/test/mixed_work_policy_test.dart @@ -0,0 +1,319 @@ +import 'package:flutter_test/flutter_test.dart'; + +import 'package:miler/data/api_config.dart'; +import 'package:miler/data/service_profile.dart'; +import 'package:miler/data/task_profile.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// ONE MILER, MIXED WORK — asked of the job, not of the rider +/// +/// The scenario this exists for, stated as the operation states it: +/// +/// A rider in RS Puram is assigned **10 kitchen parcels and 5 CX customer +/// pickups**. All fifteen are his, in one shift, on one route. +/// +/// Before this, every operational question was answered by +/// `ServiceProfile.active` — a static resolved once from the rider's tenant, +/// giving the *same answer for all fifteen rows*. So on a meal tenant the five +/// CX pickups were walked straight past the door flow (no parcel count, no +/// destination, no weight, no payment) and pivoted on whatever the customer had +/// typed into the app days earlier. On a parcel tenant the reverse: ten kitchen +/// bags with a known manifest asked the rider to itemise each one. +/// +/// [WorkPolicy] is the bridge. It asks the **row**, and falls back to the line +/// only where the row is silent — because most rows still are (see BE-6), and a +/// screen that took `unknown` at face value would lose the milk round its +/// kitchen headings, which is what happened the two previous times a call site +/// was migrated wholesale. +/// +/// These tests pin both halves: the row wins when it speaks, and nothing +/// changes when it does not. +/// ───────────────────────────────────────────────────────────────────────── +void main() { + setUp(() => ServiceProfile.setActive(ServiceProfile.parcel)); + tearDown(() => ServiceProfile.setActive(ServiceProfile.parcel)); + + /// A CX customer pickup, as the queue returns it. + Map cxPickup() => ApiConfig.pickupFromBooking(const { + 'bookingid': 101, + 'orderid': 'CX-1', + 'pickup_source_type': 'customer', + 'pickup_source_name': 'Anitha R', + }); + + /// A kitchen / counter collection. + Map sourcePickup() => ApiConfig.pickupFromBooking(const { + 'bookingid': 201, + 'orderid': 'KIT-1', + 'pickup_source_type': 'merchant', + 'pickup_source_name': 'Sri Balaji Kitchen', + }); + + /// A row from before these fields existed — the common case today. + Map silentRow() => ApiConfig.pickupFromBooking(const { + 'bookingid': 301, + 'orderid': 'OLD-1', + }); + + group('the row decides, when the row has said something', () { + test('a CX customer pickup captures shipment details', () { + // The five pickups in the scenario. True on BOTH tenants, which is the + // whole change: it is a fact about the job, not about the rider. + for (final line in [ServiceProfile.parcel, ServiceProfile.milkMan]) { + ServiceProfile.setActive(line); + expect( + WorkPolicy.capturesShipmentDetails(cxPickup()), + isTrue, + reason: 'a customer door needs the verify flow on ${line.label}', + ); + } + }); + + test('a kitchen collection does not', () { + // The ten parcels. Also true on both tenants — the manifest is known + // before the rider arrives, whoever he works for. + for (final line in [ServiceProfile.parcel, ServiceProfile.milkMan]) { + ServiceProfile.setActive(line); + expect( + WorkPolicy.capturesShipmentDetails(sourcePickup()), + isFalse, + reason: 'a counter manifest is not itemised at the door', + ); + } + }); + + test('the two coexist in one queue, answering differently', () { + // The regression in one line: fifteen rows, two workflows, one rider. + final queue = [cxPickup(), sourcePickup()]; + expect( + queue.map(WorkPolicy.capturesShipmentDetails).toList(), + [true, false], + reason: + 'this list was [true, true] or [false, false] before — whichever ' + 'the tenant happened to say', + ); + }); + + test('a CX pickup never becomes a source/kitchen task', () { + final profile = TaskProfile.of(cxPickup()); + expect(profile.kind, WorkKind.customerPickup); + expect( + profile.groupsBySource, + isFalse, + reason: 'a customer door is not a counter and must not be grouped ' + 'under one like a kitchen manifest', + ); + }); + }); + + group('the line still decides where the row is silent', () { + // The guard that makes this safe to ship: every existing row and every + // existing fixture takes this path, and must behave exactly as before. + test('a silent row follows the parcel line', () { + ServiceProfile.setActive(ServiceProfile.parcel); + expect( + WorkPolicy.capturesShipmentDetails(silentRow()), + ServiceProfile.parcel.needsVerification, + ); + expect( + WorkPolicy.capturesShipmentAddresses(silentRow()), + ServiceProfile.parcel.capturesShipmentAddresses, + ); + expect( + WorkPolicy.initiatesShipment(silentRow()), + ServiceProfile.parcel.initiatesShipment, + ); + }); + + test('a silent row follows the meal line', () { + ServiceProfile.setActive(ServiceProfile.milkMan); + expect( + WorkPolicy.capturesShipmentDetails(silentRow()), + ServiceProfile.milkMan.needsVerification, + ); + expect( + WorkPolicy.deliversToCustomer(silentRow()), + ServiceProfile.milkMan.deliversToCustomer, + ); + }); + }); + + group('when an accepted stop leaves Home', () { + test('a CX pickup leaves on accept; a kitchen load does not', () { + // The per-item replacement for `handsOffAtCollection`. Accepting a + // customer visit is an appointment the rider now owes somebody, so it + // belongs on Bookings. Accepting a counter load puts nothing in his + // hands — he still has to go and collect it. + expect(WorkPolicy.staysOnHomeUntilCollected(cxPickup()), isFalse); + expect(WorkPolicy.staysOnHomeUntilCollected(sourcePickup()), isTrue); + }); + + test('and the answer does not change with the tenant', () { + // The whole failure this replaces: one answer for every row on the + // screen. On a meal tenant an accepted CX pickup sat on Home beside ten + // kitchen bags with nothing to tell them apart. + for (final line in [ServiceProfile.parcel, ServiceProfile.milkMan]) { + ServiceProfile.setActive(line); + expect( + WorkPolicy.staysOnHomeUntilCollected(cxPickup()), + isFalse, + reason: line.label, + ); + expect( + WorkPolicy.staysOnHomeUntilCollected(sourcePickup()), + isTrue, + reason: line.label, + ); + } + }); + + test('a silent row still follows the line', () { + for (final line in [ServiceProfile.parcel, ServiceProfile.milkMan]) { + ServiceProfile.setActive(line); + expect( + WorkPolicy.staysOnHomeUntilCollected(silentRow()), + line.handsOffAtCollection, + reason: 'existing payloads must behave exactly as before', + ); + } + }); + }); + + group('which surface an item belongs on', () { + test('an accepted CX pickup moves Home → Bookings', () { + // The rule the scenario names: accepting a customer visit makes it an + // appointment the rider owes somebody. + final row = cxPickup(); + expect(WorkPolicy.surfaceOf(row, accepted: false), WorkSurface.home); + expect(WorkPolicy.surfaceOf(row, accepted: true), WorkSurface.bookings); + }); + + test('an accepted kitchen load stays on Home', () { + // Accepting a counter load puts nothing in his hands — he still has to go + // and collect it. + final row = sourcePickup(); + expect(WorkPolicy.surfaceOf(row, accepted: false), WorkSurface.home); + expect( + WorkPolicy.surfaceOf(row, accepted: true), + WorkSurface.home, + reason: 'nothing is in his hands yet', + ); + }); + + test('in-custody work is Deliveries, whichever kind it is', () { + for (final action in const ['deliver', 'start_delivery', 'inward_at_hub']) { + final row = ApiConfig.pickupFromBooking({ + 'bookingid': 401, + 'next_action': action, + }); + expect( + WorkPolicy.surfaceOf(row, accepted: true), + WorkSurface.deliveries, + reason: action, + ); + } + }); + + test('finished work is Activity', () { + final row = ApiConfig.pickupFromBooking(const { + 'bookingid': 501, + 'next_action': 'handed_to_hub', + }); + expect(WorkPolicy.surfaceOf(row, accepted: true), WorkSurface.activity); + }); + }); + + group('a base leg is read, never guessed', () { + test('only an explicit inward_at_hub ends at a base', () { + expect( + WorkPolicy.endsAtHub( + ApiConfig.pickupFromBooking(const { + 'bookingid': 601, + 'next_action': 'inward_at_hub', + }), + ), + isTrue, + ); + }); + + test('a silent row does NOT fall back to the line here', () { + // Deliberately the one method with no fallback. Guessing a base leg from + // the rider's tenant is how a hyperlocal parcel gets routed to a counter + // and sits there. + ServiceProfile.setActive(ServiceProfile.parcel); + expect(ServiceProfile.parcel.endsAtHub, isTrue); + expect( + WorkPolicy.endsAtHub(silentRow()), + isFalse, + reason: 'the line says the day ends at a base; this ROW does not', + ); + }); + + test('a customer delivery does not end at a base', () { + expect( + WorkPolicy.endsAtHub( + ApiConfig.pickupFromBooking(const { + 'bookingid': 701, + 'next_action': 'deliver', + }), + ), + isFalse, + ); + }); + }); + + group('the RS Puram round', () { + test('15 assigned items keep 15 distinct answers, and none vanish', () { + ServiceProfile.setActive(ServiceProfile.milkMan); + + final round = >[ + for (var i = 0; i < 10; i++) + ApiConfig.pickupFromBooking({ + 'bookingid': 1000 + i, + 'orderid': 'KIT-$i', + 'pickup_source_type': 'merchant', + 'pickup_source_name': 'Kitchen 2', + }), + for (var i = 0; i < 5; i++) + ApiConfig.pickupFromBooking({ + 'bookingid': 2000 + i, + 'orderid': 'CX-$i', + 'pickup_source_type': 'customer', + 'pickup_source_name': 'Customer $i', + }), + ]; + + expect(round, hasLength(15), reason: 'nothing may be filtered away'); + + final kitchens = round.where( + (r) => TaskProfile.of(r).kind == WorkKind.sourcePickup, + ); + final customers = round.where( + (r) => TaskProfile.of(r).kind == WorkKind.customerPickup, + ); + expect(kitchens, hasLength(10)); + expect(customers, hasLength(5)); + + // Accepting all fifteen sends the two kinds to two different surfaces. + expect( + kitchens.map((r) => WorkPolicy.surfaceOf(r, accepted: true)).toSet(), + {WorkSurface.home}, + ); + expect( + customers.map((r) => WorkPolicy.surfaceOf(r, accepted: true)).toSet(), + {WorkSurface.bookings}, + ); + + // And only the five customer doors run the verify flow — on a meal + // tenant, where previously none of them would have. + expect( + customers.every(WorkPolicy.capturesShipmentDetails), + isTrue, + ); + expect( + kitchens.any(WorkPolicy.capturesShipmentDetails), + isFalse, + ); + }); + }); +} diff --git a/test/multi_destination_stop_test.dart b/test/multi_destination_stop_test.dart new file mode 100644 index 0000000..24749fb --- /dev/null +++ b/test/multi_destination_stop_test.dart @@ -0,0 +1,193 @@ +import 'package:flutter_test/flutter_test.dart'; + +import 'package:miler/data/api_config.dart'; +import 'package:miler/data/milk_run.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// ONE PICKUP, SEVERAL DOORS +/// +/// A customer-app booking carries N destinations. `GET /miler/bookings` +/// returns ONE ROW PER DESTINATION once the visit has been collected — same +/// `bookingid`, same `bookingreference`, different door, distinguished only by +/// `destinationseq` / `destinationcount`. +/// +/// Everything the device remembers about a stop is keyed on `orderid`: the +/// accepted store dedupes on it (`byId[id] = copy`), the consignment-id map +/// files under it, the collected and out-for-delivery sets hold it. So three +/// doors sharing one `orderid` was not a display bug — two of the three rows +/// were deduped away before any screen saw them, and the two consignment ids +/// that lost the race were unrecoverable. A rider holding three bags could +/// deliver one. +/// +/// These tests pin the two halves of the fix that can silently rot: +/// +/// 1. The **suffix exists only where there is something to tell apart**, so +/// every single-destination booking — which is all of logistics and all of +/// the milk run — keys exactly as it did before. +/// 2. **Collection is per visit, delivery is per door.** The collected set is +/// written once, under the booking key, while the pickup is still a single +/// pre-pickup row; the rows that replace it each carry their own key and +/// must still read as collected. +/// ───────────────────────────────────────────────────────────────────────── +void main() { + /// One row of `GET /miler/bookings` for a three-destination customer pickup, + /// after `pickup-complete` has fanned it out. + Map destinationRow(int seq, {int count = 3}) => { + 'bookingid': 482913, + 'bookingreference': 'DM-482913', + 'status': 'Converted_To_Consignment', + 'destinationseq': seq, + 'destinationcount': count, + 'consignmentid': 500 + seq, + 'trackingno': 'DM-TRK-884102$seq', + 'recipientname': ['Anitha R', 'Suresh K', 'Meena P'][seq], + 'recipientphone': '987654321$seq', + 'deliveryaddress': ['Chennai', 'Ernakulam', 'Bengaluru'][seq], + 'customername': 'Ravi Kumar', // the SENDER — not at any of the three doors + 'pickupaddress': 'Gandhipuram, Coimbatore', + }; + + group('the three doors are three stops', () { + test('each destination gets its own key', () { + final keys = [ + for (var seq = 0; seq < 3; seq++) + MilkRun.idOf(ApiConfig.pickupFromBooking(destinationRow(seq))), + ]; + + expect(keys.toSet(), hasLength(3), reason: 'three doors, three keys'); + expect(keys, ['DM-482913#0', 'DM-482913#1', 'DM-482913#2']); + }); + + test('a store keyed by orderid keeps all three', () { + // Exactly what addAcceptedBookings does: `byId[orderid] = row`. + final byId = >{}; + for (var seq = 0; seq < 3; seq++) { + final stop = ApiConfig.pickupFromBooking(destinationRow(seq)); + byId[MilkRun.idOf(stop)] = stop; + } + expect(byId, hasLength(3), reason: 'none of the three deduped away'); + }); + + test('each keeps its own consignment id', () { + // The map rememberConsignmentId writes into. One slot per door, so no + // door can lose its parcel to whichever wrote last. + final consignments = {}; + for (var seq = 0; seq < 3; seq++) { + final stop = ApiConfig.pickupFromBooking(destinationRow(seq)); + consignments[MilkRun.idOf(stop)] = MilkRun.consignmentIdOf(stop); + } + expect(consignments.values.toSet(), {'500', '501', '502'}); + }); + + test('the booking key is recoverable from any of them', () { + for (var seq = 0; seq < 3; seq++) { + final stop = ApiConfig.pickupFromBooking(destinationRow(seq)); + expect(MilkRun.bookingKeyOf(stop), 'DM-482913'); + expect(stop['bookingreference'], 'DM-482913'); + } + }); + }); + + group('the suffix appears only where it is needed', () { + test('a single-destination booking keys exactly as before', () { + final stop = ApiConfig.pickupFromBooking({ + 'bookingid': 77, + 'bookingreference': 'DM-000077', + 'status': 'Pending_Pickup', + 'destinationseq': 0, + 'destinationcount': 1, + }); + expect(MilkRun.idOf(stop), 'DM-000077'); + expect(MilkRun.bookingKeyOf(stop), 'DM-000077'); + }); + + test('a booking with no destination fields at all is untouched', () { + // Every console booking, and any backend that has not shipped the + // destination fields yet. The absence must cost nothing. + final stop = ApiConfig.pickupFromBooking({ + 'bookingid': 42, + 'bookingreference': 'DM-000042', + 'status': 'Pending_Pickup', + }); + expect(MilkRun.idOf(stop), 'DM-000042'); + expect(MilkRun.destinationCountOf(stop), 1); + expect(MilkRun.stopLabel(stop), isEmpty); + }); + }); + + group('the rider is told which door this is', () { + test('Stop N of M, counting from one', () { + final stop = ApiConfig.pickupFromBooking(destinationRow(1)); + expect(MilkRun.stopLabel(stop), 'Stop 2 of 3'); + }); + + test('says nothing when there is only one door', () { + final stop = ApiConfig.pickupFromBooking(destinationRow(0, count: 1)); + expect( + MilkRun.stopLabel(stop), + isEmpty, + reason: 'a label on every card in the app stops being read', + ); + }); + + test('the receiver has a number of their own to ring', () { + // The card dials `recipientphone` on a delivery leg, not the booking's + // `pickupcontactno` — that one is the SENDER, and ringing him from the + // receiver's gate is what it used to do at all three gates. + final stop = ApiConfig.pickupFromBooking(destinationRow(0)); + expect(stop['recipientphone'], '9876543210'); + expect( + stop['pickupcontactno'], + isNot(stop['recipientphone']), + reason: 'sender and receiver are different people', + ); + }); + + test('the receiver and the tracking number survive the adapter', () { + // The adapter builds a fixed map, so a field it does not name is a field + // the UI can never see. These three were unnamed until now, which put the + // SENDER's name over the receiver's door. + final stop = ApiConfig.pickupFromBooking(destinationRow(2)); + expect(stop['recipientname'], 'Meena P'); + expect(stop['recipientphone'], '987654321 2'.replaceAll(' ', '')); + expect(stop['trackingno'], 'DM-TRK-8841022'); + expect( + stop['pickupcustomer'], + 'Ravi Kumar', + reason: 'the sender is still carried, just no longer as the drop', + ); + }); + }); + + group('collection is per visit, delivery is per door', () { + test('a collected visit keeps its doors collected after the split', () { + // The rider collected the pickup while it was still ONE row, so this is + // what the collected set holds. Nothing in it mentions a destination. + final collected = {'DM-482913'}; + + for (var seq = 0; seq < 3; seq++) { + final stop = ApiConfig.pickupFromBooking(destinationRow(seq)); + expect( + MilkRun.wasCollected(stop, collected), + isTrue, + reason: + 'door $seq must not drop back to "not collected" at the moment ' + 'the rider actually has the bag in his hands', + ); + } + }); + + test('a door the rider has not collected still reads as uncollected', () { + final stop = ApiConfig.pickupFromBooking(destinationRow(0)); + expect(MilkRun.wasCollected(stop, {'DM-999999'}), isFalse); + expect(MilkRun.wasCollected(stop, const {}), isFalse); + }); + + test('a per-door key in the set also counts', () { + // Once the delivery half starts writing per-door keys, both spellings + // have to answer — the set can hold either during a single shift. + final stop = ApiConfig.pickupFromBooking(destinationRow(1)); + expect(MilkRun.wasCollected(stop, {'DM-482913#1'}), isTrue); + }); + }); +} diff --git a/test/multidrop_shot_test.dart b/test/multidrop_shot_test.dart new file mode 100644 index 0000000..b8dc487 --- /dev/null +++ b/test/multidrop_shot_test.dart @@ -0,0 +1,170 @@ +@Tags(['shots']) +library; + +import 'dart:io'; + +import 'package:flutter/material.dart'; +import 'package:flutter_screenutil/flutter_screenutil.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:get/get.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'support/golden_fonts.dart'; + +import 'package:miler/data/api_config.dart'; +import 'package:miler/data/mock_backend.dart'; +import 'package:miler/data/service_profile.dart'; +import 'package:miler/views/Dashboard/pickups/pickups.dart'; +import 'package:miler/views/helpers/constants/Colorconstants.dart'; + +/// A look at the three-door card, rendered from the SAME fixture the running +/// app serves — `MockBackend.multiDropVisit` — through the SAME adapter the +/// network path uses. Nothing here hand-builds a stop map, so what is on screen +/// is what a rider sees when the backend sends a fanned-out customer pickup. +/// +/// Writes PNGs to `test/shots_out/` rather than comparing goldens: the point is +/// to look at it, not to pin it. +void main() { + setUpAll(() async { + await loadGoldenFonts(); + Directory('test/shots_out').createSync(recursive: true); + }); + + tearDown(() { + Get.reset(); + ServiceProfile.setActive(ServiceProfile.parcel); + }); + + Future shoot( + WidgetTester tester, + String name, + Widget child, { + double height = 340, + double width = 390, + }) async { + SharedPreferences.setMockInitialValues({'userid': 1}); + tester.view.physicalSize = Size(width * 3, height * 3); + tester.view.devicePixelRatio = 3.0; + addTearDown(tester.view.reset); + + await tester.pumpWidget( + ScreenUtilInit( + designSize: Size(width, height), + builder: (_, _) => MaterialApp( + debugShowCheckedModeBanner: false, + home: Scaffold( + backgroundColor: ColorConstants.daylightSurface, + body: SingleChildScrollView( + child: Padding( + padding: const EdgeInsets.symmetric(vertical: 12), + child: child, + ), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + await expectLater( + find.byType(MaterialApp), + matchesGoldenFile('shots_out/$name.png'), + ); + } + + /// The fixture the app serves, put through the real adapter. + List> stops() => [ + for (final row in MockBackend.multiDropVisit) + ApiConfig.pickupFromBooking(Map.from(row)), + ]; + + testWidgets('door 1 of 3 — Chennai, two parcels', (tester) async { + ServiceProfile.setActive(ServiceProfile.milkMan); + final all = stops(); + await shoot( + tester, + 'multidrop_stop1', + PickupCard( + item: all[0], + displayStep: 1, + distanceMeters: 1400, + collectedIds: {all[0]['orderid'] as String}, + outForDeliveryIds: {all[0]['orderid'] as String}, + ), + ); + }); + + testWidgets('door 2 of 3 — Ernakulam', (tester) async { + ServiceProfile.setActive(ServiceProfile.milkMan); + final all = stops(); + await shoot( + tester, + 'multidrop_stop2', + PickupCard( + item: all[1], + displayStep: 2, + distanceMeters: 3100, + collectedIds: {all[1]['orderid'] as String}, + outForDeliveryIds: {all[1]['orderid'] as String}, + ), + ); + }); + + testWidgets('the three stacked, as the queue shows them', (tester) async { + ServiceProfile.setActive(ServiceProfile.milkMan); + final all = stops(); + await shoot( + tester, + 'multidrop_queue', + Column( + children: [ + for (var i = 0; i < all.length; i++) + Padding( + padding: const EdgeInsets.only(bottom: 10), + child: PickupCard( + item: all[i], + displayStep: i + 1, + distanceMeters: 1400.0 + i * 900, + collectedIds: {all[i]['orderid'] as String}, + outForDeliveryIds: {all[i]['orderid'] as String}, + ), + ), + ], + ), + height: 960, + ); + }); + + testWidgets('a single-destination booking is unchanged', (tester) async { + ServiceProfile.setActive(ServiceProfile.milkMan); + final one = ApiConfig.pickupFromBooking({ + 'bookingid': 900777, + 'bookingreference': 'MOCK-000777', + 'status': 'Converted_To_Consignment', + 'consignmentid': 900777, + 'consignmentstatus': 'Out_for_Delivery', + 'next_action': 'deliver', + 'destinationseq': 0, + 'destinationcount': 1, + 'pickup_source_name': 'Ravi Kumar', + 'pickupaddress': '14, Cross Cut Road, Gandhipuram, Coimbatore 641012', + 'deliveryaddress': '12, SNS Colony, Peelamedu, Coimbatore 641004', + 'customername': 'Ravi Kumar', + 'recipientname': 'Joe Mathew', + 'parcels': [ + {'bookingparcelid': 1, 'itemcategory': 'General'}, + ], + }); + await shoot( + tester, + 'multidrop_single', + PickupCard( + item: one, + displayStep: 1, + distanceMeters: 900, + collectedIds: {one['orderid'] as String}, + outForDeliveryIds: {one['orderid'] as String}, + ), + ); + }); +} diff --git a/test/offline_banner_test.dart b/test/offline_banner_test.dart index a08adec..26383c2 100644 --- a/test/offline_banner_test.dart +++ b/test/offline_banner_test.dart @@ -69,7 +69,7 @@ void main() { expect(tester.getSize(find.byType(OfflineBanner)).height, lessThan(64)); }); - testWidgets('it says what happens next, not just what is wrong', ( + testWidgets('it says what happens next, and promises nothing it cannot do', ( tester, ) async { await pump(tester, offline: true); @@ -83,7 +83,28 @@ void main() { .first .data!; expect(text, contains('Offline')); - expect(text.toLowerCase(), contains('sync')); + + // ── This used to assert the opposite ── + // + // It required the word "sync", and the banner duly said "Changes will sync + // when you're back online". **There is no mutation queue in this app** — no + // outbox, no replay, no retry loop. A status the rider set with no signal + // was attempted once, failed, and was never sent again. The banner was + // promising a mechanism that does not exist, to the one person who would be + // blamed when the hub had no record of the stop. + // + // If a queue is ever built, this assertion is the place to reverse — and + // the copy may promise a sync again on the day something performs one. + expect( + text.toLowerCase(), + isNot(contains('sync')), + reason: 'nothing in this app replays a failed mutation', + ); + expect( + text.toLowerCase(), + contains('connection'), + reason: 'it must still say what he needs, not only that he is offline', + ); }); testWidgets('online, it occupies no space at all', (tester) async { diff --git a/test/parcel_photos_test.dart b/test/parcel_photos_test.dart new file mode 100644 index 0000000..d6f3fa8 --- /dev/null +++ b/test/parcel_photos_test.dart @@ -0,0 +1,213 @@ +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:miler/data/miler_api.dart'; +import 'package:miler/providers/pickuplog/pickuplog_provider.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// THE DOOR PHOTOGRAPH REACHES THE HUB +/// +/// `POST /miler/bookings/:id/parcel` has accepted `photos` for as long as the +/// route has existed. The server's own note says why it is there: *"Weight +/// without a photograph is a number the customer has no way to check, and this +/// is the only point in the flow where anyone is standing next to the parcel."* +/// +/// The app never sent it — and not by oversight in one place, but through three +/// separate defects that each had to be fixed: +/// +/// 1. `ParcelEntry.toJson()` had no `photos` key at all. +/// 2. `uploadProof` kept the public **URL** from the sign response and threw +/// away the **key**, which is what `photos` is specified in. +/// 3. The pickup upload passed a **booking** id in the `consignmentid` slot, +/// so every pickup proof this app ever uploaded was filed under a +/// consignment number that did not exist. +/// +/// Meanwhile `StopVerificationPage` *compels* the photograph — the rider cannot +/// confirm the stop without one. So the app demanded the single piece of +/// evidence that settles a dispute, uploaded it, and dropped the reference. +/// ───────────────────────────────────────────────────────────────────────── +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('ParcelEntry serialisation', () { + test('photos are sent when there are any', () { + final json = const ParcelEntry( + weight: 2.5, + photos: ['pickup_proof/pickup_proof-4821-20260916-101402-a1b2.jpg'], + ).toJson(); + + expect(json['photos'], isA()); + expect( + json['photos'], + ['pickup_proof/pickup_proof-4821-20260916-101402-a1b2.jpg'], + ); + expect(json['weight'], 2.5); + }); + + test('the key is a storage key, never a URL', () { + // The customer is served a short-lived signed link *derived from* the + // key. A URL stored here either expires or, worse, never does. + final json = const ParcelEntry( + weight: 1, + photos: ['pickup_proof/pickup_proof-4821-20260916-101402-a1b2.jpg'], + ).toJson(); + final photo = (json['photos'] as List).single.toString(); + + expect(photo, isNot(startsWith('http'))); + expect(photo, isNot(contains('://'))); + expect(photo, contains('/'), reason: 'a key is {folder}/{file}'); + }); + + test('no photo means the field is OMITTED, not sent empty', () { + // An empty array is a claim that no photograph was taken. A failed + // upload is not that claim, and the difference matters to whoever reads + // the record afterwards. + final json = const ParcelEntry(weight: 1).toJson(); + expect(json.containsKey('photos'), isFalse); + }); + + test('a local filesystem path is never what gets sent', () { + // The shape the bug produced: `parcelImage` held `/data/user/0/…/x.jpg` + // and nothing ever turned it into a reference the server could resolve. + final json = const ParcelEntry(weight: 1).toJson(); + expect(json.toString(), isNot(contains('/data/user/'))); + expect(json.toString(), isNot(contains('.jpg'))); + }); + }); + + group('the parcel request that actually leaves the phone', () { + late List sent; + + setUp(() { + SharedPreferences.setMockInitialValues({'userid': 38, 'authtoken': 't'}); + sent = []; + MilerApi.client = MockClient((req) async { + sent.add(req); + return http.Response('{"success":true,"data":{}}', 200); + }); + }); + + tearDown(() => MilerApi.client = http.Client()); + + Map bodyOf(http.Request r) => + jsonDecode(r.body) as Map; + + test('photos ride on the wire to /parcel', () async { + await MilerApi.submitParcels(4821, const [ + ParcelEntry(weight: 2, photos: ['pickup_proof/p-4821-x.jpg']), + ParcelEntry(weight: 2), + ]); + + expect(sent, hasLength(1)); + expect(sent.single.url.path, endsWith('/miler/bookings/4821/parcel')); + + final parcels = bodyOf(sent.single)['parcels'] as List; + expect(parcels, hasLength(2)); + expect((parcels[0] as Map)['photos'], ['pickup_proof/p-4821-x.jpg']); + expect( + (parcels[1] as Map).containsKey('photos'), + isFalse, + reason: 'one photograph of the load, not one claimed per box', + ); + }); + + test('the sign call keys a PRE-pickup proof on the BOOKING', () async { + // There is no consignment yet — `pickup-complete` has not run. The server + // builds the storage key from consignmentid, then bookingid, then the + // rider; passing a booking id in the consignment slot filed the proof + // under a consignment number that did not exist. + await MilerApi.signUpload( + purpose: MilerApi.proofPickup, + bookingId: 4821, + ); + + final body = bodyOf(sent.single); + expect(body['bookingid'], 4821); + expect( + body.containsKey('consignmentid'), + isFalse, + reason: 'a booking id must never be sent as a consignment id', + ); + expect(body['purpose'], 'pickup_proof'); + }); + + test('a delivery proof still keys on the consignment', () async { + // The other half of the contract, unchanged — `deliver` takes a URL and + // its proof belongs to a consignment that exists by then. + await MilerApi.signUpload( + purpose: MilerApi.proofDelivery, + consignmentId: 77, + ); + + final body = bodyOf(sent.single); + expect(body['consignmentid'], 77); + expect(body.containsKey('bookingid'), isFalse); + }); + }); + + group('the provider turns a verification map into parcels', () { + late List sent; + + setUp(() { + SharedPreferences.setMockInitialValues({'userid': 38, 'authtoken': 't'}); + sent = []; + MilerApi.client = MockClient((req) async { + sent.add(req); + return http.Response('{"success":true,"data":{}}', 200); + }); + }); + + tearDown(() => MilerApi.client = http.Client()); + + test('a key on the verification map reaches the first parcel', () async { + await UpdatePickupProvider().submitParcels(4821, const { + 'pickup': { + 'collected': 3, + 'weight': '6', + 'photos': ['pickup_proof/p-4821-x.jpg'], + }, + }); + + final parcels = + (jsonDecode(sent.single.body) as Map)['parcels'] as List; + expect(parcels, hasLength(3), reason: 'three boxes were collected'); + expect((parcels[0] as Map)['photos'], ['pickup_proof/p-4821-x.jpg']); + expect((parcels[1] as Map).containsKey('photos'), isFalse); + expect((parcels[2] as Map).containsKey('photos'), isFalse); + + // The weight is still split evenly across the three — unchanged. + expect((parcels[0] as Map)['weight'], 2.0); + }); + + test('a failed upload still records the parcels', () async { + // A dead object store must not strand a rider at a door holding parcels + // he cannot record. The pickup goes through; the gap is logged. + await UpdatePickupProvider().submitParcels(4821, const { + 'pickup': {'collected': 1, 'weight': '2'}, + }); + + expect(sent, hasLength(1), reason: 'the parcels were still submitted'); + final parcels = + (jsonDecode(sent.single.body) as Map)['parcels'] as List; + expect((parcels[0] as Map).containsKey('photos'), isFalse); + }); + + test('a blank key is dropped rather than sent', () async { + await UpdatePickupProvider().submitParcels(4821, const { + 'pickup': { + 'collected': 1, + 'weight': '2', + 'photos': ['', ' '], + }, + }); + + final parcels = + (jsonDecode(sent.single.body) as Map)['parcels'] as List; + expect((parcels[0] as Map).containsKey('photos'), isFalse); + }); + }); +} diff --git a/test/set_pin_flow_test.dart b/test/set_pin_flow_test.dart new file mode 100644 index 0000000..5057b53 --- /dev/null +++ b/test/set_pin_flow_test.dart @@ -0,0 +1,386 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:miler/data/api_config.dart'; +import 'package:miler/data/miler_api.dart'; + +/// ───────────────────────────────────────────────────────────────────────── +/// SELF-SET PIN ON FIRST SIGN-IN +/// +/// The console no longer issues a rider a PIN. `POST /miler/login` answers +/// `pin_set`, and that one boolean decides which screen he is owed: +/// +/// pin_set: false → Set-PIN → POST /miler/set-pin → logged in +/// pin_set: true → Enter-PIN → POST /miler/verify-pin → logged in +/// +/// ── What this replaces, and why it was a dead end ── +/// +/// The app read only the **status code** of `/miler/login` and threw the body +/// away. From "an account exists" it inferred Enter-PIN; from "it does not" it +/// inferred an OTP branch that verified nothing — `verifyOtp` returned `true` +/// without checking a digit — and ended at a Create-MPIN screen whose save +/// button called `updatePin`, which had no route behind it and returned a +/// manufactured `403 "Your MPIN is issued by your office and cannot be changed +/// from the app."` +/// +/// So a rider with no PIN could not sign in by any path, and the three seeded +/// test riders (`8000000001`–`3`) were unreachable. +/// +/// ── The rule these tests exist to hold ── +/// +/// **Branch on the boolean, never on the message.** `"PIN verification +/// required"` is prose and prose gets reworded; the day it does, a +/// string-matching client sends every rider to the wrong screen. +/// ───────────────────────────────────────────────────────────────────────── +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late List sent; + + /// Installs a fake server. [handler] answers by path. + void serve(http.Response Function(http.Request req) handler) { + sent = []; + MilerApi.client = MockClient((req) async { + sent.add(req); + return handler(req); + }); + } + + setUp(() => SharedPreferences.setMockInitialValues({})); + tearDown(() => MilerApi.client = http.Client()); + + Map bodyOf(http.Request r) => + jsonDecode(r.body) as Map; + + /// The session envelope `set-pin` and `verify-pin` both answer with. + String sessionBody({int userId = 46}) => jsonEncode({ + 'success': true, + 'token': 'jwt-for-$userId', + 'tenantid': 1, + 'tenantname': 'Doormile Coimbatore Logistics', + 'user': { + 'userid': userId, + 'authname': 'Seed Rider', + 'contactno': '8000000001', + 'profile': { + 'userid': userId, + 'displayname': 'Seed Rider', + 'phone': '8000000001', + 'availabilitystatus': 'Offline', + }, + }, + }); + + group('pin_set decides the screen', () { + test('pin_set:false → the rider must set one', () async { + serve( + (_) => http.Response( + jsonEncode({ + 'success': true, + 'message': 'PIN verification required', + 'phone': '8000000001', + 'pin_set': false, + }), + 200, + ), + ); + + final res = await MilerApi.login('8000000001'); + expect(res.ok, isTrue); + expect(MilerApi.pinSetOf(res), isFalse); + }); + + test('pin_set:true → the existing Enter-PIN path', () async { + serve( + (_) => http.Response( + jsonEncode({ + 'success': true, + 'message': 'PIN verification required', + 'phone': '9000000009', + 'pin_set': true, + }), + 200, + ), + ); + + final res = await MilerApi.login('9000000009'); + expect(MilerApi.pinSetOf(res), isTrue); + }); + + test('the MESSAGE is identical in both cases — only the flag differs', () { + // The reason this is a boolean and not a string match. Both responses + // carry "PIN verification required"; a client reading the sentence cannot + // tell a first-time rider from a returning one. + const first = '{"success":true,"message":"PIN verification required",' + '"pin_set":false}'; + const returning = '{"success":true,"message":"PIN verification required",' + '"pin_set":true}'; + expect( + jsonDecode(first)['message'], + jsonDecode(returning)['message'], + ); + expect( + jsonDecode(first)['pin_set'], + isNot(jsonDecode(returning)['pin_set']), + ); + }); + + test('an absent pin_set is null, and callers fall back to Enter-PIN', () async { + // An older server, or a body that did not carry the field. Enter-PIN is + // the safe direction: a rider who HAS a PIN can sign in, and one who does + // not gets a refusal he can report — rather than a Set-PIN screen that + // will 409 and strand him. + serve((_) => http.Response(jsonEncode({'success': true}), 200)); + final res = await MilerApi.login('8000000001'); + expect(MilerApi.pinSetOf(res), isNull); + }); + + test('a string "false" is read as false, not as truthy prose', () async { + serve( + (_) => http.Response( + jsonEncode({'success': true, 'pin_set': 'false'}), + 200, + ), + ); + expect(MilerApi.pinSetOf(await MilerApi.login('8000000001')), isFalse); + }); + }); + + group('POST /miler/set-pin', () { + test('sends the contract body and stores the session', () async { + serve((req) => http.Response(sessionBody(), 200)); + + final res = await MilerApi.setPin( + phone: '8000000001', + pin: '4271', + deviceToken: 'fcm-token-abc', + ); + + expect(res.ok, isTrue); + expect(sent.single.url.path, endsWith('/miler/set-pin')); + + final body = bodyOf(sent.single); + expect(body['phone'], '8000000001'); + expect(body['new_pin'], '4271'); + expect(body['configid'], MilerApi.configId); + expect(body['device_token'], 'fcm-token-abc'); + expect( + body.containsKey('pin'), + isFalse, + reason: 'the field is new_pin on this route, not pin', + ); + + // The whole point: the rider is signed in by this one call. + expect(await ApiConfig.getToken(), 'jwt-for-46'); + }); + + test('the PIN goes as a STRING, so a leading zero survives', () async { + // The backend bcrypt-compares the PIN as text. Round-tripping "0512" + // through an int gives 512, and a valid PIN is refused forever. + serve((req) => http.Response(sessionBody(), 200)); + await MilerApi.setPin(phone: '8000000001', pin: '0512'); + expect(bodyOf(sent.single)['new_pin'], '0512'); + }); + + test('device_token is omitted rather than sent empty', () async { + serve((req) => http.Response(sessionBody(), 200)); + await MilerApi.setPin(phone: '8000000001', pin: '4271'); + expect(bodyOf(sent.single).containsKey('device_token'), isFalse); + }); + + test('409 — a PIN already exists, and no session is minted', () async { + serve( + (_) => http.Response( + jsonEncode({ + 'success': false, + 'message': 'PIN already set for this account', + }), + 409, + ), + ); + + final res = await MilerApi.setPin(phone: '8000000001', pin: '4271'); + expect(res.ok, isFalse); + expect(res.status, 409); + expect( + await ApiConfig.getToken(), + isNull, + reason: 'a refused set-pin must never leave a token behind', + ); + }); + + test('404 — the number is not registered', () async { + serve( + (_) => http.Response( + jsonEncode({'success': false, 'message': 'no miler account'}), + 404, + ), + ); + final res = await MilerApi.setPin(phone: '7000000000', pin: '4271'); + expect(res.status, 404); + expect(res.ok, isFalse); + }); + + test('403 — the account is not an active miler', () async { + serve( + (_) => http.Response( + jsonEncode({'success': false, 'message': 'inactive'}), + 403, + ), + ); + final res = await MilerApi.setPin(phone: '8000000001', pin: '4271'); + expect(res.status, 403); + expect(res.ok, isFalse); + }); + + test('a 200 with success:false is still a failure', () async { + // The envelope rule: the status line alone is not the answer. + serve( + (_) => http.Response( + jsonEncode({'success': false, 'message': 'nope'}), + 200, + ), + ); + final res = await MilerApi.setPin(phone: '8000000001', pin: '4271'); + expect(res.ok, isFalse); + expect(await ApiConfig.getToken(), isNull); + }); + + test('a success carrying no token leaves no session', () async { + // The credentials were accepted and there is nothing to sign in with. + // An integration fault — and it must not read as a stored session. + serve( + (_) => http.Response(jsonEncode({'success': true}), 200), + ); + final res = await MilerApi.setPin(phone: '8000000001', pin: '4271'); + expect(res.ok, isTrue); + expect(await ApiConfig.getToken(), isNull); + }); + }); + + group('the returning rider is untouched', () { + test('verify-pin still sends `pin` to its own route', () async { + serve((req) => http.Response(sessionBody(userId: 38), 200)); + + await MilerApi.verifyPin(phone: '9000000009', pin: '1234'); + + expect(sent.single.url.path, endsWith('/miler/verify-pin')); + final body = bodyOf(sent.single); + expect(body['pin'], '1234'); + expect( + body.containsKey('new_pin'), + isFalse, + reason: 'new_pin belongs to set-pin alone', + ); + expect(await ApiConfig.getToken(), 'jwt-for-38'); + }); + + test('the two routes are genuinely different endpoints', () async { + serve((req) => http.Response(sessionBody(), 200)); + await MilerApi.setPin(phone: '8000000001', pin: '4271'); + await MilerApi.verifyPin(phone: '8000000001', pin: '4271'); + + expect(sent, hasLength(2)); + expect(sent[0].url.path, endsWith('/miler/set-pin')); + expect(sent[1].url.path, endsWith('/miler/verify-pin')); + }); + }); + + group('the seeded riders', () { + // The three created with no PIN, for exactly this flow. Before this change + // none of them could sign in: precheck read "account exists" and sent them + // to Enter-PIN, where no PIN they typed would ever match. + for (final phone in const ['8000000001', '8000000002', '8000000003']) { + test('$phone is routed to Set-PIN and signs in', () async { + serve((req) { + if (req.url.path.endsWith('/miler/login')) { + return http.Response( + jsonEncode({ + 'success': true, + 'message': 'PIN verification required', + 'phone': phone, + 'pin_set': false, + }), + 200, + ); + } + return http.Response(sessionBody(), 200); + }); + + final login = await MilerApi.login(phone); + expect( + MilerApi.pinSetOf(login), + isFalse, + reason: 'seeded riders are created without a PIN', + ); + + final set = await MilerApi.setPin(phone: phone, pin: '4271'); + expect(set.ok, isTrue); + expect(await ApiConfig.getToken(), isNotNull); + + expect(sent, hasLength(2)); + expect(sent[0].url.path, endsWith('/miler/login')); + expect(sent[1].url.path, endsWith('/miler/set-pin')); + }); + } + }); + + group('no master PIN and no dead OTP path survive', () { + /// Source with every comment line stripped. + /// + /// These assertions are about **code**, not prose — and the distinction is + /// not academic: the comments that explain why the master-PIN constants + /// were removed necessarily quote their names, and a naive scan matches its + /// own explanation and fails. Strip the commentary, assert the code. + String codeOf(String path) => File(path) + .readAsLinesSync() + .where((l) => !l.trimLeft().startsWith('//')) + .join('\n'); + + final auth = codeOf('lib/controllers/auth.dart'); + final signIn = codeOf('lib/views/onboardscreens/Sign_in.dart'); + + test('the 1234 master-PIN constants are gone', () { + // `_masterPinValue = '1234'`, `masterPinValue` and + // `forceMasterPinPrefKey` were declared on AuthController and read by + // nothing — leftovers of a removed feature. A public constant named + // `masterPinValue` holding four digits reads as a back door whether or + // not anything calls it, and it collided with a PIN a rider could now + // legitimately choose. + expect(auth, isNot(contains('masterPinValue'))); + expect(auth, isNot(contains('forceMasterPinPrefKey'))); + expect(auth, isNot(contains('_forceMasterPinFlow'))); + }); + + test('the office-issues-your-PIN dead end is gone', () { + // The sentence the Create-MPIN screen used to end at, because there was + // no route behind its save button. There is one now. + expect( + auth, + isNot(contains('cannot be changed from the app')), + reason: 'riders set their own PIN on first sign-in', + ); + }); + + test('sign-in no longer routes anyone to the unverified OTP screen', () { + // `verifyOtp` returned true without checking a digit, and the screen it + // led to could not write a PIN. Nothing may route there. + expect(signIn, isNot(contains('OtpPage'))); + expect(signIn, contains('AuthNext.setPin')); + expect(signIn, contains('AuthNext.verifyPin')); + }); + + test('sign-in branches on the enum, never on response prose', () { + expect( + signIn, + isNot(contains('PIN verification required')), + reason: 'the message is prose and prose gets reworded', + ); + }); + }); +} diff --git a/test/shots_out/multidrop_queue.png b/test/shots_out/multidrop_queue.png new file mode 100644 index 0000000..ae71c3e Binary files /dev/null and b/test/shots_out/multidrop_queue.png differ diff --git a/test/shots_out/multidrop_single.png b/test/shots_out/multidrop_single.png new file mode 100644 index 0000000..dc5484a Binary files /dev/null and b/test/shots_out/multidrop_single.png differ diff --git a/test/shots_out/multidrop_stop1.png b/test/shots_out/multidrop_stop1.png new file mode 100644 index 0000000..bb7ca2d Binary files /dev/null and b/test/shots_out/multidrop_stop1.png differ diff --git a/test/shots_out/multidrop_stop2.png b/test/shots_out/multidrop_stop2.png new file mode 100644 index 0000000..366f606 Binary files /dev/null and b/test/shots_out/multidrop_stop2.png differ diff --git a/test/task_profile_test.dart b/test/task_profile_test.dart new file mode 100644 index 0000000..d2e95ef --- /dev/null +++ b/test/task_profile_test.dart @@ -0,0 +1,253 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:miler/data/task_profile.dart'; + +/// One rider, mixed work, and every row deciding its own workflow. +/// +/// These tests exist because the app used to answer "what kind of work is +/// this?" by looking at the **rider** — his tenant, resolved once at login into +/// a single app-wide service line. That answer is the same for every row on the +/// screen, so a customer pickup sitting beside a kitchen collection either took +/// the kitchen's rules or disappeared. The rider is one person with a mixed +/// queue; the queue decides. +/// +/// Every input below is what `GET /miler/bookings` actually sends. + +Map _row({ + String? nextAction, + String? pickupSourceType, + String? stopType, +}) => { + if (nextAction != null) 'next_action': nextAction, + if (pickupSourceType != null) 'pickup_source_type': pickupSourceType, + if (stopType != null) 'stoptype': stopType, +}; + +void main() { + group('a row is classified from the server\'s own words', () { + test('a customer door pickup is a customer pickup', () { + final t = TaskProfile.of( + _row(nextAction: 'pickup', pickupSourceType: 'customer'), + ); + expect(t.kind, WorkKind.customerPickup); + expect(t.capturesShipmentDetails, isTrue); + expect(t.groupsBySource, isFalse); + }); + + test('a kitchen load is a source pickup, and groups', () { + final t = TaskProfile.of( + _row(nextAction: 'pickup', pickupSourceType: 'kitchen'), + ); + expect(t.kind, WorkKind.sourcePickup); + expect(t.groupsBySource, isTrue); + expect(t.capturesShipmentDetails, isFalse); + }); + + test('a base collection is a source pickup too', () { + expect( + TaskProfile.of(_row(nextAction: 'pickup', pickupSourceType: 'hub')).kind, + WorkKind.sourcePickup, + ); + }); + + test('carrying it to a receiver is a delivery', () { + expect( + TaskProfile.of(_row(nextAction: 'deliver')).kind, + WorkKind.customerDelivery, + ); + expect( + TaskProfile.of(_row(nextAction: 'start_delivery')).kind, + WorkKind.customerDelivery, + ); + }); + + test('carrying it to a base is a handover', () { + final t = TaskProfile.of(_row(nextAction: 'inward_at_hub')); + expect(t.kind, WorkKind.hubHandover); + expect(t.endsAtHub, isTrue); + expect(t.deliversToCustomer, isFalse); + }); + + test('terminal rows are done', () { + expect(TaskProfile.of(_row(nextAction: 'none')).kind, WorkKind.done); + expect( + TaskProfile.of(_row(nextAction: 'handed_to_hub')).kind, + WorkKind.done, + ); + }); + + test('collected with no stated leg is custody-without-destination', () { + // NextLeg.unknown's case: it is in his bag and nobody has said where it + // goes. Not a delivery (no destination to offer) and not done (he is + // still holding it). + final t = TaskProfile.of(_row(stopType: 'delivery')); + expect(t.kind, WorkKind.unknown); + expect(t.deliversToCustomer, isFalse); + expect(t.endsAtHub, isFalse); + }); + + test('an unheard-of source takes the safe shape, never a guess', () { + final t = TaskProfile.of( + _row(nextAction: 'pickup', pickupSourceType: 'something_new'), + ); + expect(t.kind, WorkKind.unknown); + expect( + t.capturesShipmentDetails, + isFalse, + reason: 'the door-capture flow is a claim this row has not earned', + ); + expect( + t.groupsBySource, + isFalse, + reason: 'nor is a heading it can be filed under', + ); + }); + + test('a row with no type fields at all does not group', () { + // Every fixture written before pickup_source_type shipped, and every + // booking still in flight from then. This defaulted to sourcePickup once + // and split a one-base logistics day into a group per customer -- + // hub_pickup_group_test.dart is the test that caught it. + final legacy = { + 'orderid': '9', + 'pickupid': '9', + 'pickupaddress': 'RS Puram, Coimbatore', + 'type': 'pickup', + }; + final t = TaskProfile.of(legacy); + expect(t.kind, WorkKind.unknown); + expect(t.groupsBySource, isFalse); + expect(t.capturesShipmentDetails, isFalse); + }); + + test('the leg outranks where it was collected from', () { + // Picked up at a customer's door, now in the bag on its way to a base. + // It is a handover, not a pickup — it stopped being a pickup the moment + // it was collected, and only the leg knows that. + final t = TaskProfile.of( + _row(nextAction: 'inward_at_hub', pickupSourceType: 'customer'), + ); + expect(t.kind, WorkKind.hubHandover); + }); + }); + + group('surfaces', () { + test('an unaccepted customer pickup waits on Home', () { + final t = TaskProfile.of( + _row(nextAction: 'pickup', pickupSourceType: 'customer'), + ); + expect(t.surfaceWhen(accepted: false), WorkSurface.home); + }); + + test('accepting a customer pickup moves it to Bookings', () { + final t = TaskProfile.of( + _row(nextAction: 'pickup', pickupSourceType: 'customer'), + ); + expect(t.surfaceWhen(accepted: true), WorkSurface.bookings); + }); + + test('a kitchen load stays on Home even once accepted', () { + // Accepting a manifest puts nothing in his hands. It leaves Home when he + // is actually carrying it, not when he agrees to fetch it. + final t = TaskProfile.of( + _row(nextAction: 'pickup', pickupSourceType: 'kitchen'), + ); + expect(t.surfaceWhen(accepted: true), WorkSurface.home); + }); + + test('anything in custody is on Deliveries', () { + for (final action in ['deliver', 'start_delivery', 'inward_at_hub']) { + expect( + TaskProfile.of(_row(nextAction: action)).surfaceWhen(accepted: true), + WorkSurface.deliveries, + reason: action, + ); + } + expect( + TaskProfile.of(_row(stopType: 'delivery')).surfaceWhen(accepted: true), + WorkSurface.deliveries, + ); + }); + + test('finished work is on Activity', () { + expect( + TaskProfile.of(_row(nextAction: 'none')).surfaceWhen(accepted: true), + WorkSurface.activity, + ); + }); + }); + + group('the mixed shift', () { + // The case the architecture exists for: one rider around RS Puram holding + // ten meal parcels to collect at a Gandhipuram kitchen and five customer + // pickups at five different doors, all at once. + final queue = >[ + for (var i = 0; i < 10; i++) + _row(nextAction: 'pickup', pickupSourceType: 'kitchen'), + for (var i = 0; i < 5; i++) + _row(nextAction: 'pickup', pickupSourceType: 'customer'), + ]; + + test('all fifteen survive classification', () { + final kinds = queue.map((r) => TaskProfile.of(r).kind).toList(); + expect(kinds.length, 15); + expect(kinds.where((k) => k == WorkKind.sourcePickup).length, 10); + expect(kinds.where((k) => k == WorkKind.customerPickup).length, 5); + expect( + kinds.contains(WorkKind.unknown), + isFalse, + reason: 'nothing falls through to the safe shape when the server spoke', + ); + }); + + test('accepting the five does not move the ten', () { + final surfaces = queue + .map((r) => TaskProfile.of(r).surfaceWhen(accepted: true)) + .toList(); + expect(surfaces.where((s) => s == WorkSurface.home).length, 10); + expect(surfaces.where((s) => s == WorkSurface.bookings).length, 5); + }); + + test('collected meal parcels coexist with a CX handover on Deliveries', () { + final afterCollection = >[ + for (var i = 0; i < 10; i++) _row(nextAction: 'deliver'), + // The CX pickup, now completed, has become a base handover. + _row(nextAction: 'inward_at_hub', pickupSourceType: 'customer'), + ]; + final surfaces = afterCollection + .map((r) => TaskProfile.of(r).surfaceWhen(accepted: true)) + .toSet(); + expect(surfaces, {WorkSurface.deliveries}); + }); + }); + + group('a trip is described by what is actually in it', () { + Map row({String? a, String? src}) => { + if (a != null) 'next_action': a, + if (src != null) 'pickup_source_type': src, + }; + + test('a mixed trip both groups and ends at a base', () { + final stops = [ + row(a: 'pickup', src: 'kitchen'), + row(a: 'pickup', src: 'customer'), + row(a: 'inward_at_hub'), + ]; + expect(TripShape.groupsBySource(stops), isTrue); + expect(TripShape.endsAtHub(stops), isTrue); + expect(TripShape.hasCustomerPickup(stops), isTrue); + }); + + test('one parcel bound for a base ends the whole day at that base', () { + // `any`, not `every`. Telling a rider "END · HOME" while he is still + // carrying something the network needs is the failure worth avoiding. + final stops = [row(a: 'deliver'), row(a: 'inward_at_hub')]; + expect(TripShape.endsAtHub(stops), isTrue); + }); + + test('a pure meal round neither groups to a base nor returns to one', () { + final stops = [row(a: 'deliver'), row(a: 'deliver')]; + expect(TripShape.endsAtHub(stops), isFalse); + expect(TripShape.hasCustomerPickup(stops), isFalse); + }); + }); +} diff --git a/test/work_scope_test.dart b/test/work_scope_test.dart index 4a1e277..c36b727 100644 --- a/test/work_scope_test.dart +++ b/test/work_scope_test.dart @@ -8,7 +8,7 @@ import 'package:miler/data/service_profile.dart'; import 'package:miler/data/work_scope.dart'; /// ───────────────────────────────────────────────────────────────────────── -/// TWO LINES, ONE HANDSET, NO MIXING +/// ONE RIDER, ONE DRAWER /// /// Miler runs a milk-man round and a logistics day off one app and one login, /// and it kept finished work under *global* SharedPreferences keys — @@ -18,75 +18,81 @@ import 'package:miler/data/work_scope.dart'; /// /// These pin the ownership rule at the data boundary. Nothing here filters on /// a display string — no kitchen names, no "Milk", no screen titles — only on -/// identity the session can prove: rider, tenant, line. +/// identity the session can prove: rider and tenant. +/// +/// ── The line came OUT of the key, deliberately ── +/// +/// It used to be a third part: `completed_bookings::u38.t13.milkMan`. That was +/// right while a rider was one kind of rider for the life of his account. He is +/// not. One Miler carries meal parcels, logistics collections and customer +/// pickups in the same shift, and keying his records by line split one day +/// across two drawers — so work he finished an hour ago vanished when the app +/// resolved him differently. Two tests below used to assert that split and now +/// assert the opposite; they are the record of the rule changing. /// ───────────────────────────────────────────────────────────────────────── void main() { - const milkA = WorkScope(userId: 38, tenantId: 13, line: ServiceLine.milkMan); - const logisticsA = WorkScope( - userId: 38, - tenantId: 13, - line: ServiceLine.parcel, - ); - const milkB = WorkScope(userId: 99, tenantId: 13, line: ServiceLine.milkMan); - const otherTenant = WorkScope( - userId: 38, - tenantId: 77, - line: ServiceLine.milkMan, - ); + const riderA = WorkScope(userId: 38, tenantId: 13); + const riderB = WorkScope(userId: 99, tenantId: 13); + const otherTenant = WorkScope(userId: 38, tenantId: 77); group('the key', () { - test('rider, tenant and line each change the drawer', () { + test('rider and tenant each change the drawer', () { final keys = { - milkA.scoped('completed_bookings'), - logisticsA.scoped('completed_bookings'), - milkB.scoped('completed_bookings'), + riderA.scoped('completed_bookings'), + riderB.scoped('completed_bookings'), otherTenant.scoped('completed_bookings'), }; expect( keys.length, - 4, + 3, reason: - 'all three identity parts must be in the key — two scopes ' - 'sharing a key is the leak itself', + 'both identity parts must be in the key — two scopes sharing a ' + 'key is the leak itself', ); expect( - milkA.scoped('completed_bookings'), + riderA.scoped('completed_bookings'), contains('completed_bookings'), ); }); + test('the line is no longer part of the drawer', () { + expect( + riderA.key, + isNot(contains('milkMan')), + reason: 'one rider, one day, one drawer — whatever he is carrying', + ); + for (final legacy in riderA.legacyScopedKeys('completed_bookings')) { + expect(legacy, isNot(riderA.scoped('completed_bookings'))); + expect(legacy, startsWith('completed_bookings::u38.t13.')); + } + }); + test('the same session resolves to the same drawer', () { expect( - milkA.scoped('skipped_bookings'), - const WorkScope( - userId: 38, - tenantId: 13, - line: ServiceLine.milkMan, - ).scoped('skipped_bookings'), + riderA.scoped('skipped_bookings'), + const WorkScope(userId: 38, tenantId: 13).scoped('skipped_bookings'), ); }); }); group('ownership', () { - test('a milk-man record is not a logistics record', () { - final row = milkA.stamp({'orderid': 'A1'}); - expect(milkA.owns(row), isTrue); - expect( - logisticsA.owns(row), - isFalse, - reason: 'same rider, same tenant, other LINE — must not cross', - ); - expect(logisticsA.excludes(row), isTrue); + test('one rider owns his record whatever he was carrying', () { + // This asserted the opposite once: same rider, same tenant, other line + // "must not cross". Under one-Miler that rule loses him his own work — + // a meal drop and a parcel collection in the same shift are both his. + final row = riderA.stamp({'orderid': 'A1'}); + expect(riderA.owns(row), isTrue); + expect(riderA.excludes(row), isFalse); }); test('another rider cannot claim it', () { - final row = milkA.stamp({'orderid': 'A1'}); - expect(milkB.owns(row), isFalse); - expect(milkB.excludes(row), isTrue); + final row = riderA.stamp({'orderid': 'A1'}); + expect(riderB.owns(row), isFalse); + expect(riderB.excludes(row), isTrue); }); test('another tenant cannot claim it', () { - final row = milkA.stamp({'orderid': 'A1'}); + final row = riderA.stamp({'orderid': 'A1'}); expect(otherTenant.owns(row), isFalse); expect(otherTenant.excludes(row), isTrue); }); @@ -94,8 +100,8 @@ void main() { test('the API stamps its own identity and it is honoured', () { // Rows straight off `/miler/assignments` carry the rider; no scope stamp // is involved and it must still be respected. - expect(milkA.excludes({'orderid': 'A1', 'mileruserid': 99}), isTrue); - expect(milkA.excludes({'orderid': 'A1', 'mileruserid': 38}), isFalse); + expect(riderA.excludes({'orderid': 'A1', 'mileruserid': 99}), isTrue); + expect(riderA.excludes({'orderid': 'A1', 'mileruserid': 38}), isFalse); }); test('silence is read differently by the two questions', () { @@ -103,8 +109,8 @@ void main() { // rows, where inventing ownership IS the leak), `excludes` keeps it // (used on rows the API just returned for this session). final bare = {'orderid': 'A1'}; - expect(milkA.owns(bare), isFalse); - expect(milkA.excludes(bare), isFalse); + expect(riderA.owns(bare), isFalse); + expect(riderA.excludes(bare), isFalse); }); }); @@ -141,34 +147,35 @@ void main() { }, ); - test('switching line cannot expose the other line’s history', () async { + test('a mixed shift is one history, not two', () async { + // The inverse of what this asserted before. One Miler collects meals and + // parcels in the same shift; his Activity is his day, and a label the app + // resolved him with must not hide half of it. SharedPreferences.setMockInitialValues({'userid': 38}); ServiceProfile.setActive(ServiceProfile.milkMan); await addCompletedBookings([ {'orderid': 'ROUND-1'}, ], terminalStatus: 'delivered'); - expect( - (await getCompletedBookings()).map((r) => r['orderid']), - contains('ROUND-1'), - ); - // Same rider, same tenant, logistics day. ServiceProfile.setActive(ServiceProfile.parcel); - expect( - (await getCompletedBookings()).where((r) => r['orderid'] == 'ROUND-1'), - isEmpty, - reason: 'a milk-man delivery belongs only to milk-man Activity', - ); - - // And a logistics collection stays out of the round's history. await addCompletedBookings([ {'orderid': 'PARCEL-1'}, ], terminalStatus: 'picked'); + + final ids = (await getCompletedBookings()) + .map((r) => r['orderid']) + .toList(); + expect( + ids, + containsAll(['ROUND-1', 'PARCEL-1']), + reason: 'both were finished by rider 38 on tenant 13, in one shift', + ); + + // And it still holds from the other side. ServiceProfile.setActive(ServiceProfile.milkMan); expect( - (await getCompletedBookings()).where((r) => r['orderid'] == 'PARCEL-1'), - isEmpty, - reason: 'a logistics pickup belongs only to logistics Activity', + (await getCompletedBookings()).map((r) => r['orderid']), + containsAll(['ROUND-1', 'PARCEL-1']), ); }); @@ -179,7 +186,11 @@ void main() { ], terminalStatus: 'delivered'); final row = (await getCompletedBookings()).single; expect(row['scopeuserid'], 38); - expect(row['scopeline'], ServiceLine.milkMan.name); + expect( + row.containsKey('scopeline'), + isFalse, + reason: 'the line is not part of ownership any more', + ); }); test('logout empties this scope', () async { @@ -197,6 +208,52 @@ void main() { ); }); + test('work stored under the old line-suffixed key is not lost', () async { + // The upgrade a real rider takes mid-shift. His finished stops were + // written to `completed_bookings::u38.t13.milkMan`; the key no longer has + // that suffix. Dropping them would lose work he actually did, and unlike + // a global key these rows are NOT anonymous — the key itself names him. + final today = DateTime.now(); + final stamp = + '${today.year}-${today.month.toString().padLeft(2, '0')}-' + '${today.day.toString().padLeft(2, '0')}'; + + SharedPreferences.setMockInitialValues({ + 'userid': 38, + 'completed_bookings::u38.t0.milkMan': jsonEncode([ + {'orderid': 'OLD-MILK', 'completedday': stamp}, + ]), + 'completed_bookings::u38.t0.parcel': jsonEncode([ + {'orderid': 'OLD-PARCEL', 'completedday': stamp}, + ]), + }); + + // The migration has to be *run*. It is not lazy — `getCompletedBookings` + // reads the scoped key and nothing else — and this test called the reader + // without the migrator, so it was asserting against a drawer nobody had + // filled yet. `main()` awaits this on startup, before the first frame. + await migrateLegacyStores(); + + final rows = await getCompletedBookings(); + expect( + rows.map((r) => r['orderid']), + containsAll(['OLD-MILK', 'OLD-PARCEL']), + reason: 'both old drawers belong to rider 38 — merge, never drop', + ); + + // And the old keys are gone, so a second run cannot duplicate them. + final prefs = await SharedPreferences.getInstance(); + expect(prefs.containsKey('completed_bookings::u38.t0.milkMan'), isFalse); + expect(prefs.containsKey('completed_bookings::u38.t0.parcel'), isFalse); + expect( + (await getCompletedBookings()) + .where((r) => r['orderid'] == 'OLD-MILK') + .length, + 1, + reason: 'the merge must be idempotent', + ); + }); + test('unattributable legacy rows are dropped, not adopted', () async { // The pre-scoping global key, holding somebody's rows. Nothing on them // says whose, so adopting them would be inventing ownership.