Added button w/ password to update finish line location#274
Open
alexwhelan12 wants to merge 3 commits intomainfrom
Open
Added button w/ password to update finish line location#274alexwhelan12 wants to merge 3 commits intomainfrom
alexwhelan12 wants to merge 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds a UI component to set the finish line location that emits a socket event with a password; socket handler now accepts a callback and returns a CoordUpdateResponse; server password constant for finish line updates switched to FINISH_LINE_UPDATE_PASSWORD. Changes
Sequence DiagramsequenceDiagram
actor User
participant ClientUI as Client UI
participant Socket as Socket.IO Handler
participant Controller as Lap Controller
participant Broadcast as Broadcast/Other Clients
User->>ClientUI: enter password & click set finish line
ClientUI->>Socket: emit setLapCoords(coords, callback)
activate Socket
Socket->>Controller: setFinishLineLocation(coords, password)
activate Controller
Controller->>Controller: validate password (FINISH_LINE_UPDATE_PASSWORD)
Controller->>Controller: convert/assign coordinates
Controller-->>Socket: return CoordUpdateResponse
deactivate Controller
alt response has no error
Socket->>Broadcast: broadcast lap coords
else response has error
Socket->>Socket: skip broadcast
end
Socket->>ClientUI: invoke callback(response)
deactivate Socket
alt response has error
ClientUI->>User: show error notification
else success
ClientUI->>User: show success notification
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/client/src/components/molecules/LogoStatusMolecules/Settings/SetFinishLineLocation.tsx`:
- Around line 16-40: handleSetFinishLineLocation currently calls
(currentPacket?.Telemetry.GpsLatitude).toString() and
(currentPacket?.Telemetry.GpsLongitude).toString() which can throw if
currentPacket or its Telemetry props are undefined; ensure you safely coalesce
before calling toString by extracting numeric/string values defensively (e.g.,
compute latVal = currentPacket?.Telemetry?.GpsLatitude ?? 0 and longVal =
currentPacket?.Telemetry?.GpsLongitude ?? 0) and then call toString() on those
coalesced values, and use these latVal.toString()/longVal.toString() in the
socketIO.emit payload so socketIO.emit and the CoordUpdateResponse handling
remain unchanged.
🧹 Nitpick comments (2)
packages/server/src/datasources/SocketIO/SocketIO.ts (1)
71-92: LGTM with a minor observation.The callback-based response mechanism is properly implemented: broadcasting only on success while always invoking the callback ensures the client receives feedback regardless of outcome.
Minor note:
passwordis destructured on line 77 but not used directly sincenewCoordInfois passed tosetFinishLineLocation. Consider removing it from the destructuring if it's only for logging purposes (thoughlatandlongare logged, so they're used).♻️ Optional cleanup
( newCoordInfo: CoordInfoUpdate, callback: (response: CoordUpdateResponse) => void, ) => { - const { lat, long, password } = newCoordInfo; + const { lat, long } = newCoordInfo; logger.info("UPDATED COORDS: "); logger.info("lat: ", lat, "long: ", long);packages/client/src/components/molecules/LogoStatusMolecules/Settings/SetFinishLineLocation.tsx (1)
48-74: Consider adding a loading state during the async operation.The button can be clicked multiple times while awaiting the server response, potentially causing duplicate requests. Consider adding a loading state to improve UX.
♻️ Suggested improvement
const SetFinishLineLocation = () => { const { currentPacket } = usePacketStore(); const { resolvedTheme } = useTheme(); const [password, setPassword] = React.useState(""); + const [isLoading, setIsLoading] = React.useState(false); const handleSetFinishLineLocation = () => { + setIsLoading(true); socketIO.emit( "setLapCoords", { lat: String(currentPacket?.Telemetry?.GpsLatitude ?? 0), long: String(currentPacket?.Telemetry?.GpsLongitude ?? 0), password: password, }, (response: CoordUpdateResponse) => { + setIsLoading(false); if ("error" in response) { // ... error handling } // ... success handling }, ); };Then update the button:
<Button className="h-14 min-w-0 flex-1" - disabled={!password} + disabled={!password || isLoading} onClick={handleSetFinishLineLocation}
packages/client/src/components/molecules/LogoStatusMolecules/Settings/SetFinishLineLocation.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/server/src/datasources/SocketIO/SocketIO.ts`:
- Around line 71-91: The handler registered in socket.on for "setLapCoords"
currently invokes callback(res) unconditionally which will throw if the client
omitted the acknowledgement; update the handler (the function receiving
newCoordInfo: CoordInfoUpdate and callback: (response: CoordUpdateResponse) =>
void) to treat callback as optional and only call it when typeof callback ===
"function" (or callback != null and is a function), leaving the rest of the flow
intact (invoke
this.backendController.lapController.setFinishLineLocation(newCoordInfo),
broadcast via this.broadcastLapCoords(res) on success, then guard the final
callback invocation).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/client/src/components/molecules/LogoStatusMolecules/Settings/SetFinishLineLocation.tsx`:
- Around line 16-39: The response handler in handleSetFinishLineLocation treats
any response without an "error" key as success; update the socketIO.emit
callback that receives CoordUpdateResponse to also treat responses containing a
non-empty invalidFields property as errors: check if ("error" in response) ||
("invalidFields" in response && response.invalidFields?.length > 0) and call
notifications.show with a red/error notification using response.error ||
response.invalidFields.join(", ") || "Invalid fields in request" as the message;
only show the green success notification when neither error nor invalidFields
are present.
Comment on lines
+16
to
+39
| const handleSetFinishLineLocation = () => { | ||
| socketIO.emit( | ||
| "setLapCoords", | ||
| { | ||
| lat: String(currentPacket?.Telemetry?.GpsLatitude ?? 0), | ||
| long: String(currentPacket?.Telemetry?.GpsLongitude ?? 0), | ||
| password: password, | ||
| }, | ||
| (response: CoordUpdateResponse) => { | ||
| if ("error" in response) { | ||
| notifications.show({ | ||
| color: "red", | ||
| message: response.error, | ||
| title: "Error Setting Finish Line Location", | ||
| }); | ||
| return; | ||
| } | ||
| notifications.show({ | ||
| color: "green", | ||
| message: "Finish Line Location Updated Successfully", | ||
| title: "Success", | ||
| }); | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
Handle invalidFields-only responses as errors.
CoordUpdateResponse can include invalidFields without an error message. In that case, the current logic shows a success notification or an empty error message. Consider treating invalidFields as an error with a fallback message.
💡 Suggested fix
(response: CoordUpdateResponse) => {
- if ("error" in response) {
+ if (
+ ("error" in response && response.error) ||
+ ("invalidFields" in response && response.invalidFields?.length)
+ ) {
notifications.show({
color: "red",
- message: response.error,
+ message:
+ response.error ??
+ `Invalid fields: ${response.invalidFields?.join(", ")}`,
title: "Error Setting Finish Line Location",
});
return;
}📝 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
| const handleSetFinishLineLocation = () => { | |
| socketIO.emit( | |
| "setLapCoords", | |
| { | |
| lat: String(currentPacket?.Telemetry?.GpsLatitude ?? 0), | |
| long: String(currentPacket?.Telemetry?.GpsLongitude ?? 0), | |
| password: password, | |
| }, | |
| (response: CoordUpdateResponse) => { | |
| if ("error" in response) { | |
| notifications.show({ | |
| color: "red", | |
| message: response.error, | |
| title: "Error Setting Finish Line Location", | |
| }); | |
| return; | |
| } | |
| notifications.show({ | |
| color: "green", | |
| message: "Finish Line Location Updated Successfully", | |
| title: "Success", | |
| }); | |
| }, | |
| ); | |
| const handleSetFinishLineLocation = () => { | |
| socketIO.emit( | |
| "setLapCoords", | |
| { | |
| lat: String(currentPacket?.Telemetry?.GpsLatitude ?? 0), | |
| long: String(currentPacket?.Telemetry?.GpsLongitude ?? 0), | |
| password: password, | |
| }, | |
| (response: CoordUpdateResponse) => { | |
| if ( | |
| ("error" in response && response.error) || | |
| ("invalidFields" in response && response.invalidFields?.length) | |
| ) { | |
| notifications.show({ | |
| color: "red", | |
| message: | |
| response.error ?? | |
| `Invalid fields: ${response.invalidFields?.join(", ")}`, | |
| title: "Error Setting Finish Line Location", | |
| }); | |
| return; | |
| } | |
| notifications.show({ | |
| color: "green", | |
| message: "Finish Line Location Updated Successfully", | |
| title: "Success", | |
| }); | |
| }, | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In
`@packages/client/src/components/molecules/LogoStatusMolecules/Settings/SetFinishLineLocation.tsx`
around lines 16 - 39, The response handler in handleSetFinishLineLocation treats
any response without an "error" key as success; update the socketIO.emit
callback that receives CoordUpdateResponse to also treat responses containing a
non-empty invalidFields property as errors: check if ("error" in response) ||
("invalidFields" in response && response.invalidFields?.length > 0) and call
notifications.show with a red/error notification using response.error ||
response.invalidFields.join(", ") || "Invalid fields in request" as the message;
only show the green success notification when neither error nor invalidFields
are present.
burtonjong
requested changes
Jan 24, 2026
| ): CoordUpdateResponse { | ||
| logger.info(JSON.stringify(newCoordInfo)); | ||
| const { lat, long, password } = newCoordInfo; | ||
| if (password !== process.env.LAP_POSITION_PASSWORD) { |
Member
There was a problem hiding this comment.
If we are renaming this we have to remove references of lap_position_password
| <label className="mr-2">Update Finish Line Location:</label> | ||
| </div> | ||
| <div className="col-span-1 flex flex-col items-stretch gap-2 sm:flex-row sm:items-center"> | ||
| <Button |
Member
There was a problem hiding this comment.
im going to create a ticket to remove mui from our repo bro this is crazy
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
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
New Features
Behavior Changes
✏️ Tip: You can customize this high-level summary in your review settings.