Sanc 28 implement driver trip assignment system #56
Conversation
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
Lujarios
requested review from
JustinTan-1,
Yemyam,
burtonjong,
jason-duong4509,
promatty,
tanvimahal,
themaxboucher and
wesleylui
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces driver availability validation and travel-time checking for bookings. It adds Google Maps Distance Matrix integration for calculating travel times, new utility functions for datetime rounding, constants for driver assignment configuration, and significantly expands the booking debug page with validation logic and UI enhancements. Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as BookingDebugPage
participant API as Bookings Router
participant DB as Database
participant Maps as Google Maps API
User->>Frontend: Submit booking with driver selection
Frontend->>API: Call isDriverAvailable (driverId, startTime, endTime)
API->>DB: findOverlappingBooking (check conflicts)
DB-->>API: Return overlapping booking (if any)
alt Overlap detected
API-->>Frontend: Return {available: false}
Frontend-->>User: Show error alert
else No overlap
API->>DB: Query adjacent bookings
DB-->>API: Return previous/next bookings
API->>Maps: getTravelTimeMinutes (prev booking → new booking)
Maps-->>API: Return travel duration in minutes
API->>Maps: getTravelTimeMinutes (new booking → next booking)
Maps-->>API: Return travel duration in minutes
API->>API: validateTravelBetweenAdjacentBookings
alt Travel validation fails
API-->>Frontend: Return {available: false, reason}
Frontend-->>User: Show availability error
else Validation succeeds
API-->>Frontend: Return {available: true}
Frontend-->>User: Enable booking submission
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 5
🤖 Fix all issues with AI agents
In `@src/app/debug/bookings/page.tsx`:
- Around line 322-334: The onChange handler for DateInput uses
v.toISOString().slice(0,10) when v is a Date which converts to UTC and causes
off-by-one days in UTC+ timezones; update the branch handling v as a Date to
build the local date instead (use v.getFullYear(), v.getMonth(), v.getDate() or
construct new Date(v.getFullYear(), v.getMonth(), v.getDate())) and then call
setSelectedDay with that local-date value; keep the string branch parsing logic
for string inputs unchanged and ensure you still null-check v and call
setSelectedDay(null) when v is null (locations: the onChange handler, variable
v, and setSelectedDay).
In `@src/lib/datetime.ts`:
- Around line 3-14: Update the docstring in src/lib/datetime.ts for the rounding
function (the top comment block describing "Rounds a date up to the nearest time
slot increment") to correct the example showing 14:45 → 15:00; change it to
14:45 → 14:45 so the example accurately reflects that inputs already on the
increment boundary are unchanged (e.g., 14:07 → 14:15, 14:15 → 14:15, 14:16 →
14:30, 14:45 → 14:45).
In `@src/lib/google-maps.ts`:
- Around line 34-42: The JSDoc for the Google Maps travel-time function is
inaccurate: only the missing-API-key check actually throws, while HTTP/API
errors are caught and the function returns null; update the docblock for the
function (the "Calculates travel time between two addresses..." comment) to
remove or narrow the `@throws` description to only the missing API key case
(mention the specific missing-key check) and clarify in `@returns` that API
request failures result in a null return rather than an exception; reference the
function name/description in the file so you update the correct comment.
In `@src/server/api/routers/bookings.ts`:
- Around line 514-528: The current guard only calls validateDriverForSlot when
updatesToApply.driverId is present, so changes to startTime/endTime/addresses on
a booking with an existing driver skip re-validation; change the condition to
also run validation when there is an existing.driverId and any of
startTime/endTime/pickupAddress/destinationAddress are being changed. Inside
that block use driverId = updatesToApply.driverId ?? existing.driverId and
compute proposedStart/proposedEnd/proposedPickup/proposedDest the same way, then
await validateDriverForSlot(ctx, { driverId, startTime: proposedStart, endTime:
proposedEnd, pickupAddress: proposedPickup, destinationAddress: proposedDest,
excludeBookingId: id }); ensure the condition references
updatesToApply.startTime, updatesToApply.endTime, updatesToApply.pickupAddress,
and updatesToApply.destinationAddress and existing.driverId.
- Around line 222-234: listDrivers currently returns driver emails to any
authenticated caller; restrict access by checking the caller's role before
selecting the email column. In the listDrivers handler (protectedProcedure ->
listDrivers) either (A) enforce admin-only access by using your admin middleware
or checking ctx.session.user.role === "admin" and aborting with an unauthorized
error if not admin, or (B) keep the route public to authenticated users but
remove the email from the select for non-admins (select only id and name when
ctx.session.user.role !== "admin"). Update the select projection (the current
select of user.id, user.name, user.email) and the access check in listDrivers
accordingly so emails are only returned to admins.
🧹 Nitpick comments (5)
.gitignore (1)
12-13: Redundant ignore pattern.Since
drizzleon line 13 ignores the entire directory, thedrizzle/metaentry on line 12 is redundant and can be removed.♻️ Proposed cleanup
-drizzle/meta drizzlesrc/app/debug/bookings/page.tsx (3)
83-83:endPickerValuestate is set but never read.
endPickerValueis written on lines 184 and 225 but never consumed in the render output. The "Ends" field (line 442) reads fromestimatedEndQuery.data?.estimatedEndTimedirectly. Remove the unused state to avoid confusion.🧹 Remove unused state
- const [endPickerValue, setEndPickerValue] = useState<string | null>(null);And remove references on lines 184 and 225:
useEffect(() => { if (estimatedEndQuery.data) { - setEndPickerValue(estimatedEndQuery.data.estimatedEndTime); form.setFieldValue("end", estimatedEndQuery.data.estimatedEndTime); } }, [estimatedEndQuery.data, form.setFieldValue]);form.reset(); // ... reset selectedDay / startTimeOfDay ... - setEndPickerValue(null);
419-437: Remove commented-out code.Lines 423–426 contain commented-out JSX for location display. If this is intentionally omitted, remove it to keep the debug output clean.
364-407: Consider extracting the IIFE into a named component or variable.The inline IIFE
(() => { ... })()for rendering the driver bookings table works but reduces readability. Extracting it to a small helper or auseMemo-derived variable would improve clarity.src/server/api/routers/bookings.ts (1)
277-308:getEstimatedEndTimesilently uses a fallback when the API call fails — consider surfacing this.When
getTravelTimeMinutesreturnsnull, the fallback of 15 minutes is used without informing the client. If the actual travel time is significantly longer, the estimated end time could be too short, leading to booking conflicts. Consider returning a flag (e.g.,usedFallback: true) so the UI can display a warning.💡 Example: surface fallback usage
.query(async ({ input }) => { - const drivingTimeMinutes = - (await getTravelTimeMinutes(input.pickupAddress, input.destinationAddress)) ?? - FALLBACK_TRAVEL_MINUTES; + const apiResult = await getTravelTimeMinutes(input.pickupAddress, input.destinationAddress); + const usedFallback = apiResult === null; + const drivingTimeMinutes = apiResult ?? FALLBACK_TRAVEL_MINUTES; // ... existing calculation ... return { location1: input.pickupAddress, location2: input.destinationAddress, drivingTimeMinutes, totalBookingMinutes, startTime: input.startTime, estimatedEndTime, + usedFallback, }; }),
Comment on lines
+322
to
+334
| onChange={(v) => { | ||
| if (v == null) { | ||
| setSelectedDay(null); | ||
| return; | ||
| } | ||
| // Parse as local date to avoid UTC-off-by-one (e.g. "2025-02-15" -> Feb 14 in PST) | ||
| const str = typeof v === "string" ? v : (v as Date).toISOString().slice(0, 10); | ||
| const parts = str.split("-").map(Number); | ||
| const [y, m, day] = parts; | ||
| if (y != null && m != null && day != null) { | ||
| setSelectedDay(new Date(y, m - 1, day)); | ||
| } | ||
| }} |
There was a problem hiding this comment.
DateInput onChange can produce an off-by-one date for UTC+ timezones.
When v is a Date, v.toISOString().slice(0, 10) converts to UTC first. For a user in UTC+X, midnight local on Feb 15 becomes Feb 14 in UTC, so the extracted date string is one day behind. Use local accessors instead to stay consistent with the local-date intent.
🐛 Suggested fix
onChange={(v) => {
if (v == null) {
setSelectedDay(null);
return;
}
- // Parse as local date to avoid UTC-off-by-one (e.g. "2025-02-15" -> Feb 14 in PST)
- const str = typeof v === "string" ? v : (v as Date).toISOString().slice(0, 10);
- const parts = str.split("-").map(Number);
- const [y, m, day] = parts;
- if (y != null && m != null && day != null) {
- setSelectedDay(new Date(y, m - 1, day));
+ if (v instanceof Date) {
+ // Use local accessors to avoid UTC off-by-one
+ setSelectedDay(new Date(v.getFullYear(), v.getMonth(), v.getDate()));
+ } else {
+ const parts = (v as string).split("-").map(Number);
+ const [y, m, day] = parts;
+ if (y != null && m != null && day != null) {
+ setSelectedDay(new Date(y, m - 1, day));
+ }
}
}}🤖 Prompt for AI Agents
In `@src/app/debug/bookings/page.tsx` around lines 322 - 334, The onChange handler
for DateInput uses v.toISOString().slice(0,10) when v is a Date which converts
to UTC and causes off-by-one days in UTC+ timezones; update the branch handling
v as a Date to build the local date instead (use v.getFullYear(), v.getMonth(),
v.getDate() or construct new Date(v.getFullYear(), v.getMonth(), v.getDate()))
and then call setSelectedDay with that local-date value; keep the string branch
parsing logic for string inputs unchanged and ensure you still null-check v and
call setSelectedDay(null) when v is null (locations: the onChange handler,
variable v, and setSelectedDay).
Comment on lines
+3
to
+14
| /** | ||
| * Rounds a date up to the nearest time slot increment (default: 15 minutes) | ||
| * Examples: | ||
| * - 14:07 → 14:15 | ||
| * - 14:15 → 14:15 | ||
| * - 14:16 → 14:30 | ||
| * - 14:45 → 15:00 | ||
| * | ||
| * @param date - The date to round up | ||
| * @param incrementMinutes - The increment in minutes (default: 15) | ||
| * @returns A new Date rounded up to the nearest increment | ||
| */ |
There was a problem hiding this comment.
Incorrect docstring example: 14:45 → 15:00 should be 14:45 → 14:45.
Math.ceil(45 / 15) * 15 = 45, so an input already on a 15-minute boundary is returned unchanged. The example on line 9 is misleading.
📝 Fix the docstring example
* Examples:
* - 14:07 → 14:15
* - 14:15 → 14:15
* - 14:16 → 14:30
- * - 14:45 → 15:00
+ * - 14:45 → 14:45
+ * - 14:46 → 15:00📝 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.
Suggested change
| /** | |
| * Rounds a date up to the nearest time slot increment (default: 15 minutes) | |
| * Examples: | |
| * - 14:07 → 14:15 | |
| * - 14:15 → 14:15 | |
| * - 14:16 → 14:30 | |
| * - 14:45 → 15:00 | |
| * | |
| * @param date - The date to round up | |
| * @param incrementMinutes - The increment in minutes (default: 15) | |
| * @returns A new Date rounded up to the nearest increment | |
| */ | |
| /** | |
| * Rounds a date up to the nearest time slot increment (default: 15 minutes) | |
| * Examples: | |
| * - 14:07 → 14:15 | |
| * - 14:15 → 14:15 | |
| * - 14:16 → 14:30 | |
| * - 14:45 → 14:45 | |
| * - 14:46 → 15:00 | |
| * | |
| * `@param` date - The date to round up | |
| * `@param` incrementMinutes - The increment in minutes (default: 15) | |
| * `@returns` A new Date rounded up to the nearest increment | |
| */ |
🤖 Prompt for AI Agents
In `@src/lib/datetime.ts` around lines 3 - 14, Update the docstring in
src/lib/datetime.ts for the rounding function (the top comment block describing
"Rounds a date up to the nearest time slot increment") to correct the example
showing 14:45 → 15:00; change it to 14:45 → 14:45 so the example accurately
reflects that inputs already on the increment boundary are unchanged (e.g.,
14:07 → 14:15, 14:15 → 14:15, 14:16 → 14:30, 14:45 → 14:45).
Comment on lines
+34
to
+42
| /** | ||
| * Calculates travel time between two addresses using Google Maps Distance Matrix API | ||
| * | ||
| * @param originAddress - Origin address as a string | ||
| * @param destinationAddress - Destination address as a string | ||
| * @param options - Optional travel mode and unit settings | ||
| * @returns Travel duration in minutes, or null if calculation fails | ||
| * @throws Error if API key is missing or API request fails critically | ||
| */ |
There was a problem hiding this comment.
Docstring is misleading — HTTP errors do not propagate to callers.
The @throws annotation claims the function throws when "API request fails critically" (line 41), but the throw on line 72 is inside the try block and is caught by the catch on line 101, which returns null. Only the missing-API-key check (line 50–52) actually throws. Update the JSDoc to reflect reality.
📝 Suggested docstring fix
* `@returns` Travel duration in minutes, or null if calculation fails
- * `@throws` Error if API key is missing or API request fails critically
+ * `@throws` Error if GOOGLE_MAPS_API_KEY is not configured
*/📝 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.
Suggested change
| /** | |
| * Calculates travel time between two addresses using Google Maps Distance Matrix API | |
| * | |
| * @param originAddress - Origin address as a string | |
| * @param destinationAddress - Destination address as a string | |
| * @param options - Optional travel mode and unit settings | |
| * @returns Travel duration in minutes, or null if calculation fails | |
| * @throws Error if API key is missing or API request fails critically | |
| */ | |
| /** | |
| * Calculates travel time between two addresses using Google Maps Distance Matrix API | |
| * | |
| * `@param` originAddress - Origin address as a string | |
| * `@param` destinationAddress - Destination address as a string | |
| * `@param` options - Optional travel mode and unit settings | |
| * `@returns` Travel duration in minutes, or null if calculation fails | |
| * `@throws` Error if GOOGLE_MAPS_API_KEY is not configured | |
| */ |
🤖 Prompt for AI Agents
In `@src/lib/google-maps.ts` around lines 34 - 42, The JSDoc for the Google Maps
travel-time function is inaccurate: only the missing-API-key check actually
throws, while HTTP/API errors are caught and the function returns null; update
the docblock for the function (the "Calculates travel time between two
addresses..." comment) to remove or narrow the `@throws` description to only the
missing API key case (mention the specific missing-key check) and clarify in
`@returns` that API request failures result in a null return rather than an
exception; reference the function name/description in the file so you update the
correct comment.
Comment on lines
+222
to
+234
| listDrivers: protectedProcedure.query(async ({ ctx }) => { | ||
| const drivers = await ctx.db | ||
| .select({ | ||
| id: user.id, | ||
| name: user.name, | ||
| email: user.email, | ||
| }) | ||
| .from(user) | ||
| .where(eq(user.role, "driver")) | ||
| .orderBy(asc(user.name)); | ||
|
|
||
| return drivers; | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify the file exists and check the actual content at lines 222-234
echo "=== Checking file content at lines 222-234 ==="
sed -n '222,234p' src/server/api/routers/bookings.ts
echo -e "\n=== Searching for all listDrivers usage ==="
rg -n --type=ts --type=tsx 'listDrivers' || echo "No results found"
echo -e "\n=== Checking for any usage in debug or test files ==="
rg -n --type=ts --type=tsx 'listDrivers' -g '*debug*' -g '*test*' || echo "No debug/test usage found"Repository: Code-the-Change-YYC/salvationarmy
Length of output: 646
🏁 Script executed:
#!/bin/bash
# Search without file type restrictions
echo "=== Broader search for listDrivers ==="
rg -n 'listDrivers'
echo -e "\n=== Check the full router file to see context ==="
wc -l src/server/api/routers/bookings.ts
echo -e "\n=== Check router exports and if this is used in tRPC ==="
grep -n "export" src/server/api/routers/bookings.ts | head -20
echo -e "\n=== Look for protectedProcedure definition ==="
rg -n 'protectedProcedure' src/server/api/trpc.ts | head -10
echo -e "\n=== Check what protectedProcedure actually is ==="
grep -A 5 'protectedProcedure.*=' src/server/api/trpc.ts | head -20Repository: Code-the-Change-YYC/salvationarmy
Length of output: 1066
🏁 Script executed:
#!/bin/bash
# Check the debug page and see if it has role-based access control
echo "=== Check debug page for access restrictions ==="
head -50 src/app/debug/bookings/page.tsx
echo -e "\n=== Check if there's a layout or middleware protecting debug pages ==="
ls -la src/app/debug/ | head -20
echo -e "\n=== Look for any redirects or authorization in debug pages ==="
rg -n 'redirect\|unauthorized\|role\|admin' src/app/debug/ | head -20Repository: Code-the-Change-YYC/salvationarmy
Length of output: 2152
🏁 Script executed:
#!/bin/bash
# Check for layout files that might protect debug routes
echo "=== Check for layout files in debug directory ==="
find src/app -name 'layout.tsx' | xargs grep -l 'debug\|redirect\|auth' 2>/dev/null || echo "No protective layout found"
echo -e "\n=== Check for any layout.tsx in /debug ==="
cat src/app/debug/layout.tsx 2>/dev/null || echo "No layout.tsx in debug directory"
echo -e "\n=== Check the main app layout ==="
head -50 src/app/layout.tsx | grep -A 20 'children'
echo -e "\n=== Search for environment or feature flag checks related to debug ==="
rg -n 'debug\|DEBUG' src/app/debug/ | head -20Repository: Code-the-Change-YYC/salvationarmy
Length of output: 1063
listDrivers exposes email addresses to all authenticated users.
Any authenticated user can call listDrivers and see all driver email addresses via the debug page without role restrictions. Consider adding role-based access control to protectedProcedure for this endpoint, restricting it to admins, or omitting the email field for non-admin callers.
🤖 Prompt for AI Agents
In `@src/server/api/routers/bookings.ts` around lines 222 - 234, listDrivers
currently returns driver emails to any authenticated caller; restrict access by
checking the caller's role before selecting the email column. In the listDrivers
handler (protectedProcedure -> listDrivers) either (A) enforce admin-only access
by using your admin middleware or checking ctx.session.user.role === "admin" and
aborting with an unauthorized error if not admin, or (B) keep the route public
to authenticated users but remove the email from the select for non-admins
(select only id and name when ctx.session.user.role !== "admin"). Update the
select projection (the current select of user.id, user.name, user.email) and the
access check in listDrivers accordingly so emails are only returned to admins.
Comment on lines
+514
to
+528
| if (updatesToApply.driverId !== undefined && updatesToApply.driverId) { | ||
| const proposedStart = updatesToApply.startTime ?? existing.startTime; | ||
| const proposedEnd = updatesToApply.endTime ?? existing.endTime; | ||
| const proposedPickup = updatesToApply.pickupAddress ?? existing.pickupAddress; | ||
| const proposedDest = updatesToApply.destinationAddress ?? existing.destinationAddress; | ||
|
|
||
| await validateDriverForSlot(ctx, { | ||
| driverId: updatesToApply.driverId, | ||
| startTime: proposedStart, | ||
| endTime: proposedEnd, | ||
| pickupAddress: proposedPickup, | ||
| destinationAddress: proposedDest, | ||
| excludeBookingId: id, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Driver validation is skipped when only startTime/endTime are updated on a booking that already has a driver.
The guard on line 514 only triggers when driverId is included in the update payload. If a caller updates startTime or endTime (or addresses) without re-sending driverId, the existing driver's slot is not re-validated, potentially creating overlapping or infeasible bookings.
🐛 Proposed fix — also validate against the existing driver
- if (updatesToApply.driverId !== undefined && updatesToApply.driverId) {
- const proposedStart = updatesToApply.startTime ?? existing.startTime;
- const proposedEnd = updatesToApply.endTime ?? existing.endTime;
- const proposedPickup = updatesToApply.pickupAddress ?? existing.pickupAddress;
- const proposedDest = updatesToApply.destinationAddress ?? existing.destinationAddress;
+ const effectiveDriverId = updatesToApply.driverId !== undefined
+ ? updatesToApply.driverId
+ : existing.driverId;
+
+ const timeOrAddressChanged =
+ updatesToApply.startTime !== undefined ||
+ updatesToApply.endTime !== undefined ||
+ updatesToApply.pickupAddress !== undefined ||
+ updatesToApply.destinationAddress !== undefined;
+
+ if (effectiveDriverId && (updatesToApply.driverId !== undefined || timeOrAddressChanged)) {
+ const proposedStart = updatesToApply.startTime ?? existing.startTime;
+ const proposedEnd = updatesToApply.endTime ?? existing.endTime;
+ const proposedPickup = updatesToApply.pickupAddress ?? existing.pickupAddress;
+ const proposedDest = updatesToApply.destinationAddress ?? existing.destinationAddress;
- await validateDriverForSlot(ctx, {
- driverId: updatesToApply.driverId,
+ await validateDriverForSlot(ctx, {
+ driverId: effectiveDriverId,
startTime: proposedStart,
endTime: proposedEnd,
pickupAddress: proposedPickup,
destinationAddress: proposedDest,
excludeBookingId: id,
});
}📝 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.
Suggested change
| if (updatesToApply.driverId !== undefined && updatesToApply.driverId) { | |
| const proposedStart = updatesToApply.startTime ?? existing.startTime; | |
| const proposedEnd = updatesToApply.endTime ?? existing.endTime; | |
| const proposedPickup = updatesToApply.pickupAddress ?? existing.pickupAddress; | |
| const proposedDest = updatesToApply.destinationAddress ?? existing.destinationAddress; | |
| await validateDriverForSlot(ctx, { | |
| driverId: updatesToApply.driverId, | |
| startTime: proposedStart, | |
| endTime: proposedEnd, | |
| pickupAddress: proposedPickup, | |
| destinationAddress: proposedDest, | |
| excludeBookingId: id, | |
| }); | |
| } | |
| const effectiveDriverId = updatesToApply.driverId !== undefined | |
| ? updatesToApply.driverId | |
| : existing.driverId; | |
| const timeOrAddressChanged = | |
| updatesToApply.startTime !== undefined || | |
| updatesToApply.endTime !== undefined || | |
| updatesToApply.pickupAddress !== undefined || | |
| updatesToApply.destinationAddress !== undefined; | |
| if (effectiveDriverId && (updatesToApply.driverId !== undefined || timeOrAddressChanged)) { | |
| const proposedStart = updatesToApply.startTime ?? existing.startTime; | |
| const proposedEnd = updatesToApply.endTime ?? existing.endTime; | |
| const proposedPickup = updatesToApply.pickupAddress ?? existing.pickupAddress; | |
| const proposedDest = updatesToApply.destinationAddress ?? existing.destinationAddress; | |
| await validateDriverForSlot(ctx, { | |
| driverId: effectiveDriverId, | |
| startTime: proposedStart, | |
| endTime: proposedEnd, | |
| pickupAddress: proposedPickup, | |
| destinationAddress: proposedDest, | |
| excludeBookingId: id, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@src/server/api/routers/bookings.ts` around lines 514 - 528, The current guard
only calls validateDriverForSlot when updatesToApply.driverId is present, so
changes to startTime/endTime/addresses on a booking with an existing driver skip
re-validation; change the condition to also run validation when there is an
existing.driverId and any of startTime/endTime/pickupAddress/destinationAddress
are being changed. Inside that block use driverId = updatesToApply.driverId ??
existing.driverId and compute
proposedStart/proposedEnd/proposedPickup/proposedDest the same way, then await
validateDriverForSlot(ctx, { driverId, startTime: proposedStart, endTime:
proposedEnd, pickupAddress: proposedPickup, destinationAddress: proposedDest,
excludeBookingId: id }); ensure the condition references
updatesToApply.startTime, updatesToApply.endTime, updatesToApply.pickupAddress,
and updatesToApply.destinationAddress and existing.driverId.
burtonjong
reviewed
Feb 11, 2026
| # database | ||
| drizzle/meta | ||
| drizzle/meta | ||
| drizzle |
Contributor
There was a problem hiding this comment.
drizzle shouldnt be added
burtonjong
reviewed
Feb 11, 2026
|
|
||
| // Optional fields | ||
| purpose: z.string().optional(), | ||
| phoneNumber: z.string().max(25).optional().nullable(), |
Contributor
There was a problem hiding this comment.
why are we adding phone number here?
burtonjong
reviewed
Feb 11, 2026
Comment on lines
+56
to
+59
| eq(bookings.driverId, driverId), | ||
| ne(bookings.status, "cancelled"), | ||
| lt(bookings.startTime, endTime), | ||
| gt(bookings.endTime, startTime), |
Contributor
There was a problem hiding this comment.
Eventually this query might be slow
It would be good to add an index on all 4 of these fields
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes