From 936cabe35300395771ee2ff6ef8d3631e09a587a Mon Sep 17 00:00:00 2001 From: Ian Philips Date: Wed, 8 Jun 2022 08:43:24 -0600 Subject: [PATCH] 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 --- common/notification.ts | 5 + functions/src/create-notification.ts | 34 +++- functions/src/index.ts | 1 + functions/src/on-create-answer.ts | 1 + functions/src/on-create-comment.ts | 4 +- functions/src/on-create-contract.ts | 24 +++ functions/src/on-follow-user.ts | 1 + functions/src/on-update-contract.ts | 2 + web/components/feed/copy-link-date-time.tsx | 18 +- .../feed/feed-answer-comment-group.tsx | 3 +- web/components/feed/feed-comments.tsx | 3 +- web/hooks/use-notifications.ts | 4 +- web/pages/notifications.tsx | 190 +++++++++++------- 13 files changed, 198 insertions(+), 92 deletions(-) create mode 100644 functions/src/on-create-contract.ts diff --git a/common/notification.ts b/common/notification.ts index 9ccf6f98..d6615514 100644 --- 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' diff --git a/functions/src/create-notification.ts b/functions/src/create-notification.ts index 308ed490..892b42eb 100644 --- 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 } diff --git a/functions/src/index.ts b/functions/src/index.ts index f3099fec..85e6e779 100644 --- 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 diff --git a/functions/src/on-create-answer.ts b/functions/src/on-create-answer.ts index 17863205..78fd1399 100644 --- 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 ) }) diff --git a/functions/src/on-create-comment.ts b/functions/src/on-create-comment.ts index 75bfc6b1..8d52fd46 100644 --- 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([ diff --git a/functions/src/on-create-contract.ts b/functions/src/on-create-contract.ts new file mode 100644 index 00000000..040ba378 --- /dev/null +++ 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 + ) + }) diff --git a/functions/src/on-follow-user.ts b/functions/src/on-follow-user.ts index 61c671db..07fd0454 100644 --- 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 diff --git a/functions/src/on-update-contract.ts b/functions/src/on-update-contract.ts index 3bf202a2..4a11d811 100644 --- 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 ) } diff --git a/web/components/feed/copy-link-date-time.tsx b/web/components/feed/copy-link-date-time.tsx index 0ffac52e..ac5b54c6 100644 --- 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 ) { 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: {
answered diff --git a/web/components/feed/feed-comments.tsx b/web/components/feed/feed-comments.tsx index fb72179b..8034144c 100644 --- a/web/components/feed/feed-comments.tsx +++ b/web/components/feed/feed-comments.tsx @@ -245,7 +245,8 @@ export function FeedComment(props: { )} diff --git a/web/hooks/use-notifications.ts b/web/hooks/use-notifications.ts index ece1a8fa..051d6cbb 100644 --- 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(() => { diff --git a/web/pages/notifications.tsx b/web/pages/notifications.tsx index c6579cab..dc258a38 100644 --- 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 ? (
{unseenNotificationGroups.length === 0 && "You don't have any new notifications."} @@ -113,11 +114,13 @@ export default function Notifications() { ) )}
+ ) : ( + ), }, { title: 'All Notifications', - content: ( + content: allNotificationGroups ? (
{allNotificationGroups.length === 0 && "You don't have any notifications. Try changing your settings to see more."} @@ -138,6 +141,8 @@ export default function Notifications() { ) )}
+ ) : ( + ), }, { @@ -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('') const [contract, setContract] = useState(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'} />
- {sourceType && - reason && - getReasonForShowingNotification( - sourceType, - reason, - sourceUpdateType, - contract, - true - ).replace(' on', '')} + + {sourceType && + reason && + getReasonForShowingNotification( + sourceType, + reason, + sourceUpdateType, + contract, + true + ).replace(' on', '')} +
- {contract ? ( - - ) : sourceType != 'follow' ? ( - - ) : ( -
- )} +
@@ -666,35 +690,40 @@ function NotificationItem(props: { contract )} - {contract?.question} + {contract?.question || sourceContractTitle}
)} - {contract && sourceId && ( + {sourceId && contract && ( )} + {sourceId && + sourceContractSlug && + sourceContractCreatorUsername && ( + + )}
- {contract ? ( - - ) : sourceType != 'follow' ? ( - - ) : ( -
- )} +
@@ -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 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')