Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Notification Email Data #1693

Merged
merged 11 commits into from
Feb 5, 2025
Merged

Conversation

Mephistic
Copy link
Collaborator

Summary

This PR adds the email digest data compilation needed to trigger the social layer notification email digests - I had to tweak the nextDigestAt logic a bit because the email template design has shifted from when the underlying mail system was originally built.

Main goal here is to unblock the UI changes to the email template by making the EmailDigest data type available

This addresses #1570

Screenshots

N/A

Known issues

A few issues I'm still confirming/ironing out while this builds:

  • Data inconsistencies (some users in firestore have email, but we keep email/emailVerified on the user object in firebase auth, but we have some emails in profiles - want to confirm what is source of truth here. We'll obviously only want to send emails to users with verified emails). Will add a quick backfill script to this PR once that's confirmed
  • This flow should work fine for the volume we're hoping for with the user rollout - I flagged a few points where I want to come back in post-V2 to improve the throughput / reliability (especially re: batching + template rendering).
  • Tests tests tests
  • The emergency kill switch for this change is to pause either the deliverNotifications firebase function, or the firestore-send-email queue extension function. The former is less likely to bite us with quirks if we need to pause, but the latter is closest to where the actual email sending happens if the need arises to shut everything down.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ser record. We send digests at a user level, so it no longer makes sense to track digests individually by topic
…have pending digests, rather than by scanning the digest topics directly
Copy link

vercel bot commented Jan 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
maple-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:10am

Copy link
Collaborator

@kiminkim724 kiminkim724 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! just some logic check

…gible by grouping the counts by bill
@Mephistic Mephistic marked this pull request as ready for review February 5, 2025 01:22
@Mephistic Mephistic merged commit f5a188f into codeforboston:main Feb 5, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants