chore(points): add error handling for points balance fetch (valora-in…

…c#5392) ### Description This PR adds a warning toast when there is an error fetching the history / balance on the points home screen. I made some changes to the history error handling so that we have consistent approaches between the 2 screens: 1. don't use the error state to erase data in redux - i think this is an unexpected pattern, usually we'd want to keep the existing data in place and show an error 2. use the Toast component so that we have nice animations ### Test plan Error on fetch first page: https://github.com/valora-inc/wallet/assets/20150449/6ff4675c-7127-45b8-9a64-10e60f8abef5 Error on fetch next pages (I reduced the size of the bottom sheet because of the small amount of test data available): https://github.com/valora-inc/wallet/assets/20150449/4a3dbe37-59ac-412f-a8e2-f7ec45c94021 ### Related issues - Related to RET-1073 ### Backwards compatibility Y ### Network scalability If a new NetworkId and/or Network are added in the future, the changes in this PR will: - [x] Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)
zed-io · May 15, 2024 · 0e202a0 · 0e202a0
1 parent a695124
commit 0e202a0
Show file tree

Hide file tree

Showing 9 changed files with 95 additions and 71 deletions.
diff --git a/locales/base/translation.json b/locales/base/translation.json
@@ -2289,6 +2289,11 @@
         "refresh": "Refresh"
       }
     },
+    "fetchBalanceError": {
+      "title": "Unable to load your points",
+      "description": "Oops, something went wrong when trying to load your points. The information on this page may be out of date. Please refresh.",
+      "retryCta": "Refresh"
+    },
     "loading": {
       "title": "Gathering your points",
       "description": "We're in the process of tallying up your points, hang tight"

diff --git a/src/points/PointsHistoryBottomSheet.test.tsx b/src/points/PointsHistoryBottomSheet.test.tsx
@@ -1,14 +1,14 @@
-import * as React from 'react'
 import { fireEvent, render, waitFor } from '@testing-library/react-native'
-import PointsHistoryBottomSheet from 'src/points/PointsHistoryBottomSheet'
-import { createMockStore, RecursivePartial } from 'test/utils'
-import { RootState } from 'src/redux/reducers'
 import { FetchMock } from 'jest-fetch-mock/types'
+import * as React from 'react'
 import { Provider } from 'react-redux'
-import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
 import { PointsEvents } from 'src/analytics/Events'
+import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
+import PointsHistoryBottomSheet from 'src/points/PointsHistoryBottomSheet'
 import { getHistoryStarted } from 'src/points/slice'
 import { GetHistoryResponse } from 'src/points/types'
+import { RootState } from 'src/redux/reducers'
+import { RecursivePartial, createMockStore } from 'test/utils'
 
 jest.mock('src/statsig', () => ({
   getDynamicConfigParams: jest.fn().mockReturnValue({
@@ -116,12 +116,12 @@ describe(PointsHistoryBottomSheet, () => {
   })
 
   it('shows error screen if fetch fails', async () => {
-    const tree = renderScreen({ points: { getHistoryStatus: 'error' } })
+    const tree = renderScreen({ points: { getHistoryStatus: 'errorFirstPage' } })
     expect(tree.getByTestId('PointsHistoryBottomSheet/Error')).toBeTruthy()
   })
 
   it('dispatches an action when try again is pressed', async () => {
-    const { dispatch, getByText } = renderScreen({ points: { getHistoryStatus: 'error' } })
+    const { dispatch, getByText } = renderScreen({ points: { getHistoryStatus: 'errorFirstPage' } })
     fireEvent.press(getByText('points.history.error.tryAgain'))
     await waitFor(() =>
       expect(ValoraAnalytics.track).toHaveBeenCalledWith(
@@ -151,14 +151,14 @@ describe(PointsHistoryBottomSheet, () => {
 
   it('shows inline error if failure while fetching subsequent page', async () => {
     const tree = renderScreen({
-      points: { getHistoryStatus: 'error', pointsHistory: MOCK_RESPONSE_NO_NEXT_PAGE.data },
+      points: { getHistoryStatus: 'errorNextPage', pointsHistory: MOCK_RESPONSE_NO_NEXT_PAGE.data },
     })
     expect(tree.getByTestId('PointsHistoryBottomSheet/ErrorBanner')).toBeTruthy()
   })
 
   it('refreshes if error banner CTA is pressed', async () => {
     const { dispatch, getByText } = renderScreen({
-      points: { getHistoryStatus: 'error', pointsHistory: MOCK_RESPONSE_NO_NEXT_PAGE.data },
+      points: { getHistoryStatus: 'errorNextPage', pointsHistory: MOCK_RESPONSE_NO_NEXT_PAGE.data },
     })
     fireEvent.press(getByText('points.history.pageError.refresh'))
     await waitFor(() =>

diff --git a/src/points/PointsHistoryBottomSheet.tsx b/src/points/PointsHistoryBottomSheet.tsx
@@ -1,14 +1,15 @@
 import GorhomBottomSheet, { BottomSheetSectionList } from '@gorhom/bottom-sheet'
-import React, { useEffect, useMemo, useState } from 'react'
+import React, { useMemo } from 'react'
 import { useTranslation } from 'react-i18next'
 import { ActivityIndicator, ListRenderItem, StyleSheet, Text, View } from 'react-native'
 import { useSafeAreaInsets } from 'react-native-safe-area-context'
 import { PointsEvents } from 'src/analytics/Events'
 import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
 import BottomSheetBase from 'src/components/BottomSheetBase'
 import Button, { BtnSizes, BtnTypes } from 'src/components/Button'
-import InLineNotification, { NotificationVariant } from 'src/components/InLineNotification'
+import { NotificationVariant } from 'src/components/InLineNotification'
 import SectionHead from 'src/components/SectionHead'
+import Toast from 'src/components/Toast'
 import { default as Attention, default as AttentionIcon } from 'src/icons/Attention'
 import LogoHeart from 'src/icons/LogoHeart'
 import { HistoryCardMetadata, useGetHistoryDefinition } from 'src/points/cardDefinitions'
@@ -60,8 +61,6 @@ function PointsHistoryBottomSheet({ forwardedRef }: Props) {
   const pointsHistory = useSelector(pointsHistorySelector)
   const hasNextPage = useSelector(nextPageUrlSelector)
 
-  const [showError, setShowError] = useState(false)
-
   const getHistoryDefinition = useGetHistoryDefinition()
 
   const insets = useSafeAreaInsets()
@@ -120,7 +119,7 @@ function PointsHistoryBottomSheet({ forwardedRef }: Props) {
     ) : null
 
   const EmptyOrError =
-    pointsHistoryStatus === 'error' ? (
+    pointsHistoryStatus === 'errorFirstPage' ? (
       <View testID={'PointsHistoryBottomSheet/Error'} style={styles.emptyContainer}>
         <View style={styles.messageContainer}>
           <Attention size={48} color={Colors.black} />
@@ -159,12 +158,6 @@ function PointsHistoryBottomSheet({ forwardedRef }: Props) {
     return groupFeedItemsInSections([], pointsHistory)
   }, [pointsHistory, pointsHistoryStatus])
 
-  useEffect(() => {
-    if (pointsHistory.length && pointsHistoryStatus === 'error') {
-      setShowError(true)
-    }
-  }, [pointsHistory, pointsHistoryStatus])
-
   return (
     <BottomSheetBase snapPoints={['80%']} forwardedRef={forwardedRef}>
       {!isEmpty && <Text style={styles.contentHeader}>{t('points.history.title')}</Text>}
@@ -187,29 +180,23 @@ function PointsHistoryBottomSheet({ forwardedRef }: Props) {
         onEndReachedThreshold={0.2}
         stickySectionHeadersEnabled={false}
       />
-      {showError && (
-        <InLineNotification
-          variant={NotificationVariant.Error}
-          title={t('points.history.pageError.title')}
-          description={t('points.history.pageError.subtitle')}
-          ctaLabel={t('points.history.pageError.refresh')}
-          onPressCta={() => onPressTryAgain(true)}
-          withBorder={true}
-          style={{
-            ...styles.errorNotification,
-            marginBottom: Math.max(insets.bottom, Spacing.Thick24),
-          }}
-          customIcon={<AttentionIcon color={colors.errorDark} size={20} />}
-          testID={'PointsHistoryBottomSheet/ErrorBanner'}
-        />
-      )}
+      <Toast
+        showToast={pointsHistoryStatus === 'errorNextPage'}
+        variant={NotificationVariant.Error}
+        title={t('points.history.pageError.title')}
+        description={t('points.history.pageError.subtitle')}
+        ctaLabel={t('points.history.pageError.refresh')}
+        onPressCta={() => onPressTryAgain(true)}
+        style={styles.errorNotification}
+        customIcon={<AttentionIcon color={colors.errorDark} size={20} />}
+        testID={'PointsHistoryBottomSheet/ErrorBanner'}
+      />
     </BottomSheetBase>
   )
 }
 
 const styles = StyleSheet.create({
   errorNotification: {
-    positioning: 'absolute',
     marginHorizontal: Spacing.Regular16,
   },
   emptyContainer: {

diff --git a/src/points/PointsHome.test.tsx b/src/points/PointsHome.test.tsx
@@ -7,36 +7,31 @@ import { navigate } from 'src/navigator/NavigationService'
 import { Screens } from 'src/navigator/Screens'
 import PointsHome from 'src/points/PointsHome'
 import { getHistoryStarted, getPointsConfigRetry } from 'src/points/slice'
-import { PointsActivityId } from 'src/points/types'
-import { createMockStore, getMockStackScreenProps } from 'test/utils'
+import { RootState } from 'src/redux/store'
+import { RecursivePartial, createMockStore, getMockStackScreenProps } from 'test/utils'
 
 jest.mock('src/points/PointsHistoryBottomSheet')
 
 const mockScreenProps = () => getMockStackScreenProps(Screens.PointsHome)
 
-const renderPointsHome = (
-  pointsConfigStatus: 'idle' | 'loading' | 'error' | 'success' = 'success',
-  activitiesById?: {
-    [activityId in PointsActivityId]?: {
-      pointsAmount: number
-    }
-  }
-) => {
-  const store = createMockStore({
-    points: {
-      pointsConfig: {
-        activitiesById: activitiesById ?? {
-          swap: {
-            pointsAmount: 50,
-          },
-          'create-wallet': {
-            pointsAmount: 20,
+const renderPointsHome = (storeOverrides?: RecursivePartial<RootState>) => {
+  const store = createMockStore(
+    storeOverrides ?? {
+      points: {
+        pointsConfig: {
+          activitiesById: {
+            swap: {
+              pointsAmount: 50,
+            },
+            'create-wallet': {
+              pointsAmount: 20,
+            },
           },
         },
+        pointsConfigStatus: 'success',
       },
-      pointsConfigStatus,
-    },
-  })
+    }
+  )
   const tree = render(
     <Provider store={store}>
       <PointsHome {...mockScreenProps()} />
@@ -55,7 +50,9 @@ describe(PointsHome, () => {
   })
 
   it('renders a loading state while loading config', async () => {
-    const { getByText, queryByText } = renderPointsHome('loading')
+    const { getByText, queryByText } = renderPointsHome({
+      points: { pointsConfigStatus: 'loading' },
+    })
 
     expect(getByText('points.loading.title')).toBeTruthy()
     expect(getByText('points.loading.description')).toBeTruthy()
@@ -64,7 +61,9 @@ describe(PointsHome, () => {
   })
 
   it('renders the error state on failure to load config', async () => {
-    const { getByText, queryByText, store } = renderPointsHome('error')
+    const { getByText, queryByText, store } = renderPointsHome({
+      points: { pointsConfigStatus: 'error' },
+    })
 
     expect(getByText('points.error.title')).toBeTruthy()
     expect(getByText('points.error.description')).toBeTruthy()
@@ -96,6 +95,24 @@ describe(PointsHome, () => {
     )
   })
 
+  it('displays an error toast on failure to refresh the balance and history', async () => {
+    const { store, getByText } = renderPointsHome({
+      points: {
+        getHistoryStatus: 'errorFirstPage',
+      },
+    })
+
+    expect(getByText('points.fetchBalanceError.title')).toBeTruthy()
+    expect(getByText('points.fetchBalanceError.description')).toBeTruthy()
+
+    store.clearActions()
+    fireEvent.press(getByText('points.fetchBalanceError.retryCta'))
+
+    await waitFor(() =>
+      expect(store.getActions()).toEqual([getHistoryStarted({ getNextPage: false })])
+    )
+  })
+
   it('opens activity bottom sheet', async () => {
     const { getByTestId, store } = renderPointsHome()
 
@@ -124,7 +141,11 @@ describe(PointsHome, () => {
   })
 
   it('renders only the balance if there are no supported activities', async () => {
-    const { getByTestId, getByText, queryByText } = renderPointsHome('success', {})
+    const { getByTestId, getByText, queryByText } = renderPointsHome({
+      points: {
+        pointsConfigStatus: 'success',
+      },
+    })
 
     expect(getByText('points.title')).toBeTruthy()
     expect(getByTestId('PointsBalance')).toBeTruthy() // balance is animated so we cannot properly test the value programatically

diff --git a/src/points/PointsHome.tsx b/src/points/PointsHome.tsx
@@ -11,6 +11,7 @@ import BottomSheet, { BottomSheetRefType } from 'src/components/BottomSheet'
 import Button, { BtnSizes, BtnTypes } from 'src/components/Button'
 import InLineNotification, { NotificationVariant } from 'src/components/InLineNotification'
 import NumberTicker from 'src/components/NumberTicker'
+import Toast from 'src/components/Toast'
 import CustomHeader from 'src/components/header/CustomHeader'
 import AttentionIcon from 'src/icons/Attention'
 import LogoHeart from 'src/icons/LogoHeart'
@@ -22,6 +23,7 @@ import {
   pointsBalanceSelector,
   pointsBalanceStatusSelector,
   pointsConfigStatusSelector,
+  pointsHistoryStatusSelector,
   pointsSectionsSelector,
 } from 'src/points/selectors'
 import { getHistoryStarted, getPointsConfigRetry } from 'src/points/slice'
@@ -42,6 +44,7 @@ export default function PointsHome({ route, navigation }: Props) {
   const pointsConfigStatus = useSelector(pointsConfigStatusSelector)
   const pointsBalance = useSelector(pointsBalanceSelector)
   const pointsBalanceStatus = useSelector(pointsBalanceStatusSelector)
+  const pointsHistoryStatus = useSelector(pointsHistoryStatusSelector)
 
   const lastKnownPointsBalance = useRef(pointsBalance)
   const historyBottomSheetRef = useRef<BottomSheetRefType>(null)
@@ -175,6 +178,14 @@ export default function PointsHome({ route, navigation }: Props) {
           </>
         )}
       </ScrollView>
+      <Toast
+        showToast={pointsBalanceStatus === 'error' || pointsHistoryStatus === 'errorFirstPage'}
+        variant={NotificationVariant.Warning}
+        title={t('points.fetchBalanceError.title')}
+        description={t('points.fetchBalanceError.description')}
+        ctaLabel={t('points.fetchBalanceError.retryCta')}
+        onPressCta={onRefreshHistoryAndBalance}
+      />
       <PointsHistoryBottomSheet forwardedRef={historyBottomSheetRef} />
       <BottomSheet forwardedRef={activityCardBottomSheetRef} testId={`PointsActivityBottomSheet`}>
         {bottomSheetParams && (

diff --git a/src/points/saga.test.ts b/src/points/saga.test.ts
@@ -146,7 +146,7 @@ describe('getHistory', () => {
       .withState(createMockStore({ web3: { account: null } }).getState())
       .put(
         getHistoryError({
-          resetHistory: true,
+          getNextPage: false,
         })
       )
       .run()
@@ -180,7 +180,7 @@ describe('getHistory', () => {
       .provide([[matchers.call.fn(fetchHistory), throwError(new Error('failure'))]])
       .put(
         getHistoryError({
-          resetHistory: true,
+          getNextPage: false,
         })
       )
       .run()

diff --git a/src/points/saga.ts b/src/points/saga.ts
@@ -98,7 +98,7 @@ export function* getHistory({ payload: params }: ReturnType<typeof getHistorySta
     Logger.error(TAG, 'No wallet address found when fetching points history')
     yield* put(
       getHistoryError({
-        resetHistory: !params.getNextPage,
+        getNextPage: params.getNextPage,
       })
     )
     return
@@ -133,7 +133,7 @@ export function* getHistory({ payload: params }: ReturnType<typeof getHistorySta
     Logger.error(TAG, 'Error fetching points history', e)
     yield* put(
       getHistoryError({
-        resetHistory: !params.getNextPage,
+        getNextPage: params.getNextPage,
       })
     )
   }

diff --git a/src/points/slice.ts b/src/points/slice.ts
@@ -13,7 +13,7 @@ interface GetPointsHistoryStartedAction {
 }
 
 interface GetPointsHistoryErrorAction {
-  resetHistory: boolean
+  getNextPage: boolean
 }
 
 export type PointsConfig = {
@@ -33,7 +33,7 @@ export interface PendingPointsEvent {
 export interface State {
   pointsHistory: ClaimHistory[]
   nextPageUrl: string | null
-  getHistoryStatus: 'idle' | 'loading' | 'error'
+  getHistoryStatus: 'idle' | 'loading' | 'errorFirstPage' | 'errorNextPage'
   pointsConfig: PointsConfig
   pointsConfigStatus: 'idle' | 'loading' | 'error' | 'success'
   pendingPointsEvents: PendingPointsEvent[]
@@ -70,8 +70,7 @@ const slice = createSlice({
     }),
     getHistoryError: (state, action: PayloadAction<GetPointsHistoryErrorAction>) => ({
       ...state,
-      getHistoryStatus: 'error',
-      pointsHistory: action.payload.resetHistory ? [] : state.pointsHistory,
+      getHistoryStatus: action.payload.getNextPage ? 'errorNextPage' : 'errorFirstPage',
     }),
     getPointsConfigStarted: (state) => ({
       ...state,

diff --git a/test/RootStateSchema.json b/test/RootStateSchema.json
@@ -4311,7 +4311,8 @@
             "properties": {
                 "getHistoryStatus": {
                     "enum": [
-                        "error",
+                        "errorFirstPage",
+                        "errorNextPage",
                         "idle",
                         "loading"
                     ],