diff --git a/website/components/group-rides/group-rides-management.tsx b/website/components/group-rides/group-rides-management.tsx index 50c4423..3a0ec1b 100644 --- a/website/components/group-rides/group-rides-management.tsx +++ b/website/components/group-rides/group-rides-management.tsx @@ -183,7 +183,7 @@ type MyRidesFilter = 'all' | 'organized' | 'joined'; interface CreateRideModalProps { isOpen: boolean; onClose: () => void; - onCreateRide: (ride: GroupRideSummary) => void | Promise; + onCreateRide: (ride: GroupRideSummary) => void | boolean | Promise; } function CreateGroupRideModal({ isOpen, onClose, onCreateRide }: CreateRideModalProps) { @@ -326,8 +326,10 @@ function CreateGroupRideModal({ isOpen, onClose, onCreateRide }: CreateRideModal setIsSubmitting(true); try { - await onCreateRide(newRide); - onClose(); + const outcome = await onCreateRide(newRide); + if (outcome !== false) { + onClose(); + } } finally { setIsSubmitting(false); } @@ -1014,9 +1016,9 @@ export function GroupRidesManagement({ }, [currentUser?.uid]); // Handlers for Add, Alter, Remove, Join, Leave - const handleCreateRide = async (newRide: GroupRideSummary & { visibility?: string; clubId?: string; clubName?: string }) => { + const handleCreateRide = async (newRide: GroupRideSummary & { visibility?: string; clubId?: string; clubName?: string }): Promise => { // Prevent duplicate submissions while a create is already in flight. - if (creatingRideRef.current) return; + if (creatingRideRef.current) return false; const uid = currentUser?.uid; // Check authentication and persistence availability BEFORE touching UI state, so an @@ -1024,7 +1026,7 @@ export function GroupRidesManagement({ // 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; + return false; } creatingRideRef.current = true; @@ -1055,12 +1057,15 @@ export function GroupRidesManagement({ }; setRides((prev) => (prev.some((r) => r.id === result.rideId) ? prev : [rideWithOrganizer, ...prev])); showAlert(`Created new group ride "${newRide.title}"! Ride Code: ${result.rideId}`, 'success'); + return true; } 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'); + return false; } else { // unauthenticated / unavailable discovered inside the helper (defensive — guarded above). showAlert('Could not create the ride right now. Please try again.', 'error'); + return false; } } finally { creatingRideRef.current = false; diff --git a/website/lib/create-group-ride.ts b/website/lib/create-group-ride.ts index aafc310..088dc41 100644 --- a/website/lib/create-group-ride.ts +++ b/website/lib/create-group-ride.ts @@ -37,10 +37,10 @@ export interface CommitGroupRideDeps { * - unavailable: no Firestore handle * - commit-failed: the write was attempted but rejected (nothing persisted) */ -let rideIdCounter = 0; +let lastRideTimestamp = 0; export function resetRideIdCounter(): void { - rideIdCounter = 0; + lastRideTimestamp = 0; } export async function commitGroupRide( @@ -52,9 +52,10 @@ export async function commitGroupRide( if (!deps.firestore) return { ok: false, reason: 'unavailable' }; const now = deps.now ?? new Date(); - const seq = ++rideIdCounter; - const seqSuffix = seq > 1 ? `${seq}` : ''; - const rideId = `ride_${now.getTime()}${seqSuffix}_${uid.substring(0, Math.min(6, uid.length))}`; + const rawMs = now.getTime(); + const timestamp = Math.max(rawMs, lastRideTimestamp + 1); + lastRideTimestamp = timestamp; + const rideId = `ride_${timestamp}_${uid.substring(0, Math.min(6, uid.length))}`; const payload = buildWebGroupRidePayload(newRide, { rideId, diff --git a/website/lib/telemetry-track.ts b/website/lib/telemetry-track.ts index 77f7baa..ec91254 100644 --- a/website/lib/telemetry-track.ts +++ b/website/lib/telemetry-track.ts @@ -62,22 +62,26 @@ export function advancePlaybackIndex( budgetMs: number ): { index: number; remainingMs: number } { if (points.length === 0) return { index: 0, remainingMs: 0 }; - let i = Math.max(0, Math.min(fromIndex, points.length - 1)); - let budget = Number.isFinite(budgetMs) ? Math.max(0, budgetMs) : 0; - while (i < points.length - 1) { - const connected = areTelemetryPointsConnected(points[i], points[i + 1]); + const i = Math.max(0, Math.min(fromIndex, points.length - 1)); + if (!Number.isFinite(budgetMs) || budgetMs <= 0) { + return { index: i, remainingMs: 0 }; + } + let budget = budgetMs; + let curr = i; + while (curr < points.length - 1) { + const connected = areTelemetryPointsConnected(points[curr], points[curr + 1]); const rawDt = connected - ? Date.parse(points[i + 1].timestamp) - Date.parse(points[i].timestamp) + ? Date.parse(points[curr + 1].timestamp) - Date.parse(points[curr].timestamp) : 50; const dt = Number.isFinite(rawDt) ? Math.max(0, rawDt) : 50; if (budget >= dt) { budget -= dt; - i++; + curr++; } else { break; } } - return { index: i, remainingMs: i < points.length - 1 ? budget : 0 }; + return { index: curr, remainingMs: curr < points.length - 1 ? budget : 0 }; } /** Leaflet's geographic API is [latitude, longitude], never normalized screen pixels. */ diff --git a/website/tests/create-group-ride.test.ts b/website/tests/create-group-ride.test.ts index b6cdac5..3a37d1f 100644 --- a/website/tests/create-group-ride.test.ts +++ b/website/tests/create-group-ride.test.ts @@ -1,5 +1,5 @@ -import { describe, it, expect, vi } from 'vitest'; -import { commitGroupRide } from '../lib/create-group-ride'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { commitGroupRide, resetRideIdCounter } from '../lib/create-group-ride'; import type { WebGroupRideInput } from '../lib/group-ride-payload'; const baseRide: WebGroupRideInput = { @@ -41,6 +41,10 @@ function makeDeps(overrides: Partial[1]> = {} } describe('commitGroupRide (WEB Phase 1 — persisted ride creation)', () => { + beforeEach(() => { + resetRideIdCounter(); + }); + it('refuses to create for an unauthenticated user (no uid) and never writes', async () => { const { deps, commitSpy } = makeDeps({ uid: undefined }); const res = await commitGroupRide(baseRide, deps); @@ -101,6 +105,24 @@ describe('commitGroupRide (WEB Phase 1 — persisted ride creation)', () => { } }); + it('preserves clean timestamps without string suffix corruption on successive rides', async () => { + const t1 = new Date('2026-10-14T12:00:00.000Z'); + const t2 = new Date('2026-10-14T12:05:00.000Z'); + const { deps: deps1 } = makeDeps({ now: t1 }); + const { deps: deps2 } = makeDeps({ now: t2 }); + + const res1 = await commitGroupRide(baseRide, deps1); + const res2 = await commitGroupRide(baseRide, deps2); + + expect(res1.ok).toBe(true); + expect(res2.ok).toBe(true); + if (res1.ok && res2.ok) { + // Must equal exact millisecond timestamp, NOT corrupted with appended counter digits + expect(res1.rideId).toBe(`ride_${t1.getTime()}_rider-`); + expect(res2.rideId).toBe(`ride_${t2.getTime()}_rider-`); + } + }); + it('handles permission-denied or rejected commit cleanly without throwing', async () => { const permDenied = new Error('permission-denied: caller lacks organizer permission'); const { deps } = makeDeps({ diff --git a/website/tests/group-rides-page.test.tsx b/website/tests/group-rides-page.test.tsx index d391d7d..47889ba 100644 --- a/website/tests/group-rides-page.test.tsx +++ b/website/tests/group-rides-page.test.tsx @@ -488,6 +488,9 @@ describe('Group Rides Interactive Page & Components', () => { await screen.findByText(/Could not create the ride/i); expect(screen.queryByRole('heading', { name: /Doomed Ride/i })).toBeNull(); + // Form inputs remain preserved in the modal so the user does not lose their typed data + expect(screen.getByLabelText(/ride title/i).value).toBe('Doomed Ride'); + expect(screen.getByLabelText(/region \/ city/i).value).toBe('Nowhere'); }); it('enforces commit-before-UI: never inserts into UI state while commit is in-flight', async () => { diff --git a/website/tests/telemetry-playback-sync.test.ts b/website/tests/telemetry-playback-sync.test.ts index effeef2..e901897 100644 --- a/website/tests/telemetry-playback-sync.test.ts +++ b/website/tests/telemetry-playback-sync.test.ts @@ -97,6 +97,17 @@ describe('replay regression tests: irregular timestamps', () => { expect(r.remainingMs).toBe(0); }); + it('does not advance on zero or negative budget even if adjacent samples have identical timestamp (dt === 0)', () => { + const dups = [mk(0), mk(0), mk(50)]; + const rZero = advancePlaybackIndex(dups, 0, 0); + expect(rZero.index).toBe(0); + expect(rZero.remainingMs).toBe(0); + + const rNeg = advancePlaybackIndex(dups, 0, -10); + expect(rNeg.index).toBe(0); + expect(rNeg.remainingMs).toBe(0); + }); + it('treats inverted / backwards timestamps as disconnected (50 ms step) instead of looping or stalling', () => { // Sample 2 is out of order (timestamp goes backwards) const backwards = [mk(0), mk(50), mk(30), mk(100)];