Implement follow/notifications for ballot questions#27
Implement follow/notifications for ballot questions#27tripledoublev merged 2 commits intofeat/ballot-questionsfrom
Conversation
fastfadingviolets
force-pushed
the
feat/bq-follow
branch
2 times, most recently
from
6ffea98 to
5c9425c
Compare
| const bqId: string | null = newData?.ballotQuestionId ?? null | ||
| let bqCourt: number | null = null | ||
| if (bqId) { | ||
| const bqSnap = await db.doc(`/ballotQuestions/${bqId}`).get() | ||
| bqCourt = bqSnap.exists | ||
| ? (bqSnap.data()?.court as number) ?? null | ||
| : null | ||
| } |
There was a problem hiding this comment.
Non-blocking (future work): As this adds an extra ballotQuestions read for every testimony tied to a ballot question in order to recover court and since publish-time validation already loads the ballot question, it may be worth denormalizing ballotQuestionCourt on the published testimony (or event payload) in follow-up work to keep this trigger simpler.
There was a problem hiding this comment.
yeah let me actually knock this out as part of this. we can just denormalize into the event payload.
There was a problem hiding this comment.
it's already denormalized lol! i just had to grab it from the object. good catch.
| } | ||
| } | ||
|
|
||
| if (notification.isBallotQuestionMatch && notification.ballotQuestionId) { |
There was a problem hiding this comment.
Question for you: if a user follows both the bill and the ballot question, would the same testimony be in both digest sections? Or am I reading this wrong?
There was a problem hiding this comment.
No, bc they're different notification objects ultimately. If a user follows both they'd see notifications for the bill when the bill changes, for the ballot question when the ballot question changes. The source of truth for this is the userNotificationFeed, and that gets written to once when the testimony goes in, and points to either the ballot question if it's ballot question testimony, the bill if it's bill testimony
There was a problem hiding this comment.
Ok, thanks for the clarification
| } else { | ||
| ballotQuestionsById[bqId] = { | ||
| ballotQuestionId: bqId, | ||
| description: notification.header, |
There was a problem hiding this comment.
description: notification.header -> Does this mean this section is displaying the related bill title?
There was a problem hiding this comment.
This should come from the ballot question's description, which actually, I think is null right now.
fastfadingviolets
force-pushed
the
feat/bq-follow
branch
from
5c9425c to
b56d1be
Compare
prevents extra fetch of a ballot question
fastfadingviolets
force-pushed
the
feat/bq-follow
branch
from
cd9cdd2 to
0a639d2
Compare
2 participants
Summary
Add a short summary of the changes, and a reference to the original issue using
#and the issue number, like #1Checklist
firestore.indexes.json(Please do not only create indexes through the Firebase Web UI, even though the error messages may reccommend it - indexes created this way may be obliterated by subsequent deploys)Screenshots
Add some screenshots highlighting your changes.
Known issues
If you've run against limitations or caveats, include them here. Include follow-up issues as well.
Steps to test/reproduce
For each feature or bug fix, create a step by step list for how a reviewer can test it out. E.g.: