Multiple notifications fixes (#2154)

* Dont reset notifications feed on push notification event

* Dont separate notifications by read state to avoid jank

* On notifications screen focus, check latest and only rerender if not scrolled down

* Reuse the cached notifs page when its not stale

* Bump ios build number

* Improve comments

* Change the 'mark all read' condition to avoid firing too early

src/state/queries/notifications/feed.ts

@@ -16,7 +16,7 @@
  * 3. Don't call this query's `refetch()` if you're trying to sync latest; call `checkUnread()` instead.
  */
 
-import {useEffect} from 'react'
+import {useEffect, useRef} from 'react'
 import {AppBskyFeedDefs} from '@atproto/api'
 import {
   useInfiniteQuery,
@@ -49,6 +49,8 @@ export function useNotificationFeedQuery(opts?: {enabled?: boolean}) {
   const threadMutes = useMutedThreads()
   const unreads = useUnreadNotificationsApi()
   const enabled = opts?.enabled !== false
+  // state tracked across page fetches
+  const pageState = useRef({pageNum: 0, hasMarkedRead: false})
 
   const query = useInfiniteQuery<
     FeedPage,
@@ -60,17 +62,44 @@ export function useNotificationFeedQuery(opts?: {enabled?: boolean}) {
     staleTime: STALE.INFINITY,
     queryKey: RQKEY(),
     async queryFn({pageParam}: {pageParam: RQPageParam}) {
-      let page = await fetchPage({
-        limit: PAGE_SIZE,
-        cursor: pageParam,
-        queryClient,
-        moderationOpts,
-        threadMutes,
-      })
+      let page
+      if (!pageParam) {
+        // for the first page, we check the cached page held by the unread-checker first
+        page = unreads.getCachedUnreadPage()
+        // reset the page state
+        pageState.current = {pageNum: 0, hasMarkedRead: false}
+      }
+      if (!page) {
+        page = await fetchPage({
+          limit: PAGE_SIZE,
+          cursor: pageParam,
+          queryClient,
+          moderationOpts,
+          threadMutes,
+        })
+      }
 
-      // if the first page has an unread, mark all read
-      if (!pageParam && page.items[0] && !page.items[0].notification.isRead) {
-        unreads.markAllRead()
+      // NOTE
+      // this section checks to see if we need to mark notifs read
+      // we want to wait until we've seen a read notification because
+      // of a timing challenge; marking read on the first page would
+      // cause subsequent pages of unread notifs to incorrectly come
+      // back as "read". we use page 6 as an abort condition, which means
+      // after ~180 notifs we give up on tracking unread state correctly
+      // -prf
+      if (!pageState.current.hasMarkedRead) {
+        let hasMarkedRead = false
+        if (
+          pageState.current.pageNum > 5 ||
+          page.items.some(item => item.notification.isRead)
+        ) {
+          unreads.markAllRead()
+          hasMarkedRead = true
+        }
+        pageState.current = {
+          pageNum: pageState.current.pageNum + 1,
+          hasMarkedRead,
+        }
       }
 
       return page

src/state/queries/notifications/types.ts

@@ -28,7 +28,10 @@ export interface FeedPage {
 }
 
 export interface CachedFeedPage {
-  sessDid: string // used to invalidate on session changes
+  /**
+   * if true, the cached page is recent enough to use as the response
+   */
+  usableInFeed: boolean
   syncedAt: Date
   data: FeedPage | undefined
 }

src/state/queries/notifications/unread.tsx

@@ -37,7 +37,7 @@ const apiContext = React.createContext<ApiContext>({
 })
 
 export function Provider({children}: React.PropsWithChildren<{}>) {
-  const {hasSession, currentAccount} = useSession()
+  const {hasSession} = useSession()
   const queryClient = useQueryClient()
   const moderationOpts = useModerationOpts()
   const threadMutes = useMutedThreads()
@@ -46,7 +46,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
 
   const checkUnreadRef = React.useRef<ApiContext['checkUnread'] | null>(null)
   const cacheRef = React.useRef<CachedFeedPage>({
-    sessDid: currentAccount?.did || '',
+    usableInFeed: false,
     syncedAt: new Date(),
     data: undefined,
   })
@@ -65,7 +65,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
   React.useEffect(() => {
     const listener = ({data}: MessageEvent) => {
       cacheRef.current = {
-        sessDid: currentAccount?.did || '',
+        usableInFeed: false,
         syncedAt: new Date(),
         data: undefined,
       }
@@ -75,7 +75,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
     return () => {
       broadcast.removeEventListener('message', listener)
     }
-  }, [setNumUnread, currentAccount])
+  }, [setNumUnread])
 
   // create API
   const api = React.useMemo<ApiContext>(() => {
@@ -119,7 +119,7 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
           const lastIndexed =
             page.items[0] && new Date(page.items[0].notification.indexedAt)
           cacheRef.current = {
-            sessDid: currentAccount?.did || '',
+            usableInFeed: !!invalidate, // will be used immediately
             data: page,
             syncedAt: !lastIndexed || now > lastIndexed ? now : lastIndexed,
           }
@@ -136,14 +136,13 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
       },
 
       getCachedUnreadPage() {
-        // return cached page if was for the current user
-        // (protects against session changes serving data from the past session)
-        if (cacheRef.current.sessDid === currentAccount?.did) {
+        // return cached page if it's marked as fresh enough
+        if (cacheRef.current.usableInFeed) {
           return cacheRef.current.data
         }
       },
     }
-  }, [setNumUnread, queryClient, moderationOpts, threadMutes, currentAccount])
+  }, [setNumUnread, queryClient, moderationOpts, threadMutes])
   checkUnreadRef.current = api.checkUnread
 
   return (

src/state/queries/notifications/util.ts

@@ -119,8 +119,7 @@ function groupNotifications(
           Math.abs(ts2 - ts) < MS_2DAY &&
           notif.reason === groupedNotif.notification.reason &&
           notif.reasonSubject === groupedNotif.notification.reasonSubject &&
-          notif.author.did !== groupedNotif.notification.author.did &&
-          notif.isRead === groupedNotif.notification.isRead
+          notif.author.did !== groupedNotif.notification.author.did
         ) {
           groupedNotif.additional = groupedNotif.additional || []
           groupedNotif.additional.push(notif)