feat(m5): Live Location Tracking & Ride Completion#17
Merged
codeRisshi25 merged 4 commits intomainfrom Mar 7, 2026
Merged
Conversation
- Throttled driver location broadcast (SETNX 3s interval)
- Always updates Redis GEO position
- Broadcasts to ride:{tripId} room with remaining distance + ETA
- PostGIS ST_DistanceSphere for remaining distance calculation
- ETA based on 30 km/h city average speed
- COMPLETE lifecycle action (ride-lifecycle worker)
- PostGIS ST_DistanceSphere for trip distance (meters → km)
- Fare formula: max(50, 50 + distanceKm × 12)
- Updates Trip: COMPLETED, fare, distance, completedAt
- Removes driver from drivers:busy set
- Cleans up Redis (lock, OTP keys)
- Notifies ride:{tripId} + rider personal room via ride:completed
- Fare constants (packages/common/constants/fare.ts)
- BASE_FARE: 50, PER_KM_RATE: 12, MIN_FARE: 50
- CITY_AVG_SPEED_KMH: 30, LOCATION_THROTTLE_SECONDS: 3
- History schemas (packages/common/schemas/history.schema.ts)
- rideHistoryQuerySchema (page/limit with coerce)
- tripIdParamSchema (UUID validation)
- New REST endpoints:
POST /rides/:tripId/complete (driver)
GET /rides/history (paginated, role-aware)
GET /rides/:tripId (owner-only detail)
GET /user/driver/stats (aggregate: rides, earnings, distance)
GET /user/driver/current-ride (ACCEPTED/STARTED trip + rider info)
GET /user/rider/current-ride (active trip + driver info + Redis GEO location)
Tests: 76 passed (63 api-gateway + 13 ride-worker)
TypeScript: api-gateway OK | ride-worker OK | common OK
Closes #10
📝 WalkthroughWalkthroughAdds M5 features: throttled live driver location streaming (Redis GEO + ETA via PostGIS), ride completion workflow with distance/fare calculation and notifications, paginated ride history/detail endpoints, and driver/rider stats/current-ride endpoints across routes, services, sockets, and the ride-worker. Changes
Sequence DiagramssequenceDiagram
actor Driver
participant Handler as Socket Handler
participant Redis
participant DB as Database (PostGIS)
participant Rider
Driver->>Handler: emit driver:location-update {lat,lng,heading?,speed?}
Handler->>Redis: GEOADD drivers:active (driverId lng lat)
Handler->>Redis: SETNX driver:location-throttle:{driverId} TTL=3s
alt Throttled
Handler-->>Handler: skip broadcast
else Not throttled
Handler->>DB: Query STARTED trip for driver
alt Trip found
Handler->>DB: ST_DistanceSphere(current_pos, dropoff) -> meters
DB-->>Handler: remaining_meters
Handler->>Handler: calc remainingKm & etaMinutes
Handler->>Rider: emit ride:driver-location {lng,lat,heading,speed,remainingDistanceKm,etaMinutes,timestamp}
else No active trip
Handler-->>Handler: skip broadcast
end
end
sequenceDiagram
actor Driver
participant API as API Gateway
participant Service as Ride Service
participant Queue as Job Queue
participant Worker as Ride Worker
participant DB as Database (PostGIS)
participant Redis
Driver->>API: POST /rides/:tripId/complete
API->>Service: completeRide(driverId, tripId)
Service->>DB: validate trip status == STARTED && driver assigned
alt Valid
Service->>Queue: enqueue job {action: COMPLETE, tripId}
Queue-->>Worker: deliver job
Worker->>DB: SELECT ST_DistanceSphere(pickup, dropoff) -> meters
DB-->>Worker: distance_meters
Worker->>Worker: compute fare = max(MIN_FARE, BASE_FARE + km * PER_KM_RATE)
Worker->>DB: update Trip {status:COMPLETED, fare, distance, completedAt}
Worker->>Redis: SREM drivers:busy driverId
Worker->>Redis: DEL ride:lock:{tripId}, OTP keys
Worker->>API: emit ride:completed to ride:{tripId} and rider room
else Invalid
Service-->>API: return error 400/403
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/api-gateway/src/sockets/handlers/driver.ts (1)
73-89: Consider logging PostGIS query failures for debugging.The empty catch block silently swallows errors, which is acceptable for graceful degradation but makes debugging difficult when the query fails unexpectedly.
🔧 Suggested fix
- } catch { - // PostGIS query might fail — still broadcast location without ETA + } catch (distErr) { + logger.warn({ err: distErr, tripId: activeTrip.id }, 'PostGIS distance query failed, broadcasting without ETA'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/sockets/handlers/driver.ts` around lines 73 - 89, The catch block that wraps the PostGIS prisma.$queryRaw call currently swallows errors silently; update it to log the caught error (including context such as activeTrip.id and the query purpose) using the existing logger (or processLogger) so failures are visible while preserving graceful degradation of remainingDistanceKm and etaMinutes; specifically, in the try/catch around prisma.$queryRaw in the handler (where remainingDistanceKm and etaMinutes are computed, referencing activeTrip.id and CITY_AVG_SPEED_KMH), catch the error as e and call the logger.error with a concise message and the error object before continuing.apps/api-gateway/src/routes/ride.routes.ts (1)
58-66: Fragile HTTP status code determination based on message content.The status code logic on line 62 relies on checking if the error message contains the word "access" to return 403 vs 400. This is brittle and could break if the message wording changes.
Consider returning a status code or error type from the service layer instead:
♻️ Suggested approach
In
getRideDetail, return an error code alongside the message:// In ride.service.ts return { success: false, message: 'You do not have access...', code: 'FORBIDDEN' };Then in the route:
- return res.status(result.success ? 200 : (result.message.includes('access') ? 403 : 400)).json(result); + const status = result.success ? 200 : (result.code === 'FORBIDDEN' ? 403 : 400); + return res.status(status).json(result);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/routes/ride.routes.ts` around lines 58 - 66, The route handler in ride.routes.ts currently infers HTTP status by checking for the word "access" in the getRideDetail message; update getRideDetail (in the ride service) to return a structured error indicator (e.g., { success: boolean, message: string, code?: 'FORBIDDEN' | 'NOT_FOUND' | 'BAD_REQUEST' }) and then change the route handler for GET '/:tripId' to map that returned code to an explicit HTTP status (e.g., FORBIDDEN -> 403, NOT_FOUND -> 404, BAD_REQUEST -> 400) while keeping a 500 fallback for thrown errors; ensure the handler still validates req.user and uses result.success to decide success responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-gateway/src/routes/user.routes.ts`:
- Around line 173-180: The route handler userRouter.get('/rider/current-ride',
authenticate, ...) is missing role-based authorization: add the authorize
middleware for riders (authorize('rider')) before the async handler so only
users with the rider role reach getRiderCurrentRide; alternatively, if drivers
should also access it, use authorize('driver' | 'rider'). Ensure you import/use
the existing authorize middleware and keep the authenticate middleware first,
e.g., authenticate, authorize('rider'), async (req: Request, res: Response) => {
... } to reject non-rider users at the middleware boundary.
In `@apps/api-gateway/src/services/ride.service.ts`:
- Around line 447-476: The query and response currently expose t.otp to drivers;
update the logic in getRideDetail (and similarly in getDriverCurrentRide) to
avoid returning the OTP to driver-accessible APIs: remove t.otp from the SELECT
or delete tripData.otp before returning for non-rider users, and only include
OTP in the response when isRider is true (or after a rider ownership check).
Ensure the SELECT/response uses ST_AsText(...) and other fields unchanged but
excludes OTP for driver or public/owner-with-driver flows.
- Around line 603-610: The current redis.geopos call in the ride service can
return [null,null] for a missing member and the existing check (if pos &&
pos[0]) will still treat that as truthy and set tripData.driverLocation to NaN
values; update the logic around redis.geopos('drivers:active',
driverRecord.userId) to verify that pos[0] exists and both pos[0][0] and
pos[0][1] are non-null (e.g. !== null/undefined) before calling parseFloat, and
only assign tripData.driverLocation when both coordinates are present and valid;
keep existing keys (driverRecord.userId, drivers:active,
tripData.driverLocation) to locate the change.
In `@learn/m5-tracking-completion.md`:
- Around line 15-38: The fenced ASCII-diagram blocks (for example the block
starting with "Driver GPS update (every ~1s)" and the other similar diagrams)
are missing a fence language which triggers MD040; update each triple-backtick
fence that contains those ASCII diagrams to include a language label (use
"text") — i.e., change ``` to ```text for the diagram blocks (including the
other two diagrams referenced) so markdownlint stops flagging them.
---
Nitpick comments:
In `@apps/api-gateway/src/routes/ride.routes.ts`:
- Around line 58-66: The route handler in ride.routes.ts currently infers HTTP
status by checking for the word "access" in the getRideDetail message; update
getRideDetail (in the ride service) to return a structured error indicator
(e.g., { success: boolean, message: string, code?: 'FORBIDDEN' | 'NOT_FOUND' |
'BAD_REQUEST' }) and then change the route handler for GET '/:tripId' to map
that returned code to an explicit HTTP status (e.g., FORBIDDEN -> 403, NOT_FOUND
-> 404, BAD_REQUEST -> 400) while keeping a 500 fallback for thrown errors;
ensure the handler still validates req.user and uses result.success to decide
success responses.
In `@apps/api-gateway/src/sockets/handlers/driver.ts`:
- Around line 73-89: The catch block that wraps the PostGIS prisma.$queryRaw
call currently swallows errors silently; update it to log the caught error
(including context such as activeTrip.id and the query purpose) using the
existing logger (or processLogger) so failures are visible while preserving
graceful degradation of remainingDistanceKm and etaMinutes; specifically, in the
try/catch around prisma.$queryRaw in the handler (where remainingDistanceKm and
etaMinutes are computed, referencing activeTrip.id and CITY_AVG_SPEED_KMH),
catch the error as e and call the logger.error with a concise message and the
error object before continuing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 736d4aa0-dc68-4be9-9e75-a260e9d60d6f
📒 Files selected for processing (12)
apps/api-gateway/src/routes/ride.routes.tsapps/api-gateway/src/routes/user.routes.tsapps/api-gateway/src/services/__tests__/ride.service.test.tsapps/api-gateway/src/services/ride.service.tsapps/api-gateway/src/sockets/__tests__/driver-handler.test.tsapps/api-gateway/src/sockets/handlers/driver.tsapps/ride-worker/src/workers/ride-lifecycle.worker.tslearn/m5-tracking-completion.mdpackages/common/constants/fare.tspackages/common/index.tspackages/common/schemas/history.schema.tspackages/common/schemas/index.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
29-47: Optional: Satisfy markdown linter for fenced code blocks.The markdownlint tool flags the fenced code blocks at lines 29 and 53 for missing language specifications. While these are intentional ASCII diagrams (not code), you can optionally add
textas the language identifier to satisfy the linter.📝 Optional fix for line 29
-``` +```text Rider requests ride📝 Optional fix for line 53
-``` +```text ┌──────────────────────────────────────────────────────┐Also applies to: 53-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 29 - 47, The fenced ASCII diagrams in README.md are missing a language specifier; update each triple-backtick fence that contains the ride flow diagram (the block starting at the Rider requests ride diagram) and the larger ASCII box diagram (lines around 53-73) to use ```text so markdownlint stops flagging them; locate the two fenced code blocks in README.md and add the language identifier "text" immediately after the opening backticks for both blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@README.md`:
- Around line 29-47: The fenced ASCII diagrams in README.md are missing a
language specifier; update each triple-backtick fence that contains the ride
flow diagram (the block starting at the Rider requests ride diagram) and the
larger ASCII box diagram (lines around 53-73) to use ```text so markdownlint
stops flagging them; locate the two fenced code blocks in README.md and add the
language identifier "text" immediately after the opening backticks for both
blocks.
- GEOPOS null check: verify both coords are non-null before parsing
(Redis returns [null,null] for missing members → was producing NaN)
- handleComplete idempotent: skip validation + DB update on retry
when trip is already COMPLETED, still run cleanup/notifications
- Markdown fence labels: add 'text' to ASCII diagram code blocks
- OTP security: strip OTP from driver-facing APIs (already applied)
- authorize('rider'): rider current-ride route (already applied)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api-gateway/src/services/ride.service.ts (1)
390-418: Prefer parameterized queries over$queryRawUnsafewith string interpolation.Using
$queryRawUnsafewith string-interpolated values (lines 393, 398, 417) is a security anti-pattern. While therider.id/driver.idare UUIDs from the database andlimit/offsetare numbers, this approach is fragile if input validation changes or is bypassed.Refactor to use conditional
$queryRawblocks with proper template literal parameterization:♻️ Suggested refactor using parameterized queries
- let whereClause = ''; - let countClause = ''; - - if (role === 'rider') { - const rider = await prisma.rider.findUnique({ where: { userId }, select: { id: true } }); - if (!rider) return { success: false, message: 'Rider not found' }; - whereClause = `WHERE t."riderId" = '${rider.id}'`; - countClause = whereClause; - } else { - const driver = await prisma.driver.findUnique({ where: { userId }, select: { id: true } }); - if (!driver) return { success: false, message: 'Driver not found' }; - whereClause = `WHERE t."driverId" = '${driver.id}'`; - countClause = whereClause; - } - const offset = (page - 1) * limit; - const rides = await prisma.$queryRawUnsafe<Record<string, unknown>[]>(` + if (role === 'rider') { + const rider = await prisma.rider.findUnique({ where: { userId }, select: { id: true } }); + if (!rider) return { success: false, message: 'Rider not found' }; + + const rides = await prisma.$queryRaw<Record<string, unknown>[]>` SELECT t.id, t.status, ST_AsText(t."pickupLocation") as "pickupLocation", ST_AsText(t."dropoffLocation") as "dropoffLocation", t.fare, t.distance, t."createdAt", t."completedAt" FROM "Trip" t - ${whereClause} + WHERE t."riderId" = ${rider.id} ORDER BY t."createdAt" DESC LIMIT ${limit} OFFSET ${offset} - `); + `; + + const countResult = await prisma.$queryRaw<{ count: bigint }[]>` + SELECT COUNT(*) as count FROM "Trip" t WHERE t."riderId" = ${rider.id} + `; + const total = Number(countResult[0]?.count ?? 0); + + return { + success: true, + message: 'Ride history retrieved', + data: { rides, pagination: { page, limit, total, totalPages: Math.ceil(total / limit) } }, + }; + } + + // Driver case + const driver = await prisma.driver.findUnique({ where: { userId }, select: { id: true } }); + if (!driver) return { success: false, message: 'Driver not found' }; + + const rides = await prisma.$queryRaw<Record<string, unknown>[]>` + SELECT ... WHERE t."driverId" = ${driver.id} ... + `; + // ... similar pattern🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-gateway/src/services/ride.service.ts` around lines 390 - 418, The current code in ride.service.ts uses prisma.$queryRawUnsafe with string-interpolated whereClause, limit and offset (see variables whereClause, countClause, limit, offset and call to prisma.$queryRawUnsafe), which is unsafe; refactor to use parameterized prisma.$queryRaw and pass values as parameters instead of embedding them in the SQL string: build the SQL with a conditional WHERE clause placeholder (e.g., include WHERE t."riderId" = $1 or WHERE t."driverId" = $1 only when needed) and supply the corresponding id as a parameter, and pass limit/offset as parameters too; do the same for the count query (countClause) so both queries use prisma.$queryRaw with parameter arrays rather than string interpolation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api-gateway/src/services/ride.service.ts`:
- Around line 390-418: The current code in ride.service.ts uses
prisma.$queryRawUnsafe with string-interpolated whereClause, limit and offset
(see variables whereClause, countClause, limit, offset and call to
prisma.$queryRawUnsafe), which is unsafe; refactor to use parameterized
prisma.$queryRaw and pass values as parameters instead of embedding them in the
SQL string: build the SQL with a conditional WHERE clause placeholder (e.g.,
include WHERE t."riderId" = $1 or WHERE t."driverId" = $1 only when needed) and
supply the corresponding id as a parameter, and pass limit/offset as parameters
too; do the same for the count query (countClause) so both queries use
prisma.$queryRaw with parameter arrays rather than string interpolation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d4c8392-bc8f-440e-a3ce-12cd6551e0aa
📒 Files selected for processing (4)
apps/api-gateway/src/routes/user.routes.tsapps/api-gateway/src/services/ride.service.tsapps/ride-worker/src/workers/ride-lifecycle.worker.tslearn/m5-tracking-completion.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ride-worker/src/workers/ride-lifecycle.worker.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Implements Issue #10 — live location tracking with throttling, ride completion with PostGIS fare calculation, and ride history/dashboard endpoints. This is the final milestone that completes the core ride-sharing flow.
What Changed
packages/common
[NEW] constants/fare.ts
driver:location-throttle:[NEW] schemas/history.schema.ts
rideHistoryQuerySchema— validatespage/limitquery params withz.coercetripIdParamSchema— validates:tripIdUUID route paramride-worker — COMPLETE Action
Replaced M4 placeholder with full implementation in
ride-lifecycle.worker.ts:ST_DistanceSphere(pickupLocation, dropoffLocation)→ distance in metersfare = max(MIN_FARE, BASE_FARE + distanceKm × PER_KM_RATE)→ round to 2 decimalsCOMPLETED, fare, distance, completedAtdrivers:busyRedis setride:completedtoride:{tripId}room + rider personal roomapi-gateway — Throttled Location Broadcast
Enhanced
driver:location-updatesocket handler:SETNXthrottle key with 3s TTL → skip broadcast if throttledSTARTEDtripST_DistanceSphere(currentPos, dropoff)→ remaining distanceremainingKm / 30 * 60minutesride:driver-locationtoride:{tripId}room:api-gateway — New REST Endpoints
/rides/:tripId/complete/rides/history?page=1&limit=20ST_AsTextfor geometry)/rides/:tripId/user/driver/stats/user/driver/current-ride/user/rider/current-rideService Methods Added
ride.service.ts:completeRide,getRideHistory,getRideDetail,getDriverStats,getDriverCurrentRide,getRiderCurrentRideTests
76 tests total (all passing):
Complete Ride Flow (M1→M5)
Depends On
Closes #10
Summary by CodeRabbit
New Features
Tests
Documentation