Merge pull request #1718 from Mephistic/move-next-digest-at

Make Profiles Source of Truth for Notification Settings

Author: Mephistic

.prettierignore

@@ -14,3 +14,4 @@ dist
 coverage
 storybook-static
 llm
+playwright-report

components/db/profile/types.ts

@@ -35,6 +35,7 @@ export type Profile = {
   senator?: ProfileMember
   public?: boolean
   notificationFrequency?: Frequency
+  nextDigestAt?: FirebaseFirestore.Timestamp
   about?: string
   social?: SocialLinks
   profileImage?: string

firestore.rules

@@ -29,6 +29,11 @@ service cloud.firestore {
       function doesNotChangeRole() { 
         return !request.resource.data.diff(resource.data).affectedKeys().hasAny(['role'])
       }
+      function doesNotChangeNextDigestAt() {
+        // Only admins/automatic processes should be able to change the
+        // email digest notification times
+        return !request.resource.data.diff(resource.data).affectedKeys().hasAny(['nextDigestAt'])
+      }
       // either the change doesn't include the public field, 
       // or the user is a base user (i.e. not an org)
       function validPublicChange() {
@@ -47,7 +52,7 @@ service cloud.firestore {
 
       // Allow users to make updates except to delete their profile or set the role field.
       // Only admins can delete a user profile or set the user role field. 
-      allow update: if validUser() && doesNotChangeRole() && validPublicChange()
+      allow update: if validUser() && doesNotChangeRole() && validPublicChange() && doesNotChangeNextDigestAt()
     }
     // Allow querying publications individually or with a collection group.
     match /{path=**}/publishedTestimony/{id} {

functions/src/notifications/deliverNotifications.ts

@@ -5,7 +5,7 @@ import * as fs from "fs"
 import { Timestamp } from "../firebase"
 import { getNextDigestAt, getNotificationStartDate } from "./helpers"
 import { startOfDay } from "date-fns"
-import { TestimonySubmissionNotificationFields, User } from "./types"
+import { TestimonySubmissionNotificationFields, Profile } from "./types"
 import {
   BillDigest,
   NotificationEmailDigest,
@@ -44,38 +44,42 @@ const deliverEmailNotifications = async () => {
   prepareHandlebars()
   console.log("Handlebars helpers and partials prepared")
 
-  const usersSnapshot = await db
-    .collection("users")
+  const profilesSnapshot = await db
+    .collection("profiles")
     .where("nextDigestAt", "<=", now)
     .get()
 
-  const emailPromises = usersSnapshot.docs.map(async userDoc => {
-    const user = userDoc.data() as User
-    if (!user || !user.notificationFrequency) {
-      console.log(`User ${userDoc.id} has no notificationFrequency - skipping`)
+  const emailPromises = profilesSnapshot.docs.map(async profileDoc => {
+    const profile = profileDoc.data() as Profile
+    if (!profile || !profile.notificationFrequency) {
+      console.log(
+        `User ${profileDoc.id} has no notificationFrequency - skipping`
+      )
       return
     }
 
-    const verifiedEmail = await getVerifiedUserEmail(userDoc.id)
+    const verifiedEmail = await getVerifiedUserEmail(profileDoc.id)
     if (!verifiedEmail) {
       console.log(
-        `Skipping user ${userDoc.id} because they have no verified email address`
+        `Skipping user ${profileDoc.id} because they have no verified email address`
       )
       return
     }
 
     const digestData = await buildDigestData(
-      userDoc.id,
+      profileDoc.id,
       now,
-      user.notificationFrequency
+      profile.notificationFrequency
     )
 
     // If there are no new notifications, don't send an email
     if (
       digestData.numBillsWithNewTestimony === 0 &&
       digestData.numUsersWithNewTestimony === 0
     ) {
-      console.log(`No new notifications for ${userDoc.id} - not sending email`)
+      console.log(
+        `No new notifications for ${profileDoc.id} - not sending email`
+      )
     } else {
       const htmlString = renderToHtmlString(digestData)
 
@@ -90,13 +94,13 @@ const deliverEmailNotifications = async () => {
         createdAt: Timestamp.now()
       })
 
-      console.log(`Saved email message to user ${userDoc.id}`)
+      console.log(`Saved email message to user ${profileDoc.id}`)
     }
 
-    const nextDigestAt = getNextDigestAt(user.notificationFrequency)
-    await userDoc.ref.update({ nextDigestAt })
+    const nextDigestAt = getNextDigestAt(profile.notificationFrequency)
+    await profileDoc.ref.update({ nextDigestAt })
 
-    console.log(`Updated nextDigestAt for ${userDoc.id} to ${nextDigestAt}`)
+    console.log(`Updated nextDigestAt for ${profileDoc.id} to ${nextDigestAt}`)
   })
 
   // Wait for all email documents to be created

functions/src/notifications/types.ts

@@ -2,13 +2,6 @@ import { Frequency } from "../auth/types"
 import { BillHistory } from "../bills/types"
 import { Timestamp } from "../firebase"
 
-// This should probably live somewhere else once other code starts caring about this
-export interface User {
-  email?: string
-  notificationFrequency?: Frequency
-  nextDigestAt?: Timestamp
-}
-
 export interface Notification {
   type: string
   updateTime: Timestamp
@@ -70,3 +63,8 @@ export interface BillHistoryUpdateNotificationFields {
   }
   createdAt: FirebaseFirestore.Timestamp
 }
+
+export interface Profile {
+  notificationFrequency?: Frequency
+  nextDigestAt?: FirebaseFirestore.Timestamp
+}

functions/src/notifications/updateUserNotificationFrequency.ts

@@ -35,14 +35,13 @@ export const updateUserNotificationFrequency = functions.firestore
         return null
       }
 
-      // Update user document in the 'users' collection
+      // Update the profile document to include the computed `nextDigestAt`
       await admin
         .firestore()
-        .collection("users")
+        .collection("profiles")
         .doc(userId)
         .set(
           {
-            notificationFrequency: notificationFrequency,
             nextDigestAt: getNextDigestAt(notificationFrequency)
           },
           { merge: true }

scripts/firebase-admin/backfillNextDigestAt.ts

@@ -0,0 +1,44 @@
+/*
+    This script is intended to backfill the `nextDigestAt` field
+    into the `profiles` collection. This field is used to determine
+    when the next notifications digest email should be sent to a user
+    based on their set `notificationFrequency`.
+
+    This script can be safely removed after the `nextDigestAt` field
+    has been added to the profiles collection.
+*/
+
+import { getNextDigestAt } from "../../functions/src/notifications/helpers"
+import { Profile } from "../../components/db/profile/types"
+import { Script } from "./types"
+import { Boolean, Record } from "runtypes"
+
+const Args = Record({
+  dryRun: Boolean
+})
+
+export const script: Script = async ({ db, auth, args }) => {
+  const profilesSnapshot = await db.collection("profiles").get()
+
+  const updatePromises = profilesSnapshot.docs.map(async profileDoc => {
+    const profile = profileDoc.data() as Profile
+
+    if (profile.notificationFrequency) {
+      const nextDigestAt = getNextDigestAt(profile.notificationFrequency)
+
+      if (!args.dryRun) {
+        await profileDoc.ref.update({ nextDigestAt })
+      }
+      console.log(
+        `Updated nextDigestAt for ${profileDoc.id} to ${nextDigestAt}`
+      )
+    } else {
+      console.log(
+        `Profile ${profileDoc.id} does not have notificationFrequency - skipping`
+      )
+    }
+  })
+
+  await Promise.all(updatePromises)
+  console.log("Backfill complete")
+}

scripts/firebase-admin/backfillNotificationFrequency.ts

@@ -1,3 +1,6 @@
+// DEPRECATED - This will no longer be useful after we've moved notificationFrequency to profiles
+// This should no longer be run and will be removed in a future update
+
 import { Script } from "./types"
 import { listAllUsers } from "./list-all-users"
 
@@ -9,7 +12,7 @@ export const script: Script = async ({ db, auth }) => {
 
   for (const user of allUsers) {
     // Get user document from Firestore
-    const userDoc = db.collection("users").doc(user.uid)
+    const userDoc = db.collection("profiles").doc(user.uid)
     const doc = await userDoc.get()
 
     // If the user document exists in Firestore

scripts/firebase-admin/backfillUserEmails.ts

@@ -1,3 +1,8 @@
+// DEPRECATED - This will no longer be useful going forward
+//              We plan to rely on the `profiles` collection for casual email use
+//              and the firebase-auth API where we need verified email addresses
+// This should no longer be run and will be removed in a future update
+
 import { UserRecord } from "firebase-admin/auth"
 import { Auth } from "functions/src/types"
 import { Script } from "./types"

yarn.lock

@@ -6369,15 +6369,10 @@ camelize@^1.0.0:
   resolved "https://registry.yarnpkg.com/camelize/-/camelize-1.0.1.tgz#89b7e16884056331a35d6b5ad064332c91daa6c3"
   integrity sha512-dU+Tx2fsypxTgtLoE36npi3UqcjSSMNYfkqgmoEhtZrraP5VWq0K7FkWVTYa8eMPtnU/G2txVsfdCJTn9uzpuQ==
 
-caniuse-lite@^1.0.30001406:
-  version "1.0.30001568"
-  resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001568.tgz#53fa9297273c9a977a560663f48cbea1767518b7"
-  integrity sha512-vSUkH84HontZJ88MiNrOau1EBrCqEQYgkC5gIySiDlpsm8sGVrhU7Kx4V6h0tnqaHzIHZv08HlJIwPbL4XL9+A==
-
-caniuse-lite@^1.0.30001565:
-  version "1.0.30001572"
-  resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001572.tgz#1ccf7dc92d2ee2f92ed3a54e11b7b4a3041acfa0"
-  integrity sha512-1Pbh5FLmn5y4+QhNyJE9j3/7dK44dGB83/ZMjv/qJk86TvDbjk0LosiZo0i0WB0Vx607qMX9jYrn1VLHCkN4rw==
+caniuse-lite@^1.0.30001406, caniuse-lite@^1.0.30001565:
+  version "1.0.30001700"
+  resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001700.tgz"
+  integrity sha512-2S6XIXwaE7K7erT8dY+kLQcpa5ms63XlRkMkReXjle+kf6c5g38vyMl+Z5y8dSxOFDhcFe+nxnn261PLxBSQsQ==
 
 cardinal@^2.1.1:
   version "2.1.1"