Conversation
tanvimahal
requested review from
JustinTan-1,
Lujarios,
Yemyam,
burtonjong,
jason-duong4509,
promatty,
themaxboucher and
wesleylui
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new rider log table display feature is introduced, consisting of an AG Grid-based Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 docstrings
🧪 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/_components/riderlogcomponents/rider-log-table-view.tsx`:
- Around line 106-109: In the valueFormatter for the column (valueFormatter)
guard against malformed dates by constructing a Dayjs instance from params.value
and calling isValid() before formatting; if the date is invalid or params.value
falsy, return an empty string, otherwise return dayjs(params.value).format("MMM
D, YYYY") — update the valueFormatter logic in rider-log-table-view.tsx
accordingly.
In `@src/app/admin/rider-logs/page.tsx`:
- Around line 13-15: The "Export to CSV File" Button in
src/app/admin/rider-logs/page.tsx is interactive but has no handler; change the
Button (rendered inside Group) to a non-interactive/disabled state by adding the
disabled prop (and optionally aria-disabled and a tooltip/title like "Coming
soon") so the control is visibly inert until CSV functionality is implemented;
ensure you update the Button with the unique text/content "Export to CSV File"
so reviewers can find and verify the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39c078e6-18ee-4249-984b-83226cf05e6c
📒 Files selected for processing (4)
src/app/_components/riderlogcomponents/rider-log-table-view.module.scsssrc/app/_components/riderlogcomponents/rider-log-table-view.tsxsrc/app/admin/rider-logs/page.tsxsrc/constants/RiderLogTableConstants.ts
| valueFormatter: (params) => { | ||
| if (!params.value) return ""; | ||
| return dayjs(params.value).format("MMM D, YYYY"); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect surveys.getAll output shaping =="
rg -n "getAll: protectedProcedure|timeOfDeparture|departure|map\\(|return" src/server/api/routers/surveys.ts -C4Repository: Code-the-Change-YYC/salvationarmy
Length of output: 5699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect rider-log-table-view.tsx structure and data binding =="
# Find the component file
fd -t f "rider-log-table-view.tsx"
# Get file size first to avoid reading huge files
wc -l src/app/_components/riderlogcomponents/rider-log-table-view.tsx 2>/dev/null || echo "File not found, searching..."Repository: Code-the-Change-YYC/salvationarmy
Length of output: 274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If file exists, read around the formatter lines to see context
if [ -f "src/app/_components/riderlogcomponents/rider-log-table-view.tsx" ]; then
echo "== Context around lines 100-115 =="
sed -n '95,120p' src/app/_components/riderlogcomponents/rider-log-table-view.tsx
echo -e "\n== Check for API query usage and data flow =="
rg -n "api\.surveys\.getAll|useSurveys|timeOfDeparture" src/app/_components/riderlogcomponents/rider-log-table-view.tsx -A2 -B2
else
echo "File not found in expected location"
fd -i "rider-log" --type f
fiRepository: Code-the-Change-YYC/salvationarmy
Length of output: 1566
Guard against invalid date strings before formatting.
The formatter can output "Invalid Date" for malformed values. Even though the backend validates timeOfDeparture as ISO 8601 format, add an isValid() check to gracefully fall back to empty text:
valueFormatter: (params) => {
if (!params.value) return "";
- return dayjs(params.value).format("MMM D, YYYY");
+ const parsed = dayjs(params.value);
+ return parsed.isValid() ? parsed.format("MMM D, YYYY") : "";
},📝 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.
| valueFormatter: (params) => { | |
| if (!params.value) return ""; | |
| return dayjs(params.value).format("MMM D, YYYY"); | |
| }, | |
| valueFormatter: (params) => { | |
| if (!params.value) return ""; | |
| const parsed = dayjs(params.value); | |
| return parsed.isValid() ? parsed.format("MMM D, YYYY") : ""; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/_components/riderlogcomponents/rider-log-table-view.tsx` around lines
106 - 109, In the valueFormatter for the column (valueFormatter) guard against
malformed dates by constructing a Dayjs instance from params.value and calling
isValid() before formatting; if the date is invalid or params.value falsy,
return an empty string, otherwise return dayjs(params.value).format("MMM D,
YYYY") — update the valueFormatter logic in rider-log-table-view.tsx
accordingly.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| {/* TODO: SANC-96 adds csv functionality */} | ||
| <Button text="Export to CSV File" variant="secondary" icon={<Grid />} /> | ||
| </Group> |
There was a problem hiding this comment.
Disable the CSV button until the action exists.
On Line 14, the button is interactive but has no handler. That’s a misleading control in production UI.
Small UX-safe adjustment
- <Button text="Export to CSV File" variant="secondary" icon={<Grid />} />
+ <Button
+ text="Export to CSV File"
+ variant="secondary"
+ icon={<Grid />}
+ disabled
+ />📝 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.
| {/* TODO: SANC-96 adds csv functionality */} | |
| <Button text="Export to CSV File" variant="secondary" icon={<Grid />} /> | |
| </Group> | |
| {/* TODO: SANC-96 adds csv functionality */} | |
| <Button | |
| text="Export to CSV File" | |
| variant="secondary" | |
| icon={<Grid />} | |
| disabled | |
| /> | |
| </Group> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/admin/rider-logs/page.tsx` around lines 13 - 15, The "Export to CSV
File" Button in src/app/admin/rider-logs/page.tsx is interactive but has no
handler; change the Button (rendered inside Group) to a non-interactive/disabled
state by adding the disabled prop (and optionally aria-disabled and a
tooltip/title like "Coming soon") so the control is visibly inert until CSV
functionality is implemented; ensure you update the Button with the unique
text/content "Export to CSV File" so reviewers can find and verify the change.
There was a problem hiding this comment.
yeah lets disable the button for now @tanvimahal
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| import Rating4 from "@/assets/icons/rating4"; | ||
| import Rating5 from "@/assets/icons/rating5"; | ||
| import { | ||
| COLUMN_IDS, |
| @@ -0,0 +1,37 @@ | |||
| import type { ColDef } from "ag-grid-community"; | |||
|
|
|||
| export const DEFAULT_COLUMN_WIDTH = 150; | |||
There was a problem hiding this comment.
DEFAULT_COLUMN_WIDTH isnt used
| export interface RiderLogData { | ||
| timeOfDeparture: string | null; | ||
| originalLocationChanged: boolean | null; | ||
| passengerFitRating: number | null; | ||
| passengerInfo: string | null; | ||
| comments: string | null; | ||
| } |
There was a problem hiding this comment.
can we try using a Pick?
| export interface RiderLogData { | |
| timeOfDeparture: string | null; | |
| originalLocationChanged: boolean | null; | |
| passengerFitRating: number | null; | |
| passengerInfo: string | null; | |
| comments: string | null; | |
| } | |
| type RiderLogData = Pick< | |
| SurveySelectType, | |
| | "timeOfDeparture" | |
| | "originalLocationChanged" | |
| | "passengerFitRating" | |
| | "passengerInfo" | |
| | "comments" | |
| >; |
| [COLUMN_IDS.PASSENGER_INFO]: "Passenger name", | ||
| [COLUMN_IDS.DEPARTURE_TIME]: "Transport Date", | ||
| [COLUMN_IDS.PASSENGER_RATING]: "Fitness rating", | ||
| [COLUMN_IDS.LOCATION_CHANGED]: "Did Passenger Request to be dropped off at a different location?", |
There was a problem hiding this comment.
this string is too long to fit in a column header. it gets cut off
2 participants
Added a rider logs table view based on the vehicle logs frontend and the Figma design.
Summary by CodeRabbit
Release Notes