From 9c15d5b96c8428968de78b4d077efb0e41d384c1 Mon Sep 17 00:00:00 2001 From: James Grugett Date: Sun, 28 Aug 2022 15:20:21 -0500 Subject: [PATCH] React-query-ify notifications (#812) * Use single react query to subscribe to notifications * Remove 'preferred' in variable names --- web/components/groups/group-chat.tsx | 23 ++++--- web/components/notifications-icon.tsx | 4 +- web/hooks/use-notifications.ts | 89 ++++++++------------------- web/lib/firebase/notifications.ts | 16 ----- web/pages/notifications.tsx | 35 +++-------- 5 files changed, 51 insertions(+), 116 deletions(-) diff --git a/web/components/groups/group-chat.tsx b/web/components/groups/group-chat.tsx index 781705c2..244a3ffe 100644 --- a/web/components/groups/group-chat.tsx +++ b/web/components/groups/group-chat.tsx @@ -19,7 +19,7 @@ import { sum } from 'lodash' import { formatMoney } from 'common/util/format' import { useWindowSize } from 'web/hooks/use-window-size' import { Content, useTextEditor } from 'web/components/editor' -import { useUnseenPreferredNotifications } from 'web/hooks/use-notifications' +import { useUnseenNotifications } from 'web/hooks/use-notifications' import { ChevronDownIcon, UsersIcon } from '@heroicons/react/outline' import { setNotificationsAsSeen } from 'web/pages/notifications' import { usePrivateUser } from 'web/hooks/use-user' @@ -277,14 +277,18 @@ function GroupChatNotificationsIcon(props: { hidden: boolean }) { const { privateUser, group, shouldSetAsSeen, hidden } = props - const preferredNotificationsForThisGroup = useUnseenPreferredNotifications( - privateUser, - { - customHref: `/group/${group.slug}`, - } + const notificationsForThisGroup = useUnseenNotifications( + privateUser + // Disabled tracking by customHref for now. + // { + // customHref: `/group/${group.slug}`, + // } ) + useEffect(() => { - preferredNotificationsForThisGroup.forEach((notification) => { + if (!notificationsForThisGroup) return + + notificationsForThisGroup.forEach((notification) => { if ( (shouldSetAsSeen && notification.isSeenOnHref?.includes('chat')) || // old style chat notif that simply ended with the group slug @@ -293,13 +297,14 @@ function GroupChatNotificationsIcon(props: { setNotificationsAsSeen([notification]) } }) - }, [group.slug, preferredNotificationsForThisGroup, shouldSetAsSeen]) + }, [group.slug, notificationsForThisGroup, shouldSetAsSeen]) return (
0 && + notificationsForThisGroup && + notificationsForThisGroup.length > 0 && !shouldSetAsSeen ? 'absolute right-4 top-4 h-3 w-3 rounded-full border-2 border-white bg-red-500' : 'hidden' diff --git a/web/components/notifications-icon.tsx b/web/components/notifications-icon.tsx index dbdad6a9..55284e96 100644 --- a/web/components/notifications-icon.tsx +++ b/web/components/notifications-icon.tsx @@ -4,7 +4,7 @@ import { Row } from 'web/components/layout/row' import { useEffect, useState } from 'react' import { usePrivateUser } from 'web/hooks/use-user' import { useRouter } from 'next/router' -import { useUnseenPreferredNotificationGroups } from 'web/hooks/use-notifications' +import { useUnseenGroupedNotification } from 'web/hooks/use-notifications' import { NOTIFICATIONS_PER_PAGE } from 'web/pages/notifications' import { PrivateUser } from 'common/user' @@ -30,7 +30,7 @@ function UnseenNotificationsBubble(props: { privateUser: PrivateUser }) { else setSeen(false) }, [router.pathname]) - const notifications = useUnseenPreferredNotificationGroups(privateUser) + const notifications = useUnseenGroupedNotification(privateUser) if (!notifications || notifications.length === 0 || seen) { return
} diff --git a/web/hooks/use-notifications.ts b/web/hooks/use-notifications.ts index 32500943..d02d3d30 100644 --- a/web/hooks/use-notifications.ts +++ b/web/hooks/use-notifications.ts @@ -1,13 +1,9 @@ -import { useEffect, useMemo, useState } from 'react' +import { useMemo } from 'react' import { notification_subscribe_types, PrivateUser } from 'common/user' import { Notification } from 'common/notification' -import { - getNotificationsQuery, - listenForNotifications, -} from 'web/lib/firebase/notifications' +import { getNotificationsQuery } from 'web/lib/firebase/notifications' import { groupBy, map, partition } from 'lodash' import { useFirestoreQueryData } from '@react-query-firebase/firestore' -import { NOTIFICATIONS_PER_PAGE } from 'web/pages/notifications' export type NotificationGroup = { notifications: Notification[] @@ -17,49 +13,48 @@ export type NotificationGroup = { type: 'income' | 'normal' } -// For some reason react-query subscriptions don't actually listen for notifications -// Use useUnseenPreferredNotificationGroups to listen for new notifications -export function usePreferredGroupedNotifications( - privateUser: PrivateUser, - cachedNotifications?: Notification[] -) { +function useNotifications(privateUser: PrivateUser) { const result = useFirestoreQueryData( ['notifications-all', privateUser.id], - getNotificationsQuery(privateUser.id) + getNotificationsQuery(privateUser.id), + { subscribe: true, includeMetadataChanges: true }, + // Temporary workaround for react-query bug: + // https://github.com/invertase/react-query-firebase/issues/25 + { cacheTime: 0 } ) const notifications = useMemo(() => { - if (result.isLoading) return cachedNotifications ?? [] - if (!result.data) return cachedNotifications ?? [] + if (!result.data) return undefined const notifications = result.data as Notification[] return getAppropriateNotifications( notifications, privateUser.notificationPreferences ).filter((n) => !n.isSeenOnHref) - }, [ - cachedNotifications, - privateUser.notificationPreferences, - result.data, - result.isLoading, - ]) + }, [privateUser.notificationPreferences, result.data]) + return notifications +} + +export function useUnseenNotifications(privateUser: PrivateUser) { + const notifications = useNotifications(privateUser) + return useMemo( + () => notifications && notifications.filter((n) => !n.isSeen), + [notifications] + ) +} + +export function useGroupedNotifications(privateUser: PrivateUser) { + const notifications = useNotifications(privateUser) return useMemo(() => { if (notifications) return groupNotifications(notifications) }, [notifications]) } -export function useUnseenPreferredNotificationGroups(privateUser: PrivateUser) { - const notifications = useUnseenPreferredNotifications(privateUser, {}) - const [notificationGroups, setNotificationGroups] = useState< - NotificationGroup[] | undefined - >(undefined) - useEffect(() => { - if (!notifications) return - - const groupedNotifications = groupNotifications(notifications) - setNotificationGroups(groupedNotifications) +export function useUnseenGroupedNotification(privateUser: PrivateUser) { + const notifications = useUnseenNotifications(privateUser) + return useMemo(() => { + if (notifications) return groupNotifications(notifications) }, [notifications]) - return notificationGroups } export function groupNotifications(notifications: Notification[]) { @@ -114,36 +109,6 @@ export function groupNotifications(notifications: Notification[]) { return notificationGroups } -export function useUnseenPreferredNotifications( - privateUser: PrivateUser, - options: { customHref?: string }, - limit: number = NOTIFICATIONS_PER_PAGE -) { - const { customHref } = options - const [notifications, setNotifications] = useState([]) - const [userAppropriateNotifications, setUserAppropriateNotifications] = - useState([]) - - useEffect(() => { - return listenForNotifications(privateUser.id, setNotifications, { - unseenOnly: true, - limit, - }) - }, [limit, privateUser.id]) - - useEffect(() => { - const notificationsToShow = getAppropriateNotifications( - notifications, - privateUser.notificationPreferences - ).filter((n) => - customHref ? n.isSeenOnHref?.includes(customHref) : !n.isSeenOnHref - ) - setUserAppropriateNotifications(notificationsToShow) - }, [notifications, customHref, privateUser.notificationPreferences]) - - return userAppropriateNotifications -} - const lessPriorityReasons = [ 'on_contract_with_users_comment', 'on_contract_with_users_answer', diff --git a/web/lib/firebase/notifications.ts b/web/lib/firebase/notifications.ts index d2db3665..38d93402 100644 --- a/web/lib/firebase/notifications.ts +++ b/web/lib/firebase/notifications.ts @@ -1,7 +1,5 @@ import { collection, limit, orderBy, query, where } from 'firebase/firestore' -import { Notification } from 'common/notification' import { db } from 'web/lib/firebase/init' -import { listenForValues } from 'web/lib/firebase/utils' import { NOTIFICATIONS_PER_PAGE } from 'web/pages/notifications' export function getNotificationsQuery( @@ -23,17 +21,3 @@ export function getNotificationsQuery( limit(NOTIFICATIONS_PER_PAGE * 10) ) } - -export function listenForNotifications( - userId: string, - setNotifications: (notifs: Notification[]) => void, - unseenOnlyOptions?: { unseenOnly: boolean; limit: number } -) { - return listenForValues( - getNotificationsQuery(userId, unseenOnlyOptions), - (notifs) => { - notifs.sort((n1, n2) => n2.createdTime - n1.createdTime) - setNotifications(notifs) - } - ) -} diff --git a/web/pages/notifications.tsx b/web/pages/notifications.tsx index bfd18f7f..85cbcbae 100644 --- a/web/pages/notifications.tsx +++ b/web/pages/notifications.tsx @@ -26,7 +26,7 @@ import { } from 'web/components/outcome-label' import { NotificationGroup, - usePreferredGroupedNotifications, + useGroupedNotifications, } from 'web/hooks/use-notifications' import { TrendingUpIcon } from '@heroicons/react/outline' import { formatMoney } from 'common/util/format' @@ -45,6 +45,7 @@ import { SiteLink } from 'web/components/site-link' import { NotificationSettings } from 'web/components/NotificationSettings' import { SEO } from 'web/components/SEO' import { useUser } from 'web/hooks/use-user' +import { LoadingIndicator } from 'web/components/loading-indicator' export const NOTIFICATIONS_PER_PAGE = 30 const MULTIPLE_USERS_KEY = 'multipleUsers' @@ -58,16 +59,6 @@ export default function Notifications(props: { auth: { privateUser: PrivateUser } }) { const { privateUser } = props.auth - const local = safeLocalStorage() - let localNotifications = [] as Notification[] - const localSavedNotificationGroups = local?.getItem('notification-groups') - let localNotificationGroups = [] as NotificationGroup[] - if (localSavedNotificationGroups) { - localNotificationGroups = JSON.parse(localSavedNotificationGroups) - localNotifications = localNotificationGroups - .map((g) => g.notifications) - .flat() - } return ( @@ -84,12 +75,7 @@ export default function Notifications(props: { tabs={[ { title: 'Notifications', - content: ( - - ), + content: , }, { title: 'Settings', @@ -135,16 +121,10 @@ function RenderNotificationGroups(props: { ) } -function NotificationsList(props: { - privateUser: PrivateUser - cachedNotifications: Notification[] -}) { - const { privateUser, cachedNotifications } = props +function NotificationsList(props: { privateUser: PrivateUser }) { + const { privateUser } = props const [page, setPage] = useState(0) - const allGroupedNotifications = usePreferredGroupedNotifications( - privateUser, - cachedNotifications - ) + const allGroupedNotifications = useGroupedNotifications(privateUser) const paginatedGroupedNotifications = useMemo(() => { if (!allGroupedNotifications) return const start = page * NOTIFICATIONS_PER_PAGE @@ -163,7 +143,8 @@ function NotificationsList(props: { return maxNotificationsToShow }, [allGroupedNotifications, page]) - if (!paginatedGroupedNotifications || !allGroupedNotifications) return
+ if (!paginatedGroupedNotifications || !allGroupedNotifications) + return return (