Fix null createdAt crash in Feedback vote results (Gen2 trigger regression)#1680
Merged
Conversation
…migration The Gen1->Gen2 Firestore trigger migration (#1664) made the vote aggregation fire before serverTimestamp() resolves, writing null createdAt/updatedAt into aggregated sessionVotes docs. The Feedback app then crashed on `value2.createdAt.toDate()` with "TypeError: can't access property toDate, h.createdAt is null". - aggregateVotes.ts: normalizeVoteTimestamps() falls back to the snapshot's server-assigned createTime/updateTime when the field is null, so new aggregated docs never store a null timestamp. - getVoteResultSelectorSelector.js: guard the .toDate() calls and skip entries with no usable date, so already-corrupted prod docs no longer crash the whole results view. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a production crash in the Feedback vote-results view caused by Gen2 Firestore trigger behavior exposing intermediate serverTimestamp() writes where createdAt/updatedAt can be null.
Changes:
- Server-side: normalize vote timestamps in Gen2 triggers by falling back to Firestore snapshot
createTime/updateTimewhencreatedAt/updatedAtarenull. - Client-side: guard
.toDate()calls with optional chaining and skip corrupted entries so the results view doesn’t crash on already-bad data.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| functions/src/triggers/aggregateVotes.ts | Adds timestamp normalization to avoid writing null timestamps into aggregated sessionVotes docs under Gen2 triggers. |
| src/feedback/talk/core/getVoteResultSelectorSelector.js | Prevents UI crash by safely converting timestamps and ignoring entries without usable dates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Export the helper and cover the null/undefined createdAt/updatedAt fallback to createTime/updateTime, plus the untouched-when-set case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Visit the preview URL for this PR (updated for commit 70938ba): https://open-feedback-42--pr1680-fix-vote-aggregation-jjeqhh69.web.app (expires Mon, 22 Jun 2026 17:29:51 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 08b588459ed3335bea4061fbe93b8b77635ad43a |
open-feedback
 |
||||||||||||||||||||||||||||
| Project |
open-feedback
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 02m 42s |
| Commit |
|
| Committer | Hugo Gresse |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
1
|
 |
0
|
 |
0
|
 |
14
|
| View all changes introduced in this branch ↗︎ | |
editVoteRangeOpenTime selected the day cell via button[tabindex=-1], but MUI assigns today's cell tabindex=0. The test picks day 15, so it failed on the 15th of every month. Match the day cell by exact text and enabled state instead, removing the date dependency. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
What
Fixes
TypeError: can't access property "toDate", h.createdAt is nullcrashing the Feedback app's vote-results view.Why
Commit
e314b38("API (wip)", #1664) migrated the vote-aggregation Firestore triggers from Gen1 (functions.firestore.document().onCreate) to Gen2 (onDocumentCreated/onDocumentUpdated).Gen2 (Eventarc) surfaces the intermediate write a
serverTimestamp()makes — the create event fires before the sentinel resolves, socreatedAt/updatedAtarrive asnull. Gen1 hid that. The aggregation copied the null into thesessionVotesdoc, and the Feedback app then crashed on the unguardedvalue2.createdAt.toDate()ingetVoteResultSelectorSelector.js.Frontend feedback code was unchanged for 6 months — confirms the regression is server-side.
Changes
functions/src/triggers/aggregateVotes.ts—normalizeVoteTimestamps()falls back to the snapshot's server-assignedcreateTime/updateTimewhen the field is null, so new aggregated docs never store a null timestamp.src/feedback/talk/core/getVoteResultSelectorSelector.js— optional-chained.toDate()+ skip entries with no usable date, so docs already corrupted in prod no longer crash the whole results view.Reviewer notes
sessionVotesdocs stay corrupted in Firestore; the frontend guard hides the crash. A re-aggregation/migration script would be needed to repair existing data.normalizeVoteTimestampsis not exported). Can add one if wanted.tsc --noEmitclean,aggregateVotes.spec.ts10/10 pass.🤖 Generated with Claude Code