fix: track banner once (#2598)

* fix: track banner once * fix: rename event * fix: add test
protofire · Oct 6, 2023 · a1ea200 · a1ea200
1 parent bcd8354
commit a1ea200
Show file tree

Hide file tree

Showing 3 changed files with 124 additions and 61 deletions.
diff --git a/...nents/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.tsx b/...nents/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.tsx
@@ -1,6 +1,7 @@
 import 'fake-indexeddb/auto'
-import { set } from 'idb-keyval'
 import { hexZeroPad } from 'ethers/lib/utils'
+import * as tracking from '@/services/analytics'
+import { set } from 'idb-keyval'
 import * as navigation from 'next/navigation'
 import type { ChainInfo, SafeInfo } from '@safe-global/safe-gateway-typescript-sdk'
 
@@ -91,6 +92,56 @@ describe('PushNotificationsBanner', () => {
       })
     })
 
+    it('should only track display of the banner once', () => {
+      jest.spyOn(tracking, 'trackEvent')
+
+      const ui = (
+        <PushNotificationsBanner>
+          <></>
+        </PushNotificationsBanner>
+      )
+
+      const result = render(ui, {
+        routerProps: {
+          query: {
+            safe: `eth:${hexZeroPad('0x123', 20)}`,
+          },
+        },
+        initialReduxState: {
+          chains: {
+            loading: false,
+            error: undefined,
+            data: [
+              {
+                chainId: '1',
+                features: ['PUSH_NOTIFICATIONS'],
+              } as unknown as ChainInfo,
+            ],
+          },
+          addedSafes: {
+            '1': {
+              [hexZeroPad('0x123', 20)]: {},
+            } as unknown as AddedSafesOnChain,
+          },
+          safeInfo: {
+            loading: false,
+            error: undefined,
+            data: {
+              chainId: '1',
+              address: {
+                value: hexZeroPad('0x123', 20),
+              },
+            } as unknown as SafeInfo,
+          },
+        },
+      })
+
+      expect(tracking.trackEvent).toHaveBeenCalledTimes(1)
+
+      result.rerender(ui)
+
+      expect(tracking.trackEvent).toHaveBeenCalledTimes(1)
+    })
     it('should display the banner', () => {
       const result = render(
         <PushNotificationsBanner>

diff --git a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx
@@ -1,7 +1,7 @@
 import { Button, Chip, Grid, SvgIcon, Typography, IconButton } from '@mui/material'
 import Link from 'next/link'
 import { useRouter } from 'next/router'
-import { useCallback, useEffect } from 'react'
+import { useCallback, useEffect, useRef } from 'react'
 import type { ReactElement } from 'react'
 
 import { CustomTooltip } from '@/components/common/CustomTooltip'
@@ -86,6 +86,21 @@ export const _getSafesToRegister = (
   return { [chainId]: newlyAddedSafes }
 }
 
+const TrackBanner = (): null => {
+  const hasTracked = useRef(false)
+
+  useEffect(() => {
+    if (hasTracked.current) {
+      return
+    }
+
+    trackEvent(PUSH_NOTIFICATION_EVENTS.SHOW_BANNER)
+    hasTracked.current = true
+  }, [])
+
+  return null
+}
+
 export const PushNotificationsBanner = ({ children }: { children: ReactElement }): ReactElement => {
   const isNotificationFeatureEnabled = useHasFeature(FEATURES.PUSH_NOTIFICATIONS)
   const chain = useCurrentChain()
@@ -110,12 +125,6 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement }
     dismissPushNotificationBanner(safe.chainId)
   }, [dismissPushNotificationBanner, safe.chainId])
 
-  useEffect(() => {
-    if (shouldShowBanner) {
-      trackEvent(PUSH_NOTIFICATION_EVENTS.DISPLAY_BANNER)
-    }
-  }, [dismissBanner, shouldShowBanner])
-
   const onEnableAll = async () => {
     if (!onboard || !addedSafesOnChain) {
       return
@@ -148,57 +157,60 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement }
   }
 
   return (
-    <CustomTooltip
-      className={css.banner}
-      title={
-        <Grid container className={css.container}>
-          <Grid item xs={3}>
-            <Chip label="New" className={css.chip} />
-            <SvgIcon component={PushNotificationIcon} inheritViewBox fontSize="inherit" className={css.icon} />
-          </Grid>
-          <Grid item xs={9}>
-            <Typography variant="subtitle2" fontWeight={700}>
-              Enable push notifications
-            </Typography>
-            <IconButton onClick={dismissBanner} className={css.close}>
-              <SvgIcon component={CloseIcon} inheritViewBox color="border" fontSize="small" />
-            </IconButton>
-            <Typography mt={0.5} mb={1.5} variant="body2">
-              Get notified about pending signatures, incoming and outgoing transactions for all Safe Accounts on{' '}
-              {chain?.chainName} when Safe
-              {`{Wallet}`} is in the background or closed.
-            </Typography>
-            {/* Cannot wrap singular button as it causes style inconsistencies */}
-            <CheckWallet>
-              {(isOk) => (
-                <div className={css.buttons}>
-                  {totalAddedSafes > 0 && (
-                    <Button
-                      variant="contained"
-                      size="small"
-                      className={css.button}
-                      onClick={onEnableAll}
-                      disabled={!isOk || !onboard}
-                    >
-                      Enable all
-                    </Button>
-                  )}
-                  {safe && (
-                    <Link passHref href={{ pathname: AppRoutes.settings.notifications, query }} onClick={onCustomize}>
-                      <Button variant="outlined" size="small" className={css.button}>
-                        Customize
+    <>
+      <TrackBanner />
+      <CustomTooltip
+        className={css.banner}
+        title={
+          <Grid container className={css.container}>
+            <Grid item xs={3}>
+              <Chip label="New" className={css.chip} />
+              <SvgIcon component={PushNotificationIcon} inheritViewBox fontSize="inherit" className={css.icon} />
+            </Grid>
+            <Grid item xs={9}>
+              <Typography variant="subtitle2" fontWeight={700}>
+                Enable push notifications
+              </Typography>
+              <IconButton onClick={dismissBanner} className={css.close}>
+                <SvgIcon component={CloseIcon} inheritViewBox color="border" fontSize="small" />
+              </IconButton>
+              <Typography mt={0.5} mb={1.5} variant="body2">
+                Get notified about pending signatures, incoming and outgoing transactions for all Safe Accounts on{' '}
+                {chain?.chainName} when Safe
+                {`{Wallet}`} is in the background or closed.
+              </Typography>
+              {/* Cannot wrap singular button as it causes style inconsistencies */}
+              <CheckWallet>
+                {(isOk) => (
+                  <div className={css.buttons}>
+                    {totalAddedSafes > 0 && (
+                      <Button
+                        variant="contained"
+                        size="small"
+                        className={css.button}
+                        onClick={onEnableAll}
+                        disabled={!isOk || !onboard}
+                      >
+                        Enable all
                       </Button>
-                    </Link>
-                  )}
-                </div>
-              )}
-            </CheckWallet>
+                    )}
+                    {safe && (
+                      <Link passHref href={{ pathname: AppRoutes.settings.notifications, query }} onClick={onCustomize}>
+                        <Button variant="outlined" size="small" className={css.button}>
+                          Customize
+                        </Button>
+                      </Link>
+                    )}
+                  </div>
+                )}
+              </CheckWallet>
+            </Grid>
           </Grid>
-        </Grid>
-      }
-      open
-    >
-      <span>{children}</span>
-    </CustomTooltip>
+        }
+        open
+      >
+        <span>{children}</span>
+      </CustomTooltip>
+    </>
   )
 }
diff --git a/src/services/analytics/events/push-notifications.ts b/src/services/analytics/events/push-notifications.ts
@@ -26,9 +26,9 @@ export const PUSH_NOTIFICATION_EVENTS = {
     action: 'Unregister device notifications',
     category,
   },
-  // Notification banner displayed
-  DISPLAY_BANNER: {
-    action: 'Display notification banner',
+  // Notification banner shown
+  SHOW_BANNER: {
+    action: 'Show notification banner',
     category,
   },
   // User dismissed notfication banner