Push notification fixes (#210)

* Fix to when screen analytics events are firing

* Fix: dont trigger update state when backgrounded

* Small fix to notifee API usage

* Fix: properly load notification info for push card

src/App.native.tsx

@@ -49,6 +49,7 @@ const App = observer(() => {
           store.nav.switchTo(TabPurpose.Notifs, true)
         }
       })
+      notifee.onBackgroundEvent(async _e => {}) // notifee requires this but we handle it with onForegroundEvent
     })
   }, [])
 

src/state/index.ts

@@ -1,5 +1,5 @@
 import {autorun} from 'mobx'
-import {Platform} from 'react-native'
+import {AppState, Platform} from 'react-native'
 import {AtpAgent} from '@atproto/api'
 import {RootStoreModel} from './models/root-store'
 import * as libapi from './lib/api'
@@ -37,7 +37,14 @@ export async function setupState(serviceUri = DEFAULT_SERVICE) {
 
   // periodic state fetch
   setInterval(() => {
-    rootStore.updateSessionState()
+    // NOTE
+    // this must ONLY occur when the app is active, as the bg-fetch handler
+    // will wake up the thread and cause this interval to fire, which in
+    // turn schedules a bunch of work at a poor time
+    // -prf
+    if (AppState.currentState === 'active') {
+      rootStore.updateSessionState()
+    }
   }, STATE_FETCH_INTERVAL)
 
   return rootStore

src/state/models/notifications-view.ts

@@ -200,7 +200,7 @@ export class NotificationsViewModel {
   notifications: NotificationsViewItemModel[] = []
 
   // this is used to help trigger push notifications
-  mostRecentNotification: NotificationsViewItemModel | undefined
+  mostRecentNotificationUri: string | undefined
 
   constructor(
     public rootStore: RootStoreModel,
@@ -211,7 +211,7 @@ export class NotificationsViewModel {
       {
         rootStore: false,
         params: false,
-        mostRecentNotification: false,
+        mostRecentNotificationUri: false,
       },
       {autoBind: true},
     )
@@ -245,7 +245,7 @@ export class NotificationsViewModel {
     this.hasMore = true
     this.loadMoreCursor = undefined
     this.notifications = []
-    this.mostRecentNotification = undefined
+    this.mostRecentNotificationUri = undefined
   }
 
   /**
@@ -365,23 +365,21 @@ export class NotificationsViewModel {
   }
 
   async getNewMostRecent(): Promise<NotificationsViewItemModel | undefined> {
-    let old = this.mostRecentNotification
+    let old = this.mostRecentNotificationUri
     const res = await this.rootStore.api.app.bsky.notification.list({
       limit: 1,
     })
-    if (
+    if (!res.data.notifications[0] || old === res.data.notifications[0].uri) {
-      !res.data.notifications[0] ||
-      old?.uri === res.data.notifications[0].uri
-    ) {
       return
     }
-    this.mostRecentNotification = new NotificationsViewItemModel(
+    this.mostRecentNotificationUri = res.data.notifications[0].uri
+    const notif = new NotificationsViewItemModel(
       this.rootStore,
       'mostRecent',
       res.data.notifications[0],
     )
-    await this.mostRecentNotification.fetchAdditionalData()
+    await notif.fetchAdditionalData()
-    return this.mostRecentNotification
+    return notif
   }
 
   // state transitions
@@ -408,11 +406,7 @@ export class NotificationsViewModel {
 
   private async _replaceAll(res: ListNotifications.Response) {
     if (res.data.notifications[0]) {
-      this.mostRecentNotification = new NotificationsViewItemModel(
+      this.mostRecentNotificationUri = res.data.notifications[0].uri
-        this.rootStore,
-        'mostRecent',
-        res.data.notifications[0],
-      )
     }
     return this._appendAll(res, true)
   }

src/view/com/posts/Feed.tsx

@@ -37,13 +37,9 @@ export const Feed = observer(function Feed({
   testID?: string
   headerOffset?: number
 }) {
-  const {screen, track} = useAnalytics()
+  const {track} = useAnalytics()
   const [isRefreshing, setIsRefreshing] = React.useState(false)
 
-  React.useEffect(() => {
-    screen('Feed')
-  }, [screen])
-
   // TODO optimize renderItem or FeedItem, we're getting this notice from RN: -prf
   //   VirtualizedList: You have a large list that is slow to update - make sure your
   //   renderItem function renders components that follow React performance best practices

src/view/screens/Home.tsx

@@ -21,7 +21,7 @@ const HITSLOP = {left: 20, top: 20, right: 20, bottom: 20}
 export const Home = observer(function Home({navIdx, visible}: ScreenParams) {
   const store = useStores()
   const onMainScroll = useOnMainScroll(store)
-  const {track} = useAnalytics()
+  const {screen, track} = useAnalytics()
   const safeAreaInsets = useSafeAreaInsets()
   const scrollElRef = React.useRef<FlatList>(null)
   const [wasVisible, setWasVisible] = React.useState<boolean>(false)
@@ -59,6 +59,9 @@ export const Home = observer(function Home({navIdx, visible}: ScreenParams) {
       feedCleanup()
     }
 
+    // guard to only continue when transitioning from !visible -> visible
+    // TODO is this 100% needed? depends on if useEffect() is getting refired
+    //      for reasons other than `visible` changing -prf
     if (!visible) {
       setWasVisible(false)
       return cleanup
@@ -67,6 +70,8 @@ export const Home = observer(function Home({navIdx, visible}: ScreenParams) {
     }
     setWasVisible(true)
 
+    // just became visible
+    screen('Feed')
     store.nav.setTitle(navIdx, 'Home')
     store.log.debug('Updating home feed')
     if (store.me.mainFeed.hasContent) {
@@ -75,7 +80,7 @@ export const Home = observer(function Home({navIdx, visible}: ScreenParams) {
       store.me.mainFeed.setup()
     }
     return cleanup
-  }, [visible, store, store.me.mainFeed, navIdx, doPoll, wasVisible, scrollToTop])
+  }, [visible, store, store.me.mainFeed, navIdx, doPoll, wasVisible, scrollToTop, screen])
 
   const onPressCompose = (imagesOpen?: boolean) => {
     track('Home:ComposeButtonPressed')

src/view/screens/Notifications.tsx

@@ -14,10 +14,6 @@ export const Notifications = ({navIdx, visible}: ScreenParams) => {
   const scrollElRef = React.useRef<FlatList>(null)
   const {screen} = useAnalytics()
 
-  useEffect(() => {
-    screen('Notifications')
-  }, [screen])
-
   const onSoftReset = () => {
     scrollElRef.current?.scrollToOffset({offset: 0})
   }
@@ -34,9 +30,10 @@ export const Notifications = ({navIdx, visible}: ScreenParams) => {
     store.me.notifications.update().then(() => {
       store.me.notifications.updateReadState()
     })
+    screen('Notifications')
     store.nav.setTitle(navIdx, 'Notifications')
     return cleanup
-  }, [visible, store, navIdx])
+  }, [visible, store, navIdx, screen])
 
   const onPressTryAgain = () => {
     store.me.notifications.refresh()