From dd35b216d1b77aa167061671b01973a6dbc6558e Mon Sep 17 00:00:00 2001 From: Developer Date: Wed, 14 Oct 2026 13:12:49 -0400 Subject: [PATCH] fix(website): persisted-only ride creation and reconciled replay timing (PR #38 hardening) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 — group ride creation (no phantom rides): - Check authentication and Firestore availability BEFORE touching UI state; an unauthenticated user (or unavailable Firestore) now gets an actionable error and no phantom ride, instead of a local entry that was never saved. - Insert the ride into UI state ONLY after batch.commit() confirms the write; on commit failure nothing is inserted and an error is shown. - Prevent duplicate submissions: the create modal disables its submit button while in flight, and the hub guards with a ref. - The join code remains the real Firestore document id (no fabricated ids). - Extract commitGroupRide (pure, dependency-injected) so every path is unit-testable. Phase 2 — replay playback timing: - The elapsed clock and rider marker both derive from the same cursor, so they cannot drift. - Manual seeks (scrub, restart, corner jump, arrow keys) go through seekTo(), which reconciles playback timing (clears carry/last-frame) so no stale budget is applied after a seek. - Make gap compression explicit: a caption labels the timeline as recorded elapsed time and warns that dead time in multi-segment recordings is skipped during playback. Tests: commitGroupRide (unauthenticated / unavailable / commit-ok / commit-failed); group-rides hub now uses an authenticated mock with deterministic Firebase — authenticated create persists, unauthenticated create is blocked with no phantom, a failed commit leaves no phantom, and the create modal blocks duplicate submits. Co-Authored-By: Claude Opus 4.8 --- .../group-rides/create-ride-modal.tsx | 23 +++- .../group-rides/group-rides-management.tsx | 89 +++++++------- .../components/web-telemetry-replay-modal.tsx | 45 ++++++-- website/lib/create-group-ride.ts | 78 +++++++++++++ website/tests/create-group-ride.test.ts | 84 ++++++++++++++ website/tests/group-rides-page.test.tsx | 109 ++++++++++++++++-- 6 files changed, 359 insertions(+), 69 deletions(-) create mode 100644 website/lib/create-group-ride.ts create mode 100644 website/tests/create-group-ride.test.ts diff --git a/website/components/group-rides/create-ride-modal.tsx b/website/components/group-rides/create-ride-modal.tsx index 458eb51..6e996a3 100644 --- a/website/components/group-rides/create-ride-modal.tsx +++ b/website/components/group-rides/create-ride-modal.tsx @@ -6,7 +6,7 @@ import { GroupRideSummary } from './types'; interface CreateRideModalProps { isOpen: boolean; onClose: () => void; - onCreateRide: (ride: GroupRideSummary) => void; + onCreateRide: (ride: GroupRideSummary) => void | Promise; } export function CreateRideModal({ isOpen, onClose, onCreateRide }: CreateRideModalProps) { @@ -24,6 +24,7 @@ export function CreateRideModal({ isOpen, onClose, onCreateRide }: CreateRideMod const [participantLimit, setParticipantLimit] = useState(12); const [description, setDescription] = useState(''); const [errorMsg, setErrorMsg] = useState(null); + const [isSubmitting, setIsSubmitting] = useState(false); // Reset form when modal opens useEffect(() => { @@ -42,6 +43,7 @@ export function CreateRideModal({ isOpen, onClose, onCreateRide }: CreateRideMod setParticipantLimit(12); setDescription(''); setErrorMsg(null); + setIsSubmitting(false); } }, [isOpen]); @@ -58,9 +60,12 @@ export function CreateRideModal({ isOpen, onClose, onCreateRide }: CreateRideMod if (!isOpen) return null; - const handleSubmit = (e: React.FormEvent) => { + const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); + // Prevent duplicate submissions while a create is in flight. + if (isSubmitting) return; + if (!title.trim()) { setErrorMsg('Please enter a ride title.'); return; @@ -92,8 +97,13 @@ export function CreateRideModal({ isOpen, onClose, onCreateRide }: CreateRideMod status: 'scheduled', }; - onCreateRide(newRide); - onClose(); + setIsSubmitting(true); + try { + await onCreateRide(newRide); + onClose(); + } finally { + setIsSubmitting(false); + } }; return ( @@ -349,9 +359,10 @@ export function CreateRideModal({ isOpen, onClose, onCreateRide }: CreateRideMod diff --git a/website/components/group-rides/group-rides-management.tsx b/website/components/group-rides/group-rides-management.tsx index 8b92257..f1394f1 100644 --- a/website/components/group-rides/group-rides-management.tsx +++ b/website/components/group-rides/group-rides-management.tsx @@ -1,6 +1,6 @@ 'use client'; -import React, { useState, useMemo, useEffect } from 'react'; +import React, { useState, useMemo, useEffect, useRef } from 'react'; import Link from 'next/link'; import { getApps, initializeApp } from 'firebase/app'; import { getAuth, connectAuthEmulator } from 'firebase/auth'; @@ -23,7 +23,7 @@ import { connectFirestoreEmulator, } from 'firebase/firestore'; import { firestore } from '../../lib/firebase-client'; -import { buildWebGroupRidePayload } from '../../lib/group-ride-payload'; +import { commitGroupRide } from '../../lib/create-group-ride'; function getActiveFirestore() { if ( @@ -725,6 +725,8 @@ export function GroupRidesManagement({ onViewShowcase, }: GroupRidesManagementProps) { const [rides, setRides] = useState(INITIAL_GROUP_RIDES); + // Guards against duplicate create submissions while a write is in flight. + const creatingRideRef = useRef(false); const [communityRides, setCommunityRides] = useState([]); const [isCommunityLoading, setIsCommunityLoading] = useState(false); const [communityError, setCommunityError] = useState(null); @@ -1002,66 +1004,55 @@ export function GroupRidesManagement({ // Handlers for Add, Alter, Remove, Join, Leave const handleCreateRide = async (newRide: GroupRideSummary & { visibility?: string; clubId?: string; clubName?: string }) => { + // Prevent duplicate submissions while a create is already in flight. + if (creatingRideRef.current) return; + const uid = currentUser?.uid; - const now = new Date(); - const rideId = uid - ? `ride_${now.getTime()}_${uid.substring(0, Math.min(6, uid.length))}` - : `local_${now.getTime()}`; - - const rideWithOrganizer: GroupRideSummary = { - ...newRide, - id: rideId, - isOrganizer: true, - status: 'scheduled', - userStatus: 'joined', - rideCode: rideId, - leadHandle: currentUser?.displayName || currentUser?.email?.split('@')[0] || 'MyHandle', - leadAvatarText: (currentUser?.displayName?.[0] || currentUser?.email?.[0] || 'M').toUpperCase(), - }; - - // Optimistic local insert — rolled back below if a backend write is attempted and fails. - setRides((prev) => [rideWithOrganizer, ...prev]); - - // Without an authenticated session and Firestore we cannot persist the ride. Keep the - // local entry for responsiveness, but never claim it was saved to the cloud. + // Check authentication and persistence availability BEFORE touching UI state, so an + // unauthenticated user (or an unavailable Firestore) never sees a phantom ride that was + // never saved. They get an actionable error instead. if (!uid || !firestore) { + showAlert('Sign in to create a group ride — it could not be saved.', 'error'); return; } + creatingRideRef.current = true; try { const leadDisplayName = currentUser.displayName || currentUser.email?.split('@')[0] || 'Organizer'; const leadHandle = currentUser.displayName?.toLowerCase().replace(/\s+/g, '_') || currentUser.email?.split('@')[0] || 'rider'; - const firestoreRideData = buildWebGroupRidePayload(newRide, { - rideId, + const result = await commitGroupRide(newRide, { uid, - leadHandle, + firestore, leadDisplayName, - now, + leadHandle, + writeBatch: (fs) => writeBatch(fs), + docRef: (fs, ...segments) => doc(fs, segments[0], ...segments.slice(1)), }); - const organizerParticipant = { - userId: uid, - role: 'organizer', - status: 'accepted', - riderHandle: leadHandle, - riderDisplayName: leadDisplayName, - createdAt: now.toISOString(), - updatedAt: now.toISOString(), - }; - - const batch = writeBatch(firestore); - batch.set(doc(firestore, 'group_rides', rideId), firestoreRideData); - batch.set(doc(firestore, 'group_rides', rideId, 'participants', uid), organizerParticipant); - await batch.commit(); - - // Report success only after the write actually commits. - showAlert(`Created new group ride "${newRide.title}"! Ride Code: ${rideId}`, 'success'); - } catch (err: any) { - console.error('Failed to persist group ride to Firestore:', err); - // Roll back the optimistic insert and report the failure truthfully. - setRides((prev) => prev.filter((r) => r.id !== rideId)); - showAlert('Could not create the ride. Please check your connection and try again.', 'error'); + if (result.ok) { + // Insert into UI state ONLY after Firestore confirms the write — no phantom rides. + const rideWithOrganizer: GroupRideSummary = { + ...newRide, + id: result.rideId, + isOrganizer: true, + status: 'scheduled', + userStatus: 'joined', + rideCode: result.rideId, + leadHandle: currentUser?.displayName || currentUser?.email?.split('@')[0] || 'MyHandle', + leadAvatarText: (currentUser?.displayName?.[0] || currentUser?.email?.[0] || 'M').toUpperCase(), + }; + setRides((prev) => (prev.some((r) => r.id === result.rideId) ? prev : [rideWithOrganizer, ...prev])); + showAlert(`Created new group ride "${newRide.title}"! Ride Code: ${result.rideId}`, 'success'); + } else if (result.reason === 'commit-failed') { + console.error('Failed to persist group ride to Firestore:', result.error); + showAlert('Could not create the ride. Please check your connection and try again.', 'error'); + } else { + // unauthenticated / unavailable discovered inside the helper (defensive — guarded above). + showAlert('Could not create the ride right now. Please try again.', 'error'); + } + } finally { + creatingRideRef.current = false; } }; diff --git a/website/components/web-telemetry-replay-modal.tsx b/website/components/web-telemetry-replay-modal.tsx index 9349b81..a326c77 100644 --- a/website/components/web-telemetry-replay-modal.tsx +++ b/website/components/web-telemetry-replay-modal.tsx @@ -131,6 +131,26 @@ export function WebTelemetryReplayModal({ ride, onClose }: WebTelemetryReplayMod indexRef.current = currentIndex; }, [currentIndex]); + // True when the recording has disconnected segments / GPS-loss gaps. Playback compresses + // those gaps (advancePlaybackIndex), so the elapsed clock — which shows real recorded time — + // will jump forward at a gap even though wall-clock playback does not pause. We surface this + // explicitly rather than let the jump look like a glitch. + const hasCompressedGaps = useMemo( + () => points.some((p, i) => i > 0 && !areTelemetryPointsConnected(points[i - 1], p)), + [points] + ); + + // Manual seek (scrub, restart, corner jump, arrow keys): reconcile playback timing so the + // next animation frame does not apply stale budget accumulated before the seek. + const seekTo = (index: number) => { + const maxIndex = Math.max(0, totalPoints - 1); + const clamped = Math.max(0, Math.min(maxIndex, index)); + playbackCarryRef.current = 0; + lastTimeRef.current = null; + indexRef.current = clamped; + setCurrentIndex(clamped); + }; + // Dynamically load Leaflet library and CSS with multi-CDN fallback useEffect(() => { if (typeof window === 'undefined') return; @@ -443,7 +463,7 @@ export function WebTelemetryReplayModal({ ride, onClose }: WebTelemetryReplayMod { direction: 'top' } ); cornerMarker.on('click', () => { - setCurrentIndex(corner.apexIndex); + seekTo(corner.apexIndex); }); routeGroup.addLayer(cornerMarker); } @@ -666,9 +686,9 @@ export function WebTelemetryReplayModal({ ride, onClose }: WebTelemetryReplayMod e.preventDefault(); setIsPlaying((prev) => !prev); } else if (e.code === 'ArrowRight' && totalPoints > 0) { - setCurrentIndex((prev) => Math.min(totalPoints - 1, prev + 20)); + seekTo(indexRef.current + 20); } else if (e.code === 'ArrowLeft') { - setCurrentIndex((prev) => Math.max(0, prev - 20)); + seekTo(indexRef.current - 20); } else if (e.code === 'Escape') { onClose(); } @@ -1064,7 +1084,7 @@ export function WebTelemetryReplayModal({ ride, onClose }: WebTelemetryReplayMod key={corner.id} className="hover:bg-brand-cardAlt/50 transition cursor-pointer" onClick={() => { - setCurrentIndex(corner.apexIndex); + seekTo(corner.apexIndex); setActiveTab('map'); }} > @@ -1105,7 +1125,7 @@ export function WebTelemetryReplayModal({ ride, onClose }: WebTelemetryReplayMod