Address review: export votes hardening + 404 enumeration fix

- Default a missing vote `plus` to 0 instead of emitting "(undefined)".
- Convert fetch failures (network error, non-2xx, invalid JSON) into a
  400 with a clear message instead of an opaque 500.
- Fetch session vote docs concurrently (Promise.all) and pre-index vote
  items by id, avoiding sequential awaits and repeated .find() on large
  events.
- Org-key path: a non-existent project and a wrong-org project now return
  the identical 404 "Event not found", so event ids stay non-enumerable.

Tests cover the unified 404, the plus default, and the fetch-error 400.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: HugoGresse

functions/src/api/routes/events/getVotes.spec.ts

@@ -14,7 +14,14 @@ const seededProject = {
     voteItems: [{ id: 'q1', name: 'Quality' }],
     _collections: {
         sessionVotes: [
-            { id: 'session1', q1: { uidA: { text: 'Great talk', plus: 2 } } },
+            {
+                id: 'session1',
+                // uidB has no `plus` — must default to 0, not "(undefined)".
+                q1: {
+                    uidA: { text: 'Great talk', plus: 2 },
+                    uidB: { text: 'Loved it' },
+                },
+            },
         ],
     },
 }
@@ -32,10 +39,10 @@ const openFeedbackJson = {
     speakers: { spk1: { name: 'Alice' } },
 }
 
-const stubFetch = (json: unknown = openFeedbackJson) =>
+const stubFetch = (json: unknown = openFeedbackJson, ok = true, status = 200) =>
     vi.stubGlobal(
         'fetch',
-        vi.fn(async () => ({ json: async () => json }))
+        vi.fn(async () => ({ ok, status, json: async () => json }))
     )
 
 // Stub the private-subcollection key resolution used by authenticateRequest:
@@ -76,7 +83,7 @@ const expectedRow = {
     speakersName: 'Alice',
     tags: 'frontend',
     trackTitle: 'Track A',
-    Quality: 'Great talk (2)',
+    Quality: 'Great talk (2), Loved it (0)',
 }
 
 describe('/events/:projectId/votes', () => {
@@ -158,6 +165,34 @@ describe('/events/:projectId/votes', () => {
         })
 
         expect(response.statusCode).toBe(404)
+        expect(JSON.parse(response.body).error).toBe('Event not found')
+    })
+
+    it('returns the same 404 for a non-existent event (not enumerable)', async () => {
+        mockOrgKey({ id: 'org_123' })
+
+        const response = await fastify.inject({
+            method: 'GET',
+            url: '/events/proj_missing/votes',
+            headers: { 'x-api-key': 'oforg_test-key' },
+        })
+
+        expect(response.statusCode).toBe(404)
+        // Identical message to the wrong-org case above: ids stay opaque.
+        expect(JSON.parse(response.body).error).toBe('Event not found')
+    })
+
+    it('returns 400 when the public data URL responds with an error', async () => {
+        stubFetch(openFeedbackJson, false, 502)
+        mockProjectKey()
+
+        const response = await fastify.inject({
+            method: 'GET',
+            url: '/events/proj_123/votes',
+            headers: { 'x-api-key': 'ofproj_test-key' },
+        })
+
+        expect(response.statusCode).toBe(400)
     })
 
     it('returns 400 when the event has no public data URL', async () => {

functions/src/api/routes/events/getVotes.ts

@@ -54,10 +54,21 @@ export const getEventVotesRoute: FastifyPluginAsync = async (server) => {
                 }
                 project = request.project as unknown as ExportableProject
             } else if (request.organization) {
-                const resolved = await ProjectDao.getProjectFromId(
-                    server.firebase,
-                    projectId
-                )
+                // Resolve the event and confirm it belongs to this org. A
+                // missing project and a wrong-org project must look identical
+                // (same 404 message) so event ids stay non-enumerable.
+                let resolved
+                try {
+                    resolved = await ProjectDao.getProjectFromId(
+                        server.firebase,
+                        projectId
+                    )
+                } catch (error) {
+                    if (error instanceof NotFoundError) {
+                        throw new NotFoundError('Event not found')
+                    }
+                    throw error
+                }
                 if (resolved.organizationId !== request.organization.id) {
                     throw new NotFoundError('Event not found')
                 }

functions/src/api/services/exportEventVotes.ts

@@ -48,7 +48,7 @@ const voteValueToString = (voteResult: unknown): string => {
         voteResult as Record<string, { text?: string; plus?: number }>
     )
         .filter((v) => !!v?.text)
-        .map((v) => `${v.text} (${v.plus})`)
+        .map((v) => `${v.text} (${v.plus ?? 0})`)
         .join(', ')
         .replaceAll('\n', ' ')
 }
@@ -73,51 +73,81 @@ export const buildEventVotesExport = async (
     }
 
     const db = getFirestore(firebaseApp)
-    const data = (await fetch(jsonUrl).then((res) =>
-        res.json()
-    )) as OpenFeedbackData
+    const data = await fetchEventData(jsonUrl)
 
     const sessions = Object.keys(data.sessions || {}).map((id) => ({
         id,
         ...data.sessions[id],
     }))
-    const voteItems = project.voteItems || []
+    // Index vote items by id once instead of re-scanning per session.
+    const voteItemsById = new Map(
+        (project.voteItems || []).map((item) => [item.id, item])
+    )
 
-    const rows: EventVoteRow[] = []
-    for (const session of sessions) {
-        const sessionVotes = (await db
-            .collection('projects')
-            .doc(project.id)
-            .collection('sessionVotes')
-            .doc(session.id)
-            .get()
-            .then((doc) => doc.data())) as Record<string, unknown> | undefined
+    // Fetch every session's votes concurrently; sequential awaits would be slow
+    // and risk route timeouts on events with many sessions. Promise.all keeps
+    // the original session order.
+    return Promise.all(
+        sessions.map(async (session) => {
+            const sessionVotes = (await db
+                .collection('projects')
+                .doc(project.id)
+                .collection('sessionVotes')
+                .doc(session.id)
+                .get()
+                .then((doc) => doc.data())) as
+                | Record<string, unknown>
+                | undefined
 
-        const speakersName = (session.speakers || [])
-            .map((speakerId) => data.speakers?.[speakerId]?.name)
-            .filter((name): name is string => !!name)
-            .join(', ')
+            const speakersName = (session.speakers || [])
+                .map((speakerId) => data.speakers?.[speakerId]?.name)
+                .filter((name): name is string => !!name)
+                .join(', ')
 
-        const voteColumns = Object.keys(sessionVotes || {}).reduce<
-            Record<string, string>
-        >((acc, key) => {
-            const voteItem = voteItems.find((item) => item.id === key)
-            if (voteItem?.name) {
-                acc[voteItem.name] = voteValueToString(sessionVotes![key])
-            }
-            return acc
-        }, {})
+            const voteColumns = Object.keys(sessionVotes || {}).reduce<
+                Record<string, string>
+            >((acc, key) => {
+                const voteItem = voteItemsById.get(key)
+                if (voteItem?.name) {
+                    acc[voteItem.name] = voteValueToString(sessionVotes![key])
+                }
+                return acc
+            }, {})
 
-        rows.push({
-            sessionId: session.id,
-            title: session.title,
-            speakers: (session.speakers || []).join(', '),
-            speakersName,
-            tags: (session.tags || []).join(', '),
-            trackTitle: session.trackTitle,
-            ...voteColumns,
+            return {
+                sessionId: session.id,
+                title: session.title,
+                speakers: (session.speakers || []).join(', '),
+                speakersName,
+                tags: (session.tags || []).join(', '),
+                trackTitle: session.trackTitle,
+                ...voteColumns,
+            }
         })
-    }
+    )
+}
 
-    return rows
+// Load the event's public sessions/speakers JSON, turning a bad/expired URL or
+// unparseable body into a 400 instead of an opaque 500.
+const fetchEventData = async (jsonUrl: string): Promise<OpenFeedbackData> => {
+    let response: Response
+    try {
+        response = await fetch(jsonUrl)
+    } catch {
+        throw new BadRequestError(
+            'Could not reach the event public data URL (config.jsonUrl).'
+        )
+    }
+    if (!response.ok) {
+        throw new BadRequestError(
+            `The event public data URL returned ${response.status}.`
+        )
+    }
+    try {
+        return (await response.json()) as OpenFeedbackData
+    } catch {
+        throw new BadRequestError(
+            'The event public data URL did not return valid JSON.'
+        )
+    }
 }