fix(website): truthful group-ride creation — no false success, no fabricated GPS (Phase 3)
Creating a ride from the hub had two integrity problems: - The "Created … success" toast fired optimistically before the Firestore write, and a commit failure was only console.error'd — so a user could be told the ride was created when nothing persisted (the ride would then vanish on the next snapshot). - Every created ride wrote a hardcoded meetingPoint at 34.5378, -83.9849 (Helen, GA) regardless of the typed location — fabricated GPS attached to real rides. Fixes: - Report success only after `batch.commit()` resolves; on failure, roll back the optimistic insert and show an error. When there is no authenticated session / Firestore, keep the local entry for responsiveness but make no cloud-success claim. - Extract buildWebGroupRidePayload (pure): stores the rally point as displayName/address text and omits latitude/longitude (the web form has no map picker / geocoder, so we never fabricate coordinates). mapFirestoreRideToSummary already treats a missing coordinate as "not pinned yet". Join-by-code, join/leave, edit, and delete flows were reviewed and persist/authorize correctly; no changes needed there. Tests: buildWebGroupRidePayload asserts no fabricated lat/lon, rally-point text/fallback, authoritative organizer/access fields, scheduled-time handling, and club identity. Existing group-ride UI tests remain green. Known limitation: true address autocomplete and automatic route-distance calculation require a geocoding/routing API that does not exist in this codebase; distance and rally point remain free-text until that backend lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Developer
parent
ae1980b4ed
commit
6956e809a3
|
|
@ -23,6 +23,7 @@ import {
|
|||
connectFirestoreEmulator,
|
||||
} from 'firebase/firestore';
|
||||
import { firestore } from '../../lib/firebase-client';
|
||||
import { buildWebGroupRidePayload } from '../../lib/group-ride-payload';
|
||||
|
||||
function getActiveFirestore() {
|
||||
if (
|
||||
|
|
@ -1000,12 +1001,12 @@ export function GroupRidesManagement({
|
|||
}, [currentUser?.uid]);
|
||||
|
||||
// Handlers for Add, Alter, Remove, Join, Leave
|
||||
const handleCreateRide = async (newRide: GroupRideSummary & { visibility?: string; clubId?: string }) => {
|
||||
const handleCreateRide = async (newRide: GroupRideSummary & { visibility?: string; clubId?: string; clubName?: string }) => {
|
||||
const uid = currentUser?.uid;
|
||||
const now = new Date();
|
||||
const rideId = uid
|
||||
? `ride_${now.getTime()}_${uid.substring(0, Math.min(6, uid.length))}`
|
||||
: `MS-${Math.floor(1000 + Math.random() * 9000)}`;
|
||||
: `local_${now.getTime()}`;
|
||||
|
||||
const rideWithOrganizer: GroupRideSummary = {
|
||||
...newRide,
|
||||
|
|
@ -1018,80 +1019,49 @@ export function GroupRidesManagement({
|
|||
leadAvatarText: (currentUser?.displayName?.[0] || currentUser?.email?.[0] || 'M').toUpperCase(),
|
||||
};
|
||||
|
||||
// Optimistic local update
|
||||
// Optimistic local insert — rolled back below if a backend write is attempted and fails.
|
||||
setRides((prev) => [rideWithOrganizer, ...prev]);
|
||||
showAlert(`Created new group ride "${newRide.title}"! Ride Code: ${rideId}`, 'success');
|
||||
|
||||
if (uid && firestore) {
|
||||
try {
|
||||
const leadDisplayName = currentUser.displayName || currentUser.email?.split('@')[0] || 'Organizer';
|
||||
const leadHandle = currentUser.displayName?.toLowerCase().replace(/\s+/g, '_') || currentUser.email?.split('@')[0] || 'rider';
|
||||
// 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.
|
||||
if (!uid || !firestore) {
|
||||
return;
|
||||
}
|
||||
|
||||
let scheduledStartAt = new Date(now.getTime() + 3600000).toISOString();
|
||||
let departureAt = new Date(now.getTime() + 3600000 + 1800000).toISOString();
|
||||
try {
|
||||
const leadDisplayName = currentUser.displayName || currentUser.email?.split('@')[0] || 'Organizer';
|
||||
const leadHandle = currentUser.displayName?.toLowerCase().replace(/\s+/g, '_') || currentUser.email?.split('@')[0] || 'rider';
|
||||
|
||||
if (newRide.scheduledDate) {
|
||||
const parsed = new Date(`${newRide.scheduledDate} ${newRide.scheduledTime}`);
|
||||
if (!isNaN(parsed.getTime()) && parsed.getTime() > now.getTime() - 86400000) {
|
||||
scheduledStartAt = parsed.toISOString();
|
||||
departureAt = new Date(parsed.getTime() + 30 * 60000).toISOString();
|
||||
}
|
||||
}
|
||||
const firestoreRideData = buildWebGroupRidePayload(newRide, {
|
||||
rideId,
|
||||
uid,
|
||||
leadHandle,
|
||||
leadDisplayName,
|
||||
now,
|
||||
});
|
||||
|
||||
const firestoreRideData: any = {
|
||||
id: rideId,
|
||||
organizerUid: uid,
|
||||
organizerHandle: leadHandle,
|
||||
organizerDisplayName: leadDisplayName,
|
||||
title: newRide.title.trim(),
|
||||
description: newRide.description?.trim() || '',
|
||||
visibility: newRide.visibility || 'public',
|
||||
joinPolicy: newRide.joinPolicy || 'requestToJoin',
|
||||
status: 'scheduled',
|
||||
scheduledStartAt,
|
||||
departureAt,
|
||||
participantLimit: Number(newRide.participantLimit) || 12,
|
||||
participantCount: 1,
|
||||
accessUids: [uid],
|
||||
meetingPoint: {
|
||||
latitude: 34.5378,
|
||||
longitude: -83.9849,
|
||||
displayName: newRide.meetingPoint || newRide.region || 'Designated Rally Point',
|
||||
address: newRide.region || '',
|
||||
locationType: 'meetup',
|
||||
notes: '',
|
||||
},
|
||||
stops: [],
|
||||
createdAt: now.toISOString(),
|
||||
updatedAt: now.toISOString(),
|
||||
paceClassification: newRide.paceClassification || 'Brisk Pace',
|
||||
discipline: newRide.discipline || 'Sport / Canyon',
|
||||
formationProfile: newRide.formationProfile || 'Staggered Dual Column [B]',
|
||||
routeTitle: newRide.routeSnippet || '',
|
||||
};
|
||||
const organizerParticipant = {
|
||||
userId: uid,
|
||||
role: 'organizer',
|
||||
status: 'accepted',
|
||||
riderHandle: leadHandle,
|
||||
riderDisplayName: leadDisplayName,
|
||||
createdAt: now.toISOString(),
|
||||
updatedAt: now.toISOString(),
|
||||
};
|
||||
|
||||
if (newRide.clubId) {
|
||||
firestoreRideData.clubId = newRide.clubId;
|
||||
if (newRide.clubName) firestoreRideData.clubName = newRide.clubName;
|
||||
}
|
||||
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();
|
||||
|
||||
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();
|
||||
} catch (err: any) {
|
||||
console.error('Failed to persist group ride to Firestore:', err);
|
||||
}
|
||||
// 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');
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,81 @@
|
|||
import type { GroupRideSummary } from '../components/group-rides/types';
|
||||
|
||||
export interface GroupRidePayloadContext {
|
||||
rideId: string;
|
||||
uid: string;
|
||||
leadHandle: string;
|
||||
leadDisplayName: string;
|
||||
now: Date;
|
||||
}
|
||||
|
||||
export type WebGroupRideInput = GroupRideSummary & {
|
||||
visibility?: string;
|
||||
clubId?: string;
|
||||
clubName?: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Builds the Firestore `group_rides/{id}` document for a ride created from the web hub.
|
||||
*
|
||||
* The web create form captures a free-text rally point only (there is no map picker or
|
||||
* geocoder), so the meeting point stores `displayName`/`address` text and deliberately omits
|
||||
* `latitude`/`longitude`. We never fabricate coordinates: a missing coordinate means
|
||||
* "location not pinned yet", which `mapFirestoreRideToSummary` already handles (it derives
|
||||
* `meetingPointCoords` only when both lat and lon are present).
|
||||
*
|
||||
* Pure and dependency-free so it can be unit-tested without Firebase.
|
||||
*/
|
||||
export function buildWebGroupRidePayload(
|
||||
newRide: WebGroupRideInput,
|
||||
ctx: GroupRidePayloadContext
|
||||
): Record<string, unknown> {
|
||||
const { rideId, uid, leadHandle, leadDisplayName, now } = ctx;
|
||||
|
||||
let scheduledStartAt = new Date(now.getTime() + 3600000).toISOString();
|
||||
let departureAt = new Date(now.getTime() + 3600000 + 1800000).toISOString();
|
||||
|
||||
if (newRide.scheduledDate) {
|
||||
const parsed = new Date(`${newRide.scheduledDate} ${newRide.scheduledTime}`);
|
||||
if (!isNaN(parsed.getTime()) && parsed.getTime() > now.getTime() - 86400000) {
|
||||
scheduledStartAt = parsed.toISOString();
|
||||
departureAt = new Date(parsed.getTime() + 30 * 60000).toISOString();
|
||||
}
|
||||
}
|
||||
|
||||
const payload: Record<string, unknown> = {
|
||||
id: rideId,
|
||||
organizerUid: uid,
|
||||
organizerHandle: leadHandle,
|
||||
organizerDisplayName: leadDisplayName,
|
||||
title: newRide.title.trim(),
|
||||
description: newRide.description?.trim() || '',
|
||||
visibility: newRide.visibility || 'public',
|
||||
joinPolicy: newRide.joinPolicy || 'requestToJoin',
|
||||
status: 'scheduled',
|
||||
scheduledStartAt,
|
||||
departureAt,
|
||||
participantLimit: Number(newRide.participantLimit) || 12,
|
||||
participantCount: 1,
|
||||
accessUids: [uid],
|
||||
meetingPoint: {
|
||||
displayName: newRide.meetingPoint || newRide.region || 'Designated Rally Point',
|
||||
address: newRide.region || '',
|
||||
locationType: 'meetup',
|
||||
notes: '',
|
||||
},
|
||||
stops: [],
|
||||
createdAt: now.toISOString(),
|
||||
updatedAt: now.toISOString(),
|
||||
paceClassification: newRide.paceClassification || 'Brisk Pace',
|
||||
discipline: newRide.discipline || 'Sport / Canyon',
|
||||
formationProfile: newRide.formationProfile || 'Staggered Dual Column [B]',
|
||||
routeTitle: newRide.routeSnippet || '',
|
||||
};
|
||||
|
||||
if (newRide.clubId) {
|
||||
payload.clubId = newRide.clubId;
|
||||
if (newRide.clubName) payload.clubName = newRide.clubName;
|
||||
}
|
||||
|
||||
return payload;
|
||||
}
|
||||
|
|
@ -0,0 +1,79 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import { buildWebGroupRidePayload, type WebGroupRideInput } from '../lib/group-ride-payload';
|
||||
|
||||
const baseRide: WebGroupRideInput = {
|
||||
id: 'ride-temp',
|
||||
title: ' Canyon Dawn Patrol ',
|
||||
description: ' Fast sweepers ',
|
||||
region: 'Santa Cruz Mountains',
|
||||
scheduledDate: '',
|
||||
scheduledTime: '',
|
||||
paceClassification: 'Brisk Pace',
|
||||
discipline: 'Sport / Canyon',
|
||||
formationProfile: 'Staggered Dual Column [B]',
|
||||
joinPolicy: 'requestToJoin',
|
||||
leadHandle: 'You (Organizer)',
|
||||
leadAvatarText: 'YOU',
|
||||
participantCount: 1,
|
||||
participantLimit: 12,
|
||||
distance: '50.0 Miles',
|
||||
meetingPoint: 'Alice’s Restaurant, Skyline Blvd',
|
||||
routeSnippet: 'Skyline > Hwy 9',
|
||||
userStatus: 'joined',
|
||||
status: 'scheduled',
|
||||
};
|
||||
|
||||
const ctx = {
|
||||
rideId: 'ride_123_abc',
|
||||
uid: 'rider-uid-1',
|
||||
leadHandle: 'rider_one',
|
||||
leadDisplayName: 'Rider One',
|
||||
now: new Date('2026-10-14T12:00:00.000Z'),
|
||||
};
|
||||
|
||||
describe('buildWebGroupRidePayload (WEB Phase 3 ride creation)', () => {
|
||||
it('never fabricates meeting-point coordinates', () => {
|
||||
const p = buildWebGroupRidePayload(baseRide, ctx) as any;
|
||||
expect(p.meetingPoint.latitude).toBeUndefined();
|
||||
expect(p.meetingPoint.longitude).toBeUndefined();
|
||||
expect('latitude' in p.meetingPoint).toBe(false);
|
||||
expect('longitude' in p.meetingPoint).toBe(false);
|
||||
// The rider's typed rally point is preserved as text.
|
||||
expect(p.meetingPoint.displayName).toBe('Alice’s Restaurant, Skyline Blvd');
|
||||
expect(p.meetingPoint.address).toBe('Santa Cruz Mountains');
|
||||
});
|
||||
|
||||
it('falls back to the region for the rally point when none is typed', () => {
|
||||
const p = buildWebGroupRidePayload({ ...baseRide, meetingPoint: '' }, ctx) as any;
|
||||
expect(p.meetingPoint.displayName).toBe('Santa Cruz Mountains');
|
||||
});
|
||||
|
||||
it('sets authoritative organizer and access fields and trims text', () => {
|
||||
const p = buildWebGroupRidePayload(baseRide, ctx) as any;
|
||||
expect(p.id).toBe('ride_123_abc');
|
||||
expect(p.organizerUid).toBe('rider-uid-1');
|
||||
expect(p.accessUids).toEqual(['rider-uid-1']);
|
||||
expect(p.participantCount).toBe(1);
|
||||
expect(p.title).toBe('Canyon Dawn Patrol');
|
||||
expect(p.description).toBe('Fast sweepers');
|
||||
expect(typeof p.scheduledStartAt).toBe('string');
|
||||
expect(Number.isNaN(Date.parse(p.scheduledStartAt as string))).toBe(false);
|
||||
});
|
||||
|
||||
it('honors a valid future scheduled date/time', () => {
|
||||
const p = buildWebGroupRidePayload(
|
||||
{ ...baseRide, scheduledDate: '2026-12-25', scheduledTime: '09:30' },
|
||||
ctx
|
||||
) as any;
|
||||
expect(p.scheduledStartAt).toBe(new Date('2026-12-25 09:30').toISOString());
|
||||
});
|
||||
|
||||
it('includes club identity only when provided', () => {
|
||||
const withClub = buildWebGroupRidePayload({ ...baseRide, clubId: 'club-1', clubName: 'Apex MC' }, ctx) as any;
|
||||
expect(withClub.clubId).toBe('club-1');
|
||||
expect(withClub.clubName).toBe('Apex MC');
|
||||
|
||||
const withoutClub = buildWebGroupRidePayload(baseRide, ctx) as any;
|
||||
expect('clubId' in withoutClub).toBe(false);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue