Files
doormile_backend/docs/customer-app-api.md

929 lines
59 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Customer App API (`doormile_cx`) — change record & integration guide
**Audience:** the `doormile_cx` app developer, plus anyone reviewing this before
it goes to production.
**Backend:** Go + Fiber, `api.doormile.com/api/v1`, namespace `/customer/*`.
**Status:** code complete, compiler- and unit-verified, **not yet run against a
real database**. Read §11 before deploying.
| | |
|---|---|
| OpenAPI 3.1 spec | [`openapi-customer.yaml`](openapi-customer.yaml) |
| Staging seed | [`../seed_customer_app.sql`](../seed_customer_app.sql) |
| Requirements this answers | *Doormile — Backend Requirements (Customer App v1)* |
| Reviewed | Once, by a reviewer with the Flutter apps but not this repo — **§13** records every finding and what changed |
---
## 0. The one question first: does this affect the Miler app or the consoles?
# YES.
Not "no". Anyone telling you otherwise has not read the diff. The customer work
could not be built without touching shared code, because the thing it changes —
one pickup producing many orders — is a fact the rider app and the consoles both
have to cope with.
Here is the complete list, with severity. Nothing is omitted.
### 🔴 Behaviour changes those clients WILL observe
| # | What changed | Who sees it | Action needed |
|---|---|---|---|
| 1 | **Booking references are now `DM-482913`**, not `DM-BK-A1B2C3D4-48291`. New bookings only; existing rows keep their old strings. | Admin console, hub console, Miler app — anywhere a booking number is displayed | None in code (nothing parses the format). Tell ops the format changed. |
| 2 | **Tracking numbers are now `DMX10482913`**, not `DM-TRK-A1B2C3D4-48291`. New consignments only. | Same as above, plus any printed label or customer-facing tracking link | Same. Check label templates for a hardcoded width. |
| 3 | **`GET /miler/bookings` can return more than one row per booking.** A customer-app pickup with 3 destinations becomes 3 stops *after* collection — each with its own `consignmentid`, `trackingno`, address and COD. Before collection it is still 1 row. Console/express bookings are still always 1 row. | Miler app | **A multi-file change, not a one-liner. See §0.6.** |
| 4 | **`POST /miler/bookings/{id}/pickup-complete` gained a `consignments` array** listing every order the pickup produced. All previous top-level fields still describe the first order, unchanged. | Miler app | Purely additive — safe to ignore, but a multi-destination build should read it. |
| 5 | **`POST /miler/bookings/{id}/parcel` accepts an optional `photos: []` per parcel** (storage keys from `/miler/uploads/sign`). Response shape unchanged. | Miler app | Optional. Without photos the customer receipt shows a weight and no evidence. |
| 6 | **`PickupBooking` JSON gained 10 fields** (`slotid`, `customerstage`, `customerstatus`, `estimateminrupees`, `estimatemaxrupees`, `routekm`, `pickuptitle`, `pickupsub`, `cancelreason`, `destinations`) and `BookingParcel` gained `bookingdestinationid`. These serialise on every admin/hub endpoint that returns the model raw. | Admin + hub consoles | Additive JSON. Safe for a JS client that reads named keys. |
| 7 | **Every response now carries an `X-Request-Id` header.** | Everyone | None. Log it — it is how a support report gets correlated. |
| 8 | **Unknown paths under `/customer/*` answer `401`, not `404`.** So the retired `doormile_customer_app` calling `POST /customer/login` gets `401 "authorization header is required"`. | The old customer app | Expected — that app is being retired. Pre-existing Fiber group behaviour, not introduced here. |
### 🟡 Internal changes with no client-visible contract change
| # | What changed | Why it is safe |
|---|---|---|
| 9 | Three miler endpoints now resolve a consignment through `bookingdestinations` instead of `pickupbookings.consignmentid` | Bug fix. Identical result for a single-destination booking; the old query returned *nothing* for orders 2..N. See §5. |
| 10 | `assignMilerTx` and `commitAssignment` record a customer stage | Guarded: returns immediately for any booking whose source is not `Customer_App`. An express booking does zero extra writes. |
| 11 | `AcceptMilerAssignment`, `BookingReachedCustomer`, `MilerCancelAssignment`, `MilerStartDelivery`, `MilerInwardConsignmentAtHub`, `MilerDeliverConsignment` record customer stages | Same guard. Response bodies unchanged. |
| 12 | `middlewares.Idempotency` scopes anonymous callers differently | Authenticated key format is **byte-identical** to before (`idem:<uid>:<key>`), so no in-flight rider key is orphaned at deploy. See §5.4. |
| 13 | Request logs gained `client`, `platform`, `requestid` fields | Log-only. |
| 14 | `utils.GenerateToken` delegates to a new `GenerateTokenWithTTL` | Still 24h for miler/admin/hub. Only the customer surface passes a different TTL. |
| 15 | `internal/storage` gained `PresignGet` | New function; nothing existing calls it. |
| 16 | `middlewares/city_gate.go` gained an exported `PincodeInOperatingCity` | New function; `CityGateMiddleware` itself is unchanged. |
| 17 | 9 new tables + 11 new columns via `AutoMigrate`; 2 sequences, 2 indexes | All additive and nullable/defaulted. See §11 for the deploy note. |
### 🟢 Deliberately NOT changed, though I was tempted
Two things I changed while building, then **reverted**, because they were miler
behaviour and out of scope:
- **Rider availability after pickup-complete.** I had made a rider stay
`Picked_Up` when a hyperlocal parcel goes straight to `Out_for_Delivery`. The
endpoint has always set them `Available` there. The existing code comment says
the opposite of what the code does — a pre-existing inconsistency I have
flagged rather than silently "fixed", because changing it alters live rider
availability and the dispatch pool. **Ops decision, not mine.**
- **`POST /miler/consignments/{id}/deliver` error status.** I had changed a
"not yours" failure from `404` to `403` for consistency with start-delivery.
Reverted: the deployed rider app was built against `404` and its exact
messages. Only the broken *lookup* is fixed.
---
## 0.5 Page-by-page: exactly which screens are affected
Read from the client source in this workspace (`doormile_crm` = admin console,
`doormile_hub_console` = hub console), not inferred from the API. The Miler app
is not in this workspace, so its rows are derived from the endpoints it calls and
must be confirmed against the Flutter source.
Legend: 🔴 needs a code change · 🟡 visible change, no code change · 🟢 fixed by
this work · ⚪ no impact
### Miler app (Flutter — verify against the app source)
| Screen | Endpoint behind it | Impact |
|---|---|---|
| **Home / my stops** | `GET /miler/bookings` | 🔴 **A pickup can now be several rows.** After collection, a 3-destination customer booking returns 3 stops sharing one `bookingid`. **Key the list on `consignmentid`.** New fields `destinationseq` / `destinationcount` give you "Stop 2 of 3", and `trackingno`, `recipientname`, `recipientphone` are now per stop. Before collection it is still exactly 1 row, and console/express bookings are always 1 row. |
| **Pickup — complete** | `POST /miler/bookings/{id}/pickup-complete` | 🟡 Response gained `consignments[]` (every order the visit produced). All existing top-level fields still describe the first order and are unchanged, so an old build keeps working. |
| **Pickup — weigh/photograph parcels** | `POST /miler/bookings/{id}/parcel` | 🟡 Each parcel now accepts an optional `photos: []` of storage keys from `/miler/uploads/sign`. Response shape unchanged. Without photos the customer's receipt shows a weight with no evidence behind it. |
| **Delivery — deliver / skip** | `POST /miler/consignments/{id}/deliver` | ⚪ Contract unchanged. The internal lookup was broken for orders 2..N and is fixed; status codes and messages are byte-identical. |
| **Base handover** | `POST /miler/consignments/{id}/inward-at-hub` | ⚪ Contract unchanged; same lookup fix. |
| **Earnings** | `GET /miler/earnings` | 🟢 **Fixed before it shipped.** Customer-app jobs would have recorded ₹0 — see §5.6. |
| **Any screen showing a tracking number** | — | 🟡 `DMX10482913` instead of `DM-TRK-A1B2C3D4-48291`. Ten characters shorter, so nothing overflows. |
| Duty, breaks, notifications, support, profile, bases | — | ⚪ Untouched. |
### Admin console (`doormile_crm`)
Only **one** of its screens reads booking shape at all. Verified by grep across
`src/pages`.
| Screen | File | Impact |
|---|---|---|
| **Orders list** | `src/pages/bookings/Bookings.jsx` | 🟡 Four things: (1) the **Booking No** column now shows `DM-482913` — column is `proportional(1.5)`, the string is shorter, nothing overflows; (2) **search** filters `bookingno` by substring, so typing `DM-BK` finds no new bookings — tell ops; (3) the **Delivery City** column reads the flat `deliverycity`, which is **destination 0 only** — a 3-destination pickup shows "Chennai" and does not hint at the other two; (4) the **Price** column reads `serviceoptions[0].estimatedprice` and 🟢 **would have shown "N/A" for every customer booking** until §5.6 was fixed. |
| **Cancel order** (from the Orders list) | `POST /admin/bookings/{id}/cancel`, `.../bulk-cancel` | 🟢 **Fixed before it shipped.** An ops cancel never reached the customer's app — see §5.7. No console change needed; the fix is backend-side. |
| Dashboard, Tenants, Customers, Pricing, Riders, Reports, Invoices, Team, Settings, Survey, auth & error screens | — | ⚪ None of them read booking or consignment shape. |
### Hub console (`doormile_hub_console`)
| Screen | File | Reads | Impact |
|---|---|---|---|
| **Inbound** | `src/pages/operations/Inbound.jsx` | `trackingno` | 🟡 Tracking-number format change is visible in the table. Destination cell already ellipsises at `maxWidth: 200px`. |
| **Routing** | `src/pages/operations/Routing.jsx` | `trackingno` | 🟡 Format change visible in the package list and the detail card (monospace, `0.85rem`). Search-by-tracking-number still works — it matches the value the API returns, not a pattern. |
| **Order Assignment** | `src/pages/operations/OrderAssignment.jsx` | `bookingid`, `deliveryaddress` | 🟡 The row id uses the numeric `bookingid`, so the ID-format change does not touch it. But `drop` reads the flat `deliveryaddress`, which is **destination 0 only** — staff assigning a 3-destination pickup see one address. Not broken; incomplete. |
| **Rider Routes** | `src/pages/operations/RiderRoutes.jsx` | `bookingid`, `consignmentid`, `parcels` | 🟡 Reads `consignmentid` off the booking, which names only the **first** order. A rider carrying three parcels from one visit may render as one stop on this screen. |
| **Tracking Map** | `src/pages/operations/TrackingMap.jsx` | `bookingid` | ⚪ Numeric id only. |
| **Dashboard** | `src/pages/Dashboard.jsx` | `parcels` | ⚪ Counts only. Every parcel still hangs off the booking, so the totals stay correct. |
| **Dispatch** | `src/pages/operations/Dispatch.jsx` | `parcels` | ⚪ As above. |
| **Riders** | `src/pages/operations/Riders.jsx` | `parcels` | ⚪ As above. |
| Hub Settings, Login, Signup | — | ⚪ Untouched. |
### What nobody has to change today
Multi-destination pickups **only exist once the new customer app is live**. Every
booking in the database today has zero destination rows and therefore takes the
single-leg path everywhere — the rider queue returns one row, the consoles show
one address, and the fan-out never runs. The 🟡 "destination 0 only" rows above
become real on the day the first three-destination pickup is booked, not on the
day this deploys.
The exceptions that are live immediately: the **ID formats** (§0 items 1–2) and
the extra **JSON fields** (§0 item 6).
---
## 0.6 The rider-app change, scoped properly
An earlier draft of this document said "key the list on `consignmentid`" as
though that were a small edit. It is not, and the correction came from a review
with access to the Flutter source that this backend repo does not have. Recorded
here in full because the item is assigned to the app team and was understated.
**The rider app's identity key is `orderid`, not `bookingid`.** The adapter sets
it at `lib/data/api_config.dart:321`:
```dart
'orderid': ref ?? id, // ref = bookingreference, id = bookingid
```
Both of those are **booking-level**, and all three destination rows of one
pickup carry the same `bookingid` and the same `bookingreference`. So `orderid`
is identical across the three — and `orderid` is what dedupes:
- `lib/data/accepted_store.dart:527` — accepted stops live in a `Map` keyed on
`orderid`. Three stops collapse to one, and because that store is
SharedPreferences-backed, **the collapse survives an app restart.**
- `lib/data/work_repository.dart:314,321` and `order_events.dart` — per-order
event stamps key on `orderid`, so orders 2..N overwrite order 1's clocks.
`consignmentid` is available to the adapter (`api_config.dart:443`), so the
change is feasible — but it lands across `accepted_store`, `work_repository`,
`order_events` and `assignment_lookup`.
**Until it ships, a multi-destination pickup shows the rider one stop instead of
three** — the same customer-visible failure §5.1 fixed on the server. The server
being right is not enough here.
**`destinationseq` / `destinationcount` are invisible to the app today.**
`pickupFromBooking` builds a fixed map, so a field the adapter does not name is a
field the app can never see (the file says as much around line 583). "Stop 2 of
3" needs those two keys added to the adapter as well.
### The mitigation that needs no app release
`maxDestinations` is **server configuration**, not a constant — the client reads
it from `GET /customer/config/booking-limits` and adapts. Setting
`customerbookinglimits.maxdestinations = 1` keeps every pickup single-destination
until the rider build lands, which means:
- the customer app can ship and book end to end today;
- the fan-out code stays dormant, so no order can be stranded;
- lifting the cap later is one `UPDATE`, with no deploy on any side.
**Recommendation: seed it at 1 and raise it only once a rider build that keys on
`consignmentid` is live.** This is the same discipline
`MILER_HUB_HANDOVER_ENABLED` already applies in this codebase — do not turn on a
server behaviour the deployed rider app cannot complete.
---
## 0.7 Fan-out row audit — field by field
Requested audit, run against the code and locked down by tests. `GET /miler/bookings`
after collection, one row per order.
| Field | Source | Destination-specific? | Test |
|---|---|---|---|
| `consignmentid` | `bookingdestinations.consignmentid` | ✅ | `TestFanoutTrackingAndConsignmentIdsStayWithTheirStop` |
| `trackingno` | `bookingdestinations.trackingno` | ✅ | same |
| `deliveryaddress` | built from the destination's building / street / landmark / district / state | ✅ | `TestFanoutStopsHaveDistinctAddresses` |
| `deliverylatitude` | destination pin, else district centre | ✅ | `TestFanoutStopsHaveDistinctCoordinates` |
| `deliverylongitude` | same | ✅ | same |
| `recipientname` | `bookingdestinations.recipientname` | ✅ | `TestFanoutRecipientsStayWithTheirStop` |
| `recipientphone` | `bookingdestinations.recipientphone` | ✅ | same |
| `collectionamt` | consignment `codamount − codcollected`, falling back to the destination's own `codamount` | ✅ **newly added** | `TestFanoutCodIsNotCopiedAcrossDestinations` |
| `destinationseq` | `bookingdestinations.seq` | ✅ | `TestFanoutRouteOrderFollowsDestinationSeq` |
| `destinationcount` | number of stops on the pickup | ✅ uniform per booking | `TestFanoutDestinationCountIsUniform` |
**Three findings from running this audit:**
1. **`collectionamt` did not exist.** The endpoint emitted `codamount` only.
Added as an additional key rather than a rename — `codamount` is what this
endpoint has always sent and the deployed rider build parses it, so removing
it would break every stop on every existing device. Both are written from one
variable and cannot drift.
2. **COD was not on the stop at all.** It was resolved in the handler from the
consignment map, so it could not be unit-proven, and it fell back to **0** if
the consignment row failed to load. A rider shown ₹0 collects nothing. The
stop now carries the destination's own `codamount` as the fallback.
3. **Stop ordering was not guaranteed.** `milerStopsForBooking` iterated the
slice as given and relied entirely on the caller's `ORDER BY seq`. It worked,
but the ordering *is the route the rider drives*, and leaving it as an
unstated precondition means the next caller silently reorders someone's
afternoon. It now sorts by `seq` on a copy.
**Booking-level fallback is confined to one place and one case:** a booking with
**no destination rows** — every console/express booking and every row written
before this table existed — produces exactly one stop from the booking's flat
columns. A booking that *has* destination rows never reads them. Destination 0's
coordinates are used as a fallback for stop 0 only, and only when that
destination has no pin of its own, because that is the same row mirrored.
---
## 1. v1 requirements → what actually shipped
The requirements doc described the target. This is the delta between it and the
code as it now stands.
### 1.1 Endpoints — all 22 requirement endpoints implemented
Route arithmetic, since three numbers in this document have to reconcile:
**28 registered customer routes** = 8 pre-auth (4 auth + 4 catalogue) + 20
authenticated. The 22 below is the count of endpoints the *requirements* asked
for; the remaining 6 are the profile and saved-address routes in the table under
it. The OpenAPI spec now documents all 28 (24 paths / 28 operations).
| § | Endpoint | Status | Deviation from the requirement |
|---|---|---|---|
| 4.1 | `POST /customer/auth/otp/request` | ✅ | Answers identically whether or not the account exists — telling an anonymous caller "no account found" would make this a directory of who is registered. |
| 4.2 | `POST /customer/auth/signup` | ✅ | Existing phone treated as sign-in, as specified. |
| 4.3 | `POST /customer/auth/otp/verify` | ✅ | Idempotency-Key honoured. |
| 4.4 | `POST /customer/auth/refresh` | ✅ | Rotation. A **revoked** token replayed revokes the whole chain. |
| 4.4 | `POST /customer/auth/logout` | ✅ | No `refreshToken` in the body ⇒ signs out everywhere. |
| 4.4 | `GET /customer/auth/me` | ✅ | — |
| 5.1 | `GET /customer/serviceability/states` | ✅ | + ETag/304. |
| 5.2 | `GET .../states/{code}/districts` | ✅ | + ETag/304. |
| 5.3 | `GET /customer/pickup-slots` | ✅ | Slot ids encode their date (`slot_20260905_t1`) so a stale cached slot resolves to *that* day and is refused, not silently booked today. |
| 5.4 | `GET /customer/config/booking-limits` | ✅ | Accepts `lat`/`lng` for per-city limits. |
| 6.1 | `GET /customer/places/reverse-geocode` | ✅ | On upstream failure returns a coordinate label, **not** an error. |
| 6.2 | `GET /customer/places/search` | ✅ | Empty `q` ⇒ saved + recent places. |
| 7 | `POST /customer/fare/estimate` | ✅ | — |
| 9.1 | `POST /customer/bookings` | ✅ | — |
| 9.2 | `GET /customer/bookings` | ✅ | **Keyset** pagination, not offset. |
| 9.3 | `GET /customer/bookings/{reference}` | ✅ | + ETag/304 for polling. |
| 9.4 | `POST .../cancel` | ✅ | Cancelling an already-cancelled booking returns **200**, not 409. |
| 9.5 | `PATCH .../destinations/{index}` | ✅ | — |
| 9.6 | `GET /customer/orders/{trackingId}` | ✅ | Returns the full booking object (the requirement offered a slimmer shape; the client already parses this one). |
| 10 | `POST /customer/devices` | ✅ | + `DELETE /customer/devices/{token}`. |
| 10 | Push on milestone change | ✅ | `on_the_way` and `order_created` deliberately silent, per §10. |
| 11 | `POST /customer/ops/bookings/{ref}/stage` | ✅ | Double-gated. Walks every intermediate stage rather than jumping. |
**Also kept** (not in the requirements, not removed): `GET/PUT
/customer/profile`, `GET/POST/PUT/DELETE /customer/locations` — these answer §13.5
"saved addresses".
### Identifier scrambling — sequence-backed, but not readable
Both identifiers come off a Postgres sequence, because a sequence is the only
generator here that can promise uniqueness: the columns are `UNIQUE`, and a
random 8-digit id collides with ~43% probability by the ten-thousandth parcel —
a rider unable to complete a pickup.
But a raw sequence is readable. `DMX10000042` and `DMX10000043` are visibly
adjacent, so anyone holding two numbers learns the throughput between them, and
anyone holding one can guess its neighbours. So the index goes through a keyed
4-round Feistel permutation before it is formatted
(`controllers/cxIdentifierScramble.go`):
```
seq 10004200 -> DMX35031894 seq 100000 -> DM-499505
seq 10004201 -> DMX31070203 seq 100001 -> DM-341775
seq 10004202 -> DMX62788115 seq 100002 -> DM-106366
```
A Feistel network is a bijection for **any** round function, so uniqueness is
untouched. `TestBookingScrambleIsCollisionFree` proves it **exhaustively across
all 900,000 booking references**; the tracking test covers 200,000 consecutive
values; `TestFeistelIsAPermutation` checks the primitive itself exhaustively.
Deliberately **not** the obvious one-liner (multiply by a coprime). That is also
a bijection, but it is *linear* — and a multi-destination pickup hands one
customer three consecutive sequence values, so the differences between their
three tracking numbers would all equal the multiplier. A single booking would
reveal the mapping and make the whole range walkable.
`TestOneBookingDoesNotLeakTheMapping` rejects that design.
Past the fixed-width range (900,000 bookings / 90,000,000 orders) the identifier
grows a digit rather than wrapping onto one already issued. Uniqueness is never
traded for appearance.
**This is not a security boundary.** Every route that resolves a tracking number
is already authenticated and owner-scoped — that is what stops a stranger reading
someone's parcel. This removes the leak in the identifier itself, so
authorisation is not the only thing standing between an outsider and your volume
figures. Key: `CX_ID_SCRAMBLE_KEY`; a working default ships so the scrambling can
never be silently off.
### 1.2 Contract decisions the requirements left open
| Question | Decision | Consequence for the app |
|---|---|---|
| §3.2 field naming | **camelCase**, as preferred | **No mapping layer needed.** Served from purpose-built projections, so `/miler/*` keeps its lowercase keys. |
| §3.3 timestamps | **Epoch milliseconds, UTC, integer** | `DateTime.fromMillisecondsSinceEpoch(int)` works as-is. |
| §3.1 envelope | Payload in `data` on **every** response, auth included | One parser for the whole surface. |
| §9.6 order shape | Full booking object | ⚠️ **Not "reuse `Booking.fromJson`" as written earlier.** That parser exists but today reads only `reference`, `stage`, `cancellable`, `createdAt`, `pickup`, `slotId` and four destination fields. It ignores `status`, `miler`, `deliveryAgent`, `fare`, `amountPaid`, `deliveredAt`, `history`, `verification`, `details`, per-destination `stage`, `routeKm` and `expectedDelivery`. The server shape is right; the client parser has to be finished. Appendix B of the requirements said so and this document should not have glossed it. |
| §9.3 `miler.phone` | Proxy when `MILER_CALL_PROXY` is set, real number otherwise | **Currently unset ⇒ real number.** See §10.2. |
| Identifier format | Sequence-backed `DM-######` / `DMX########` | Matches the designs. |
### 1.3 Timestamps — the defect that would have shipped silently
This database stores **IST wall-clock digits** in its timestamp columns (the DSN
sets `TimeZone=Asia/Kolkata`; `utils.DBNow` exists because of it). Calling
`.UnixMilli()` on a value read back from those columns is **off by 5h30m** — the
same class of fault as a naive local string with a `Z` on it, which already
produced "yesterday's work shown as today" on the Miler app.
Every timestamp leaving `/customer/*` goes through `utils.EpochMillis`, which
reinterprets the wall clock in IST first. It is correct for **both** shapes the
driver can produce (UTC-tagged-with-IST-digits, and correctly `+05:30`-tagged),
so a future column-type change cannot silently shift the tracking screen.
Asserted in `utils/epoch_test.go`.
---
## 2. The structural change: one pickup → many orders
`pickupbookings` carried exactly **one** delivery address in its own columns.
There was nowhere to put a second, so the "one visit, three orders" model in §1
of the requirements was not expressible at all.
```
Pickup booking DM-482913
├── destination 0 : Chennai, TN · 2 packages
├── destination 1 : Ernakulam, KL · 1 package
└── destination 2 : Bengaluru Urban, KA· 1 package
│ miler collects everything in ONE visit
▼
pickup-complete fans out → 3 consignments, 3 tracking numbers
DMX10482913 DMX10559120 DMX10662004
```
**The compatibility rule that makes this safe — do not break it:** destination 0
is mirrored onto the booking's flat `delivery*` columns. The Miler app, the hub
console, the routing code and the hyperlocal check all read those columns and
**none of them changed**. A booking with *no* destination rows — every
console-created express booking and every pre-existing row — produces exactly one
consignment through the same loop, byte-for-byte as before.
Single-destination is one leg, not a special case, in either direction.
---
## 3. File-by-file: what changed and why
### 3.1 New files (23)
| File | Lines | Purpose |
|---|---|---|
| `models/customer_app.go` | 241 | The 9 new tables. `BookingDestination` is the one that makes multi-destination expressible. |
| `internal/cxstage/stage.go` | 405 | **The core of §8.3.** Records one append-only stage row per transition, in the caller's transaction. Owns the slowest-order rollup and the `Release` walk-back. |
| `controllers/cxBookingView.go` | 744 | The canonical §9.3 booking object. Every read that returns a booking goes through `renderCxBooking`; `loadCxBundle` batches so a tracking poll is a fixed number of queries. |
| `controllers/cxBookingController.go` | 819 | §9.1–9.6 — create, list, detail, cancel, patch, order-by-tracking-id. |
| `controllers/cxAuthController.go` | 614 | §4 — OTP over phone/email, rotation-backed refresh, E.164 normalisation. |
| `controllers/cxCatalogueController.go` | 516 | §5 — serviceability, capacity-aware slots, limits, ETag support. |
| `controllers/cxPlacesController.go` | 350 | §6 — geocoder proxy + Redis cache. |
| `controllers/cxFareController.go` | 266 | §7 — estimate range, multi-stop uplift. |
| `controllers/cxPickupFanout.go` | 251 | Splits a booking into the journeys created at pickup-complete. Decides single-vs-fan-out; nothing downstream needs to know which. |
| `controllers/cxOpsController.go` | 160 | §11 QA stage override, double-gated. |
| `controllers/cxConsignmentHooks.go` | 100 | Maps a consignment status onto a customer stage for one order. |
| `controllers/cxDeviceController.go` | 85 | §10 push registration. |
| `controllers/cxIdentifiers.go` | 80 | Sequence-backed `DM-######` / `DMX########`. |
| `utils/epoch.go` | 103 | IST→epoch conversion. **Read §1.3.** |
| `utils/response_cx.go` | 97 | The customer envelope. Separate from `utils.OK/Fail` on purpose. |
| `internal/sms/sms.go` | 105 | The seam for an SMS gateway. **No provider is wired in.** |
| `middlewares/requestid.go` | 33 | `X-Request-Id` echo. |
| `seed_customer_app.sql` | 153 | Staging data mirroring the client mock. |
| `controllers/cxHttp_test.go` | 606 | HTTP status-code + envelope contract tests. |
| `controllers/cxCustomerApp_test.go` | 473 | Pure-logic unit tests. |
| `routes/routes_customer_test.go` | 323 | Routing + **the regression guard proving miler/console are untouched.** |
| `internal/cxstage/stage_test.go` | 126 | Stage rollup and cancellation-window tests. |
| `utils/epoch_test.go` | 116 | Timestamp conversion tests. |
### 3.2 Modified files (20)
| File | Δ | What changed | Why |
|---|---|---|---|
| `controllers/customerController.go` | +98 / −593 | PIN auth + single-destination booking handlers **deleted**; profile and locations kept, moved to the customer envelope | Replaced by the new surface; the old request shape is no longer a valid booking |
| `controllers/milerController.go` | +593 | `BookingPickupComplete` rewritten for fan-out; `BookingParcelConfirm` accepts photos + writes transactionally; stage hooks added to accept/reached/cancel | §8.3 derivation and the §1 fan-out |
| `controllers/milerAppController.go` | +410 | `MilerGetMyBookings` emits one stop per order after collection; `MilerDeliverConsignment` + `MilerStartDelivery` lookups fixed; stage hooks | **Without this, orders 2..N are undeliverable** — see §5.1 |
| `routes/routes.go` | +82 | Customer routes replaced: 19 → 28 | The new contract |
| `models/booking.go` | +60 | 10 columns on `PickupBooking`, 1 on `BookingParcel`, `Destinations` relation | Storage for the customer projection |
| `constants/constants.go` | +71 | 9 stage keys, 3 statuses, 4 actor types, rank/cancellable helpers | The wire contract; never inline these |
| `migrations/migrate.go` | +55 | 9 tables, 2 sequences, 2 indexes | Schema |
| `internal/storage/spaces.go` | +74 | `PresignGet` | Signed parcel-photo URLs |
| `dto/auth.go` | −41 | Retired customer PIN DTOs removed | Dead after the auth replacement |
| `middlewares/logger.go` | +38 | Logs `client`, `platform`, `requestid` | §3 "accept and log" |
| `middlewares/idempotency.go` | +34 | Anonymous-caller scoping | **Security fix** — see §5.4 |
| `controllers/logisticsHandoverController.go` | +24 | Consignment→booking lookup fixed; `in_transit` recorded | Fan-out correctness |
| `middlewares/city_gate.go` | +18 | Exported `PincodeInOperatingCity` | **Security fix** — see §5.3 |
| `internal/assignment/crm_assignment.go` | +17 | `assigned` recorded on auto-assign | Most B2C bookings get a rider this way |
| `controllers/booking_assignment_service.go` | +16 | `assigned` recorded on manual assign | Console assignment must reach the customer too |
| `config/config.go` | +13 | `GEOCODER_URL`, `GEOCODER_EMAIL` | §6 proxy |
| `utils/helper.go` | +13 | `GenerateTokenWithTTL` | 1h access tokens for customers only |
| `main.go` | +7 | `RequestID()` registered | §3 |
| `controllers/otpController.go` | −128 | **Deleted** | Its two routes were customer-only and are superseded |
| `CLAUDE.md` | +147 | Project memory updated | Continuity |
---
## 4. Stage derivation (§8.3)
The operational writes already existed and were already correct. What did not
exist was any record of *when* a parcel reached a stage in the vocabulary the
customer sees. A consignment status says where a parcel is **now**; it cannot say
when it got there.
| Existing Miler write | Customer stage produced |
|---|---|
| `assignMilerTx` / `commitAssignment` | `assigned` |
| `POST /miler/assignments/{id}/accept` | `on_the_way` (re-asserts `assigned`; deduped) |
| `POST /miler/bookings/{id}/reached` | `arrived` — **cancellation closes here**, recorded in the same transaction as the arrival fact so there is no window to cancel through |
| `POST /miler/bookings/{id}/parcel` | populates `verification` (weight per destination + photos) |
| `POST /miler/bookings/{id}/pickup-complete` | `picked_up`, then `order_created` **per destination** |
| `POST /miler/consignments/{id}/inward-at-hub` | `in_transit`, per order |
| `POST /miler/consignments/{id}/start-delivery` | `out_for_delivery`, per order |
| `POST /miler/consignments/{id}/deliver` | `delivered`, per order |
| `POST /miler/bookings/{id}/cancel` | **`Release`**, not cancel |
**Four rules worth knowing as the app developer:**
1. **Nothing is backfilled.** A booking that predates this work has a short
history. A short honest history beats a long invented one — the customer
cannot tell which entries were guessed.
2. **A booking rolls up from its SLOWEST order.** Taking the maximum would show
"Delivered" while one of three parcels was still at a hub.
3. **`on_the_way` comes from accept, not the GPS stream.** Deriving it from
location pings means re-deriving on every ping. Distance and ETA are still
live from the rider's Redis position.
4. **`Release` is the one place a stage moves backwards.** A miler cancelling
returns the pickup to the pool. Without walking the stage back, the customer
keeps seeing a rider card for someone who is not coming. The `assigned` /
`on_the_way` history rows stay — those things happened.
### §8.4 gaps — all five closed
| Gap in the requirements | Status |
|---|---|
| No per-destination consignment id | ✅ `bookingdestinations.consignmentid` + `trackingno` |
| No COD amount on the booking | ✅ `bookingdestinations.codamount` → `consignments.codamount`. **Per destination**, because COD is collected per delivery |
| No per-stop attribution for stages 6–8 | ✅ Every per-order stage records against its destination |
| No customer-side cancel | ✅ Server-enforced through `arrived` |
| Parcel weight/photos not exposed | ✅ `verification` block, photos as signed URLs |
---
## 5. Bugs found and fixed (not requested — found because the fan-out forced every consignment lookup to be re-read)
### 5.1 🔴 The rider could never deliver orders 2..N
`GET /miler/bookings` emitted one row per booking, keyed on
`pickupbookings.consignmentid` — a column that names only the **first** order.
On a three-destination pickup, two parcels would sit in the rider's bag with no
stop, no deliver button and no way to close them.
**Without this fix the entire multi-destination feature would have created orders
nobody could deliver.** `milerStopsForBooking` now emits one stop per order after
collection (and still exactly one visit before it — the rider goes to the door
once). Tested in `controllers/cxCustomerApp_test.go`.
### 5.2 🔴 Three endpoints broken for orders 2..N (same root cause)
- **`MilerDeliverConsignment`** returned `404 "assigned consignment not found"` —
the rider **could not complete the delivery at all**.
- **`MilerStartDelivery`** sent no push and no receiver OTP.
- **`MilerInwardConsignmentAtHub`** left the assignment open (so the rider could
not go off duty — `MilerEndDuty` refuses on an open assignment) and recorded
the leg's distance and earnings as **zero**.
All three now resolve through `cxDestinationForConsignment`.
### 5.3 🟠 `CityGateMiddleware` was a no-op for customer bookings
It sniffs the request body for `pickuppincode`. The new request shape does not
carry one — its pickup is a `title/sub/lat/lng` from the place search — so
**every customer booking sailed past the operating-city gate.** Now checked in
the handler against the pincode resolved from the coordinates; a pickup outside
an operating city returns `422 unserviceable`.
### 5.4 🔴 Idempotency key collision (introduced by me, caught in review)
Keys were namespaced by user id. On the **unauthenticated**
`POST /customer/auth/otp/verify` that id is `0` — so two customers who happened
to pick the same `Idempotency-Key` would collide and **the second would be handed
the first's access token, refresh token and customer record.**
Fixed: anonymous callers are scoped by request path + body hash. The
authenticated key format is left byte-identical, so no in-flight rider key is
orphaned at deploy.
### 5.9 🟠 Fan-out row audit: three gaps closed
Found by auditing every row-level field the rider app reads, rather than
assuming the fan-out was complete:
- **`collectionamt` was never emitted** — only `codamount`. Added alongside it,
not as a rename, so the deployed build keeps working.
- **COD fell back to 0** when a consignment row did not load, instead of to the
destination's own figure. A rider shown ₹0 collects nothing.
- **Stop ordering depended on the caller's `ORDER BY`.** Now guaranteed inside
`milerStopsForBooking`, because the ordering is the route.
Also hardened at the same time: `CreateCxBooking` now rejects an absurd
destination count **before** any database work (`cxAbsoluteMaxDestinations`),
because the district lookup built a `WHERE districtcode IN (...)` from unbounded
caller input; and `cxDefaultMaxDestinations` dropped from 5 to **1**, so a
missing configuration row resolves to the safest cap rather than the most
permissive — a gate that opens when its config is absent is not a gate.
### 5.10 🟠 Two different mistakes shared one error message
`POST /customer/bookings` answered an **empty** `destinations` array with
*"Every destination needs a serviceable state and district"* — the same string
it uses when a destination is present but missing its state or district.
The contract renders `message` verbatim, so a customer who had added nothing was
told to go and fix details on destinations they did not have. The message
described a problem they did not have and hid the one they did. Both cases also
carried the same `error.code`, so the app could not tell them apart either.
Now *"Add at least one destination"*, matching what `/customer/fare/estimate`
already said for the identical mistake. `TestEmptyDestinationsSaysAddOneNotFixTheirDetails`
asserts the two endpoints agree and that the message names the actual fix.
Found by reading the messages rather than the status codes — every one of them
is customer-facing copy, and a 400 being *correct* says nothing about whether it
is *useful*.
### 5.5 🟡 Pre-existing, flagged not fixed
`BookingPickupComplete`'s comment says a hyperlocal parcel keeps the rider
`Picked_Up`; the code sets them `Available`. This is the default path today.
Changing it alters live rider availability and the dispatch pool — **ops
decision.**
---
### 5.6 🔴 Rider earnings would have read ₹0 on every customer job (introduced by me, caught while mapping screens)
`CreateCxBooking` stored the estimate in its own new columns and did **not**
create a `BookingServiceOption` row — but three places still read one:
- `MilerDeliverConsignment` and `MilerInwardConsignmentAtHub` copy
`Estimatedprice` onto `BookingAssignment.ridercharges` when a leg closes.
- `GET /miler/earnings` sums that column.
- The admin console's Orders list renders `serviceoptions[0].estimatedprice` as
its **Price** column.
So every customer-app job a rider completed would have recorded **zero earnings**
on their Earnings screen, and the admin Orders list would have shown **"N/A"**
for the price of every customer booking. Fixed: the booking now creates a service
option carrying the midpoint of the band the customer was shown, which is what
`lookupDoormilePrice` returns everywhere else in this codebase.
### 5.7 🔴 An ops cancellation never reached the customer
`AdminCancelBooking`, `AdminBulkCancelBookings` and `AdminUpdateBookingStatus`
all cancel by writing `pickupbookings.status` directly. None of them knows the
customer projection exists — and because `customerstatus` is written as
`"active"` at booking time, the projection's empty-string fallback never fired.
**A customer whose pickup ops cancelled would have kept seeing it as active and
cancellable, indefinitely.** They would also still have received stage
notifications for it.
Fixed in two places, deliberately: the projection now treats the **operational**
status as the authority on cancellation, which covers all three paths and any
added later; and the two real cancel paths call `cxstage.Cancel` so the customer
gets a reason and the audit trail records which ops user did it. Both are no-ops
for console-created bookings.
### 5.8 🟠 An expired slot was reported as a full one
`CreateCxBooking` answered a slot whose window had already passed with
`409 "That pickup window just filled up"`. That is untrue and points the
customer at the wrong recovery — they need to re-fetch the slot list, not retry
for a place in a queue. A client that caches slots for a session and is left open
across midnight hits this on the first booking of the day.
Now `400 invalid` with *"That pickup time has passed — pick a new slot"*, which
is what the contract already maps to "Pick a pickup slot". A stale **date** is
also now caught by `CxSlotDateIsPast` before any database access, since the slot
id carries its own date — the cheapest validation in the path, and the one that
actually fires. `409` is reserved for the genuine capacity race.
Found by a reviewer noticing that §5.3 and §7.7 of this document could not both
be comfortable at once. They were right.
## 6. Tests
`go build ./...`, `go vet ./...`, `go test ./...` all pass.
**305 passing assertions/subtests**, all runnable with no database.
| File | What it proves |
|---|---|
| `controllers/cxHttp_test.go` | **Status codes and envelopes through a real router.** Every validation path answers 4xx with a machine-readable `error.code` and customer-safe English — never 500, never a router 404. Malformed JSON is 400. All 9 contract error codes map to the right status. `CxInternal` never leaks the cause. The QA override is invisible unless both gates are open. |
| `routes/routes_customer_test.go` | All **20** authenticated routes exist and return **401 without a token** (not 404, not 500). All refuse roles 1/3/4/5/6 with **403**. Pre-auth routes are reachable and answer from the handler. Retired PIN routes no longer mint a credential. **`TestMilerSurfaceIsUnchanged` / `TestConsoleSurfaceIsUnchanged` are the regression guard for §0.** |
| `controllers/cxCustomerApp_test.go` | Phone normalisation (7 spellings → one stored value, so a build change cannot create a duplicate account); the fan-out stop split; timeline dedup and per-order timestamping; `deliveredAt` waiting for the last parcel; pickup title/sub never empty. |
| `internal/cxstage/stage_test.go` | Slowest-order rollup; stage ordering as a wire contract; the cancellation window closing exactly after `arrived`. |
| `utils/epoch_test.go` | IST→epoch for both driver taggings; null stays null; slot window and day formatting. |
### What the tests do NOT prove
**No happy path is asserted.** A `200` from `CreateCxBooking` needs Postgres,
Redis and NATS. Mocking them would test the mock. Everything from the handler
inwards is still unverified — that is the integration pass in §11.
---
## 7. Answers to the requirements' §13 open questions
*(“§13” here and in §9 means section 13 of the requirements document. Section 13
of THIS document, at the bottom, is the review log.)*
**1. Payment.** v1 is cash/UPI at the door, settled outside the app. No payment
step exists and there is no payment contract to hand you. `amountPaid` is the sum
of `bookingpayments` rows the miler recorded, in whole rupees, present from
`picked_up`. Two different pots of money: the **pickup fee** is Doormile's,
collected once per visit and recorded against the first order; **COD at the
destination** is the customer's own collection, per destination. Doormile is the
carrier, never the seller. ⚠️ *No settlement/remittance flow exists — money can be
collected and recorded, nothing pays it back to the customer.*
**2. Masked calling.** Implemented as a switch: `MILER_CALL_PROXY` returns a proxy
when set, the rider's real number otherwise. **Recommendation: set it before
launch.** A customer handed a rider's personal mobile has it permanently.
⚠️ *Currently unset.*
**3. Partial pickup.** Not modelled, not guessed at. Pickup-complete is
all-or-nothing. The fan-out makes it *expressible* (legs are built per
destination), but there is no stage key, no miler endpoint and no screen.
⚠️ *Product decision needed.*
**4. Failed delivery / reattempt.** Exists operationally —
`POST /miler/consignments/{id}/skip` increments `attemptcount` and leaves the
parcel `Out_for_Delivery`. **Deliberately not mapped to a customer stage**: there
is no key for it in the nine, and an unknown key renders as `booked` on the
client. So today a failed attempt is **invisible to the customer** — their parcel
stays "Out for delivery". ⚠️ *This is the most customer-visible gap in v1. Needs a
tenth stage key + a client release.*
**5. Saved addresses / notification preferences / payment methods.** Saved
addresses **built** (capped at 10, same `title`/`sub` shape as a place search
result). Notification preferences **not built**. Payment methods **not built**,
correctly, while v1 settles at the door.
**6. Support.** Not built for the customer. `/miler/support` exists and the shape
would extend. ⚠️ *A phone number is the cheapest thing that is not a dead end.*
**7. Slot capacity semantics.** **Per zone.** A window's remaining capacity counts
live bookings whose `preferredpickupfrom` falls inside it, within 12km of the
pickup point, excluding cancelled and completed. The list read is advisory and
cached 30s; capacity is **re-checked at confirm** and a lost race returns `409`.
**The client does not need to re-fetch before confirming.**
**8. Pricing.** Reuses the existing `doormile_pricing` slab (zone × service type ×
weight band → min/max), with a distance-and-weight fallback for unpriced lanes.
Additional destinations carry a **+35% uplift each**. Weight assumed 3kg/package
until weighed. ⚠️ *The 35% is a placeholder, and there is **no breakdown when
`amountPaid` exceeds `fare.max`** — the receipt can show `amountPaid − fare.min`
as a weight adjustment, but nothing explains an overage. Product owns both.*
**9. Cancellation fee.** **Confirmed free through `arrived`.** No fee is computed,
charged or recorded anywhere in the cancel path. The UI copy is accurate. From
`picked_up` onward the server returns `409`.
**10. Retention.** **Not implemented — no policy exists to implement.** Parcel
photos stored indefinitely, PII stored indefinitely, `bookingstageevents` never
pruned. The 30-minute signed-URL TTL limits *link* lifetime, not object lifetime.
⚠️ *This is the answer most likely to matter to a regulator and it is currently
blank.*
**11. Tenancy.** **Logistics only in v1.** Customer JWTs carry `tenantid: 0` and
B2C bookings leave `pickupbookings.tenantid` null. The customer app does not
switch mode on `tenantid` the way the Miler app does. ⚠️ *Decide before revenue
reporting depends on it.*
---
## 8. Non-functional (§3.5)
- **Idempotency** — `POST /customer/bookings` and `POST /customer/auth/otp/verify`.
24h replay.
- **Caching** — serviceability `ETag`/`If-None-Match` → 304, `max-age=300`. Slots
`max-age=30`. Booking detail supports 304 (it is polled).
- **Rate limits** — OTP ≤5 per identifier per hour, ≤3 verify attempts per code,
30s resend cooldown that does **not** consume one of the five. Plus the shared
10/minute per-IP credential budget.
- **Pagination** — `?limit=&cursor=`, default 20, max 50. **Keyset**, so a new
booking landing mid-scroll cannot show the same row twice. `total` reflects the
filtered tab.
- **PII** — phone masked in SMS logs; parcel photos as signed URLs.
- **Audit** — actor type, actor id, source endpoint and timestamp on every
transition.
- **Tenancy** — every customer read is scoped by `appcustomerid` in the query
itself, never by a check afterwards.
- ⚠️ **Latency is UNMEASURED.** Reads are batched (`loadCxBundle` is a fixed query
count regardless of page size) and pricing is Redis-warmed — but p95 ≤ 400ms is
an argument, not a measurement.
---
## 9. Deliverables (§11)
| Asked for | Status |
|---|---|
| OpenAPI 3.1 spec | ✅ `openapi-customer.yaml` — 24 paths, 28 operations, validated. Covers every registered route including profile and saved addresses. |
| Postman collection | ❌ Not produced. The OpenAPI file imports directly into Postman. |
| Staging seed mirroring the mock | ✅ TN/KL/KA open, PY serviceable with no open district, Madurai/Kozhikode/Mangaluru unavailable with reasons, slot `t5` at capacity 0 |
| Test accounts + fixed staging OTP | ✅ Two accounts seeded; `CX_STAGING_OTP=1234`, refused when `ENV=production` |
| Force a booking to any stage | ✅ Double-gated; walks every intermediate stage |
| Documented error responses | ✅ Implemented verbatim; asserted in tests |
| Error injection | ❌ Not built. Happy-path states are forceable; failure states are not. |
| Written answer to the requirements' §13 | ✅ §7 above |
---
## 10. Environment variables
```bash
GEOCODER_URL=https://nominatim.openstreetmap.org # place search / reverse geocode upstream
GEOCODER_EMAIL=ops@doormile.com # Nominatim policy requires a contact
MILER_CALL_PROXY= # ⚠️ empty exposes the rider's real number
CX_STAGING_OTP=1234 # refused when ENV=production
CX_ID_SCRAMBLE_KEY= # keys the identifier permutation; a working default ships
CX_ALLOW_STAGE_OVERRIDE=false # also requires ENV != production
```
---
## 11. Before this goes to production — read this
**Blockers, stated plainly:**
1. 🔴 **No SMS provider exists.** `internal/sms` is the seam (a `Sender`
interface, a logging sink, `sms.Register()`). Until a gateway is plugged in,
**OTP codes go to the application log and nowhere else.** Real customer
sign-in is blocked on this. It is the same wall the previous end-to-end test
hit. `sms.Configured()` reports it.
2. 🔴 **No integration test has touched any of this.** The 305 passing tests cover
pure logic, routing, status codes and envelopes. Nothing has run against a
real Postgres, Redis or NATS. **A clean compile says the code is well-formed,
not that it works.**
3. 🟠 **The migration has not run against a real database.** It is additive
(`AutoMigrate` + `CREATE SEQUENCE`/`CREATE INDEX IF NOT EXISTS`) and Postgres
11+ adds nullable/defaulted columns without a table rewrite — but *should be
safe* is not *has been observed*. Run it on staging first and watch
`pickupbookings`.
4. 🟠 **`MILER_CALL_PROXY` is unset**, so the tracking screen will show riders'
personal mobile numbers.
5. 🟠 **No retention policy** for parcel photos or PII.
6. 🟡 **Failed delivery is invisible to the customer** (§7.4).
7. 🟡 **Latency unmeasured** (§8).
8. 🔴 **Multi-destination pickups must stay capped at 1 destination** until a
rider build keying on `consignmentid` ships (§0.6). The cap is a database
value, not code — verify it, do not assume it.
**Suggested deploy order:**
1. Staging: deploy, let `AutoMigrate` run, apply `seed_customer_app.sql`, set
`CX_STAGING_OTP=1234`.
2. **Confirm `maxdestinations = 1`** in `customerbookinglimits` (the seed sets
it). This is the gate that keeps the fan-out dormant until the rider app can
handle it — see §0.6. Nothing else prevents a stranded parcel.
```sql
SELECT applocationid, maxpackages, maxdestinations FROM customerbookinglimits;
```
3. Smoke the customer surface end to end — this is the pass that has never
happened.
4. **Regression-test the Miler app against staging**, specifically: pickup-complete
on a single-destination booking, deliver, inward-at-hub, and off-duty. Those
are the paths §0 items 3–5 and §5.2 touch.
5. Plug in the SMS gateway before any production customer traffic.
6. **Only after a rider build keying on `consignmentid` is live**, raise the cap
and test a multi-destination pickup — the genuinely new path:
```sql
UPDATE customerbookinglimits SET maxdestinations = 5 WHERE applocationid IS NULL;
```
---
## 11.5 Client-side prerequisites — none of §12 works until these land
§12 below is a correct set of instructions for a client that can make requests.
Today it cannot, and these are outstanding on the app side (all four are in
Appendix B of the requirements, and this document should have carried them
forward rather than assuming them done):
1. **`pubspec.yaml` has no `http` and no `shared_preferences`.** Every rule about
headers, idempotency keys and token rotation presumes a network layer that is
not yet added.
2. **`Booking.fromJson` parses roughly a third of §9.3.** See the §9.6 row above.
3. **`PickupSlot.fromJson` drops `tag`, `milersNearby` and `caption`.** The
server sends all three; the picker will not show any of them.
4. **`AppState.loadSlots` caches for the whole session and `startBooking` clears
only `districtCache`** (`lib/state/app_state.dart:89`). An app left open
across midnight books against yesterday's slot. The server now answers that
with `400` and *"That pickup time has passed — pick a new slot"* rather than
the misleading "just filled up" it used to (§5.8) — but the client fix is to
clear `slotsCache` in `startBooking` so the customer never reaches that error.
---
## 12. Quick reference for the app developer
- **Base:** `https://api.doormile.com/api/v1`
- **Auth:** `Authorization: Bearer <accessToken>` on everything except
`/customer/auth/otp/request`, `/auth/signup`, `/auth/otp/verify`,
`/auth/refresh`, and the four catalogue reads.
- **Access token 1h, refresh 60 days, rotated on every use.** A revoked refresh
token replayed kills the whole chain — always store the newest one.
- **Send `X-Client: doormile-cx/<version>+<build>` and `X-Platform: android|ios`.**
They are logged and are the only way to tell one build's failures from another's.
- **Send `Idempotency-Key` on booking create and OTP verify.**
- **Every timestamp is epoch milliseconds, UTC, integer.**
- **Every string in `message` is safe to render verbatim.** Branch on
`error.code`, never on the message text.
- **`pickup`, `slotId`, `destinations[].stateName` and `districtName` are never
null.** The rest of `details` may be absent — that is the "Not added" state, not
an error.
- **`trackingId` and `stage` on a destination are null until `order_created`.**
There is no order to track before the parcels are collected.
- **An unknown stage key renders as `booked`.** If you see that on a moving
parcel, the backend sent a key this build does not know — tell us; it means a
new stage shipped without a client release.
---
## 13. Review log
This document has been reviewed once, by a reviewer with access to the two
Flutter apps but **not** to this Go backend. That split matters and is why the
review was useful: every claim it made about the apps was checkable by them and
not by me, and every claim about this repo was checkable by me and not by them.
The findings below are recorded verbatim in substance, with what was done about
each.
**Standing caveat the reviewer stated and this document repeats:** the 305 tests,
the fan-out, the idempotency fix and the epoch conversion are unverified from
their side. Nothing in §11 has changed — this code has still never run against a
real database.
### 13.1 Findings — all upheld
| # | Finding | Status | What changed |
|---|---|---|---|
| 1 | **The rider-app action item is materially understated.** The app's identity key is `orderid` (`api_config.dart:321`, `'orderid': ref ?? id`), not `bookingid`. Both are booking-level, so all three fan-out rows collapse — in `accepted_store.dart:527` (SharedPreferences-backed, survives restart) and in `work_repository.dart:314,321` / `order_events.dart`. Keying on `consignmentid` touches four files. | **Upheld.** Consistent with what this server sends: the three rows share `bookingid` *and* `bookingreference`. | New **§0.6** replaces the one-line framing. §0 item 3 now points at it. Mitigation added: `maxdestinations` seeded at **1**, and a deploy-checklist step to verify it. |
| 2 | `destinationseq` / `destinationcount` are invisible — `pickupFromBooking` builds a fixed map, so a field the adapter does not name cannot reach the app. | **Upheld.** | Recorded in §0.6. Adding fields server-side does not help until the adapter names them. |
| 3 | **"Reuse `Booking.fromJson`" is optimistic.** It parses only `reference`, `stage`, `cancellable`, `createdAt`, `pickup`, `slotId` and four destination fields — ignoring `status`, `miler`, `deliveryAgent`, `fare`, `amountPaid`, `deliveredAt`, `history`, `verification`, `details`, per-destination `stage`, `routeKm`, `expectedDelivery`. | **Upheld, and it was avoidable** — Appendix B of the requirements said exactly this and this document glossed it. | §1.2's §9.6 row rewritten to state the gap instead of assuming the parser. |
| 4 | **Slot staleness is a trap handed to the client.** §5.3 and §7.7 could not both be comfortable: `AppState.loadSlots` caches for a session, `startBooking` clears only `districtCache` (`app_state.dart:89`), so an app open across midnight books yesterday's slot. | **Upheld — and it exposed a server defect.** The server answered `409 "That pickup window just filled up"`, which is untrue and points at the wrong recovery. | **Code fixed** (§5.8): now `400` with *"That pickup time has passed — pick a new slot"*, plus a new DB-free `CxSlotDateIsPast` guard that runs before any query. Test added. Client-side fix (clear `slotsCache`) recorded in §11.5. |
| 5 | §12's header/idempotency/rotation rules assume a client that can make requests; `pubspec.yaml` has no `http`, no `shared_preferences`. | **Upheld.** | New **§11.5** lists the client prerequisites ahead of §12. |
| 6 | `PickupSlot.fromJson` drops `tag`, `milersNearby`, `caption` — the server now sends all three. | **Upheld.** | §11.5 item 3. |
| 7 | §3.2 contains a literal unfilled placeholder: `customerController.go │ −691/+?`. | **Upheld.** | Corrected to the real churn, **+98 / −593**. |
| 8 | The counts do not reconcile: 19→28 routes, 8 exempt from auth ⇒ 20 authenticated, but §6 asserts 22. | **Upheld.** Verified: 28 registered = 8 pre-auth + **20** authenticated; the routing test has exactly 20 entries. | §6 corrected to 20. §1.1 now states the arithmetic explicitly so the three numbers reconcile on the page. |
| 9 | The OpenAPI file's 22 operations imply `/customer/profile` and `/customer/locations` are absent, though §1.1 relies on them to answer §13.5. | **Upheld, and the worst of the three** — the app developer works from the spec and would not have found saved addresses. | **6 endpoints added to the spec** with `SavedAddress` / `SavedAddressInput` schemas. Now **24 paths / 28 operations**, matching the 28 registered routes exactly. |
### 13.2 What the review confirmed as correct
Recorded because it is the half that did not change, and it is the half the app
is built against:
- camelCase throughout — no mapping layer needed on the client.
- Epoch-millis UTC — `DateTime.fromMillisecondsSinceEpoch` works as-is.
- Payload always in `data`, auth included.
- `pickup`, `slotId`, `stateName`, `districtName` non-null — the client types
these non-nullable and would throw otherwise.
- "An unknown stage key renders as `booked`" is accurate against `models.dart`
(`JourneyStage.fromKey` falls back to `booked`).
- The §1.3 IST timestamp defect is real and is the bug that already bit the
Miler app.
- §8.4's five gaps match independently.
- The two reverts — rider availability, and the 404→403 change — were the right
call.
- §11's honesty about what is unverified was called the most useful section here.
It has not been softened.
### 13.3 Reviewer's verdict, unedited in substance
> The backend contract is sound and answers the requirements; the impact
> assessment is accurate in direction but under-scopes the rider-app work, and
> its "reuse the existing parser" claims describe the client we intend to have,
> not the one in the repo. Nothing here contradicts the requirements doc.
Both criticisms are now addressed in the text: §0.6 for the rider-app scope,
§11.5 and the §9.6 row for the parser claims.
### 13.4 What still cannot be checked from either side
- Nobody has run this against a real Postgres, Redis or NATS.
- The reviewer could not verify the Go changes; this document's author could not
open the Flutter apps. **Nothing in §0.6 or §11.5 has been executed** — those
file and line references come from the review, not from this repo.
- The single integration pass in §11 remains the thing that would close both
gaps at once.