Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: notifications renewal [SW-631] #4699

Merged
merged 28 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
250d2ae
feat: enhance notification link handling to support onClick actions
tmjssz Dec 20, 2024
adf93b6
feat: add `useNotificationsTokenVersion` hook to store the notificati…
tmjssz Dec 20, 2024
3dfac6a
feat: in notification registration flow update token version to be up…
tmjssz Dec 20, 2024
5eaa612
feat: add `useNotificationsRenewal` hook to manage notification renew…
tmjssz Dec 20, 2024
475171a
feat: integrate `useNotificationsRenewal` hook to display renewal mes…
tmjssz Dec 20, 2024
15289b3
feat: add `NotificationRenewal` component to handle notification rene…
tmjssz Dec 20, 2024
1471581
feat: merge Safes for notification registration and renewal in Global…
tmjssz Dec 20, 2024
69a17a9
feat: put notifications renewal behind a feature flag
tmjssz Dec 20, 2024
7ddc782
fix: linter warnings
tmjssz Dec 20, 2024
1bae52d
chore: update firebase dependency to version 11.1.0
tmjssz Dec 20, 2024
3bfe5bb
refactor: simplify passed styling props for Typography
tmjssz Dec 30, 2024
c28d2f0
refactor: replace mergeNotifiableSafes with _mergeNotifiableSafes and…
tmjssz Dec 30, 2024
3bb52bc
feat: handle errors in notifications renewal
tmjssz Dec 30, 2024
8e24f03
feat: split useNotificationsRenewal hook
tmjssz Dec 30, 2024
4aa0bc5
feat: add tests for getChainPreferences in useNotificationPreferences…
tmjssz Dec 30, 2024
1482c28
test: add mock for useNotificationsTokenVersion and verify token vers…
tmjssz Dec 30, 2024
0c86a65
feat: add tests for useNotificationsTokenVersion hook
tmjssz Dec 30, 2024
9428b9f
test: add tests for useNotificationsRenewal hook
tmjssz Dec 30, 2024
219a651
test: add tests for useShowNotificationsRenewalMessage hook
tmjssz Dec 30, 2024
9ad37bc
refactor: remove unused lodash import in PushNotifications logic
tmjssz Dec 30, 2024
3341601
fix: update dependency array in useNotificationsRenewal hook to inclu…
tmjssz Dec 30, 2024
c93846d
refactor: use property shorthand for chainId assignment
tmjssz Dec 30, 2024
36a5fa7
refactor: change renewal message + extract it to constants for better…
tmjssz Jan 7, 2025
f7ee1db
fix: show renewal notification again when switching the chain for a m…
tmjssz Jan 7, 2025
36a6075
fix: update snapshot files
tmjssz Jan 7, 2025
ee0f523
fix: update snapshots
tmjssz Jan 7, 2025
72fd18d
Merge branch 'dev' into feat/notifications-renewal
katspaugh Jan 8, 2025
94c917d
Update snapshots
katspaugh Jan 8, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"date-fns": "^2.30.0",
"ethers": "^6.13.4",
"exponential-backoff": "^3.1.0",
"firebase": "^10.3.1",
"firebase": "^11.1.0",
"fuse.js": "^7.0.0",
"idb-keyval": "^6.2.1",
"js-cookie": "^3.0.1",
Expand Down
33 changes: 26 additions & 7 deletions apps/web/src/components/common/Notifications/index.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import type { ReactElement, SyntheticEvent } from 'react'
import { useCallback, useEffect } from 'react'
import React, { useCallback, useEffect } from 'react'
import groupBy from 'lodash/groupBy'
import { useAppDispatch, useAppSelector } from '@/store'
import type { Notification } from '@/store/notificationsSlice'
import { closeNotification, readNotification, selectNotifications } from '@/store/notificationsSlice'
import type { AlertColor, SnackbarCloseReason } from '@mui/material'
import { Alert, Link, Snackbar, Typography } from '@mui/material'
import { Alert, Box, Link, Snackbar, Typography } from '@mui/material'
import css from './styles.module.css'
import NextLink from 'next/link'
import ChevronRightIcon from '@mui/icons-material/ChevronRight'
Expand All @@ -26,20 +26,39 @@ export const NotificationLink = ({
return null
}

const LinkWrapper = ({ children }: React.PropsWithChildren) =>
'href' in link ? (
<NextLink href={link.href} passHref legacyBehavior>
{children}
</NextLink>
) : (
<Box display="flex">{children}</Box>
)

const handleClick = (event: SyntheticEvent) => {
if ('onClick' in link) {
link.onClick()
}
onClick(event)
}

const isExternal =
typeof link.href === 'string' ? !isRelativeUrl(link.href) : !!(link.href.host || link.href.hostname)
'href' in link &&
(typeof link.href === 'string' ? !isRelativeUrl(link.href) : !!(link.href.host || link.href.hostname))

return (
<Track {...OVERVIEW_EVENTS.NOTIFICATION_INTERACTION} label={link.title} as="span">
<NextLink href={link.href} passHref legacyBehavior>
<LinkWrapper>
<Link
className={css.link}
onClick={onClick}
onClick={handleClick}
sx={{ cursor: 'pointer' }}
{...(isExternal && { target: '_blank', rel: 'noopener noreferrer' })}
>
{link.title} <ChevronRightIcon />
{link.title}
<ChevronRightIcon />
</Link>
</NextLink>
</LinkWrapper>
</Track>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { trackEvent, OVERVIEW_EVENTS } from '@/services/analytics'
import SvgIcon from '@mui/icons-material/ExpandLess'
import { useHasFeature } from '@/hooks/useChains'
import { FEATURES } from '@/utils/chains'
import { useShowNotificationsRenewalMessage } from '@/components/settings/PushNotifications/hooks/useShowNotificationsRenewalMessage'

const NOTIFICATION_CENTER_LIMIT = 4

Expand All @@ -38,6 +39,9 @@ const NotificationCenter = (): ReactElement => {
const hasPushNotifications = useHasFeature(FEATURES.PUSH_NOTIFICATIONS)
const dispatch = useAppDispatch()

// This hook is used to show the notification renewal message when the app is opened
useShowNotificationsRenewalMessage()

const notifications = useAppSelector(selectNotifications)
const chronologicalNotifications = useMemo(() => {
// Clone as Redux returns read-only array
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { useState, type ReactElement } from 'react'
import { Alert, Box, Button, Typography } from '@mui/material'
import useSafeInfo from '@/hooks/useSafeInfo'
import CheckWallet from '@/components/common/CheckWallet'
import { useNotificationsRenewal } from '@/components/settings/PushNotifications/hooks/useNotificationsRenewal'
import { useIsNotificationsRenewalEnabled } from '@/components/settings/PushNotifications/hooks/useNotificationsTokenVersion'
import { RENEWAL_MESSAGE } from '@/components/settings/PushNotifications/constants'

const NotificationRenewal = (): ReactElement => {
const { safe } = useSafeInfo()
const [isRegistering, setIsRegistering] = useState(false)
const { renewNotifications, needsRenewal } = useNotificationsRenewal()
const isNotificationsRenewalEnabled = useIsNotificationsRenewalEnabled()

if (!needsRenewal || !isNotificationsRenewalEnabled) {
// No need to renew any Safe's notifications
return <></>
}

const handeSignClick = async () => {
setIsRegistering(true)
await renewNotifications()
katspaugh marked this conversation as resolved.
Show resolved Hide resolved
setIsRegistering(false)
}

return (
<>
<Alert severity="warning">
<Typography variant="body2" fontWeight={700} mb={1}>
Signature needed
</Typography>
<Typography variant="body2">{RENEWAL_MESSAGE}</Typography>
</Alert>
<Box>
<CheckWallet allowNonOwner checkNetwork={!isRegistering && safe.deployed}>
{(isOk) => (
<Button
variant="contained"
size="small"
sx={{ width: '200px' }}
onClick={handeSignClick}
disabled={!isOk || isRegistering || !safe.deployed}
>
Sign now
</Button>
)}
</CheckWallet>
</Box>
</>
)
}

export default NotificationRenewal
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import useAllOwnedSafes from '@/features/myAccounts/hooks/useAllOwnedSafes'
import useWallet from '@/hooks/wallets/useWallet'
import { selectAllAddedSafes, type AddedSafesState } from '@/store/addedSafesSlice'
import { maybePlural } from '@/utils/formatters'
import { useNotificationsRenewal } from './hooks/useNotificationsRenewal'

// UI logic

Expand Down Expand Up @@ -268,6 +269,8 @@ export const GlobalPushNotifications = (): ReactElement | null => {
const { unregisterDeviceNotifications, unregisterSafeNotifications, registerNotifications } =
useNotificationRegistrations()

const { safesForRenewal } = useNotificationsRenewal()

// Safes selected in the UI
const [selectedSafes, setSelectedSafes] = useState<NotifiableSafes>({})

Expand Down Expand Up @@ -349,7 +352,11 @@ export const GlobalPushNotifications = (): ReactElement | null => {

const registrationPromises: Array<Promise<unknown>> = []

const safesToRegister = _getSafesToRegister(selectedSafes, currentNotifiedSafes)
const newlySelectedSafes = _getSafesToRegister(selectedSafes, currentNotifiedSafes)

// Merge Safes that need to be registered with the ones for which notifications need to be renewed
const safesToRegister = _mergeNotifiableSafes(newlySelectedSafes, {}, safesForRenewal)

if (safesToRegister) {
registrationPromises.push(registerNotifications(safesToRegister))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const RENEWAL_NOTIFICATION_KEY = 'renewal'
export const RENEWAL_MESSAGE =
'We’ve upgraded your notification experience! To continue receiving important updates seamlessly, you’ll need to sign this message on every chain you use.'
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,71 @@ describe('useNotificationPreferences', () => {
})
})

describe('getChainPreferences', () => {
const chainId1 = '1'
const safeAddress1 = toBeHex('0x1', 20)
const safeAddress2 = toBeHex('0x2', 20)

const chainId2 = '2'

const preferences = {
[`${chainId1}:${safeAddress1}`]: {
chainId: chainId1,
safeAddress: safeAddress1,
preferences: DEFAULT_NOTIFICATION_PREFERENCES,
},
[`${chainId1}:${safeAddress2}`]: {
chainId: chainId1,
safeAddress: safeAddress2,
preferences: DEFAULT_NOTIFICATION_PREFERENCES,
},
[`${chainId2}:${safeAddress1}`]: {
chainId: chainId2,
safeAddress: safeAddress1,
preferences: DEFAULT_NOTIFICATION_PREFERENCES,
},
}

it('should return existing chain preferences', async () => {
await setMany(Object.entries(preferences), createPushNotificationPrefsIndexedDb())

const { result } = renderHook(() => useNotificationPreferences())

await waitFor(() => {
expect(result.current.getChainPreferences(chainId1)).toEqual([
{
chainId: chainId1,
safeAddress: safeAddress1,
preferences: DEFAULT_NOTIFICATION_PREFERENCES,
},
{
chainId: chainId1,
safeAddress: safeAddress2,
preferences: DEFAULT_NOTIFICATION_PREFERENCES,
},
])
})
})

it('should return an empty array if no preferences exist for the chain', async () => {
await setMany(Object.entries(preferences), createPushNotificationPrefsIndexedDb())

const { result } = renderHook(() => useNotificationPreferences())

await waitFor(() => {
expect(result.current.getChainPreferences('3')).toEqual([])
})
})

it('should return an empty array if no preferences exist', async () => {
const { result } = renderHook(() => useNotificationPreferences())

await waitFor(() => {
expect(result.current.getChainPreferences('1')).toEqual([])
})
})
})

describe('createPreferences', () => {
it('should create preferences, then hydrate the preferences state', async () => {
const { result } = renderHook(() => useNotificationPreferences())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@ import * as web3 from '@/hooks/wallets/web3'
import * as wallet from '@/hooks/wallets/useWallet'
import * as logic from '../../logic'
import * as preferences from '../useNotificationPreferences'
import * as tokenVersion from '../useNotificationsTokenVersion'
import * as notificationsSlice from '@/store/notificationsSlice'
import type { ConnectedWallet } from '@/hooks/wallets/useOnboard'
import { MockEip1193Provider } from '@/tests/mocks/providers'
import { NotificationsTokenVersion } from '@/services/push-notifications/preferences'

jest.mock('@safe-global/safe-gateway-typescript-sdk')

jest.mock('../useNotificationPreferences')
jest.mock('../useNotificationsTokenVersion')

Object.defineProperty(globalThis, 'crypto', {
value: {
Expand All @@ -28,9 +31,19 @@ describe('useNotificationRegistrations', () => {
})

describe('registerNotifications', () => {
const setTokenVersionMock = jest.fn()

beforeEach(() => {
const mockProvider = new BrowserProvider(MockEip1193Provider)
jest.spyOn(web3, 'createWeb3').mockImplementation(() => mockProvider)
jest
.spyOn(tokenVersion, 'useNotificationsTokenVersion')
.mockImplementation(
() =>
({ setTokenVersion: setTokenVersionMock }) as unknown as ReturnType<
typeof tokenVersion.useNotificationsTokenVersion
>,
)
jest.spyOn(wallet, 'default').mockImplementation(
() =>
({
Expand Down Expand Up @@ -83,6 +96,7 @@ describe('useNotificationRegistrations', () => {
await result.current.registerNotifications({})

expect(registerDeviceSpy).not.toHaveBeenCalled()
expect(setTokenVersionMock).not.toHaveBeenCalled()
})

it('does not create preferences/notify if registration does not succeed', async () => {
Expand Down Expand Up @@ -115,6 +129,7 @@ describe('useNotificationRegistrations', () => {
expect(registerDeviceSpy).toHaveBeenCalledWith(payload)

expect(createPreferencesMock).not.toHaveBeenCalled()
expect(setTokenVersionMock).not.toHaveBeenCalled()
})

it('does not create preferences/notify if registration throws', async () => {
Expand Down Expand Up @@ -147,6 +162,7 @@ describe('useNotificationRegistrations', () => {
expect(registerDeviceSpy).toHaveBeenCalledWith(payload)

expect(createPreferencesMock).not.toHaveBeenCalledWith()
expect(setTokenVersionMock).not.toHaveBeenCalledWith()
})

it('creates preferences/notifies if registration succeeded', async () => {
Expand Down Expand Up @@ -181,6 +197,9 @@ describe('useNotificationRegistrations', () => {

expect(createPreferencesMock).toHaveBeenCalled()

expect(setTokenVersionMock).toHaveBeenCalledTimes(1)
expect(setTokenVersionMock).toHaveBeenCalledWith(NotificationsTokenVersion.V2, safesToRegister)

expect(showNotificationSpy).toHaveBeenCalledWith({
groupKey: 'notifications',
message: 'You will now receive notifications for these Safe Accounts in your browser.',
Expand Down
Loading
Loading