Skip to content

Commit

Permalink
Broadcast comment notifs, make isSeen more robust & transfer less data
Browse files Browse the repository at this point in the history
  • Loading branch information
IanPhilips committed Jul 30, 2024
1 parent ed15491 commit c9444b4
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 94 deletions.
5 changes: 3 additions & 2 deletions backend/api/src/get-notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@ export const getNotifications: APIHandler<'get-notifications'> = async (
props,
auth
) => {
const { limit } = props
const { limit, after } = props
const pg = createSupabaseDirectClient()
const query = `
select data from user_notifications
where user_id = $1
and ($3 is null or (data->'createdTime')::bigint > $3)
order by (data->'createdTime')::bigint desc
limit $2
`
return await pg.map(
query,
[auth.uid, limit],
[auth.uid, limit, after],
(row) => row.data as Notification
)
}
3 changes: 3 additions & 0 deletions backend/shared/src/supabase/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,7 @@ export const bulkInsertNotifications = async (
data: n,
}))
)
notifications.forEach((notification) =>
broadcast(`user-notifications/${notification.userId}`, { notification })
)
}
1 change: 1 addition & 0 deletions common/src/api/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1327,6 +1327,7 @@ export const API = (_apiTypeCheck = {
returns: [] as Notification[],
props: z
.object({
after: z.coerce.number().optional(),
limit: z.coerce.number().gte(0).lte(1000).default(100),
})
.strict(),
Expand Down
74 changes: 20 additions & 54 deletions web/components/notifications-icon.tsx
Original file line number Diff line number Diff line change
@@ -1,82 +1,48 @@
'use client'
import { BellIcon } from '@heroicons/react/outline'
import { BellIcon as SolidBellIcon } from '@heroicons/react/solid'
import { Row } from 'web/components/layout/row'
import { useEffect, useState } from 'react'
import { useEffect } from 'react'
import { usePrivateUser } from 'web/hooks/use-user'
import { useGroupedUnseenNotifications } from 'web/hooks/use-notifications'
import { PrivateUser } from 'common/user'
import { NOTIFICATIONS_PER_PAGE } from './notifications/notification-helpers'
import {
notification_source_types,
NotificationReason,
} from 'common/notification'

import { usePathname } from 'next/navigation'
import { usePersistentInMemoryState } from 'web/hooks/use-persistent-in-memory-state'

export function NotificationsIcon(props: {
className?: string
selectTypes?: notification_source_types[]
selectReasons?: NotificationReason[]
}) {
export function NotificationsIcon(props: { className?: string }) {
const privateUser = usePrivateUser()
const { selectTypes, selectReasons, className } = props
const { className } = props

return (
<Row className="relative justify-center">
{privateUser && (
<UnseenNotificationsBubble
selectTypes={selectTypes}
privateUser={privateUser}
selectReasons={selectReasons}
/>
)}
{privateUser && <UnseenNotificationsBubble privateUser={privateUser} />}
<BellIcon className={className} />
</Row>
)
}

export function SolidNotificationsIcon(props: {
className?: string
selectTypes?: notification_source_types[]
selectReasons?: NotificationReason[]
}) {
const privateUser = usePrivateUser()
const { selectTypes, selectReasons, className } = props
return (
<Row className="relative justify-center">
{privateUser && (
<UnseenNotificationsBubble
selectTypes={selectTypes}
privateUser={privateUser}
selectReasons={selectReasons}
/>
)}
<SolidBellIcon className={className} />
</Row>
)
}

function UnseenNotificationsBubble(props: {
privateUser: PrivateUser
selectTypes?: notification_source_types[]
selectReasons?: NotificationReason[]
}) {
function UnseenNotificationsBubble(props: { privateUser: PrivateUser }) {
const pathname = usePathname()
const { privateUser, selectTypes, selectReasons } = props
const [seen, setSeen] = useState(false)
const unseenSourceIdsToNotificationIds =
useGroupedUnseenNotifications(privateUser.id, selectTypes, selectReasons) ??
[]
const { privateUser } = props
const [lastSeenTime, setLastSeenTime] = usePersistentInMemoryState(
0,
'notifications-seen-time'
)
const unseenNotificationGroups =
useGroupedUnseenNotifications(privateUser.id) ?? []

const unseenNotifs = Object.keys(unseenSourceIdsToNotificationIds).length
const unseenNotifs = unseenNotificationGroups.filter(
(ng) => ng.latestCreatedTime > lastSeenTime
).length

useEffect(() => {
if (pathname?.endsWith('notifications')) {
setSeen(pathname.endsWith('notifications'))
setLastSeenTime(Date.now())
}
}, [pathname])
}, [pathname, unseenNotifs])

if (unseenNotifs === 0 || seen) {
if (unseenNotifs === 0) {
return null
}

Expand Down
78 changes: 48 additions & 30 deletions web/hooks/use-notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
notification_source_types,
NotificationReason,
} from 'common/notification'
import { Dictionary, first, groupBy, sortBy } from 'lodash'
import { Dictionary, first, groupBy, sortBy, uniqBy } from 'lodash'
import { useEffect, useMemo } from 'react'
import { NOTIFICATIONS_PER_PAGE } from 'web/components/notifications/notification-helpers'
import { User } from 'common/user'
Expand All @@ -18,53 +18,70 @@ export type NotificationGroup = {
notifications: Notification[]
groupedById: string
isSeen: boolean
latestCreatedTime: number
}

function useNotifications(userId: string, count = 15 * NOTIFICATIONS_PER_PAGE) {
function useNotifications(
userId: string,
count = 15 * NOTIFICATIONS_PER_PAGE,
newOnly?: boolean
) {
const [notifications, setNotifications] = usePersistentLocalState<
Notification[] | undefined
>(undefined, 'notifications-' + userId)
const [latestCreatedTime, setLatestCreatedTime] = usePersistentLocalState<
number | undefined
>(undefined, 'latest-notification-time-' + userId)

useEffect(() => {
if (userId)
api('get-notifications', { limit: count }).then((data) => {
setNotifications(data)
if (userId) {
const params = {
limit: count,
after: newOnly ? latestCreatedTime : undefined,
}
api('get-notifications', params).then((newData) => {
setNotifications((oldData) => {
const updatedNotifications = uniqBy(
[...newData, ...(oldData ?? [])],
'id'
)
const newLatestCreatedTime = Math.max(
...updatedNotifications.map((n) => n.createdTime),
latestCreatedTime ?? 0
)
setLatestCreatedTime(newLatestCreatedTime)
return updatedNotifications
})
})
}, [userId])
}
}, [userId, count, newOnly])

useApiSubscription({
topics: [`user-notifications/${userId}`],
onBroadcast: ({ data }) => {
setNotifications((notifs) => [
data.notification as Notification,
...(notifs ?? []),
])
console.log('new notification', data)
setNotifications((notifs) => {
const newNotification = data.notification as Notification
setLatestCreatedTime((prevTime) =>
Math.max(prevTime ?? 0, newNotification.createdTime)
)
return [newNotification, ...(notifs ?? [])]
})
},
})
return notifications
}

function useUnseenNotifications(
userId: string,
count = NOTIFICATIONS_PER_PAGE
) {
const notifs = useNotifications(userId, count)
return notifs?.filter((n) => !n.isSeen)
}
export function useGroupedUnseenNotifications(userId: string) {
const unseenNotifs = useNotifications(
userId,
NOTIFICATIONS_PER_PAGE,
true
)?.filter((n) => !n.isSeen)

export function useGroupedUnseenNotifications(
userId: string,
selectTypes?: notification_source_types[],
selectReasons?: NotificationReason[]
) {
const notifications = useUnseenNotifications(userId)?.filter(
(n) =>
(selectTypes?.includes(n.sourceType) ||
selectReasons?.includes(n.reason)) ??
true
)
return useMemo(() => {
return notifications ? groupNotificationsForIcon(notifications) : undefined
}, [notifications])
return unseenNotifs ? groupNotificationsForIcon(unseenNotifs) : undefined
}, [unseenNotifs?.length])
}

export function useGroupedNotifications(
Expand Down Expand Up @@ -128,6 +145,7 @@ const groupNotifications = (
notifications: value,
groupedById: key,
isSeen: value.every((n) => n.isSeen),
latestCreatedTime: Math.max(...value.map((n) => n.createdTime)),
}))
}

Expand Down
11 changes: 3 additions & 8 deletions web/pages/notifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
} from 'web/hooks/use-notifications'
import { useIsPageVisible } from 'web/hooks/use-page-visible'
import { useRedirectIfSignedOut } from 'web/hooks/use-redirect-if-signed-out'
import { usePrivateUser, useIsAuthorized, useUser } from 'web/hooks/use-user'
import { usePrivateUser, useUser } from 'web/hooks/use-user'
import { XIcon } from '@heroicons/react/outline'
import { getNativePlatform } from 'web/lib/native/is-native'
import { AppBadgesOrGetAppButton } from 'web/components/buttons/app-badges-or-get-app-button'
Expand Down Expand Up @@ -240,7 +240,6 @@ export function NotificationsList(props: {
groupedNotifications,
mostRecentNotification,
} = props
const isAuthorized = useIsAuthorized()
const [page, setPage] = useState(0)
const user = useUser()

Expand All @@ -255,12 +254,8 @@ export function NotificationsList(props: {
// Mark all notifications as seen. Rerun as new notifications come in.
useEffect(() => {
if (!privateUser || !isPageVisible) return
if (isAuthorized) markAllNotifications({ seen: true })
groupedNotifications
?.map((ng) => ng.notifications)
.flat()
.forEach((n) => (!n.isSeen ? (n.isSeen = true) : null))
}, [privateUser, isPageVisible, mostRecentNotification?.id, isAuthorized])
markAllNotifications({ seen: true })
}, [privateUser?.id, isPageVisible, mostRecentNotification?.id])

return (
<Col className={'min-h-[100vh] gap-0 text-sm'}>
Expand Down

0 comments on commit c9444b4

Please sign in to comment.