Sanc 82 #53
Sanc 82 #53
Conversation
burtonjong
requested review from
JustinTan-1,
Lujarios,
Yemyam,
jason-duong4509,
promatty,
tanvimahal,
themaxboucher and
wesleylui
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughImplements a post-trip survey feature: DB schema and types for surveys, survey CRUD TRPC router, driver-facing survey UI (form, notifications, list), driver layout and surveys page, expanded seed script, navbar and styling updates, bookings router/type changes, package pins, and removal of an old SQL migration file. Changes
Sequence DiagramsequenceDiagram
actor Driver
participant Browser as Driver UI
participant SurveyPage as Surveys Page
participant TRPC as TRPC API
participant DB as Database
Driver->>Browser: Navigate to /driver/surveys
Browser->>SurveyPage: Mount (server props: bookings, surveys)
SurveyPage->>TRPC: fetch bookings (surveyCompleted: false)
TRPC->>DB: query bookings
DB-->>TRPC: bookings
SurveyPage->>TRPC: fetch surveys
TRPC->>DB: query surveys
DB-->>TRPC: surveys
SurveyPage-->>Driver: render SurveyViewToggle (Pending/Completed)
Driver->>Browser: Click "Fill out survey"
Browser->>SurveyPage: open SurveyNotification modal
Driver->>SurveyPage: submit survey form
SurveyPage->>TRPC: call surveys.create (form data)
TRPC->>DB: insert post_trip_survey record
TRPC->>DB: update booking (survey_completed = true)
DB-->>TRPC: success
TRPC-->>Browser: created survey response
Browser-->>Driver: show success notification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/_components/common/navbar.tsx (1)
65-66:⚠️ Potential issue | 🟠 MajorChange the vehicle logs route to the correct path.
The route
/admin/vehicle-logsdoes not exist. Update the link to point to/admin/driver-logs:Diff
- <Link href="/admin/vehicle-logs"> + <Link href="/admin/driver-logs"> <Button text="Vehicle Log" variant="secondary" />
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 44-45: The package.json currently pins "react" and "react-dom" to
"19.0.0"; update these dependency entries to a non-vulnerable, maintained
version (eg. use a caret range like "react": "^19.2.1" and "react-dom":
"^19.2.1" or set both to "latest") so they receive security fixes, and then
reinstall (npm/yarn) to update the lockfile; ensure you update both "react" and
"react-dom" entries together to keep them in sync.
In `@src/app/_components/common/navbar.tsx`:
- Around line 22-35: The navbarText currently builds labels using section =
pathname.split("/")[2] which can be undefined for short paths; update how
section is derived (in the same scope where section is defined) to default to an
empty string or a safer derived value (e.g., compute section =
(pathname.split("/")[2] ?? "").replace(/^\//, "") or use a conditional fallback)
so navbarText (the switch on view and cases "admin","agency","driver") never
interpolates "undefined"; ensure agencyName handling remains unchanged.
In `@src/app/_components/drivercomponents/surveyButtons/surveyButtons.tsx`:
- Around line 64-65: booking.driverId can be null but Survey.driverId is defined
not-null; fix by ensuring you never pass a null driverId when creating the
Survey in surveyButtons.tsx: check booking.driverId before using it (e.g., in
the handler that constructs the survey payload), and either
short-circuit/return/error if it's null or map it to a valid fallback (or update
the Survey creation call to require and validate driverId). Reference
booking.driverId and the Survey creation site in surveyButtons.tsx and add a
guard/validation so Survey.driverId is always a non-null value.
- Around line 58-67: The object in surveyButtons.tsx is unsafely cast to Survey
even though required fields (e.g., id, tripCompletionStatus, startReading,
endReading, passengerFitRating) are missing; instead define a narrower type like
PendingSurvey (or use Partial<Survey> / Pick<Survey, ...> for the fields you
have) and replace the `as Survey` cast with creating that PendingSurvey object,
then update SurveyNotificationProps (and any consumers such as
SurveyNotification) to accept the union of Survey | PendingSurvey (or
Partial/Picked variant) and adjust usages to handle missing fields via optional
chaining or fallbacks so the component no longer relies on an unsafe assertion.
In `@src/app/_components/drivercomponents/surveyForm/survey-form.module.scss`:
- Around line 1-4: The .formRow CSS class in survey-form.module.scss sets gap
and uses grid-related child positioning but lacks display: grid; update the
.formRow rule to include "display: grid" so its grid-column/gap rules take
effect (ensure you add it to the .formRow selector and not inadvertently to
another selector or duplicate elsewhere); after change, verify child elements
using grid-column align as expected in the survey form UI.
In `@src/app/_components/drivercomponents/surveyForm/surveyForm.tsx`:
- Around line 174-192: The Radio.Group for originalLocationChanged is
incorrectly passed form.getInputProps with type: "checkbox" which supplies
checked semantics—remove the type override and stop using checkbox semantics;
instead bind value and onChange like the tripCompletionStatus pattern: drop the
type option from form.getInputProps("originalLocationChanged"), pass the
Radio.Group a value derived from form.values.originalLocationChanged and
implement onChange to call form.setFieldValue("originalLocationChanged", value
=== "true"), keeping the existing disabled/error handling and Radio values
"true"/"false".
In
`@src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx`:
- Around line 111-185: The confirm handler and modal currently allow submitting
even for completed surveys; update handleConfirm to short‑circuit when the
survey is completed (check the completed boolean at the top of handleConfirm and
return early) and avoid calling submitSurveyMutation.mutate for completed
surveys, and also disable/hide the modal's submit action by passing a
disabled/hidden confirm state when completed (use the Modal's
confirmText/disabled or conditionally render showDefaultFooter) so the UI ("View
survey results") cannot trigger a submit.
- Around line 62-79: The startReading and endReading validator functions in
surveyNotification.tsx currently allow 0 but backend requires readings > 0;
update both validators (startReading and endReading) to reject zero by checking
value <= 0 instead of < 0 (and ensure the cross-field check in endReading still
compares numbers), and update the error messages from "must be positive" to a
clearer "must be greater than zero" while preserving the early return for
BookingStatus.CANCELLED.
In `@src/app/driver/surveys/page.tsx`:
- Line 7: Rename the misspelled variable in the page component from
initalSurveys to initialSurveys and update every usage and prop pass accordingly
(e.g., the const declaration where initalSurveys is assigned from
api.surveys.getAll() and the prop passed into <SurveyViewToggle
initialSurveys={...}>), ensuring the variable name matches the SurveyViewToggle
prop to avoid the TypeScript undefined/prop mismatch.
In `@src/server/api/routers/surveys.ts`:
- Around line 17-48: Update the Zod input schemas used in the .input(...) calls
for both the create and update routes in surveys.ts to use .superRefine on the
existing z.object so the server enforces the same rules as the client: when
tripCompletionStatus !== BookingStatus.CANCELLED require startReading,
endReading, timeOfDeparture and timeOfArrival (mark them present and non-null)
and validate endReading > startReading and timeOfArrival > timeOfDeparture; use
the BookingStatus enum in the predicate and add explicit error messages and
paths for missing fields and ordering failures so validation errors map to the
appropriate keys (e.g., "startReading", "endReading", "timeOfArrival").
- Around line 118-147: The survey insert and subsequent bookings update must be
executed in a single DB transaction to avoid inconsistent state; wrap the insert
into postTripSurveys and the update(bookings).set({ surveyCompleted: true
}).where(eq(bookings.id, input.bookingId)) inside ctx.db.transaction(async (tx)
=> { ... }), using tx.insert(...).values(...).returning() and
tx.update(bookings)... so if either operation fails the whole transaction is
rolled back, and propagate/throw the same TRPCError on failure.
🧹 Nitpick comments (8)
src/app/_components/common/navbar.module.scss (1)
11-13: Consider using existing utility classes or consolidating.This
.flexclass is quite generic. If the project uses a CSS utility framework or has global utilities, consider using those instead to avoid duplication. If this is intentional for component scoping, the implementation is fine.src/app/_components/common/navbar.tsx (1)
37-46: Add explicit default return tohomeLink().The switch statement lacks a
defaultcase. While TypeScript's exhaustive checking may help, an explicit fallback prevents unexpected behavior ifNavbarViewis extended.♻️ Proposed fix
const homeLink = () => { switch (view) { case "admin": return `/admin/home`; case "agency": return `/agency/home`; case "driver": return `/driver/home`; + default: + return `/`; } };drizzle/seed.ts (2)
246-250: Remove unusedagencyEmailsarray.The
agencyEmailsarray is defined but never used. The actual query on line 252 filters by role instead.♻️ Proposed fix
- // Get the agency users - const agencyEmails = [ - "agency-one@salvationarmy.com", - "agency-two@salvationarmy.com", - "agency-three@salvationarmy.com", - ]; - const agencyUsers = await db.select().from(user).where(eq(user.role, "agency"));
447-460: Consider environment check before printing credentials.While printing seed credentials is helpful during development, consider guarding this output with an environment check to prevent accidental exposure in CI logs or other non-local environments.
♻️ Proposed approach
+ if (process.env.NODE_ENV !== "production") { console.log("\nLogin Credentials:"); // ... credential output + }package.json (1)
33-36: Inconsistent version pinning for TRPC packages.
@trpc/clientand@trpc/react-queryare pinned to exact11.0.0, but@trpc/server(line 36) still uses^11.0.0. This inconsistency could lead to version mismatches. Consider pinning all TRPC packages to the same exact version.♻️ Proposed fix
- "@trpc/server": "^11.0.0", + "@trpc/server": "11.0.0",src/app/driver/driver-layout.module.scss (1)
2-4: Consider simplifying padding declarations.The
padding-top: 0on line 4 overrides the top padding set on line 3. This can be consolidated into a single declaration.🔧 Suggested simplification
.driverPage { min-height: 100vh; - padding: 1rem 2rem; - padding-top: 0; + padding: 0 2rem 1rem;src/server/api/routers/surveys.ts (2)
20-23:driverIdis required but ignored.
Either remove it from the input schema or validate it matchesbooking.driverIdto avoid a confusing API contract.
269-277: UseinArrayfor booking ID filtering.
Building a largeor(...)list can be inefficient as the number of bookings grows.inArrayis the idiomatic Drizzle approach forINpredicates and typically performs better.Suggested refactor
-import { and, desc, eq, gte, lt, or } from "drizzle-orm"; +import { and, desc, eq, gte, inArray, lt, or } from "drizzle-orm"; ... - const existingSurveys = await ctx.db - .select() - .from(postTripSurveys) - .where( - bookingIds.length > 0 - ? or(...bookingIds.map((id) => eq(postTripSurveys.bookingId, id))) - : undefined, - ); + const existingSurveys = await ctx.db + .select() + .from(postTripSurveys) + .where(bookingIds.length > 0 ? inArray(postTripSurveys.bookingId, bookingIds) : undefined);
| survey={ | ||
| { | ||
| createdAt: new Date(booking.startTime), | ||
| timeOfDeparture: new Date(booking.startTime), | ||
| timeOfArrival: new Date(booking.endTime), | ||
| destinationAddress: booking.destinationAddress, | ||
| bookingId: booking.id, | ||
| driverId: booking.driverId, | ||
| } as Survey | ||
| } |
There was a problem hiding this comment.
Unsafe type assertion - missing required Survey fields.
The object being cast as Survey is missing several required fields (id, tripCompletionStatus, startReading, endReading, passengerFitRating, etc.). Looking at SurveyNotification (lines 44-53 in the relevant snippet), the form accesses these fields with fallbacks, but the type assertion masks potential issues.
Consider creating a dedicated type for pending surveys or using a Partial/Pick type:
♻️ Proposed approach
+type PendingSurvey = Pick<Survey, 'bookingId' | 'driverId' | 'createdAt' | 'timeOfDeparture' | 'timeOfArrival' | 'destinationAddress'> & {
+ tripCompletionStatus?: string;
+};
<SurveyNotification
key={booking.id}
- survey={
- {
- createdAt: new Date(booking.startTime),
- timeOfDeparture: new Date(booking.startTime),
- timeOfArrival: new Date(booking.endTime),
- destinationAddress: booking.destinationAddress,
- bookingId: booking.id,
- driverId: booking.driverId,
- } as Survey
- }
+ survey={{
+ createdAt: new Date(booking.startTime),
+ timeOfDeparture: new Date(booking.startTime),
+ timeOfArrival: new Date(booking.endTime),
+ destinationAddress: booking.destinationAddress,
+ bookingId: booking.id,
+ driverId: booking.driverId,
+ } as PendingSurvey}This would also require updating SurveyNotificationProps to accept the union type.
🤖 Prompt for AI Agents
In `@src/app/_components/drivercomponents/surveyButtons/surveyButtons.tsx` around
lines 58 - 67, The object in surveyButtons.tsx is unsafely cast to Survey even
though required fields (e.g., id, tripCompletionStatus, startReading,
endReading, passengerFitRating) are missing; instead define a narrower type like
PendingSurvey (or use Partial<Survey> / Pick<Survey, ...> for the fields you
have) and replace the `as Survey` cast with creating that PendingSurvey object,
then update SurveyNotificationProps (and any consumers such as
SurveyNotification) to accept the union of Survey | PendingSurvey (or
Partial/Picked variant) and adjust usages to handle missing fields via optional
chaining or fallbacks so the component no longer relies on an unsafe assertion.
There was a problem hiding this comment.
This ones weird - I don't know when we will want an assigned driver...
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Survey type definition from the schema
echo "=== Survey Schema Definition ==="
cat src/server/db/post-trip-schema.ts
echo -e "\n=== SurveyNotification Component ==="
cat src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx | head -100
echo -e "\n=== Check if driverId can be null in bookings ==="
rg -n "driverId" src/server/db/booking-schema.ts -A2 -B2
Length of output: 6812
@burtonjong, I can clarify! The issue is that driverId is nullable in bookings (a booking might not have a driver assigned yet), but the Survey type requires driverId to be non-null.
Looking at your schema:
- Bookings:
driverIdis nullable (text("driver_id").references(...)with no.notNull()) - Surveys:
driverIdis required (text("driver_id").notNull()...)
So when you cast booking.driverId as part of a Survey object, TypeScript doesn't catch that it could be null, which would cause runtime issues.
Two options:
- Filter out bookings without drivers (recommended if drivers must be assigned before surveys):
{activeView === "pending"
? bookings
.filter((booking) => booking.driverId !== null) // Only show bookings with assigned drivers
.map((booking: Booking) => (
<SurveyNotification
key={booking.id}
survey={{
createdAt: new Date(booking.startTime),
timeOfDeparture: new Date(booking.startTime),
timeOfArrival: new Date(booking.endTime),
destinationAddress: booking.destinationAddress,
bookingId: booking.id,
driverId: booking.driverId!, // Safe now after filter
} as Survey}- Handle null drivers in SurveyNotification (if you want to show them differently)
Which approach fits your business logic better? Should drivers always be assigned before surveys are needed?
🧠 Learnings used
Learnt from: promatty
Repo: Code-the-Change-YYC/salvationarmy PR: 19
File: src/server/db/post-trip-schema.ts:14-16
Timestamp: 2025-12-12T05:24:38.595Z
Learning: In the salvationarmy project, use `onDelete: "restrict"` for the driverId foreign key in post_trip_surveys table (src/server/db/post-trip-schema.ts) to preserve historical trip data and prevent driver deletion when they have associated trips.
| bookingId: booking.id, | ||
| driverId: booking.driverId, |
There was a problem hiding this comment.
Potential null reference for driverId.
booking.driverId can be null based on the booking schema (driverId: text("driver_id").references(...)), but Survey.driverId is required (notNull()). This mismatch could cause issues when creating the survey.
#!/bin/bash
# Verify driverId nullability in booking schema
rg -A2 "driverId.*text" src/server/db/booking-schema.ts🤖 Prompt for AI Agents
In `@src/app/_components/drivercomponents/surveyButtons/surveyButtons.tsx` around
lines 64 - 65, booking.driverId can be null but Survey.driverId is defined
not-null; fix by ensuring you never pass a null driverId when creating the
Survey in surveyButtons.tsx: check booking.driverId before using it (e.g., in
the handler that constructs the survey payload), and either
short-circuit/return/error if it's null or map it to a valid fallback (or update
the Survey creation call to require and validate driverId). Reference
booking.driverId and the Survey creation site in surveyButtons.tsx and add a
guard/validation so Survey.driverId is always a non-null value.
There was a problem hiding this comment.
This ones weird - I don't know when we will want an assigned driver...
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check booking schema for driverId definition and any related comments
rg -A5 -B5 "driverId.*text" src/server/db/booking-schema.ts
# Look for driver assignment logic in bookings router
rg -n "driverId" src/server/api/routers/bookings.ts | head -20
# Check if there are any validations or checks for driver assignment
rg -n "driver.*assign|assign.*driver" --type ts
# Look for booking creation/update logic
ast-grep --pattern 'async create($$$) {
$$$
}'
Length of output: 973
🏁 Script executed:
#!/bin/bash
# Check surveys router for the driver assignment validation
rg -A10 -B2 "there should be a assigned driver" src/server/api/routers/surveys.ts
# Check if there are any filters for driverId in booking queries
rg -n "surveyCompleted|getAll" src/server/api/routers/bookings.ts -A5
# Look at the seed file to understand when drivers are assigned
rg -n "driverId" drizzle/seed.ts | head -15
Length of output: 1040
🏁 Script executed:
#!/bin/bash
# Check if bookings can be created without drivers in the bookings router
rg -B5 -A10 "async.*create" src/server/api/routers/bookings.ts | grep -A15 "\.mutation"
# Look at booking creation in seed to see the pattern
rg -B3 -A8 "await ctx.db.insert\(bookings\)" drizzle/seed.ts | head -30
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the create mutation in bookings router
sed -n '/create:.*publicProcedure/,/^\s*},/p' src/server/api/routers/bookings.ts | head -50
# Check seed file for booking creation pattern
rg -A10 "bookings.insert" drizzle/seed.ts | head -40
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Just get the create procedure from bookings router
rg -n "create:" src/server/api/routers/bookings.ts -A30 | head -50
# Get a sample from seed file
rg -n "insert.*bookings" drizzle/seed.ts -A5 | head -20
Length of output: 1452
@burtonjong Great question! I can see the confusion here. Based on the code:
Current State:
- Bookings CAN be created without drivers (driverId is
nullable().optional()in the create mutation) - Surveys CANNOT be created without drivers (the surveys router explicitly throws an error: "Booking does not have an assigned driver")
- The pending survey list shows ALL incomplete bookings, including those without assigned drivers
The Problem:
This creates a poor UX where drivers see pending surveys for bookings they can't actually complete. When they try to fill it out, the submission will fail.
Recommended Fix:
Filter the pending surveys list to only show bookings with assigned drivers:
{activeView === "pending"
? bookings
.filter((booking) => booking.driverId !== null) // Add this filter
.map((booking: Booking) => (
<SurveyNotification
key={booking.id}
survey={{
// ... rest of the code
driverId: booking.driverId, // Now guaranteed non-nullBusiness Logic Question:
To properly design this, when should drivers be assigned to bookings?
- At booking creation time (agencies select a driver upfront)?
- Later, by an admin/dispatcher before the trip?
- Some bookings never get drivers (self-serve)?
This will help determine whether to make driverId required or keep it optional with better filtering.
🧠 Learnings used
Learnt from: promatty
Repo: Code-the-Change-YYC/salvationarmy PR: 19
File: src/server/db/post-trip-schema.ts:14-16
Timestamp: 2025-12-12T05:24:38.595Z
Learning: In the salvationarmy project, use `onDelete: "restrict"` for the driverId foreign key in post_trip_surveys table (src/server/db/post-trip-schema.ts) to preserve historical trip data and prevent driver deletion when they have associated trips.
src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx
Show resolved
Hide resolved
src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx
Show resolved
Hide resolved
|
Couple notes:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/_components/common/navbar.tsx (1)
65-67:⚠️ Potential issue | 🟡 MinorFix admin driver logs route to
/admin/driver-logs.
Current link points to/admin/vehicle-logs, which is the wrong route for driver logs.Based on learnings: In the Salvation Army project, ensure the admin link for vehicle/driver logs points to /admin/driver-logs instead of /admin/vehicle-logs.🔧 Suggested fix
- <Link href="/admin/vehicle-logs"> - <Button text="Vehicle Log" variant="secondary" /> - </Link> + <Link href="/admin/driver-logs"> + <Button text="Driver Logs" variant="secondary" /> + </Link>
🤖 Fix all issues with AI agents
In
`@src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx`:
- Around line 123-139: The code builds formData from a Partial<Survey> prop
(survey) but never ensures bookingId and driverId are present, so
submitSurveyMutation.mutate can receive undefined where bookingId: number and
driverId: string are required; add an explicit guard before constructing
formData (e.g., if (!survey?.bookingId || !survey?.driverId) return/throw or
show validation), then narrow types (so subsequent code can use survey.bookingId
and survey.driverId as defined values or use non-null assertions) and only call
submitSurveyMutation.mutate(formData) after that guard; update logic around
formData construction in surveyNotification.tsx (where formData is created and
submitSurveyMutation.mutate is called) to rely on the validated
bookingId/driverId.
🧹 Nitpick comments (2)
package.json (2)
34-36: Inconsistent version pinning across tRPC packages.
@trpc/clientand@trpc/react-queryare pinned to exact11.0.0, but@trpc/server(line 36) still uses^11.0.0. This could lead to version drift where the server package auto-upgrades to a newer minor/patch while client packages remain at 11.0.0, potentially causing compatibility issues.Consider either:
- Pin all tRPC packages to the same exact version for consistency
- Use caret ranges for all tRPC packages
♻️ Suggested fix: Pin `@trpc/server` to match client packages
"@trpc/client": "11.0.0", "@trpc/react-query": "11.0.0", - "@trpc/server": "^11.0.0", + "@trpc/server": "11.0.0",
33-47: Mixed versioning strategy across dependencies.The PR introduces an inconsistent approach: some packages are exact-pinned (e.g.,
@tanstack/react-query,ag-grid-react,pg,sass) while others retain caret ranges (e.g.,@auth/drizzle-adapter,better-auth,drizzle-orm,postgres).If the intent is deterministic builds, consider documenting this strategy or applying it consistently. If using a lockfile (which yarn generates), exact pinning in
package.jsonprovides limited additional benefit over caret ranges with a committed lockfile.
| const formData = { | ||
| bookingId: survey.bookingId, | ||
| driverId: survey.driverId, | ||
| tripCompletionStatus: form.values.tripCompletionStatus, | ||
| destinationAddress: form.values.destinationAddress, | ||
| comments: form.values.comments, | ||
| ...(!isCancelled && { | ||
| startReading: Number(form.values.startReading), | ||
| endReading: Number(form.values.endReading), | ||
| timeOfDeparture: form.values.timeOfDeparture || undefined, | ||
| timeOfArrival: form.values.timeOfArrival || undefined, | ||
| originalLocationChanged: form.values.originalLocationChanged, | ||
| passengerFitRating: Number(form.values.passengerFitRating), | ||
| }), | ||
| }; | ||
|
|
||
| submitSurveyMutation.mutate(formData); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the file structure and locate the key files
find . -type f -name "surveyNotification.tsx" 2>/dev/null | head -5Repository: Code-the-Change-YYC/salvationarmy
Length of output: 157
🏁 Script executed:
# Look for the surveyNotification file and examine its types
find . -type f -name "*survey*" -o -name "*mutation*" | grep -E "\.(ts|tsx)$" | head -20Repository: Code-the-Change-YYC/salvationarmy
Length of output: 329
🏁 Script executed:
# Check the post-trip-schema to understand field requirements
find . -type f -name "*post-trip*" -o -name "*schema*" | grep -E "\.(ts|tsx)$" | head -20Repository: Code-the-Change-YYC/salvationarmy
Length of output: 203
🏁 Script executed:
# Read the surveyNotification file to understand the survey prop type and handleConfirm implementation
wc -l ./src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsxRepository: Code-the-Change-YYC/salvationarmy
Length of output: 161
🏁 Script executed:
# Read relevant sections of surveyNotification.tsx
head -150 ./src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx | tail -80Repository: Code-the-Change-YYC/salvationarmy
Length of output: 3207
🏁 Script executed:
# Look at the beginning to see props type definition
head -50 ./src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsxRepository: Code-the-Change-YYC/salvationarmy
Length of output: 1994
🏁 Script executed:
# Check the surveys router to see the mutation input type expectations
cat ./src/server/api/routers/surveys.tsRepository: Code-the-Change-YYC/salvationarmy
Length of output: 14497
🏁 Script executed:
# Check the Survey type definition
find . -type f -name "types.ts" -o -name "types.d.ts" | head -5Repository: Code-the-Change-YYC/salvationarmy
Length of output: 97
🏁 Script executed:
# Find where Survey type is defined
rg "type Survey" --type ts --type tsx | head -20Repository: Code-the-Change-YYC/salvationarmy
Length of output: 104
🏁 Script executed:
# Check post-trip-schema to see the SurveyInsertType
head -100 ./src/server/db/post-trip-schema.tsRepository: Code-the-Change-YYC/salvationarmy
Length of output: 2199
🏁 Script executed:
# Quick verification: check the actual Survey type definition to confirm bookingId/driverId can be undefined
grep -A 20 "type Survey" ./src/types/types.ts | head -30Repository: Code-the-Change-YYC/salvationarmy
Length of output: 459
🏁 Script executed:
# Let me trace through: Survey = SurveySelectType, which comes from the postTripSurveys table
# The table has bookingId and driverId as .notNull(), so they're required in the select model
# But the component receives Partial<Survey>, making them optional
# Let's verify the handleConfirm calls the mutation with these fields
grep -n "submitSurveyMutation.mutate" ./src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsxRepository: Code-the-Change-YYC/salvationarmy
Length of output: 123
🏁 Script executed:
# Double check: are there any other files or contexts where survey.bookingId could be guaranteed to exist?
# Let's see how SurveyNotification is called
rg "SurveyNotification" --type ts -B 2 -A 2 | head -40Repository: Code-the-Change-YYC/salvationarmy
Length of output: 3257
Add required guard for bookingId/driverId before calling mutation (type mismatch).
The mutation input requires bookingId: number and driverId: string, but the survey prop is Partial<Survey>, allowing both to be undefined. Without a guard, undefined values will be passed to fields that expect defined values, causing a TypeScript type error. Add a check and narrow the types before building formData.
Suggested fix
const handleConfirm = () => {
+ if (survey.bookingId == null || !survey.driverId) {
+ notify.error("Missing booking or driver details for this survey");
+ return;
+ }
+ const bookingId = survey.bookingId;
+ const driverId = survey.driverId;
const validation = form.validate();
const hasErrors = Object.keys(validation.errors).length > 0;
@@
const formData = {
- bookingId: survey.bookingId,
- driverId: survey.driverId,
+ bookingId,
+ driverId,
tripCompletionStatus: form.values.tripCompletionStatus,📝 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.
| const formData = { | |
| bookingId: survey.bookingId, | |
| driverId: survey.driverId, | |
| tripCompletionStatus: form.values.tripCompletionStatus, | |
| destinationAddress: form.values.destinationAddress, | |
| comments: form.values.comments, | |
| ...(!isCancelled && { | |
| startReading: Number(form.values.startReading), | |
| endReading: Number(form.values.endReading), | |
| timeOfDeparture: form.values.timeOfDeparture || undefined, | |
| timeOfArrival: form.values.timeOfArrival || undefined, | |
| originalLocationChanged: form.values.originalLocationChanged, | |
| passengerFitRating: Number(form.values.passengerFitRating), | |
| }), | |
| }; | |
| submitSurveyMutation.mutate(formData); | |
| const handleConfirm = () => { | |
| if (survey.bookingId == null || !survey.driverId) { | |
| notify.error("Missing booking or driver details for this survey"); | |
| return; | |
| } | |
| const bookingId = survey.bookingId; | |
| const driverId = survey.driverId; | |
| const validation = form.validate(); | |
| const hasErrors = Object.keys(validation.errors).length > 0; | |
| const formData = { | |
| bookingId, | |
| driverId, | |
| tripCompletionStatus: form.values.tripCompletionStatus, | |
| destinationAddress: form.values.destinationAddress, | |
| comments: form.values.comments, | |
| ...(!isCancelled && { | |
| startReading: Number(form.values.startReading), | |
| endReading: Number(form.values.endReading), | |
| timeOfDeparture: form.values.timeOfDeparture || undefined, | |
| timeOfArrival: form.values.timeOfArrival || undefined, | |
| originalLocationChanged: form.values.originalLocationChanged, | |
| passengerFitRating: Number(form.values.passengerFitRating), | |
| }), | |
| }; | |
| submitSurveyMutation.mutate(formData); | |
| }; |
🧰 Tools
🪛 GitHub Actions: CI
[error] 139-139: TS2345: Argument of type '{ startReading?: number | undefined; endReading?: number | undefined; timeOfDeparture?: string | undefined; timeOfArrival?: string | undefined; originalLocationChanged?: boolean | undefined; ... 5 more ...; comments: string; }' is not assignable to parameter of type '{ driverId: string; bookingId: number; tripCompletionStatus: BookingStatus; destinationAddress?: string | undefined; startReading?: number | undefined; endReading?: number | undefined; ... 4 more ...; comments?: string | undefined; }'.
🪛 GitHub Check: ci
[failure] 139-139:
Argument of type '{ startReading?: number | undefined; endReading?: number | undefined; timeOfDeparture?: string | undefined; timeOfArrival?: string | undefined; originalLocationChanged?: boolean | undefined; ... 5 more ...; comments: string; }' is not assignable to parameter of type '{ driverId: string; bookingId: number; tripCompletionStatus: BookingStatus; destinationAddress?: string | undefined; startReading?: number | undefined; endReading?: number | undefined; ... 4 more ...; comments?: string | undefined; }'.
🤖 Prompt for AI Agents
In
`@src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx`
around lines 123 - 139, The code builds formData from a Partial<Survey> prop
(survey) but never ensures bookingId and driverId are present, so
submitSurveyMutation.mutate can receive undefined where bookingId: number and
driverId: string are required; add an explicit guard before constructing
formData (e.g., if (!survey?.bookingId || !survey?.driverId) return/throw or
show validation), then narrow types (so subsequent code can use survey.bookingId
and survey.driverId as defined values or use non-null assertions) and only call
submitSurveyMutation.mutate(formData) after that guard; update logic around
formData construction in surveyNotification.tsx (where formData is created and
submitSurveyMutation.mutate is called) to rely on the validated
bookingId/driverId.
1 participant
lots
Summary by CodeRabbit
New Features
Documentation
Chores
Style