SANC-80: Driver Trip List API Endpoint#50
Conversation
jason-duong4509
requested review from
JustinTan-1,
Lujarios,
Yemyam,
burtonjong,
promatty,
tanvimahal,
themaxboucher and
wesleylui
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a DriverDashboard that manages a calendar date range and fetches driver-specific bookings via a new TRPC procedure; CalendarView gains an optional onDateChangeFunction prop that is called when the calendar's visible date range changes; backend adds getDriverTrip with validation and driver-scoped filtering. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant DD as DriverDashboard
participant CV as CalendarView
participant TRPC as TRPC Client
participant API as Bookings Router
participant DB as Database
User->>CV: change visible date range
CV->>DD: call onDateChangeFunction({startDate,endDate})
DD->>DD: set startDate/endDate state
DD->>TRPC: query getDriverTrip(startDate,endDate)
TRPC->>API: request getDriverTrip
API->>API: validate user exists & role = DRIVER
API->>API: validate ISO date formats
alt validation fails
API-->>TRPC: return error
TRPC-->>DD: error
else validation passes
API->>DB: fetch bookings where driverId=user.id and date in range
DB-->>API: bookings[]
API-->>TRPC: bookings[]
TRPC-->>DD: bookings[]
DD->>CV: render bookings
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/driver/home/page.tsx (1)
8-25: Role gate mismatch: admins can enter, but API rejects them.The page allows
Role.ADMIN, yetgetDriverTripexplicitly rejects non-drivers, which strands admins on a loading state. Align the page gate with the API (or update the API to support admin flows).✅ Suggested fix (page gate alignment)
- const session = await requireRole([Role.ADMIN, Role.DRIVER]); + const session = await requireRole([Role.DRIVER]);
🤖 Fix all issues with AI agents
In `@src/app/_components/drivercomponents/driver-dashboard.tsx`:
- Around line 21-34: Replace the placeholder single-element object
initialization for driverTrips with an empty array so calendar code doesn't
receive a partially populated Booking; specifically change the initialization of
driverTrips (currently "let driverTrips = [{}] as Booking[]") to an empty
Booking[] and keep the existing tripQuery handling
(api.bookings.getDriverTrip.useQuery and the if (tripQuery.data) assignment) so
driverTrips only contains real data from tripQuery.data.
In `@src/server/api/routers/bookings.ts`:
- Around line 281-309: The current validation only checks date formats (using
isoTimeRegex and isoTimeRegexFourDigitYears) but does not ensure the start/end
ordering, so add an explicit check after the format checks and before querying
the DB: parse or compare input.startDate and input.endDate (e.g., create Date
objects or compare normalized ISO strings) and if end < start throw a TRPCError
with code "BAD_REQUEST" and a clear message like "Invalid: End Date is before
Start Date"; keep this guard near the existing TRPCError block that validates
formats so the DB query (ctx.db.select().from(bookings) with
gte(bookings.startTime, ...) and lt(bookings.startTime, ...)) only runs for a
valid chronological range.
| let startAndEndDateErrorMessage = "Invalid: "; | ||
|
|
||
| if ( | ||
| !(isoTimeRegex.test(input.startDate) || isoTimeRegexFourDigitYears.test(input.startDate)) | ||
| ) { | ||
| startAndEndDateErrorMessage = startAndEndDateErrorMessage + "Start Date "; | ||
| } | ||
|
|
||
| if (!(isoTimeRegex.test(input.endDate) || isoTimeRegexFourDigitYears.test(input.endDate))) { | ||
| startAndEndDateErrorMessage = startAndEndDateErrorMessage + "End Date "; | ||
| } | ||
|
|
||
| if (startAndEndDateErrorMessage !== "Invalid: ") { | ||
| //Either (or both) dates failed regex check | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: startAndEndDateErrorMessage, | ||
| }); | ||
| } | ||
|
|
||
| return ctx.db | ||
| .select() | ||
| .from(bookings) | ||
| .where( | ||
| and( | ||
| eq(bookings.driverId, user.id), | ||
| gte(bookings.startTime, input.startDate), | ||
| lt(bookings.startTime, input.endDate), | ||
| ), |
There was a problem hiding this comment.
Validate start/end ordering to avoid inverted ranges.
Format checks pass even if endDate is before startDate, which yields confusing empty results. Add an explicit ordering guard.
✅ Suggested fix
if (startAndEndDateErrorMessage !== "Invalid: ") {
//Either (or both) dates failed regex check
throw new TRPCError({
code: "BAD_REQUEST",
message: startAndEndDateErrorMessage,
});
}
+
+ const start = new Date(input.startDate);
+ const end = new Date(input.endDate);
+ if (!(end > start)) {
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: "End Date must be after Start Date",
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let startAndEndDateErrorMessage = "Invalid: "; | |
| if ( | |
| !(isoTimeRegex.test(input.startDate) || isoTimeRegexFourDigitYears.test(input.startDate)) | |
| ) { | |
| startAndEndDateErrorMessage = startAndEndDateErrorMessage + "Start Date "; | |
| } | |
| if (!(isoTimeRegex.test(input.endDate) || isoTimeRegexFourDigitYears.test(input.endDate))) { | |
| startAndEndDateErrorMessage = startAndEndDateErrorMessage + "End Date "; | |
| } | |
| if (startAndEndDateErrorMessage !== "Invalid: ") { | |
| //Either (or both) dates failed regex check | |
| throw new TRPCError({ | |
| code: "BAD_REQUEST", | |
| message: startAndEndDateErrorMessage, | |
| }); | |
| } | |
| return ctx.db | |
| .select() | |
| .from(bookings) | |
| .where( | |
| and( | |
| eq(bookings.driverId, user.id), | |
| gte(bookings.startTime, input.startDate), | |
| lt(bookings.startTime, input.endDate), | |
| ), | |
| let startAndEndDateErrorMessage = "Invalid: "; | |
| if ( | |
| !(isoTimeRegex.test(input.startDate) || isoTimeRegexFourDigitYears.test(input.startDate)) | |
| ) { | |
| startAndEndDateErrorMessage = startAndEndDateErrorMessage + "Start Date "; | |
| } | |
| if (!(isoTimeRegex.test(input.endDate) || isoTimeRegexFourDigitYears.test(input.endDate))) { | |
| startAndEndDateErrorMessage = startAndEndDateErrorMessage + "End Date "; | |
| } | |
| if (startAndEndDateErrorMessage !== "Invalid: ") { | |
| //Either (or both) dates failed regex check | |
| throw new TRPCError({ | |
| code: "BAD_REQUEST", | |
| message: startAndEndDateErrorMessage, | |
| }); | |
| } | |
| const start = new Date(input.startDate); | |
| const end = new Date(input.endDate); | |
| if (!(end > start)) { | |
| throw new TRPCError({ | |
| code: "BAD_REQUEST", | |
| message: "End Date must be after Start Date", | |
| }); | |
| } | |
| return ctx.db | |
| .select() | |
| .from(bookings) | |
| .where( | |
| and( | |
| eq(bookings.driverId, user.id), | |
| gte(bookings.startTime, input.startDate), | |
| lt(bookings.startTime, input.endDate), | |
| ), |
🤖 Prompt for AI Agents
In `@src/server/api/routers/bookings.ts` around lines 281 - 309, The current
validation only checks date formats (using isoTimeRegex and
isoTimeRegexFourDigitYears) but does not ensure the start/end ordering, so add
an explicit check after the format checks and before querying the DB: parse or
compare input.startDate and input.endDate (e.g., create Date objects or compare
normalized ISO strings) and if end < start throw a TRPCError with code
"BAD_REQUEST" and a clear message like "Invalid: End Date is before Start Date";
keep this guard near the existing TRPCError block that validates formats so the
DB query (ctx.db.select().from(bookings) with gte(bookings.startTime, ...) and
lt(bookings.startTime, ...)) only runs for a valid chronological range.
| if (user.role !== Role.DRIVER) { | ||
| throw new TRPCError({ | ||
| code: "UNAUTHORIZED", | ||
| message: "User is not a driver", | ||
| }); | ||
| } |
There was a problem hiding this comment.
what if on the admin page we want to be able to see bookings for a specific driver? this endpoint wouldn't work then, would it?
There was a problem hiding this comment.
I guess we could just filter by driver id on the client side, but I think all endpoints should be accessible by admins regardless. Let me know what you think
There was a problem hiding this comment.
As of right now, the endpoint is designed to return bookings that are assigned to the user who called it. If we extended this to admins, the endpoint would only return bookings assigned to the admin. I do not think this would be useful in this context
Achieving functionality where the admin could use the endpoint to filter bookings for a specific driver would require modification of the endpoint
|
also resolve merge conflicts before we review please |
…ist-API-Endpoint # Conflicts: # src/app/driver/home/page.tsx # src/server/api/routers/bookings.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/server/api/routers/bookings.ts`:
- Around line 298-303: Change the TRPCError thrown when the role check fails to
use code "FORBIDDEN" instead of "UNAUTHORIZED": locate the role check that
compares user.role to Role.DRIVER (the block that currently throws new
TRPCError({ code: "UNAUTHORIZED", message: "User is not a driver" })) and update
the error code to "FORBIDDEN" so the handler returns a 403 for authenticated
users without the driver role; keep the existing message or adjust wording if
desired.
- Around line 287-296: The lookup that fetches the user via
ctx.db.query.user.findFirst is redundant — use the session instead; remove the
DB query and any NOT_FOUND error, read role from ctx.session.user.role and id
from ctx.session.user.id (or assign to userId) and update any subsequent query
that relied on the fetched user to use userId/ctx.session.user.role directly
(e.g., replace uses of the local user variable in this handler with
ctx.session.user.id or a userId constant and ctx.session.user.role).
🧹 Nitpick comments (2)
src/app/driver/home/page.tsx (1)
3-3: Nit:asyncis unnecessary now.The function no longer
awaits anything. Removingasynckeeps the signature honest about what this component does.-export default async function DriverHome() { +export default function DriverHome() {src/server/api/routers/bookings.ts (1)
325-336: Query looks correct — minor note on naming.The procedure is named
getDriverTrip(singular) but returns an array of bookings. ConsidergetDriverTripsfor clarity.
- Changed the error code when calling a driver-only endpoint and the user is not a driver. Changed from 401 to 403 to improve clarity
burtonjong
left a comment
burtonjong
left a comment
There was a problem hiding this comment.
k i think its fine then
3 participants
Created an endpoint that returns all trips for a driver on a given date range. Also connected the endpoint to the front-end, allowing the information to display on the calendar
The video below shows the usage of the new endpoint when connected to the front-end. It also shows filtering by driver ID through assigning a trip to another driver in the DB
Each time a new date is viewed, the endpoint is queried for trips on that date
Video.mp4
Summary by CodeRabbit
New Features
Bug Fixes / Improvements