Skip to content

SANC-86: fix updating survey and add passenger info#59

Merged
burtonjong merged 1 commit intomainfrom
SANC-86
Feb 13, 2026
Merged

SANC-86: fix updating survey and add passenger info#59
burtonjong merged 1 commit intomainfrom
SANC-86

Conversation

@burtonjong
Copy link
Contributor

@burtonjong burtonjong commented Feb 13, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Surveys can now be edited and updated after submission, with changes properly reflected in the system
    • Passenger information is now captured and stored with survey responses
  • Improvements

    • Enhanced form stability and data handling for improved survey submission reliability

@vercel
Copy link

vercel bot commented Feb 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
salvationarmy Ready Ready Preview, Comment Feb 13, 2026 3:54am

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

This pull request extends the post-trip survey feature to capture, persist, and update passenger information. Changes span the database schema (adding passengerInfo column), API layer (supporting passengerInfo in create/update operations), and UI components (refactoring to support survey updates with proper form state management).

Changes

Cohort / File(s) Summary
Database & API Layer
src/server/db/post-trip-schema.ts, src/server/api/routers/surveys.ts
Added nullable passengerInfo column to post_trip_surveys table and extended create/update input schemas to accept and persist passenger information.
Survey Notification Component
src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx
Removed additionalInformation prop, introduced mapSurveyToFormValues() helper to transform survey data into form values, added updateSurveyMutation for editing existing surveys, and refactored submit/update flow to handle both create and update operations with appropriate payloads and UI labels.
Survey Integration Components
src/app/_components/drivercomponents/surveyButtons/surveyButtons.tsx
Updated key prop format for SurveyNotification elements and added passengerInfo to the survey object passed in pending view while removing additionalInformation prop.
Trip Survey Modal Input
src/app/_components/drivercomponents/TripSurveyModal.tsx
Converted Odometer start and end TextInput fields from uncontrolled to controlled inputs by adding value props bound to form state.

Sequence Diagram

sequenceDiagram
    actor User as Driver
    participant UI as SurveyNotification
    participant Form as Form State
    participant API as Survey API
    participant DB as Database

    User->>UI: Click "Update Survey" button
    UI->>Form: Populate form with mapSurveyToFormValues(survey)
    UI->>UI: Open modal with form values
    User->>UI: Fill form (passenger info, readings, etc.)
    User->>UI: Click "Update Survey"
    UI->>Form: Validate form inputs
    Form-->>UI: Validation complete
    UI->>API: Call updateSurveyMutation({id, ...payload})
    API->>DB: Update survey record with new passengerInfo & fields
    DB-->>API: Confirmation
    API->>UI: onSuccess callback
    UI->>Form: Update cached form values
    UI->>UI: Close modal & show success
    UI-->>User: Survey updated
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • JustinTan-1
  • themaxboucher
  • Yemyam
  • promatty
  • Lujarios
  • wesleylui

Poem

🐰 Hopping through surveys with glee,
PassengerInfo now flows free,
Updates and creates in harmony,
Forms controlled, mutations with spree,
Schema dancing, databases agree! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection ⚠️ Warning ⚠️ Unable to check for merge conflicts: Failed to fetch base branch: From https://github.com/Code-the-Change-YYC/salvationarmy
! [rejected] main -> main (non-fast-forward)
+ 86f16a4...269d028 main -> origin/main (forced update)
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the two main changes: fixing survey updating functionality and adding passenger info storage capability.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch SANC-86
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch SANC-86
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/app/_components/drivercomponents/TripSurveyModal.tsx (1)

161-161: Minor: falsy check treats 0 as empty.

form.values.startReading ? ... : "" will display an empty string when the value is 0. Since the validation requires positive values this is unlikely to cause a visible bug, but using form.values.startReading !== "" ? form.values.startReading : "" (or ?? "") would be more semantically correct and future-proof.

Also applies to: 169-169

src/app/_components/drivercomponents/surveyButtons/surveyButtons.tsx (1)

56-69: The as Survey cast hides missing required fields.

The partial object constructed here (lines 58-67) is missing several required Survey fields (id, tripCompletionStatus, startReading, endReading, etc.) but is force-cast to Survey. Downstream code (surveyNotification.tsx) already accepts Partial<Survey>, so this cast is unnecessary and masks type errors.

♻️ Suggested fix
               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,
                   passengerInfo: booking.passengerInfo,
-                } as Survey // TODO: remove
+                }
               }
src/app/_components/drivercomponents/surveyNotification/surveyNotification.tsx (3)

25-37: Falsy-check on numeric fields silently drops valid zero values.

inputSurvey.startReading || "" (and similarly for endReading, passengerFitRating) will treat 0 as falsy and map it to "". While current validation requires positive values, using ?? would be more correct and resilient:

♻️ Suggested fix
  const mapSurveyToFormValues = (inputSurvey: Partial<Survey>) => ({
    tripCompletionStatus: inputSurvey.tripCompletionStatus || BookingStatus.COMPLETED,
-   startReading: inputSurvey.startReading || ("" as number | ""),
-   endReading: inputSurvey.endReading || ("" as number | ""),
+   startReading: inputSurvey.startReading ?? ("" as number | ""),
+   endReading: inputSurvey.endReading ?? ("" as number | ""),
    timeOfDeparture: inputSurvey.timeOfDeparture
      ? dayjs(inputSurvey.timeOfDeparture).toISOString()
      : "",
    timeOfArrival: inputSurvey.timeOfArrival ? dayjs(inputSurvey.timeOfArrival).toISOString() : "",
    destinationAddress: inputSurvey.destinationAddress || "",
    originalLocationChanged: inputSurvey.originalLocationChanged ?? false,
-   passengerFitRating: inputSurvey.passengerFitRating || ("" as number | ""),
+   passengerFitRating: inputSurvey.passengerFitRating ?? ("" as number | ""),
    comments: inputSurvey.comments || "",
  });

52-66: Redundant cache update after invalidation.

Lines 57-61 call invalidate() (which triggers a refetch) and then immediately setData() on the same query. The refetch from invalidate will overwrite the optimistic setData. If the intent is optimistic UI, consider using setData only (without invalidate), or just invalidate alone.


153-164: Guard against missing survey.id is good, but the completed flag semantics may be misleading.

The completed prop controls whether to create or update. However, completed is set to true only for surveys in the "completed" view (line 72 of surveyButtons.tsx). This means a driver cannot update a survey from the pending view — only view/create. The naming completed for controlling create-vs-update logic is slightly confusing. Consider renaming to something like isUpdate or mode for clarity.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@burtonjong burtonjong merged commit 58c4d58 into main Feb 13, 2026
3 of 4 checks passed
@burtonjong burtonjong deleted the SANC-86 branch February 13, 2026 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant