Better error handling for notification destinations

common/user-notification-preferences.ts

@@ -178,31 +178,44 @@ export const getNotificationDestinationsForUser = (
   reason: notification_reason_types | notification_preference
 ) => {
   const notificationSettings = privateUser.notificationPreferences
-  let destinations
-  let subscriptionType: notification_preference | undefined
-  if (Object.keys(notificationSettings).includes(reason)) {
-    subscriptionType = reason as notification_preference
-    destinations = notificationSettings[subscriptionType]
-  } else {
-    const key = reason as notification_reason_types
-    subscriptionType = notificationReasonToSubscriptionType[key]
-    destinations = subscriptionType
-      ? notificationSettings[subscriptionType]
-      : []
-  }
-  const optOutOfAllSettings = notificationSettings['opt_out_all']
-  // Your market closure notifications are high priority, opt-out doesn't affect their delivery
-  const optedOutOfEmail =
-    optOutOfAllSettings.includes('email') &&
-    subscriptionType !== 'your_contract_closed'
-  const optedOutOfBrowser =
-    optOutOfAllSettings.includes('browser') &&
-    subscriptionType !== 'your_contract_closed'
   const unsubscribeEndpoint = getFunctionUrl('unsubscribe')
-  return {
+  try {
-    sendToEmail: destinations.includes('email') && !optedOutOfEmail,
+    let destinations
-    sendToBrowser: destinations.includes('browser') && !optedOutOfBrowser,
+    let subscriptionType: notification_preference | undefined
-    unsubscribeUrl: `${unsubscribeEndpoint}?id=${privateUser.id}&type=${subscriptionType}`,
+    if (Object.keys(notificationSettings).includes(reason)) {
-    urlToManageThisNotification: `${DOMAIN}/notifications?tab=settings&section=${subscriptionType}`,
+      subscriptionType = reason as notification_preference
+      destinations = notificationSettings[subscriptionType]
+    } else {
+      const key = reason as notification_reason_types
+      subscriptionType = notificationReasonToSubscriptionType[key]
+      destinations = subscriptionType
+        ? notificationSettings[subscriptionType]
+        : []
+    }
+    const optOutOfAllSettings = notificationSettings['opt_out_all']
+    // Your market closure notifications are high priority, opt-out doesn't affect their delivery
+    const optedOutOfEmail =
+      optOutOfAllSettings.includes('email') &&
+      subscriptionType !== 'your_contract_closed'
+    const optedOutOfBrowser =
+      optOutOfAllSettings.includes('browser') &&
+      subscriptionType !== 'your_contract_closed'
+    return {
+      sendToEmail: destinations.includes('email') && !optedOutOfEmail,
+      sendToBrowser: destinations.includes('browser') && !optedOutOfBrowser,
+      unsubscribeUrl: `${unsubscribeEndpoint}?id=${privateUser.id}&type=${subscriptionType}`,
+      urlToManageThisNotification: `${DOMAIN}/notifications?tab=settings&section=${subscriptionType}`,
+    }
+  } catch (e) {
+    // Fail safely
+    console.log(
+      `couldn't get notification destinations for type ${reason} for user ${privateUser.id}`
+    )
+    return {
+      sendToEmail: false,
+      sendToBrowser: false,
+      unsubscribeUrl: '',
+      urlToManageThisNotification: '',
+    }
   }
 }

functions/src/emails.ts

@@ -12,7 +12,7 @@ import { getValueFromBucket } from '../../common/calculate-dpm'
 import { formatNumericProbability } from '../../common/pseudo-numeric'
 
 import { sendTemplateEmail, sendTextEmail } from './send-email'
-import { contractUrl, getUser } from './utils'
+import { contractUrl, getUser, log } from './utils'
 import { buildCardUrl, getOpenGraphProps } from '../../common/contract-details'
 import { notification_reason_types } from '../../common/notification'
 import { Dictionary } from 'lodash'
@@ -212,20 +212,16 @@ export const sendOneWeekBonusEmail = async (
   user: User,
   privateUser: PrivateUser
 ) => {
-  if (
+  if (!privateUser || !privateUser.email) return
-    !privateUser ||
-    !privateUser.email ||
-    !privateUser.notificationPreferences.onboarding_flow.includes('email')
-  )
-    return
 
   const { name } = user
   const firstName = name.split(' ')[0]
 
-  const { unsubscribeUrl } = getNotificationDestinationsForUser(
+  const { unsubscribeUrl, sendToEmail } = getNotificationDestinationsForUser(
     privateUser,
     'onboarding_flow'
   )
+  if (!sendToEmail) return
 
   return await sendTemplateEmail(
     privateUser.email,
@@ -247,19 +243,15 @@ export const sendCreatorGuideEmail = async (
   privateUser: PrivateUser,
   sendTime: string
 ) => {
-  if (
+  if (!privateUser || !privateUser.email) return
-    !privateUser ||
-    !privateUser.email ||
-    !privateUser.notificationPreferences.onboarding_flow.includes('email')
-  )
-    return
 
   const { name } = user
   const firstName = name.split(' ')[0]
-  const { unsubscribeUrl } = getNotificationDestinationsForUser(
+  const { unsubscribeUrl, sendToEmail } = getNotificationDestinationsForUser(
     privateUser,
     'onboarding_flow'
   )
+  if (!sendToEmail) return
   return await sendTemplateEmail(
     privateUser.email,
     'Create your own prediction market',
@@ -279,22 +271,16 @@ export const sendThankYouEmail = async (
   user: User,
   privateUser: PrivateUser
 ) => {
-  if (
+  if (!privateUser || !privateUser.email) return
-    !privateUser ||
-    !privateUser.email ||
-    !privateUser.notificationPreferences.thank_you_for_purchases.includes(
-      'email'
-    )
-  )
-    return
 
   const { name } = user
   const firstName = name.split(' ')[0]
-  const { unsubscribeUrl } = getNotificationDestinationsForUser(
+  const { unsubscribeUrl, sendToEmail } = getNotificationDestinationsForUser(
     privateUser,
     'thank_you_for_purchases'
   )
 
+  if (!sendToEmail) return
   return await sendTemplateEmail(
     privateUser.email,
     'Thanks for your Manifold purchase',
@@ -466,17 +452,13 @@ export const sendInterestingMarketsEmail = async (
   contractsToSend: Contract[],
   deliveryTime?: string
 ) => {
-  if (
+  if (!privateUser || !privateUser.email) return
-    !privateUser ||
-    !privateUser.email ||
-    !privateUser.notificationPreferences.trending_markets.includes('email')
-  )
-    return
 
-  const { unsubscribeUrl } = getNotificationDestinationsForUser(
+  const { unsubscribeUrl, sendToEmail } = getNotificationDestinationsForUser(
     privateUser,
     'trending_markets'
   )
+  if (!sendToEmail) return
 
   const { name } = user
   const firstName = name.split(' ')[0]
@@ -620,18 +602,15 @@ export const sendWeeklyPortfolioUpdateEmail = async (
   investments: PerContractInvestmentsData[],
   overallPerformance: OverallPerformanceData
 ) => {
-  if (
+  if (!privateUser || !privateUser.email) return
-    !privateUser ||
-    !privateUser.email ||
-    !privateUser.notificationPreferences.profit_loss_updates.includes('email')
-  )
-    return
 
-  const { unsubscribeUrl } = getNotificationDestinationsForUser(
+  const { unsubscribeUrl, sendToEmail } = getNotificationDestinationsForUser(
     privateUser,
     'profit_loss_updates'
   )
 
+  if (!sendToEmail) return
+
   const { name } = user
   const firstName = name.split(' ')[0]
   const templateData: Record<string, string> = {
@@ -656,4 +635,5 @@ export const sendWeeklyPortfolioUpdateEmail = async (
       : 'portfolio-update',
     templateData
   )
+  log('Sent portfolio update email to', privateUser.email)
 }

functions/src/weekly-portfolio-emails.ts

@@ -112,13 +112,12 @@ export async function sendPortfolioUpdateEmailsToAllUsers() {
       )
     )
   )
-  log('Found', contractsUsersBetOn.length, 'contracts')
-  let count = 0
   await Promise.all(
     privateUsersToSendEmailsTo.map(async (privateUser) => {
       const user = await getUser(privateUser.id)
       // Don't send to a user unless they're over 5 days old
-      if (!user || user.createdTime > Date.now() - 5 * DAY_MS) return
+      if (!user || user.createdTime > Date.now() - 5 * DAY_MS)
+        return await setEmailFlagAsSent(privateUser.id)
       const userBets = usersBets[privateUser.id] as Bet[]
       const contractsUserBetOn = contractsUsersBetOn.filter((contract) =>
         userBets.some((bet) => bet.contractId === contract.id)
@@ -219,13 +218,6 @@ export async function sendPortfolioUpdateEmailsToAllUsers() {
         (differences) => Math.abs(differences.profit)
       ).reverse()
 
-      log(
-        'Found',
-        investmentValueDifferences.length,
-        'investment differences for user',
-        privateUser.id
-      )
-
       const [winningInvestments, losingInvestments] = partition(
         investmentValueDifferences.filter(
           (diff) => diff.pastValue > 0.01 && Math.abs(diff.profit) > 1
@@ -245,29 +237,28 @@ export async function sendPortfolioUpdateEmailsToAllUsers() {
         usersToContractsCreated[privateUser.id].length === 0
       ) {
         log(
-          'No bets in last week, no market movers, no markets created. Not sending an email.'
+          `No bets in last week, no market movers, no markets created. Not sending an email to ${privateUser.email} .`
         )
-        await firestore.collection('private-users').doc(privateUser.id).update({
+        return await setEmailFlagAsSent(privateUser.id)
-          weeklyPortfolioUpdateEmailSent: true,
-        })
-        return
       }
+      // Set the flag beforehand just to be safe
+      await setEmailFlagAsSent(privateUser.id)
       await sendWeeklyPortfolioUpdateEmail(
         user,
         privateUser,
         topInvestments.concat(worstInvestments) as PerContractInvestmentsData[],
         performanceData
       )
-      await firestore.collection('private-users').doc(privateUser.id).update({
-        weeklyPortfolioUpdateEmailSent: true,
-      })
-      log('Sent weekly portfolio update email to', privateUser.email)
-      count++
-      log('sent out emails to users:', count)
     })
   )
 }
 
+async function setEmailFlagAsSent(privateUserId: string) {
+  await firestore.collection('private-users').doc(privateUserId).update({
+    weeklyPortfolioUpdateEmailSent: true,
+  })
+}
+
 export type PerContractInvestmentsData = {
   questionTitle: string
   questionUrl: string