Speed up notification loading by prepopulating relevant info (#453)

* Populate notification with relevant info * eslint * Remove duplicated code * Unused ? * Add new q notification, other small fixes
2022-06-08 08:43:24 -06:00 · 2022-06-08 08:43:24 -06:00 · 936cabe353
commit 936cabe353
parent 7e37fc776c
13 changed files with 198 additions and 92 deletions
--- a/common/notification.ts
+++ b/common/notification.ts
@ -14,6 +14,10 @@ export type Notification = {
  sourceUserName?: string
  sourceUserUsername?: string
  sourceUserAvatarUrl?: string
+  sourceText?: string
+  sourceContractTitle?: string
+  sourceContractCreatorUsername?: string
+  sourceContractSlug?: string
 }
 export type notification_source_types =
  | 'contract'
@ -42,3 +46,4 @@ export type notification_reason_types =
  | 'reply_to_users_answer'
  | 'reply_to_users_comment'
  | 'on_new_follow'
+  | 'you_follow_user'
--- a/functions/src/create-notification.ts
+++ b/functions/src/create-notification.ts
@ -26,10 +26,10 @@ export const createNotification = async (
  sourceUpdateType: notification_source_update_types,
  sourceUser: User,
  idempotencyKey: string,
+  sourceText: string,
  sourceContract?: Contract,
  relatedSourceType?: notification_source_types,
-  relatedUserId?: string,
-  sourceText?: string
+  relatedUserId?: string
 ) => {
  const shouldGetNotification = (
    userId: string,
@ -62,12 +62,37 @@ export const createNotification = async (
          sourceUserName: sourceUser.name,
          sourceUserUsername: sourceUser.username,
          sourceUserAvatarUrl: sourceUser.avatarUrl,
+          sourceText,
+          sourceContractTitle: sourceContract?.question,
+          sourceContractCreatorUsername: sourceContract?.creatorUsername,
+          sourceContractSlug: sourceContract?.slug,
        }
        await notificationRef.set(removeUndefinedProps(notification))
      })
    )
  }

+  const notifyUsersFollowers = async (
+    userToReasonTexts: user_to_reason_texts
+  ) => {
+    const followers = await firestore
+      .collectionGroup('follows')
+      .where('userId', '==', sourceUser.id)
+      .get()
+
+    followers.docs.forEach((doc) => {
+      const followerUserId = doc.ref.parent.parent?.id
+      if (
+        followerUserId &&
+        shouldGetNotification(followerUserId, userToReasonTexts)
+      ) {
+        userToReasonTexts[followerUserId] = {
+          reason: 'you_follow_user',
+        }
+      }
+    })
+  }
+
  const notifyRepliedUsers = async (
    userToReasonTexts: user_to_reason_texts
  ) => {
@ -200,7 +225,8 @@ export const createNotification = async (
      sourceContract &&
      (sourceType === 'comment' ||
        sourceType === 'answer' ||
-        sourceType === 'contract')
+        (sourceType === 'contract' &&
+          (sourceUpdateType === 'updated' || sourceUpdateType === 'resolved')))
    ) {
      if (sourceType === 'comment') {
        await notifyRepliedUsers(userToReasonTexts)
@ -212,6 +238,8 @@ export const createNotification = async (
      await notifyOtherCommentersOnContract(userToReasonTexts, sourceContract)
    } else if (sourceType === 'follow' && relatedUserId) {
      await notifyFollowedUser(userToReasonTexts, relatedUserId)
+    } else if (sourceType === 'contract' && sourceUpdateType === 'created') {
+      await notifyUsersFollowers(userToReasonTexts)
    }
    return userToReasonTexts
  }
--- a/functions/src/index.ts
+++ b/functions/src/index.ts
@ -26,6 +26,7 @@ export * from './market-close-emails'
 export * from './add-liquidity'
 export * from './on-create-answer'
 export * from './on-update-contract'
+export * from './on-create-contract'
 export * from './on-follow-user'

 // v2
--- a/functions/src/on-create-answer.ts
+++ b/functions/src/on-create-answer.ts
@ -27,6 +27,7 @@ export const onCreateAnswer = functions.firestore
      'created',
      answerCreator,
      eventId,
+      answer.text,
      contract
    )
  })
--- a/functions/src/on-create-comment.ts
+++ b/functions/src/on-create-comment.ts
@ -78,10 +78,10 @@ export const onCreateComment = functions
      'created',
      commentCreator,
      eventId,
+      comment.text,
      contract,
      relatedSourceType,
-      relatedUser,
-      comment.text
+      relatedUser
    )

    const recipientUserIds = uniq([
--- a/functions/src/on-create-contract.ts
+++ b/functions/src/on-create-contract.ts
@ -0,0 +1,24 @@
+import * as functions from 'firebase-functions'
+import { getUser } from './utils'
+import { createNotification } from './create-notification'
+import { Contract } from '../../common/contract'
+
+export const onCreateContract = functions.firestore
+  .document('contracts/{contractId}')
+  .onCreate(async (snapshot, context) => {
+    const contract = snapshot.data() as Contract
+    const { eventId } = context
+
+    const contractCreator = await getUser(contract.creatorId)
+    if (!contractCreator) throw new Error('Could not find contract creator')
+
+    await createNotification(
+      contract.id,
+      'contract',
+      'created',
+      contractCreator,
+      eventId,
+      contract.question,
+      contract
+    )
+  })
--- a/functions/src/on-follow-user.ts
+++ b/functions/src/on-follow-user.ts
@ -21,6 +21,7 @@ export const onFollowUser = functions.firestore
      'created',
      followingUser,
      eventId,
+      '',
      undefined,
      undefined,
      follow.userId
--- a/functions/src/on-update-contract.ts
+++ b/functions/src/on-update-contract.ts
@ -20,6 +20,7 @@ export const onUpdateContract = functions.firestore
        'resolved',
        contractUpdater,
        eventId,
+        contract.question,
        contract
      )
    } else if (
@ -32,6 +33,7 @@ export const onUpdateContract = functions.firestore
        'updated',
        contractUpdater,
        eventId,
+        contract.question,
        contract
      )
    }
--- a/web/components/feed/copy-link-date-time.tsx
+++ b/web/components/feed/copy-link-date-time.tsx
@ -1,7 +1,5 @@
-import { Contract } from 'common/contract'
 import React, { useState } from 'react'
 import { ENV_CONFIG } from 'common/envs/constants'
-import { contractPath } from 'web/lib/firebase/contracts'
 import { copyToClipboard } from 'web/lib/util/copy'
 import { DateTimeTooltip } from 'web/components/datetime-tooltip'
 import Link from 'next/link'
@ -11,20 +9,26 @@ import { LinkIcon } from '@heroicons/react/outline'
 import clsx from 'clsx'

 export function CopyLinkDateTimeComponent(props: {
-  contract: Contract
+  contractCreatorUsername: string
+  contractSlug: string
  createdTime: number
  elementId: string
  className?: string
 }) {
-  const { contract, elementId, createdTime, className } = props
+  const {
+    contractCreatorUsername,
+    contractSlug,
+    elementId,
+    createdTime,
+    className,
+  } = props
  const [showToast, setShowToast] = useState(false)

  function copyLinkToComment(
    event: React.MouseEvent<HTMLAnchorElement, MouseEvent>
  ) {
    event.preventDefault()
-    const elementLocation = `https://${ENV_CONFIG.domain}${contractPath(
-      contract
+    const elementLocation = `https://${ENV_CONFIG.domain}/${contractCreatorUsername}/${contractSlug}
    )}#${elementId}`

    copyToClipboard(elementLocation)
@ -35,7 +39,7 @@ export function CopyLinkDateTimeComponent(props: {
    <div className={clsx('inline', className)}>
      <DateTimeTooltip time={createdTime}>
        <Link
-          href={`/${contract.creatorUsername}/${contract.slug}#${elementId}`}
+          href={`/${contractCreatorUsername}/${contractCreatorUsername}#${elementId}`}
          passHref={true}
        >
          <a
--- a/web/components/feed/feed-answer-comment-group.tsx
+++ b/web/components/feed/feed-answer-comment-group.tsx
@ -127,7 +127,8 @@ export function FeedAnswerCommentGroup(props: {
          <div className="text-sm text-gray-500">
            <UserLink username={username} name={name} /> answered
            <CopyLinkDateTimeComponent
-              contract={contract}
+              contractCreatorUsername={contract.creatorUsername}
+              contractSlug={contract.slug}
              createdTime={answer.createdTime}
              elementId={answerElementId}
            />
--- a/web/components/feed/feed-comments.tsx
+++ b/web/components/feed/feed-comments.tsx
@ -245,7 +245,8 @@ export function FeedComment(props: {
            )}
          </>
          <CopyLinkDateTimeComponent
-            contract={contract}
+            contractCreatorUsername={contract.creatorUsername}
+            contractSlug={contract.slug}
            createdTime={createdTime}
            elementId={comment.id}
          />
--- a/web/hooks/use-notifications.ts
+++ b/web/hooks/use-notifications.ts
@ -17,8 +17,8 @@ export function usePreferredGroupedNotifications(
  options: { unseenOnly: boolean }
 ) {
  const [notificationGroups, setNotificationGroups] = useState<
-    NotificationGroup[]
-  >([])
+    NotificationGroup[] | undefined
+  >(undefined)

  const notifications = usePreferredNotifications(userId, options)
  useEffect(() => {
--- a/web/pages/notifications.tsx
+++ b/web/pages/notifications.tsx
@ -45,16 +45,17 @@ import toast from 'react-hot-toast'
 export default function Notifications() {
  const user = useUser()
  const [unseenNotificationGroups, setUnseenNotificationGroups] = useState<
-    NotificationGroup[]
-  >([])
+    NotificationGroup[] | undefined
+  >(undefined)
  const allNotificationGroups = usePreferredGroupedNotifications(user?.id, {
    unseenOnly: false,
  })

  useEffect(() => {
+    if (!allNotificationGroups) return
    // Don't re-add notifications that are visible right now or have been seen already.
    const currentlyVisibleUnseenNotificationIds = Object.values(
-      unseenNotificationGroups
+      unseenNotificationGroups ?? []
    )
      .map((n) => n.notifications.map((n) => n.id))
      .flat()
@ -92,7 +93,7 @@ export default function Notifications() {
          tabs={[
            {
              title: 'New Notifications',
-              content: (
+              content: unseenNotificationGroups ? (
                <div className={''}>
                  {unseenNotificationGroups.length === 0 &&
                    "You don't have any new notifications."}
@ -113,11 +114,13 @@ export default function Notifications() {
                    )
                  )}
                </div>
+              ) : (
+                <LoadingIndicator />
              ),
            },
            {
              title: 'All Notifications',
-              content: (
+              content: allNotificationGroups ? (
                <div className={''}>
                  {allNotificationGroups.length === 0 &&
                    "You don't have any notifications. Try changing your settings to see more."}
@ -138,6 +141,8 @@ export default function Notifications() {
                    )
                  )}
                </div>
+              ) : (
+                <LoadingIndicator />
              ),
            },
            {
@ -469,10 +474,8 @@ function isNotificationAboutContractResolution(
  contract: Contract | null | undefined
 ) {
  return (
-    (sourceType === 'contract' && !sourceUpdateType && contract?.resolution) ||
-    (sourceType === 'contract' &&
-      sourceUpdateType === 'resolved' &&
-      contract?.resolution)
+    (sourceType === 'contract' && sourceUpdateType === 'resolved') ||
+    (sourceType === 'contract' && !sourceUpdateType && contract?.resolution)
  )
 }

@ -492,32 +495,58 @@ function NotificationItem(props: {
    reason,
    sourceUserUsername,
    createdTime,
+    sourceText,
+    sourceContractTitle,
+    sourceContractCreatorUsername,
+    sourceContractSlug,
  } = notification
  const [notificationText, setNotificationText] = useState<string>('')
  const [contract, setContract] = useState<Contract | null>(null)

  useEffect(() => {
-    if (!sourceContractId) return
-    getContractFromId(sourceContractId).then((contract) => {
-      if (contract) setContract(contract)
-    })
-  }, [sourceContractId])
+    if (
+      !sourceContractId ||
+      (sourceContractSlug && sourceContractCreatorUsername)
+    )
+      return
+    getContractFromId(sourceContractId)
+      .then((contract) => {
+        if (contract) setContract(contract)
+      })
+      .catch((e) => console.log(e))
+  }, [
+    sourceContractCreatorUsername,
+    sourceContractId,
+    sourceContractSlug,
+    sourceContractTitle,
+  ])

  useEffect(() => {
-    if (!contract || !sourceContractId || !sourceId) return
    if (
+      sourceText &&
+      (sourceType === 'comment' ||
+        sourceType === 'answer' ||
+        (sourceType === 'contract' && sourceUpdateType === 'updated'))
+    ) {
+      setNotificationText(sourceText)
+    } else if (!contract || !sourceContractId || !sourceId) return
+    else if (
      sourceType === 'answer' ||
      sourceType === 'comment' ||
      sourceType === 'contract'
    ) {
-      getNotificationText(
-        sourceId,
-        sourceContractId,
-        sourceType,
-        sourceUpdateType,
-        setNotificationText,
-        contract
-      )
+      try {
+        getNotificationText(
+          sourceId,
+          sourceContractId,
+          sourceType,
+          sourceUpdateType,
+          setNotificationText,
+          contract
+        )
+      } catch (err) {
+        console.error(err)
+      }
    } else if (reasonText) {
      // Handle arbitrary notifications with reason text here.
      setNotificationText(reasonText)
@ -527,6 +556,7 @@ function NotificationItem(props: {
    reasonText,
    sourceContractId,
    sourceId,
+    sourceText,
    sourceType,
    sourceUpdateType,
  ])
@ -537,6 +567,10 @@ function NotificationItem(props: {

  function getSourceUrl() {
    if (sourceType === 'follow') return `/${sourceUserUsername}`
+    if (sourceContractCreatorUsername && sourceContractSlug)
+      return `/${sourceContractCreatorUsername}/${sourceContractSlug}#${getSourceIdForLinkComponent(
+        sourceId ?? ''
+      )}`
    if (!contract) return ''
    return `/${contract.creatorUsername}/${
      contract.slug
@ -562,19 +596,16 @@ function NotificationItem(props: {
    sourceType: 'answer' | 'comment' | 'contract',
    sourceUpdateType: notification_source_update_types | undefined,
    setText: (text: string) => void,
-    contract?: Contract
+    contract: Contract
  ) {
-    if (sourceType === 'contract' && !contract)
-      contract = await getContractFromId(sourceContractId)
-
-    if (sourceType === 'contract' && contract) {
+    if (sourceType === 'contract') {
      if (
        isNotificationAboutContractResolution(
          sourceType,
          sourceUpdateType,
          contract
        ) &&
-        contract?.resolution
+        contract.resolution
      )
        setText(contract.resolution)
      else setText(contract.question)
@ -602,31 +633,24 @@ function NotificationItem(props: {
              className={'mr-0 flex-shrink-0'}
            />
            <div className={'inline-flex overflow-hidden text-ellipsis pl-1'}>
-              {sourceType &&
-                reason &&
-                getReasonForShowingNotification(
-                  sourceType,
-                  reason,
-                  sourceUpdateType,
-                  contract,
-                  true
-                ).replace(' on', '')}
+              <span className={'flex-shrink-0'}>
+                {sourceType &&
+                  reason &&
+                  getReasonForShowingNotification(
+                    sourceType,
+                    reason,
+                    sourceUpdateType,
+                    contract,
+                    true
+                  ).replace(' on', '')}
+              </span>
              <div className={'ml-1 text-black'}>
-                {contract ? (
-                  <NotificationTextLabel
-                    contract={contract}
-                    defaultText={notificationText}
-                    className={'line-clamp-1'}
-                    sourceUpdateType={sourceUpdateType}
-                    sourceType={sourceType}
-                  />
-                ) : sourceType != 'follow' ? (
-                  <LoadingIndicator
-                    spinnerClassName={'border-gray-500 h-4 w-4'}
-                  />
-                ) : (
-                  <div />
-                )}
+                <NotificationTextLabel
+                  contract={contract}
+                  defaultText={notificationText}
+                  className={'line-clamp-1'}
+                  notification={notification}
+                />
              </div>
            </div>
          </div>
@ -666,35 +690,40 @@ function NotificationItem(props: {
                      contract
                    )}
                    <span className={'mx-1 font-bold'}>
-                      {contract?.question}
+                      {contract?.question || sourceContractTitle}
                    </span>
                  </div>
                )}
              </div>
            </div>
-            {contract && sourceId && (
+            {sourceId && contract && (
              <CopyLinkDateTimeComponent
-                contract={contract}
+                contractCreatorUsername={contract.creatorUsername}
+                contractSlug={contract.slug}
                createdTime={createdTime}
                elementId={getSourceIdForLinkComponent(sourceId)}
                className={'-mx-1 inline-flex sm:inline-block'}
              />
            )}
+            {sourceId &&
+              sourceContractSlug &&
+              sourceContractCreatorUsername && (
+                <CopyLinkDateTimeComponent
+                  contractCreatorUsername={sourceContractCreatorUsername}
+                  contractSlug={sourceContractSlug}
+                  createdTime={createdTime}
+                  elementId={getSourceIdForLinkComponent(sourceId)}
+                  className={'-mx-1 inline-flex sm:inline-block'}
+                />
+              )}
          </div>
        </Row>
        <div className={'mt-1 md:text-base'}>
-          {contract ? (
-            <NotificationTextLabel
-              contract={contract}
-              defaultText={notificationText}
-              sourceType={sourceType}
-              sourceUpdateType={sourceUpdateType}
-            />
-          ) : sourceType != 'follow' ? (
-            <LoadingIndicator spinnerClassName={'border-gray-500 h-4 w-4'} />
-          ) : (
-            <div />
-          )}
+          <NotificationTextLabel
+            contract={contract}
+            defaultText={notificationText}
+            notification={notification}
+          />
        </div>

        <div className={'mt-6 border-b border-gray-300'} />
@ -704,14 +733,21 @@ function NotificationItem(props: {
 }

 function NotificationTextLabel(props: {
-  contract: Contract
  defaultText: string
-  sourceType?: notification_source_types
-  sourceUpdateType?: notification_source_update_types
+  contract?: Contract | null
+  notification: Notification
  className?: string
 }) {
-  const { contract, className, sourceUpdateType, sourceType, defaultText } =
-    props
+  const { contract, className, defaultText, notification } = props
+  const { sourceUpdateType, sourceType, sourceText, sourceContractTitle } =
+    notification
+  if (
+    !contract &&
+    !sourceContractTitle &&
+    !sourceText &&
+    sourceType !== 'follow'
+  )
+    return <LoadingIndicator spinnerClassName={'border-gray-500 h-4 w-4'} />

  if (
    isNotificationAboutContractResolution(
@ -719,7 +755,7 @@ function NotificationTextLabel(props: {
      sourceUpdateType,
      contract
    ) &&
-    contract.resolution
+    contract?.resolution
  ) {
    if (contract.outcomeType === 'FREE_RESPONSE') {
      return (
@ -782,7 +818,8 @@ function getReasonForShowingNotification(
      else reasonText = `commented on`
      break
    case 'contract':
-      if (
+      if (reason === 'you_follow_user') reasonText = 'created a new question'
+      else if (
        isNotificationAboutContractResolution(
          source,
          sourceUpdateType,
@ -794,7 +831,8 @@ function getReasonForShowingNotification(
      break
    case 'answer':
      if (reason === 'on_users_contract') reasonText = `answered your question `
-      if (reason === 'on_contract_with_users_comment') reasonText = `answered`
+      else if (reason === 'on_contract_with_users_comment')
+        reasonText = `answered`
      else if (reason === 'on_contract_with_users_answer')
        reasonText = `answered`
      else if (reason === 'on_contract_with_users_shares_in')