Update RaceTab and lapdata to use timescale#279
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR flattens the lap data structure from a nested object format to top-level fields across the client, server, and shared type definitions. Column accessors, field references, and data construction patterns are updated to reflect the new flat structure, affecting table configuration, components, store logic, and type definitions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
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 (2)
packages/client/src/components/config/lapTableConfig.ts (1)
11-38:⚠️ Potential issue | 🟡 MinorTimeStamp sorting relies on hardcoded date format parsing.
The custom
sortingFnparses dates assuming a specific format (MM/DD/YYYY HH:MM) that matchestoLocaleString("en-US")output. However,toLocaleString("en-US")actually produces format like"1/31/2026, 3:05:08 PM"which includes:
- Commas
- AM/PM indicator
- Seconds
The current parser will fail or produce incorrect results with this format.
🐛 Proposed fix using Date constructor directly
sortingFn: (rowA, rowB, columnId) => { - const parseDate = (dateString: string) => { - const parts = dateString.split(" "); - - const dateParts = parts[0]?.split("/"); - const timeParts = parts[1]?.split(":"); - - if (!dateParts || !timeParts) { - throw new Error("Invalid Time"); - } - const hour = Number(timeParts[0]); - const minute = Number(timeParts[1]); - - const day = Number(dateParts[1]); - const month = Number(dateParts[0]); - const year = Number(dateParts[2]); - return new Date(year, month - 1, day, hour, minute).getTime(); - }; - const dateA = parseDate(rowA.getValue(columnId)); - const dateB = parseDate(rowB.getValue(columnId)); + const dateA = new Date(rowA.getValue(columnId)).getTime(); + const dateB = new Date(rowB.getValue(columnId)).getTime(); return dateB - dateA; },packages/shared/src/types.ts (1)
436-476:⚠️ Potential issue | 🟡 MinorRemove or repurpose the unused
LapDataclass.The
LapDataclass is exported but never imported or instantiated anywhere in the codebase—it appears to be dead code. All active usage is of theILapDatainterface instead. Additionally, there are naming inconsistencies between the class (camelCase:timestamp,lapTime, etc.) and the interface (PascalCase:TimeStamp,LapTime, etc.), and field mismatches (lapNumberandlapsRemainingexist only in the class;Rfidexists only in the interface).Either remove the class if it's no longer needed, or if it's intended for future use, align it with
ILapDatato prevent confusion.
🤖 Fix all issues with AI agents
In `@packages/client/src/components/molecules/RaceTabMolecules/RaceTabTable.tsx`:
- Line 53: In the RaceTabTable component update the JSX className string (the
className prop that includes `${cell.id.includes("TimeStamp") ? "left-0 z-10" :
""}`) to fix the typo by replacing "w-fullpx-4" with two separate Tailwind
classes "w-full px-4" so the utility classes are correctly parsed.
🧹 Nitpick comments (1)
packages/client/src/stores/useLapData.ts (1)
19-19: Consider making locale configurable for internationalization.The hardcoded
"en-US"locale may not be appropriate for all users. If internationalization is planned, consider using the user's browser locale or a configurable setting.TimeStamp: new Date(lapPacket.TimeStamp).toLocaleString(),
| {row.getVisibleCells().map((cell) => ( | ||
| <td | ||
| className={`text-gray-900 w-fullpx-4 sticky w-24 border-b-2 border-r-2 border-helios py-2 text-center text-sm first:border-l-2 dark:text-white ${cell.id.includes("data_timeStamp") ? "left-0 z-10" : ""}`} | ||
| className={`text-gray-900 w-fullpx-4 sticky w-24 border-b-2 border-r-2 border-helios py-2 text-center text-sm first:border-l-2 dark:text-white ${cell.id.includes("TimeStamp") ? "left-0 z-10" : ""}`} |
There was a problem hiding this comment.
Typo in className: missing space between utility classes.
The string "w-fullpx-4" should be "w-full px-4" - there's a missing space between the Tailwind classes.
🐛 Proposed fix
- className={`text-gray-900 w-fullpx-4 sticky w-24 border-b-2 border-r-2 border-helios py-2 text-center text-sm first:border-l-2 dark:text-white ${cell.id.includes("TimeStamp") ? "left-0 z-10" : ""}`}
+ className={`text-gray-900 w-full px-4 sticky w-24 border-b-2 border-r-2 border-helios py-2 text-center text-sm first:border-l-2 dark:text-white ${cell.id.includes("TimeStamp") ? "left-0 z-10" : ""}`}📝 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.
| className={`text-gray-900 w-fullpx-4 sticky w-24 border-b-2 border-r-2 border-helios py-2 text-center text-sm first:border-l-2 dark:text-white ${cell.id.includes("TimeStamp") ? "left-0 z-10" : ""}`} | |
| className={`text-gray-900 w-full px-4 sticky w-24 border-b-2 border-r-2 border-helios py-2 text-center text-sm first:border-l-2 dark:text-white ${cell.id.includes("TimeStamp") ? "left-0 z-10" : ""}`} |
🤖 Prompt for AI Agents
In `@packages/client/src/components/molecules/RaceTabMolecules/RaceTabTable.tsx`
at line 53, In the RaceTabTable component update the JSX className string (the
className prop that includes `${cell.id.includes("TimeStamp") ? "left-0 z-10" :
""}`) to fix the typo by replacing "w-fullpx-4" with two separate Tailwind
classes "w-full px-4" so the utility classes are correctly parsed.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.